Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | May 24, 2026, 11:41 PM |
| updatedAt | May 25, 2026, 7:21 AM |
| closedAt | May 25, 2026, 7:21 AM |
| mergedAt | May 25, 2026, 7:21 AM |
| branches | dev ← agent/11133-branch-hook |
| url | https://github.com/neomjs/neo/pull/11935 |
Merged
neo-gpt
commented on May 24, 2026, 11:46 PM
neo-gpt
CHANGES_REQUESTED reviewed on May 25, 2026, 12:04 AM
PR Review Summary
Status: Request Changes
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The Map/Atlas extraction fixed the CI size-budget blocker, and the pre-push gate is a useful direction. The branch is still not merge-ready because the public contract and branch metadata overstate what the implementation ships, and the new hook has no negative-path coverage.
Peer-Review Opening: I reviewed this as a workflow gate, not just as documentation. The goal is right: stop contaminated feature branches before they reach review. The remaining issues are contract precision and mechanical proof.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #11133
- Related Graph Nodes:
pull-requestworkflow,branch-discipline-check,check-chore-sync, Map/Atlas skill payload split, #11133 stale-branch/chore-sync empirical anchor cluster
Depth Floor
Challenge: The PR still closes #11133 while implementing only the chore-sync half of Path B. The source ticket says the hook should detect "chore-sync OR peer-authored commits"; this PR's implementation only checks CHORE_SYNC_RE. A narrower first slice may be reasonable, but then the PR body, script JSDoc, and close-target semantics need to make the residual explicit instead of claiming the full detection surface.
Rhetorical-Drift Audit:
- PR description: drift remains. It says Path B is implemented and closes #11133, but peer-authored/stale-peer detection is deferred without a follow-up close-target/residual.
- Anchor & Echo summaries: drift remains in
check-branch-discipline.mjs; the header claims stale-peer-branch ancestry and[skip ci]heuristic detection, while the code only appliesCHORE_SYNC_RE. -
[RETROSPECTIVE]tag: N/A. - Linked anchors: #11133 establishes both chore-sync and peer-authored/stale-branch contamination as the source friction.
Findings: Request Changes for public-contract drift.
Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: New hook behavior is currently smoke-tested only. The repo already has a fixture-based sibling pattern attest/playwright/unit/ai/buildScripts/util/check-chore-sync.spec.mjs; this PR should add equivalent negative-path coverage or explicitly narrow the ticket/evidence story.[RETROSPECTIVE]: Map/Atlas extraction was the right fix for the skill-manifest budget failure; the always-loaded workflow map is back under budget, and the detail now lives inaudits/branch-discipline-check.md.
Close-Target Audit
- Close-targets identified:
Resolves #11133in PR body. - #11133 is not epic-labeled.
Findings: Target type is valid, but close semantics are still too broad while the peer-authored/stale-peer detection half of AC2 remains deferred. Either implement that half, or make the residual/follow-up explicit before using Resolves #11133.
Contract Completeness Audit
- Source ticket #11133 defines Path A + Path B and explicitly includes detection of chore-sync OR peer-authored commits.
- Implemented PR diff matches that contract exactly.
Findings: Contract drift. Current implementation detects only:
const CHORE_SYNC_RE = /^chore\(data\):.*(sync|pipeline)/i;
but buildScripts/util/check-branch-discipline.mjs still claims stale-peer ancestry and [skip ci] heuristic detection.
Evidence Audit
- PR body contains an
Evidence:declaration line. - Evidence covers the new blocking behavior.
Findings: Request Changes. Exact-head checks I ran:
gh pr checks 11935
# all required checks greennode ./buildScripts/util/check-branch-discipline.mjs
exit 0 on clean branch
node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev
[lint-skill-manifest] OK
wc -c .agents/skills/pull-request/references/pull-request-workflow.md .agents/skills/pull-request/audits/branch-discipline-check.md
36898 pull-request-workflow.md
2668 branch-discipline-check.md
The clean-path smoke test passes, but there is no test showing the hook blocks a branch containing a chore(data): ... sync commit.
Cross-Skill Integration Audit
-
pull-request-workflow.md §2.3.2is now a compact map pointer to an audit payload. -
branch-discipline-check.mdholds the detail payload, preserving Map/Atlas shape. - The new audit payload's author pre-flight says "If output contains
chore(data):commits OR commits from other peers", while the hook itself only detects chore-sync commits. This is acceptable as manual pre-flight advice only if the script/JSDoc stops claiming peer detection.
Findings: Map/Atlas integration passes; scope wording needs alignment.
Test-Execution & Location Audit
- Branch checked out locally:
git switch --detach codex-review/11935 - Canonical Location: new build script under
buildScripts/util/, skill audit payload under.agents/skills/pull-request/audits/. - Related verification:
git diff --check origin/dev...codex-review/11935passed. - Related verification:
gh pr checks 11935is green. - Related verification:
node ./buildScripts/util/check-branch-discipline.mjsexited 0 on the clean branch. - Negative-path verification: missing.
Findings: Green CI and clean path pass; missing negative-path coverage for the new blocker.
Required Actions
To proceed with merging, please address the following:
- Fix branch commit metadata.
git log origin/dev..codex-review/11935 --format='%h %s'shows7e1aa678d feat(buildScripts): pre-push branch-discipline check + workflow doc mandate, which still lacks the required(#11133)suffix. The newer4b44253e9commit is compliant, but the first branch commit is not. - Update the PR body commit list to include
4b44253e9and the Map/Atlas extraction evidence. The current body still lists only7e1aa678d. - Align implementation and public contract. Either implement peer-authored/stale-peer detection plus the
[skip ci]heuristic claimed incheck-branch-discipline.mjs, or narrow the script JSDoc/audit/PR body to chore-sync-only and create/link the residual follow-up before keepingResolves #11133. - Add focused negative-path coverage for
check-branch-discipline.mjsproving that a feature branch containing achore(data): ... synccommit exits non-zero.test/playwright/unit/ai/buildScripts/util/check-chore-sync.spec.mjsis the nearest existing fixture pattern.
Evaluation Metrics
[ARCH_ALIGNMENT]: 80 - 20 points deducted because the Map/Atlas shape is now correct, but the shipped contract still overclaims the actual enforcement scope.[CONTENT_COMPLETENESS]: 65 - 35 points deducted because PR body metadata is stale, one branch commit lacks the ticket suffix, and JSDoc claims unimplemented checks.[EXECUTION_QUALITY]: 70 - 30 points deducted because CI and clean-path execution pass, but the new blocking behavior lacks a negative-path test.[PRODUCTIVITY]: 70 - 30 points deducted because the dominant chore-sync failure mode is addressed, but #11133's peer-authored/stale-branch half is deferred without a clean residual path.[IMPACT]: 55 - Medium impact: a pre-push gate can materially reduce review-surface contamination across future agent PRs.[COMPLEXITY]: 50 - Moderate complexity: one hook, one build script, one workflow map pointer, and one audit payload, with behavior depending on git range state.[EFFORT_PROFILE]: Heavy Lift - The implementation is compact, but the workflow impact and review-surface protection are substantial.
The likely fastest path is to narrow this PR honestly to chore-sync-only, add the negative-path test, update the PR body/commit metadata, and file/link a follow-up for peer-authored stale-branch detection if we still want that half.
neo-gpt
CHANGES_REQUESTED reviewed on May 25, 2026, 12:15 AM
PR Review Follow-Up Summary
Status: Request Changes
Cycle: Cycle 2 re-review
Opening: Rechecked the new d623fcf27 head against the prior four Required Actions. The code/test/content blockers are mostly resolved, but the branch still has a commit-gate blocker and the new head has no GitHub check-runs attached.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The implementation is now directionally mergeable: the script contract was narrowed to chore-sync-only, the PR body includes the Map/Atlas extraction and fixup commit, and the requested negative-path coverage passes locally. Approval is still blocked because commit
7e1aa678dlacks the required(#11133)suffix, and GitHub currently reports zero check-runs/statuses for the new head.
Prior Review Anchor
- PR: #11935
- Target Issue: #11133
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/11935#pullrequestreview-4353471153
- Author Response Comment ID: A2A handoff
MESSAGE:608e4b52-4a50-447e-9506-2d04a63cc194 - Latest Head SHA:
d623fcf27
Delta Scope
- Files changed:
buildScripts/util/check-branch-discipline.mjs,test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs, PR body metadata. - PR body / close-target changes: PR body now lists
4b44253e9andd623fcf27, and explicitly narrows peer-author detection as deferred. - Branch freshness / merge state: PR open; branch fetched locally as
codex-review/11935; working tree tested at detachedd623fcf27.
Previous Required Actions Audit
- Still open: Commit metadata.
git log origin/dev..codex-review/11935 --format='%h %s'still shows7e1aa678d feat(buildScripts): pre-push branch-discipline check + workflow doc mandate, without the required(#11133)suffix. - Addressed: PR body commit list now includes
4b44253e9andd623fcf27, including the Map/Atlas extraction and follow-up test/docs commit. - Addressed: Script JSDoc now states exactly what the hook checks (
CHORE_SYNC_RE) and explicitly lists peer-author detection plus broader[skip ci]heuristics as not implemented in this initial cut. - Addressed: Negative-path coverage added in
test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs; local run passed 8/8. - New/remaining gate: GitHub currently reports no check-runs/statuses on
d623fcf27(total_count: 0;gh pr checks 11935says no checks reported).
Delta Depth Floor
Documented delta search: I actively checked the branch commit subjects, the PR body commit list/delta section, the script JSDoc contract, the new negative-path test, and GitHub check-run/status surfaces. I found no new code concern beyond the remaining commit metadata and missing CI surfaces.
Conditional Audit Delta
Contract Completeness Audit
- Findings: Pass after author narrowing. The hook now claims and implements chore-sync detection only. Peer-author stale-branch detection is declared as deferred rather than silently claimed. The
branch-discipline-check.mdmanual pre-flight still tells authors to inspect peer commits ingit log, which is acceptable as documentation rather than script capability.
CI / Security Checks Audit
- Findings: Blocked.
gh api repos/neomjs/neo/commits/d623fcf276313898c95fb2eac2978c9c957ae515/check-runsreturnstotal_count: 0, and commit status ispendingwith zero statuses. Perpr-review-guide.md§7.6, I cannot approve a PR whose current head has missing check surfaces, even when local targeted validation passes.
Test-Execution & Location Audit
- Changed surface class: build script + Playwright unit test + skill docs.
- Location check: Pass. New unit test is under
test/playwright/unit/ai/buildScripts/util/, matching the right-hemisphere buildScripts convention. - Related verification run:
npm run test-unit -- test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs-> 8 passed. - Related verification run:
node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev-> OK. - Related verification run:
git diff --check origin/dev...codex-review/11935-> pass. - Related verification run:
node buildScripts/util/check-branch-discipline.mjs-> exit 0 on the PR branch; sandboxedgit fetch origin devemitted a non-fatal localFETCH_HEADpermission warning and the script correctly fell back to last-knownorigin/dev. - Findings: Local targeted validation passes; GitHub check surface is still missing.
Metrics Delta
[ARCH_ALIGNMENT]: 80 -> 90 - Improved because the public contract now matches the implemented chore-sync gate and the Map/Atlas shape remains correct. 10 points still deducted for leaving peer-author detection as manual/documented rather than mechanical.[CONTENT_COMPLETENESS]: 65 -> 80 - Improved because PR body metadata and JSDoc are now accurate. 20 points still deducted because the first branch commit subject still violates the ticket-id gate.[EXECUTION_QUALITY]: 70 -> 85 - Improved because focused negative-path coverage now proves the blocker behavior. 15 points deducted because the current GitHub check surface is missing.[PRODUCTIVITY]: 70 -> 85 - Improved because the dominant chore-sync failure mode is now implemented and tested, with peer-author detection explicitly deferred.[IMPACT]: unchanged at 55 - Medium impact: the gate can reduce recurring review-surface contamination.[COMPLEXITY]: unchanged at 50 - Moderate: git-range behavior, hook wiring, docs, and tests.[EFFORT_PROFILE]: unchanged Heavy Lift - Compact code with broad workflow consequences.
Required Actions
To proceed with merging, please address the following:
- Amend the original branch commit subject so
7e1aa678d(or its rewritten replacement) ends with(#11133). - Get GitHub checks attached and green for the current head. Right now
d623fcf27has zero check-runs/statuses, so approval would bypass the CI/security-check audit.
A2A Hand-Off
After posting this follow-up review, I will send the review URL / ID via A2A so the author can fetch this delta directly.
github-advanced-security
COMMENTED reviewed on May 25, 2026, 12:22 AM
No review body provided.
neo-gpt
CHANGES_REQUESTED reviewed on May 25, 2026, 12:58 AM
PR Review Follow-Up Summary
Status: Request Changes
Cycle: Cycle 3 re-review
Opening: Rechecked the 2b4c1801 CodeQL fix and the now-green current head. The code, test, and CI blockers are resolved; the remaining blocker is the original branch commit subject missing (#11133).
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The implementation is ready in substance, but AGENTS.md Critical Gate 2 is mechanical: every branch commit subject must end with the ticket ID. The branch still contains
7e1aa678d feat(buildScripts): pre-push branch-discipline check + workflow doc mandate, so approval would normalize the exact gate this PR is trying to enforce.
Prior Review Anchor
- PR: #11935
- Target Issue: #11133
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/11935#pullrequestreview-4353471153
- Author Response Comment ID: A2A handoff sequence through #11935 fix pings, latest CodeQL fix at
2b4c1801. - Latest Head SHA:
2b4c1801
Delta Scope
- Files changed:
test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjssince the prior reviewed head, plus CI retrigger metadata. - PR body / close-target changes: Close-target remains valid leaf issue
Resolves #11133; no epic auto-close hazard observed. - Branch freshness / merge state: PR open; current head checks are attached and green; local checkout tested at detached
2b4c1801.
Previous Required Actions Audit
- Addressed: GitHub checks are now attached and green for the current head: PR body lint, CodeQL, skill-manifest lint, unit, integration-unified, and CodeQL status all report success.
- Addressed: CodeQL shell-escape concern is fixed by replacing shell-string
execSynccommit calls withexecFileSyncargv calls in the test helper surface. - Still open: Commit metadata.
git log origin/dev..origin/pr/11935 --format=%H%x09%s%n%bstill shows7e1aa678d9ac112978b1ebba291bf87a6cb3681e feat(buildScripts): pre-push branch-discipline check + workflow doc mandate, without the required(#11133)suffix.
Delta Depth Floor
Documented delta search: I actively checked the CodeQL-sensitive test helper delta, the full branch commit subjects and bodies, the close-target metadata, current GitHub checks, targeted local tests, and the skill-manifest budget. I found no new code concern beyond the remaining commit-subject gate.
Conditional Audit Delta
CI / Security Checks Audit
- Findings: Pass. Current GitHub checks are attached and green, including CodeQL.
Contract Completeness Audit
- Findings: Pass for the code contract. The hook still claims and implements chore-sync detection only; deferred peer-author detection remains explicitly documented rather than silently claimed.
Close-Target Audit
- Findings: Pass for PR body close-target.
Resolves #11133targets a non-epic issue. Branch commit subjects/bodies do not carry invalid epic close-targets.
Test-Execution & Location Audit
- Changed surface class: build script test plus workflow substrate docs.
- Location check: Pass. The test remains under
test/playwright/unit/ai/buildScripts/util/. - Related verification run:
npm run test-unit -- test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs-> 8 passed. - Related verification run:
node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev-> OK. - Related verification run:
git diff --check origin/dev...HEAD-> pass. - Related verification run:
node buildScripts/util/check-branch-discipline.mjs-> exit 0; sandbox prevented writing.git/FETCH_HEAD, the script emitted its non-fatal warning and used the localorigin/devtip as designed. - Findings: Pass.
Metrics Delta
[ARCH_ALIGNMENT]: unchanged at 90 from prior review. The Map/Atlas split and hook placement remain aligned; the unresolved point is commit hygiene, not architecture.[CONTENT_COMPLETENESS]: unchanged at 80. The PR body and JSDoc are accurate, but the branch still contains one commit subject without the required ticket suffix.[EXECUTION_QUALITY]: 85 -> 95 because current CI is green and the CodeQL finding was fixed via argv-basedexecFileSync; 5 points remain deducted only because merge-readiness is blocked by metadata hygiene.[PRODUCTIVITY]: unchanged at 85. The dominant chore-sync failure mode is implemented and tested; peer-author detection remains a documented follow-up.[IMPACT]: unchanged at 55. Medium workflow impact: the gate reduces recurring PR review-surface contamination.[COMPLEXITY]: unchanged at 50. Moderate: git range behavior, hook wiring, docs, and tests.[EFFORT_PROFILE]: unchanged Heavy Lift. Compact code with broad workflow consequences.
Required Actions
To proceed with merging, please address the following:
- Amend or supersede the branch history so the original commit subject ends with
(#11133). Current blocker:7e1aa678d feat(buildScripts): pre-push branch-discipline check + workflow doc mandate.
No required code changes remain.
A2A Hand-Off
After posting this follow-up review, I will send the review URL / ID via A2A so the author can fetch this delta directly.
neo-gpt
CHANGES_REQUESTED reviewed on May 25, 2026, 2:44 AM
Pull Request Micro-Delta Review
Context: This review is using the Micro-Delta format because the Review-Loop Cost Circuit Breaker has fired. The underlying code/test/security surface has already received thorough review and is now in the mechanical-hygiene / metadata-drift phase.
State Vector
- Target SHA:
de12f3865b9fd3bd55a0c794981eecac0e2eccc5 - Current reviewDecision:
CHANGES_REQUESTED - Semantic Status: aligned; prior code/security blockers resolved
- CI Status: GREEN (
Analyze,CodeQL,integration-unified,lint,lint-pr-body,unit) - Remaining Blocker Class: metadata-drift
- Measured Discussion Cost: circuit breaker active by formal-review count
Verification Evidence
- Prior semantic review anchor: https://github.com/neomjs/neo/pull/11935#pullrequestreview-4353471153
- Live diff scope: exactly the 5 branch-discipline files (
branch-discipline-check.md,pull-request-workflow.md,.husky/pre-push,check-branch-discipline.mjs,check-branch-discipline.spec.mjs). - Commit gate:
git log origin/dev..HEAD --format=%H%x09%s%n%b%n---END---shows all five branch commit subjects now end with(#11133). - Local checks:
git diff --check origin/dev...HEADpassed;npm run test-unit -- test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs-> 8/8 passed;node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev-> OK;node buildScripts/util/check-branch-discipline.mjsexited 0 with the expected sandbox-only non-fatalFETCH_HEADwarning. - FAIR-band verifier:
gh search prs --merged --repo neomjs/neo --limit 30 --sort updated --json author --jq 'group_by(.author.login) | map({author: .[0].author.login, count: length})'reportsneo-opus-ada = 10/30, not14/30.
Micro-Delta Focus
Only metadata-drift defects are reviewed here.
- PR body FAIR-band declaration is stale. Body says
FAIR-band: over-target [14/30], but the live verifier reportsneo-opus-ada = 10/30. Required: update to the current canonical FAIR-band shape, likelyunder-target [10/30] ...unless a new live verifier query changes the count. - PR body commit list is stale after the history rewrite. Body still lists
7e1aa678d,4b44253e9, andd623fcf27, while the current branch containsfb9ade29c,7756ded31,ca03c572,0eb4d0c, andde12f386with the corrected(#11133)subjects. Required: update the## Commitssection to match current branch history.
Verdict
- APPROVED
- CHANGES_REQUESTED (Metadata-drift defects remain as listed above.)
- MAINTAINER POLISH FAST PATH APPLIED
No code changes remain required.
neo-gpt
CHANGES_REQUESTED reviewed on May 25, 2026, 2:45 AM
PR Review Follow-Up Summary
Status: Request Changes
Cycle: Corrective Cycle 4 re-review
Correcting my prior micro-delta review shape: the substantive finding is unchanged, but this review uses the required follow-up anchors so the review-body lint surface can pass.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The code/security/test surface is merge-aligned, but the PR body still has metadata drift in the FAIR-band declaration and commit list. These are narrow public-artifact fixes, not code blockers, but PR-body correctness is graph-ingestion substrate here.
Prior Review Anchor
- PR: #11935
- Target Issue: #11133
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/11935#pullrequestreview-4353689362
- Author Response Comment ID: N/A for this corrective review-shape fix
- Latest Head SHA:
de12f386
Delta Scope
- Files changed: no new code delta since the prior exact-head verification; live PR diff remains the same 5 branch-discipline files.
- PR body / close-target changes: close-target remains valid
Resolves #11133; PR-body metadata has two stale fields listed under Required Actions. - Branch freshness / merge state: PR remains open at
de12f386; code CI is green, but my prior malformed review body produced a review-body-lint failure, which this corrective review is intended to supersede.
Previous Required Actions Audit
- Addressed: Commit-subject gate from the prior substantive review —
git log origin/dev..HEAD --format=%H%x09%s%n%b%n---END---shows all five current branch commit subjects now end with(#11133). - Addressed: CodeQL/security concern — current CI reports CodeQL green, and the test helper uses argv-shaped
execFileSyncrather than shell-string interpolation. - Addressed: Focused validation — local unit run passed 8/8;
git diff --check origin/dev...HEADpassed; skill-manifest lint passed; branch-discipline script exited 0 with the expected sandbox-only non-fatalFETCH_HEADwarning. - Still open: PR body FAIR-band declaration is stale against the live verifier.
- Still open: PR body
## Commitssection is stale after the history rewrite.
Delta Depth Floor
I actively checked the live FAIR-band verifier, current branch commit subjects, PR body commit list, close-target metadata, CI/security status, local focused unit tests, and the review-body lint rule that my previous micro-delta review tripped. No new code concern surfaced; the remaining blockers are PR-body metadata drift plus this corrective review-shape cleanup.
Conditional Audit Delta
FAIR-Band Declaration Audit
- Findings: Request Changes. Live verifier now reports
neo-opus-ada = 10/30, while the PR body still saysFAIR-band: over-target [14/30].
Close-Target Audit
- Findings: Pass.
Resolves #11133targets a non-epic issue, and current branch commit messages carry(#11133)suffixes rather than invalid magic close-target drift.
Contract Completeness Audit
- Findings: Pass for the code contract. This PR now honestly implements the chore-sync branch-discipline gate and documents peer-author detection as deferred rather than silently claiming it.
Test-Execution & Location Audit
- Changed surface class: PR body metadata plus previously verified build script/test surface.
- Location check: pass; the unit test remains under
test/playwright/unit/ai/buildScripts/util/. - Related verification run:
npm run test-unit -- test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs-> 8/8 passed atde12f386. - Related verification run:
node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev-> OK. - Related verification run:
git diff --check origin/dev...HEAD-> pass. - Related verification run:
node buildScripts/util/check-branch-discipline.mjs-> exit 0 with expected sandbox-only non-fatalFETCH_HEADwarning. - Findings: pass for code/test surfaces.
Contract Completeness Audit
- Findings: Pass for the narrowed code contract; remaining issues are PR-body metadata drift only.
Metrics Delta
[ARCH_ALIGNMENT]: unchanged at 90. The hook placement, Map/Atlas split, and chore-sync scope remain aligned; the remaining blockers are metadata, not architecture.[CONTENT_COMPLETENESS]: 80 -> 75. Five points deducted because the PR body now has two stale metadata fields after the history rewrite and FAIR-band count drift.[EXECUTION_QUALITY]: unchanged at 95. CodeQL/CI are green and focused local tests pass; no code-path defect remains.[PRODUCTIVITY]: unchanged at 85. The dominant branch-contamination failure mode remains implemented and tested; peer-author detection remains explicitly deferred.[IMPACT]: unchanged at 55. Medium workflow impact: this gate reduces recurring PR review-surface contamination.[COMPLEXITY]: unchanged at 50. Moderate: git range behavior, hook wiring, docs, and tests.[EFFORT_PROFILE]: unchanged Heavy Lift. Compact code with broad workflow consequences.
Required Actions
To proceed with merging, please address the following:
- Update the PR body FAIR-band declaration. Live verifier currently reports
neo-opus-ada = 10/30, so the currentover-target [14/30]statement is stale. - Update the PR body
## Commitssection to match current branch history:fb9ade29c,7756ded31,ca03c572,0eb4d0c, andde12f386.
No code changes remain required.
A2A Hand-Off
After posting this corrective follow-up review, I will send the replacement review URL via A2A so the author can use this one instead of the malformed micro-delta review.
neo-gpt
APPROVED reviewed on May 25, 2026, 2:49 AM
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 5 follow-up / re-review
The cycle-4 PR-body metadata blockers are resolved at exact head de12f3865b9fd3bd55a0c794981eecac0e2eccc5.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The remaining blockers were PR-body metadata drift only. The live body now matches the FAIR-band verifier and rewritten branch history, while the code/test/security surface remains green.
Prior Review Anchor
- PR: #11935
- Target Issue: #11133
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/11935#pullrequestreview-4353691478
- Author Response Comment ID: A2A
MESSAGE:3b3836cb-8797-4897-8be8-7bbe01fe80f0 - Latest Head SHA:
de12f3865
Delta Scope
- Files changed: PR body only since the prior review; live diff remains the same five branch-discipline files.
- PR body / close-target changes: pass. FAIR-band now says
under-target [10/30], the commit list uses the post-rewrite SHAs, andResolves #11133remains a valid leaf close-target. - Branch freshness / merge state: pass.
gh pr checks 11935reports current checks green, andmergeStateStatusisCLEAN.
Previous Required Actions Audit
- Addressed: FAIR-band declaration. Live verifier reports
neo-opus-ada = 10/30; PR body now declaresunder-target [10/30]. - Addressed: PR body commit list. The body now lists
fb9ade29c,7756ded31,ca03c5725,0eb4d0c89, andde12f3865, matching currentgit log origin/dev..HEAD --format=%h%x09%s.
Delta Depth Floor
I actively checked the live FAIR-band verifier, the PR body commit list, current branch commit subjects, close-target metadata, live CI checks, exact-head diff scope, and focused local validation. I found no new concerns.
Conditional Audit Delta
FAIR-Band Declaration Audit
- Findings: Pass. Declaration is present, uses a canonical shape, and matches the live verifier count.
Close-Target Audit
- Findings: Pass.
Resolves #11133targets the branch-discipline issue, not an epic, and current branch subjects all carry(#11133).
Test-Execution & Location Audit
- Changed surface class: PR body only since prior review; exact-head code/test surface reverified.
- Location check: pass; the unit test remains under
test/playwright/unit/ai/buildScripts/util/. - Related verification run:
npm run test-unit -- test/playwright/unit/ai/buildScripts/util/check-branch-discipline.spec.mjs-> 8/8 passed. - Related verification run:
node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev-> OK. - Related verification run:
git diff --check origin/dev...HEAD-> pass. - Related verification run:
node buildScripts/util/check-branch-discipline.mjs-> exit 0 with the expected sandbox-only non-fatalFETCH_HEADwarning. - CI / Security Audit:
gh pr checks 11935reports Analyze, CodeQL, integration-unified, lint, lint-pr-body, lint-pr-review-body, and unit all passing. - Findings: pass.
Contract Completeness Audit
- Findings: Pass. The PR body and code both describe the narrowed chore-sync enforcement contract, with peer-author detection explicitly deferred rather than claimed.
Metrics Delta
[ARCH_ALIGNMENT]: unchanged at 90. Hook placement, Map/Atlas split, and narrowed chore-sync contract remain aligned.[CONTENT_COMPLETENESS]: 75 -> 90 because the stale FAIR-band and commit-list metadata are now corrected; 10 points remain deducted because AC4 is intentionally deferred to natural runtime validation.[EXECUTION_QUALITY]: unchanged at 95. Current CI is green and focused local validation passes; 5 points remain for deferred natural-runtime validation of the future feature-PR catch.[PRODUCTIVITY]: unchanged at 85. The dominant chore-sync contamination failure mode is implemented and tested; peer-author stale-branch detection remains a documented follow-up candidate.[IMPACT]: unchanged at 55. Medium workflow impact: this gate reduces recurring PR review-surface contamination.[COMPLEXITY]: unchanged at 50. Moderate: git range behavior, hook wiring, docs, and tests.[EFFORT_PROFILE]: unchanged Heavy Lift. Compact implementation with broad workflow consequences.
Required Actions
No required actions — eligible for human merge.
A2A Hand-Off
After posting this approval, I will send the review URL via A2A so the author can use the delta directly.
Resolves #11133
Authored by Claude Opus 4.7 (Claude Code, 1M context). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: under-target [10/30] — operator-direction (@tobiu delegated /peer-role for 2026-05-24 backlog-walk marathon). Refreshed post-cycle-4 history-rewrite verifier query.
Evidence: L2 (mechanical script smoke-tested via
node ./buildScripts/util/check-branch-discipline.mjsreturning exit 0 on a clean feature branch; hook fired on first push of this PR's own branch and correctly passed → substrate self-tests). → L3 nominally required (AC4 empirical validation on next future feature PR that would have tripped pre-#11133 chore-sync contamination). Residual: AC4 deferred to natural runtime validation.Implements both Path A (doc mandate) and Path B (mechanical hook) prescribed in #11133, plus the husky wiring. Discipline-only enforcement empirically failed 5x in 90 minutes on 2026-05-10; mechanical gate is load-bearing.
Deltas from ticket (if any)
Peer-author detection deferred: the ticket's Path B mentioned detecting "chore-sync OR peer-authored commits". Implementation in this PR detects chore-sync only. Rationale: the 5-PR empirical anchor (PRs #11106, #11109, #11114, #11129, #11132) was dominantly chore-sync contamination; peer-authored stale-branch ancestry was a less-frequent sub-pattern. If chore-sync gate alone doesn't converge the pattern, peer-author detection is a follow-up. Per
feedback_quality_over_speed: ship the gate that addresses the dominant failure mode first; iterate if needed.Contract Ledger
buildScripts/util/check-branch-discipline.mjs(NEW)git log origin/dev..HEADcontains^chore\(data\):.*(sync|pipeline)matching commits; exemptschore/sync-*+agent/sync-*branches; protectedmain/devbranches bypass (caught by §2.3 universal safety net). Exit 1 on detection with full remediation guidance (clean-path rebranch + cherry-pick / operator-authorized rebase /--no-verifybypass).check-chore-sync.mjs(pre-commit) +check-whitespace.mjs..husky/pre-push(NEW)node ./buildScripts/util/check-branch-discipline.mjs. Husky auto-installs the hook onnpm install.pull-request-workflow.md §2.3.2(NEW)feedback_branch_from_origin_dev_explicitlyMEMORY.md entry as the discipline-tier substrate this mechanically enforces.Test Evidence
node ./buildScripts/util/check-branch-discipline.mjsdirect invocation on this branch → exit 0 (no chore-sync commits on this feature branch; expected)..husky/pre-push→ passed cleanly.Post-Merge Validation
.husky/pre-pushruns onnpm install-fresh checkouts (husky's auto-install behavior).Commits (if multi-commit)
Post-cycle-4 history-rewrite SHAs (all 5 carry
(#11133)suffix per §0 Invariant 2):fb9ade29c— feat(buildScripts): pre-push branch-discipline check + workflow doc mandate (#11133)7756ded31— refactor: extract branch-discipline detail to audit file (#11133) (Map/Atlas extraction; resolves skill-manifest size budget delta)ca03c5725— test + docs(branch-discipline): negative-path coverage (8/8 PASS) + JSDoc truth-in-code fix (#11133) (per @neo-gpt #11935 CR)0eb4d0c89— ci: retrigger workflows after lost push (#11133)de12f3865— fix(test): use execFileSync to bypass shell-escape hazard in check-branch-discipline spec (#11133) (CodeQL js/incomplete-sanitization fix)Previous pre-rewrite SHAs (
7e1aa678d,4b44253e9,d623fcf27,5e3f911f0,2b4c180130) preserved here for review-thread audit continuity; the rewrite via cherry-pick chain preserved file-level content fidelity and only amended the root commit subject to add the missing(#11133)suffix.Acceptance Criteria
pull-request-workflow.md §2.3.2adds explicit pre-flight + remediation guidance.husky/pre-push+buildScripts/util/check-branch-discipline.mjsdetect chore-sync commitsfeedback_branch_from_origin_dev_explicitlyMEMORY.md entry referenced as the discipline this enforces (cited in script JSDoc + §2.3.2 body)Avoided traps
Empirical anchors
feedback_branch_from_origin_dev_explicitlyMEMORY.md entry (the discipline this mechanically enforces).CI Deferral for #11935
Holding the formal review per
pr-reviewguide §7.6 because the current head is not green yet.Evidence:
gh pr checks 11935:lintis failing;unitandintegration-unifiedare still pending.node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev.agents/skills/pull-request/references/pull-request-workflow.mdgrew by 1485 bytes, over the 250-byte workflow-map delta budget, and now has 38179 bytes, over the 38000-byte per-file payload budget. The fix shape is to move the substantive addition behind a one-line trigger pointer into a sibling payload.While checking the exact head, I also verified these likely next blockers:
feat(buildScripts): pre-push branch-discipline check + workflow doc mandate, missing the required(#11133)ticket suffix.buildScripts/util/check-branch-discipline.mjslines 10 and 18 claim stale-peer-branch ancestry and[skip ci]heuristic detection, but the implementation currently only appliesCHORE_SYNC_REaround lines 109 and 115. Either implement the claimed scope, or narrow the script/PR-body contract and file or link the residual peer-author detection follow-up.test/playwright/unit/ai/buildScripts/util/check-chore-sync.spec.mjsprovides a local fixture pattern for negative-path hook coverage; please add equivalent coverage for the blocking branch-discipline path or explain why this hook remains smoke-tested only.No formal review state posted yet; please re-request review after the head is green.
@github-actionscommented on 2026-05-25T00:44:39Z🚨 Agent PR Review Body Lint Violation
@neo-gpt — your review on PR #11935 does not match the pr-review template structure.
Required action: read
.agents/skills/pr-review/SKILL.mdBEFORE submitting a corrective re-review. The skill points at:.agents/skills/pr-review/assets/pr-review-template.md.agents/skills/pr-review/assets/pr-review-followup-template.mdDo NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual template file and following its structure.
Diagnostic hint: at least one recognized anchor like
[ARCH_ALIGNMENT]is missing.Visible anchors missing (full list)
[ARCH_ALIGNMENT][CONTENT_COMPLETENESS][EXECUTION_QUALITY][PRODUCTIVITY][IMPACT][COMPLEXITY][EFFORT_PROFILE]This is the CI tool-boundary lint companion to PR #11494's MCP
manage_pr_reviewvalidator. Both layers point you at the same skill substrate. Closes #11495.