From ad276ff3245b9152c2392a743e35828de7a8e50d Mon Sep 17 00:00:00 2001 From: "carthick@amazon.com" Date: Thu, 2 Jul 2026 16:28:21 -0700 Subject: [PATCH] fix(migration-to-aws): catch dangling _advances_to (validator false-green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontmatter validator did not catch a phase whose _advances_to names a phase that does not exist. The backbone chain-consistency block is gated on `advTargetsResolvable` — it SELF-SKIPS the instant any _advances_to is unresolvable, assuming "that phase just isn't annotated yet (partial rollout)". So a genuine dangling/typo'd forward edge slipped through as a false "OK", and unlike _requires_phase there was no independent membership check on _advances_to. Reproduced: setting generate._advances_to to a non-existent phase (generate has no _re_entry_guard to incidentally trip) reported OK / exit 0 — CI would pass a backbone that dead-ends nowhere. Fix: an independent _advances_to membership check that resolves each non-terminal target against the phase DIRECTORY on disk (references/phases//.md), not the frontmatter-declared set. This preserves partial-rollout tolerance — an edge to a real phase whose file exists but has no frontmatter yet still resolves — while failing only on an edge to a phase that does not exist at all (the real heroku rollout always kept the prose file present when frontmatter lagged). - check.ts: +independent dangling-forward-edge check (disk membership). - tests: goodSkill now includes a frontmatter-less clarify.md stub so its partial-rollout test is FAITHFUL (real phase, no frontmatter → tolerated); +1 regression test for a truly-absent target (typo → fail). 41 tests total. --- .../tests/tools/frontmatter-validator.test.ts | 26 +++++++++++++++++++ .../tools/frontmatter-validator/check.ts | 21 +++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/migrate/plugins/migration-to-aws/tests/tools/frontmatter-validator.test.ts b/migrate/plugins/migration-to-aws/tests/tools/frontmatter-validator.test.ts index a69950f..a891b95 100644 --- a/migrate/plugins/migration-to-aws/tests/tools/frontmatter-validator.test.ts +++ b/migrate/plugins/migration-to-aws/tests/tools/frontmatter-validator.test.ts @@ -77,6 +77,16 @@ _produces: --- # Assembler prose body. +`, + // A partial-rollout stub: 'clarify' is a REAL downstream phase whose file + // exists but carries NO frontmatter yet (mid phase-by-phase rollout). It is + // invisible to the typed model (bindSkill skips no-frontmatter files) but makes + // discover's `_advances_to: clarify` resolve on disk — so the dangling-edge + // check tolerates it (a real phase, not a typo). + 'references/phases/clarify/clarify.md': +`# Clarify + +prose-only phase (no frontmatter yet). `, }; } @@ -171,6 +181,9 @@ _contributes: }); it('does NOT fail an _advances_to that points at a phase without frontmatter (partial rollout)', () => { + // goodSkill includes a frontmatter-less clarify.md stub — a real phase mid- + // rollout. discover._advances_to: clarify must resolve (dir exists) and NOT be + // flagged as dangling, even though clarify has no typed frontmatter yet. const findings = validateFixture(goodSkill()); assert.ok( !findings.some((f) => /advances_to|clarify/.test(f.message)), @@ -178,6 +191,19 @@ _contributes: ); }); + it('rejects an _advances_to that names a phase with no file on disk (dangling forward edge)', () => { + const files = goodSkill(); + // Point discover at a phase that does not exist at all (a typo, not rollout). + files['references/phases/discover/discover.md'] = files[ + 'references/phases/discover/discover.md' + ].replace('_advances_to: clarify', '_advances_to: clarify_TYPO'); + const findings = validateFixture(files); + assert.match( + findings.map((f) => f.message).join('\n'), + /_advances_to 'clarify_TYPO' names neither a terminal.*nor an existing phase.*dangling forward edge/s, + ); + }); + // ---- backbone / checkpoint + chain-consistency ---- // A fully-declared 2-phase backbone (discover -> clarify -> complete) plus a diff --git a/migrate/plugins/migration-to-aws/tools/frontmatter-validator/check.ts b/migrate/plugins/migration-to-aws/tools/frontmatter-validator/check.ts index e5d89d7..d42fe02 100644 --- a/migrate/plugins/migration-to-aws/tools/frontmatter-validator/check.ts +++ b/migrate/plugins/migration-to-aws/tools/frontmatter-validator/check.ts @@ -229,6 +229,27 @@ export function check(skill: BoundSkill): Finding[] { } } + // ---- _advances_to membership (dangling forward-edge check) ---- + // A phase's _advances_to must name either a terminal (complete/done/end) or a + // phase that EXISTS ON DISK (a references/phases//.md file). This is + // independent of the chain-consistency block below (which is gated on the whole + // backbone being frontmatter-present and self-SKIPS the moment any _advances_to + // is unresolvable — so a dangling edge would otherwise slip through as a false + // OK). Resolving against the phase DIRECTORY (not the frontmatter-declared set) + // preserves partial-rollout tolerance: an edge to a real phase that has no + // frontmatter yet still resolves; only an edge to a phase that does not exist at + // all fails. + for (const phase of skill.phases) { + if (!phase.advancesTo || TERMINALS.has(phase.advancesTo)) continue; + const targetFile = join(skill.referencesRoot, "phases", phase.advancesTo, `${phase.advancesTo}.md`); + if (!existsSync(targetFile)) { + add( + skill.rel(phase.sourceFile), + `_advances_to '${phase.advancesTo}' names neither a terminal (complete/done/end) nor an existing phase (no references/phases/${phase.advancesTo}/${phase.advancesTo}.md) — dangling forward edge`, + ); + } + } + // ---- Backbone chain-consistency (backbone phases only; checkpoints excluded) ---- // The backbone is the linear lifecycle wired by _advances_to (forward) and // _requires_phase (backward). Checkpoints (_kind: checkpoint) are off-backbone and