fix(normalize-spec): cover both committed pairs and make overrides atomic - #209
Merged
Merged
Conversation
buildFactoryExecutionPlan pushed RedesignIntegrityReceiptStage after
ReleaseReceiptFinalizerStage and HandoffEmitterStage. For an end-to-end
REDESIGN_IMPROVE run the emitted order was:
... visual-qa, release-receipt-finalizer, handoff-emitter,
redesign-integrity-receipt, terminal-convergence
The finalizer writes status: "succeeded" to the release receipt and the
emitter performs the external POST to SEO-Bot plus recordWebsiteRelease
(externalMutation: true). A gate in that position can only reject a run
that has already been recorded as successful and handed off — it cannot
prevent one. An invalid redesign was therefore publishable.
The gate now runs after visual-qa, whose ctx.qualityEvidence.visualQa it
reads, and before the finalizer and emitter. It stays outside the
end-to-end guard on purpose: REDESIGN_ADDED_MANDATORY makes
redesign-integrity-receipt mandatory in every mode, so folding it into
that branch would make requireMandatoryConvergence throw on plan,
local-proof and publish-proof redesign runs.
No stage, receipt type, or handoff contract is added — this is a reorder.
The gate reads only context produced upstream of its new position and
declares no evidence block, so it imposes no runner ordering constraint.
The COPY end-to-end stage list is unchanged; the existing deepEqual
assertion in tests/unit/factory-execution-plan.test.ts passes unmodified.
Two intended consequences:
- On the visual-QA-not-passed path a redesign end-to-end run now fails as
REDESIGN_PIPELINE_INCOMPLETE rather than RELEASE_EVIDENCE_INCOMPLETE.
That is the fail-closed-earlier behaviour, not a regression.
- executed_stages in the emitted receipt no longer lists
release-receipt-finalizer or handoff-emitter, since it snapshots
ctx.stageResults at emit time. Nothing validates that field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
…plan The durable path computed ctx.buildIntent from the spec and then built its execution plan without it, so buildFactoryExecutionPlan always derived redesign === false there. A REDESIGN_IMPROVE spec submitted through Inngest therefore ran the COPY topology: no seo-build-intelligence-preflight, no competitive-intelligence, the legacy content/schema stages instead of the redesign authority chain, and no redesign-integrity-receipt stage at all. The build context said one thing and the plan built another. This also bounds the preceding commit. Reordering the integrity gate ahead of the finalizer and emitter fixes the plan builder, but on this path the gate was never in the plan to begin with, so that fix would have been inert here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
There was no regression coverage for the terminal ordering of a redesign
plan. The two existing REDESIGN_IMPROVE tests assert only the head of the
pipeline (preflight before competitive-intelligence, competitive-intelligence
before design-intelligence), and redesign-integrity-receipt.test.ts asserts
that the stage is present, not where. All of them passed with the gate sitting
after the finalizer and the external handoff.
This asserts the chain that actually matters:
visual-qa < redesign-integrity-receipt < release-receipt-finalizer
< handoff-emitter < terminal-convergence
Verified to discriminate: against the parent commit's plan builder it fails
(8 pass / 1 fail); against the reordered one it passes (9 pass). A test that
cannot fail proves nothing, so it was run both ways before being committed.
The COPY end-to-end deepEqual assertion is deliberately left untouched — it
is the byte-identity guard proving the reorder did not disturb the non-redesign
topology, and editing it would destroy the evidence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
executed_stages and the STAGES list that drives synthetic checkpoint timestamps both hard-code the pipeline's terminal order as static data rather than deriving it from buildFactoryExecutionPlan. They still carried the pre-fix tail, with redesign-integrity-receipt trailing the finalizer and the handoff emitter. Nothing fails as a result — the golden oracle checks a relative ordered subsequence that omits both of those stages, so it is satisfied by either order. That is precisely the problem: the fixtures would have gone on describing an ordering the code no longer has, and a reviewer re-deriving a fixture would have been misled by it. Data-only; no generator logic changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
validation/validation_report.yaml is per-run output of scripts/validation-executor.ts, but it was tracked. Every `npm run site:validate` — and therefore every `verify:all` and every `make pr` — rewrote it with a fresh run_id, fresh timestamps, the local absolute target_roots and the running user's git identity. That is not cosmetic. It hard-blocks the sanctioned publish path: the L4 release receipt is bound to a tree digest, so `make pr` runs `pr-check`, `pr-check` dirties this file, and the L4 remote check then rejects the push as a stale receipt. The gate invalidates the authorization it is about to verify, and no amount of re-authorizing escapes it, because the next run dirties the tree again. Untracking it is the repo's own established convention, not a new one: .gitignore already ignores validation/launch_env_report.json and validation/deploy_secrets_report.json, which are the same class of per-run artifact from the same directory. Nothing consumes the committed copy. scripts/validation-executor.ts only writes it, no test or script reads it, and build-and-validate.yml generates its own at the repository root and uploads that as a workflow artifact. The file stays on disk; only the index entry is removed. As a side benefit the committed copy stops leaking one contributor's worktree path and email address into the repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
This repo is npm-only: .github/workflows/l9-lint-test.yml and l9-lint-test-node.yml both pin PACKAGE_MANAGER="npm", regen-lockfile.yml knows only `npm install --package-lock-only`, and four package-lock.json files are tracked. .gitignore carried no lockfile entries at all, so a stray pnpm-lock.yaml sat untracked and committable. It is container debris — never tracked on any ref, and its mtime matches every other file in the clone rather than any commit. Left unignored it is one `git add -A` away from landing, and the dormant pnpm/yarn arms of the lint workflows would treat it as load-bearing the moment PACKAGE_MANAGER moved. Appended at end of file rather than in the secrets block: PR #207 deletes .gitignore:41, and a hunk there would overlap one written nearby. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L (cherry picked from commit 3bf384caca440a43d4e5466957c671b35c353057)
…omic Two defects in one CLI, both reachable today. Coverage. The default check and write were hardcoded to examples/supplemental-insurance-pros, so the second committed pair was verified by nothing. It duly rotted: examples/quantum-ai-partners was last written in #174, the v1.1 semantic compiler landed in #196, and the committed IR is six top-level keys behind it — routes (missing purpose, template and priority on every route), seo_contract (missing route_targets and schema_application), and business_facts, conversion_authority, content_guardrails and semantic_provenance absent entirely. An artifact nothing verifies is an artifact that drifts. Both pairs are now checked and regenerated together, and the regenerated quantum IR lands in this commit because the script alone would leave the gate red. Pairing. A source and its normalized output are one unit, but a one-sided override filled the missing half from the first committed pair: { in: inArg ?? COMMITTED_SPECS[0].in, out: outArg ?? COMMITTED_SPECS[0].out } So `--in examples/quantum-ai-partners/domain_spec.source.yaml` with no --out compiled quantum and wrote it over supplemental's committed artifact — the wrong client's file, overwritten while the invocation looked entirely deliberate. An override now names both sides, or one side that resolves to exactly one committed pair; anything else is an error rather than a guess. Also pins stringify lineWidth: 0. The committed artifacts were generated unfolded while the writer kept the yaml package's 80-column default, so a regeneration rewrapped an in-sync file by +76 bytes and the gate stayed green through it — it compares parsed objects, not bytes. Verified inert: after this change `npm run normalize-spec` leaves the supplemental artifact byte-identical. The three-function split that the superseded #198 used for this is deliberately not carried over: a loop over the pair list is the same behaviour at roughly a quarter of the diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
Four tests, each verified to fail against the parent commit's script and pass against this one (14 pass / 4 fail before, 18 pass after). The load-bearing one asserts that `--check --in <quantum source>` is judged against quantum's OWN output and leaves supplemental's committed artifact byte-identical and untouched by mtime. That is the exact shape of the defect: the old resolution would have compared quantum's compile against supplemental's artifact, and without --check would have overwritten it. The others cover the mirror case (a one-sided --out), the failure case (a one-sided path matching no committed pair must error rather than pick one), and the preservation obligation that the no-flag default still names every committed pair, so the gate cannot quietly narrow back to one client. These reuse the spawnSync + mtime/body snapshot harness already used by the entry-point-guard test, which is the only existing test that exercises the CLI rather than importing buildFlatSpec. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Hmr3SKRhYQ56omTzNFZ4L
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes address the stated correctness/coverage defects with targeted implementation updates and regression tests that directly exercise the previously unsafe override and ordering paths.
Pull request overview
This PR fixes normalize-spec target selection so --in/--out overrides resolve to an atomic (source, output) pair, preventing cross-client writes, and expands the default normalization gate to cover all committed spec pairs. It also reorders the redesign integrity gate in the execution plan so it can veto before success finalization and external handoff.
Changes:
- Normalize-spec now iterates all committed source→normalized pairs by default and resolves one-sided overrides only when they uniquely identify a committed pair.
- Redesign execution plan now runs
redesign-integrity-receiptbeforerelease-receipt-finalizerandhandoff-emitter, with a regression test covering the ordering. - Removes a tracked per-run validation report artifact and updates ignore rules to prevent reintroducing it (and other non-npm lockfiles).
File summaries
| File | Description |
|---|---|
scripts/normalize-spec.ts |
Adds committed pair coverage and atomic override resolution; pins YAML writer to lineWidth: 0 to prevent byte-churn. |
tests/unit/normalize-spec.test.ts |
Adds negative/guard tests for one-sided overrides and default coverage across committed pairs. |
src/pipeline/FactoryExecutionPlan.ts |
Reorders redesign integrity stage ahead of success finalization/handoff for end-to-end redesign runs. |
tests/unit/factory-execution-plan.test.ts |
Adds regression test asserting redesign integrity gate ordering relative to visual QA, finalization, and handoff. |
src/inngest/website-pipeline.ts |
Ensures buildIntent is passed into the plan builder on the durable/Inngest path. |
scripts/golden-safehaven/fixtures/build-fixtures.mjs |
Updates golden fixture stage ordering to match the new plan ordering. |
examples/quantum-ai-partners/domain_spec.normalized.yaml |
Regenerates committed normalized IR to include v1.1 semantics and consistent YAML formatting. |
.gitignore |
Ignores generated validation report output and prevents accidental commits of non-npm lockfiles. |
validation/validation_report.yaml |
Removes a committed per-run artifact from the repo. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 17, 2026
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.
Summary
Closes audit
website-bot-core-pipeline-2026-09-17findings F2 (the remediation was stranded on a merged feature branch) and F3 (a one-sided normalizer override paired one client's source with another client's output). Replays the substance of the closed #198 onto currentmain, with the pairing defect fixed rather than carried along.Type of change
type:bugfix— bug fixArea
area:backend— src/, packages/Checklist
npm run verify:allpasses locallynpx tsc --noEmitpasses (no type errors)VALIDATION.mdupdated — not applicableDECISION_LOG.mdupdated — not applicableRelated issues / ADRs / contracts
website-bot-core-pipeline-2026-09-17findings F2 (COMPLETENESS, High) and F3 (CORRECTNESS, Medium)Reading this PR
It is stacked on #208 and currently shows 8 commits; only the last two are this PR's.
claude/audit-f3-normalizer-atomic-pairsis cut from #208's head because it needs that PR's untracking fix in order to publish at all —site:validaterewrote a tracked file, which invalidated the release receipt mid-gate. Once #208 merges, this diff collapses to its own two commits.It is based on
mainrather than on #208's branch becausePR_STACK=autocannot resolve in this repository right now — ten open PRs all root onmain, so there is no unambiguous chain tip — and the only other route to a non-main base is an authorization escape that should be granted by a human, not self-issued. Merge #208 first.da0da51--in/--outoverrides atomic; pinlineWidth: 0; regenerate the quantum IR02a3a75F3 — the cross-client write
A source and its normalized output are one unit, but a one-sided override filled the missing half from the first committed pair:
So
--in examples/quantum-ai-partners/domain_spec.source.yamlwith no--outcompiled quantum and wrote it over supplemental's committed artifact — the wrong client's file, overwritten while the invocation looked entirely deliberate. This defect predates #198; that PR preserved it.An override now names both sides, or one side that resolves to exactly one committed pair. Anything else is an error rather than a guess.
F2 — coverage, and why the second artifact was stale
The default check and write were hardcoded to
examples/supplemental-insurance-pros, so the second committed pair was verified by nothing and duly rotted.examples/quantum-ai-partnerswas last written in #174; the v1.1 semantic compiler landed in #196. The committed IR was six top-level keys behind:routes(nopurpose,templateorpriorityon any route),seo_contract(noroute_targets, noschema_application), andbusiness_facts,conversion_authority,content_guardrails,semantic_provenanceabsent entirely.An artifact nothing verifies is an artifact that drifts. Both pairs are now checked and regenerated together, and the regenerated IR lands in the same commit as the script because either alone leaves the gate red.
Why
lineWidth: 0is hereThe committed artifacts were generated unfolded while the writer kept the
yamlpackage's 80-column default, so regenerating an in-sync file rewrapped it by +76 bytes — and the gate stayed green through the churn, because it compares parsed objects rather than bytes.Verified inert: after this change,
npm run normalize-specleavesexamples/supplemental-insurance-pros/domain_spec.normalized.yamlbyte-identical (git diff --statempty). The only artifact that moves is the quantum one, at +156/−10.What was deliberately not carried over from #198
The
compileSpec/checkSpec/writeSpecsplit. It is presentation rather than capability — a loop over the pair list is the same behaviour at roughly a quarter of the diff (+18/−4 against #198's +63/−19).Verification
npm run verify:all— 588 tests, all passing, on02a3a75.The four new tests were verified to discriminate: against the parent commit's script they fail (14 pass / 4 fail); against this one they pass (18 pass). The load-bearing one asserts that
--check --in <quantum source>is judged against quantum's own output and leaves supplemental's artifact byte-identical and untouched by mtime — the exact shape of the defect.Publication note
make prran the full gate — L4 release check,pr-check(588 tests), main-bound ancestry and the overlap probe — and pushed this branch. It could not compose the PR body:open_pr_after_gate.shrequiresops/config/root-file-protection.jsonunder the workspace root, and Website-Bot has noops/directory. Same consumer-side governance wiring gap as #208, reported rather than worked around by inventing a root-file policy for this repo.Generated by Claude Code
Correction — stack topology (audit
website-bot-all-open-prs-2026-09-17, F3)An earlier version of this description claimed a linear chain
#208 → #209 → #210 → #211. That was wrong, and Git disproves it. Verified withgit rev-list --left-right --count:#210 was cut from #208's head, not #209's, so it never contained #209's normalizer delta. The accurate dependency model is:
#209 and #210 do not require a linear relationship with each other.
#209 is additionally one commit behind #208 — it lacks the fixture fix
f67f02d, which I merged forward into #210 and #211 but not #209, while stating it had reached all of them. #209 does not touchbuild-fixtures.mjs, so nothing there is functionally wrong; the claim was simply inaccurate. #209 should be refreshed against #208 once that lands.Generated by Claude Code