From d8f15e0c8dbcb5c86bf991163809e7e21ca769ab Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Sat, 29 Aug 2026 16:23:01 -0400 Subject: [PATCH 1/2] test: guard the canonical spec's SHAPE, not just its commands The existing drift test compares only `command` strings between the flow yaml and the canonical spec. That is why PR #30's defect reached main and stayed there: it copied `dependsOn` from the yaml when the kernel reads `depends_on`, so the `build-sdk` step it added arrived at the kernel with no dependencies, no retry policy, no verification and no iteration cap, and two other steps carried a stray camelCase alias beside the real key. Every command matched, so the check passed the whole time. PR #35 cleaned it up; nothing stopped it recurring. This adds the missing guard: every canonical step must carry the same field set as its siblings, and no step may carry an authoring-surface camelCase key the kernel does not read. Confirmed to FAIL against both variants of the original bug, by reintroducing them into the canonical spec and running it: build-sdk missing the kernel fields: step "build-sdk" has a different field set than "read-backlog": expected 'command,dependsOn,id,type' to be 'command,depends_on,...' select-entry carrying the stray alias: step "select-entry" has a different field set than "read-backlog": expected 'command,dependsOn,depends_on,id,max_i...' to be 'command,d...' Verified: sdk 182 passed (13 files), tsc clean, canonical spec byte-restored. Co-Authored-By: Claude Fable 5 --- sdk/tests/backlog-picker-flow.test.ts | 34 +++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/sdk/tests/backlog-picker-flow.test.ts b/sdk/tests/backlog-picker-flow.test.ts index 4fcd1ec04..cef972bed 100644 --- a/sdk/tests/backlog-picker-flow.test.ts +++ b/sdk/tests/backlog-picker-flow.test.ts @@ -124,6 +124,40 @@ describe('backlog-picker canonical spec', () => { } }); + it('gives every canonical step the same field set, in the kernel\'s own names', () => { + // Comparing only `command` was not enough. PR #30 regenerated this file by + // copying `dependsOn` straight from the yaml, but the kernel reads + // `depends_on` — so the step that change added reached the kernel with no + // dependencies, no retry policy, no verification and no iteration cap, + // while two other steps carried a stray camelCase key alongside the real + // one. Every command matched, so the check above passed throughout. PR #35 + // cleaned it up; this is the guard that would have caught it. + // + // The rule is shape, not content: whatever fields the kernel-authored + // steps carry, every step must carry, and no step may carry an authoring + // -surface alias the kernel does not read. + const root = join(__dirname, '..', '..'); + const canonical = JSON.parse( + readFileSync(join(root, 'testdata', 'backlog-picker.spec.canonical.json'), 'utf8'), + ) as { steps: Array> }; + + expect(canonical.steps.length).toBeGreaterThan(1); + const fieldSets = canonical.steps.map((step) => Object.keys(step).sort().join(',')); + const expected = fieldSets[0]; + canonical.steps.forEach((step, index) => { + expect( + fieldSets[index], + `step "${String(step['id'])}" has a different field set than "${String(canonical.steps[0]?.['id'])}"`, + ).toBe(expected); + }); + + // camelCase is the authoring surface's spelling; the kernel reads snake_case. + for (const step of canonical.steps) { + const camel = Object.keys(step).filter((key) => /[a-z][A-Z]/.test(key)); + expect(camel, `step "${String(step['id'])}" carries authoring-surface keys`).toEqual([]); + } + }); + it('keeps directories and extensionless paths, and rejects prose', () => { const steps = stepCommands(); const dir = mkdtempSync(join(tmpdir(), 'backlog-scope-')); From 6bf0b601ebc450d27943b44a674cb51bcd43deb2 Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Sat, 29 Aug 2026 16:32:38 -0400 Subject: [PATCH 2/2] test: assert canonical dependencies against the yaml, not against sibling steps (PR #37 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the hole and it is a real one: comparing field sets between steps only catches an INCONSISTENT regeneration. Drop `depends_on` from every step at once and all sets still match, none are camelCase, both existing checks pass — while the kernel loses the entire dependency graph and runs the steps in whatever order it likes. So compare against the authority. The yaml declares the dependencies; the canonical spec must carry the same ones under the kernel's `depends_on`. That asserts a relationship rather than a field list, so it does not go stale when the kernel's step schema grows — which was my reason for avoiding a hardcoded list in the first place, and is satisfied better this way. Also guards itself: if the yaml ever stops declaring dependencies the test would assert nothing and pass, so it fails instead. Confirmed to FAIL against the exact case review described — depends_on removed from ALL steps: step "build-sdk" loses the dependencies the yaml declares: expected undefined to deeply equal [ 'read-backlog' ] The sibling-consistency check passed in that same run, which is the point. Verified: sdk 183 passed (13 files), tsc clean, canonical spec byte-restored. Co-Authored-By: Claude Fable 5 --- sdk/tests/backlog-picker-flow.test.ts | 34 +++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/sdk/tests/backlog-picker-flow.test.ts b/sdk/tests/backlog-picker-flow.test.ts index cef972bed..af7ead69a 100644 --- a/sdk/tests/backlog-picker-flow.test.ts +++ b/sdk/tests/backlog-picker-flow.test.ts @@ -158,6 +158,40 @@ describe('backlog-picker canonical spec', () => { } }); + it('carries every dependency the yaml declares, under the kernel\'s key', () => { + // Consistency between steps is not enough, and review caught that (PR #37): + // if regeneration dropped `depends_on` from EVERY step, all field sets + // would still match and none would be camelCase, so both checks above pass + // while the kernel silently loses the whole dependency graph and runs the + // steps in the wrong order. + // + // So compare against the authority instead of against the siblings. The + // yaml declares the dependencies; the canonical spec must carry the same + // ones under `depends_on`. This cannot go stale as the kernel's schema + // grows, because it asserts a relationship rather than a field list. + const root = join(__dirname, '..', '..'); + const flow = load(readFileSync(join(root, 'testdata', 'backlog-picker.flow.yaml'), 'utf8')) as { + steps: Array<{ id: string; dependsOn?: string[] }>; + }; + const canonical = JSON.parse( + readFileSync(join(root, 'testdata', 'backlog-picker.spec.canonical.json'), 'utf8'), + ) as { steps: Array<{ id: string; depends_on?: string[] }> }; + + const canonicalById = new Map(canonical.steps.map((step) => [step.id, step])); + let declared = 0; + for (const step of flow.steps) { + if (step.dependsOn === undefined) continue; + declared += 1; + expect( + canonicalById.get(step.id)?.depends_on, + `step "${step.id}" loses the dependencies the yaml declares`, + ).toEqual(step.dependsOn); + } + // If the yaml ever stops declaring dependencies this test would assert + // nothing at all, and pass while proving nothing. + expect(declared, 'the flow yaml declares no dependencies to check').toBeGreaterThan(0); + }); + it('keeps directories and extensionless paths, and rejects prose', () => { const steps = stepCommands(); const dir = mkdtempSync(join(tmpdir(), 'backlog-scope-'));