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..a7ad7064 --- /dev/null +++ b/sdk/tests/backlog-picker-flow.test.ts @@ -0,0 +1,94 @@ +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 }); + } + }); + + 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 037e057f..030f216b 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");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] 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"}