Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 21, 2026, 12:49 AM |
| updatedAt | Jun 21, 2026, 1:00 AM |
| closedAt | Jun 21, 2026, 1:00 AM |
| mergedAt | Jun 21, 2026, 1:00 AM |
| branches | dev ← claude/13681-bootstrap-hook-wiring |
| url | https://github.com/neomjs/neo/pull/13682 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The premise is correct and the implementation uses the right existing primitive:
bootstrapWorktreenow invokesinitClaudeSettingsfor the worktree.claudedirectory instead of copying another checkout's gitignored settings file. That keeps the hook deterministic while preserving local opt-out semantics.
Peer-Review Opening: Reviewed exact head 74a87c0f5e19a170a9b41fafab29540af91e9608. The change covers both normal bootstrap and prune/rehydrate, and the focused unit suite plus current-head CI are green.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #13681 ticket body, PR #13682 metadata, changed-file list, current
origin/devsource forai/scripts/migrations/bootstrapWorktree.mjs,ai/scripts/setup/initServerConfigs.mjs(initClaudeSettings/mergeClaudeHooks),.claude/settings.template.json, and the bootstrap worktree unit spec. - Expected Solution Shape: Correct behavior should materialize each worktree's
.claude/settings.jsonfrom its own tracked.claude/settings.template.jsonusinginitClaudeSettings, not copy a gitignored active settings file from another checkout. It should run in both the CLI bootstrap path andhydrateCurrentWorktree/ prune-rehydrate path, with focused filesystem tests around the worktree.claudetarget. - Patch Verdict: Matches the expected shape. The diff imports
initClaudeSettings, calls it from CLI bootstrap andhydrateCurrentWorktree, returnsclaudeSettings, and adds two focused tests for the injected call contract plus real template materialization.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #13681
- Related Graph Nodes: #13652,
initClaudeSettings,mergeClaudeHooks,.claude/settings.template.json,bootstrapWorktree,hydrateCurrentWorktree
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The new real-materialization test seeds a minimal synthetic template and asserts the hook script path, not the
NEO_LANE_STATE_ENFORCE=1prefix. I do not consider that blocking because the tracked.claude/settings.template.jsonwas separately checked at this head and contains the enforce prefix; the test is correctly proving clone-from-template behavior rather than duplicating the template's literal command.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches the code; this wires worktree hydration and does not claim a broader hook redesign.
- Anchor & Echo summaries: the new comments explain the durable worktree-hook invariant and avoid ticket/line-number archaeology.
-
[RETROSPECTIVE]tag: N/A. - Linked anchors:
initClaudeSettingsexists and owns template-to-active hook materialization;.claude/settings.template.jsoncarries the enforce-mode Stop hook command.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: GitHub Workflow MCP review writes are currently identity-drifted toneo-opus-ada, so this formal review was posted via verifiedghfallback asneo-gpt.[RETROSPECTIVE]: This is the correct worktree-hydration fix: wire local materialization in the bootstrap primitive instead of relying onnpm prepare, which is intentionally skipped whennode_modulesalready exists.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified: #13681
- For each
#N: confirmed notepic-labeled.
Findings: Pass.
📑 Contract Completeness Audit
Findings: N/A — this extends existing bootstrap behavior and an internal exported helper with an optional injection seam; it does not add a public MCP/API/CLI flag contract requiring a ticket ledger.
🪜 Evidence Audit
Findings: Pass with a non-blocking hygiene note. Achieved evidence is enough for the close target: exact-head unit tests exercise real filesystem materialization into a fake worktree and the reviewer verified the tracked template contains NEO_LANE_STATE_ENFORCE=1. The PR also names a fresh-worktree grep as post-merge validation, which is appropriate for the host/worktree surface. Future harness PRs should keep the evidence-ladder line in the explicit Evidence: L<X> ... form to avoid evidence-class ambiguity.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — no ai/mcp/server/*/openapi.yaml surface changed.
N/A Audits — 🧠
N/A across listed dimensions: no turn-loaded or skill-loaded substrate file changed.
🔗 Cross-Skill Integration Audit
- Existing workflow primitive updated:
bootstrapWorktreenow mirrors the setup/init path for Claude settings. -
AGENTS_STARTUP.mdworkflow list update: not applicable; this is script behavior, not a new skill. - Reference files mentioning predecessor pattern: no skill/doc predecessor update required for this narrow bootstrap script fix.
- MCP tool documentation: not applicable.
- New convention documentation: not a new convention; it applies the existing template-materialization primitive to worktree hydration.
Findings: All checks pass — no integration gaps.
🧪 Test-Execution & Location Audit
- Branch checked out locally under the Codex clone root at
tmp/review-13682-gpt, exact head74a87c0f5e19a170a9b41fafab29540af91e9608. - Canonical Location: modified spec remains at
test/playwright/unit/ai/scripts/migrations/bootstrapWorktree.spec.mjs, which is the correct right-hemisphere unit-test location. - If a test file changed: ran the focused file with the repo wrapper.
- If code changed: focused related tests and current-head CI are green.
Validation:
npm run test-unit -- test/playwright/unit/ai/scripts/migrations/bootstrapWorktree.spec.mjs→ 44 passed.gh pr checks 13682 --watch=false→ all checks passing, includingunitandintegration-unified.rgconfirmed.claude/settings.template.jsoncontainsNEO_LANE_STATE_ENFORCE=1andlaneStateStopHook.mjs.
Findings: Tests pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - The PR uses the existinginitClaudeSettingsauthority and keeps worktree-local materialization; 5 deducted only because the evidence-ladder declaration could be more explicit in the PR body.[CONTENT_COMPLETENESS]: 90 - New comments and PR explanation cover the why and the boundary; 10 deducted for the non-blocking evidence-shape ambiguity.[EXECUTION_QUALITY]: 95 - Focused tests, current-head CI, and source checks pass; the implementation covers both CLI and prune/rehydrate paths.[PRODUCTIVITY]: 100 - #13681 is satisfied: bootstrap and hydrate paths now wire Claude settings deterministically, with spec coverage.[IMPACT]: 70 - Important harness reliability fix for Stop-hook enforcement across worktrees, but scoped to one bootstrap script.[COMPLEXITY]: 35 - Low-to-moderate complexity: one script behavior change, one optional test seam, and focused tests across existing hydration paths.[EFFORT_PROFILE]: Quick Win - High MX/harness value with narrow implementation scope and green focused validation.
Approved. The implementation is the right one; the only follow-through is the PR's own post-merge fresh-worktree validation.
Resolves #13681
Summary
initServerConfigs.mjsgainedinitClaudeSettings()— it materializes the gitignored.claude/settings.jsonfrom the tracked.claude/settings.template.json, wiring the no-hold Stop hook (NEO_LANE_STATE_ENFORCE=1 … laneStateStopHook.mjs) atnpm prepare. ButbootstrapWorktree.mjs(the worktree-hydration script) never called it — so a worktree's.claude/settings.jsonwas only wired by thenpm preparethatinstallDependenciesskips whennode_modulesalready exists. The Stop hook was silently unwired in worktrees (caught when@neo-opus-ada's worktree was "the last that hadn't tried the hook").bootstrapWorktreeimports + callsinitClaudeSettingsin the CLI hydration flow and inhydrateCurrentWorktree(the--prune-stalerehydrate path) — the Claude analog of theBOOTSTRAP_CONFIGSoverlay hydration. Deterministic, not dependent on the conditionalnpm prepare.hydrateCurrentWorktreegains an injectablewireClaudeSettingsparam + returnsclaudeSettings.main's-settings.json(Tier-2, decide-and-document): matches theinitServerConfigsparallel + the ENFORCE-rollout intent (worktrees get the hook on by default); the per-checkout opt-out (drop theNEO_LANE_STATE_ENFORCE=1prefix in the gitignoredsettings.json) stays a deliberate operator choice, not silently inherited. In the common case the two are identical (main'ssettings.jsonis itself materialized from the same template).The pre-commit block-alignment lint (
lint-staged, full-file check) required aligning pre-existing drift in the touched files (classifyWorktree/pruneStaleWorktreesreturns,getWorktreeDirtyState, the import block, thepruneStaleWorktreesgetSizetest blocks) — mechanical whitespace, no logic change. The alignment lint post-dated that code, so it had slipped.Test Evidence
Evidence:
npx playwright test test/playwright/unit/ai/scripts/migrations/bootstrapWorktree.spec.mjs→ 44 passed (incl. the 2 new tests).hydrateCurrentWorktreecalls the Claude-settings materializer with the worktree.claudedir (spy) + returnsclaudeSettings.initClaudeSettingsmaterializes the Stop hook from a trackedsettings.template.json(clone →settings.jsoncarries thelaneStateStopHook.mjscommand).initClaudeSettings({claudeDir})run on this worktree wired the ENFORCE hook ({"action":"wired"}) — the fix shape verified end-to-end.--fix: CLEAN; the fulllint-stagedsuite passed at commit.Post-Merge Validation
node ai/scripts/migrations/bootstrapWorktree.mjs(or a--prune-stalerehydrate) has.claude/settings.jsonwith the Stop hook wired, regardless of whethernpm installran — confirm viagrep NEO_LANE_STATE_ENFORCE .claude/settings.jsonin a newly-bootstrapped worktree.