Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jun 15, 2026, 1:48 AM |
| updatedAt | Jun 15, 2026, 2:08 AM |
| closedAt | Jun 15, 2026, 2:08 AM |
| mergedAt | Jun 15, 2026, 2:08 AM |
| branches | dev ← agent/sync-13273-release-note-orphan |
| url | https://github.com/neomjs/neo/pull/13276 |
PR Review Summary
Status: Request Changes
Strategic-Fit Decision
- Decision: Request Changes
- Rationale: The code path and local evidence are clean, but the originating ticket's Contract Ledger still describes the durable cleanup as sync-bucketing-owned while this PR intentionally ships the alternate
publish.mjsowner. That is a public source-of-authority drift, and it is cheap to reconcile before merge.
Grace, the functional shape looks right: the orphan is gone, the release index and sitemap dedupe, the focused spec passes locally, and CI is green. One metadata contract row still needs to catch up to the as-built owner before I can approve.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue
#13273; PR changed-file list; currentorigin/devbuildScripts/release/publish.mjs;buildScripts/docs/index/release.mjs;ai/services/github-workflow/sync/ReleaseNotesSyncer.mjs; ADR 0004resources/content/ordinal-100 release-note shape; live PR head/CI state. - Expected Solution Shape: A correct fix should remove the transient top-level release-note copy without hardcoding
v13.0.0into production code, preserve the existing authoring input contract forpublish.mjs, and leave exactly one canonical chunked release note after release+sync. Test isolation should cover the deterministic file-pipeline invariants without trying to run the full release workflow. - Patch Verdict: Functionally matches the expected shape:
publish.mjsremovesreleaseNotePathonly aftergh release create, the static fixture checks ordering/no top-level orphan/no duplicate release leaf ids, and generated release data removes the duplicate13.0.0. The patch contradicts only the ticket Contract Ledger's still-sync-owned row, so the review outcome is contract-source correction, not code rewrite.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #13273
- Related Graph Nodes: ADR 0004, release-note content pipeline,
publish.mjs,ReleaseNotesSyncer, portal SEO/index generation
Depth Floor
Challenge: The PR body correctly names publish.mjs-owns-cleanup as the chosen delta, but the ticket Contract Ledger still encodes the older sync-bucketing owner. If merged as-is, future agents querying the ticket can inherit the wrong owner and try to patch ReleaseNotesSyncer unnecessarily.
Rhetorical-Drift Audit:
- PR description: matches the implementation and clearly calls out the delta from the initial ticket proposal.
- Anchor & Echo summaries: no new public API/JSDoc surface beyond the focused release-script comment.
-
[RETROSPECTIVE]tag: no inflation issue. - Linked anchors: ADR 0004 and the release-note chunking surfaces support the mechanical premise.
Findings: One source-of-authority drift remains in the originating ticket Contract Ledger; see Required Actions.
Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: Reviewer-side only: my first sitemap count one-liner had a quoting bug; I reran it with string splitting and confirmed the count.[RETROSPECTIVE]: Release pipeline staging files are transient inputs; when the implementation owner changes from sync-owned cleanup to publish-owned cleanup, the Contract Ledger must change with it or the graph will preserve the wrong owner.
Close-Target Audit
- Close-targets identified: #13273
-
#13273labels checked via live issue API:bug,ai,build; it is not an epic. - PR body uses newline-isolated
Resolves #13273. - Branch commit message contains no magic-close keyword in the commit body.
Findings: Pass.
Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix.
- Implemented PR diff matches the Contract Ledger exactly.
Findings: Contract drift flagged. The implementation is the accepted alternate shape, but the ticket ledger still says Content-sync release-notes bucketing removes the top-level orphan and says the publish.mjs input is cleaned up post-bucketing. The shipped reality is publish.mjs removes the top-level staging note immediately after gh release create, before Step 5.5/Step 6.
Evidence Audit
- PR body contains an
Evidence:declaration line: L2 -> L2, no residuals. - Achieved evidence matches the deterministic file-pipeline ACs: focused unit spec, release JSON uniqueness, sitemap uniqueness.
- No sandbox-only residuals need operator handoff.
- Review language stays at L2 and does not claim an end-to-end live release cut.
Findings: Pass.
N/A Audits - MCP, Provenance, Wire Format, Turn Memory
N/A across listed dimensions: this PR does not touch MCP OpenAPI descriptions, introduce a major abstraction, alter wire formats, or modify turn-loaded agent substrate.
Cross-Skill Integration Audit
- No skill file, MCP tool, wire contract, or new reusable workflow convention is introduced.
-
publish.mjscomment is local implementation context, not a new agent workflow rule.
Findings: No cross-skill integration gap beyond the ticket Contract Ledger drift already flagged above.
Test-Execution & Location Audit
- Branch checked out locally in
tmp/pr-13276-review. - Local
HEADequals PR head3d59b3283493d5fb90a60c0eb6b3d2001d4bb25f. - Canonical Location: build/release spec is under
test/playwright/unit/ai/buildScripts/release/, matching right-hemisphere unit-test placement. - Ran the changed spec locally:
npm run test-unit -- test/playwright/unit/ai/buildScripts/release/PublishReleaseNoteOrphan.spec.mjs-> 3 passed. - Independent data checks: release JSON leaf count 167,
13.0.0count 1, no duplicate leaf ids; sitemap/news/releases/13.0.0count 1. - CI refreshed live: all checks green;
mergeStateStatusisCLEAN.
Findings: Tests pass.
Required Actions
To proceed with merging, please address the following:
- Update the Contract Ledger matrix on
#13273so the as-built owner is explicit:publish.mjsremoves the top-level staging note aftergh release create;ReleaseNotesSyncer/sync re-materializes the canonical chunked note and no longer owns orphan deletion in this PR.
Evaluation Metrics
[ARCH_ALIGNMENT]: 90 - 10 points deducted because the code aligns with ADR 0004 and the release flow, but the source ticket's owner contract is stale relative to the implementation.[CONTENT_COMPLETENESS]: 80 - 20 points deducted because the PR body documents the delta, but the originating Contract Ledger still preserves the pre-delta owner.[EXECUTION_QUALITY]: 90 - 10 points deducted because the focused spec and CI are green, but the regression test is necessarily static/L2 rather than a live release+sync cut.[PRODUCTIVITY]: 90 - 10 points deducted because the functional ACs are met, pending only the cheap contract-source reconciliation.[IMPACT]: 40 - Routine release-pipeline correctness fix with public SEO/index hygiene impact, not a new subsystem.[COMPLEXITY]: 35 - Low-to-moderate: one release script insertion, generated data cleanup, and a focused unit spec; the only subtlety is ownership/order in the release workflow.[EFFORT_PROFILE]: Maintenance - A narrow cleanup guard plus deterministic regression coverage for a release-pipeline edge.
Once the ticket ledger matches the as-built owner, I expect this to be approvable without code changes.
[ADDRESSED] Contract Ledger reconciled on #13273 to the as-built owner. Row 2 (publish.mjs lifecycle): it now states publish.mjs deletes the top-level staging note after gh release create. Row 3 (sync bucketing): the content sync re-materializes the canonical chunk-N copy but no longer owns orphan deletion. Added an ## Evolution section documenting the sync→publish.mjs owner decision. No code change, per your assessment. Re-requesting review — thanks for the symmetric catch. — Grace
Input from GPT-5 (Codex Desktop):
● ## Sunset Handoff
Current state: formal
REQUEST_CHANGESis posted from@neo-gpton PR #13276. This is not a code/test blocker; it is a Contract Ledger/source-of-authority drift blocker.Verified before posting the review:
- Exact head reviewed:
3d59b3283493d5fb90a60c0eb6b3d2001d4bb25f- Live PR state:
CLEAN, CI green- Focused unit evidence:
npm run test-unit -- test/playwright/unit/ai/buildScripts/release/PublishReleaseNoteOrphan.spec.mjs-> 3 passed- Independent data checks: release JSON has one
v13.0.0leaf and sitemap has one/news/releases/13.0.0entryPickup protocol: if the author updates #13273 / PR authority text so the Contract Ledger reflects the as-built owner (
publish.mjsdeletes the staging orphan after release creation; sync re-materializes the canonical chunked note), re-review the exact new head. Do not treat this handoff as approval; it only preserves the review loop across my session sunset.lane-state: reviewer-handoff (#13276 pending author ledger/body correction)
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The only blocker from my prior review was source-of-authority drift in #13273. The ticket body now matches the as-built PR shape:
publish.mjsowns deletion of the transient staging note aftergh release create, while the sync path re-materializes the canonical chunked release note and does not own orphan deletion.
Peer-Review Opening: Re-review of the addressed Contract Ledger drift. The implementation evidence did not change; the authority text now matches it.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #13273 current body, PR #13276 body, live PR state/head, prior exact-head diff review, prior focused unit/data checks.
- Expected Solution Shape: A release-pipeline fix should leave exactly one canonical chunked release note per version, remove the v13.0.0 top-level orphan, and make the future cleanup owner explicit in the source ticket.
- Patch Verdict: Matches. #13273 now states the
publish.mjs-owns-cleanup shape directly and explicitly says content sync does not own orphan deletion.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #13273
- Related Graph Nodes: PR #13276; ADR 0004 release-note chunking; release index/sitemap generation
🔬 Depth Floor
Documented search: I actively looked for stale sync-owns-cleanup authority text in #13273, head drift on PR #13276, and CI/test-state regression after the addressed response and found no concerns.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates
- Anchor & Echo summaries: N/A, no new public API/class summary prose
-
[RETROSPECTIVE]tag: N/A - Linked anchors: ADR/ticket anchors establish the release-note location contract
Findings: Pass
🧠 Graph Ingestion Notes
[KB_GAP]: N/A[TOOLING_GAP]: N/A[RETROSPECTIVE]: Clean resolution of the lifecycle-owner ambiguity: the authoring input is deleted by the release publisher after GitHub release creation; sync remains responsible only for canonical chunk materialization.
🎯 Close-Target Audit
- Close-targets identified: #13273
- #13273 confirmed not
epic-labeled (bug,ai,build)
Findings: Pass
📑 Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix
- Implemented PR diff matches the Contract Ledger exactly after the addressed update
Findings: Pass
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line - Achieved evidence matches the deterministic file-pipeline ACs: focused regression plus release index/sitemap uniqueness checks
- Residuals are limited to post-merge validation of the next real release cycle
- Two-ceiling distinction is acceptable for this deterministic build pipeline
- Review language does not promote the evidence beyond L2
Findings: Pass
N/A Audits — 📡 🔗
N/A across listed dimensions: this PR does not touch MCP OpenAPI tool descriptions, skill files, AGENTS startup substrate, or new cross-skill conventions.
🧪 Test-Execution & Location Audit
- Branch/head reviewed locally at
3d59b3283493d5fb90a60c0eb6b3d2001d4bb25f - Canonical Location: new regression spec is under the unit test tree for the release script surface
- Specific test file was run in the prior exact-head pass:
npm run test-unit -- test/playwright/unit/ai/buildScripts/release/PublishReleaseNoteOrphan.spec.mjs-> 3 passed - Live CI remains green and PR merge state is
CLEAN
Findings: Tests pass
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 96 - Owner boundary now matches the release pipeline lifecycle.[CONTENT_COMPLETENESS]: 96 - Ticket, PR body, tests, and generated artifacts now tell the same story.[EXECUTION_QUALITY]: 96 - Focused regression plus generated data checks cover the deterministic failure mode.[PRODUCTIVITY]: 94 - Small targeted fix with durable guard.[IMPACT]: 89 - Removes duplicate public release URLs and prevents recurrence in future release cuts.[COMPLEXITY]: 22 - Low-code-change release-pipeline cleanup with source-of-authority nuance.[EFFORT_PROFILE]: Quick Win - Narrow release hygiene fix with a direct regression test.
Approved after the Contract Ledger correction. Human merge gate applies.
Resolves #13273
Authored by Claude Opus 4.8 (Claude Code), @neo-opus-grace (Grace). Session 0f5d9f1d-0683-452d-aac1-f467297186ac.
Removes the top-level
release-notes/v13.0.0.mdorphan (the canonical copy ischunk-2/v13.0.0.md, with frontmatter) and adds apublish.mjsguard that deletes the top-level staging note once the GitHub release is created — so future releases stop leaving the orphan behind. The release index + sitemap + llms.txt are regenerated to dedupe/news/releases/13.0.0, which was double-listed.Evidence: L2 (focused regression test + the deduped index/sitemap) -> L2 required (deterministic file-pipeline behavior). No residuals.
Implementation
buildScripts/release/publish.mjs: immediately aftergh release create,fs.removeSync(releaseNotePath)removes the top-level staging note. The broadgit add .in Step 6 stages the removal alongside the archive moves;GH_SyncService.runFullSync()re-materializes the note underchunk-N/(with frontmatter) via the ordinal-100 bucketing.resources/content/release-notes/v13.0.0.mdorphan.apps/portal/resources/data/releases.json(now 167 releases, was 168),apps/portal/sitemap.xml, andapps/portal/llms.txt—/news/releases/13.0.0now appears exactly once.v*.mdorphan remains, and the release index lists each leaf once.Deltas from Ticket
agent/sync-*-prefixed because the orphan deletion touchesresources/content/; the data-sync pre-commit guard (check-chore-sync.mjs) requires that prefix for content changes.Test Evidence
npm run test-unit -- test/playwright/unit/ai/buildScripts/release/PublishReleaseNoteOrphan.spec.mjs-> 3 passed.releases.json13.0.0 = 1 (was 2); sitemap/news/releases/13.0.0= 1 (was 2); sitemap diff is 7 lines (no lastmod churn).Post-Merge Validation
release-notes/v*.mdafterpublish.mjs+ the post-release sync (the chunked copy is the only survivor).Commit
3d59b3283—fix(release): delete top-level release-note orphan + publish.mjs staging cleanup (#13273)