From 1a4f5cbade8cfb8bcd883ccd8684bc224f5a49f8 Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Sat, 29 Aug 2026 07:34:32 -0400 Subject: [PATCH 1/2] gate 3: snapshot the backlog once, and prove the steps agree (PR #20 P2s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review refinements from PR #20, plus the test its DoD asked for. The flow re-read ops/BACKLOG.md in every step, so a backlog edit between select-entry and emit-package produced a package describing an entry that was never selected — a Garden reporting work it did not choose. read-backlog now snapshots the file once and the later steps read the snapshot and the selected entry, so the steps cannot disagree. Package metadata is derived from the selected entry rather than hardcoded. The test runs the flow's ACTUAL shell commands, not a reimplementation — a test of a paraphrase would pass while the flow stayed broken — and mutates the backlog mid-run to force the condition. Confirmed it FAILS against the unfixed flow before trusting it: × expected '{"title":"Swapped entry"...}' to contain 'Original entry' and passes against the fixed one. A regression test never seen to fail proves nothing; PR #18 is still carrying exactly that gap. Verified: sdk 159 passed across 12 files, tsc --noEmit clean. Co-Authored-By: Claude Fable 5 --- sdk/package-lock.json | 42 +++++++++++++-- sdk/package.json | 2 + sdk/tests/backlog-picker-flow.test.ts | 59 +++++++++++++++++++++ testdata/backlog-picker.flow.yaml | 6 +-- testdata/backlog-picker.spec.canonical.json | 2 +- 5 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 sdk/tests/backlog-picker-flow.test.ts diff --git a/sdk/package-lock.json b/sdk/package-lock.json index b905dd02..44ace6f1 100644 --- a/sdk/package-lock.json +++ b/sdk/package-lock.json @@ -8,12 +8,14 @@ "name": "@relayflows/sdk", "version": "0.1.0", "license": "UNLICENSED", - "bin": { - "flows": "dist/cli.js" - }, "dependencies": { + "@types/js-yaml": "^4.0.9", + "js-yaml": "^5.4.1", "yaml": "^2.5.1" }, + "bin": { + "flows": "dist/cli.js" + }, "devDependencies": { "@types/node": "^22.7.0", "typescript": "^5.6.0", @@ -792,6 +794,12 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/js-yaml": { + "version": "4.0.9", + "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", + "integrity": "sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==", + "license": "MIT" + }, "node_modules/@types/node": { "version": "22.20.1", "resolved": "https://registry.npmjs.org/@types/node/-/node-22.20.1.tgz", @@ -915,6 +923,12 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/argparse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", + "license": "Python-2.0" + }, "node_modules/assertion-error": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/assertion-error/-/assertion-error-2.0.1.tgz", @@ -1071,6 +1085,28 @@ "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } }, + "node_modules/js-yaml": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-5.4.1.tgz", + "integrity": "sha512-28R/k+NAjeuf7+CKlTxWZVExJGwVVLwY06DgEnOMz2gEpfNkDcD7QvyiVPT0xy0XXhU8vHsd4Ot42OOPdJG7dQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], + "license": "MIT", + "dependencies": { + "argparse": "^2.0.1" + }, + "bin": { + "js-yaml": "bin/js-yaml.mjs" + } + }, "node_modules/loupe": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/loupe/-/loupe-3.2.1.tgz", diff --git a/sdk/package.json b/sdk/package.json index 5bc78998..8be48996 100644 --- a/sdk/package.json +++ b/sdk/package.json @@ -28,6 +28,8 @@ "license": "UNLICENSED", "private": true, "dependencies": { + "@types/js-yaml": "^4.0.9", + "js-yaml": "^5.4.1", "yaml": "^2.5.1" }, "devDependencies": { diff --git a/sdk/tests/backlog-picker-flow.test.ts b/sdk/tests/backlog-picker-flow.test.ts new file mode 100644 index 00000000..e79f3249 --- /dev/null +++ b/sdk/tests/backlog-picker-flow.test.ts @@ -0,0 +1,59 @@ +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { load } from 'js-yaml'; +import { describe, expect, it } from 'vitest'; + +/** + * The backlog-picker flow's steps must agree with each other. + * + * Originally every step re-read ops/BACKLOG.md, so if the file changed between + * `select-entry` and `emit-package` the emitted package would describe an entry + * that was never selected — a Garden reporting work it did not choose. Review + * flagged it (PR #20, P2) and the fix snapshots the backlog once in + * `read-backlog`; this test is the proof that was asked for and not delivered. + * + * It runs the flow's ACTUAL shell commands rather than a reimplementation. A + * test of a paraphrase would pass while the flow stayed broken. + */ +function stepCommands(): Record { + const flowPath = join(__dirname, '..', '..', 'testdata', 'backlog-picker.flow.yaml'); + const flow = load(readFileSync(flowPath, 'utf8')) as { steps: Array<{ id: string; command: string }> }; + return Object.fromEntries(flow.steps.map((s) => [s.id, s.command])); +} + +function run(command: string, cwd: string): string { + return execFileSync('sh', ['-c', command], { cwd, encoding: 'utf8' }); +} + +describe('backlog-picker flow', () => { + it('emit-package describes the entry select-entry chose, even if the backlog changes between them', () => { + const dir = mkdtempSync(join(tmpdir(), 'backlog-picker-')); + try { + mkdirSync(join(dir, 'ops'), { recursive: true }); + writeFileSync( + join(dir, 'ops', 'BACKLOG.md'), + '# Backlog\n\n- **Original entry** the one that must win\n', + ); + + const steps = stepCommands(); + run(steps['read-backlog'], dir); + + const selected = run(steps['select-entry'], dir).trim(); + expect(selected).toContain('Original entry'); + + // The backlog changes underneath the flow, mid-run. + writeFileSync( + join(dir, 'ops', 'BACKLOG.md'), + '# Backlog\n\n- **Swapped entry** must NOT appear in the package\n', + ); + + const emitted = run(steps['emit-package'], dir); + expect(emitted).toContain('Original entry'); + expect(emitted).not.toContain('Swapped entry'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/testdata/backlog-picker.flow.yaml b/testdata/backlog-picker.flow.yaml index 037e057f..baa5873c 100644 --- a/testdata/backlog-picker.flow.yaml +++ b/testdata/backlog-picker.flow.yaml @@ -6,14 +6,14 @@ description: Read ops/BACKLOG.md and deterministically emit its first work packa steps: - id: read-backlog type: deterministic - command: "cat ops/BACKLOG.md" + command: "mkdir -p .relayflow && cat ops/BACKLOG.md > .relayflow/backlog-picker-source.md" - id: select-entry type: deterministic dependsOn: [read-backlog] command: >- - node -e 'const fs=require("node:fs");const text=fs.readFileSync("ops/BACKLOG.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);process.stdout.write(match[1])' + node -e 'const fs=require("node:fs");const text=fs.readFileSync(".relayflow/backlog-picker-source.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);const entry={title:match[1],body:match[2].replace(/\s+/g," ").trim()};fs.writeFileSync(".relayflow/backlog-picker-entry.json",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry))' - id: emit-package type: deterministic dependsOn: [select-entry] command: >- - node -e 'const fs=require("node:fs");const text=fs.readFileSync("ops/BACKLOG.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);process.stdout.write(JSON.stringify({title:match[1],description:match[2].replace(/\s+/g," ").trim(),files_in_scope:["sdk/src/preflight.ts","sdk/tests/preflight.test.ts"],gate:3}))' + node -e 'const fs=require("node:fs");const entry=JSON.parse(fs.readFileSync(".relayflow/backlog-picker-entry.json","utf8"));const text=entry.title+" "+entry.body;const files=[...new Set([...text.matchAll(/`([^`\s]*\/[^`]*)`/g)].map(match=>match[1]))];const gate=text.match(/\bgate[ -]?(\d+)\b/i);process.stdout.write(JSON.stringify({title:entry.title,description:entry.body,files_in_scope:files,gate:gate?Number(gate[1]):null}))' diff --git a/testdata/backlog-picker.spec.canonical.json b/testdata/backlog-picker.spec.canonical.json index c60298d7..874d6c1a 100644 --- a/testdata/backlog-picker.spec.canonical.json +++ b/testdata/backlog-picker.spec.canonical.json @@ -1 +1 @@ -{"description":"Read ops/BACKLOG.md and deterministically emit its first work package.","name":"backlog-picker","steps":[{"command":"cat ops/BACKLOG.md","depends_on":[],"id":"read-backlog","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}},{"command":"node -e 'const fs=require(\"node:fs\");const text=fs.readFileSync(\"ops/BACKLOG.md\",\"utf8\");const match=text.match(/^- \\*\\*(.+?)\\*\\*\\s*(.*(?:\\n .*)*)/m);if(!match)process.exit(1);process.stdout.write(match[1])'","depends_on":["read-backlog"],"id":"select-entry","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}},{"command":"node -e 'const fs=require(\"node:fs\");const text=fs.readFileSync(\"ops/BACKLOG.md\",\"utf8\");const match=text.match(/^- \\*\\*(.+?)\\*\\*\\s*(.*(?:\\n .*)*)/m);if(!match)process.exit(1);process.stdout.write(JSON.stringify({title:match[1],description:match[2].replace(/\\s+/g,\" \").trim(),files_in_scope:[\"sdk/src/preflight.ts\",\"sdk/tests/preflight.test.ts\"],gate:3}))'","depends_on":["select-entry"],"id":"emit-package","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}}],"version":"0.1.0"} +{"description":"Read ops/BACKLOG.md and deterministically emit its first work package.","name":"backlog-picker","steps":[{"command":"mkdir -p .relayflow && cat ops/BACKLOG.md > .relayflow/backlog-picker-source.md","depends_on":[],"id":"read-backlog","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}},{"command":"node -e 'const fs=require(\"node:fs\");const text=fs.readFileSync(\".relayflow/backlog-picker-source.md\",\"utf8\");const match=text.match(/^- \\*\\*(.+?)\\*\\*\\s*(.*(?:\\n .*)*)/m);if(!match)process.exit(1);const entry={title:match[1],body:match[2].replace(/\\s+/g,\" \").trim()};fs.writeFileSync(\".relayflow/backlog-picker-entry.json\",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry))'","depends_on":["read-backlog"],"id":"select-entry","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}},{"command":"node -e 'const fs=require(\"node:fs\");const entry=JSON.parse(fs.readFileSync(\".relayflow/backlog-picker-entry.json\",\"utf8\"));const text=entry.title+\" \"+entry.body;const files=[...new Set([...text.matchAll(/`([^`\\s]*\\/[^`]*)`/g)].map(match=>match[1]))];const gate=text.match(/\\bgate[ -]?(\\d+)\\b/i);process.stdout.write(JSON.stringify({title:entry.title,description:entry.body,files_in_scope:files,gate:gate?Number(gate[1]):null}))'","depends_on":["select-entry"],"id":"emit-package","max_iterations":1,"retry":{"initial_backoff_ms":100,"jitter_percent":20,"max_backoff_ms":60000,"multiplier":2},"type":"deterministic","verification":{}}],"version":"0.1.0"} From 2b9cddeb149f690cb0ef8a5db1c93fa9d21b25c0 Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Sat, 29 Aug 2026 07:52:47 -0400 Subject: [PATCH 2/2] fix: clear the selected entry before selecting, so no run inherits the last one (PR #21 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own fix created this. Snapshotting the backlog stopped the two steps disagreeing within a run, but it did so with shared persistent state — and shared state leaks across runs. If select-entry finds nothing actionable it exits before writing, so emit-package read the PREVIOUS run's entry and presented it as this run's choice. A Garden confidently proposing yesterday's work as today's. select-entry now removes .relayflow/backlog-picker-entry.json before it attempts selection, so a failed selection leaves nothing behind to inherit. Confirmed the test FAILS without the fix before trusting it: × expected '{"title":"Yesterday entry"...}' not to contain 'Yesterday entry' Verified: sdk 160 passed across 12 files. The P2 (the files_in_scope regex matching backticked prose that contains a slash) is not addressed — real, cosmetic, and safe to take next. Co-Authored-By: Claude Fable 5 --- sdk/tests/backlog-picker-flow.test.ts | 35 +++++++++++++++++++++++++++ testdata/backlog-picker.flow.yaml | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/sdk/tests/backlog-picker-flow.test.ts b/sdk/tests/backlog-picker-flow.test.ts index e79f3249..a7ad7064 100644 --- a/sdk/tests/backlog-picker-flow.test.ts +++ b/sdk/tests/backlog-picker-flow.test.ts @@ -56,4 +56,39 @@ describe('backlog-picker flow', () => { rmSync(dir, { recursive: true, force: true }); } }); + + it('does not emit a stale entry left by a previous run', () => { + // The snapshot fix introduced shared persistent state, and shared state + // leaks across runs: if select-entry finds nothing actionable it exits + // before writing, so emit-package would happily read the PREVIOUS run's + // entry and present it as this run's choice. Review caught it (PR #21, P1) + // — a fix for cross-step disagreement that created cross-run staleness. + const dir = mkdtempSync(join(tmpdir(), 'backlog-picker-stale-')); + try { + mkdirSync(join(dir, 'ops'), { recursive: true }); + const steps = stepCommands(); + + // Run one: a real entry, which populates the shared state. + writeFileSync(join(dir, 'ops', 'BACKLOG.md'), '# Backlog\n\n- **Yesterday entry** old\n'); + run(steps['read-backlog'], dir); + run(steps['select-entry'], dir); + expect(run(steps['emit-package'], dir)).toContain('Yesterday entry'); + + // Run two: nothing actionable. select-entry must fail AND must not leave + // the previous choice behind for emit-package to pick up. + writeFileSync(join(dir, 'ops', 'BACKLOG.md'), '# Backlog\n\nnothing actionable today\n'); + run(steps['read-backlog'], dir); + expect(() => run(steps['select-entry'], dir)).toThrow(); + + let emitted = ''; + try { + emitted = run(steps['emit-package'], dir); + } catch { + return; // Failing outright is the correct outcome. + } + expect(emitted).not.toContain('Yesterday entry'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/testdata/backlog-picker.flow.yaml b/testdata/backlog-picker.flow.yaml index baa5873c..030f216b 100644 --- a/testdata/backlog-picker.flow.yaml +++ b/testdata/backlog-picker.flow.yaml @@ -11,7 +11,7 @@ steps: type: deterministic dependsOn: [read-backlog] command: >- - node -e 'const fs=require("node:fs");const text=fs.readFileSync(".relayflow/backlog-picker-source.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);const entry={title:match[1],body:match[2].replace(/\s+/g," ").trim()};fs.writeFileSync(".relayflow/backlog-picker-entry.json",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry))' + node -e 'const fs=require("node:fs");fs.rmSync(".relayflow/backlog-picker-entry.json",{force:true});const text=fs.readFileSync(".relayflow/backlog-picker-source.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);const entry={title:match[1],body:match[2].replace(/\s+/g," ").trim()};fs.writeFileSync(".relayflow/backlog-picker-entry.json",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry))' - id: emit-package type: deterministic dependsOn: [select-entry]