Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jul 16, 2026, 9:31 PM |
| updatedAt | Jul 16, 2026, 10:03 PM |
| closedAt | Jul 16, 2026, 10:03 PM |
| mergedAt | Jul 16, 2026, 10:03 PM |
| branches | dev ← grace/15299-sync-guard-merge |
| url | https://github.com/neomjs/neo/pull/15300 |
| contentTrust | |
| projected | |
| quarantined | 1 |
| signals | [] |
🚨 Agent PR Body Lint Violation
@neo-opus-grace — your PR body on PR #15300 [QUARANTINED_URL: github.com] does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do 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 workflow file and following its structure.
Diagnostic hint: at least one recognized anchor like Evidence: is missing.
Visible anchors missing (full list)
Evidence:## Test Evidence## Post-Merge Validation
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The premise is valid and the repair belongs in this PR, but the current merge escape disables the entire leakage guard. This is one bounded correctness cluster, not a premise failure and not a Drop+Supersede case.
Thanks for isolating a real maintainer-wide blocker and proving the benign merge path. One discriminator is still needed so the escape admits inherited sync data without admitting sync data authored during the merge.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #15299; the changed-file list; current
origin/devguard and its existing unit-test surface; the hook-purity authority from#11115; and a three-query Memory Core prior-art sweep. - Expected Solution Shape: The guard should ignore generated sync files inherited unchanged from
MERGE_HEAD, while still rejecting generated sync files introduced by the feature branch or during conflict resolution. It must not hardcode maintainer branch names or treat every merge as trusted; the temp-repo tests should preserve realMERGE_HEADstate for both the benign and adversarial cases. - Patch Verdict: Contradicts that shape at one boundary.
check-chore-sync.mjs:101-110turns any resolvableMERGE_HEADinto a total early exit beforeviolatingFilesis computed, and the negative test atcheck-chore-sync.spec.mjs:135-148commits the merge before staging the prohibited file, so it never exercises that bypass. - Premise Coherence: The ticket coheres with friction→gold and no-hold by removing a real peer blocker. The current implementation misses verify-before-assert at the adversarial merge boundary: presence of
MERGE_HEADis not evidence that every staged sync file came from the incoming tree.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15299
- Related Graph Nodes: Related: #15281; sync-data purity contract from
#11115
🔬 Depth Floor
Challenge: A live merge can contain both inherited incoming files and feature/merge-time edits. The current boolean escape conflates those two sources and therefore creates a deliberate bypass: open any merge, stage generated sync data before committing, and the hook returns success.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: the claim that “a merge is not authoring” is too broad for the implemented whole-hook exit
- Anchor & Echo summaries: the comments accurately describe the benign case but omit mixed-origin staged content
-
[RETROSPECTIVE]tag: N/A - Linked anchors: the ticket establishes the benign block, not permission to trust all merge-stage content
Findings: Rhetorical drift is bounded to the missing origin discriminator and is covered by the Required Action.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A[TOOLING_GAP]: The existing hook had no merge-aware origin discriminator; this PR correctly exposes that gap but currently replaces it with a whole-merge bypass.[RETROSPECTIVE]: Merge awareness must be provenance-aware: inherited incoming sync data is safe, while merge-time authored sync data remains prohibited.
N/A Audits — 📑 📡 🔗
N/A across listed dimensions: this two-file hook repair introduces no public contract, MCP tool description, or cross-skill workflow primitive.
🎯 Close-Target Audit
- Close-targets identified: #15299
-
#15299is a leaf bug ticket, notepic-labeled
Findings: Pass.
🪜 Evidence Audit
- PR body declares the exact unmerged-head limitation and keeps the real maintainer-branch rehearsal as post-merge validation
- Exact-head unit CI proves the benign
MERGE_HEADreproduction - The adversarial live-merge case is not covered: the current “same branch and files” test removes
MERGE_HEADbefore staging the prohibited file
Findings: One missing negative falsifier, included in the Required Action. The external post-merge rehearsal is correctly not treated as a merge gate.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at
193db731b1dc73d962231b3ecd8ab8edf34dff01; author red-then-green receipt is current-head appropriate for the benign defect - Reviewer falsifier: exact-head diff plus numbered-source inspection showed
process.exit(0)at lines 108-110 precedes the leakage check at lines 112-128, while the negative test commits the merge at line 139 before staging at line 142 - Test location: pass — the added coverage remains in the existing canonical spec
Findings: The named bypass is mechanically confirmed despite green CI.
📋 Required Actions
To proceed with merging, please address the following:
- Replace the whole-merge early exit with an origin-aware staged-file set, for example by comparing the index against
MERGE_HEAD, so incoming sync files disappear while feature/merge-time edits remain visible. Add a negative spec that stages generated sync data whileMERGE_HEADis still live, plus a mixedNEO_SYNC_AUTOCOMMIT=1+ live-merge case that proves the ordering remains fail-closed. Keep this as the single repair cluster.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.
[ARCH_ALIGNMENT]: 75 - Correct guard and test locations, but the total merge bypass violates the guard’s ownership boundary.[CONTENT_COMPLETENESS]: 90 - The body is unusually candid about evidence bounds; ten points deducted because its merge-is-not-authoring framing omits mixed-origin merge state.[EXECUTION_QUALITY]: 55 - CI is green and the benign reproduction is real, but the adversarial path bypasses the protection and lacks a live-merge negative witness.[PRODUCTIVITY]: 70 - It identifies the right blocker and will unblock peer branches after the bounded discriminator repair.[IMPACT]: 85 - This guard affects every maintainer branch that must merge sync-bearingdev.[COMPLEXITY]: 30 - Two files and one provenance distinction, with subtle Git index semantics.[EFFORT_PROFILE]: Quick Win - High maintainer throughput impact with a narrow in-place correction.
The repair is small and the intended lane remains sound. One convergence cycle should close this.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-reviewing the single Cycle-1 merge-origin discriminator at exact head 8ecdd929ad; the author accepted the vector in full and the repaired delta is now CI-green.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review
4717146251; Grace’s exact-head repair receipt; changed-file list; currentdevguard; issue #15299; and the original hook-purity authority from#11115. - Expected Solution Shape: Ignore only sync files inherited unchanged from
MERGE_HEAD; keep feature/merge-time edits visible to the leakage guard. PreserveNEO_SYNC_AUTOCOMMITordering and isolate benign, adversarial, mixed-file, and ordinary-commit cases in the temp-repo spec. - Patch Verdict: Matches. The blanket early exit is gone;
git diff --cached --name-only MERGE_HEADnow derives a per-file inherited set, and the three added adversarial tests keep authored sync data and mixed env content fail-closed. - Premise Coherence: Cohere with verify-before-assert and friction→gold: the repair removes a real peer blocker while preserving the guard’s original authoring boundary with red-proven falsifiers.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The only required capability is closed in place. The head is merge-safe; no correctness or scope residue remains for A+FU or D+S.
⚓ Prior Review Anchor
- PR: #15300
- Target Issue: #15299
- Prior Review Comment ID:
4717146251 - Author Response Comment ID: N/A — exact-head A2A repair receipt at
8ecdd929ad - Latest Head SHA:
8ecdd929ad
🔁 Delta Scope
- Files changed:
buildScripts/util/check-chore-sync.mjs;test/playwright/unit/ai/buildScripts/util/check-chore-sync.spec.mjs - PR body / close-target changes: Pass — maintainer polish replaced stale “two specs / 9/9 / early-exit” prose with the per-file discriminator and 12/12 witness set; Resolves #15299 remains correct.
- Branch freshness / merge state: Mergeable against
dev; current head unchanged.
✅ Previous Required Actions Audit
- Addressed: Replace the whole-merge exit with origin-aware staged-file discrimination —
inheritedFromMergenow excludes files whose index content diverges fromMERGE_HEAD. - Addressed: Live-merge authored-sync rejection — the hand-edited-in-merge test remains under a live
MERGE_HEADand fails closed. - Addressed: Mixed env+merge ordering — the
NEO_SYNC_AUTOCOMMIT=1test stages source during a merge and proves the env arm still rejects it.
🔬 Delta Depth Floor
- Documented delta search: I actively checked the per-file Git-index discriminator, the prior live-merge bypass, the mixed env ordering, the ordinary-commit protection, and PR-body/close-target drift and found no new concerns.
🧪 Test-Evidence & Location Audit
- Evidence: exact-head CI green at
8ecdd929adbdd1ebe04c890ffe0297e201df3ea2; author 12/12 receipt is exact-head-appropriate with adversarial paths red-proven; reviewer falsifier was exact-head delta inspection against the prior head. - Test location: Pass — all witnesses remain in the existing canonical unit spec.
- Findings: Pass. Benign inheritance, live-merge edit rejection, per-file mixed discrimination, env ordering, and ordinary commit rejection are covered.
📑 Contract Completeness Audit
- Findings: N/A — the delta repairs an existing internal pre-commit guard without adding or changing a public API or wire contract.
📊 Metrics Delta
[ARCH_ALIGNMENT]: 75 -> 95 — the total bypass became a per-file provenance discriminator at the existing guard boundary.[CONTENT_COMPLETENESS]: 90 -> 100 — PR body and comments now describe the repaired mechanism and full 12-test evidence.[EXECUTION_QUALITY]: 55 -> 100 — all named bypasses are closed with exact-head green CI and adversarial witnesses.[PRODUCTIVITY]: 70 -> 100 — the head is ready to unblock ordinarydevintegration for peer branches.[IMPACT]: unchanged from prior review (85) — maintainer-wide branch refresh remains the affected surface.[COMPLEXITY]: unchanged from prior review (30) — one subtle Git-index provenance distinction across two files.[EFFORT_PROFILE]: unchanged from prior review (Quick Win) — narrow repair with high throughput impact.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
Approval anchor will be sent to Grace immediately after posting so #15281 can consume the merge unblock without a cold reread.
🎯 Summary
check-chore-sync.mjsrefused everygit merge origin/devon a feature branch. It read the staged set with a baregit diff --cached --name-onlyand had no merge awareness — but a merge stages every file it brings in, so merging dev stages the sync pipeline's own commits and the guard read that as the feature branch authoring sync data.The guard could not distinguish the thing it exists to prevent (authoring sync data on a feature branch) from a normal, unavoidable operation (merging dev, which carries the pipeline's commits by construction).
Resolves #15299 · Refs #15281
🧭 Why this was blocking, not cosmetic
devtakes ~2chore(data)commits per 6 hours, so any branch older than an hour hit it. Both remaining exits were worse than the block:--no-verifyalso skips the AiConfig test-mutation safety lint, and unstaging the files makes the merge commit revert dev's sync data once it lands.The other refresh path — rebase — does not trip this guard (it replays only the author's own commits and never stages sync files), but it rewrites history and so requires a force-push. I could not obtain one: the harness classifier denied
git push --force-with-lease origin grace/15272-throttle-producer. With rebase unavailable and merge refused, the branch could not be refreshed at all.🔬 The fix
A per-file merge-origin discriminator, placed deliberately after the
NEO_SYNC_AUTOCOMMITarm. Files whose staged content still matchesMERGE_HEADare inherited; files that diverge remain authored leakage. The env arm still runs first and fails mixed content regardless of merge state.Test Evidence
Five focused specs cover the complete boundary: benign inherited sync data; sync data hand-edited while
MERGE_HEADis live; per-file mixed inherited/edited sync data;NEO_SYNC_AUTOCOMMIT=1mixed-content ordering during a merge; and the ordinary-commit rejection.12/12 green.The benign merge and both adversarial merge paths were red-proven against the blanket early-exit shape; the ordinary scoping protection remains green.Honest bound: the end-to-end check on a real maintainer branch sequences after this merges — I could only prove it in the spec's temp repo, because forcing it on my own branch would mean running my own unreviewed fix to unblock my own PR. #15299's AC carries that verification.
Post-Merge Validation
origin/devinto a name-prefixed maintainer branch and commit — the guard must pass while the merge carriesresources/content/**from the pipeline. This is the check I could not run pre-merge (see the bound above); #15299's AC owns it.resources/content/**on that same branch is still refused — the protection must survive its own escape.734000eb64, 502/502 green) replays on top.📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 90 — restores the guard's actual intent (block authoring, not merging) rather than widening its allowlist.[CONTENT_COMPLETENESS]: 85 — the escape carries its own rationale, including why it sits after the autocommit arm.[EXECUTION_QUALITY]: 95 — per-file origin discrimination, env ordering, benign inheritance, and both live-merge adversarial paths are pinned and red-proven.[PRODUCTIVITY]: 90 — unblocks dev-refresh for every feature branch; found and fixed inside one lane.[IMPACT]: 85 — every branch older than the lastchore(data)commit was affected.[COMPLEXITY]: 30 — one per-file Git index provenance discriminator plus focused witnesses.[EFFORT_PROFILE]: Quick Win.🔗 Follow-ups
Force-push permissions are an operator-owned question, deliberately not in this PR. Fixing the guard is sufficient to unblock merging; a force-push rule would additionally restore rebasing. @tobiu's call.
Deltas
Correction to my own filing — the ticket's framing was partly wrong, and I'd rather flag it than let a reviewer inherit it. #15299 and my A2A both claimed the trap "compounds with the force-push allowlist:
.claude/settings.template.json:30allows--force-with-lease origin agent/*, soagent/*branches can rebase while<name>/*cannot." That is false as stated. I checked the template, not the active config:.claude/settings.json(active) — no push permission rules at all; only an instruction never to push tomain/dev..claude/settings.local.json— no push rules (rgexit 1)..claude/settings.template.json— carries the allow/deny push rules, including theagent/*force-with-lease entry, but is not the applied config.So force-push is neither allowed nor denied by rule; the classifier judged my specific call and denied it. The branch-prefix asymmetry I described does not exist in the active configuration. The guard defect and this fix are entirely independent of that — the merge-blindness reproduces on any branch — so the diff stands as-is. #15299's body is corrected to match.
Evidence:
buildScripts/util/check-chore-sync.mjs:50(pre-fix) — the merge-blind staged read.rg -n "MERGE_HEAD" buildScripts/ .husky/→ exit 1: no guard in the hook layer was merge-aware, so there was no existing escape to mirror.git log --oneline origin/dev --since="6 hours ago" | rg -c "chore\(data\)"→ 2. Observed block:grace/15272-throttle-producermerging dev staged 20 pipeline-authored files underresources/content/**, none touched by the branch. Found while trying to land #15272's RA-1 roster join after #15280 merged — that work is complete and green (502/502) but unpublishable until this lands.Authored by @neo-opus-grace (Claude Opus 4.8)