Repository navigation
[bug] pull / push / status --dry-run still write: the loaders they use take no { dryRun } #850
Description
Summary
--dry-run is documented as "Preview mode, no changes made", and #837 made that true for tags subscribe, tags unsubscribe and roles set. It is still not true for pull, push and status: the scope-detection block they run before any dry-run guard calls config loaders that were never given { dryRun }, so a preview still persists the legacy role migration — and, in a git repo, can still adopt a pre-#546 partition and run the single-repo self-heal bootstrap.
#837 named exactly this and left it out:
pull --dry-run,push --dry-runandstatusreach the same writes through loaders that do not pass{ dryRun }(loadLocalConfigForScope, andautoDetectInit/detectProjectConfigwithout options). They are outside #836 and are not changed here. Callers that pass nothing behave as before.
All line numbers and code below were re-checked on main @ e0bf2e96 (blobs: src/config.ts 04cc88c9, src/pull.ts 2faf8492, src/push.ts 0ad93fd7, src/__tests__/dry-run-load-path.test.ts 247a79a9).
Root cause
#837 threaded LoadOptions into loadLocalConfig, requireInit, autoDetectInit, detectProjectConfig, resolvePartitionDir, readConfigFrom and migrateLegacyRoleConfig. loadLocalConfigForScope was missed, and it calls them bare.
src/config.ts:229 — no options parameter, three bare calls:
export async function loadLocalConfigForScope(
scope: Scope,
projectRoot?: string,
): Promise<LocalConfig | null> {
if (scope === 'project') {
if (!projectRoot) return null;
const detected = await detectProjectConfig(projectRoot); // :237 no options
if (!detected) return null;
return migrateLegacyRoleConfig(detected, path.join(getDataHome(detected), 'config.yaml')); // :239 no options
}
const configPath = getConfigPath(scope, projectRoot);
const content = await readFileSafe(expandHome(configPath));
if (!content) return null;
try {
const raw = YAML.parse(content);
const parsed = LocalConfigSchema.parse(raw);
return await migrateLegacyRoleConfig(parsed, configPath); // :247 no options
} catch (e) { … }
}Callers that run under a dry run:
| Caller | Line | Call | Note |
|---|---|---|---|
src/pull.ts |
1885 | detectProjectConfig(undefined, sink) |
options not passed |
src/pull.ts |
1914 | loadLocalConfigForScope('user') |
the loader takes no options at all |
src/push.ts |
731 | autoDetectInit() |
options not passed |
src/status.ts |
46 (and list, :252) |
autoDetectInit() |
status(options) already has options in hand |
pull() starts at pull.ts:1817. Every options.dryRun guard in that file is later (:1071, :1099, :1119, :1177, …), so the scope-detection block at ~1878–1939 runs first, unguarded.
Bare calls mean options = {}, so inside the loaders:
migrateLegacyRoleConfigskips itsif (options.dryRun) return migratedearly-out and writesconfig.yaml.resolvePartitionDir(utils/partition.ts:219) skips its dry-run branch (:223) and runsadoptLegacyPartition→fs.promises.rename(legacyDir, canonical)(:237) plus therepo.localPathrewrite that follows.selfHealAndReadPartition(config.ts:470) skipsif (options.dryRun) return previewSelfHeal(…)(:476) and runs the realbootstrapSelfRepo(:479) — partition config, state, anchor, tool dirs, hooks,git worktree add, member registration and its push, plus the provider auth callspreviewSelfBootstrapdeliberately avoids.
The plumbing itself already exists — autoDetectInit(cwd?, options = {}) (config.ts:642) forwards options to detectProjectConfig (:643). These commands simply do not supply their flag.
Reproduction — real CLI, single-variable control
CLI built from unmodified main (npm run build, node dist/index.js). One fixture — an isolated HOME holding a role-less ~/.teamai/config.yaml next to a team repo whose manifest/roles.yaml declares hai — and the only variable is the command. Tree compared by sha256 over every file, the same method as dry-run-load-path.test.ts.
| Command | Files changed | Output |
|---|---|---|
tags subscribe testing --dry-run (fixed by #837 — control) |
0 | ℹ [dry-run] Would migrate legacy teamai config to default role profile: haiℹ [dry-run] Would subscribe to: testing |
pull --dry-run |
1 — ~/.teamai/config.yaml rewritten |
ℹ Migrated legacy teamai config to default role profile: hai |
~/.teamai/config.yaml after pull --dry-run gains primaryRole: hai, resourceProfileVersion: 1, scope: user and additionalRoles: []. The control command writes nothing, so the harness is not a false negative — it does see writes, and this one is real.
Evidence level. The user-scope write above is run and observed. The project-scope writes in a git repo (partition adoption, self-heal bootstrap) are located by code path from the same unguarded calls — quoted line by line above, not yet run. I will run them before opening a PR.
Proposed fix
Same shape as #837, applied to the loader it missed:
loadLocalConfigForScope(scope, projectRoot?, options: LoadOptions = {})— passoptionsintodetectProjectConfigand bothmigrateLegacyRoleConfigcalls. Callers that pass nothing behave as before, the same compatibility promise fix(tags,roles): honor --dry-run and count namespaced skills in tags list (#836) #837 made.pull.ts:1885andpull.ts:1914pass{ dryRun: options.dryRun };push.ts:731passes it toautoDetectInit.status.ts:46andstatus.ts:252:statusandlistare read-only, so rather than reading the global flag they would pass{ dryRun: true }unconditionally — a read command should never migrate, adopt or bootstrap. Happy to use a narrower option name if you prefer one.- Extend
src/__tests__/dry-run-load-path.test.ts.COMMANDSis at:142–:146; the three fixtures (:136), the provider-call recorder (:10–:24) and theupdateReportsspy (:27–:30) already exist.pullandpushfit the existing row shape —pullemits[<scope>] [dry-run] Would …(pull.ts:1071,:1099,:1119,:1177), so the shared assertion holds.statusneeds a second assertion: it prints no[dry-run] Wouldline today, so its row should assert the absence of writes, not a message.
Scope
- In: the loader's
{ dryRun }threading and the three commands; the test matrix. - Out:
init's call sites (it has no--dry-runto honour), and any change to what a real, non-dry run writes. - Not audited, same bare-call shape:
requireInitForScope(config.ts:625),recall.ts:513,contribute.ts:197,save-session.ts:102. They may be affected the same way; I would keep this change to the three commands fix(tags,roles): honor --dry-run and count namespaced skills in tags list (#836) #837 named unless you want them swept in.
Questions
- Do you want this tracked as a follow-up under oxlint rollout: bugs found in the baseline, and next rules to evaluate #836, or as its own issue? I filed it separately because oxlint rollout: bugs found in the baseline, and next rules to evaluate #836's checklist is the oxlint rule rollout, but it can be folded in.
- For
status: pass{ dryRun: true }unconditionally, or would you rather have a distinct read-only option? - When a read-only command does not write, should it still print a
[dry-run] Would bootstrap …preview line for the bootstrap it declined to run, or stay silent?
Happy to take it if it is not already spoken for.
Activity
Smilewithoutfalling commentedon Sep 27, 2026
Filed from #837's own out-of-scope note, with the evidence and a proposed shape above.
@SaulMoro @jeff-r2026 — the two open questions at the bottom are yours to call, and I will not code before you answer:
status/listpassing{ dryRun: true }unconditionally, versus a distinct read-only option.One caveat stated in the report and repeated here: the user-scope write is run and observed, but the project-scope writes (partition adoption, self-heal bootstrap) are located by code path only. I will run those before opening a PR, and I will drop the whole thing if the fix is already in flight.
Happy to take it.
ydflow commentedon Sep 27, 2026
Filed the three "not audited, same bare-call shape" commands as #853 —
contribute,session saveandrecall, each loading its config before its own dry-run guard. The loader change is the same one your item 1 proposes (loadLocalConfigForScopegainsLoadOptions), so the two PRs apply cleanly in either order — happy to fold this into #850 instead if you'd rather land one PR. No rush on my side either way; your open questions come first.fix(dry-run): thread { dryRun } through the loaders contribute, sessi…
SaulMoro commentedon Sep 27, 2026
Thanks for the detailed write-up. On the two questions:
contribute/session save/recallpart under it.{ dryRun: true }unconditionally fromstatusandlist. They are read-only, so the preview path is exactly what they need, and fix(tags,roles): honor --dry-run and count namespaced skills in tags list (#836) #837'sLoadOptionsalready carries it. A separate read-only option would duplicate it. The migration then persists on the next command that writes.Go ahead whenever you're ready.
fix(dry-run): thread { dryRun } through the loaders pull, push, statu…