fix(cli): upgrade patches repo-local node_modules; doctor verifies two-layer commit hooks

[omercangumus]

What

Findings 1 and 2 from #1190: squad upgrade now applies the ESM import patch to the repo-local node_modules, and squad doctor fails when the pre-commit/post-commit hooks required by the two-layer/orphan backends are missing.

Why

Part of #1190 — finding 3 (teamRoot portability) lands separately; asked on the issue whether peers.<machineId>.teamRoot is the intended convention before building it.

Two gaps left a repo subtly broken after upgrading:

1. npm runs postinstall with cwd inside the installed package dir, so a global npm install -g only ever patched the global copy of vscode-jsonrpc / copilot-sdk — the consumer repo's node_modules stayed unpatched and squad doctor kept failing its squad-cli fails with ERR_MODULE_NOT_FOUND on Node 22 (vscode-jsonrpc ESM import) #449 checks while the patcher reported "already patched — skipping". The process.cwd() entry added to SEARCH_ROOTS in 2d9f0b4 couldn't help: both patch functions returned at the first root that already had a patched copy, which on a global install is always the global one, so the cwd entry was effectively dead code.
2. doctor's hook check only covered the four sync hooks (pre-push/post-merge/post-rewrite/post-checkout). A two-layer repo missing pre-commit/post-commit — the hooks that actually guard and flush state to the squad-state branch — passed the check while the state branch sat dormant, which is the exact post-squad upgrade --self --insider: misplaced templates, Rai not installed, --state-backend ignored #1185 condition described in the issue.

How

• scripts/patch-esm-imports.mjs: both patch functions now iterate every search root instead of returning at the first hit, log which path they patched or skipped, and are exported. A direct-invocation guard keeps node scripts/patch-esm-imports.mjs (postinstall) behavior unchanged.
• core/upgrade.ts: new ensureEsmImportsPatched(dest) locates the script by walking up from the compiled file (same approach as getTemplatesDir()), imports it, and points it at <dest>/node_modules. Called from runEnsureChecks(), so it covers both the normal and already-current upgrade paths — rerunning squad upgrade on an up-to-date repo still repairs the patch.
• commands/doctor.ts: REQUIRED_SYNC_HOOKS now includes pre-commit/post-commit, matching the full set install-hooks.ts installs and ensureHooksForBackend verifies. The existing fail message already names each missing hook and points at squad install-hooks.

Testing

• New test/cli/patch-esm-imports.test.ts (6 tests): the all-roots regression for both layers (an already-patched earlier root no longer masks a later unpatched one), idempotency, and ensureEsmImportsPatched wiring against a temp project's node_modules.
• test/cli/doctor.test.ts: new regression test for the exact bug: upgrade pipeline gaps — postinstall misses repo-local node_modules, two-layer hooks not installed, teamRoot not portable #1190 state (four sync hooks present, commit hooks missing → fail, message contains both hook names and the remediation); the two existing PASS fixtures updated to install all six hooks.
• npm run build — passes. npm run lint (tsc) — passes. npm run lint:eslint — 0 errors (the new helper carries the same pre-existing n/no-sync warning class the rest of core/upgrade.ts already has).
• npx vitest run test/cli/patch-esm-imports.test.ts test/cli/doctor.test.ts — 42 passed.
• Full npm test on this Windows box: 59 failed / 6962 passed with these changes vs 64 failed / 6950 passed on a clean upstream/dev checkout using the same build+run procedure. The failing sets shift run to run and are the known local timing/locale flakes (5s timeouts in journey/e2e/shell tests, a unicode-vs-ascii rendering assertion); every test that failed only in the with-changes run passes standalone against the same build, and 50 of the failures are common to both runs.

---

⚠️ Quick Check

• Changeset added: .changeset/fix-1190-esm-patch-doctor-commit-hooks.md (patch bump, @bradygaster/squad-cli only — no SDK or governed template paths touched)

PR Readiness Checklist

Branch & Commit

• Branch created from dev
• Branch is up to date with dev
• Verified diff contains only intended changes (git diff --cached --stat, plus a git diff -w pass for whitespace-only hunks — none)
• PR is not in draft mode
• Commit history is clean (single commit)

Build & Test

• npm run build passes
• npm test passes (see testing notes above for pre-existing unrelated local failures)
• npm run lint passes (type check clean)
• npm run lint:eslint passes (0 errors)

Changeset

• Changeset added (patch, @bradygaster/squad-cli)

Docs

• N/A — behavior fix inside existing upgrade/doctor flows, no user-facing API change

Exports

• N/A — ensureEsmImportsPatched is exported from core/upgrade, which already has a subpath export

---

Breaking Changes

None.

Waivers

None.

[github-actions]

🟡 Impact Analysis — PR #1463

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	6
Files added	2
Files modified	4
Files deleted	0
Modules touched	3

🎯 Risk Factors

• 6 files changed (6-20 → MEDIUM)
• 3 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)

• .changeset/fix-1190-esm-patch-doctor-commit-hooks.md

squad-cli (3 files)

• packages/squad-cli/scripts/patch-esm-imports.mjs
• packages/squad-cli/src/cli/commands/doctor.ts
• packages/squad-cli/src/cli/core/upgrade.ts

tests (2 files)

• test/cli/doctor.test.ts
• test/cli/patch-esm-imports.test.ts

---

This report is generated automatically for every PR. See #733 for details.

[github-actions]

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit f00a2d0

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 2 item(s) to address before review

Status	Check	Details
✅	Single commit	1 commit — clean history
✅	Not in draft	Ready for review
✅	Branch up to date	Up to date with dev
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	Changeset file found
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
✅	Copilot threads resolved	No Copilot review threads
❌	CI passing	7 check(s) still running

Files Changed (6 files, +273 −22)

File	+/−
.changeset/fix-1190-esm-patch-doctor-commit-hooks.md	+5 −0
packages/squad-cli/scripts/patch-esm-imports.mjs	+26 −19
packages/squad-cli/src/cli/commands/doctor.ts	+3 −1
packages/squad-cli/src/cli/core/upgrade.ts	+51 −0
test/cli/doctor.test.ts	+26 −2
test/cli/patch-esm-imports.test.ts	+162 −0

Total: +273 −22

---

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

[github-actions]

🏗️ Architectural Review

⚠️ Architectural review: 1 warning(s).

Severity	Category	Finding	Files
🟡 warning	bootstrap-area	1 file(s) in the bootstrap area (packages/squad-cli/src/cli/core/) were modified. These files must maintain zero external dependencies. Review carefully.	packages/squad-cli/src/cli/core/upgrade.ts

---

Automated architectural review — informational only.

[tamirdresher]

✅ Reviewed by Squad (Data). Two real bugs, two targeted fixes. The early-return → continue fix for the ESM patcher is obviously correct, and extending REQUIRED_SYNC_HOOKS to include pre-commit/post-commit closes the doctor gap for two-layer state. 6+ regression tests cover exact failure scenarios from #1190.