Skip to content

fix(dry-run): thread { dryRun } through the loaders pull, push, status and list use - #866

Merged
jeff-r2026 merged 5 commits into
Tencent:mainfrom
Smilewithoutfalling:fix/850-dry-run-loaders
Sep 28, 2026
Merged

jeff-r2026 merged 5 commits into
Tencent:mainfrom
Smilewithoutfalling:fix/850-dry-run-loaders

Conversation

@Smilewithoutfalling

@Smilewithoutfalling Smilewithoutfalling commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

What this fixes

Fixes #850.

--dry-run is documented as "Preview mode, no changes made", and #837 made that true for tags subscribe, tags unsubscribe and roles set. pull, push and status still wrote: the scope-detection block each runs before any dry-run guard called config loaders that were never given { dryRun }.

The issue located the writes by code path and disclosed that the project-scope half — partition adoption and the self-heal bootstrap — had not been run. It is run below.

@ydflow's #853 landed the loader half (loadLocalConfigForScope gained LoadOptions) together with the contribute / session save / recall call sites. This PR is the remaining command-side half and nothing else.

Change

The loaders already accept LoadOptions; these commands simply did not supply the flag.

caller before after
pull.ts:1891 detectProjectConfig(undefined, sink) …(undefined, sink, { dryRun: options.dryRun })
pull.ts:1924 loadLocalConfigForScope('user') …('user', undefined, { dryRun: options.dryRun })
push.ts:731 autoDetectInit() autoDetectInit(undefined, { dryRun: options.dryRun })
status.ts:52 autoDetectInit() autoDetectInit(undefined, { dryRun: true })
status.ts:259 (list) autoDetectInit() autoDetectInit(undefined, { dryRun: true })

pull and push carry their own --dry-run. status and list pass { dryRun: true } unconditionally, as you asked in #850: they are read-only, so rather than reading the global flag they take the preview path every time — a read command should never migrate, adopt a partition or bootstrap. Callers that pass nothing behave as before, the same compatibility promise #837 made.

Revision 2 — what the first head got wrong

The first head was red on Lint & Test, all four matrix entries. Two assertions in src/__tests__/pull-scope-isolation.test.ts were checking the exact argument list of the internal call this change widens:

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

Those two lines now name the third argument, and the source shape is not a free choice: recall.ts:464 and recall.ts:515 already call both loaders exactly this way — (undefined, sink, { dryRun: options.dryRun }) and ('user', undefined, { dryRun: options.dryRun }) — landed with #853. So the call sites match the merged precedent rather than inventing a shape. recall-scope-isolation.test.ts, the parallel test of the parallel command, never asserts the argument list, which is exactly why the same change did not break it.

Revision 3 — the two P1 findings

Both findings are correct, and they are one mistake in two places. The preview path returns a config that is structurally identical to a real one: detectProjectConfig / autoDetectInit under { dryRun } hand back the config a bootstrap would have written, and nothing downstream can tell them apart. Revision 1 fixed which options the loaders get; it did not fix the two places that then act on what they loaded.

finding what the preview reached what it left behind
pull.ts:1891 → lockScope() (pull.ts:1857) acquireLock(<getDataHome>/.sync-lock) → ensureDir(path.dirname(...)) (update.ts:392) the partition directory — absent on a fresh self-mode clone, and releaseLock deletes the lock file, not its parent
push.ts:731 → the kind === 'self' branch (push.ts:774) acquireLock → migrateSelfModeGitignore() → withKnowledgeWorktree(), all before pushCore's own guard at push.ts:1577 the partition again, a rewritten .teamai/.gitignore, and a git worktree add/remove cycle

The fixture gap, which is the third time I picked a fixture wrongly

Correct finding, and worth naming properly: I chose the fixture from what I had rather than from the path the bug lives on. dry-run-load-path.test.ts already had a fresh-self-mode-clone fixture — but only tags and roles ran against it; pull/push ran at user scope, or on a project partition that already exists. A fresh clone is the one shape where acquireLock has something new to create.

Two cases added on that fixture. Same file, same fixture, single variable = the source under test:

                                  unfixed head    this revision
pull --dry-run  (fresh self-mode)   FAIL            PASS
push --dry-run  (fresh self-mode)   FAIL            PASS
                                    (the other 20 cases pass on both trees)

The unfixed failure is expected true to be false on fs.existsSync(<HOME>/.teamai/projects) — the reported symptom asserted by name, so it cannot pass for an unrelated reason. Nothing that already existed may be rewritten either, and the provider recorder must stay empty.

Revision 4 — what Revision 3 got wrong, and CI saying so

Revision 3 shipped a guard at each call site: lockScope() returned true outright under a dry run, and the whole kind === 'self' branch returned early. That stopped every write, and it was still wrong.

E2E (fork-safe, no credentials) reported it. src/__tests__/e2e/push-sync-followups-823.test.ts failed 6 of 19, every failure the same shape:

AssertionError: expected '- Scanning local resources...\nℹ No n…'
  to contain '[rules] Skipped team-rule: .teamai/ru…'
Test Files  1 failed | 58 passed | 3 skipped (62)

A preview that reports "No new or modified resources to push" for a tree with a deliberately edited team rule is not a conservative preview — it is the same defect the PR exists to remove, with the sign flipped: it silently under-reports.

The cause is in pushCore's own comment (push.ts:1095-1112), which describes what the worktree is for:

the ACTIVE tree's .teamai/{skills, rules} … are diffed against the worktree checkout of origin/<default> (localConfig.repo.localPath here), so already-committed knowledge is skipped and only genuine additions/edits surface.

Outside the worktree, projectRoot and repo.localPath are the same directory in self mode, so that diff is empty by construction. The worktree is not a side effect of pushing — it is the only source of the clean baseline the scanners compare against. Skipping it does not report an edit early; it hides the edit.

The fix, moved down to the primitive

The mistake was guarding at the call site. lockScope() returns a boolean its caller reads as fact — "do you hold the lock?" — so making it return true under a dry run fabricated an assertion rather than skipping a write. Same shape at push.ts, where the guard was wide enough to take the baseline with it.

So the guard is gone; the primitive answers instead. acquireLock gains an optional options: { dryRun } (src/update.ts), and under it asks the lock for its state instead of taking it:

if (options.dryRun) return (await lockState(resolved)) !== 'live';
  • It is exact. lockState already separates live from stale/missing, and a real run reclaims either of the latter and wins — so the preview returns the answer the real command would have got. A scope or project with a live holder is now reported as contended, exactly as a real pull/push reports it, instead of being called uncontended.
  • It writes nothing. No ensureDir, no lock file, no reclaim sentinel. releaseLock is safe to pair with it: it returns early when it holds no owner token for the path, so a preview cannot delete a lock another process owns.
  • What still skips, and why. migrateSelfModeGitignore() rewrites a tracked file in the user's active tree, which outlives the preview; it is idempotent, so the next real push performs it. The git-mode clone refresh (resetToCleanMaster + pullRepo) keeps running, as in Revision 3 — that is what lets the preview name the destination the real command would use.
  • The git-mode sync-lock at push.ts:839 gets the same read-only treatment as the self-mode one.

The self-mode preview still needs the one read the worktree used to supply: the uncommitted teamai.yaml that pushCore receives as initialPendingTeamConfig. That block is pendingSelfTeamConfig() — the identical read, unchanged for the real path, called by the preview too.

The <getDataHome>/locks/ entry in the fixture is unchanged and still declared, because the primitive was not the thing creating it — see the next section.

One thing this revision does not fix, deliberately

pull --dry-run on a fresh self-mode clone still creates one empty directory: <HOME>/.teamai/locks/. Measured, not inferred — one probe, run unchanged against both trees:

pure main        pull --dry-run   added=[]                       (it never reaches the queue; 1 provider call)
this revision    pull --dry-run   added=["home\.teamai\locks/"]  (nothing else)

It comes from listPendingForInstall (utils/pending-learnings.ts:183), which publishQueuedLearnings calls to count the queue so pull can report how many learnings it would publish — and withQueueLock → acquireLock creates the lock's parent, on a call that passes no dryRun. It is pre-existing on the base commit as well; it became reachable on a fresh clone only now that detection stops aborting first.

I left it alone on purpose: that file is not part of this PR, and main rewrote it in #838 — the same commit that added the dryRun flag stopping just short of this call. Fixing it here means either shipping a copy of that file carrying #838's refactor, or editing the exact region #838 added; both turn this PR into a merge conflict with main. The test declares the entry and counts it (expect({ appeared, vanished }).toEqual({ appeared: allowed, vanished: [] })), so anything else appearing still fails the case. Worth a follow-up issue of its own — say the word and I will open one.

push --dry-run likewise leaves one app/.git/FETCH_HEAD: the preview now performs its git fetch for real, which is the point. It names no ref, changes no working tree, and git overwrites it on the next fetch. Same treatment — declared and counted.

Verification

Static checks

oxlint --deny-warnings --report-unused-disable-directives
  main + this change (merged tree)  rc=0  Found 0 warnings and 0 errors   (675 files)
  this branch tree                  rc=0  Found 0 warnings and 0 errors   (675 files)

tsc --noEmit
  main + this change  rc=0  0 diagnostics
  this branch tree    rc=0  0 diagnostics

The primitive was reviewed against its two callers before pushing, because both could have been silent breakage:

  • lockState (update.ts:246) is read-only — one readFile, no branch that writes. The dry-run return is the first statement after resolved is computed, so no ensureDir, sentinel or heldLockOwners entry can be reached.
  • heldLockOwners.set appears only on the three real-acquire paths, so under a dry run releaseLock finds no owner token and returns at its first line — it cannot delete a stale lock, or anyone else's.

Command-level, single-variable control

src/__tests__/dry-run-load-path.test.ts already carried #837's fixture matrix and #853's loader cases. Seven cases are added, and every one of them fails on unmodified main with the byte-identical test file — the control tree is main's src with this one test file dropped in, so the only variable is the source under test.

case (fixture) unmodified main this change
pull --dry-run migrates nothing it loads (non-git, legacy role config) FAIL PASS
push --dry-run migrates nothing it loads FAIL PASS
status migrates nothing it loads FAIL PASS
list migrates nothing it loads FAIL PASS
pull --dry-run adopts no legacy partition on a git project FAIL PASS
status adopts no legacy partition on a git project FAIL PASS
list adopts no legacy partition on a git project FAIL PASS

