fix(migration-to-aws): catch dangling _advances_to (validator false-green)#113
Merged
Merged
Conversation
…reen) 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/<name>/<name>.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The frontmatter validator silently passed a phase whose
_advances_tonames a phase that doesn't exist — a false green on a broken backbone.Root cause
The backbone chain-consistency block is gated on
advTargetsResolvable, which self-skips the moment any_advances_tois unresolvable (it assumes "that phase just isn't frontmatter-annotated yet — partial rollout"). And unlike_requires_phase,_advances_tohad no independent membership check. So a genuine dangling/typo'd forward edge disables the very checks meant to catch it.Reproduced: setting
generate._advances_toto a non-existent phase (generatehas no_re_entry_guardto incidentally trip) → validator reportsOK, exit 0. CI would pass a backbone that dead-ends nowhere. (A typo ondiscoveronly went red by luck — its_re_entry_guard._stale_if_completedcross-check tripped a different rule.)Fix
An independent dangling-forward-edge check: each non-terminal
_advances_totarget must resolve to a phase directory on disk (references/phases/<name>/<name>.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 doesn't exist at all. (In the real heroku rollout the prose file was always present when frontmatter lagged, so this matches how rollout actually happened.)
Changes
check.ts— +independent dangling-forward-edge check (disk membership), placed before the rollout-gated chain block so it can't be self-skipped.goodSkill()now includes a frontmatter-lessclarify.mdstub so its partial-rollout test is faithful (real phase, no frontmatter → tolerated, resolves on disk); +1 regression test for a truly-absent target (typo → fail). 41 tests total.Verification
mise run buildgreen: lint:md, lint:types (tsc strict), lint:frontmatter (real skill clean), 41/41 tests, fmt:check.generate._advances_tonow fails with a clear message; real skill stays clean.Ownership
Touches only team-owned validator + tests — no admin-owned paths.