From 3b3c97b7c5aee7f21906b5b86656a45732d882e3 Mon Sep 17 00:00:00 2001 From: ydflow Date: Sun, 27 Sep 2026 12:18:03 +0800 Subject: [PATCH] fix(dry-run): thread { dryRun } through the loaders contribute, session save and recall use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #837 threaded LoadOptions through the config loaders, and #850 fixes the three commands that still reach them bare (pull, push, status). Three more commands load their config before their own dry-run guard and pass nothing: - contribute (--scope project loads, then --scope user / auto-detect) - session save (same three branches) - recall (detection, the inherited user scope, and the user branch) On a config pending the legacy role migration each of them rewrote ~/.teamai/config.yaml under --dry-run, printing the migration line without any [dry-run] marker; the auto-detect and project branches can also adopt a pre-#546 partition and run the single-repo self-heal bootstrap. loadLocalConfigForScope is the loader #837 missed: it now takes LoadOptions and forwards them to detectProjectConfig and both migrateLegacyRoleConfig calls. Callers that pass nothing behave as before — a real run still migrates in place. --- src/__tests__/dry-run-load-path.test.ts | 56 +++++++++++++++++++++++++ src/config.ts | 7 ++-- src/contribute.ts | 13 +++--- src/recall.ts | 8 ++-- src/save-session.ts | 11 +++-- 5 files changed, 80 insertions(+), 15 deletions(-) diff --git a/src/__tests__/dry-run-load-path.test.ts b/src/__tests__/dry-run-load-path.test.ts index 247a79a94..284714e0b 100644 --- a/src/__tests__/dry-run-load-path.test.ts +++ b/src/__tests__/dry-run-load-path.test.ts @@ -29,6 +29,9 @@ vi.mock('../utils/reports-branch.js', async (importOriginal) => ({ updateReports: vi.fn(), })); +import { contribute } from '../contribute.js'; +import { loadLocalConfigForScope } from '../config.js'; +import { recall } from '../recall.js'; import { rolesSet } from '../roles-cmd.js'; import { tagsSubscribe, tagsUnsubscribe } from '../tags.js'; import { updateReports } from '../utils/reports-branch.js'; @@ -181,3 +184,56 @@ describe.each(FIXTURES)('--dry-run on %s', (_fixture, setup) => { expect(log.info).toHaveBeenCalledWith(expect.stringContaining('[dry-run] Would')); }); }); + +describe('--dry-run through the loaders the commands share (#850)', () => { + const originalCwd = process.cwd(); + const roots: string[] = []; + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + process.chdir(originalCwd); + for (const dir of roots.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + function legacyRoot(): { root: string; configPath: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-dry-run-loader-')); + roots.push(root); + const home = path.join(root, 'home'); + fs.mkdirSync(path.join(home, '.teamai'), { recursive: true }); + vi.stubEnv('HOME', home); + process.chdir(setupLegacyRoleConfig(root)); + return { root, configPath: path.join(home, '.teamai', 'config.yaml') }; + } + + it('recall --dry-run writes no file: the user scope loads through the flag (#850)', async () => { + const { root } = legacyRoot(); + const before = snapshotTree(root); + await recall('dry run probe', { dryRun: true }); + expect(snapshotTree(root)).toEqual(before); + }); + + it('contribute --scope user --dry-run writes no file on a config pending the role migration (#850)', async () => { + const { root } = legacyRoot(); + // In the tree before the snapshot, so the run itself adds nothing. + const file = path.join(process.cwd(), 'note.md'); + fs.writeFileSync(file, 'Learned: a dry run must not migrate the teamai config.\n'); + const before = snapshotTree(root); + await contribute({ file, scope: 'user', dryRun: true }); + expect(snapshotTree(root)).toEqual(before); + }); + + it('the loader previews the legacy role migration under --dry-run and writes nothing (#850)', async () => { + const { configPath } = legacyRoot(); + const loaded = await loadLocalConfigForScope('user', undefined, { dryRun: true }); + expect(loaded?.primaryRole).toBe('hai'); + expect(fs.readFileSync(configPath, 'utf-8')).not.toContain('primaryRole'); + }); + + it('the loader still migrates in place when the caller passes nothing, as before (#850)', async () => { + const { configPath } = legacyRoot(); + const loaded = await loadLocalConfigForScope('user'); + expect(loaded?.primaryRole).toBe('hai'); + expect(fs.readFileSync(configPath, 'utf-8')).toContain('primaryRole: hai'); + }); +}); diff --git a/src/config.ts b/src/config.ts index 04cc88c99..cce792f94 100644 --- a/src/config.ts +++ b/src/config.ts @@ -229,14 +229,15 @@ async function throwTeamConfigMissingOrInvalid(repoPath: string): Promise export async function loadLocalConfigForScope( scope: Scope, projectRoot?: string, + options: LoadOptions = {}, ): Promise { if (scope === 'project') { if (!projectRoot) return null; // Reuse the single detection path so config location never drifts between // "detect the active project" and "load a named project's config". - const detected = await detectProjectConfig(projectRoot); + const detected = await detectProjectConfig(projectRoot, undefined, options); if (!detected) return null; - return migrateLegacyRoleConfig(detected, path.join(getDataHome(detected), 'config.yaml')); + return migrateLegacyRoleConfig(detected, path.join(getDataHome(detected), 'config.yaml'), options); } const configPath = getConfigPath(scope, projectRoot); const content = await readFileSafe(expandHome(configPath)); @@ -244,7 +245,7 @@ export async function loadLocalConfigForScope( try { const raw = YAML.parse(content); const parsed = LocalConfigSchema.parse(raw); - return await migrateLegacyRoleConfig(parsed, configPath); + return await migrateLegacyRoleConfig(parsed, configPath, options); } catch (e) { log.error(`Invalid ${scope} config at ${configPath}: ${describeConfigError(e)}`); return null; diff --git a/src/contribute.ts b/src/contribute.ts index 2bde9d058..0a14e9420 100644 --- a/src/contribute.ts +++ b/src/contribute.ts @@ -191,19 +191,22 @@ export async function contribute( return; } - // Init check — select scope based on --scope flag or auto-detect + // Init check — select scope based on --scope flag or auto-detect. The flag + // reaches the loaders: a bare load migrates the legacy role config in place, + // which would write under --dry-run (#850). + const loadOpts = { dryRun: options.dryRun }; let localConfig: LocalConfig; if (options.scope === 'project') { - const cfg = await loadLocalConfigForScope('project', process.cwd()); + const cfg = await loadLocalConfigForScope('project', process.cwd(), loadOpts); if (!cfg) { log.error('No project-level teamai config in this directory'); return; } localConfig = cfg; } else if (options.scope === 'user') { - const { localConfig: userCfg } = await requireInit(); + const { localConfig: userCfg } = await requireInit(loadOpts); localConfig = userCfg; } else { // Auto-detect (unchanged default behavior) - const projectConfig = await detectProjectConfig(); - localConfig = projectConfig ?? (await requireInit()).localConfig; + const projectConfig = await detectProjectConfig(undefined, undefined, loadOpts); + localConfig = projectConfig ?? (await requireInit(loadOpts)).localConfig; } assertNotReadOnly(localConfig, 'teamai contribute'); const username = localConfig.username; diff --git a/src/recall.ts b/src/recall.ts index 94905ffef..0761f2443 100644 --- a/src/recall.ts +++ b/src/recall.ts @@ -459,7 +459,9 @@ export async function recall( // not reach that scope's team (#787). let projectUnreadable = false; try { - projectConfig = await detectProjectConfig(undefined, (configPath, error) => { unreadable.push(`${configPath}: ${error}`); }); + // The flag reaches detection: a bare load migrates the legacy role config + // in place, which would write under --dry-run (#850). + projectConfig = await detectProjectConfig(undefined, (configPath, error) => { unreadable.push(`${configPath}: ${error}`); }, { dryRun: options.dryRun }); } catch (e) { // A cwd that no longer exists holds no project: user scope, as in // resolveConfigForDir. @@ -510,7 +512,7 @@ export async function recall( if (projectConfig.inheritUserScope === true) { try { - const userConfig = await loadLocalConfigForScope('user'); + const userConfig = await loadLocalConfigForScope('user', undefined, { dryRun: options.dryRun }); if (userConfig) { const result = await loadOrBuildScopeIndex(userConfig, 'user'); if (result === 'build-failed') indexBuildFailed = true; @@ -525,7 +527,7 @@ export async function recall( } else { // User mode: user scope only. try { - const { localConfig: userConfig } = await requireInit(); + const { localConfig: userConfig } = await requireInit({ dryRun: options.dryRun }); const result = await loadOrBuildScopeIndex(userConfig, 'user'); if (result === 'build-failed') indexBuildFailed = true; else if (result && result.index.entries.length > 0) { diff --git a/src/save-session.ts b/src/save-session.ts index cb7e8f4c6..6101aeeac 100644 --- a/src/save-session.ts +++ b/src/save-session.ts @@ -96,20 +96,23 @@ export async function saveSession(options: SaveSessionOptions): Promise { return; } + // The flag reaches the loaders: a bare load migrates the legacy role config + // in place, which would write under --dry-run (#850). + const loadOpts = { dryRun: options.dryRun }; let localConfig: LocalConfig; try { if (options.scope === 'project') { - const cfg = await loadLocalConfigForScope('project', process.cwd()); + const cfg = await loadLocalConfigForScope('project', process.cwd(), loadOpts); if (!cfg) { log.error('No project-level teamai config in the current directory.'); return; } localConfig = cfg; } else if (options.scope === 'user') { - localConfig = (await requireInit()).localConfig; + localConfig = (await requireInit(loadOpts)).localConfig; } else { - const projectConfig = await detectProjectConfig(); - localConfig = projectConfig ?? (await requireInit()).localConfig; + const projectConfig = await detectProjectConfig(undefined, undefined, loadOpts); + localConfig = projectConfig ?? (await requireInit(loadOpts)).localConfig; } } catch (e) { log.error(`Cannot push: ${(e as Error).message}`);