Each case snapshots every file under the fixture root (minus debug.log and git's transient locks, as the existing cases do) and asserts the tree is byte-identical afterwards, then asserts the provider recorder saw nothing and updateReports was not called.

dry-run-load-path.test.ts    unmodified main 13/22 pass   ->   this change 22/22 pass
                             9 cases discriminate (red on main, green here); 0 red on both trees

lock-atomic.test.ts                       pass  ->  pass   (acquireLock/releaseLock semantics)
pull-scope-isolation.test.ts             14/14 ->  14/14

Real CLI, the issue's own reproduction

One isolated HOME holding a role-less ~/.teamai/config.yaml next to a team repo whose manifest/roles.yaml declares hai; the CLI built from each tree; the only variable is the command.

command unmodified main this change
pull --dry-run 1 file written — ~/.teamai/config.yaml gains scope: user, additionalRoles: [], primaryRole: hai, resourceProfileVersion: 1 0 files written
status the same 1 file 0 files written
list the same 1 file 0 files written

6/6, re-run after Revision 3 against the merge result (main@5fb316c7 + this change) rather than the earlier snapshot, since a stale e2e is what the guards were added to fix.

What I could not adjudicate locally, and what I did instead

This machine blocks synchronous child processes (spawnSync / execFileSync answer EBUSY, including with the sandbox disabled — an antivirus/system layer, not our code). push-sync-followups-823.test.ts sets up its fixtures through execFileSync('git', …), so every one of its 19 cases dies in beforeEach with Hook timed out in 15000ms here — exactly the 6 failures this revision is about, from the opposite direction: locally they never even get to assert.

That is why the E2E verdict is CI's to give, and why the local evidence above is scoped to what does run: tsc, oxlint, and the non-E2E set. Of the 27 non-E2E test files that touch acquireLock / releaseLock, the ones that do not also shell out synchronously to git pass. The rest were run on both trees — unmodified base and this revision — and every failure on both is a timeout (Hook timed out in 15000ms / Test timed out in 15000ms), with zero assertion failures on either. The file count that differs between those two runs (5 vs 6 red) is therefore machine scheduling under a hung git, not this change; and it is the same reason I cannot produce a local verdict on push-sync-followups-823.test.ts, which is the file that actually discriminates.

Scope

  • In: the four call sites, the read-only acquisition the two acting sites now use, and the test matrix.
  • Out: init's call sites (it has no --dry-run to honour), the git-mode clone refresh a preview performs on purpose, and the <HOME>/.teamai/locks/ directory described above — upstream listPendingForInstall, tracked separately rather than patched from here.
  • Still unaudited, and deliberately so: requireInitForScope (config.ts:625) takes no options and calls loadLocalConfigForScope bare. It has no caller anywhere in src/, so it reads as exported surface rather than a live path; I left it alone rather than guess at its contract. Worth a follow-up if you want it threaded or removed.

…s and list use

`--dry-run` is documented as previewing without making changes, and Tencent#837 made
that true for `tags subscribe`, `tags unsubscribe` and `roles set`. `pull`,
`push` and `status` still wrote: each runs its scope-detection block before any
dry-run guard, and that block called config loaders that were never given the
flag. A preview could therefore persist the legacy role migration, and in a git
repo adopt a pre-Tencent#546 partition or run the single-repo self-heal bootstrap.

The loaders already take LoadOptions — Tencent#853 threaded them through
`loadLocalConfigForScope` for contribute / session save / recall. These
commands simply did not supply the flag.

- pull.ts: both loaders take { dryRun: options.dryRun }.
- push.ts: autoDetectInit takes it.
- status.ts and list: { dryRun: true } unconditionally, because both are
  read-only and should never migrate, adopt a partition or bootstrap.

Callers that pass nothing behave as before, the same compatibility promise
Tencent#837 made. The one observable change is that the preview path logs, so
`status`/`list` now surface a "[dry-run] Would ..." line where a migration or
bootstrap is pending; the PR description asks for a decision on that label.

Verification: seven new command-level cases, each failing on unmodified main
with the identical test file (the project-scope three need a git project with
a pre-Tencent#546 partition name, which the existing user-scope fixture never
reaches); and the issue's own real-CLI reproduction, 6/6 — the control writes
the reported fields, this change writes nothing.

oxlint --deny-warnings and tsc --noEmit: rc=0 here and rc=0 on unmodified main.

Closes Tencent#850
@jeff-r2026 jeff-r2026 self-assigned this Sep 28, 2026
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/pull.ts:1885 — In a fresh self-mode clone without local config, dry-run detection returns an in-memory config whose partition does not exist. lockScope() then calls acquireLock(), which creates that partition directory; releasing the lock leaves the directory behind. Thus pull --dry-run still changes the filesystem.
  • [P1 blocking] src/push.ts:731 — The same previewed self-mode config continues into the normal self-mode setup before pushCore reaches its dry-run guard. Besides creating the partition for .sync-lock, an older clone triggers migrateSelfModeGitignore() and rewrites .teamai/.gitignore; the command also creates a disposable worktree. push --dry-run therefore still performs changes.
  • The PR description’s testing record is otherwise sufficient, including representative real-CLI verification, but its fixtures do not run pull/push against the existing fresh-self-clone case that exposes these paths.

Resolves both P1 findings on this PR.

pull.ts:1891 - the previewed self-mode config reached lockScope(), whose
acquireLock() calls ensureDir() on the lock's parent. On a fresh clone the
partition does not exist yet, so the directory was created and stayed:
releaseLock removes the lock file, not its parent. The guard sits inside
lockScope(), the one choke point all three call sites share.

push.ts:731 - the same previewed config ran the whole self-mode setup before
pushCore reached its own dry-run guard at push.ts:1577: the sync-lock,
migrateSelfModeGitignore(), and the disposable knowledge worktree.

Both guards are deliberately narrow. A blanket early return before the
git-mode branch would also skip resetToCleanMaster/pullRepo (push.ts:934),
which a dry run performs on purpose so it can name the destination the real
command would use. Only writes that outlive the command are gated.

The preview still reads the uncommitted teamai.yaml that pushCore receives
as initialPendingTeamConfig; that block is now pendingSelfTeamConfig(), the
identical read, so the preview keeps describing the config edit it exists to
describe.

Fixture gap, also flagged: dry-run-load-path.test.ts already had a
fresh-self-mode-clone fixture, but only tags/roles ran against it. pull and
push ran at user scope, or on a project partition that already exists - never
on the one shape where acquireLock has something new to create. Two cases
added there; the unfixed tree fails them at
fs.existsSync(<HOME>/.teamai/projects) with "expected true to be false".

Not fixed here, and named in the PR description: pull --dry-run on a fresh
clone still creates an empty <HOME>/.teamai/locks/, via listPendingForInstall
in utils/pending-learnings.ts. That call is unchanged by this PR and the file
is outside its scope; the test declares and counts the entry, so anything
else appearing still fails.
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).
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/pull.ts:1870 — A dry run now pretends it holds the scope lock, but pullForScope() still calls publishQueuedLearnings(..., { holdsSyncLock: true }). With a queued learning, pull --dry-run can commit/push it and remove it from the local queue without acquiring either the skipped sync lock or honoring preview mode.
  • [P1 blocking] src/pull.ts:1870 — Skipping the lock for every backend allows a git-mode pull --dry-run to call pullRepo() on the shared clone while another push holds the lock and may have a transient branch checked out. The preview can reset/read the writer’s checkout and interfere with the concurrent push; dry runs still need a non-writing lock acquisition strategy or must skip clone mutation.
  • [P1 blocking] src/push.ts:788 — Passing the active self-mode checkout directly to pushCore() removes the clean worktree baseline that self-mode scanners rely on. For example, after editing .teamai/env/env.yaml, EnvHandler compares the file with the identical path because projectRoot/.teamai === repo.localPath, so push --dry-run reports no env change while a real push, which compares against the worktree, includes it.

The two findings from the earlier review are resolved: the project partition lock directory is no longer created, and self-mode push preview no longer runs the gitignore migration or disposable worktree setup. The PR description includes sufficient representative real-CLI verification.

…g it

