Repository navigation
fix(uninstall): remove what teamai created and left empty - #1817
Conversation
1627d73 to
927cba3
Compare
The PR description includes a representative before/after real-CLI world record, satisfying the testing requirement. No PR code was run, built, or installed. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Reviewed with the built CLI in verify worlds, on the merge base (531eb15f) and on 1627d739. 927cba33 is the same change rebased onto 09f3ca7d, so everything below applies to the current head.
The default path works: init, pull and uninstall end with verify reporting CLEAN and only the two known leftovers. debug.log staying, and KNOWN_LEFTOVERS cut to two, match the decisions in the body.
Blocking
-
[P1 blocking] The "another project still needs them" guard does nothing outside user scope.
sweepCreatedLeftoversreturns early whenplan.keptForProjects.projectsis non-empty (src/uninstall.ts:3320). That list is filled only whenincludeShared && globalAdapters(:1554), andglobalAdaptersis user scope only. In workspace scope, which is the default forinit, the list is always empty. Setup:- two checkouts on one HOME, each with
init --agent codebuddyin workspace scope; teamai uninstallin checkout A.
~/.teamai/bin/teamaiis removed. Checkout B's CodeBuddy hooks runPATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch … || true, so with no otherteamaion the tool's PATH, B's hooks now find nothing and fail silently. On the merge base the wrapper stays. Computing the kept projects for every scope, or checking the other data homes before the sweep, would close it. A workspace-scope case with a second checkout inuninstall.test.tswould hold it. - two checkouts on one HOME, each with
Non-blocking
-
[P2 non-blocking] Uninstall deletes a member's empty files that existed before
init.- Shell profiles. A pre-existing empty
~/.zshenv, or a~/.bashrcwith only whitespace, is removed once teamai's block leaves it (src/uninstall.ts:3136). - Settings files. A
~/.claude/settings.jsonthat was{}beforeinitis removed (:450-467, called at:2800), now for every tool. By reading the code, a member's{"hooks":{"Stop":[]}}qualifies too; I did not run that one.
No content is lost, so this does not block. But the body says "a profile that held only teamai's block is teamai's", and that holds only when teamai created the file. Noting at write time whether the file existed, and removing it only when teamai made it, would make the rule exact. JSONC hook files survive today only because strict
JSON.parsefails on them, which is luck rather than a rule. - Shell profiles. A pre-existing empty
-
[P2 non-blocking] The sweep runs after a failed removal. With a read-only settings file, uninstall ends with "Uninstall incomplete" and exit 1, yet the wrapper is removed. That settings file's hooks still call it, so the retry path loses the wrapper it needs. Skipping the sweep when any removal is left would fix it.
-
[P3 nit]
teamai.cmdandteamai-codebuddy-hookare removed without the marker check thatteamaigets. -
[P3 nit] The CHANGELOG line could name
~/.claudeand<project>/.claude, and say the rule covers files teamai created.
CI has not run at this head yet, and CHANGELOG.md was the only conflict with main before the rebase.
927cba3 to
6485eca
Compare
|
Both earlier findings remain unresolved in the current diff:
The PR description provides sufficient representative real-CLI before/after verification. Its older recorded head is not blocking. No PR code was run, built, or installed. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Checked 6485eca4. It adapts two real-CLI tests to settings files that uninstall now removes, which is consistent with the change, but it does not touch src/uninstall.ts. The points from my review stand. Two of them are blocking now, in line with the CI review at this head:
- [P1 blocking] The sweep ignores
plan.includeShared.sweepCreatedLeftovers(src/uninstall.ts:3303,:3320) checks only the kept-projects list. That list is empty outside user scope, and it is also empty when no other project is registered. Souninstall --agent claude, with CodeBuddy still set up, removes the wrapper CodeBuddy's hooks call. The same holds for the workspace-scope second checkout from my review. Skipping the sweep wheneverincludeSharedis false, and computing the kept projects for every scope, covers both. - [P1 blocking] The wrapper goes after a failed removal. I had this at P2. The CI review rates it blocking, and our run reproduced it: with a read-only settings file, uninstall exits 1 with "Uninstall incomplete", yet
~/.teamai/bin/teamaiis removed while the kept hooks still call it. The sweep should not run whilehooksLeft,blocksLeftorfilesLeftis non-empty.
The P2 on deleting a member's pre-existing empty files, and the two nits, are unchanged.
6485eca to
902215e
Compare
|
All three blocking findings are fixed in
One thing worth flagging from writing that guard test: my two earlier cases were vacuous. With a user-scope config the data home is World re-run after the guard change ( The |
Checked
Good catch on the two vacuous cases. Moving them to a project partition makes them prove what they claim. On One point from my first review is still open before I approve:
The CI review at this head is still running. |
|
No new findings confidently supported by the scoped diff. Both earlier findings are resolved: The PR description includes sufficient representative before/after real-CLI world verification. The older recorded head is not blocking under the trusted review rules. No PR code was run, built, or installed. |
902215e to
e3b0baa
Compare
|
Round two, all self-inflicted and all found by running things rather than reading them. Your two P1s, and the bot's two, are fixed in this push:
Then CI and my own e2e run turned up a third problem, which is the interesting one. I had the sweep prune So the decision changed: a tool's own directory stays. teamai removes what it put inside One process note that cost me an hour and is worth knowing for review: the e2e suites run World re-run after the change ( |
Checked The one open point from my previous comment still needs an answer before I approve: a member's empty profile or CI and the CI review are still running at this head. |
|
No new findings confidently supported by the scoped diff. Both earlier findings are resolved: The PR description includes sufficient representative before/after real-CLI world verification. An older recorded commit is not blocking under the trusted review rules. No PR code was run, built, or installed. |
e3b0baa to
4575579
Compare
|
E2E shard 2 turned up the last one, and it changed the rule I was applying.
So the rule is not "an empty directory teamai made goes". It is whether anything resolves a path through it:
Final count: 15 known leftovers to 5. Ten removed — the three shell profiles that held only the block, the emptied settings file, the wrapper, the empty dashboard file, World
|
Checked On That leaves the one point I have raised twice: a member's empty profile or
|
Both earlier findings are resolved by the The PR description provides sufficient representative real-CLI verification; its older recorded commit is not blocking. No PR code was run, built, or installed. |
A shell profile whose only content was teamai's block became a file holding a newline; a tool settings file whose only content was teamai's hooks stayed as empty lists; the hook wrapper, an empty dashboard file and the directories that held them stayed too. Uninstall now removes each of those, and the ledger's known leftovers drop from 15 entries to 2 (the log and the directory holding it). Refs Tencent#1808
4575579 to
ea437ba
Compare
|
The P1 is fixed in the next push: the sweep now calls On the pre-existing empty file, I took your second option rather than recording creation at write time:
I did not add write-time tracking because there is no per-file record to put it in —
Latest world run and the suites are in the body; I will push once the local run finishes so the recorded commit matches the head. |
|
No new findings confidently supported by the scoped diff. All earlier findings are resolved: The PR description satisfies the testing requirement with representative before/after real-CLI world verification, including head No PR code was run, built, or installed. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Checked ea437ba0. Everything from my reviews is resolved:
- Blocking items. The sweep skips while shared resources are kept (
includeShared) and while a hook could not be removed (hooksLeft). It also skips while another install on this HOME is left: the user-scope config or another partition (installsSharingHome), and, as before, a registered project (keptForProjects). In the user-scope case the CI review raised, the user scope's hooks therefore keep their wrapper. - Pre-existing empty files. The CHANGELOG line and both FAQ pages now say that a file left empty by taking teamai's part out goes. That was the one point open. A write-time record is not needed for a file that held nothing.
- What stays. The tool directories,
~/.teamai/projects,~/.teamaianddebug.logstay for the reasons in the body.
The CI review at this head has no findings.
Approving. I'll merge once CI is green at this head.
…them A project uninstall kept Codex's ~/.codex/hooks.json, OpenCode's plugin and the Pi, OMP, Kiro and Antigravity hook files for "other teamai installs (user scope, HTTP agent or other projects)" without checking that one existed. On the last install those dispatchers stayed, fired on every session and found no config, with `teamai hooks remove` as the only way out. The decision is the one Tencent#1817 added for the wrapper: installsSharingHome() for the user scope and the other project partitions, plus the local-agent manifest for an HTTP agent. When any of them is set up, nothing changes. Refs Tencent#1825
…them A project uninstall kept Codex's ~/.codex/hooks.json, OpenCode's plugin and the Pi, OMP, Kiro and Antigravity hook files for "other teamai installs (user scope, HTTP agent or other projects)" without checking that one existed. On the last install those dispatchers stayed, fired on every session and found no config, with `teamai hooks remove` as the only way out. The decision is the one Tencent#1817 added for the wrapper: installsSharingHome() for the user scope and the other project partitions, plus the local-agent manifest for an HTTP agent. When any of them is set up, nothing changes. Refs Tencent#1825
…them A project uninstall kept Codex's ~/.codex/hooks.json, OpenCode's plugin and the Pi, OMP, Kiro and Antigravity hook files for "other teamai installs (user scope, HTTP agent or other projects)" without checking that one existed. On the last install those dispatchers stayed, fired on every session and found no config, with `teamai hooks remove` as the only way out. The decision is the one Tencent#1817 added for the wrapper: installsSharingHome() for the user scope and the other project partitions, plus the local-agent manifest for an HTTP agent. When any of them is set up, nothing changes. Refs Tencent#1825
Summary
teamai uninstallnow removes what it created and then emptied. The issue lists 15 entries theverifyledger knew about; this leaves 5, and the 10 it removes become a LEAK if they ever come back, becauseKNOWN_LEFTOVERSno longer tolerates them.What decides an empty directory here is not that it is empty, but whether anything resolves a path through it.
~/.claudeis a tool's own directory,~/.teamai/projectsis where every checkout's data home is looked up, and~/.teamaiholds the log — those stay.bin/,dashboard/andlocks/only ever hold what teamai writes into them by name, so they go.The decisions the issue asked for, entry by entry:
~/.bashrc,~/.profile,~/.zshenv(a 2 B newline)initand held nothing else goes too, which the CHANGELOG anddocs/guide/faq.mdnow say.~/.claude/settings.json(hookscleared to empty lists)~/.teamai/bin/teamai(and the Windows.cmdand CodeBuddy wrappers)Auto-generated by teamai.~/.teamai/bin/,dashboard/,locks/~/.teamai/projects/ENOENTwhen it was pruned.~/.teamai/dashboard/session-owners.jsonl<project>/.claude/skills/~/.claude/,<project>/.claude/~/.claudegone, Claude's MCP config stopped being written inmcp-uninstall— the command reported success and leftteam-mcpin~/.claude.json. teamai removes what it put inside them, not the directories the tool reads.~/.teamai/debug.log~/.teamai/projects/doSmall code, no new command: the emptied-hook-file deletion that Copilot already had now runs for every tool, a shell profile that ends up empty is removed instead of rewritten, and one sweep at the end of removal takes the wrapper, the empty dashboard file and the directories that held only them.
The sweep is skipped when this uninstall keeps shared resources (
--agent), when a hook it could not remove is still there, and while any other install on the machine still runs HOME's hooks:installsSharingHome()covers the user scope and every partition, andkeptForProjects()is read at sweep time rather than from the user-scope-only plan field.Refs #1808. Fixes #1808 for the 10 entries above; the other 5 are the two tool directories,
projects/, the log, and the root that holds them.Validation
npm run lint— passes, including the documentation checksnpm run typecheck— passesnpx vitest run src/__tests__/uninstall.test.ts— 256/256 passkeptForProjectstaken from the plan, which is filled for the user scope only); so does one while a user-scope install shares the HOME (it fails withoutinstallsSharingHome); and one with nothing else on the machine removes it. The first pair of cases here were vacuous before: with a user-scope config the data home is~/.teamaiitself, so the existingremoveAllButalready took the wrapper.hooks-project-isolation-issue373(absent file reads as no hooks),config-entry-ownership-993(asserts the file is gone),submodule-checkout-1041(absent reads as empty),copilot-global-hooks(same).codebuddy-mcp-local-915found~/.teamai/projectswas pruned recursively bypruneEmptyDirs; the sweep no longer prunes the root, so it stays.Real-CLI run, before and after
World from
origin/main(09f3ca7d), world20261110T095827-d7f1a6:Same run on this head (
ea437ba0, world20261110T115430-01ee20), withKNOWN_LEFTOVERScut to the five entries this change keeps:Nothing else is left, and the 10 entries are no longer tolerated: if one comes back, the ledger reports a LEAK.
Change Surface
.claudecomes from the plan'sprojectRoot.plan.projectRoot; a linked worktree's files are unchanged.teamai uninstallonly.teamai hooks removekeeps its current behavior.teamai.cmdand the CodeBuddy wrapper; this was not run on Windows.skill-data/:docs/guide/faq.mdalready says "Files you created stay", which still holds; CHANGELOG updated.Limitations
uninstall --dry-runlists the resources it will remove, not these emptied files and directories. They hold nothing of the member's, so the preview stays accurate about what matters, but it is not exhaustive.debug.logis left on purpose; see the table.gitUntracked: one a repository tracks stays, as the hook pass already keeps it.