Repository navigation
fix(locate): every command finds a tool where pull wrote it, also when its home moved (#1221) - #1486
Conversation
The PR description documents sufficient testing, including representative real-CLI runs at HEAD. I did not run, build, or install PR code. |
|
Fixed in ed17dd6. Regression test: |
ed17dd6 to
5e9f32c
Compare
The earlier instructions-only reproduction is resolved and covered by a regression test; the extras-edit scenario above remains. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. I did not run, build, or install PR code. |
|
Fixed in b9da8f5. When push rebuilds Regression test: |
|
No new findings in the scoped PR diff. Both earlier model-pin-loss findings are resolved: omitted pins are preserved for instructions-only edits and when another extra changes. Regression tests cover both scenarios. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. Additional provider/platform coverage is explicitly deferred to CI. I did not run, build, or install PR code. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Reviewed at b9da8f5. Both CI review findings on the Trae CN model pin are fixed. With the built CLI, an instructions-only edit and a disallowedTools edit to the project file both keep tool_extras.trae-cn.model after push. Each regression test fails on the commit before its fix. skill list now finds a moved Hermes home, which main misses. doctor output, severities and exit code are the same from the project root and a subdirectory.
Blocking
- [P1 blocking] After a home move this PR introduces,
uninstallcleans only the new home.- Hermes. Main installs into
~/.hermeswhile~/.hermes/active_profileiswork. This build'spullmoves everything toprofiles/work, thenuninstall --forcecleansprofiles/work. What stays in~/.hermes:hooks/teamai-status-report.sh;plugins/teamai-instructions/*;- the
config.yamlentries (hooks.on_session_start,plugins.enabled: [teamai-instructions]) and the allowlist approval; - the SOUL.md team-rules block.
- Oh My Pi. Main installs
~/.omp/agent/extensions/teamai-hooks.ts. AfterOMP_PROFILE=work teamai pull,uninstall --forceremoves only the profile's extension.~/.omp/agent, which the comment insrc/tools.tscalls the live default profile, keeps loading it. - Main's
uninstallremoves all of these. pull's "no longer updates" notes name the skills, rules and blocks left behind, but not the hook, extension or plugin.doctorstays green.- The same happens without an upgrade, for a member who switches profile and later uninstalls.
- Suggested fix: have
uninstallalso clean the earlier homes' hook and adapter files that teamai recognises by marker, asearlierHookAdapterDiralready does for OMP's pre-#1221 extension inreconcileOmpExtension, and name those files in pull's notes.
- Hermes. Main installs into
- [P1 blocking] With
HERMES_HOMErecorded at init,uninstallfrom a shell without it leaves the Hermes hook and plugin.- Scenario:
HERMES_HOME=<dir> teamai init … --agent hermes --scope user, thenpullanduninstall --forcewithout the variable. - Skills are removed, but
<dir>/hooks/teamai-status-report.sh,plugins/teamai-instructions/*, theconfig.yamlentries and the approval stay. - Cause:
executeRemovaldeletes~/.teamai, andtool-homes.jsonwith it (src/uninstall.ts:2426), beforeremoveHermesHooks()runs (:2471). That call then resolvestoolHome('hermes')to~/.hermes. - The PR already moved the SOUL.md path into the plan (
hermesSoul) to survive this ordering. The hook, plugin and config paths need the same treatment, or the Hermes step needs to run before~/.teamaiis removed. - Main has this too, but the PR promises that
uninstallfinds a tool where pull wrote it. Copilot and OpenClaw work in the same scenario.
- Scenario:
Non-blocking
- [P3 nit] The homes table lists Oh My Pi "Before:
PI_CODING_AGENT_DIR". At the merge base OMP's home did not move (omp: { home: { dir: '.omp' } }), so a member withPI_CODING_AGENT_DIRset sees no change. Correcting the Before cell keeps a changelog from announcing a removal.
Checks: tsc and lint exit 0. The changed unit files plus the README-matrix, tool-support and command-reference tests pass (36 files, 5118 tests), also with a symlinked TMPDIR. 19 of the 21 new tests fail on the base. The PR's e2e file passes 8/8.
Not checked: real Hermes, Oh My Pi, OpenClaw and Trae binaries; Windows beyond CI; the gitlab/github providers, left to CI. The OpenClaw variants' hook isolation and uninstall --agent <tool without settings file> are covered by unit tests only.
This now conflicts with #1474 and #1477 in src/__tests__/doctor.test.ts, and with #1414 in src/resources/agents.ts and src/uninstall.ts.
…the server sends no workspace (Tencent#1221)
…rom locate or toolHome (Tencent#1221)
…em in the Windows job (Tencent#1221)
…cent#1221) A team repo's workspace keeps Copilot's team hooks in the project's .github/hooks/teamai.json and the built-ins in the user file. TOOLS now declares that file as hooks.workspace, locate answers it beside the user file, and pull, hooks list and remove, uninstall and doctor read both from locate instead of joining the project root or Copilot's home by hand. The same files are written, read and removed.
Both editions load Markdown subagents from a project's .trae/agents, and Trae CN also from ~/.trae-cn/agents (Trae 3.5.67, Trae CN 3.3.67). Where international Trae reads user agents is not established: its docs give Trae CN's path, so user scope stays unknown and teamai writes none there. Trae takes tools as one comma-separated string and a model only from its own ids, which differ per edition while both read the same project directory. The render leaves out the agent's own model and writes one an alias or tool_extras gives the edition. Loading also needs Trae's Beta "Enable Subagents Directory" switch, which teamai cannot read; the descriptor and the sharing guide say so.
…1221) The agent spec's tool_extras keys come from the tools teamai renders agents for, and the Codex-family lists in the auto-compact and co-author reconcilers use CODEX_TOOL_IDS instead of restating it. The capability allow-lists outside TOOLS (shell-dependent hooks, SubagentStop, Stop stdout, project MCP judging, preapproving skills, search evidence, bundled shells, agent type aliases, main-checkout team hooks, the single-repo tool choices) are checked against ToolId, so a renamed or misspelled tool fails tsc instead of silently dropping out.
…encent#1221) Each resource matrix now checks that every home move TOOLS declares (an env variable, XDG_CONFIG_HOME, %LOCALAPPDATA% or a profile suffix) which moves a write of that resource has a row setting it, so a new move or a new channel under a moving home fails until its cell is written. The cells it found missing are added from each tool's source: OpenClaw's state dir and profile for its rules, instruction blocks, project skills and hooks, CodeBuddy's CODEBUDDY_CONFIG_DIR for user skills, rules and blocks and project sessions, Hermes' HERMES_HOME and %LOCALAPPDATA% for its SOUL.md and project hooks, and the workspace's user hooks under Pi, OpenCode and Muse home moves. OpenClaw's OPENCLAW_WORKSPACE_DIR and OPENCLAW_CONFIG_PATH, which TOOLS does not record as home moves, get rules, blocks and hooks cells by hand.
…1221) Removing teamai's hooks keeps the events it empties, so uninstall left $COPILOT_HOME/hooks/teamai.json holding only empty arrays. Copilot runs nothing from that file, and the file is teamai's: uninstall now deletes it once nothing else is left in it and git does not track it. A file still holding a member's hook stays.
…wn rules (Tencent#1221) OPENCLAW_HOME replaces the OS home for OpenClaw's state dir and composes with OPENCLAW_PROFILE. Oh My Pi's agent dir follows OMP_PROFILE, PI_PROFILE, PI_CONFIG_DIR and, for the default profile, PI_CODING_AGENT_DIR, which its task agents do not follow. Hermes's home takes HERMES_DATA_DIR_SUFFIX and the profile active_profile names.
…er (Tencent#1221) injectHooksToAllTools sent qclaw, easyclaw and autoclaw to the OpenClaw adapter, which writes into OpenClaw's own workspace, so the last variant in toolPaths left OpenClaw dispatching under its name. Inject now routes only openclaw there, as reconcile already does.
…ncent#1221) A source destination outside the scope root had to lie in a hand list of homes (Hermes, OpenClaw, Copilot), so a skill pulled into a home that XDG_CONFIG_HOME, PI_CODING_AGENT_DIR or CODEBUDDY_CONFIG_DIR moves outside HOME made the next pull reject the installation record. The list is now every tool's home and OpenClaw's workspace.
… in (Tencent#1221) doctor judged each read from its own working directory, so its notes changed with where it ran. A project install's reads are now judged from a session at the project root, and a user install's from any session, so a read limited to sessions under one directory is always named.
…me (Tencent#1221) Trae loads a subagent only when its name starts with a letter, holds only letters, digits and -, ends with a letter or digit, and is at most 50 characters. Pull wrote such an agent anyway and Trae dropped it in silence; it now skips it for trae and trae-cn and says why, while other tools still receive it.
…r profile pulls (Tencent#1221) OMP profiles run side by side, so ~/.omp/agent is the live default profile, not a leftover. Pull, hooks remove and uninstall no longer delete its extension or name its MCP servers as left behind.
…encent#1221) Every tool without a settings file looked in OpenClaw's workspace hooks, so uninstalling Hermes or Trae deleted OpenClaw's handler and left its openclaw.json entry enabled. Only OpenClaw and its variants look for the HOOK.md dir now, and only OpenClaw in its workspace.
…ent#1221) Both editions read a project's .trae/agents, so a tool_extras model for one edition landed in the other's file and the last writer won. A project copy now never carries a model; Trae CN's user copy in ~/.trae-cn/agents still takes its edition's model. Pull, push and copy proofs render with the scope they compare in.
…encent#1221) A pull on Windows records %LOCALAPPDATA%\hermes, and init records HERMES_HOME. The record then won over active_profile, so after `hermes profile use work` teamai kept writing to the default profile. The record now holds Hermes's home only, and the profile active_profile names is applied on top of it on every run. Init also records HERMES_HOME again while a profile is active.
…#1221) Trae gives every tool to an agent with no tools field, and none to one with tools: ''. An empty tools list was dropped from the render, so it granted every tool.
…encent#1221) Each edition rendered its own tool_extras into the shared .trae/agents file, so one edition's restriction depended on which pull wrote last and the other edition's delivery check failed. The project file now takes both editions' tool_extras but model. An agent whose editions give one key different values is skipped for Trae there, with a warning naming the key.
… file (Tencent#1221) A project's Trae copy renders the other edition's tool_extras, so push treats those keys as inherited, as it does for tclaude and tcodex: removing one is refused, and an edit that would give the two editions different values is refused too. A copy pull skips for a tool is no longer a delivery doctor checks.
… keeps its old copies' rules (Tencent#1221) A tool that would not load an agent as teamai renders it (a name Trae ignores, or a Trae project key the two editions give different values) is left out of pull's and doctor's delivery set, and doctor now names each such agent, tool and reason instead of reporting it unreachable or saying nothing. Namespace cleanup still sees the agent's render, so a skipped replacement keeps the root copy it replaces and an inactive copy is removed as before.
…ut (Tencent#1221) A Trae project copy carries no edition model, so push compared the pin in tool_extras.<edition>.model against nothing and dropped it from the team file on any other edit. A pin the copy does not carry is no longer compared.
…as changes (Tencent#1221) Rebuilding tool_extras.<edition> from a Trae project copy dropped its model, which the copy never carries. A model pin the copy does not carry is now kept.
…recorded (Tencent#1221) Removing ~/.teamai took the tool-homes record with it before the Hermes step ran, so that step looked in ~/.hermes and left the hook script, the plugin, and their config.yaml entries and approval in the recorded home. The plan now names the home, and the Hermes removers take it.
…home and profile (Tencent#1221) Pull writes only the active Hermes or Oh My Pi profile, so a pull before a home move or a profile switch leaves teamai's hook script, plugin, config.yaml entries and SOUL.md block, or its Oh My Pi extension, in another home. Uninstall cleaned only the active one. It now looks in the default, recorded and moved homes and every profile under them, removes what carries teamai's marker or exact command, and lists each in its summary. Pull and hooks remove still touch only the active profile.
b5b3831 to
fb14f46
Compare
|
Both blocking findings are fixed. The branch is rebased on e0d17c4.
Each finding has a test through
None of the three leaves a teamai file in any home, and Not changed: pull's moved-home notes still name only skills, rules and blocks. Naming the hook, extension or plugin there needs a new record field that remembers each tool's hook files per pull, and it still could not name what a release before this PR wrote. Uninstall now covers both cases. The P3 is fixed in the body: Oh My Pi's "Before" is |
Both earlier model-pin-loss findings are resolved, with regression tests covering instructions-only and extras edits. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. Additional provider/platform coverage is explicitly deferred to CI. I did not run, build, or install PR code. |
…rded home (Tencent#1221) A home recorded under PI_CONFIG_DIR left that root's default profile and other profiles unscanned when uninstall ran from a shell without PI_CONFIG_DIR, so their extension stayed.
|
Fixed in 30914dc. Regression test: |
All three earlier findings are resolved: both Trae model-pin-loss cases and OMP’s recorded-root cleanup have fixes and regression tests. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. Additional provider/platform coverage is explicitly deferred to CI. I did not run, build, or install PR code. |
… profile home (Tencent#1221) A HERMES_HOME recorded as <root>/profiles/<name> left <root> and its other profiles unscanned when uninstall ran from a shell without HERMES_HOME, so the hook and plugin there stayed.
|
Fixed in 53b051c. When a recorded or moved Hermes home is Regression test: |
All four earlier findings are resolved, with regression tests. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. Additional provider/platform coverage is explicitly deferred to CI. I did not run, build, or install PR code. |
…encent#1221) Trae and Trae CN share a project's .trae/agents, so uninstall --agent for one deleted the agent files the other still reads. They stay now, as shared skill directories do.
|
Fixed in db61733. Regression tests in
|
|
No new findings in the scoped PR diff. All five earlier findings are resolved, with regression tests: both Trae model-pin-loss cases, OMP and Hermes recorded-root cleanup, and shared Trae agents during targeted uninstall. The PR description documents sufficient testing, including representative real-CLI runs at HEAD. Additional provider/platform coverage is explicitly deferred to CI. I did not run, build, or install PR code. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Approving at db61733. Everything from our review and from each CI review since is fixed. Each case was reproduced on the commit before its fix and checked with the built CLI after it:
- After a Hermes
active_profileswitch or a pull underOMP_PROFILE,uninstallcleans teamai's hook, plugin,config.yamlentries, approval and SOUL.md block (Hermes), or its extension (Oh My Pi), in every home and profile teamai wrote. - With
HERMES_HOMErecorded at init,uninstallfrom a shell without it cleans the Hermes hook and plugin, and so does a home re-recorded as<root>/profiles/work. - An Oh My Pi home moved with
PI_CONFIG_DIR, then re-init underOMP_PROFILE, is cleaned in both profiles. uninstall --agent traekeeps.trae/agents/reviewer.mdwhile Trae CN still reads it. A lateruninstall --agent trae-cnremoves it.- Member-owned hooks, extensions, plugins and SOUL.md content in other profiles survive unchanged, and the plan lists every file before removal.
- The Trae CN model pin survives
pushafter an instructions-only edit and after an edit to another extra.
tsc and lint exit 0. The changed unit files pass (uninstall.test.ts 230/230, also with a symlinked TMPDIR). The PR's e2e file passes.
Non-blocking
- [P3 nit] When
uninstall --agent traekeeps shared agent files, the success line (src/uninstall.ts:2674) says only "its shared skills and rules stay". It could name agents too. - [P3 nit] The teamai skill copy in other Hermes and Oh My Pi profiles survives
uninstall. The Hermes hook script is matched by file name rather than by its marker.
Not checked: real Hermes, Oh My Pi, OpenClaw and Trae binaries, and Windows beyond CI. The gitlab and github providers are left to CI.
Summary
When a member moves a tool's home with an environment variable,
pullwrote to the moved home, butskill list,doctor's install checks,uninstall, hook writers and the local agent still looked in the default one, each through its own resolver. Now every caller asks one resolver,locate(tool, resource, install, host), and the per-tool resolvers are deleted. What was still hand-coded per tool is modeled inTOOLS.Homes now follow each tool's own rules, read from upstream source or docs:
OPENCLAW_STATE_DIR, profile suffixOPENCLAW_STATE_DIRwins, elseOPENCLAW_HOMEcomposed withOPENCLAW_PROFILE~/.omp/agent, whatever the envPI_CONFIG_DIR,OMP_PROFILE(orPI_PROFILE).PI_CODING_AGENT_DIRstays Pi's: teamai does not follow it for Oh My Pi.models.ymlfollows the same homeHERMES_HOMEHERMES_HOME,HERMES_DATA_DIR_SUFFIXor WindowsLOCALAPPDATAname the hometool-homes.jsonrecords. The profileactive_profilenames is applied inside that home on every run and never recorded, sohermes profile usetakes effect at the next pull, from any shellFixes found on the way:
Invalid source ownership recorduninstallhooks/teamai.jsonuninstall --agent <tool without settings file>uninstallfor Hermes and Oh My Piconfig.yamlentries and SOUL.md block (Hermes) or the extension (Oh My Pi) stayed in the other; withHERMES_HOMErecorded at init, uninstall from a shell without it looked in~/.hermes~/.teamaiis removed.pullandhooks removestill touch only the active profiletoolsas one string (tools: ''for none, since Trae gives every tool to an agent without the field). A name Trae ignores is skipped with a warning.trae/agentsfile, so it gets one render: no edition model, both editions' othertool_extras. A key the two give different values skips the agent for Trae, with a warning;pushrefuses an edit that changes or removes a key the other edition gives, and keeps an edition model the file leaves out, whatever else changes. User scope keepstool_extras.trae-cn.modeldoctoragentsdoctorread notesOMP_PROFILE=workleaves the default profile's extension in place, since profiles run side by sideType of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses: at db61733 (on e0d17c4), 18529 passed, 22 skipped, 2 failed:workflow-local-driverandworkflow-settle-costare timing tests that miss under full-suite load; both files pass alone (156/156), and this PR does not touch workflows. The run before (53b051c) had 0 failures.locate,nextToolHomes/toolHome,reconcileHooksToAllTools,uninstall,AgentsHandler.pullItem/scanLocalForPushand doctor's checks from several directories. The locate matrix gate fails when a home move has no matrix row.Real CLI (
node dist/index.js, throwawayHOME, local bare git team repo,--provider git), at HEAD:Hermes,
HERMES_HOMErecorded while theworkprofile is active, then a switch back todefault:Trae and Trae CN in one project:
Earlier real-CLI runs on this branch, same setup:
XDG_CONFIG_HOME:pullwrites$XDG_CONFIG_HOME/muse/skills,teamai list skills --source locallists that dir with 2 skills,doctorexits 0 with all Muse checks ✔.COPILOT_HOME:pullanduninstall --forcework only in$COPILOT_HOME; uninstall printsRemoved <copilot-home>/hooks/teamai.json: no hooks left in it.pullwrites~/.trae-cn/agents/reviewer.md(user) and<proj>/.trae/agents/reviewer.md(project) withtools: 'Read, Grep, Glob'.OPENCLAW_HOME=<oc-home>: init records<oc-home>/.openclaw, the hook and skills land in<oc-home>/.openclaw/workspace.PI_CODING_AGENT_DIRwith Pi and Oh My Pi: Pi's extension in that dir, OMP's in~/.omp/agent, both unchanged across three pulls including--force.OMP_PROFILE=work teamai models switch local:tokenhub --agent ompwrites~/.omp/profiles/work/agent/models.yml.Synced 1 skills (all updated), previouslyInvalid source ownership record.doctorfrom the root and fromsrc/: identical output.uninstall --forceafter (a)HERMES_HOMErecorded at init, run without it, (b) Hermes pulled underdefaultthenwork, (c) Oh My Pi pulled without a profile then withOMP_PROFILE=work: no teamai file is left in any home;config.yamland the allowlist keep no teamai entry.Not run with a real binary of the tool: OpenClaw, Oh My Pi, Hermes, Trae loading what teamai wrote. Only
gitprovider and user/project scope on macOS; other providers and Windows are left to CI.Related Issues
Continues #1221 (item 2 of "Done when").
Notes for Reviewers
For an unmoved home, no path teamai reads or writes changes; the only differences are the fixes listed above.
Not moved or not done here:
claude-hook-host.tsCOPILOT_HOMEin a generated shell snippet and calls none of the deleted resolvers~inOPENCLAW_CONFIG_PATH/OPENCLAW_WORKSPACE_DIROPENCLAW_HOME(config) or not at all (workspace); unchanged heresatisfies ToolId[]catches typos only; 15 of 32 tables stay untyped. Exhaustiveness needs each table moved intoTOOLSactive_profileon Windowsdoctornames the skipRolling back is a revert:
tool-homes.jsonkeeps the same shape (a directory per tool), so an older release reads what this one records.