fix(pull): report hooks and MCP entry warnings on a dry run

[ydflow]

What

pull --dry-run returned before the hooks and MCP reconcile stages, so the warnings those stages raise never reached the maintainer who ran the dry run to see exactly them (#822, item 3):

teamai pull --dry-run
  reconcileHooksAllScopes   if (dryRun) return
  reconcileMcpAllScopes     if (dryRun) return
today     no unknown-id, `roles:` deprecation or conflict warnings
expected  resolve and warn, then skip the write

A dry run now resolves the entries and reports what it finds, then skips the writes. The MCP summary line matches: on a dry run it reads Would make N change(s)… and omits the Restart your AI tool session instruction, since nothing was written for a session to load.

Why

The warnings come from the entry resolution, which is read-only: resolveEntriesFor → reportEntryResolution (unknown ids, a deprecated per-entry roles:, a hooks.yaml that does not parse). Only the later reconcile writes. Returning early skipped the resolve, which is the half that carries the information a maintainer wants from --dry-run.

How

• MCP — McpReconcileOptions.dryRun already gates every write in mcp-reconcile.ts, and teamai mcp inject --dry-run uses it. reconcileMcpAllScopes only forwards it; no new mechanism. Its summary line branches on the dry run: Would make N change(s) across M server(s) when nothing was written, the applied wording with the session-restart hint on a real pull.

• hooks — had no dry-run path at all. reconcileTeamHooksForConfig takes dryRun, resolves the entries, reports them, and returns before the first write (reconcileHooksToAllTools, the copilot branch, and sweepLegacyProjectHooks). resolveTeamHooks takes preview so the transparency line reads "Would apply N team hook(s)" rather than claiming they were applied. One report happens inside the per-tool pass, so the dry run repeats it before stopping: Pi cannot run custom team hooks, and a tools: [pi] hook must draw the same "Pi supports built-in lifecycle hooks only; skipping N custom team hook(s)" warning a real pull prints.

• docs — docs/designs/multi-project-management.md no longer says that pull --dry-run prints no hooks or MCP warnings; it now describes the resolve-and-warn behavior this PR ships.

reconcileCoAuthorAllScopes keeps its early return: it is write-only with no warnings to report, so a dry run has nothing to show there.

Tests

src/__tests__/pull-dry-run-hooks-mcp.test.ts drives pull() the way pull-env-shape-warning.test.ts does, since the defect was in the orchestration layer:

• warns about the deprecated per-entry roles: key on a dry run
• writes no hook settings or manifest on a dry run (saveStateForScope also untouched)
• still warns on a real pull, so the dry run reports what would happen
• forwards dryRun to the MCP reconcile
• reports MCP changes on a dry run without claiming they were applied (Would make …, no restart line) — RED on the previous head
• keeps the applied wording on a real pull
• reports the Pi skip on a dry run, like a real pull would (RED without the fix)

RED first: the dry-run cases failed before their fixes (the MCP-summary one on the previous head), the behavior-preserving cases passed.

Real-CLI verification (ea39829)

npm run build, then node dist/index.js against a sandbox HOME and a local team-repo fixture whose hooks/hooks.yaml carries the deprecated per-entry roles: key and whose mcp/mcp.yaml declares one stdio server.

pull --dry-run --force — warns, previews, writes nothing:

⚠ hooks/hooks.yaml: hook "lint" is scoped with per-entry `roles:`, which is deprecated and stops working in the next minor release. Move it to hooks/dev/hooks.yaml (declare hooks: [dev] for role dev in manifest/roles.yaml) and drop the key.
ℹ Would apply 1 team hook(s):
ℹ   [lint] teamai hook-dispatch post-tool-use
⚠ Pi supports built-in lifecycle hooks only; skipping 1 custom team hook(s) from hooks/hooks.yaml
ℹ MCP: [dry-run] Would make 1 change(s) across 1 server(s)

Afterwards the sandbox HOME held only the pre-existing config.yaml and debug.log — no .claude/settings.json, no .claude.json, no .teamai/managed-mcp.json.

pull --force on the same fixture — same warning, applied wording, writes:

⚠ hooks/hooks.yaml: hook "lint" is scoped with per-entry `roles:`, which is deprecated and stops working in the next minor release. …
ℹ Applying 1 team hook(s):
ℹ   [lint] teamai hook-dispatch post-tool-use
✔ Updated teamai hooks in …\home\.claude\settings.json
⚠ Pi supports built-in lifecycle hooks only; skipping 1 custom team hook(s) from hooks/hooks.yaml
ℹ MCP: 1 change(s) across 1 server(s). Restart your AI tool session to load them.

Verified locally: vitest run src/__tests__/pull-dry-run-hooks-mcp.test.ts → 7 passed; tsc --noEmit → exit 0. The full suite takes ~13.5 minutes on this Windows machine and fails the same way on the merge-base as on this branch, so I ran it on both: 201 failed / 4770 passed on 87a606b, 198 failed / 4779 passed on this head — the branch adds 7 tests and introduces no new local failure. The failures are Windows-environment classes on both sides (EPERM symlinks without privilege, POSIX path-separator and 0o600 file-mode assertions, npm ENOENT in spawnSync, git CRLF churn, 15s timeouts on git-heavy tests); the same suite is green in CI on ubuntu/macos × Node 20/22.

[ydflow]

Checked for overlap with the open PRs that touch the same files:

• refactor(providers): unify Git/HTTP resource backends, ClawPro as HTTP adapter (#404) #780 (refactor(providers)) changes src/hooks.ts once, around reconcilePiExtension (line ~1415). This PR changes reconcileTeamHooksForConfig (~1791) and resolveTeamHooks (~154) — different functions, no textual overlap, so git should merge them cleanly.
• fix(data-layout): let every self-mode worktree publish learnings and keep its queue (#808) #814 (fix(data-layout)) changes src/pull.ts in six hunks, all inside pullForScope and the import block (lines ≤1344). This PR changes reconcileHooksAllScopes and reconcileMcpAllScopes (lines ~2108-2160) — same file, disjoint regions.

Happy to rebase on either once they land.

[ydflow]

Added a real-CLI check, since the unit tests drive pull() through mocks.

Built dist/index.js (npm run build) and ran it against a sandbox with an isolated HOME and a team repo whose hooks/hooks.yaml carries the deprecated per-entry roles: key.

pull --dry-run --force — warns, writes nothing:

⚠ hooks/hooks.yaml: hook "lint" is scoped with per-entry `roles:`, which is deprecated
  and stops working in the next minor release. Move it to hooks/dev/hooks.yaml ...
ℹ Would apply 1 team hook(s):
ℹ   [lint] teamai hook-dispatch post-tool-use

After it, the sandbox HOME held only the pre-existing config.yaml and debug.log — no .claude/settings.json, no .teamai/managed-hooks.json, and the team repo was unmodified.

pull --force on the same fixture — same warning, and it does write:

⚠ hooks/hooks.yaml: hook "lint" is scoped with per-entry `roles:`, which is deprecated ...
ℹ Applying 1 team hook(s):
✔ Updated teamai hooks in ...\.claude\settings.json

So the dry run reports exactly what a real pull would say, with Would apply in place of Applying, and the write is the only difference. (✖ Pull failed: fatal: 'origin' does not appear to be a git repository in the transcript is the sandbox having no git remote — it does not gate the reconcile stage, which runs after it.)

[github-actions]

Findings

• [P1 blocking] src/pull.ts:2162 reports dry-run MCP changes as completed. When an MCP addition/update/removal is pending, reconcileMcpForConfig(..., { dryRun: true }) returns it in changes, causing “Restart your AI tool session to load them” even though nothing was written. Use dry-run wording such as “Would make…” and omit the restart instruction.
• [P1 blocking] src/pull.ts:2156 changes documented behavior, but docs/designs/multi-project-management.md:442 still states that pull --dry-run prints no hooks or MCP warnings. This directly violates the repository rule requiring behavior changes to update every affected document, including docs/designs/.
• [P1 blocking] The PR changes runtime behavior but its testing record contains only Vitest and TypeScript checks, with no representative built real-CLI/end-to-end run. The Code Review Rules explicitly require an end-to-end/real-CLI verification record for runtime changes.

[jeff-r2026]

Please resolve the P1 findings.

[github-actions]

Findings

• [P1 blocking] src/pull.ts:2127 still claims hooks were reconciled during a dry run. With debug logging enabled and at least one applicable team hook, pull --dry-run prints [scope] Reconciled N team hook(s) despite writing nothing. Guard this message or use preview wording.
• [P1 blocking] The PR changes runtime behavior, but the description still documents only Vitest and TypeScript checks—not a built real-CLI/end-to-end run. This violates the explicit Code Review Rules requirement for one representative real-CLI verification.

Resolved

• The MCP dry-run summary no longer claims changes were applied or requests a restart.
• docs/designs/multi-project-management.md now documents the new dry-run behavior.

[ydflow]

All findings are resolved on 39c0017 + 256090b:

• MCP summary on a dry run — now reads MCP: [dry-run] Would make N change(s) across M server(s) and omits the restart instruction; a real pull keeps the applied wording. The new test is RED on 6c4fe2af and green on 39c0017.
• docs/designs/multi-project-management.md — no longer says pull --dry-run prints no hooks or MCP warnings; it now describes the resolve-and-warn behavior.
• Testing record — a built real-CLI run is now in the PR description (the earlier review snapshot predates that edit): npm run build → dist/index.js against a sandbox HOME and a local team-repo fixture with the deprecated per-entry roles: key and one stdio MCP server. Dry run warns, previews Would make 1 change(s), and writes nothing (no .claude/settings.json, no .claude.json, no managed-mcp.json); real pull applies with the restart line.
• [scope] Reconciled N team hook(s) debug line — same preview rule now: Would apply N team hook(s) on a dry run, Reconciled on a real pull, pinned by the dry-run tests.

The full suite still exceeds ~13 minutes on this Windows machine, so the authoritative full-suite signal remains CI (green on ubuntu/macos × Node 20/22); the PR description documents the representative real-CLI run the review rules ask for.

#814 landed while this PR was open; the regions are disjoint and GitHub reports the branch mergeable, so I have not rebased — happy to if you prefer.

[github-actions]

Findings

• [P1 blocking] src/hooks.ts:1832 returns before tool-specific validation. With Pi installed and a hook scoped to tools: [pi], dry-run prints “Would apply 1 team hook(s)” while a real pull applies none and warns that Pi skips custom hooks (src/hooks.ts:1633). The preview therefore still misreports the actual outcome.

Resolved

• MCP dry-run wording and restart guidance are corrected.
• Hook debug output now uses preview wording.
• The design document is updated.
• The PR description now includes sufficient real-CLI verification.

[ydflow]

The Pi finding is fixed on ea39829: the "Pi supports built-in lifecycle hooks only; skipping N custom team hook(s)" report is extracted from the per-tool loop, and the dry run now prints it before it stops — gated on the same toolPaths and agent filters the real pass uses, so the preview matches a real reconcile line for line. The new test is RED on 256090b and green on ea39829, and the real-CLI record in the description shows the warning on both the dry run and the real pull.

Also verified the full-suite question from the description with a baseline run: 201 failed / 4770 passed on the merge-base (87a606b) vs 198 failed / 4779 passed on this head — the ~200 local failures pre-exist on main (Windows-environment classes), and the branch adds tests without adding a failure. Details in the description.

[github-actions]

Findings

• None.

Resolved

• The Pi-specific dry-run warning now matches the real reconcile path.
• Earlier MCP wording, hook debug output, and design-document findings remain fixed.

Testing

• The PR description includes a representative built real-CLI dry-run and real pull, satisfying the testing requirement. The recorded commit predates the current head, which is non-blocking under the review rules.