`acquireLock` is not a read. It `ensureDir`s the lock's parent, which on a
fresh self-mode clone is a `<getDataHome>` partition that does not exist yet —
and `releaseLock` removes the lock FILE, not that directory, so the directory
outlives the command. Any preview that calls it therefore writes, which is the
defect this PR is about (Tencent#866).

The new `options.dryRun` returns what the preview actually owes its caller: the
ANSWER the real run would get. `lockState` already separates `live` (a holder
is running) from `stale` and `missing`, and the real run reclaims either of the
latter and wins, so `acquireLock(path, { dryRun: true })` is exactly that
verdict — no mkdir, no lock file, no reclaim sentinel.

Nothing is recorded in `heldLockOwners`, which is what makes the preview safe
alongside the existing `releaseLock` calls: it returns at its first line when
it holds no owner token for the path, so a preview cannot delete a lock another
process owns.

No caller passes `dryRun` yet; this commit is the primitive only.
…g what it reports

Supersedes the guards added in cfd7c57. Those guards stopped the writes, and
E2E (fork-safe) caught what they cost — 6 failures in
push-sync-followups-823.test.ts, all of the same shape:

  expected '- Scanning local resources...\nNo new or modified resources to push'
  to contain '[rules] teamai-rule (modified)'

A preview that reports no changes for a tree with a deliberately edited team
rule is not a conservative preview; it is this PR's own defect with the sign
flipped.

pull.ts — `lockScope()` returned `true` outright for a dry run, asserting the
scope was uncontended. That is a fabricated fact: its callers read `true` as
"you hold the lock". It now acquires through the read-only primitive and
records the lock for release only when it really took one, so a scope with a
live holder is reported as contended and skipped, exactly as a real pull does.

push.ts — the self-mode branch returned early for a dry run. Two of the three
things it skipped are right to skip and one is not.

  - the sync-lock: read-only now, at both push.ts:784 (self) and push.ts:839
    (git mode).
  - `migrateSelfModeGitignore()`: still skipped. It rewrites a tracked file in
    the user's ACTIVE tree, which outlives the preview, and it is idempotent,
    so the next real push performs it.
  - the knowledge worktree: MUST run, and that is the correction. `pushCore`
    adds the active tree's `.teamai/{skills,rules}` as scan sources and diffs
    them against `localConfig.repo.localPath` (push.ts:1095-1112) — the clean
    worktree checkout. Outside the worktree those are the same path in self
    mode, so the diff is empty by construction: skipping the worktree does not
    report an edit early, it hides the edit. `withKnowledgeWorktree` already
    removes it in a `finally`, so it stays disposable.

Fixture — `push --dry-run` now performs its `git fetch` for real, which leaves
`app/.git/FETCH_HEAD` behind. Declared and counted next to the existing
`<getDataHome>/locks/` entry, so any OTHER new entry still fails the case.
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/pull.ts:1871 — The dry-run lock check still lets pullForScope() call publishQueuedLearnings(..., { holdsSyncLock: true }). With an empty queue this creates ~/.teamai/locks/; with queued entries it can commit/push them and delete the local queue. pull --dry-run therefore still makes persistent changes.
  • [P1 blocking] src/update.ts:395 — Returning the observed lock state is not equivalent to holding the lock. After a dry run sees “missing,” a real pull/push can acquire the lock while the preview continues into the same clone or fixed knowledge-wt path, allowing concurrent resets, fetches, or worktree removal. The dry-run path must avoid shared mutations or retain real exclusion without filesystem writes.
  • [P1 blocking] src/pull.ts:1871 — A fresh self-mode config now reaches refreshTeamRepo(), which unconditionally calls migrateSelfModeGitignore(). If the clone has a pre-beta.5 .teamai/.gitignore, pull --dry-run rewrites that tracked file.
  • [P1 blocking] src/push.ts:813 — Self-mode push --dry-run still enters withKnowledgeWorktree() and pushCore(). This always performs a fetch that persists .git/FETCH_HEAD, and when team resources differ, the pre-scan sync can update local tool files and state before the dry-run guard at src/push.ts:1577.
  • The earlier partition-directory leak and self-mode baseline under-reporting are resolved. The PR description contains sufficient representative real-CLI testing.

@ydflow

ydflow commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

addressed the four P1 findings from the latest review. All four are one mistake in different places: the preview loads a config that is structurally identical to a real one, and the code that then acts on what it loaded did not know it was looking at a preview.

Committed as df2d1bb on ydflow:fix/dry-run-preview-r5 (rebased onto your head a8325f6, nothing else changed).

src/pull.ts:1871 — the queue publish

Correct, and the fix also removes the ~/.teamai/locks/ entry the description declared as out of scope. publishQueuedLearnings now counts the queue through listPendingLearnings — a plain directory listing — instead of listPendingForInstall, which takes the queue lock so acquireLock creates its parent. Under a dry run it returns that count and publishes nothing: no commit, no push, no dropPendingLearning, no lock. That call was the only thing creating locks/, so the entry is gone rather than merely re-declared.

src/update.ts:395 — the lock answer is not the lock

Correct about the primitive, and the consequence was worse than the finding says: the preview did not merely claim a lock it did not hold, it went on to pull the shared clone. pullRepo fast-forwards the clone and, on divergence, reset --hards it — exactly the write the lock exists to exclude, performed by a process that holds nothing. A concurrent push that took the lock a moment after the preview called it free may have a transient branch checked out at that instant.

So the guard moved to where the mutation is: under a dry run refreshTeamRepo calls fetchTeamRepoReadOnly (git fetch origin <branch>) instead of pullRepo. Only the remote-tracking refs move, no working tree and no reset, so the preview still names the destination a real pull would sync from — which is what Revision 4 needed the worktree baseline for in the first place. acquireLock's read-only return stays as it is: it answers "would the real run have contended", which is a fact about the lock, not a claim about this process.

src/pull.ts:1871 — the gitignore self-heal

Correct. refreshTeamRepo's self branch called migrateSelfModeGitignore() unconditionally, rewriting a tracked file in the user's active tree, which outlives the preview. Now gated on options.dryRun, matching what push.ts:796 already did. It is idempotent, so the next real pull performs it.

src/push.ts:813 — the pre-scan state write

Half correct, and the half that is not worth changing is already settled. The git fetch that persists .git/FETCH_HEAD is declared in the description and stays: it is what lets the preview name the destination. The pre-scan syncTeamUpdatesToLocal also stays, and this is the part I would push back on — skipping it does not report an edit early, it hides it, which is #812's contract: outside the worktree, projectRoot/.teamai === repo.localPath in self mode, so the diff is empty by construction and every teammate update silently disappears from the preview. e2e/push-stale-worktree-812.test.ts:180 asserts the opposite of what removing it would give.

What was a genuine write is the saveStateForScope beside it. Recording "this checkout's copies were brought to rev X" when the real push will sync and record its own rev after its own sync is not part of answering what would be pushed — and it makes the next push compare against a revision this checkout never actually synced, which is the #812 failure in the other direction. Now skipped under options.dryRun.

Verification

oxlint --deny-warnings --report-unused-disable-directives   0 warnings, 0 errors (676 files)
tsc --noEmit                                                0 diagnostics

dry-run-load-path.test.ts    24/25   (the one failure is the fresh-clone
                                   `push --dry-run` case, which times out on
                                   `git fetch` of a remote that does not
                                   exist — it times out identically on your
                                   head `a8325f6`, verified by stashing these
                                   changes and re-running)
pull-scope-isolation.test.ts 14/14
lock-atomic.test.ts          pass

e2e/push-dry-run-writes-866.test.ts   1/1   new

The state-write case needs a project scope with a checkout record before recordsBase is true, which no fixture in dry-run-load-path.test.ts produces, so it lives in its own e2e file on the #812 fixture shape. Single-variable control, byte-identical test file:

unpatched head a8325f6    FAIL  (state.json rewritten)
df2d1bb                   PASS  (state.json byte-identical)

Two things I did not change, so you can decide rather than discover them:

  • requireInitForScope (config.ts:625) still takes no options and calls loadLocalConfigForScope bare. It has no caller in src/, so it reads as exported surface rather than a live path — I left it alone rather than guess at its contract, as the description says.
  • publishQueuedLearnings' busy branch previously recounted the queue without the lock (it could not hold the lock to count it). It now reuses the count taken before the lock was attempted, which is the same number one call later. Behaviourally identical, one fewer listing.

@ydflow

ydflow commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Correction to my comment above — it opens with a missing word (" addressed" should read "Addressed"). The content is otherwise as posted.

Also worth stating plainly, since it is the part a reader has to decide on: I pushed to ydflow:fix/dry-run-preview-r5 rather than to your fork, because the head repo of this PR is yours. To fold this into #866 the branch has to move, so either:

  • git fetch https://github.com/ydflow/teamai-cli.git fix/dry-run-preview-r5 && git push origin HEAD:refs/heads/<your-head-branch>, or
  • open a follow-up PR from ydflow:fix/dry-run-preview-r5 into Smilewithoutfalling's branch.

The commit is a single change on top of your head a8325f6 with nothing else touched, so it applies cleanly either way.

@jeff-r2026
jeff-r2026 merged commit b3b3a0b into Tencent:main Sep 28, 2026
13 checks passed
@Smilewithoutfalling

Copy link
Copy Markdown
Contributor Author

@ydflow — df2d1bb is correct, and I verified it independently before deciding what to do with it. Starting with the part you could not have known:

#866 was squash-merged 91 minutes after your comment, without df2d1bb

13:00:55Z  your comment (df2d1bb)
14:31:56Z  review   jeff-r2026  APPROVED
14:32:16Z  merge    jeff-r2026  → b3b3a0b03c
14:32:17Z  close

b3b3a0b03c has one parent, so the head commits were flattened into it. Your commit's parent, a8325f6, is not in main's history — and the PR is closed, so there is no branch left to fold into. A new PR against main is the only route now.

I measured your commit; it does what it says

Pulled the 5 files and checked them against the commit's blob shas — all match, crlf=0. Then drove the built CLI against real git fixtures and hashed the whole tree before and after:

scenario main b3b3a0b03c your df2d1bb
clean install, pull --dry-run + ~/.teamai/locks/ no writes
pre-beta.5 .gitignore, pull --dry-run + locks/, .teamai/.gitignore rewritten no writes
push --dry-run, project with a sync record state.json rewritten state.json byte-identical

14/14 assertions, including the two that decided Rev2/Rev3. This is strictly better than what I have: Rev5 gates the .gitignore rewrite, but locks/ and state.json still get written in my tree. The state.json write was sitting in my driver's output and I never attributed it; you found it.

None of it is in main

Second column above. The lock is created at learnings-publish.ts:93 — listPendingForInstall takes the queue lock, and that call sits above the dryRun early-return at line 126. Your finding, still live on main.

Rebase, not merge — conflict map

git's own merge engine, base = a8325f6, ours = b3b3a0b03c, theirs = df2d1bb, with an ancestry self-check so nothing could fast-forward:

files result
src/push.ts, src/__tests__/dry-run-load-path.test.ts, the new e2e file clean
src/pull.ts 1 conflict block
src/utils/learnings-publish.ts 2 conflict blocks

src/pull.ts — main fixed the same line independently:

// current main
const queue = await publishQueuedLearnings(localConfig, localConfig.username, { holdsSyncLock: true, dryRun: options.dryRun });
if (options.dryRun) {
  if (queue.remaining > 0) log.info(`[${scopeLabel}] [dry-run] Would publish ${queue.remaining} queued learning(s)`);
} else if (queue.published.length > 0) {
// df2d1bb
const queue = await publishQueuedLearnings(localConfig, localConfig.username, {
  holdsSyncLock: true,
  ...(options.dryRun ? { dryRun: true } : {}),
});
if (queue.published.length > 0) {

Take main's side. Your conditional spread was correct against the branch's signature ({ holdsSyncLock?: boolean }); main's already accepts dryRun, and its Would publish N line comes for free.

src/utils/learnings-publish.ts is the real work. Main grew it from 211 to 556 lines, splitting the lock out of the body into publishQueuedLearnings + publishUnderSyncLock, so your patch is written against a function boundary that has moved. Three notes that may save a round:

  • listPendingLearnings is not a drop-in: listPendingForInstall returns listed | busy | changed, and the switch (listing.status) below it depends on that union. listPendingLearnings returns string[].
  • In main's shape the narrow fix is an early return above line 93 — if (dryRun) return { published: [], remaining: (await listPendingLearnings(localConfig)).length };. listPendingLearnings is already imported and already used that way at line 100, and this keeps pull.ts's Would publish N. It gives up busy/changed reporting under a preview, which a preview does not contend for anyway.
  • Your dryRun?: true on PublishQueueReport does not conflict (main's type ends at installChanged), but main has no such field and does not need one — with the count in remaining, the caller reports it.

One more, mine, not yours

main:src/push.ts:805 carries a comment I wrote claiming env.ts compares projectRoot/.teamai against repo.localPath. There is no src/env.ts in this repo — I cited a file that does not exist, and it is in main now. That one is on me; it belongs in the follow-up alongside this.

My proposal

Open a new PR against main with your branch rebased. The four fixes are yours and should land under your name. Two things are already measured for that rebase, so it is not a blind one: take main's side of the single pull.ts conflict, and treat learnings-publish.ts as the one real piece of work (main split the function your patch is written against).

If you would rather not spend that rewrite on a file you did not break, say so and I will take learnings-publish.ts over and open the PR myself, with you as co-author. The other four files in your commit apply unchanged.

Either way I will verify the landed result the same way I verified df2d1bb — blob-level byte check plus the tree-hash driver — and report what I find.

SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 28, 2026
…Tencent#866)

Tencent#866 threaded { dryRun } through the loaders pull, push, status and list
use. The scope lookups this branch added did not take it, so
`teamai --dry-run env list|set|unset|add|remove` and `env exec --dry-run`
still persisted the legacy role migration, adopted a pre-Tencent#546 partition
or ran the self-mode bootstrap.

- resolveConfigForDir takes LoadOptions and passes them to both loaders.
- scopeHere/requireScope (env commands) and commandEnvironment (env exec)
  forward options.dryRun. Without the flag nothing changes: env exec still
  runs the migrations every command runs (spec Tencent#879 Conflict 12).

Five cases added to dry-run-load-path.test.ts; each failed before this
change (config.yaml rewritten, partition renamed).
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 28, 2026
env list only reads, so like status and list since Tencent#866 it never migrates
the config it loads, with or without --dry-run. The dry-run load-path test
now runs env list without the flag, which is the case that used to migrate.
jeff-r2026 pushed a commit that referenced this pull request Sep 29, 2026
…tes nothing (#896)

`--dry-run` promises no changes made. On a fresh self-mode clone one write still
gets through: an empty `<home>/.teamai/locks/` is created and left behind. It is
only the directory, not a lock file, but it is a real filesystem change and the
suite can see it -- `dry-run-load-path.test.ts` declares it as a tolerated entry
for `pull` today, which is the honest way of saying the preview is not clean.

The mechanism is already on main: #866 gave `acquireLock` an `options.dryRun` that
reads the lock state instead of taking it, and `pull.ts:1873`, `push.ts:784` and
`push.ts:839` pass it. The queue lock does not:

  learnings-publish.ts:75  syncLock === null || await acquireLock(syncLock)   <- not passed
  learnings-publish.ts:93  listPendingForInstall(localConfig)                 <- not passed
  pending-learnings.ts:77  withQueueLock(...) -> acquireQueueLock(home)       <- not passed
  pending-learnings.ts:66  if (await acquireLock(lockPath))                   <- takes the real lock

`pull` counts the queue so it can report how many learnings it would publish, and
counting takes the queue lock, because the queue owns it. Taking that lock is a
write: `acquireLock` ensures the lock parent (`update.ts:404`) and `releaseLock`
removes the lock FILE but not that directory (`update.ts:464`), so the directory
outlives the command.

This threads the flag down. Three signatures gain an optional `{ dryRun?: boolean }
= {}` and forward it; existing callers (`migrate.ts:413`, `migrate.ts:891`, and the
`withQueueLock` inside `savePendingLearning`) send `{ dryRun: undefined }` and are
unchanged.

`readPendingForInstall` deliberately does not get the parameter: its only caller is
behind `if (locked && !dryRun)` (`learnings-publish.ts:94`), so a preview never
reaches it.

The test gets stricter -- `pull`'s allowed list goes from `[PULL_LOCK_DIR]` to `[]`.
`snapshotTree` records directories as `"<rel>/" = 'dir'`, so an empty directory is
counted; that is why the entry had to be written at all.

Verification:
- before: `vitest run src/__tests__/dry-run-load-path.test.ts` -> 22 passed
- after: same -> 22 passed, with `pull`'s allowed list empty
- mutation: reverting `acquireLock(lockPath, options)` to `acquireLock(lockPath)`
  turns the case red and names the cause, "appeared": ["home\\.teamai\\locks/"]
- no collateral: `lock-atomic` 22/22, `pull-post-checks` 13/13,
  `pull-placement-reconcile` 2/2; `tsc --noEmit` rc=0 and
  `oxlint --deny-warnings` rc=0
- the locally-failing files (`pending-learnings`, `git-kind-learnings`,
  `checkout-refusal-silent`) fail identically on the unmodified base: POSIX path
  assertions on Windows and EBUSY from git-worktree fixtures in workers
jeff-r2026 pushed a commit that referenced this pull request Sep 29, 2026
…igrations (#893) (#901)

* fix(dry-run): load mcp inject and mcp list without persisting a migration (#893)

mcp inject loaded its scope bare, so --dry-run still saved a pending role
migration, partition rename or self-mode bootstrap. It now forwards dryRun;
mcp list is read-only and loads with dryRun: true unconditionally, as status
and list do since #866.

* fix(dry-run): forward dryRun to the loader in roles init/add/remove/update (#893)

roles list is read-only and loads with dryRun: true unconditionally.

* fix(dry-run): forward dryRun to the loader in projects add/update/remove (#893)

projects list and projects members are read-only and load with dryRun: true.

* fix(dry-run): forward dryRun to the loader in tags add/remove (#893)

tags list is read-only and loads with dryRun: true.

* fix(dry-run): forward dryRun to the loader in source add/remove/add-http (#893)

source list and source browse are read-only and load with dryRun: true.
source list also reached a second bare load through loadLocalAgentConfig,
whose HTTP backfill reads only repo.kind and repo.url; it now loads with
dryRun: true, and the migration persists on the next command that writes.

* fix(dry-run): forward dryRun to the loader in remove and uninstall (#893)

* fix(dry-run): forward dryRun to the loader in packages install (#893)

* fix(dry-run): forward dryRun to the loader in import --from-iwiki/mr/claude/repo (#893)

--from-repo-list and --from-org reach the same load through importFromRepo.

* fix(dry-run): forward dryRun to the codebase loader; --lint and --status load read-only (#893)

* fix(dry-run): forward dryRun to the loader in models switch; models list loads read-only (#893)

* fix(dry-run): load read-only commands without persisting a migration (#893)

hooks list, members, exclude list, recall status and doctor load with
dryRun: true unconditionally, as status and list do since #866.

* docs(designs): name which commands take the dry-run detection path (#893)

* test(dry-run): pin that every loader on a dry-run or read-only path migrates nothing (#893)

LOAD_ONLY_COMMANDS gains the read-only commands and the previews that stay
clean on the legacy-role fixture. PREVIEWS covers the commands that fail past
the loader on that fixture: it asserts only config.yaml, with the same call
without --dry-run as the positive control that the load is on the path.

* fix(dry-run): forward dryRun through resolveMemberToolRoots for import --from-claude (#893)

scanCandidates resolves Claude's tool root before the loader import.ts already
fixed, through a bare load in resolveMemberToolRoots, so the dry run still
saved the migration. resolveConfigForDir and findUnreadableProjectConfig take
the same optional LoadOptions for the read-only callers below; hook and usage
callers pass nothing and behave as before.

* fix(dry-run): pass dryRun into loadLocalAgentConfig instead of forcing it (#893)

Forcing dryRun there made real hook runs print the [dry-run] migration
preview and stop persisting it. source list now passes it through
describeLocalAgent; every other caller is unchanged. source add-http forwards
{ dryRun: options.dryRun } like the rest of the file.

* fix(dry-run): load skill list/show, webhook list, stats and digest read-only (#893)

detectTeam takes optional LoadOptions; the Stop-hook share gate passes none.

* docs(designs): name read-only commands by example, not as every list subcommand (#893)

* test(dry-run): prove each row reached the load, and cover the remaining changed sites (#893)

Every dry-run row now asserts the loader logged its migration preview, so an
early return cannot pass. Adds source browse, codebase --lint, skill list/show,
webhook list, digest, projects update/remove, import --from-claude, and a
config-only table for members, projects members and stats.

* fix(dry-run): keep the pre-command migration from adopting a partition on a dry run (#893)

The preAction hook passes dryRun to maybeMigrate, but planMigration and
queueKeptInCheckout resolved the partition bare, so pull/push --dry-run
still renamed a pre-#546 partition. Both now take the option.

* fix(dry-run): load skill get/path read-only through the share gate (#893)

blockReason fell back to shareGate(), a bare load, when no team was passed;
only skill get, skill path and the catalog reach that fallback. The Stop
hook still asks shareGate directly and is unchanged.

* refactor(dry-run): give loadWebhookConfig the option instead of copying its branch (#893)

Also note in loadLocalAgentConfig that dryRun covers only its config.yaml load.

* revert(dry-run): leave stats out of this change (#893)

stats also loads bare per session through the dashboard scope helpers on the
pull/report path; a partial fix would claim more than it does. Tracked in the
follow-up issue with the other dry-run gaps.

* test(dry-run): cover the pre-command migration and skill get/path share (#893)

* refactor(dry-run): give shareGate the option instead of repeating it in blockReason (#893)

Also correct the test comment on when the pre-command migration runs.

* fix(dry-run): keep loadLocalAgentConfig from writing config.json under dryRun (#893)

The option reached only the config.yaml load. The legacy group-binding
cleanup, the binding-key canonicalization and the HTTP backfill still saved
config.json, so `teamai source list` could rewrite or create it. Under
dryRun they now stay in memory, and the cleanup prints a `[dry-run] Would
remove` preview instead of `Removed`.

* fix(dry-run): keep models list from saving a re-bound beta key (#893)

models list loaded the scope read-only but read the team keys through
loadTeamValues without the option, so a key a 0.26.0 beta stored under the
profile id alone was bound to its gateway and the values file rewritten.
It now binds the key in memory only; the next write command saves it.
jeff-r2026 pushed a commit that referenced this pull request Sep 30, 2026
* refactor(entries): let a namespaced entry reader declare its own layout (#879)

An entry reader took its directory, file name, activation key and failure
wording from its EntryType. It can now declare them as an EntryLayout,
defaulting to entryLayout(type), which gives today's values. This lets a
later reader read env/secrets.yaml and env/<ns>/secrets.yaml activated by
resources.env. No behaviour change: env, hooks, MCP and models resolve and
report as before.

Part of #875.

* refactor(models): share the team values path and stdin reader with a second store

getTeamValuesPath takes the store directory (defaulting to models/teams)
and keeps its <team>-<hash>.json naming. The piped-stdin reader moves to
utils/prompt.ts as readStdin; the --api-key-stdin checks and messages stay
in the models command. No behaviour change.

Refs #879 (S2), #875

* feat(env): declare team secrets in env/secrets.yaml and show their state (#879)

A team repo can declare the secrets its members need, with no value, in
env/secrets.yaml and env/<ns>/secrets.yaml (key, optional description and
url). They resolve like env.yaml: active through resources.env, a namespace
entry replaces the root entry with the same key. The declarations are
absent, valid or failed; a broken file fails the secrets only, is reported
in secret wording by pull, env list and doctor, and env variables are still
delivered.

- env list and list env show each declared secret as environment or
  missing, never its value, --reveal included.
- doctor fails "Team secrets can be resolved" on a broken file, and its
  notes name env/secrets.yaml, not env/env.yaml, for an override or a key
  repeated in legacy mode (describeEntryNotes takes the reader's layout).
- push lists a changed secrets.yaml, in single-repo mode too.
- docs/designs/team-secrets.md and .zh-CN.md start here, with the #818
  boundary; usage guide, product overview, multi-project, management
  backend and the admin reference updated.

Part of #875.

* feat(env): declare a team secret with env add --secret (#879)

env add <key> [value] --secret [-d] [--url] [--role|--project] writes
env/secrets.yaml or env/<ns>/secrets.yaml with no value; a value is
rejected and never printed. env remove removes a declared secret when
env.yaml does not set the key, and --secret removes only the declaration
for a key both files carry. entryNamespaceFromFlags takes a layout so
the --role warning names secrets.yaml.

* feat(mcp): keep the entry an earlier pull wrote when a declared secret is missing (#879)

The session-start pull inherits the agent's environment, which often lacks
the member's shell export, so it removed the MCP entry the interactive pull
had written. A server whose only missing variables are declared secrets now
keeps its entry and ownership record; it is removed when it leaves mcp.yaml
or by removeAll. A failed secrets declaration keeps managed MCP state.

* feat(env): keep a member's value for a team secret and resolve it in MCP servers (#879)

teamai env set KEY (hidden prompt, --stdin, --from-env VAR) and env unset KEY
store a member's value per team repo in ~/.teamai/secrets/teams/, 0600,
accepting only keys the scope declares as secrets. ${VAR} in MCP servers
resolves a declared secret from that value, then from the member's own
environment, which leaves out values a teamai env.sh exported (Conflict 10).
A key declared as a secret and set in env.yaml resolves as the secret: its
repo value leaves env.sh, the env backup, both list renderers and doctor's
expected set (Conflict 13). A failed declaration leaves env.sh and the backup
as they are (Conflict 14). env list shows team.

* docs(env): document team secret values, storage and MCP resolution (#879)

* feat(env): set one value for a team secret for every team on the machine (#879)

env set/unset --global keep the value in ~/.teamai/secrets/machine.json.
Resolution becomes team value > machine value > the member's environment,
for MCP servers and env list (state `global`). In a scope --global still
accepts only a declared secret; outside any scope it accepts any valid key
and notes that no team declares it yet.

* docs(env): document the machine value for team secrets (#879)

* feat(mcp): keep project MCP configs with resolved values out of git (#882)

A project-scope MCP config that carries a resolved ${VAR} sat untracked
and unignored in the business repo, one `git add -A` from committing the
token. After the reconcile writes such a file and git would track it,
teamai lists its path in the clone's .git/info/exclude inside a marked
block (resolved via `git rev-parse --git-path`, so linked worktrees and
submodules work). The committed .gitignore is never touched; an ignored
path or a config with no resolved value adds nothing; dry runs write
nothing. Project-scope uninstall removes only teamai's block, and doctor
reports such a file git would still commit.

The hook sits after the appliers in reconcileMcpForConfig, outside
desiredMcpForTarget/applyJson/applyCodex, so it merges cleanly with #880.

* feat(env): name a missing team secret and the command that sets it (#879)

Interactive pull, mcp list, env list and doctor print one line per declared
secret with no value, naming the MCP servers that use it, `teamai env set KEY`
and the declared url. doctor prints it as a note and no longer fails the MCP
delivery check for a server skipped only for a missing declared secret. Pull
and doctor also note a kept entry that may hold an old value and a key
declared as a secret and set in env.yaml. The silent pull prints nothing.

The lines come from one envAdvisories() result that later pull notices extend.

* docs(env): document the missing-secret advisory in pull, doctor and the lists (#879)

* fix(uninstall): count the .git/info/exclude block in the removal plan (#882)

The plan now records whether the project's .git/info/exclude holds
teamai's MCP config block (gitExcludeBlock). It counts toward
isPlanEmpty, is listed in the summary and dry run, and gates the
removal, so a plan whose only teamai leftover is the block removes it
instead of reporting "Nothing to uninstall".

* feat(env): run a command with the directory's team env and secrets via env exec (#879)

* docs(env): document env exec, what the command can reach and the ANTHROPIC_* caveat (#879)

* fix(tests): isolate Claude config dir from model tests

(cherry picked from commit e567ac3)

* fix(env): warn when env add --secret updates a declaration an unknown key keeps undeclared (#879)

* feat(env): tell agents at session start which team secrets exist and to run their CLIs through env exec (#879)

* docs(env): teach agents to use team secrets through env exec and leave values to the member (#879)

* feat(env): resolve plain env variables in one order, with a member override (#879)

The environment no longer overrides an env.yaml variable in MCP servers,
env exec or env.sh. A member sets their value for a team with
`teamai env set KEY`; an interactive pull and doctor say when an export
differs and is ignored. env.sh exports a literal override and leaves out a
--from-env one; doctor's env delivery check expects the same.

* docs(env): document the variable order and the member override (#879)

* fix(mcp): keep the exclude block until every MCP config is clean (#882)

- uninstall keeps a repository's .git/info/exclude block while a config in
  it could not be parsed and still holds teamai servers, and warns
- uninstall finds and removes the block in nested repositories holding an
  MCP config, across every worktree
- the block opens at the last start marker, so an orphaned start never
  pairs with a later block's end and takes the member's lines
- doctor counts only servers the ownership manifest records, not a member's
  own server under a team name

* fix(env): discount an old env.sh export in every later command (#879)

A shell opened before a pull keeps the values env.sh exported then. Only
the process that rewrote env.sh discounted them, so the next command read
the team's old token as the member's own (Conflict 10). Each env.sh now
keeps env.sh.exports.json beside it: SHA-256 of KEY=VALUE for the last 20
values per key, mode 0600, never a value. memberEnvironment discounts any
recorded export, which replaces the in-memory snapshot.

* fix(uninstall): remove the exclude block only once its files are proven clean (#882)

A missing or unreadable managed-mcp.json made the MCP cleanup return early
without reporting anything, so uninstall removed the block while .mcp.json
still held the resolved token.

Uninstall now inspects every path the block protects after the cleanup. The
block goes only when each one is missing, or parses and holds none of the
team's servers that need a resolved ${VAR}. Anything it cannot check keeps
the block, with a warning naming the file. This replaces the leftInPlace
report from the reconcile, which the check subsumes.

* fix(mcp): protect every project MCP config holding a resolved value (#882)

Pull, doctor and uninstall each skipped a case they had not inspected and
treated it as safe. Now:

- pull lists a config in .git/info/exclude whether or not it delivered to
  it this run: a disabled or undetected tool's file, a team with automatic
  delivery off, an unreadable mcp.yaml (any teamai entry counts), a failed
  write to another tool's config, and a lost ownership manifest (the
  resolved value found in the file)
- doctor checks the same files, including one that does not parse, and
  counts a git error as a failure
- git check-ignore failing inside a repository is no longer read as "not
  tracked": the path is excluded anyway, or teamai warns with git's error
- uninstall also keeps the block while a file contains the value (8+
  characters, not a path or the login name) of a variable still set in the
  environment, which finds a server since dropped from mcp.yaml

* refactor(env): resolve a scope's env once per command (#879)

resolveTeamEnv (src/env-resolution.ts) reads env.yaml, secrets.yaml,
both value stores and the env.sh exports once. buildVarTable, the MCP
reconcile, envAdvisories, env exec, doctor and pull take that result:
an interactive pull resolves once per scope in its env stage and reuses
it for MCP and the advisories (was up to four secrets.yaml reads).

- Variable and secret values move out of resources/secrets.ts, which now
  holds declarations only (R3); one StoreResolution<V> and storeEntry
  (R2); resolveSecretDeclarations takes { active } instead of a
  tri-state array (R4); reportMissingSecrets lives in env-advisories (X1).
- ENV_KEY_RE moves to resources/env-key.ts, so env.ts imports
  SECRETS_LAYOUT statically (EV1).
- env list and `teamai list env` print one listing (env-listing.ts);
  a variable shows the value it resolves to and its source, team or
  env.yaml, like a secret (S1).
- A failed declaration is never "no secrets": the listings show no
  variable value then, --reveal included, and a server skipped for a
  missing variable is kept (S2).
- SecretState gains `unreadable` for a store that can't be read,
  handled with a never check (R1).

* fix(mcp): mcp list reports a broken secrets.yaml and exits 1 (#879)

A failed declaration read as no secrets, so every server variable came
from the raw environment, another team's token included, and mcp list
said 'all set' while pull kept MCP frozen. It now names the file, exits
1, and shows a server's variables as 'not resolved' (Conflict 14, C3).

* fix(env): env commands outside a scope say so and exit 1 (#879)

env list, add, remove, set and unset threw NotInitializedError with a
stack trace outside any teamai scope. They now print its message and
exit 1; env set/unset --global still work there.

* fix(env): env exec refuses a command without -- before it (#879)

Commander drops --, so `teamai env exec gh pr list --dry-run` read the
command's flag as teamai's and ran nothing. env exec now takes what was
typed after `exec`, and without -- before the command it says
"Put -- before the command: teamai env exec -- <command>" and exits 2.
teamai's own options may still come before --.

* feat(doctor): check that the member's secret values can be read (#879)

While teams/<team>.json or machine.json can't be read, every secret has
no value and MCP keeps what the last pull wrote, yet the MCP check
passed and only a pull warning said why. doctor now fails
'Your team secret values can be read' with the reason, for a scope whose
secrets or variables read those files.

* fix(env): name the unset variable a --from-env secret reads (#879)

The missing-secret line told the member to run `teamai env set KEY`,
which replaces the reference they chose. For a secret whose deciding
entry reads an unset variable it now says: KEY reads VAR, which is not
set. Set VAR, or run `teamai env set KEY [--global]` to replace the
reference.

* test(mcp): env unset keeps the entry, still owned, until a new value (#879)

env set, pull, env unset with nothing exported, pull: the entry keeps
the old value and the manifest still claims it, so the next value
replaces it instead of colliding with a user-owned server.

* refactor(env): expected env set/remove failures as values, one wording (#879)

- secretInput returns a tagged result; an unexpected stdin or prompt
  failure propagates instead of printing as a user error (E1).
- Every error path in env-commands uses fail() and invalidKeyMessage(),
  so each exits 1, env add's invalid key included (E2).
- removeSecret returns removed | absent | reported: env remove of a
  variable env.yaml lacks still says it is not found next to a broken
  secrets.yaml (E3).
- env add branches on --secret, not on a missing value (E4).
- env unset with declarations that can't be read no longer calls the key
  a variable (E5).
- "Secret is not declared" names the file and the next step (E6).
- Messages call the machine store "global value (every team on this
  machine)", matching --global and the env list label (R5).

* refactor(entries): EntryFailure always carries its layout (#879)

An optional layout fell back to the type's, so a failure site that forgot
it would word a broken secrets.yaml as env's. activeEntryNamespaces now
takes the layout and every failure sets it (N1).

* refactor(doctor): name entry resolution checks with their layout (#879)

The check names were looked up by the layout's message label, so a
wording change to a label would silently drop its doctor check. Each
entry set now carries its check name (DD1).

* refactor(secrets): report an unparsable value store by path only (#879)

- Drop src/utils/json-position.ts, a second JSON grammar kept only to
  print a line and column: a parse error now reports the path alone,
  which is what the spec asks (never the input), and a disagreement with
  JSON.parse can no longer leak the parser's quote (SS3).
- The unreadable-store error names what to check and the way out, and
  reads the error code without a cast (SS1).
- SecretStore is inferred from its schema (SS2).
- The design doc says an unreadable store leaves secrets 'unreadable'.

* test(secrets): typed fixtures instead of casts in the new tests (#879)

The LocalConfig and TeamaiConfig fixtures in env-exec, mcp-secrets and
secret-values are now checked against the types, so a new required
field breaks them instead of testing a shape production never has (T1).

* docs(env): env set takes variables, listings show sources, exec needs -- (#879)

- Usage guide: `env set` accepts an env.yaml variable without --global,
  not only a declared secret (D1/C2); `env exec` links the resolution
  order instead of "the order above", which the section never gave (C4).
- team-secrets.md intro states the variable order; a --from-env variable
  override leaves the key out of new shells entirely.
- Document the shared listing (variable source, `unreadable`, no values
  while the declarations fail), `mcp list`'s `not resolved`, the doctor
  check for an unreadable values file, and `env exec`'s exit 2 without --.

* fix(env): refuse env set/unset/list when the project config can't be read (#879)

Detection returned null for an unreadable project config, so env set fell
back to the user scope and stored the value for that team. Report the file
and why, exit 1, and write nothing, as pull and env exec do.

* fix(env-exec): exit 128 + signal for a command killed by SIGPIPE or SIGUSR1 (#879)

Re-raising SIGPIPE on teamai does nothing (Node ignores it) and SIGUSR1
starts the inspector, so teamai exited 0. Set the shell's exit code first
and re-raise only the signals Node ends on.

* fix(env-exec): don't send the command a second SIGINT on Ctrl-C (#879)

The terminal sends Ctrl-C and Ctrl-\ to the whole foreground process group,
so the command already has them; forwarding sent a second SIGINT, which
tools such as terraform take as force-quit. Ignore SIGINT and SIGQUIT while
the command runs and keep forwarding SIGTERM and SIGHUP.

* docs(env): exec signal handling and env set on an unreadable project config (#879)

* fix(mcp): judge MCP configs by disk and manifest, not current config (#882)

Pull, doctor and uninstall still decided "clean" from the current team
config in places. Now one function, resolvedValueEvidence, decides for all
three:

- a teamai-owned entry still in the file counts when its server has left
  mcp.yaml, as well as when it needs a resolved ${VAR} or mcp.yaml cannot
  be read (doctor no longer skips that case)
- targets include the built-in location of a tool the team dropped from
  toolPaths or moved
- doctor names a file two tools share once
- exclude updates take the existing acquireLock helper, re-read the file
  and write it atomically, so concurrent commands keep each other's paths
- uninstall inspects every worktree of each repository owning a block,
  including a nested repository's linked worktrees, and applies the
  manifest rule per worktree
- the kept-block warning names each file and why, such as the variable
  whose value matched

* fix(env): the env commands and env exec load config through --dry-run (#866)

#866 threaded { dryRun } through the loaders pull, push, status and list
use. The scope lookups this branch added did not take it, so
`teamai --dry-run env list|set|unset|add|remove` and `env exec --dry-run`
still persisted the legacy role migration, adopted a pre-#546 partition
or ran the self-mode bootstrap.

- resolveConfigForDir takes LoadOptions and passes them to both loaders.
- scopeHere/requireScope (env commands) and commandEnvironment (env exec)
  forward options.dryRun. Without the flag nothing changes: env exec still
  runs the migrations every command runs (spec #879 Conflict 12).

Five cases added to dry-run-load-path.test.ts; each failed before this
change (config.yaml rewritten, partition renamed).

* fix(secrets): name the team values file by the repo identity hash alone (#879)

Renaming team: in teamai.yaml changed <team>-<hash>.json and orphaned every
member's values. The secrets store now uses ~/.teamai/secrets/teams/<hash>.json;
the models key files keep their names. The path has not shipped, so there is
no migration.

* fix(env): keep a __proto__ env key through the store, MCP and env exec (#879)

ENV_KEY_RE accepts __proto__, but z.record dropped it from the value store,
assigning it on an ordinary object hit the inherited setter, and reading it
unset returned Object.prototype. The store, the MCP var table and the env exec
overlay are now built without a prototype (envTable), and the member's
environment and --from-env references are read as own keys (envValue).

* fix(env): env list loads config with --dry-run always (#879)

env list only reads, so like status and list since #866 it never migrates
the config it loads, with or without --dry-run. The dry-run load-path test
now runs env list without the flag, which is the case that used to migrate.

* revert: drop the #890 cherry-pick from this branch

2e15c3f was picked only to protect the local Claude config while this
branch's tests ran; #890 lands it on main on its own.

* fix(env): env exec applies no team env while the declarations fail (#879)

A legacy GITHUB_TOKEN in env.yaml plus a secrets.yaml that fails gave the
command the repo value, though the key may be a declared secret. Like
env.sh and the backup (Conflict 14), env exec now overlays nothing on a
failed declaration: the command gets the inherited environment, and
stderr names the failure.

* fix(fs): create the atomic-write temp file with the target mode (#879)

writeFileAtomic and writeJsonAtomic wrote the temp file with the umask's
default mode (0644 under umask 022) and narrowed it by chmod afterwards,
so a secret or model key was readable by other users until then. The temp
file is now opened exclusively with the target mode; the chmod stays for
the bits the umask removes.

* fix(env): env set --dry-run previews without asking for the value (#879)

`teamai --dry-run env set KEY` prompted for the value, or read stdin with
--stdin, before printing the preview, so it failed without a terminal. The
preview now comes first, and no value is read.

* fix(env): serialize env set and env unset on the values file (#879)

Two env set or env unset runs at once each read the store, changed it and
wrote it back, so the later write dropped the other's change. Both now go
through updateSecretStore, which takes <store>.lock (the acquireLock
helper), re-reads the file inside it and writes the result. A --dry-run
takes no lock.

* fix(env): keep a secret's stored value from becoming a variable override (#879)

Secrets and variable overrides share the team store. env set now records
kind: secret | variable from what the scope declares; variable resolution
uses only variable entries, secret resolution only secret ones, and an entry
without kind counts as a secret. env list flags an entry of the other kind
with the fix.

* fix(mcp): write an MCP config that holds a resolved value 0600 (#879)

writeJsonAtomic preserved an existing file's mode, so a 0644 .mcp.json or
~/.claude.json kept 0644 after teamai wrote a resolved secret into it. A
config holding a resolved ${VAR} value (a kept entry included) is now
written 0600; one without keeps its mode.

* fix(mcp): create the Codex config temp file 0600 (#879)

applyCodex wrote config.toml.<pid>.tmp with the umask's mode and chmodded it
after, so a resolved secret was briefly readable at 0644. Both Codex writes
now go through writeFileAtomic with mode 0600 (random temp name, created
with the mode, symlinked targets written through).

* docs(env): say env exec ignores a SIGINT or SIGQUIT sent to teamai alone (#879)

* fix(mcp): tighten an unchanged config that holds a resolved value to 0600 (#879)

* fix(env): mark what each env.sh exported so a value from one no scan finds is not the member's (#879)

* fix(env): pass on a SIGINT or SIGQUIT sent to env exec outside the terminal's foreground (#879)

* fix(env): keep marking what an env.sh exported before a rewrite dropped it (#879)

* docs(env): say a SIGINT sent to a foreground env exec alone is not passed on (#879)

* fix(secrets): name the team values file by the configured team repo URL, not teamai.yaml's repo: (#879)

A copied or hostile team repo could claim another team's repo: and receive
that team's stored values. The secrets file now hashes the URL from the
member's own config, normalized so ssh, https and credentialed forms match.
The models key store keeps its naming (#894).

* fix(env): remove what a teamai env.sh exported from env exec while the declarations fail (#879)

An invalid secrets.yaml left the inherited environment untouched, so a
shell that had sourced an env.sh still passed the repo's GITHUB_TOKEN to
the command. Every inherited value the member-environment rule discounts
is now removed, named by key on stderr; the member's own exports stay.

* fix(mcp): never write a declared secret into a project MCP config git tracks (#879)

.git/info/exclude (#882) stops git add, not a file git already tracks. For
such a file the server is skipped for that tool and an entry an earlier pull
wrote stays as it is; pull warns, mcp list shows it as withheld and doctor
fails the delivery check, each naming the file and git rm --cached.

* fix(env): leave no duplicate declaration of a key env add --secret or env remove edits (#879)

A key declared twice fails every read of secrets.yaml, and both commands
edited only the first declaration. env add --secret now updates the first
and removes the rest; env remove removes every one; both say how many.

* test: keep the real normalizeRepoUrlForCompare in utils/git mocks the secrets store reaches (#879)

* fix(env): keep the port in the URL that names a team's secrets file (#879)

normalizeRepoUrlForCompare drops explicit ports, so two team repos on one
host with different ports shared one values file and one team's secret
reached the other. The file is now named by the URL's scheme family,
lowercased host, non-default port and path; only credentials, the ssh user,
a trailing .git and slashes are dropped. The scp form and the ssh URL of a
repo still share a file; its ssh and https URLs no longer do.

The utils/git mocks that kept the real normalizeRepoUrlForCompare for the
store are no longer needed and are reverted.

* fix(mcp): skip the .git/info/exclude write while another command holds its lock (#882)

After the 2.5 s wait for the exclude file's lock, updateExclude wrote without
it, so two writers could drop each other's pattern and leave a plaintext MCP
config committable. It now writes nothing and reports 'locked': pull warns that
the file is not excluded yet and to run `teamai pull` again (doctor's exclude
check keeps reporting it meanwhile), and uninstall keeps the block and warns.

* fix(uninstall): keep an exclude entry unless its MCP config is proven free of teamai's servers (#882)

Uninstall judged a protected file clean from the current mcp.yaml, manifest
and resolvable values, so with the manifest lost, the server gone from
mcp.yaml and its value unset, a plaintext token looked like the member's own
server and the exclusion went. It now fails closed and works per entry: a
pattern goes only when its file is gone, holds no server, or holds none of
teamai's servers with managed-mcp.json still there to say what teamai wrote.
A kept entry is named with its file, why, and how to clean it by hand, since
a rerun of uninstall finds no config after a full uninstall.

* fix(env): keep http and https team repos in separate secrets files (#879)

The store identity mapped https and http to one `http` family and dropped
each default port, so `http://host/team.git` and `https://host/team.git`
read one file: if the two endpoints serve different repos, one team got the
other's stored secrets. The identity now keeps the scheme, still dropping
443 and 80; the ssh forms (`ssh://`, `git+ssh://`, `ssh+git://`, scp) stay
one family.

* fix(env): strip teamai env.sh exports in env exec when the project config can't be read (#879)

With an unreadable project config, env exec passed the inherited
environment unchanged, so a value another scope's env.sh exported (a legacy
token, a member override) reached the command while the warning said no team
values were applied. It now removes what the member-environment rule
discounts (env.sh file, record or marker, the env.sh beside the unreadable
config included), keeps the member's own exports, and names the removed keys,
never values, as the failed-declaration path does. With no config at all the
inherited environment is still passed as is.

* fix(mcp): exclude a project MCP config from git before writing a resolved value into it (#882)

Pull listed the file in .git/info/exclude only after writing the plaintext,
and a failed exclusion only warned, so the secret-bearing file stayed
eligible for git add -A. The exclusion now comes first; when it cannot be
established (exclude file or .git/info not writable, lock held past the
wait, file already tracked, git error) the file is left as it was and the
warning names the reason and the fix.

* fix(mcp): report a server withheld from a file git would commit in mcp list and doctor (#882)

* fix(secrets): keep the ssh user in a team's values-file identity (#879)

alice@host:team.git and bob@host:team.git can be different repos in each
user's home; they no longer share one values file. The scp and ssh:// forms
of one user, host, port and path still match; http(s) credentials are still
dropped.

* fix(env): overlay and remove env exec keys case-insensitively on Windows (#879)

Windows environment names are case-insensitive, so a declared api_url left an
inherited API_URL in place (Node keeps the first name of a case-folded pair)
and a removed secret survived in another case. On win32 a key now replaces or
removes every case variant; elsewhere nothing changes.

* fix(mcp): report a tracked file on a dry run and a withheld server already installed (#882)

Backports #880's merge 0d9f7fa: a dry run (doctor, mcp list) names a
tracked file before any pull has listed it, mcp list reports withheld for a
server an earlier pull installed, and doctor's withheld note carries the
exclusion's own fix instead of the pull --force advice.

* fix(mcp): name a tracked MCP config before an unwritable .git/info/exclude (#882)

A tracked file needs `git rm --cached` whatever else is wrong, so
ensureExcludedFromGit checks gitTracks before the writability check, on a
pull and a dry run alike, and lists nothing for it.

* fix(mcp): take a project MCP config's exclude line back out once it holds no resolved value (#882)

A pull that lists a config in .git/info/exclude and then writes no value
into it (it does not parse, a member's server holds the team's name, the
write fails) removes the line it added. After a pull or `teamai mcp
remove`, a line whose configs are proven clean in every worktree, by the
proof uninstall uses (moved to mcp-reconcile.ts), is removed under the
lock; one not proven clean stays. A config listed before its write is
listed again after it, so a concurrent uninstall that dropped the line
between the check and the write does not leave the value unprotected.

* fix(secrets): tell an scp path in the ssh user's home from an ssh:// path from the root (#879)

git@host:acme/team is relative to the ssh user's home, ssh://git@host/acme/team
is absolute; on a plain ssh host they can be different repos, yet they shared
one values file. An scp path starting with neither / nor ~ is now keyed as
~/path, the path ssh://host/~/path names; host:/abs and ssh://host/abs still
match. The scp form without a user (host:path) is now read as ssh too.

* fix(mcp): judge a project MCP config by the manifest as it stood before the pull rewrote it (#882)

A pull whose manifest was lost before it ran recreates managed-mcp.json
while reconciling, so the clean-file proof read the new record and took
the exclude line out of a file still holding a teamai server that left
mcp.yaml with its variable unset. The proof now uses this worktree's
manifest as read before the reconcile.

* fix(mcp): log a rolled-back exclude line at debug level (#882)

A line this pull added and took back out, because it wrote no resolved
value into the file, was reported as removed although the member never
saw it added. Only removing a line an earlier run added stays at info.

* fix(mcp): keep a shared exclude line while another worktree's config holds a server (#882)

A pull or `teamai mcp remove` judged every linked worktree's MCP config
with today's definitions and values. Once a server's ${VAR} became a
literal, and the value was no longer set, a pull in worktree A took
worktree B's stale token-bearing entry for clean and removed the shared
/.mcp.json line, so `git add -A` in B staged the token.

These commands now release a line only when the current worktree's file
passes the full proof and every other worktree's file is missing or holds
no MCP server. `teamai uninstall` keeps its full proof in each worktree.

* test(mcp): build the other worktree from the real temp path so the test checks what it names (#882)

* fix(env): strip teamai env.sh exports from env exec with no config or an HTTP scope (#879)

Neither path applies team values, yet both passed on whatever a sourced
teamai env.sh exported, another team's credentials included. Remove every
inherited value that is not the member's own, as the unreadable-config
path does, and name the removed keys on stderr.

* fix(secrets): name a team's values file by the full SHA-256 digest (#879)

The file name is the boundary between teams, and 40 bits let a hostile
team search for a repo URL whose name matches another team's file and
read its values. Nothing has shipped, so no migration.

* fix(env): name the env.sh provenance marker by the full SHA-256 of its data home (#879)

* test(push): push publishes the secrets.yaml env add --secret leaves (#879)

* fix(uninstall): list no worktrees for a project root that no longer exists (#882)

buildRemovalPlan now lists every worktree to find teamai's exclude blocks,
outside the MCP cleanup's try. simple-git throws synchronously for a missing
directory, so a project uninstall whose root is gone crashed; main's #878 test
caught it after the merge.

* fix(secrets): keep the URL query and fragment in a team's values file name (#879)

* fix(mcp): treat an empty, unparsable or tool-less managed-mcp.json as no record (#882)

The clean-file proof took any managed-mcp.json on disk as teamai's record,
so an empty or truncated one let a pull, `mcp remove` or uninstall judge a
file still holding a stale secret-bearing entry clean and drop its exclude
line. A file now counts as recorded only when the manifest parses and holds
an entry for that tool's file. A project record teamai empties stays as []
so a file left with only the member's own servers can still be released.

* fix(env): compare exported, recorded and marked keys case-insensitively on Windows (#879)

* fix(uninstall): list no worktrees for a project root that no longer exists (#879)

* fix(mcp): keep the exclude line of a config this pull wrote when a later step fails (#882)

* fix(mcp): read a check-ignore error as unsafe unless ls-files proves the config untracked (#882)

* fix(mcp): report withheld only for targets delivery would write the server to (#882)

* docs(mcp): describe the .git/info/exclude block in the setup skill and the stricter git check (#882)

* fix(mcp): keep the exclude line of an entry a pull wrote with a resolved value after its definition turns literal (#882)

* fix(mcp): judge a nested repository's linked worktree config by its sibling's tool (#882)

* feat(mcp): record the project MCP configs a pull wrote a resolved value to in managed-mcp-files.json (#882)

* fix(mcp): keep protecting a config a pull wrote under a toolPaths mapping the team has since changed (#882)

* fix(mcp): keep a config's exclude line past the pull that rebuilt its lost record (#882)

* test(mcp): pin today's exclude rules for a missing, corrupt or locked managed-mcp-files.json (#882)

* docs(mcp): describe managed-mcp-files.json and the configs it keeps protected (#882)

* refactor(mcp): keep the #882 record edits off the lines #880 changes (#882)

* test(mcp): keep excluding a config whose entry a pull kept for a missing declared secret (#875, #882)

* fix(mcp): protect a config an older teamai wrote under a mapping an earlier teamai.yaml made (#882)

A teamai from before managed-mcp-files.json kept no record of the path it
wrote a resolved value to. Once the team changed that toolPaths mapping, no
pull visited the file. The first pull on this version now reads every
mcpProject path the team repo's history of teamai.yaml mapped, once per
worktree: a file under the project root that no current mapping or record
reaches, and that holds a resolved value, is listed in .git/info/exclude and
recorded. A git error leaves the read for the next pull; a shallow clone
reads the history it has.

* fix(mcp): keep a rebuilt record from persisting without its note of the file's other servers (#882)

When a pull rebuilt a lost managed-mcp.json and could not note the other
servers in the file (managed-mcp-files.json locked, an I/O error), it still
wrote the rebuilt record, so no later pull knew the record was rebuilt and a
stale server's line could go. The same manifest write now marks those records
unnoted: the file counts as having no record, so it keeps its line while it
holds a server, and the next pull notes them and clears the mark.

* fix(mcp): take back a managed-mcp-files.json record for a config the pull then did not write (#882)

A pull records a config before writing a resolved value to it. When the
write failed or did not happen (the file does not parse), the record stayed,
and once the mapping changed a config of the member's own at that path was
kept excluded while it held any server. The pull now takes back a record it
added for a file it did not write, as it does the file's exclude line; the
settle after records it again if the file holds a resolved value anyway.

* docs(mcp): describe the teamai.yaml history read, the unnoted rebuilt record and the record a failed write takes back (#882)

* refactor(mcp): keep the r3 edits off the lines #880 changes (#882)

* fix(mcp): also protect a config an older teamai wrote under a built-in default it has since changed (#882)

* fix(mcp): replace a symlinked Codex config instead of writing into the file it links to (#875, #882)

* fix(env): drop what a teamai env.sh exported from env exec when env.yaml or the values file fails (#875)

* fix(env): on Windows, match a declared secret to an env.yaml variable in any case (#875)

* fix(mcp): judge a project MCP config under a symlinked directory where the write lands (#882)

The appliers replace the file itself (tmp + rename) but follow its
directories. Every git check now judges realFilePath(file), the one
resolver the release keying already used: a directory linked out of any
repository no longer withholds the servers on git's "not a git
repository", and a tracked file there is named with both paths, with a
git rm --cached that works (git refuses the path through the link).

* docs(mcp): describe how a config under a symlinked directory is kept out of git (#882)

* refactor(mcp): keep realFilePath next to existingAncestor, without an import cycle (#882)

* refactor(mcp): key each worktree's targets with realFilePath, the one rule for where a write lands (#882)

* fix(mcp): judge a config under an earlier teamai.yaml mapping as a recorded file, not by today's records (#882)

* fix(mcp): have doctor check the configs earlier teamai.yaml mappings reach until a pull reads them (#882)

* docs(mcp): describe how a config under an earlier teamai.yaml mapping is judged, and doctor's check of it (#882)

* fix(mcp): record a config under an earlier teamai.yaml mapping that git tracks, and judge it once git no longer does (#882)

* fix(mcp): keep judging a recorded config for a tool the team moved while another tool still maps it (#882)

* docs(mcp): describe the tracked config an earlier mapping reached, and a moved tool's config another tool still maps (#882)

* fix(mcp): find a config an older teamai wrote under an earlier mapping another tool maps today, and judge it by that tool's records (#882)

* docs(mcp): describe the history read's configs another tool maps today (#882)

* fix(mcp): prove a shared config clean only while every tool that wrote a resolved value there has its record (#882)

* fix(env): on Windows, set and unset a key under the name the scope declares, in any case typed (#875)

* fix(mcp): judge a built-in location no mapping reaches today as an earlier-mapped file, and a shared config no pull recorded by every tool mapping it (#882)

* docs(mcp): describe the built-in location of a moved or dropped tool, and a shared config no pull recorded (#882)

* fix(env): on Windows, match a secret's declaration and stored value in any case (#875)

* fix(mcp): name the ignore rule that re-includes a config teamai just listed, instead of saying git tracks it (#882)

* fix(mcp): judge a moved tool's built-in location another tool maps for that tool too, and hold a config's line while no managed-mcp.json claims its servers (#882)

* docs(mcp): describe the no-manifest rule, a moved tool's built-in location another tool maps, and a re-including ignore rule (#882)

* fix(env): key a team's values by its repo URL when the configured remote is only an alias (#875)

* fix(env): on Windows, recognise a secret's env.yaml value under another case of its name in the environment (#875)

* fix(mcp): on Windows, keep an inherited value under another case of a team variable's name out of MCP servers (#875)

* fix(env): on Windows, replace and remove every stored entry under another case of the key (#875)

* fix(mcp): keep a tool's record as it was when its config does not parse, and take back each tool a write that did not happen recorded (#882)

* fix(mcp): mark the records a pull with no managed-mcp.json writes as unnoted until the servers no record claims are noted (#882)

* fix(mcp): have doctor judge a record marked unnoted like a missing managed-mcp.json (#882)

* fix(mcp): judge a config tools of different formats share in each of their formats before releasing its line (#882)

* fix(mcp): note the servers no record claims in the file of a tool whose record a pull writes first, as with no managed-mcp.json at all (#882)

* fix(mcp): take back a tool managed-mcp-files.json recorded before a write unless its own records hold a resolved value there (#882)

* fix(mcp): treat an installed tool's missing record as lost at every pull and in doctor, not only an empty managed-mcp.json (#882)

* fix(mcp): tighten to 0600 every project config the protection pass keeps out of git, not only the ones a pull writes (#879)

* fix(mcp): on Windows, fill a placeholder from the variable of the same name in another case (#875)

* fix(mcp): note the unclaimed servers under every format of a shared config, and pin an uninstalled tool's leftover config (#882)

* fix(mcp): count an uninstalled tool's missing record when no installed tool maps its file, and name a re-including .gitignore rule on a dry run (#882)

* fix(mcp): on Windows, match a placeholder to its secret in any case in mcp list and the missing-secret notice (#875)

* fix(mcp): scope a shared config's claims to the tools reading the same key, and settle its notes on every format's view (#882)

* fix(env): keep a file:// repo's .git suffix in its values file identity: team and team.git are two directories (#875)

* fix(env): on Windows, update and remove an env.yaml variable typed in another case (#875)

* fix(mcp): keep suspect an uninstalled tool managed-mcp-files.json lists as a writer, though an installed tool maps the file (#882)
@Smilewithoutfalling

Copy link
Copy Markdown
Contributor Author

@ydflow — status delta on the four findings, and a correction of mine first.

Correction

In my comment above I wrote that main:src/push.ts:805 cited a file that does not exist — "There is no src/env.ts in this repo … That one is on me." That was wrong. Retracting it.

src/env.ts is 404 on main, but src/resources/env.ts is there, 545 lines, and does exactly what the comment describes: under isSelfMode with a projectRoot, line 243 builds path.join(localConfig.projectRoot, '.teamai') and line 246 builds baseEnv from localConfig.repo.localPath — the two paths the comment names. The comment sits at push.ts:871-877 and is accurate. Nothing to fix, nothing for me to take over; the error was my contents lookup against the literal path I guessed, not the comment.

Your four findings on today's main (b2d3598b)

finding state
queue publish / <home>/.teamai/locks/ closed — #896, merged 09-29T08:17:20Z
the pull.ts .gitignore self-heal still live — re-measured today, PR #944
the lock answer → read-only fetch not landed — fetchTeamRepoReadOnly has 0 hits on main, and refreshTeamRepo's git branch still reaches pullRepo unconditionally at :132
the pre-scan state write kept on purpose — push.ts:1189-1203, with the reason written next to it

Closed, verified by file and not by title: learnings-publish.ts has exactly one commit on main since #866 merged — 671f509f (#896). Line 99 now hands dryRun to listPendingForInstall, so a preview never takes the queue lock, and line 132 returns before the locked branch.

Still live, measured on the file main carries now (src/pull.ts blob 509a8135, byte-identical to the one I ran):

before pull --dry-run   .teamai/.gitignore sha256 ed747943d24dc463
after  pull --dry-run   .teamai/.gitignore sha256 f572d520732a16c2

Real CLI, self-mode clone, the file committed in the pre-beta.5 shape. The whole-fixture tree differed in exactly one path, app\.teamai\.gitignore; removed paths: none. The shape is the one you named: refreshTeamRepo (main :87) takes no options, so it cannot tell a preview from a real run, and its call site (:916) sits above every guard in pull — while push.ts:864 has the if (!options.dryRun) that pull lacks.

The other two I did not drive; both are read off the code, and they point opposite ways. The read-only fetch is absent rather than different — fetchTeamRepoReadOnly has no hits on main and pullRepo is still called unconditionally at pull.ts:132. The state write is not an oversight: push.ts:1189-1195 says the record is kept even under --dry-run, because the sync has already written the files, and the dry-run exit is at :1717, after it. So one is an unfinished half, and the other is a decision I would have to argue against on its merits.

jeff-r2026 pushed a commit that referenced this pull request Oct 2, 2026
`refreshTeamRepo` self-heals an older `.teamai/.gitignore` -- one that still
ignores a bare `env` (pre-beta.5). That rewrites a *tracked* file in the member's
own checkout, and `pull` reaches the call above every dry-run guard: the function
takes no options, so it cannot tell a preview from a real run.

Thread `options.dryRun` into the refresh and gate the self-heal on it. The
migration is idempotent, so the next real pull still performs it.

Measured with the CLI on a self-mode clone whose `.teamai/.gitignore` is the
pre-beta.5 shape and is committed:

  before pull --dry-run   .teamai/.gitignore sha256 ed747943d24dc463
  after  pull --dry-run   .teamai/.gitignore sha256 f572d520732a16c2

Whole-fixture tree diff: exactly one path changed, and it was that file.

This is the one finding from the review on #866 that I could still reproduce on
main today; the empty `locks/` directory it also reported is fixed in #896.
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.

[bug] pull / push / status --dry-run still write: the loaders they use take no { dryRun }

3 participants