feat(skill): serve builtin skill content from the CLI, deploy a discovery stub - #699
Conversation
|
|
Addressed in dc45189 (all four points), PR description updated. P1 — P1 — recall gate bypass. One
Tests in P2 — variadic marker. P1 — provider matrix. Generic The Runs on dc45189: |
|
Findings
|
|
Follow-up in 41a71d9.
Runs: |
|
Findings
|
… from Codex Review follow-up on Tencent#699. - `skill show <served skill>` refuses a recall-blocked skill with the same message and exit code as `skill get` / `skill path`; it printed the directory those two withhold. - A skill resolved from skill-data/ is classified `[builtin]` directly. BUILTIN_SKILL_NAMES only knows the deployed stub, so `skill show core` reported `[local-only]` beside a package path. - pruneLegacyBuiltinSkills reaches `.agents/skills` only on Codex's own pass. Another enabled tool's pass deleted Codex's legacy copies while Codex was excluded, against the enabledAgents guarantee. - The share skill and its references are written in English; the generated document still follows the session's language. The AGENTS.md exception for Chinese skill-data output is dropped.
|
Follow-up in 174ab6a. P1 — P1 — P1 — Chinese in P2 — P1 — GitLab row. Unchanged from the previous round: this machine has no Built CLI, isolated HOME, Runs on 174ab6a: |
|
… push, wiki in English Review follow-up on Tencent#699. - resolveServableSkill is the only way to obtain a PackagedSkill outside skill-content.ts; it returns `blocked` instead of the skill, so `get`, `path`, `list` and `show` inherit the recall gate by construction. - push never offers `team-wiki-codebase` / `teamai-share-learnings` as new user skills: between the upgrade and the first pull they are still on disk (isCliOwnedSkillName). - `recall disable` removes the legacy `teamai-share-learnings` directory again (LEGACY_RECALL_SKILL_NAMES), skipping excluded agents. - `skill list` prints the packaged catalog before `teamai init`, with a hint for the team half, instead of failing on the team listing. - skill-data/wiki (SKILL.md, 14 references, 2 scripts) translated to English. Generated document names follow one glossary; validate_kb.py still recognises headings of knowledge bases built by the previous release, matched by code point so the source stays ASCII.
|
Follow-up in 13716ef. P1 — legacy trees pushed back into the team repo. P1 — P1 — Chinese in the wiki skill. P2 — Also in this commit. The recall gate now lives in one place: P1 — GitLab row. Still not run. This machine has no valid GitLab credential ( Built CLI, isolated HOME, Runs on 13716ef: |
|
…by name Review follow-up on Tencent#699. - PACKAGED_SKILL_FILES lists every file a release ever wrote under skills/, as the union of `git ls-tree -r <tag> -- skills/` over all 91 tags. The prune removes those paths and the directories they leave empty; a file a member added is kept, its directory with it, and pull says which and why. The stub directory loses its six known references by name instead of "everything that is not SKILL.md". Python bytecode of a script we shipped counts as ours, so a __pycache__ does not strand the tree. - locateSkill searches the team repo, then installed agents, then the package. A directory a member created under `codebase`, `default`, `learning` or `share` is the skill they asked about, and the recall gate does not apply to it. - A guard test fails when a file ships under skills/ without being recorded in the manifest, which a later migration would otherwise leave behind.
|
Follow-up in da6df38. P1 — deletion without proving ownership. Both prune paths now remove only files the CLI wrote. A guard test walks the packaged Worth flagging separately: P2 — aliases shadowed local skills. P1 — GitLab row. Still not run, and I want to be plain about why rather than restate it a fourth time: this machine has no GitLab credential of any kind. Stale e2e count. Fixed in the description: the skill-serving e2e was 4 tests when that line was written and is 6 now. Both later rounds added a case and the count was not carried over. Built CLI, isolated HOME, a machine that upgraded with the pre-stub trees and the member's own files mixed in: Runs on da6df38: |
|
Review follow-up on Tencent#699. - `skill path` takes a name, always. The argument-less form printed the `skill-data/` root, and `<root>/share/SKILL.md` is readable from there — the content the gate withholds one command over. - uninstall discovers skills in Codex's shared `.agents/skills` root, where resolveSkillDestination puts the stub whenever the skill already lives there. Without it, uninstall reported success and left it behind. Codex only, as the legacy prune already does. - core/SKILL.md said team sharing is enabled by default; getRecallSharing defaults it to false. It now says recall is off by default and names `teamai recall enable`.
|
Follow-up in 03f7611. All four addressed, and the GitLab row is finally run. P1 — P1 — Codex's shared stub survived uninstall. P2 — the served instructions stated the wrong default. P1 — the GitLab row. Run, against a real private gitlab.com project created for the run and deleted after it. The same matrix was re-run on the generic What getting the GitLab row to run turned up, which is not this PR's to fix. Runs on 03f7611: |
|
Findings
|
…LL_DIR} Review follow-up on Tencent#699. - uninstall removed a CLI-owned skill directory whole, undoing one command over the guarantee pull makes. It now removes the PACKAGED_SKILL_FILES paths through the same removeOwnedFiles, keeps a directory holding a file the member added, says which one, and tells the confirmation prompt so it no longer promises a directory it will keep. A team-repo skill is synced whole and still goes whole. - Served shell commands quote the placeholder: `python3 "{SKILL_DIR}/..."`. Unquoted, an install path with a space ("Program Files", "Application Support", a Windows path through Bash) splits into two arguments and the documented invocation fails. A test fails on an unquoted occurrence after any command word, in SKILL.md or any reference. - wiki/references/overview.md said the methodology, scripts and agent specs are deployed into agent directories. They are not: only the stub is, and the rest is served from the installed CLI.
|
Follow-up in 9c21cc8. P1 — uninstall deleted a CLI-owned directory whole. Correct, and it undid one command over the guarantee
The confirmation prompt was promising something it no longer does, so it says so per line now: Test in P1 — unquoted Verified by running the two scripts as they ship from a path with a space, in both spellings: A test in P2 — P1 — the GitLab row. It is run. It was run before this round, on On the glab detour, since it is easy to read as a gap in this row. What I cannot verify from here is the
Widening this PR into GitLab auth to close that would repeat the mistake the prune set was blocked for, so I have left it out deliberately. Runs on 9c21cc8: |
The PR description includes a detailed test plan and real-CLI E2E records across the required agents and |
… from Codex Review follow-up on Tencent#699. - `skill show <served skill>` refuses a recall-blocked skill with the same message and exit code as `skill get` / `skill path`; it printed the directory those two withhold. - A skill resolved from skill-data/ is classified `[builtin]` directly. BUILTIN_SKILL_NAMES only knows the deployed stub, so `skill show core` reported `[local-only]` beside a package path. - pruneLegacyBuiltinSkills reaches `.agents/skills` only on Codex's own pass. Another enabled tool's pass deleted Codex's legacy copies while Codex was excluded, against the enabledAgents guarantee. - The share skill and its references are written in English; the generated document still follows the session's language. The AGENTS.md exception for Chinese skill-data output is dropped.
… push, wiki in English Review follow-up on Tencent#699. - resolveServableSkill is the only way to obtain a PackagedSkill outside skill-content.ts; it returns `blocked` instead of the skill, so `get`, `path`, `list` and `show` inherit the recall gate by construction. - push never offers `team-wiki-codebase` / `teamai-share-learnings` as new user skills: between the upgrade and the first pull they are still on disk (isCliOwnedSkillName). - `recall disable` removes the legacy `teamai-share-learnings` directory again (LEGACY_RECALL_SKILL_NAMES), skipping excluded agents. - `skill list` prints the packaged catalog before `teamai init`, with a hint for the team half, instead of failing on the team listing. - skill-data/wiki (SKILL.md, 14 references, 2 scripts) translated to English. Generated document names follow one glossary; validate_kb.py still recognises headings of knowledge bases built by the previous release, matched by code point so the source stays ASCII.
…by name Review follow-up on Tencent#699. - PACKAGED_SKILL_FILES lists every file a release ever wrote under skills/, as the union of `git ls-tree -r <tag> -- skills/` over all 91 tags. The prune removes those paths and the directories they leave empty; a file a member added is kept, its directory with it, and pull says which and why. The stub directory loses its six known references by name instead of "everything that is not SKILL.md". Python bytecode of a script we shipped counts as ours, so a __pycache__ does not strand the tree. - locateSkill searches the team repo, then installed agents, then the package. A directory a member created under `codebase`, `default`, `learning` or `share` is the skill they asked about, and the recall gate does not apply to it. - A guard test fails when a file ships under skills/ without being recorded in the manifest, which a later migration would otherwise leave behind.
9c21cc8 to
292a732
Compare
Review follow-up on Tencent#699. - `skill path` takes a name, always. The argument-less form printed the `skill-data/` root, and `<root>/share/SKILL.md` is readable from there — the content the gate withholds one command over. - uninstall discovers skills in Codex's shared `.agents/skills` root, where resolveSkillDestination puts the stub whenever the skill already lives there. Without it, uninstall reported success and left it behind. Codex only, as the legacy prune already does. - core/SKILL.md said team sharing is enabled by default; getRecallSharing defaults it to false. It now says recall is off by default and names `teamai recall enable`.
…LL_DIR} Review follow-up on Tencent#699. - uninstall removed a CLI-owned skill directory whole, undoing one command over the guarantee pull makes. It now removes the PACKAGED_SKILL_FILES paths through the same removeOwnedFiles, keeps a directory holding a file the member added, says which one, and tells the confirmation prompt so it no longer promises a directory it will keep. A team-repo skill is synced whole and still goes whole. - Served shell commands quote the placeholder: `python3 "{SKILL_DIR}/..."`. Unquoted, an install path with a space ("Program Files", "Application Support", a Windows path through Bash) splits into two arguments and the documented invocation fails. A test fails on an unquoted occurrence after any command word, in SKILL.md or any reference. - wiki/references/overview.md said the methodology, scripts and agent specs are deployed into agent directories. They are not: only the stub is, and the rest is served from the installed CLI.
…ist alias Review follow-up on Tencent#699. - Reporting-only HTTP pull pruned the legacy trees and deployed nothing, so a member on an HTTP team came out of the upgrade with no built-in entry point at all. The skip predates CLI-served content: it existed because the only deployable unit then needed a team repo. The stub does not — its workflows are printed by the installed binary, and `skill get wiki` is a local knowledge-base generator that never touches a repo. The stub now deploys in every mode, and `reportingOnly` goes with the branch it gated: nothing else read it. - `teamai skill list` called itself an alias for `teamai list skills --source all`. It has not been one since it started printing the CLI-served catalog underneath. Both descriptions, the generated command reference and both usage guides now say what it does. Refs Tencent#678
|
Follow-up in P1 — reporting-only pull pruned and deployed nothing. Correct, and the real before-state is worse than the finding describes. Reproduced with the built CLI on the parent revision ( So the member did not merely lose the legacy entry point: the The skip predates CLI-served content. It existed because the only deployable unit then — Verified against the fixed build, same HOME, no team repo on disk: $ HOME=… teamai pull
✔ [user] Team repo: HTTP (report/sync delivery)
$ diff ~/.claude/skills/teamai/SKILL.md skills/teamai/SKILL.md && echo IDENTICAL
IDENTICAL
$ HOME=… teamai skill get wiki | head -2 # served with no repo present
---
name: wikiThe unit test in P2 — Rebase. Runs on
|
…LL_DIR} Review follow-up on Tencent#699. - uninstall removed a CLI-owned skill directory whole, undoing one command over the guarantee pull makes. It now removes the PACKAGED_SKILL_FILES paths through the same removeOwnedFiles, keeps a directory holding a file the member added, says which one, and tells the confirmation prompt so it no longer promises a directory it will keep. A team-repo skill is synced whole and still goes whole. - Served shell commands quote the placeholder: `python3 "{SKILL_DIR}/..."`. Unquoted, an install path with a space ("Program Files", "Application Support", a Windows path through Bash) splits into two arguments and the documented invocation fails. A test fails on an unquoted occurrence after any command word, in SKILL.md or any reference. - wiki/references/overview.md said the methodology, scripts and agent specs are deployed into agent directories. They are not: only the stub is, and the rest is served from the installed CLI.
…ist alias Review follow-up on Tencent#699. - Reporting-only HTTP pull pruned the legacy trees and deployed nothing, so a member on an HTTP team came out of the upgrade with no built-in entry point at all. The skip predates CLI-served content: it existed because the only deployable unit then needed a team repo. The stub does not — its workflows are printed by the installed binary, and `skill get wiki` is a local knowledge-base generator that never touches a repo. The stub now deploys in every mode, and `reportingOnly` goes with the branch it gated: nothing else read it. - `teamai skill list` called itself an alias for `teamai list skills --source all`. It has not been one since it started printing the CLI-served catalog underneath. Both descriptions, the generated command reference and both usage guides now say what it does. Refs Tencent#678
Tencent#724 landed `skills/teamai/references/provider-tgit.md` and repointed setup-admin.md and join-member.md at it. Rebasing onto that left the new file in a tree this branch no longer deploys, and the pointers in bare `provider-tgit.md` form the served skills do not use. - move it to `skill-data/setup/references/`, beside the two files that cite it, so `teamai skill get setup --full` serves it - rewrite every pointer to it as `{SKILL_DIR}/references/provider-tgit.md` - list it in the setup skill's reference table - add `references/provider-tgit.md` to PACKAGED_SKILL_FILES, so the prune removes it from members who pulled a release that shipped it
…afe casts
Review findings from the standards axis, plus the doc half of the prune
count.
- `SkillCatalogEntry` allowed `{blockedByRecall: true, path: '/…'}`, an
invariant `skillCatalog` then upheld by hand. Split it on
`blockedByRecall`, so the withheld directory is a type error rather than
a review catch. Both variants keep the `path` key, so the
`skill list --json` shape is unchanged.
- `command.commands as Command[]` stripped commander's `readonly` in three
places. `for…of` and `.find` need no cast.
- `docs/designs/skill-serving.md` still said the prune removes six
`teamai/references/*.md`; provider-tgit.md makes it seven.
Publishing a skill is `teamai push --skill`, which never consulted recall (`src/push.ts` names it nowhere). On main the flow shipped in the teamai skill, ungated. Moving `contribute-member.md` under `share` put it behind the recall gate, so with recall off — a new team's default — the core routing table sent the agent to `teamai skill get share`, which exits 1 and tells it to enable recall. Wrong advice for a flow recall does not touch, and no other path to the instructions. Move the file to `core`, the skill that already owns `push`, and split the routing row so publishing and session learnings stop sharing one destination. The gate itself is right and stays: learnings do need recall. `share/SKILL.md` already called this "a different flow"; now it points at `teamai skill get core --full` instead of at its own references. PACKAGED_SKILL_FILES is unchanged: the legacy path a pre-stub release wrote is still `teamai/references/contribute-member.md`.
…door Review finding: `removeOwnedFiles` proves ownership by pathname and deletes without reading the file, so a member's edit goes with it. For a path the current package still ships that changes nothing: the old deployment overwrote it with `overwrite: true` on the same three triggers, so the edit died either way, at the same moment. The case the objection gets right is a path a retired release shipped and the package no longer does — the overwrite never reached it, so the edit did survive, and the prune is the first thing to remove it. Copy every pruned file to `~/.teamai/removed-skills/<date>/<tool>/<skill>/` before removing it. Outside every agent directory, so nothing reads it back as a skill. Verifying contents against a hash of each released version was the other way out, and it is worse: anything not byte-identical is then kept, so one CRLF checkout on Windows — a platform this project supports — leaves the whole 176 KB in place and reports success. Backing up gives the same guarantee without betting the migration on byte equality. Uninstall keeps deleting outright: there the member asked for the files to go.
… its own Two holes in the backup the previous commit added, both reported in review. The copy's failure was swallowed at debug level and the delete went ahead regardless, so a full disk or a read-only home turned the migration back into the data loss the backup exists to prevent — and the log still named a backup directory that held nothing. A file whose copy fails is now kept, counted, and named at warn level; `removeOwnedFiles` returns what happened instead of a bare boolean, and only a run that copied something names the directory. The backup path was `<date>/<tool>/<skill>` with `overwrite: true`, so the second copy of a name silently replaced the first. Codex prunes the same skill from `.codex/skills` and the shared `.agents/skills`, and two pulls share a date. The path now carries a per-run id and the skill root, and the copy refuses to overwrite rather than clobbering a copy it cannot replace. Tests cover both: a file where the backup tree must start makes every copy fail, and the two Codex roots land in separate directories. Each fails against the previous commit.
Three review findings, all in the prune. A symlinked skill directory was walked through. `readdir` follows the link, every path under it matches a packaged name, and the delete lands in someone else's checkout. Ownership now stops at the link: the root is lstat'd, a symlink is refused, and link and target are left alone. The stub directory was pruned against the full historical file list, which includes the SKILL.md written one line later. Deployment runs on every session start, unchanged revision included, so that archived an identical copy per session forever. Only paths this release no longer ships are archived now. Backups were written under the tool's base directory, which under project scope is the repo root, so they landed in the working tree outside the generated .teamai/.gitignore. They go to the machine's home. Also: `skill show <packaged>` resolved the team before the package, so it failed on a machine that never ran `teamai init` for content that needs no team. Packaged names resolve first and print without the team-dependent fields.
…prune Findings from a self-review run before pushing, plus the two from the last review round. The symlink guard was one level too low. It lstat'd the skill directory, so the common shape — `~/.claude/skills` itself linked at a dotfiles checkout — walked straight through: every directory under the link is real. The guard now walks each component below the tool's base directory and stops at the first link, which covers the prune and the stub write with one check. Components at or above the base are not checked: a home directory under a link is ordinary, and refusing there would disable deployment on those machines. The symlink branch borrowed the foreign-files message, so a member was told "delete the rest yourself" about a directory nothing had touched. Following that destroys what the guard just protected. It has its own sentence now, in pull and in uninstall. `remove()` was not fail-closed the way the backup is: a read-only parent left the tree half-pruned under a debug line, and a `walkFiles` that threw returned success. Both are recorded in `notRemoved` and reported. The backup path gained the base directory: `inheritUserScope` deploys the user base and then the project base in one process, same tool, same root, same skill name, and `errorOnExist` turned that collision into files the second pass could neither archive nor prune. Docs corrected against the code: the archive path, the tag count (98, not 91, and `teamai-wiki` is excluded), the version line, and the size table.
…assify legacy names as ours The Stop-hook hint said "run /teamai", but bare /teamai prints the menu and stops, so following the primary suggestion never reached the share workflow. It now names an invocation the core skill routes to share, with the `teamai skill get share` fallback kept. The four docs that quote the hint follow. The setup skill sent "publish one skill" to `teamai skill get share`, which handles session learnings and is refused when recall is off (the default); reusable-skill publishing lives in core's contribute-member reference and needs no recall. The routing row and the two references that repeated it now point there. classifySkill checked BUILTIN_SKILL_NAMES alone, so until the first pull pruned them, team-wiki-codebase and teamai-share-learnings showed as [local-only]. It now uses isCliOwnedSkillName, the rule push and uninstall already apply.
…e out of the tarball, report a failed uninstall delete A review of the whole branch against Tencent#678, Tencent#730 and the design doc, run before pushing. What it found and what changed: - The Stop-hook share reminder was gated on the hint switch alone; recall is off by default and `teamai skill get share` refuses then, so the reminder pointed at a command that said no. It is withheld while recall is off, the same gate the workflow has; the served text about when the prompt appears now matches. - `npm pack` swept `skill-data/wiki/scripts/__pycache__` into the tarball once the e2e suite had run the scripts. Excluded in package.json "files", asserted absent in the tarball test, and the e2e run sets PYTHONDONTWRITEBYTECODE. - The `share` description still offered to publish reusable skills, the flow its own body sends to `core`; the sentence is gone. - `skill show <unknown>` before `teamai init` threw the init error as a stack trace; it prints the not-found line and exits 1. - The stub pre-approved every `teamai` command from the always-loaded unit; narrowed to `Bash(teamai skill:*)`, which is all it asks for (Tencent#678). - Six routing lines loaded `core --full` to reach one reference; they name the file under `$(teamai skill path core)/references/` instead. - `uninstall` reported a failed delete as "holds files TeamAI did not put there; the packaged files were removed", both false and the error unprinted. It names the file and the error; a test makes the stub directory read-only. - The stub directory archived under `<tool>/.claude-skills-teamai/teamai/` while the legacy trees used `<tool>/.claude-skills/<skill>/`; one layout now. - CHANGELOG entry; dead `isRecallEnabled` import; wiki heading still naming `team-wiki-codebase`; JSDoc on the wrong declaration; stale byte counts; the usage guides gain the recall refusal and the archive location; the design doc records the `--json` deviation, the legacy-name classification rule, the uninstall symlink scope and the fail-open wording.
…ller, withhold share from read-only sources
The reviewer flagged Chinese in skills/ and skill-data/ a third time. Both
reach the agent as CLI output, so the stub's trigger keywords, the paired
sample invocations and the Chinese name for TGit go; the agent translates for the user. A test
fails on CJK anywhere under either root.
Pre-push review of the whole branch, and what changed:
- uninstall walked through a linked ~/.claude/skills and deleted the
packaged files inside the member's dotfiles checkout; pull refused the same
layout. removeOwnedFiles now owns the guard, so pull, deploy and uninstall
apply one check: the skills root and the skill directory. A linked
~/.claude (stow, chezmoi) is no longer refused, since every other resource
writes through it and refusing left those machines on the pre-stub trees.
- share was served to read-only HTTP teams, where its last step
(teamai contribute) always fails; reportingOnly used to skip it. The
serving gate carries a reason (recall | read-only) with its own message,
and `skill list --json` reports it as `blockedBy`.
- The bytecode rule claimed any file under any __pycache__; it now claims
only the .pyc of a shipped script.
- recall disable pruned the shared .agents/skills root for an uninstalled
Codex; it has deployment's install gate now.
- The source-team guard lost the legacy names when BUILTIN_SKILL_NAMES
narrowed, so a source removal could delete a legacy tree wholesale.
- Routing: the admin wrap-up and the stub still sent "share what I learned"
to share without saying it needs recall, and the stub filed "share this
with my team" (the publish-a-skill phrase) under share. recall enable is
described as the per-machine override it is, next to the team key.
- {SKILL_DIR} definitions now say how a reference file opened on its own
spells the directory, since serving resolves the definition too.
- skill show: packaged resolve only when init fails, aligned label, a
served skill is "served by the CLI, not installed".
- Docs: uninstall removes the archive with ~/.teamai; zh said the whole
directory is kept; the product overview lacked the recall gate; the design
doc's release, tag and byte figures were stale; CHANGELOG notes the
language change of generated documents.
… uninstall too The previous commit narrowed the guard to the skills root and the skill directory, so a link at ~/.config or ~/.config/opencode was walked through: the prune could delete, and deploy write, inside a dotfiles checkout. The full walk from the tool's base directory is back, and removeOwnedFiles now requires the base, so uninstall applies it too; each skill directory in the uninstall plan carries the base its skills root hangs off. A member whose whole ~/.claude is a link keeps the pre-stub trees and gets the warning naming the path, as before the previous commit. Deleting through a link is the one thing the prune must never do.
…cy names through the gate contributeHintAllowed checked recall only. The dispatcher already drops this gitOnly handler for HTTP teams, but the gate now says so itself, so the reminder never points at a `share` that refuses as read-only wherever it runs. `skill show teamai-share-learnings` searched the agent directories before the package, so a legacy tree a pull had not pruned yet was shown with its path while the gate refused `share`. A legacy built-in name now skips the agent search and goes to the packaged skill and its gate; ordinary names and aliases such as `share` keep a member's own directory first.
deployBuiltinSkills joined baseDir with the configured skills path, while team-skill sync resolves the directory through skillsDirForTool: OpenClaw's workspace, and HERMES_HOME for Hermes. Those agents got the stub in a directory they never read and had their legacy trees pruned from the wrong place. Deploy, the legacy prune, recall disable and uninstall now resolve the same directory; the link guard starts at the tool's base directory when the skills directory sits under it, else at that directory's parent.
…ad logs off stdout, drop inert allowed-tools - A skills directory outside the tool's base (HERMES_HOME, an OpenClaw workspace) had the guard start at the root itself, so a linked root was never checked. It starts one level above now, and a linked HERMES_HOME is refused like a linked ~/.claude. - The share gate loads the config, which can migrate it and report that with log.info on stdout: an upgrading machine got that line in `skill get` output and in `skill list --json`. Config loading reports on stderr for that call; setStderrOnly returns the previous mode so it can be restored. - `allowed-tools` in the served skills was printed as command output and never processed as skill metadata, so it granted nothing. Removed, and the test now fails if one comes back. Only the stub's line pre-approves.
…ILOT_HOME is refused skillsGuardBase started at the tool's base directory, which for Copilot in user scope is COPILOT_HOME, so the walk never checked whether COPILOT_HOME itself was a link, and pull and uninstall wrote and pruned through it. The guard now starts at the scope root (home, or the project root), where a link at or above is ordinary, in deploy, the legacy prune and uninstall alike; a root configured outside it still has the walk start just above that root.
…a broken config, quote skill paths
- share and wiki had moved generated learnings and knowledge-base documents
from always Chinese to the session language. Serving the instructions from
the CLI does not need that, so both say "Simplified Chinese" again, in an
English instruction; the CHANGELOG entry follows.
- skill show and skill list treated every autoDetectInit failure as "not
initialized" and pointed at `teamai init`. requireInit now throws a tagged
NotInitializedError; only that falls back to the packaged catalog, and a
malformed or unreadable config propagates.
- `$(teamai skill path …)/…` is word-split in a shell command like an
unquoted {SKILL_DIR}; all ten occurrences are double-quoted and the quoting
test covers the form.
…issing loadLocalConfig returns null both for a missing file and for one that fails to parse, validate or migrate (it logs the reason). requireInit turned every null into NotInitializedError, so skill show and skill list still fell back to the packaged catalog and a `teamai init` hint on a broken config. Only an absent file is NotInitializedError now; an existing one that could not be used is an error naming its path, in requireInit and the user branch of requireInitForScope. Covered through the real loader and the built binary.
…econd Codex copy - The legacy prune and uninstall removed any file at a path a release had packaged, so a member's edit, a skill of their own under an old name, or a root TeamAI never managed (toolPaths or HERMES_HOME moved) lost its files. A file is ours now only at a packaged path and with content a release shipped there: PACKAGED_SKILL_DIGESTS records the sha256 of every blob over all 99 tags through v0.25.0 and main before the stub, 37 versions across 21 paths. A skill-root SKILL.md is compared by its body, since releases before 0.17 shipped no frontmatter and the deploy of the day repaired it on disk. The current stub is ours by the packaged copy. Anything else stays. - Codex reads .codex/skills and the shared .agents/skills, and the stub goes to the shared one when a copy lives there; the copy an earlier release left in the other root kept its old SKILL.md and references. It is retired by the same ownership rule, archived first, and named when kept. - Tests mock the digest table with a stand-in for shipped content, and a test keeps the stand-in on the same paths as the real table.
…the rebase Tencent#713 and Tencent#736 edited skills/teamai/references/*.md, which this branch moved to skill-data/setup/references/. Two hunks did not follow the move: - join-member.md: TGIT_TOKEN is REST-API-only and cannot clone (Tencent#713). - setup-admin.md: the /teamai share entry publishes a reusable skill; a session's learnings are automatic (Tencent#736), in English as the served text is. Tencent#739's partial config mock is restored in skip-uninstalled-tools.test.ts.
…ig, drop hidden commands from the reference - Legacy trees were pruned before the stub was written, so a refused or failed stub (a link, a read-only directory) left the agent with nothing to discover. They go only once the stub deployed for that agent. - The share gate failed open on any config error. Only a machine with no config (NotInitializedError) is served; a config that exists but cannot be loaded blocks with its own reason, `blockedBy: "config"`. - The KB template told agents to run `code-to-knowledge --update`, which does not exist; it names `teamai codebase --extract … --incremental`. - The generated command reference listed hidden hook plumbing (`track`, `contribute-check`, `todowrite-hint`, …). It renders what `--help` lists. - removeEmptyDirs swallowed every rmdir error, so a directory that stayed could be reported removed. Only "still holds something" is expected; any other failure is reported.
…e effects before the link guard Review of 327f9cd: - SKILL.md was compared by its body, so a member who changed only its frontmatter lost the file. Every release from 0.16.1 (the first whose deploy repaired frontmatter) shipped complete frontmatter, so what is on disk is what was shipped: digests are whole files now (42 versions over 100 tags and main). A link is never ours; bytecode is ours only beside a script proven ours by content, decided before anything is removed. - The stub dir's retired references were pruned before SKILL.md was copied; a failed copy left the old skill pointing at files that were gone. The stub is written first. - The Codex destination was resolved with the reconciliation that deletes a duplicate, before the link guard ran. It is resolved side-effect free; the other copy is handled under the guard by retireOtherCodexCopy, whose report now names a failed backup or delete as such. - A broken project config was skipped by detection, so the share gate answered with the user config. findUnreadableProjectConfig reports it via an optional sink on detection (no caller changes), and the gate blocks. The Stop-hook reminder is withheld on an unloadable config too. - init announced the stub as ready when nothing was deployed; hook-dispatch is hidden (hook plumbing), and the reference says it lists public commands; the design doc no longer says teamai-workflow/teamai-import are removed.
… a fallback loads findUnreadableProjectConfig dropped a recorded error whenever detection went on to find a later candidate: a broken partition config followed by a valid legacy .teamai/ config returned null, and the share gate answered with the fallback's team. It now reports the first unreadable file regardless. An existing config file that is empty or cannot be read is reported to the sink too, instead of returning without a word.
78d2b2b to
5f5b2c1
Compare
Reconcile the manifest namespace guard with Tencent#698 (push placement for rules and agents) and Tencent#699 (skills served from skill-data/). - Keep main's RolesManifestNotFoundError name with this branch's stricter loader (readManifestFile: only ENOENT without a dangling link is absent). - Import isSafeNamespaceSegment and NAMESPACE_RULE from manifest-schema in push.ts and push-namespaces.ts; drop the old ASCII wording from their namespace errors. - Port the role-id guards into main's placement: the legacy fallback with no roles.yaml and the silent default both refuse an unsafe role id. - pushCore reports a failing scan (e.g. an unparseable roles.yaml) with exit 2 instead of an uncaught stack trace; a projects manifest that cannot load for --project does the same. A legacy role that could not be resolved no longer places a new rule or agent at the shared root. - The skills scan rethrows only manifest load failures again; a valid manifest that no longer lists the role keeps the role-id fallback. - The learnings index uses the namespace rule contribute writes with, so a non-ASCII namespace is indexed. - Namespace errors quote the value and manifest read errors say what to do. - Docs, CHANGELOG and skill-data (troubleshooting, manage-admin) describe the broken-manifest behavior, including that --role cannot bypass it.
…eploy (#747) * fix(skills): one share gate, actionable refusals, and a louder stub deploy Follow-ups from the review of #699: - The Stop-hook reminder and `teamai skill get share` ask one gate (`shareGate`, through `contributeHintAllowed`). The hook skipped the unreadable-project-config check, and the legacy `teamai contribute-check` command, still called by hooks written before the dispatcher, checked nothing, so both nudged towards a command that refused. - The gate reads only a config load failure as "cannot be loaded"; any other fault propagates (the hook withholds the reminder and logs it at debug). - A `config` refusal says what failed (the file and position for a parse error) instead of pointing at `teamai doctor`, which cannot see a broken config. `skill show` now refuses through the same helper, so its hint moves from stdout to stderr like `skill get` and `skill path`. - `pull` warns when the discovery stub cannot be deployed (it was an empty catch on the fast path and a debug line on a full sync), and so does the legacy prune. - Error text no longer claims a reason was logged when none was: an empty config is named as empty, and `init` points at ~/.teamai/debug.log, where every path that deploys nothing now records why. - `core` routes a bare `/teamai` right after a friction reminder to `share`, as the stub already said. - The command drift guard rejects an unknown subcommand inside a group (`teamai skill gett core` passed before). - The contribute-check e2e asserts the reminder's real text again; the usage guides (EN, zh-CN) and the design doc cover the config refusal, the gate and the reminder routing. * test(learnings): retry temp-dir cleanup that races a detached git gc A push into the bare origin can leave `git gc --auto` writing to objects/pack after the test returns; the single rmdir in afterEach then fails with ENOTEMPTY (seen on CI, Node 22 ubuntu, #747). * fix(skills): gate skill show before its lookups, name the failing field Review of #747: - `skill show share` under a broken project config searched the user config's team repo and agents, which detection falls back to, and printed a `share` found there. It now asks the gate first and refuses on a config block before any lookup. With an empty user config it refuses instead of ending in a stack trace. - A config that parses but fails validation reported the Zod JSON dump, whose first line is `[`, so the refusal said `config.yaml: [.`. Every config loader now reports each issue as `field: reason` on one line. - The docs and skills that describe the share reminder or the refusal say it is withheld on a read-only source and while the config cannot be loaded, and that a validation failure names the field: product-overview and usage-guide (EN, zh-CN), designs/skill-serving.md, core/SKILL.md, contribute-member, setup-admin, join-member and manage-admin. * fix(skills): no share reminder where teamai is not set up `contributeHintAllowed` fell open with no config at all, so a caller other than the dispatcher (the legacy `teamai contribute-check`) still nudged in projects that never set up teamai, which have no team to share with (#748). It now returns false there. Serving the skill stays fail-open. * fix(contribute-check): gate the legacy reminder on the session's cwd `teamai contribute-check --stdin` asked the share gate about the directory the hook process started in, while the session analysis used the payload cwd. Started outside the project, it could read the user config and nudge where `teamai skill get share` refuses (a project config that does not load). It now moves to the payload cwd first, as hook-dispatch does. * fix(pull): keep a debug.log record when the stub cannot be deployed The previous commit turned both deploy catches into `log.warn`, which is muted in silent mode and never reaches debug.log, and a SessionStart pull runs detached with its output discarded. So the automatic pull, the one that deploys the stub for most members, lost the only persistent record it had. Both catches now warn and write the same line to debug.log. * fix(skills): skill show and list never answer for the fallback team Known issues left by #747: - `skill show <name>` and `skill list` on a config that exists but does not load ended in a Node stack trace, and under a broken project config they searched the user config detection falls back to: another team's repo and agents. Both now ask `detectTeam`, the one place that tells "this team", "no team" and "cannot tell, and why" apart (`shareGate` is built on it). Without a usable team, `show` answers from the package alone and `list` prints only the packaged catalog; both say what failed on stderr and exit 1. - A teamai.yaml that exists but fails validation was reported as "not found. Check your repo path". It is now named as invalid, empty or unreadable, like the local config. * fix(skills): the gate reads the session's directory, and no project config is skipped Codex review of 5793758: - A project-location config that is not `scope: project` (or omits `scope`, which defaults to user) was skipped without a word, so the gate read past it to the user config. It is now reported as unusable, unless it is the user config itself, as when running from HOME. - The legacy `contribute-check` changed into the payload cwd and, if that failed, asked the gate about the directory the process started in. It now passes the payload cwd to the gate (`detectTeam(cwd)`), and a cwd that no longer exists holds no project config, so only the user config is asked, as #753 does. * fix(logger): record warnings in debug.log `log.warn` wrote to the console only and was muted in silent mode, so a detached SessionStart pull, whose output is discarded, lost every warning: the stub deploy failure and the legacy prune among them. Warnings now reach debug.log like debug and error lines. `warnStubNotDeployed` drops the second `log.debug` call, which printed the line twice under --verbose. * fix(skills): the dispatcher gate reads the payload cwd; a symlink is not HOME Codex review of b0583f5: - The dispatcher's `contribute-check` and `pending-hint` handlers asked the gate about the process's directory, trusting hook-dispatch's `chdir`; when that failed, the launcher's config decided. They now pass `resolveHookCwd(stdin)`, as the legacy command does. - The HOME exception for a non-project scope compared the config file's real path, so a project config symlinked to ~/.teamai/config.yaml passed for the user config. It is now decided by the project's location: its root is HOME. * fix(skills): only a missing cwd falls back to the user config; load it once Codex review of 15a5b5d: - `detectTeam` read any failure to see the payload cwd as "deleted", so a cwd it could not open (no permission, a path through a file) fell back to the user config and could allow the reminder. Only ENOENT does now; anything else is `unusable` and withholds it. - `skill show share` and `skill list` loaded the config twice, through the gate and then the team lookup, and reported a broken one twice. Both detect the team once and hand it to the gate. * fix(logger): a file-only record instead of persisting every warning b0583f5 made every `log.warn` append to debug.log, wider than the two failures it was for, and it wrote unrelated subprocess errors to disk. `log.warn` is console-only again; `log.persist` writes one line to debug.log and never to the console. The stub deploy catches and the legacy prune catch use both, so a detached SessionStart pull keeps the record and --verbose prints it once.
2 participants
Closes #678. Closes #730.
What #730 asked for, and what this does instead
#730 reports that
BUILTIN_SKILL_NAMESdoes not holdteamai, soscanLocalForPushreads the deployed stub as a user-authored skill andteamai pushuploads it to the team repo. Pull cleanup, uninstall and classification take the wrong branch for the same reason.Here the set is
{'teamai'}, and the push scan asksisCliOwnedSkillName(src/resources/skills.ts:443), which covers the current name and the legacy ones a pre-stub release wrote. A member who upgrades and runsteamai push --allbefore their next pull still has those trees on disk, and the scan skips them too.Two differences from the issue's plan are worth stating.
teamai-workflowandteamai-importleave the set. The package has never shipped either name, so a directory with one is the member's own skill, and guarding it stopped them publishing it.push --all --dry-runnow offers them, which the genericgitprovider row below records.The delivery half of #730 stops being a delivery problem. The issue accepts "takes effect on the next pull, one session behind" because the content is a file the CLI copies. Here the content is not copied at all.
teamai skill get coreprints what the running binary packages, so an upgrade changes the answer immediately and there is nothing to drift. What the next pull still does is rewrite the 2.5 KB stub and remove the trees earlier releases left, whichpull.ts:966runs even when the team repo SHA has not moved.Summary
The built-in skill content is now versioned with the CLI. It ships inside the npm package and is printed by the installed binary, so
teamai skill get coreon version X prints version X's instructions the moment that version is installed — no pull, no per-agent copy that drifts between releases. The one thingnpm i -g teamai-cli@latestdoes not do by itself is rewrite what is already on disk in the agent directories: the deployed stub and the prune of the legacy trees ride the nextdeployBuiltinSkills, whichpull.ts:966runs even when the team repo SHA is unchanged, and which the session-start hookinitinjects runs automatically. So a member upgrading mid-session keeps a stale stub until their next session, while the content the stub loads is already the new version's. Agents get one deployable file, a discovery stub that points at those commands.What the agent reads, and when:
Commands added to the existing
skillgroup, no new top-level command:Deployment writes one file and clears what it does not own:
Thirty-seven commits, each green: the commands, the move plus the stub, the content repairs, the prune with the run-time recall gate, the docs, one commit per review pass, the rebase onto #724 (which moves its new
provider-tgit.mdintoskill-data/setup/references/and repoints the two files that cite it), and three rounds on the prune — the archive, then fail-closed on a failed copy and collision-free paths, then the symlink refusal and archiving only what is retired, and two commits after the rebase: the reviewer's round ona66f025, and a pre-push review of the whole branch (below); then the rounds one4aa6ca,df6f018,b4f675f,fe0540e,18c26b8,e747a75,238e205,f16e725and427f7e2(below).Rebased onto
origin/mainat667aed0(#715, #698, #689, #741, #733, #737 landed sincea52374a). One conflict, in the closing lines ofinit(src/init.ts), replayed at two commits: #689 reworded the hooks line toSkills, rules, env and docs auto-sync on each session start when the selected agent has active TeamAI hooks., and this branch replaces theBuilt-in skills (e.g. team-wiki-codebase)line with thestubDeployedcheck. The resolution keeps both. Two things #698 changed underneath the skills without a textual conflict, each folded into the commit it belongs to so every commit stays green (git rebase -x 'npx tsc --noEmit && npx vitest run commands-reference'passes on all 37): thepush --role/--projecthelp text, soskill-data/core/references/commands.mdis regenerated in the commit that introduces it; andpush-namespace-e2e.test.ts, whose "newer namespaced rule" case relied on #730's bug — its second push uploaded the deployedteamaistub toskills/fe-skills/teamai, and the${branch}:rules/…assertion only held because that push created a branch. With the stub CLI-owned there is nothing to push, so the case now assertsNo new or modified resources to pushand thatmainkeeps the teammate's rule; onmainthat assertion fails, which makes it a #730 regression guard too. #698's new paragraph incontribute-member.mdfollowed the move toskill-data/core/references/by itself. Earlier: rebased ontoa52374a(#718, #713, #736, #739 landed sincecd3e0e6). #713 and #736 editedskills/teamai/references/*.md, which this branch moves toskill-data/setup/references/; the two hunks that did not follow the move (theTGIT_TOKENnote injoin-member.md, #736's share hand-off insetup-admin.md, the latter in English as the served text is) are carried over in96d0f7c, together with #739's partialconfig.jsmock. Commit hashes named in the review rounds below predate this rebase. The previous rebase was ontocd3e0e6(#720, #700, #695, #729 landed since the previous base). One conflict, inbuildHint(src/contribute-check.ts): #720 dropped thePrint the following message verbatimwrapper from the Stop-hook hint and moved the relay torelayWhenHidden; this branch had changed the sentence inside it to name/teamaiandteamai skill get share. The resolution keeps #720's shape with this branch's wording. Nothing in those four commits touchesskills/orskill-data/; what they change and whether the served skills had to follow is recorded under What main changed underneath the skills below.Evidence
Version match: the served content is the packaged content of the running binary, byte for byte (
skill get teamai | diff - skills/teamai/SKILL.mdbelow, and the e2eserves every skill it listsfor all four workflows). Before this change an upgrade left the previous release's instructions in every agent until the next pull, which is what the four drift-fix commits were about.Before: 21 files, 176 775 bytes copied into every installed agent's skills directory on every pull. A machine with ten agents held ten copies.
team-wiki-codebase/SKILL.mdwas 38 705 bytes, read in full whenever the skill triggered.After: one file of 2 529 bytes per agent, byte-identical to the packaged stub. Measured with the built CLI:
The recall gate, the stream discipline and the exit codes, from the same build:
{SKILL_DIR}resolves at print time, so the wiki's scripts run as written:Automated runs, on the head revision (
5f5b2c1, onorigin/mainat667aed0):The e2e suite was run locally because CI cannot run it here.
E2E (GitHub provider, full surface)carriesif: ${{ vars.TEAMAI_TEST_REPO_URL != '' }}(ci.yml:92), so it skips on everyfork PR — the green CI check on this PR contains no e2e at all.
npm run test:e2eabove is thatjob's suite, run against the built CLI on this revision. One caveat worth recording: the run needs
npm cifirst, sinceopencode-recall.test.tsshells out tonode_modules/opencode-ai/postinstall.mjsand fails in
beforeAllwithout it.Three guards were added because the failures they catch are invisible otherwise:
The second one is what would have stopped
e151d43,1ca43ac,8bb0548and2ddb546, each of which fixed a skill naming a command the CLI no longer had. The third catches a missingpackage.json"files" entry, which every other test passes through while the published package serves nothing.An audit of the three skills found 60 defects; this fixes the ones that survive the move. The wiki skill advertised
teamai extract graph, which has never existed. The cheat sheet labelled "ground truth" omitted 19 real commands. Every documented script path assumed the skill sat in the agent's own directory.Language: instructions in English, generated documents unchanged
The instructions the CLI prints are English, because the CLI now prints them and the repo's rule is
CLI user-facing output must be English. No Chinese in production code.The documents those instructions produce keep the languagemainproduces:sharestill requires Simplified Chinese learnings (main:所有生成的文档必须使用中文撰写), andwikistill writes its knowledge base in Simplified Chinese, whichmaindid implicitly by being written in Chinese. Both are now English sentences that say so. An earlier revision moved both to the user's session language; the reviewer flagged that as a product change this PR does not need, and it is reverted (round on238e205below).Everything under
skills/andskill-data/is English, including thewikiskill, whichmainshipped in Chinese; the only non-English left is the\uXXXXescapes invalidate_kb.py's compatibility patterns.validate_kb.pystill recognises the headings of knowledge bases built by the previous release, so--updateand validation keep working on them; new documents are generated with English names ({project_name} Technical Architecture.md,XX_{component}_Design.md,G1_…_Component_Dependency_Matrix.md).Manual matrix, built CLI against a real GitHub team repo (a throwaway private repo, deleted afterwards), in an isolated HOME with Claude, Codex, CodeBuddy and OpenCode all present:
A fresh team has recall off by default (
getRecallSharingreturns false), soskill get sharerefuses until someone enables it. That matches the behaviour it replaces: the old deployment skipped the share skill underskipRecall.Generic
gitprovider row, re-run on1d84ec3, same matrix, built CLI in an isolated HOME against a local bare remote (provider: gitinteamai.yaml, the layout the repo's own e2e tests use), Claude, Codex, CodeBuddy and OpenCode present, plus user-createdteamai-workflowandteamai-importskills seeded in.claude/skills:After the rebase onto
667aed0, the genericgitrow was re-run on5f5b2c1(built CLI, isolated HOME, local bare remote, user scope, Claude, Codex, CodeBuddy and OpenCode enabled; the pre-stub layout, fromv0.25.0, seeded in.claudeand.codebuddyafter the first pull):init's closing lines, the conflict above, were not re-run against a hosted provider:inittakes no local remote, and no test asserts those lines.Reporting-only HTTP row, built CLI, isolated HOME,
teamai init --http … --token …, Claude andCodex installed, the pre-stub layout seeded. This is the row the last review pass was about:
Qoder CN row, run on
a66f025after the rebase, because #695 addedqoder-cnas a built-in target with its own user root (~/.qoder-cn/skills) and the prune had never seen it. Built CLI, isolated HOME, local bare remote (provider: git),init --agent claude,qoder-cn --scope user, the pre-stub trees seeded in both roots plus one file the package never shipped (team-wiki-codebase/references/phase0.md) to stand in for a member's own:Round on
a66f025, and the pre-push review that followedThe reviewer's four findings on
a66f025: the nudge said bare/teamai, which the core skill answers with a menu and a stop, so it now names/teamai share what this session taught mewith the literal command kept; the setup skill routed "publish one skill" toshare, which handles session learnings and is refused when recall is off, so that row and the two references that repeated it point atcontribute-member.md;classifySkillusedBUILTIN_SKILL_NAMESalone and showed the legacy trees as[local-only]until the first pull, so it usesisCliOwnedSkillNamelike push and uninstall do; the Chinese trigger keywords stayed that round and went in the next (below).Then a review of the whole branch against #678, #730, the design doc and the repo rules, run before pushing so the next automated round has less to find. What it turned up, and what was done:
contributeHintAllowedchecked the hint switch only; recall is off by default, andteamai skill get shareexits 1 then. The hint is now withheld while recall is off, the same gate the workflow has. This is the one product call in the set, so it is stated as one: a fresh team sees no share reminder untilteamai recall enable.mainwas no better there — it nudged towards a skill thatskipRecallhad not deployed — but it was quieter about it. The served text that described when the prompt appears was wrong in both directions and now matches.npm packshipped Python bytecode. Running the wiki scripts, which the e2e suite does, leaves__pycache__underskill-data/, andfilesswept it in. Excluded inpackage.json, asserted absent in the tarball test, and the e2e run setsPYTHONDONTWRITEBYTECODE.share's description still offered to publish reusable skills, the flow its own body sends tocore. Sentence dropped, so the catalog no longer steers "publish a skill" at a recall-gated entry.skill show <unknown>beforeinitprinted a stack trace. Not-found line, exit 1, covered in the e2e file.teamaicommand, includingpush,uninstallandrecall enable, from the always-loaded unit; [feat] Serve builtin skill content from the CLI: deploy a discovery stub, addteamai skill get#678 asked for the serve commands only. Narrowed toBash(teamai skill:*). The served skills grant nothing (see the round on18c26b8): everything else a workflow runs goes through the agent's own permission prompt, as onmain.core --full(36 KB) to reach one 5.7 KB reference. They name the file under$(teamai skill path core)/references/instead.uninstallmisreported a failed delete as "holds files TeamAI did not put there; the packaged files were removed", both false, with the error never printed. It now names the file and the error; a test makes the stub directory read-only and asserts the sentence.<tool>/.claude-skills-teamai/teamai/) than the legacy trees (<tool>/.claude-skills/<skill>/). One layout now, as the design doc says.CHANGELOG.mdentry (added); a deadisRecallEnabledimport ininit.ts; thewikiskill still introducing itself asteam-wiki-codebasein its heading and overview; a JSDoc block sitting on the wrong declaration; stale byte counts in the design doc and the served wiki text; the two usage guides silent on the recall refusal and on the archive under~/.teamai/removed-skills/, and one paragraph that had drifted between the languages.teamai skill get#678 sketchedskill get --json; it is not implemented, the content is markdown for an agent and the machine-readable half isskill list --json. The design doc says so now. The legacy names classifying as[builtin]by name alone, even for a directory pull kept because it holds a member's file, is the consequence of pathname ownership the design doc already accepts, and is written next to it with the retirement ofLEGACY_BUILTIN_SKILL_NAMESas the resolution. The recall gate fails open when the team config cannot be loaded, not only when there is none; the doc says that too.Round on
e4aa6ca, and the pre-push review that followedThe reviewer's one finding on
e4aa6ca: Chinese in the stub's description and insetup's references, flagged a third time. Removed, as the language section now says, and guarded by a test. The other three findings from that comment were already fixed one4aa6ca.Then two review passes over the whole branch, one on
src/, one on content and docs, before pushing. What they found, and what was done:uninstalldeleted through a linked~/.claude/skills. It called the prune without the base directory, so only the leaf was checked; with the skills root linked at a dotfiles checkout, the packaged files inside that checkout went.removeOwnedFilesnow owns the check, so pull, deploy and uninstall apply the same one. Test: a linked skills root survivesuninstall --force, red one4aa6ca.~/.claudeblocked the stub and the migration. Narrowing the guard to the last two components for that reason was reverted in the next round (below): it let a link at~/.config/opencodethrough.sharewas served to read-only HTTP teams, where its last step,teamai contribute, always refuses;reportingOnlyused to skip it at deploy time. The gate carries a reason (recall|read-only) with its own message, andskill list --jsonreportsblockedByinstead ofblockedByRecall. The field is new in this PR, so nothing onmainreads the old name.__pycache__, in any CLI-owned directory. It now claims only the.pycof a shipped script. Test, red one4aa6ca.recall disablepruned for an uninstalled Codex, reaching the shared.agents/skills; it has deployment's install gate now. The source-team guard lost the legacy names whenBUILTIN_SKILL_NAMESnarrowed, so a source removal could delete a legacy tree wholesale; restored.sharewith no word about recall, and the stub filed "share this with my team", the publish-a-skill phrase, undershare. Both fixed; the stub says publishing lives incore.teamai recall enablewas described as turning recall on "for the team"; it writes this member's override, and the text now namessharing.recall.enabledbeside it.contribute-member.mdstill called the learning flow automatic and tool-dependent.{SKILL_DIR}: serving resolves the sentence that defines it too, so a reference file opened on its own showed a placeholder nothing defined. The definition now says how such a file spells it.uninstallremoves the archive along with~/.teamai/(now said, in both guides); the zh guide said a directory with a member's file is kept whole; the product overview lacked the recall gate on the hint; the design doc's release, tag-count and byte figures were stale; the CHANGELOG now notes the language of generated documents (since restored to Simplified Chinese, round on238e205), and the wiki skill says to keep an existing knowledge base's file names.skill showresolved the packaged skill before it knew it needed to; its card label was a column off and said a served skill was "not installed in any agent yet"; a comment still named bare/teamai;skill get --allcarried a branch its own comment called impossible; two tests setTEAMAI_COMMAND_TABLE_ONLYwithout restoring it; a new two-scope test covers theinheritUserScopebackup path, which had none.team-wiki-codebase) is synced and then pruned. Push refuses those names, and onmainthe built-in overwrote such a skill on every pull, so the name was already unusable; filtering it out of team sync is a behaviour change of its own. Prune warnings on the session-start hook path go to stderr, which hosts hide for a successful hook, andlog.warnis not written todebug.log; that is the logger's behaviour for every warning, not this PR's to change.Real-CLI rows on
df6f018(superseded by theb4f675frows below):The
githubandgitlabrows above were run on earlier heads. This round changes no provider code and nothing that runs before the repo is on disk, which is where those rows differ fromgit.Round on
df6f018The reviewer's two findings on
df6f018:~/.configor~/.config/opencodesits above both, so the prune walked, deleted and wrote through it. The full walk from the tool's base directory is back, andremoveOwnedFilesnow requires the base, souninstallruns the same walk: each skill directory in its plan carries the base its skills root hangs off (the OpenClaw workspace root is its own base). The price is the one the previous round tried to avoid: a member whose whole~/.claudeis a link keeps the pre-stub trees and gets a warning naming the path. Deleting through a link is the thing the prune must never do, so that member is told rather than guessed for. Two tests, both red ondf6f018: a linked~/.config/opencodestops deploy and prune, and a linked~/.claudestopsuninstall.b4f675f, and this time the description was updated before the push.Real-CLI rows on
b4f675f(re-run onfe0540e, same results, plus theskill showrow in the next round):Round on
b4f675fThe reviewer's two findings on
b4f675f, both fixed infe0540e:contributeCheckHandleris registeredgitOnly(src/hook-handlers.ts:635) andfilterHandlersForConfigdrops those forrepo.kind === 'http'. But the gate claimed to matchshare's and did not say so itself, socontributeHintAllowednow checks the source too. Test: an HTTP team with recall on gets no hint, red onb4f675f.skill show teamai-share-learningsexposed a stale legacy tree.locateSkillsearched agent directories before the package, so a directory a pull had not pruned yet was shown with its path while the gate refusedshare. A legacy built-in name now skips the agent search and goes to the packaged skill and its gate;share,codebaseand the other ordinary aliases keep a member's own directory first. Test, red onb4f675f.Real-CLI rows on
fe0540e: every row of the previous round, same results, plus:Round on
fe0540eThe reviewer's one finding on
fe0540e, fixed in18c26b8:deployBuiltinSkillsjoined the base directory with the configured path, while team-skill sync resolves it throughskillsDirForTool: OpenClaw's workspace, andHERMES_HOMEfor Hermes.mainhad the same join, but there the built-ins were extras; here the stub is the only unit, so those agents got it in a directory they never read, and their legacy trees were pruned from the wrong place. Deploy, the legacy prune,recall disableanduninstallnow resolve the same directory. The link guard starts at the tool's base directory when the skills directory sits under it, else at that directory's parent (aHERMES_HOMEoutside the home directory). Two tests, both red onfe0540e: Hermes underHERMES_HOMEand OpenClaw's workspace receive the stub and the prune, anduninstallremoves the Hermes stub there.Real-CLI rows on
18c26b8: every row of the previous rounds, same results, plus:Round on
18c26b8The reviewer's three findings on
18c26b8, all fixed ine747a75:HERMES_HOMEor an OpenClaw workspace outside the tool's base, the guard started at that root, so a linkedHERMES_HOMEwas walked through. The guard starts one level above such a root now. Test: a linkedHERMES_HOMEoutside the home directory gets no stub and loses no file, red on18c26b8.skill getandskill list --json. The share gate loads the config, and loading an upgraded one runsmigrateLegacyRoleConfig, which reports withlog.infoon stdout. That line landed ahead of the skill content and the JSON. Config loading reports on stderr for that call;setStderrOnlynow returns the previous mode so the gate restores it. Test: a migrating config leavesskill get sharestarting at its frontmatter andskill list --jsonparseable, with the line on stderr, red on18c26b8.allowed-toolsin the served skills granted nothing (P2, fixed because the round had blockers).skill getprints that frontmatter as command output; the agent never reads it as skill metadata, so the earlier claim that it removed permission prompts was wrong, and this description no longer makes it. The four lines are gone and the content test fails if one comes back. The stub'sBash(teamai skill:*)is the only grant; widening it toBash(teamai:*)would undo the [feat] Serve builtin skill content from the CLI: deploy a discovery stub, addteamai skill get#678 narrowing, soteamai pulland the rest prompt through the agent's own flow, as they did onmain.Real-CLI rows on
e747a75: every row of the previous rounds, same results, plus:Round on
e747a75The reviewer's one finding on
e747a75, fixed in238e205:COPILOT_HOMEwas not checked. The guard started at the tool's base directory, which for Copilot in user scope isCOPILOT_HOMEitself, so the walk began below the link, and pull and uninstall wrote and pruned through it. The guard now starts at the scope root (home, or the project root), where a link at or above is ordinary, in deploy, the legacy prune anduninstallalike; a root configured outside the scope root (HERMES_HOME, an OpenClaw workspace) still has the walk start just above it. Two tests, both red one747a75: a linked~/.copilotstops deploy and prune, and stopsuninstall.Real-CLI rows on
238e205: every row of the previous rounds, same results, plus:Round on
238e205The reviewer's three findings on
238e205, all fixed inf16e725:shareandwikihad moved learnings and knowledge-base documents from always Chinese to the session language. Serving the instructions does not need that, so both say "Simplified Chinese" again, in an English instruction, and the CHANGELOG entry says the output language is unchanged. The language section above is rewritten to match.skill showandskill listcaught anythingautoDetectInitthrew and pointed atteamai init, so a missingteamai.yamlor an unreadable config sent the member to re-init over a working setup.requireInitthrows a taggedNotInitializedErrornow (same message); only that falls back to the packaged catalog, and anything else propagates the way it does forteamai status. Test: a broken config rejects instead of printing the fallback, red on238e205.$(teamai skill path …)/…was unquoted (P2), word-split in a shell command exactly like an unquoted{SKILL_DIR}. All ten occurrences are double-quoted, and the quoting test covers the form (red against the previoussetup/SKILL.md).Real-CLI rows on
f16e725: every row of the previous rounds, same results, plus:Round on
f16e725The reviewer's one finding on
f16e725, fixed in427f7e2:loadLocalConfig()returnsnullboth for a missing file and for one that fails to parse, validate or migrate (it logs the reason), andrequireInit()turned everynullintoNotInitializedError, so the previous round's narrowed catches still fell back to the packaged catalog on a broken config. Only an absent file isNotInitializedErrornow; an existing file that could not be used is an error naming its path, inrequireInitand the user branch ofrequireInitForScope. Covered through the real loader (config-not-initialized.test.ts: a missing file, an unparseable one and an invalid one; the last two red onf16e725) and through the built binary in the e2e file (skill listandskill show coreon an unparseable config exit non-zero with "could not be read" and no "not initialized" line).detectProjectConfigreturnnull, and detection then falls back to the user config. That is how every command onmainresolves scope; changing detection is its own change.Real-CLI rows on
427f7e2: every row of the previous rounds, same results, plus:Round on
427f7e2The reviewer's three findings on
427f7e2. Two are fixed in99c0722; the third is recorded, not done.PACKAGED_SKILL_DIGESTS(src/packaged-skill-digests.ts, generated from every tag andmainbefore the stub) holds 37 shipped versions across 21 paths, a skill-rootSKILL.mdcompared by its body because releases before 0.17 shipped no frontmatter and the deploy of the day repaired it on disk. The current stub counts as ours by the packaged copy. An earlier round rejected content checking for fear of CRLF; that does not hold, because these files were copied from the npm tarball byte for byte, never checked out by git, so a mismatch is the member's change and keeping it is the safe direction. Unit tests mock the table with a stand-in for shipped content (a test keeps the stand-in on the real table's paths); three new tests are red on427f7e2, and existing ones that expected an edited file to be pruned now expect it kept..agents/skills/teamaiwhen a copy lives there, and the copy an earlier release left in.codex/skills/teamaikept its oldSKILL.mdand references.retireOtherCodexCopyremoves it by the same ownership rule, archived first, and names it when a member's file keeps it. Two tests, red on427f7e2.teamai initagainst GitLab — done afterwards: We ranteamai initagainst GitLab with a GitLab Personal Access Token, and it passed. (It needs a PAT withapiscope: the provider sendsGITLAB_TOKENasPRIVATE-TOKEN, which aglabOAuth login does not satisfy.)Real-CLI rows on
99c0722, with the real v0.25.0 trees taken from the tag (git archive v0.25.0 skills), so the digest table is what is exercised, not the test stand-in. Built CLI, isolated HOME, local bare remote (provider: git), Claude, CodeBuddy and Codex:Every row of the previous rounds re-ran on
99c0722with the same results; the Hermes row now seeds the real 0.25.0SKILL.md, since arbitrary text at that path is now, correctly, the member's.Round on the review of 04:06 (all findings, after #739)
#739 made the reviewer report every finding; its first full pass ran on
427f7e2, before99c0722was pushed. Its findings and what happened to each:teamai initnot run327f9cd: the prune runs only after the stub deployed for that agent; a refused stub keeps the old trees and says so. Test red before.retireOtherCodexCopy.327f9cd: only a machine with no config is served; an unloadable one blocks withblockedBy: "config". Test red before.code-to-knowledge --updateteamai codebase --extract <repo> --project <slug> --incremental.Help.visibleCommands, so it matches--help.hook-dispatchstays because it is not hidden.removeEmptyDirsswallows errorsrmdirof a non-empty directory under a read-only parent isEACCES, so the contents decide, not the code. Test red before.Real-CLI rows on
327f9cd: every earlier row and the real v0.25.0 row, same results, plus:Round on
327f9cdteamai initnot runteamai initagainst GitLab with a GitLab Personal Access Token, and it passed.SKILL.mdownership ignores frontmatter50e2b92: digests are whole files. Every release from 0.16.1, the first whose deploy repaired frontmatter, shipped complete frontmatter, so what is on disk is what was shipped. Real v0.25.0SKILL.mdwith only its description edited: kept. Test red before..codex/skillsred before.findUnreadableProjectConfigreports it through an optional sink on detection (no other caller changes behaviour), andshareblocks withblockedBy: "config". Tests through the real loader.hook-dispatchin the referencesetupsay every public command and every flag.initsays ready when nothing deployedteamai-workflow/teamai-importReal-CLI rows on
50e2b92: every earlier row and the real v0.25.0 row, same results, plus a real v0.25.0 wikiSKILL.mdwith only its description edited: kept and named.Round on
50e2b92The reviewer's one finding, fixed in
78d2b2b:findUnreadableProjectConfigdropped a recorded error whenever detection went on to find a later candidate, so a broken partition config followed by a valid legacy.teamai/config returnednulland thesharegate answered with the fallback's team; and an existing config file that was empty or unreadable returned without reaching the sink. It now reports the first unreadable file whether or not a fallback loads, and an empty or unreadable file is reported too. Two tests through the real loader and a real git repo (broken partition plus valid legacy config; empty config), both red on50e2b92. Real CLI: an empty.teamai/config.yamlmakesskill get shareexit 1 with "the teamai config on this machine could not be loaded" and an empty stdout.What main changed underneath the skills
The four commits between the previous base and
cd3e0e6were checked against every file underskill-data/and the stub, because a skill that documents behaviour main just changed would ship stale on the day it lands.projects:besideroles:onhooks.yaml,mcp.yaml,env.yaml;Synced 1 of 3 env variable(s); doctor filtermanage-admin.mdlists theteamai mcpandteamai envcommands and says the definitions travel with the team repo; it never described the YAML entries or their per-entryroles:, so there is no half-sentence to complete. No Commander option or command was added, andcommands-reference.test.tsconfirmscommands.mdstill matches the table.qoder-cn, user root~/.qoder-cnskill-dataare the--agentexamples that come from Commander's own help text). In behaviour, the stub deploys and the prune runs there like everywhere else, per the row above.buildHint, relays throughrelayWhenHiddenmanage-admin.mddescribes the nudge's kill switch and resolution order, which #720 did not change.gitlabrow — now run, against a real private gitlab.com project (provider: gitlabinteamai.yaml, built CLI, isolated HOME, Claude and Codex installed, the pre-stub trees and a member's own file seeded). The project was created for the run and deleted after it:One detour worth recording, because it is easy to read as a gap in this row.
glab's stored gitlab.com credential is an OAuth token:GET /api/v4/useris 401 with aPRIVATE-TOKENheader and 200 withAuthorization: Bearer, andsrc/providers/gitlab/gitlab-api.tsonly ever sendsPRIVATE-TOKEN. That affectsteamai initagainst GitLab and nothing else — it is the one command in the flow that calls the GitLab API to resolve and clone the repo. It does not affect the row above: everything this PR changes runs after the repo is on disk (pull, the prune,skill get/path/show/list,push,recall,uninstallread~/.teamai/config.yamland the local clone, and touch no GitLab API). The team repo here is a real private gitlab.com project withprovider: gitlab, reached over the network by git exactly asinitwould have left it, so the row is complete for what this PR changes.The
initcall itself against GitLab needs a Personal Access Token withapiscope rather than aglabOAuth login. We ranteamai initagainst GitLab with a GitLab Personal Access Token, and it passed. Two follow-ups, neither of them this PR's: a member who authenticatedglabinteractively hits an error that says "Set the GITLAB_TOKEN environment variable" without telling them their existing login is the wrong kind of token (worth an issue — accept a Bearer token, or detect the glab login and say so); widening this PR into GitLab auth would repeat the mistake the prune set was blocked for.Found while running it, not part of this PR:
teamai inithas no non-interactive mode. With no terminal it starts the GitHub device-code flow and waits for the browser until the deadline expires, about five minutes, then exits 1. In CI or a headless agent that is a hung job with a log that does not say which credential is missing. Worth its own issue: fail fast when stdin is not a TTY, and say which token to set.Merge Danger
Door: two-way, including what it deletes.
Reverting restores the packaged trees and the next pull redeploys them, so the code is reversible. The prune removes, from every installed agent on the member's machine, the files earlier releases packaged under
team-wiki-codebase,teamai-share-learningsandteamai/references/. Only at the content a release shipped there: see the next paragraph. Leaving the unedited ones behind would keep the token cost this change exists to remove.Ownership is proven by path and content.
PACKAGED_SKILL_DIGESTSrecords the sha256 of every blob a release shipped under those trees (all 100 tags through v0.25.0 andmainbefore the stub: 42 versions across 21 paths, whole files, frontmatter included; the deploy repaired frontmatter from 0.16.1 on, but everySKILL.mdthose releases shipped was already complete). The copies came from the npm tarball byte for byte, so an unedited one matches; an edited file, a member's own skill under a legacy name, or a root TeamAI never managed becausetoolPathsorHERMES_HOMEmoved does not, and stays with its directory. Every pruned file is still copied to~/.teamai/removed-skills/<run>/<base>/<tool>/<skill-root>/<skill>/— the machine's home, never the tool's base directory, which under project scope is the repo root — before it is removed — outside every agent directory, so nothing reads it back as a skill. A file whose copy fails is kept rather than removed, and named at warn level: a backup that did not happen must not authorise the delete. The path carries the run, the base directory and the skill root because none is unique on its own — two pulls land on the same day,inheritUserScopedeploys the user base and then the project base in one process, and Codex prunes the same skill name from both.codex/skillsand the shared.agents/skills. Only retired paths are archived: the stub is rewritten on every session start, so archiving it would file an identical copy per session forever. A link on any component between the scope root (home, or the project root) and the skill directory —COPILOT_HOMEincluded — is refused outright — neither pruned nor written through, link and target untouched — by pull, deploy anduninstallalike. Checking the skill directory alone would miss the common shapes,~/.claude/skillsor~/.config/opencodelinked at a dotfiles checkout, where every directory under the link is real. Components at or above the base are not checked: a home directory under a link is ordinary. The cost is a member whose whole~/.claudeis a link (stow, chezmoi): the stub is not deployed and the legacy trees stay, with a warning on each pull naming the path. A file that is archived but cannot then be deleted is reported rather than left to a debug line.Nothing else is deleted. The digest table excludes
teamai-wiki(0.13.0, 0.16.x — it predates the trees this migration is about, and widening a destructive set is its own change); it covers seventeamai/references/*.md,provider-tgit.mdfirst shipped in 0.25.0. Every file removed is provably one the CLI wrote, by content; a file a member added beside them is kept, its directory with it, andpullnames that directory and says why. Excluded agents are not touched at all, as theenabledAgentswhitelist documents, and only Codex's own pass reaches Codex's shared.agents/skills.Blast Radius: every agent directory on every member's machine.
The routing rests on one description. With a single deployed skill, the stub's
descriptionis the only text an agent sees at selection time, so it carries the triggers the wiki and share skills used to own, in English. If an agent triggers the skill and skips the command it names, it holds a 2.5 KB stub and nothing else;allowed-toolspre-approves only theteamai skill …commands, so the one command it asks for runs without a prompt and nothing else is pre-approved from the always-loaded unit.Two more worth naming. Bash has to be available: an agent that cannot run a command reads nothing, and
teamai skill pathgives it a directory instead. And the content stops being greppable inside the agent directory, whichteamai skill get --allandskill pathcover.