From e1718ed0b6fb370e319686ac08619481a7e179fd Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Tue, 29 Sep 2026 12:08:21 +0200 Subject: [PATCH 1/3] fix(dry-run): let --dry-run reach recall maintenance and recall promote (#900) The root program declares --dry-run, and Commander gives it to the root wherever it is written, so these two actions, which read only their own options, never saw it: --prune --dry-run pruned and published, --update-quality --dry-run wrote AI drafts, and promote --dry-run promoted and published. Both actions now merge program.opts() as the other actions do, and load with { dryRun }. --confidence-writeback, which ignored the flag, now reports what it would write and publishes nothing. --- src/__tests__/maintenance-two-roots.test.ts | 16 +++ .../recall-maintenance-dry-run.test.ts | 101 ++++++++++++++++++ src/index.ts | 18 ++-- src/maintenance/confidence.ts | 10 +- 4 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/recall-maintenance-dry-run.test.ts diff --git a/src/__tests__/maintenance-two-roots.test.ts b/src/__tests__/maintenance-two-roots.test.ts index 3ee8286db..68af61f21 100644 --- a/src/__tests__/maintenance-two-roots.test.ts +++ b/src/__tests__/maintenance-two-roots.test.ts @@ -50,6 +50,22 @@ describe('maintenance across the write root and the inherited root', () => { expect(fs.readFileSync(path.join(inherited, 'old.md'), 'utf8')).toContain('confidence: 0.1'); }); + it('under dryRun names the files it would write and writes none (#900)', async () => { + // Its own content: gray-matter caches a parse by its input string. + fs.writeFileSync(path.join(inherited, 'old.md'), '---\ntitle: preview\nconfidence: 0.1\n---\nbody'); + + const written = await writeBackConfidence( + [writeRoot, inherited], + new Map([['old', 0.9]]), + writeRoot, + { dryRun: true }, + ); + + expect(written).toEqual([path.join(writeRoot, 'old.md')]); + expect(fs.existsSync(path.join(writeRoot, 'old.md'))).toBe(false); + expect(fs.readFileSync(path.join(inherited, 'old.md'), 'utf8')).toContain('confidence: 0.1'); + }); + it('updates a learning already in the write root in place', async () => { fs.writeFileSync(path.join(writeRoot, 'new.md'), '---\ntitle: new\nconfidence: 0.1\n---\nbody'); diff --git a/src/__tests__/recall-maintenance-dry-run.test.ts b/src/__tests__/recall-maintenance-dry-run.test.ts new file mode 100644 index 000000000..fcc08482f --- /dev/null +++ b/src/__tests__/recall-maintenance-dry-run.test.ts @@ -0,0 +1,101 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// `recall maintenance` and `recall promote` declare their own `--dry-run`, but +// the root program declares one too, and Commander gives it to the root +// wherever it appears. These actions read only their own options, so a preview +// pruned, drafted and promoted for real (#900). They are driven through the +// real command table here, because that is where the flag went missing. +vi.mock('../migrate.js', () => ({ + maybeMigrate: vi.fn(async () => undefined), + queueKeptInCheckout: vi.fn(async () => null), +})); +vi.mock('../config.js', async (importOriginal) => ({ + ...(await importOriginal()), + autoDetectInit: vi.fn(async () => ({ + localConfig: { repo: { localPath: '/team-repo', remote: 'owner/repo' }, username: 'dev' }, + teamConfig: { team: 'demo', repo: 'owner/repo' }, + })), +})); +const candidate = { filename: 'l.md', path: '/l.md', confidence: 0.05, lastActivity: '', reason: 'low', docId: 'l' }; +vi.mock('../maintenance/index.js', async (importOriginal) => ({ + ...(await importOriginal()), + resolveMaintenancePaths: vi.fn(async () => ({ + repoPath: '/team-repo', votesDir: '/votes', learningsWriteDir: '/learnings', learningsReadDirs: ['/learnings'], + })), + findPruneCandidates: vi.fn(async () => [candidate]), + executePrune: vi.fn(async (_dir: string, _c: unknown, o: { dryRun?: boolean }) => + (o.dryRun ? { archived: 0, removed: 0, changed: [] } : { archived: 0, removed: 1, changed: ['/l.md'] })), + computeAllConfidence: vi.fn(async () => new Map([['l', 0.9]])), + writeBackConfidence: vi.fn(async () => ['/learnings/l.md']), + findStaleEntries: vi.fn(async () => [{ path: '/team-repo/docs/a.md' }]), + reportStaleEntries: vi.fn(), + findRelatedAdoptedLearnings: vi.fn(async () => []), + generateUpdateDraft: vi.fn(async () => null), + findPromotionCandidates: vi.fn(async () => [{ ...candidate, suggestedCategory: 'docs' }]), + executePromotion: vi.fn(async (_c: unknown, _r: string, o: { dryRun?: boolean }) => + ({ targetPath: '/team-repo/docs/l.md', marked: o.dryRun ? null : '/learnings/l.md' })), +})); +vi.mock('../utils/learnings-publish.js', async (importOriginal) => ({ + ...(await importOriginal()), + publishLearningsMaintenance: vi.fn(async () => ({ status: 'published' })), +})); +vi.mock('../utils/logger.js', async (importOriginal) => ({ + ...(await importOriginal()), + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: vi.fn() }, +})); + +beforeEach(() => { + vi.stubEnv('TEAMAI_COMMAND_TABLE_ONLY', '1'); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); + vi.clearAllMocks(); +}); + +async function run(args: string[]): Promise { + const { program } = await import('../index.js'); + await program.parseAsync(['node', 'teamai', ...args]); +} + +async function mocks() { + return { + ...(await import('../maintenance/index.js')), + ...(await import('../utils/learnings-publish.js')), + ...(await import('../config.js')), + }; +} + +const PREVIEWS: Array<[string, string[]]> = [ + ['recall maintenance --prune --dry-run', ['recall', 'maintenance', '--prune', '--dry-run']], + ['--dry-run recall maintenance --prune', ['--dry-run', 'recall', 'maintenance', '--prune']], + ['recall maintenance --confidence-writeback --dry-run', ['recall', 'maintenance', '--confidence-writeback', '--dry-run']], + ['recall maintenance --update-quality --dry-run', ['recall', 'maintenance', '--update-quality', '--dry-run']], + ['recall promote --dry-run', ['recall', 'promote', 'l', '--dry-run']], +]; + +describe('recall maintenance and recall promote under --dry-run (#900)', () => { + it.each(PREVIEWS)('%s loads, changes and publishes nothing', async (_label, args) => { + await run(args); + const m = await mocks(); + expect(m.autoDetectInit).toHaveBeenCalledWith(undefined, { dryRun: true }); + for (const call of vi.mocked(m.executePrune).mock.calls) expect(call[2]).toMatchObject({ dryRun: true }); + for (const call of vi.mocked(m.executePromotion).mock.calls) expect(call[2]).toMatchObject({ dryRun: true }); + for (const call of vi.mocked(m.writeBackConfidence).mock.calls) expect(call[3]).toEqual({ dryRun: true }); + expect(m.generateUpdateDraft).not.toHaveBeenCalled(); + expect(m.publishLearningsMaintenance).not.toHaveBeenCalled(); + }); + + // The positive control: the same commands without the flag do reach the + // write, so the assertions above are not passing on a path that never gets there. + it.each([ + ['recall maintenance --prune', ['recall', 'maintenance', '--prune']], + ['recall maintenance --confidence-writeback', ['recall', 'maintenance', '--confidence-writeback']], + ['recall promote ', ['recall', 'promote', 'l']], + ])('%s still publishes without the flag', async (_label, args) => { + await run(args); + const m = await mocks(); + expect(m.publishLearningsMaintenance).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/index.ts b/src/index.ts index c3d07a564..fdb10f7a5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1253,7 +1253,11 @@ recallCmd .option('--confidence-writeback', 'Update frontmatter confidence scores') .option('--update-quality', 'Find stale docs/rules/skills and suggest updates') .option('--dry-run', 'Show what would be done without making changes') - .action(async (cmdOpts) => { + .action(async (localOpts) => { + // The root program takes `--dry-run` wherever it is written, so this + // command's own declaration never sets it: read it merged, as the other + // actions do (#900). + const cmdOpts = { ...(program.opts() as GlobalOptions), ...localOpts }; if (!cmdOpts.confidenceWriteback && !cmdOpts.prune && !cmdOpts.updateQuality) { const { log } = await import('./utils/logger.js'); log.info('Usage: teamai recall maintenance --prune | --confidence-writeback | --update-quality'); @@ -1261,7 +1265,7 @@ recallCmd } const { autoDetectInit } = await import('./config.js'); - const { localConfig } = await autoDetectInit(); + const { localConfig } = await autoDetectInit(undefined, { dryRun: cmdOpts.dryRun }); const paths = await maintenancePathsOrExit(localConfig); if (!paths) return; const { @@ -1271,8 +1275,8 @@ recallCmd if (cmdOpts.confidenceWriteback) { const { computeAllConfidence, writeBackConfidence } = await import('./maintenance/index.js'); const map = await computeAllConfidence(votesDir); - const written = await writeBackConfidence(learningsReadDirs, map, learningsWriteDir); - if (written.length > 0) { + const written = await writeBackConfidence(learningsReadDirs, map, learningsWriteDir, { dryRun: cmdOpts.dryRun }); + if (written.length > 0 && !cmdOpts.dryRun) { await publishMaintenance(localConfig, `[teamai] Update confidence for ${written.length} learning(s)`, written); } return; @@ -1340,9 +1344,11 @@ recallCmd .description('Promote a high-confidence learning to formal knowledge (docs/skills/rules)') .option('--category ', 'Target category: skills | rules | docs') .option('--dry-run', 'Show what would be done without making changes') - .action(async (learningId, cmdOpts) => { + .action(async (learningId, localOpts) => { + // As in `recall maintenance`: `--dry-run` reaches the root's options only (#900). + const cmdOpts = { ...(program.opts() as GlobalOptions), ...localOpts }; const { autoDetectInit } = await import('./config.js'); - const { localConfig } = await autoDetectInit(); + const { localConfig } = await autoDetectInit(undefined, { dryRun: cmdOpts.dryRun }); const { findPromotionCandidates, executePromotion, diff --git a/src/maintenance/confidence.ts b/src/maintenance/confidence.ts index 93e089213..2f79b9705 100644 --- a/src/maintenance/confidence.ts +++ b/src/maintenance/confidence.ts @@ -89,11 +89,13 @@ export async function computeAllConfidence(votesDir: string): Promise 0.05. * Returns the files it wrote, which are what a publish may stage (#823). + * Under `dryRun` it writes nothing and returns the files it would write (#900). */ export async function writeBackConfidence( learningsDirs: readonly string[], confidenceMap: Map, writeRoot?: string, + options: { dryRun?: boolean } = {}, ): Promise { const written: string[] = []; const files = await listLearningFiles(learningsDirs); @@ -121,8 +123,10 @@ export async function writeBackConfidence( const target = writeRoot && !isInWriteRoot(absPath, writeRoot) ? path.join(writeRoot, file) : absPath; - await ensureDir(path.dirname(target)); - await writeFile(target, newContent); + if (!options.dryRun) { + await ensureDir(path.dirname(target)); + await writeFile(target, newContent); + } written.push(target); } catch { log.debug(`confidence: failed to update frontmatter for: ${file}`); @@ -130,7 +134,7 @@ export async function writeBackConfidence( } if (written.length > 0) { - log.info(`Updated confidence scores for ${written.length} learning(s)`); + log.info(`${options.dryRun ? '[dry-run] Would update' : 'Updated'} confidence scores for ${written.length} learning(s)`); } return written; } From eb4c38a7600296af8299ad182155c6daea15b186 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Tue, 29 Sep 2026 12:16:58 +0200 Subject: [PATCH 2/3] fix(dry-run): create the promotion target directory only when promoting (#900) executePromotion created // before its dry-run return, so recall promote --dry-run left an empty directory in the team repo. --- src/__tests__/maintenance-promote.test.ts | 2 ++ src/maintenance/promote.ts | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/__tests__/maintenance-promote.test.ts b/src/__tests__/maintenance-promote.test.ts index 2328c4daa..06efb77be 100644 --- a/src/__tests__/maintenance-promote.test.ts +++ b/src/__tests__/maintenance-promote.test.ts @@ -137,6 +137,8 @@ describe('executePromotion', () => { expect(targetPath).toContain('skills/candidate.md'); expect(marked).toBeNull(); expect(fs.existsSync(path.join(tmpDir, 'skills', 'candidate.md'))).toBe(false); + // Not even the category directory: a preview creates nothing (#900). + expect(fs.existsSync(path.join(tmpDir, 'skills'))).toBe(false); }); it('copies file and marks original with promoted_to', async () => { diff --git a/src/maintenance/promote.ts b/src/maintenance/promote.ts index 11c21069e..1390e795d 100644 --- a/src/maintenance/promote.ts +++ b/src/maintenance/promote.ts @@ -159,8 +159,6 @@ export async function executePromotion( ): Promise<{ targetPath: string; marked: string | null }> { const category = options.category ?? candidate.suggestedCategory; const targetDir = path.join(repoPath, category); - await ensureDir(targetDir); - const targetPath = path.join(targetDir, candidate.filename); if (options.dryRun) { @@ -176,6 +174,7 @@ export async function executePromotion( // AI transforms the learning into a generalized format for the target category const promotedContent = await generatePromotedContent(originalContent, category, candidate.title); + await ensureDir(targetDir); await writeFile(targetPath, promotedContent); // Mark the learning as promoted. When it lives in a root nothing pushes, the From 67d66f0eb15015f5a701aaac98a41711a75705ef Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Tue, 29 Sep 2026 12:32:09 +0200 Subject: [PATCH 3/3] refactor(promote): drop the ensureDir that writeFile already does (#900) --- src/maintenance/promote.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/maintenance/promote.ts b/src/maintenance/promote.ts index 1390e795d..157536645 100644 --- a/src/maintenance/promote.ts +++ b/src/maintenance/promote.ts @@ -174,7 +174,6 @@ export async function executePromotion( // AI transforms the learning into a generalized format for the target category const promotedContent = await generatePromotedContent(originalContent, category, candidate.title); - await ensureDir(targetDir); await writeFile(targetPath, promotedContent); // Mark the learning as promoted. When it lives in a root nothing pushes, the