Skip to content

fix(dry-run): let --dry-run reach recall maintenance and recall promote (#900) - #903

Merged
jeff-r2026 merged 3 commits into
Tencent:mainfrom
SaulMoro:fix/900-recall-dry-run
Sep 29, 2026
Merged

jeff-r2026 merged 3 commits into
Tencent:mainfrom
SaulMoro:fix/900-recall-dry-run

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

teamai recall maintenance --prune --dry-run deleted learnings and published the deletion. recall promote <id> --dry-run promoted and published, and --update-quality --dry-run wrote AI drafts. The root program declares --dry-run too, and Commander 12 (without enablePositionalOptions()) gives the flag to the root wherever it is written. Most actions merge program.opts(), but these two read only their own options, where dryRun is always undefined.

 recall maintenance / recall promote   .action(localOpts)
+  cmdOpts = { ...program.opts(), ...localOpts }       as the mcp inject / models switch actions do
-  autoDetectInit()
+  autoDetectInit(undefined, { dryRun: cmdOpts.dryRun })
   executePrune / executePromotion / --update-quality   already honoured cmdOpts.dryRun; now it arrives
   --confidence-writeback
-    writeBackConfidence(...) + publish                ignored the flag
+    writeBackConfidence(..., { dryRun }) -> "[dry-run] Would update ...", no publish

 executePromotion(..., { dryRun })                     src/maintenance/promote.ts
-  ensureDir(<repo>/<category>)                        ran before the dry-run return
   if dryRun: log + return
   writeFile(target)                                   creates the directory itself, only on a real promotion

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 (341 files, 5366 tests, 1 skipped)
  • Added/updated tests for the change

Unit. src/__tests__/recall-maintenance-dry-run.test.ts drives the real command table (program.parseAsync), because the flag goes missing in Commander's wiring. The maintenance module and publishing are mocked at their module boundary.

recall maintenance --prune --dry-run                  load, prune, promote, write-back all see dryRun: true;
--dry-run recall maintenance --prune                  no AI draft; nothing published
recall maintenance --confidence-writeback --dry-run
recall maintenance --update-quality --dry-run
recall promote <id> --dry-run
positive control: the same without the flag           publishes (prune, write-back, promote)

maintenance-two-roots.test.ts gains the case where writeBackConfidence under dryRun names the file it would write and writes none. The existing executePromotion dry-run case in maintenance-promote.test.ts now also asserts that the category directory is not created.

  • Before (origin/main): all five dry-run rows, the write-back case and the promotion-directory check fail (7). The three controls pass.
  • After: all pass.

End-to-end, real CLI (npm run build, then node dist/index.js). The fixture is an HTTP-kind user scope, whose learnings and votes live under repo.localPath, so the run needs no network or branch worktree. It holds one learning nobody recalled (confidence 0.04, below the 0.15 threshold):

main   teamai recall maintenance --prune --dry-run                 learnings/stale.md: GONE   ✔ Removed 1 learning(s)
branch teamai recall maintenance --prune --dry-run                 learnings/stale.md: kept   [dry-run] Would remove 1 file(s)
branch teamai --dry-run recall maintenance --prune                 learnings/stale.md: kept   [dry-run] Would remove 1 file(s)
branch teamai recall maintenance --prune                           learnings/stale.md: GONE   ✔ Removed 1 learning(s)

main   teamai recall maintenance --confidence-writeback --dry-run  stale.md 931325d8 -> 19c923b6  Updated confidence scores for 1 learning(s)
branch teamai recall maintenance --confidence-writeback --dry-run  stale.md 931325d8 -> 931325d8  [dry-run] Would update confidence scores for 1 learning(s)
branch teamai recall maintenance --confidence-writeback            stale.md 931325d8 -> 19c923b6  Updated confidence scores for 1 learning(s)

main is origin/main 671f509; branch is 67d66f0. The provider does not matter here: the change is in option parsing.

A second probe diffs the whole fixture tree (v2 votes, a promotable and a stale learning). On this branch, every dry-run mode leaves the team repo unchanged, and HOME/.teamai too apart from debug.log, with the flag before or after the subcommand: --prune, --prune --archive, --confidence-writeback, --update-quality and promote <id>. On main, each of them prunes, archives, rewrites, drafts or promotes.

Related Issues

Fixes section B of #900. The issue stays open for sections C and D.

Notes for Reviewers

Deliberately not in this PR

These are known and tracked, so please do not report them as missing from this diff.

Item Why not here Tracked in
Other commands' config loaders that still persist a migration on --dry-run or on a read-only run (mcp inject, roles, tags, source, import, the pre-command auto-migration, ...) A separate, larger change with its own test tables #901
Dry-run writes past the loader in other commands (remove, source add, import --from-repo*, models switch, roles/projects edits), stats Each needs its own preview logic #900 (C)
Commands that ignore --dry-run entirely (projects set, hooks inject/remove, mcp remove, update, init, ...) A preview or a refusal has to be designed per command #900 (D)
resolveMaintenancePaths refreshes the reports and learnings checkouts on a dry run Deliberate: it runs with pushIfCreated: false and never publishes a branch, and a preview needs the current votes and learnings to name its candidates, the same way pull --dry-run fetches not a bug
loadUserVotes rewrites a v1 votes file into v2 when it reads one (src/votes.ts:199-202), and maintenance reads votes on a dry run too A write past the option parsing, in a reader shared by recall and hooks. Same on main. #900 (C)
recall promote --dry-run still asks the AI to classify the candidate, and the Claude CLI it spawns creates ~/.claude/ in a fresh HOME, while teamai appends to ~/.teamai/debug.log Not a teamai write: the preview needs the classification to name the target category. Same on main. not a bug
Enabling enablePositionalOptions() globally It would change how every subcommand parses the root's options. The per-action merge is the convention the other actions already follow. not planned

writeBackConfidence mutates the object gray-matter caches for a given input string. That is harmless in one CLI run, but it is why the new unit case uses its own frontmatter. It predates this PR and is left alone.

Review record

Two-axis /code-review (standards and spec) ran twice against origin/main. Pass 1 (on e1718ed): Standards found nothing; Spec found that recall promote --dry-run still created the empty target directory (promote.ts:162) in the real CLI, fixed in eb4c38a with its test, and noted the two pre-existing behaviours listed above. Pass 2 (on eb4c38a): Spec re-ran every dry-run mode (11 cases, flag before and after the subcommand) and every real mode against the built CLI, and found the team repo unchanged in each preview and the real modes identical to main; Standards noted that the moved ensureDir was redundant with writeFile, dropped in 67d66f0. The remaining pass-2 notes were corrections to this description, applied.

Merge danger

Door: two-way. Two actions merge their options and one function gains a defaulted option. A revert restores the old behaviour.

Blast radius: recall maintenance and recall promote only. Without --dry-run, their behaviour is unchanged (see the positive controls, unit and real CLI). With it, they now preview. That is what the help text (Show what would be done without making changes) and the docs (docs/usage-guide.md:1267, :1300, docs/product-overview.md:150 and the zh-CN pairs) already promised, so no doc changes.

…te (Tencent#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.
…ng (Tencent#900)

executePromotion created <repo>/<category>/ before its dry-run return, so
recall promote --dry-run left an empty directory in the team repo.
@github-actions

Copy link
Copy Markdown

No findings.

  • The previously reported dry-run creation of the promotion target directory is resolved at src/maintenance/promote.ts:164.
  • The PR description includes sufficient unit coverage and representative real-CLI/end-to-end verification for this runtime behavior change.
  • Per instructions, I reviewed the diff only and did not run tests or build commands.

@github-actions

Copy link
Copy Markdown

No findings.

  • The previously reported dry-run creation of the promotion target directory remains resolved at src/maintenance/promote.ts:159.
  • The PR description documents sufficient unit testing and representative real-CLI/end-to-end verification for this runtime behavior change.
  • Per instruction, I reviewed only the specified diff and did not run builds or tests.

@jeff-r2026
jeff-r2026 merged commit ed95357 into Tencent:main Sep 29, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants