fix(release): fence-aware UPGRADING extraction, absolute links on the release page, and the composer suite in ci-required - #2684
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ten link rewriting An unterminated fence made extractUpgradingSection run to the end of the document, publishing every older version's notes under '## Breaking changes'; the body-length guard could not catch it because UPGRADING.md is ~20 KB. It now throws MalformedUpgradingSectionError, which composeReleaseNotes turns into an error for BOTH tag classes. Also: angle-bracketed destinations are unwrapped, rewritten and re-wrapped instead of being concatenated onto the blob base verbatim; the reference-definition match now requires end-of-line or a CommonMark title, so prose shaped '[Note]: see ...' is left alone and its inline links are still processed.
…name the three contract suites
Fix roundRound 1's fix, on
1 — fail closed on an unterminated fence
Tests: the old 3 — angle-bracketed destinations
4 — reference-definition shape
VerificationAll commands in the worktree at Red first — the five new/changed tests added before the implementation, with only the error class
Two representative failing assertions: Green after the implementation:
Dry run over the real Second dry run over a scratch copy of Non-zero exit, the Residual, not fixed (outside the finding, flagging rather than expanding scope)The guard fires when the section's fence is open at end of document. While building the second dry Not verified
|
Review disposition (beta-platform-integrity lane): parked under SC-10This PR changes Round 1 (fresh-context, read-only reviewer at 9b17d4e): SHIP, no CRITICAL or HIGH; two MEDIUM, three LOW. Confirmed: the fence change is behaviour-identical on the shipped UPGRADING.md (all fences balanced, no Fixed in the fix round (a6ad4af, 6150eb1)
Residuals, recorded for the maintainer's review and on #2250 (no third round)
Verification at 6150eb1 (local; the hosted run on the exact head is the R4 proving check)
|
…mirror questions from the #2680 memo
…D-14's defaults; note D-10's superseded audit description
Readiness: the human-gate table was not unchanged at 15:15Z, its SC-10 row now counts twelve (#2684 and #2687 named); the cancelled 6cf9ef7 run had 15 green, Secret Scan skipped, one cancelled; #2691's seeding time and its 03:26Z red are exact; the dropped clause-4 evidence (cancelled-run range, run ids, timestamps) is restored; #2378 and #2588 occurrence lists gain the sweep's sightings. STATUS: same job tally; range note that the sweep's last merge is one past the block's declared bound. OUTSTANDING_TASKS.md: the rule-3 Changelog entry for the D-12 tick and the section K exception sentence.
|
Fresh-context re-review against the moved base (coordinator, 2026-09-06; SC-10 delegated, q-1 = A; read-only Opus 5 reviewer over the diff of the updated head against current VERDICT: SHIP. |
1 participant
Summary
Issue #2250 items 4 and 5 only. Items 1 to 3 (rehearsal preview tag, legacy-tag re-dispatch, semver
changelog base) stay open.
Item 5a, fence-aware extraction.
extractUpgradingSectionnow tracks fenced code blocks while itscans, so a
#or##line inside a fence is sample text rather than a heading. That applies inboth directions: a fenced block no longer truncates the section it sits in, and a
## <tag>lineinside a fence is no longer taken as the section start. Fence tracking follows CommonMark closely
enough for this document: up to three leading spaces, backtick or tilde runs of three or more, a
close only on the same character at least as long as the opener with nothing but whitespace after
it, and no opening backtick fence whose info string contains a backtick. An unterminated fence FAILS
CLOSED (round 1 review fix): the section would otherwise run to the end of the document and publish
every older version's notes under
## Breaking changes, and the body-length guard cannot catch thaton a ~20 KB document.
extractUpgradingSectionthrowsMalformedUpgradingSectionError, whichcomposeReleaseNotesturns into an::errorfor BOTH tag classes — missing is recoverable by apointer, silently wrong is not. The policy is recorded in the header comment and on the function.
Item 5b, links on the release page. New pure function
rewriteRelativeLinks(markdown, { repo, tag })makes relative Markdown destinations absolute. A bare
#anchorresolves againstblob/<tag>/UPGRADING.md#anchor; a relative path resolves againstblob/<tag>/<path>with./stripped and any
#fragmentkept. It uses the sameencodeURIComponent(tag)convention asupgradingUrl, and leaves path segments exactly as written. Untouched: any scheme-bearingdestination (
https:,mailto:, and the rest), root-relative/path, fenced code blocks, andinline code spans. Reference-style definitions (
[id]: dest) are handled and tested. Imagedestinations share the
](shape and get the same rewrite; a relative image is broken on a releasepage either way, and UPGRADING.md has none today, so that is noted in a comment rather than
special-cased. Round 1 review fixes: CommonMark angle-bracket destinations (
](<docs/x.md>)) areunwrapped, rewritten and re-wrapped instead of being concatenated onto the blob base verbatim; and the
reference-definition match now requires end-of-line or a CommonMark title after the destination, so
prose shaped
[Note]: see the guide …keeps its first word and still has its inline links rewritten. The rewrite is applied to the UPGRADING section only. The curated notes file has norelative-link shapes today (checked
docs/releases/notes/), and the generated changelog is alreadyabsolute, so neither is rewritten.
MAX_RELEASE_BODY_LENGTHbehaviour is unchanged: the guard still runs on the final composed bodyafter the rewrite, and a test pins that it still fires when the rewrite has made the body longer.
Item 4, wiring. One step,
Validate release notes composer, added immediately afterValidate release cache trust contractin therelease-workflow-contractjob of.github/workflows/ci-required.yml; the file's topology comment now names all three contract suitesthat job runs (round 1 review fix). Nothing pins that job's step list: grepping
scripts/ciandscripts/ci/smart-ciforrelease-cache-contract,release-desktop-dispatchand the job nameturns up no step-list assertion, and the only
ci-required.ymlreferences in the Smart CI tests areglob and control-path fixtures in
plan.test.mjs.ci/policy.v1.jsonalready routesscripts/ci/**to the control-path tier, so policy is left alone.
Root cause
Both defects come from treating the lifted Markdown as if it were still a file in the tree.
The extractor scanned line by line with
/^#{1,2} +/and no state, so any fenced sample linestarting with
#looked exactly like a heading. In the shippedUPGRADING.mdno fenced blockcurrently starts a line with
#, so no section is truncated today; the failure is latent and wouldland the first time a
powershellorbashsample carries a# comment, which is the ordinary wayto write one.
The link defect is live now. The section is rendered into a GitHub Release body, which is not a file,
so a destination that resolves against
UPGRADING.mdin the repository resolves against nothing onthe page. Composing the real
v0.3.0-rc.1section against the base commit produces three dead bareanchors.
Verification
All commands run in the worktree at
.worktrees/codex-2250-composer-fences-and-suite, Node 24.13.1.Red first. The new tests were added before the implementation, with only an identity stub for the
missing export so the module would load:
node --test scripts/ci/compose-release-notes.test.mjstests 49, pass 34, fail 15. The fifteen failures were:
Two representative failing assertions:
Green after the implementation:
node --test scripts/ci/compose-release-notes.test.mjs— tests 49, pass 49, fail 0 (33 before this PR, 16 added)node --test scripts/ci/release-desktop-dispatch.test.mjs— tests 76, pass 76, fail 0node --test scripts/ci/release-cache-contract.test.mjs— tests 28, pass 28, fail 0node --test scripts/ci/smart-ci/*.test.mjs— tests 91, pass 91, fail 0node scripts/check-github-ops-governance.mjs— exit 0, "GitHub operations governance check passed."git diff --check 0b3787d81...HEAD— no output, exit 0Dry run over the real
UPGRADING.md.parseArgsrequires--tag,--repo,--assetand--out;the checksum is optional as a flag but a missing digest is a hard error even for an RC, so a fake
sha256sumline (64acharacters plus the asset name) was written to the scratch directory andpassed as
--checksum-file.--generated-noteswas omitted, which produces the expected placeholderwarning.
Exit 0, 8933 bytes written, one
::warning::about absent generated notes. Grepping the output:grep -c "](#"— 0grep -c "blob/v0.3.0-rc.1/"— 4The four are two occurrences of
blob/v0.3.0-rc.1/UPGRADING.md#automatic-pre-migration-backups, oneof
blob/v0.3.0-rc.1/UPGRADING.md#general-upgrade-procedure, and the pre-existingblob/v0.3.0-rc.1/docs/releases/WINDOWS_QUICK_START.mdfrom the download block.Before and after. The same command run against the base commit's copy of the composer
(
git show 0b3787d81:scripts/ci/compose-release-notes.mjs) writes 8729 bytes andgrep -c "](#"returns 3. Those three are the dead anchors this PR removes.Not verified
ci-requiredrun. This is a CI-control change and R4 says the proving check is thehosted run on the exact PR head; the local runs above are additive only. The new step has not
executed on a runner yet.
backend/,frontend/ordocs/istouched by this diff.
UPGRADING.mdbut uses a fabricated checksum and no generated-notes JSON, so it is not a rehearsalof
release-desktop.yml.UPGRADING.mdcontains no images.](<docs/x.md>)).The form CONTAINING SPACES (
](<path with spaces>)) is still not handled and not tested: theinline-link pattern only matches whitespace-free destinations, so such a link is left as written.
code blocks (four-space) are not treated as code, and neither are HTML blocks.
Risk notes
repository files themselves, the curated notes, or the generated changelog.
so a composed body can get longer. The
MAX_RELEASE_BODY_LENGTHguard is unchanged and still runson the final body; a test pins that it fires when a rewritten section overflows.
UPGRADING.mdfails the compose (exit 1,::error, nothing written to--out) rather than being published. Residual, measured while building the round 1 dry run and NOTfixed here: the guard fires only when the fence is open at END OF DOCUMENT. An unterminated backtick
fence that a LATER fence in the document closes still swallows content silently (a scratch copy
composed 12801 bytes instead of 8933, with
## v0.2.0content under## Breaking changes, exit 0).Detecting that needs a policy call about the correct parse of an odd-fence-count document, which is
a separate decision from this PR.
ci-requiredstep is onenode --testinvocation in an existing job that already hasNode set up and only
contents: read. It adds no permissions, no network access and no new job.Round 1's review fixes are applied (five findings: one MEDIUM fail-closed policy, one MEDIUM doc gap,
two LOW link-rewriting shapes, one comment-only CI topology line) — see the Fix round comment for
the red-first counts and both dry runs. Two rounds is the ceiling; no further review round is owed
unless the fixes introduced a new CRITICAL.
Refs #2250 (items 4 and 5; items 1 to 3 stay open). Refs #2248, #2234, #1308. Control-plane change: parks under SC-10 for the maintainer's review after the fresh-context review.