fix(dry-run): stop dry-run and read-only commands persisting config migrations (#893)

[SaulMoro]

Summary

--dry-run still persisted config migrations (a legacy role migration, a pre-#546 partition rename, a self-mode bootstrap) because many commands loaded their scope bare. #866 fixed the loaders of pull, push, status and list. This PR does the same for every other command that honours --dry-run (it forwards { dryRun }), and for read-only commands, which now load with dryRun: true unconditionally, the way status and list do.

 mcpInject(options)                                    src/mcp-cmd.ts
-  autoDetectInit()                                    LoadOptions = {} -> migration persists
+  autoDetectInit(undefined, { dryRun: options.dryRun })
   reconcileMcpForConfig(..., { dryRun })              already honoured it

 mcpList()  /  roles list, tags list, doctor, ...      read-only
-  autoDetectInit()
+  autoDetectInit(undefined, { dryRun: true })         // Read-only: ... (#893)

The same change, per call site:

Class	Commands
Forward { dryRun }	mcp inject, roles init/add/remove/update, projects add/update/remove, tags add/remove, source add/remove/add-http, remove, uninstall, packages install, import --from-iwiki/--from-mr/--from-claude/--from-repo (and --from-repo-list/--from-org through it), codebase --reconcile/--deep-enrich, models switch, the CLI's pre-command auto-migration (planMigration, queueKeptInCheckout)
Always dryRun: true (read-only)	mcp list, roles list, projects list/members, tags list, source list/browse, hooks list, members, exclude list, recall status, doctor, models list, codebase --status/--lint, skill list/show/get/path, webhook list, digest

Shared helpers gained an optional LoadOptions that defaults to {}: resolveMemberToolRoots, findUnreadableProjectConfig, detectTeam, loadLocalAgentConfig/describeLocalAgent, loadWebhookConfig, planMigration, queueKeptInCheckout. A caller that passes nothing (hooks, the Stop-hook share gate, usage tracking, the dashboard) behaves exactly as before. Under dryRun, loadLocalAgentConfig also keeps its own ~/.teamai/local-agent/config.json migrations (legacy group-binding cleanup, binding-key canonicalization, the HTTP backfill) in memory instead of saving them.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature causing existing behavior to change)
• Documentation only
• Refactor / internal cleanup

Test Plan

• npx tsc --noEmit passes
• npm run lint passes
• npx vitest run passes (340 files, 5428 passed, 1 skipped)
• Added/updated tests for the change

Unit: src/__tests__/dry-run-load-path.test.ts (the #850/#866 file), on the fixture #866 uses: a user config.yaml due for the legacy role migration.

LOAD_ONLY_COMMANDS        +22 rows   error null, whole tree unchanged, no provider call,
                                     and the loader logged its "[dry-run] Would migrate" preview
PREVIEWS (new)            20 cmds    config.yaml unchanged under --dry-run  AND
                                     the same call WITHOUT the flag migrates it (positive control)
READ_ONLY_PAST_THE_LOAD   2 rows     members, projects members: config.yaml only
PROJECT_SCOPE_COMMANDS    +1 row     the pre-command migration adopts no legacy partition

Unit: src/__tests__/local-agent.test.ts: loadLocalAgentConfig({ dryRun: true }) on a legacy group binding, on two path aliases, and on an HTTP config.yaml with no config.json. Each returns the migrated view and leaves config.json byte-identical (or absent); the backfill case also checks that the same load without the flag creates it. All three fail at c19a8b3 and pass with the fix.

Unit: src/__tests__/pull-model-namespaces.test.ts: models list team:gw on a key a 0.26.0 beta stored under the profile id alone reports it (API key: environment COMPANY_KEY) and leaves the team values file byte-identical. It fails at 3da2705 (the file gains team:gw@https://gw.company.test) and passes with the fix. The existing tests in that file that bind such a key through pull and models switch are the positive control.

• Before (this test file run against origin/main): every new row fails (config.yaml gains primaryRole: hai, or the legacy partition is renamed). Every positive control passes.
• After: 89/89 pass.

End-to-end, real CLI (npm run build, then node dist/index.js against a temp HOME holding that same legacy config; main is origin/main 671f509, branch is c19a8b3):

main   teamai mcp inject --dry-run                     exit=0  config.yaml f5a49a54 -> 97a14eee  primaryRole=1
main   teamai mcp list                                 exit=0  config.yaml f5a49a54 -> 97a14eee  primaryRole=1
main   teamai roles add ops --namespaces ops --dry-run exit=0  config.yaml f5a49a54 -> 97a14eee  primaryRole=1
branch teamai mcp inject --dry-run                     exit=0  config.yaml fa4a626f -> fa4a626f  primaryRole=0
branch teamai mcp list                                 exit=0  config.yaml fa4a626f -> fa4a626f  primaryRole=0
branch teamai roles add ops --namespaces ops --dry-run exit=0  config.yaml fa4a626f -> fa4a626f  primaryRole=0

(The hashes differ between the two runs only because the temp path is written into repo.localPath.)

teamai source list against a temp HOME whose local-agent/config.json holds a legacy group binding and two aliases of one workspace (before is c19a8b3, after is this branch):

before  teamai source list  exit=0  config.json 1d0c86df -> b16b1088  bindings=1   ⚠ Removed 1 legacy group-based workspace binding(s); ...
after   teamai source list  exit=0  config.json 24c957aa -> 24c957aa  bindings=3   ℹ [dry-run] Would remove 1 legacy group-based workspace binding(s).

teamai models list team:gw against a temp HOME whose team values file holds a beta key team:gw (before is 3da2705, after is this branch):

before  exit=0  values 5afacabd -> 8db677eb  keys=team:gw@https://gw.company.test   API key: environment COMPANY_KEY
after   exit=0  values 5afacabd -> 5afacabd  keys=team:gw                           API key: environment COMPANY_KEY

A second real-CLI probe, on a project whose partition still has its pre-#546 name: teamai pull --dry-run, push --dry-run and --dry-run contribute --file renamed the partition on origin/main and left it in place on this branch. pull without the flag still renames it. Provider-independent: the load path does not touch a provider, so one run covers git/gitlab/github. No agent-specific path is involved.

src/__tests__/push-env.test.ts fails intermittently under full-suite load on this machine, on origin/main too (a different subset of its tests each run). It passes on its own (18/18). It is unrelated to this change.

Related Issues

Closes #893

Follow-up: #900 (everything listed under "Deliberately not in this PR" below).

Notes for Reviewers

Deliberately not in this PR

These are real --dry-run bugs found while sweeping for #893. They are left out on purpose: each has a different cause or needs its own design, and including them would bury the loader fix. They are known and tracked, so please do not report them as missing from this diff.

Item	Where	Why not here	Tracked in
recall maintenance --dry-run / recall promote --dry-run run for real (prune, drafts, promote + publish)	src/index.ts:1264, :1297, :1321, :1345, :1381	Different cause: Commander gives the root --dry-run to program.opts(), and these two actions read only cmdOpts.dryRun (always undefined). Threading their loaders alone would be a no-op. Destructive, so it gets its own PR and test.	#900, fixed in #903
Dry-run writes past the loader: remove (pull + state save before its guard), source add (clone), import --from-repo* (clone + lock), models switch (re-bound keys saved), roles/projects edits (git pull)	see #900	Not a loader leak. Each command's own preview logic has to change.	#900
stats still migrates on load	src/dashboard-scope.ts:50, :103; src/session-owners.ts:82, :357	Its loads go through dashboard scope helpers shared with the pull/report hot path, one per session. A partial fix here would claim more than it does.	#900
Commands that ignore --dry-run entirely: projects set, hooks inject/remove, mcp remove, exclude add/remove, recall enable/disable, update, review --apply, codebase --extract, init, init --http	see #900	A feature, not a leak: implement a preview or refuse the flag.	#900
env list/add/remove, env exec	src/env-commands.ts	Already threaded in open PR #880, which also edits LOAD_ONLY_COMMANDS. Whichever merges second rebases the table (a trivial conflict).	#880
import --from-repo has no test row	src/import-repo.ts:290	It clones before it loads, so a row would need a network or a clone mock. The fix is the same one-line forward as its siblings, and dryRun is destructured three lines above it.	this PR, by inspection

Review record

Two-axis /code-review (standards and spec) ran three times against origin/main. Pass 1 found import --from-claude still loading bare through resolveMemberToolRoots, read-only commands missed by the first sweep (skill list/show, webhook list, digest), and a loadLocalAgentConfig change that would have printed the [dry-run] preview on real hook runs. Pass 2 found the pre-command auto-migration still renaming a partition on pull/push --dry-run (confirmed with the real CLI), skill get/path share, and that stats could only be fixed partially. All of these are fixed, or moved to the table above. Pass 3 found no spec gaps (it re-ran pull/push --dry-run and skill get share on the built CLI, and each new row fails with its fix reverted) and two minor standards points, both applied. The Codex review then found that loadLocalAgentConfig still saved config.json under dryRun, so source list could rewrite or create it. Fixed in 3da2705. A second Codex pass found that models list read the team keys through loadTeamValues without the option, so a beta key was re-bound and saved. Fixed in 277b4ae.

Merge danger

Door: two-way. Each commit forwards an option or adds a defaulted parameter, and reverting one restores the old behaviour exactly.

Blast radius: config loading for the listed commands.

• A read-only command on a config that is due for a migration now prints [dry-run] Would migrate legacy teamai config ... (on stderr under skill) instead of silently saving it. The migration persists on the next command that writes, as with status and list since fix(dry-run): thread { dryRun } through the loaders pull, push, status and list use #866.
• A migration that used to happen as a side effect of mcp list, doctor and the like now waits for a writing command. Nothing reads the migrated file in between, because every loader returns the migrated view in memory.
• source list on a local-agent/config.json due for binding cleanup prints [dry-run] Would remove ... and leaves the file alone; the next hook run or bind saves it, as before.
• models list on a key a 0.26.0 beta stored binds it to its gateway in memory only; the next models switch, models configure or pull saves it, as before.
• No hook, usage or dashboard path changes: they pass no options.

[github-actions]

• [P1 blocking] src/local-agent.ts:584 — The new dryRun option only reaches loadLocalConfig; loadLocalAgentConfig still calls saveLocalAgentConfig during legacy binding cleanup, path canonicalization, and HTTP-config backfill. Consequently, teamai source list can rewrite or create ~/.teamai/local-agent/config.json, despite being designated read-only. Guard those writes with !options.dryRun while retaining the migrated in-memory result.

[github-actions]

Findings

• [P1 blocking] src/models-cmd.ts:206 — models list still calls loadTeamValues() without { dryRun: true }. When a team API key uses the legacy 0.26.0 binding format, bindLegacyTeamKeys() returns true and saveModelInputs() rewrites the values file, so this read-only command still persists a migration. Pass the dry-run option through this call.

Review Notes

• The earlier loadLocalAgentConfig finding is resolved in the current diff.
• The PR description includes sufficient real-CLI end-to-end testing.

[github-actions]

No findings.

• The earlier loadLocalAgentConfig persistence issue is resolved at src/local-agent.ts:584.
• The earlier models list values-file persistence issue is resolved at src/models-cmd.ts:206.
• The PR description documents sufficient real-CLI end-to-end verification.
• Review was diff-only; no code, builds, tests, or installations were run.