Skip to content

fix(dry-run): thread { dryRun } through the loaders contribute, session save and recall use (#850) - #853

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/dry-run-loader-sites
Sep 27, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/dry-run-loader-sites

Conversation

@ydflow

@ydflow ydflow commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Complements #850, which fixes pull / push / status. This covers the three commands that issue lists as "Not audited, same bare-call shape" — contribute, session save and recall — with the loader change #850 itself proposes. Happy to fold this into #850 instead if one PR is preferred.

Problem

#837 threaded LoadOptions through the config loaders, and its own out-of-scope note (quoted in #850) named the commands that still reach them bare. Beyond the three #850 fixes, three more commands load their config before their own dry-run guard and pass nothing:

Command Bare loader calls Own dry-run guard
contribute loadLocalConfigForScope('project'), requireInit() ×2, detectProjectConfig() the [dry-run] Would push check, after the load
session save loadLocalConfigForScope('project'), requireInit() ×2, detectProjectConfig() the local-log preview, then the push guard — both after the load
recall detectProjectConfig(…), loadLocalConfigForScope('user'), requireInit() the votes write at the end

On a config pending the legacy role migration, each of them rewrote ~/.teamai/config.yaml under --dry-run, printing Migrated legacy teamai config to default role profile: hai with no [dry-run] marker (real CLI, before, contribute --file note.md --scope user --dry-run):

ℹ Migrated legacy teamai config to default role profile: hai
ℹ [dry-run] Would push: learnings/session-notes-2026-09-27-pgbyvb.md (41 bytes)
config.yaml sha256: changed

recall test --dry-run did the same. The project-scope branches can also adopt a pre-#546 partition and run the single-repo self-heal bootstrap, exactly as #850 describes for pull — detectProjectConfig and selfHealAndReadPartition already honour options.dryRun; these commands simply never supplied it.

Fix

Tests

src/__tests__/dry-run-load-path.test.ts, reusing its fixtures, tree snapshot and provider-call recorder:

  • The two new dedicated tests (recall --dry-run on the legacy-role fixture; contribute --scope user --dry-run) and the loader-level pair fail 3/3 on main — each writes config.yaml — and pass on this branch.
  • A third loader test pins the compatibility direction: with no options, loadLocalConfigForScope still migrates in place.
  • The provider-call recorder asserts no provider call happens under --dry-run.

npx tsc --noEmit clean; npm run lint 0 warnings under --deny-warnings.

Real-CLI verification (3b3c97b)

npm run build, then node dist/index.js against a sandbox HOME holding a role-less ~/.teamai/config.yaml next to a team repo whose manifest/roles.yaml declares hai, the same single-variable method as #850 (sha256 over config.yaml, fixture restored between runs):

$ teamai contribute --file note.md --scope user --dry-run
ℹ [dry-run] Would migrate legacy teamai config to default role profile: hai
ℹ [dry-run] Would push: learnings/session-notes-2026-09-27-gnm594.md (41 bytes)
config.yaml sha256: unchanged

$ teamai recall test --dry-run
ℹ [dry-run] Would migrate legacy teamai config to default role profile: hai
ℹ No learnings available. Run `teamai pull` first to sync team knowledge.
config.yaml sha256: unchanged

$ teamai recall test        # no --dry-run: a real run still migrates in place
ℹ Migrated legacy teamai config to default role profile: hai
ℹ No learnings available. Run `teamai pull` first to sync team knowledge.
config.yaml now carries primaryRole: hai

…on save and recall use

Tencent#837 threaded LoadOptions through the config loaders, and Tencent#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-Tencent#546 partition and run the single-repo self-heal bootstrap.

loadLocalConfigForScope is the loader Tencent#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.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] src/save-session.ts:105 — In a fresh self-mode clone, both project loader paths now return the dry-run bootstrap preview, whose username is intentionally empty because authentication is skipped (src/bootstrap.ts:114). Consequently, teamai --dry-run session save --push --force reports sessions//<month>.md at src/save-session.ts:134, rather than a meaningful destination. Handle the unbootstrapped username explicitly, for example with a placeholder or conditional message.

The PR description includes sufficient real-CLI verification for this runtime change, so no testing-record finding is warranted.

@jeff-r2026
jeff-r2026 merged commit a8ab8e0 into Tencent:main Sep 27, 2026
11 checks passed
Smilewithoutfalling added a commit to Smilewithoutfalling/teamai-cli that referenced this pull request Sep 28, 2026
Fixes the red Lint & Test on the previous head (all four matrix entries).

pull-scope-isolation.test.ts asserted the exact argument list of
loadLocalConfigForScope, which this change widens to carry LoadOptions:

  expect(loadLocalConfigForScope).toHaveBeenCalledWith('user');
    received ['user', undefined, { dryRun: undefined }]

The shape is not a free choice: recall.ts:464 and recall.ts:515 already pass
the same third argument, landed with Tencent#853, so pull.ts matches the merged
precedent. recall-scope-isolation.test.ts never asserts the argument list,
which is why the same change left it green.

The affected set is derived from the changed symbols rather than from the
topic: every test file that mentions loadLocalConfigForScope,
detectProjectConfig or autoDetectInit (76 files, 1221 tests).
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