Skip to content

fix(init): refuse provider logins without a terminal instead of hanging (#711) - #713

Merged
jeff-r2026 merged 5 commits into
Tencent:mainfrom
SaulMoro:fix/711-init-unattended
Sep 23, 2026
Merged

jeff-r2026 merged 5 commits into
Tencent:mainfrom
SaulMoro:fix/711-init-unattended

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

teamai init with no provider session and no terminal sat inside a browser login for about five minutes, then exited with the provider's error and no hint of the missing credential (#711). The non-TTY guard lived only in utils/prompt.ts; a provider login is a child process that owns the terminal, so it never went through that guard.

 init
   provider.authenticate()
-    github   gh auth login --web   stdio inherit   blocks until the device-flow deadline
-    tgit     gf auth login         stdio inherit   blocks
-    cnb      cnb login             stdio inherit   blocks
+    github   not interactive? throw "Export GITHUB_TOKEN (or GH_TOKEN)…"   else gh auth login
+    tgit     not interactive? throw "Run `gf auth login` first…"           else gf auth login
+    cnb      not interactive? throw "Export CNB_TOKEN…"                    else cnb login
     gitcode  already guarded, the pattern copied here
     gitlab   already throws naming GITLAB_TOKEN
   provider.cloneRepo()
-    git clone                      may prompt for a password or open a credential helper dialog
+    git clone                      terminal prompt, askpass and GCM all closed by the CLI
+                                   when not interactive (utils/git-env.ts); ssh keeps
+                                   whatever core.sshCommand each repo configured

 utils/prompt.ts
-  process.stdin.isTTY                 at every prompt
+  isInteractive() = stdin.isTTY && !CI && !TEAMAI_NONINTERACTIVE

One predicate, isInteractive(), now answers "can a person reply?" for every prompt, the five isTTY checks with prompt semantics (init.ts agent picker, update.ts update prompt, gitcode token paste, local-agent.ts project binding, and the three helpers in prompt.ts), the three provider logins, and the git environment. That git environment covers git's own questions only — the terminal prompt, the askpass dialog and the credential manager window. ssh is not covered: its batch flag is only reachable through GIT_SSH_COMMAND, which would override whatever core.sshCommand each repository configured, so an ssh remote can still ask for a key passphrase or an unknown-host confirmation (see Notes for how to close it per repository). Two init prompts that have no default now fail naming what replaces them: --role <id> with the valid ids when the team has several roles, and the repo argument or --repo when none was given. The six isTTY checks that read a hook payload from stdin keep their own meaning and are untouched. No new flag: a CI runner already exports CI, and an agent sandbox that allocates a pseudo-terminal sets TEAMAI_NONINTERACTIVE=1. --force keeps its meaning.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes
  • CI=true npx vitest run: 3786 tests pass, 1 skipped, no failures, three consecutive runs on the rebased branch (five before it), launched the way CI does (SHELL=/bin/bash). Worth knowing for anyone running it from zsh: detectShellProfile reads SHELL before its platform branch, so three win32 cases in shell-profile.test.ts resolve .zshrc and fail under a zsh login shell. That is a pre-existing dependency in a file this PR does not touch — CI's four matrix legs are green — so it is left for its own PR rather than widened into this one.
  • npx vitest run --config vitest.e2e.config.ts: 40 files, 174 tests pass, 26 skipped (live provider tests without credentials). Run on this exact source; the rebase that followed carried only skill documentation.
  • Review rounds 1-3 (see the PR comments): git-env.test.ts (5 cases, including the one that pins GIT_SSH_COMMAND as never set) and the corrected TGit guard assertion in gf-cli.test.ts.
  • Added tests: prompt-interactive.test.ts (predicate and every prompt under CI=true on a TTY), guard cases in github-provider.test.ts, gf-cli.test.ts, cnb-login-host.test.ts (each asserts the login is never spawned and the message names the variable), and e2e/init-unattended.test.ts (fake gh on PATH whose auth login sleeps). The e2e test needs no network or secrets, so ci.yml runs it in the build job next to the OpenCode recall test, on every PR.

Evidence

Reproduction from the issue: fake gh with no session and a sleeping auth login, isolated HOME, no token, stdin closed.

  • Before (origin/main at 2c0ae96, killed after 8 s; the real gh takes about five minutes to reach exit=1):
    ℹ Not logged in — starting authentication
    ℹ Starting GitHub authentication via gh CLI...
    ! First copy your one-time code: XXXX-XXXX
      Open this URL to continue in your web browser: https://github.com/login/device
    <still running after 8s>
    
    After:
    ℹ Not logged in — starting authentication
    ✖ Authentication failed: GitHub authentication unavailable without a terminal. Export GITHUB_TOKEN (or GH_TOKEN) with "repo" scope, or run `gh auth login` in an interactive shell first.
    exit=1 after 0s
    

Real CLI, end to end

Built CLI, isolated HOME with the four agent roots present, CI=true, stdin closed, throwaway private GitHub repo (deleted afterwards). Paths redacted.

1. init, GitHub, no credential            exit=1 in 0s, message names GITHUB_TOKEN
2. init, GITHUB_TOKEN exported,
   --agent claude,codex,codebuddy,opencode exit=0 in 8s, hooks written for all four
3. skills landed                          teamai/ and team-wiki-codebase/ under
                                          .claude, .codex, .codebuddy, .config/opencode
4. pull                                   exit=0, already up to date
5. doctor                                 exit=0, All checks passed
6. re-init without --force                prompt answers itself: "Aborted. Existing config is unchanged." exit=0 in 0s
7. git children (wrapper on PATH logging the variable, ambient value removed first)
     stdin closed                         every git call: GIT_TERMINAL_PROMPT=0
     pseudo-terminal (script) + CI=true   every git call: GIT_TERMINAL_PROMPT=0
     pseudo-terminal, no CI (control)     every git call: GIT_TERMINAL_PROMPT=<unset>
8. uninstall --force                      exit=0
9. init --self (provider git) with the four agents, business repo on GitHub
                                          exit=0, .claude .codex .codebuddy .opencode .teamai created,
                                          "[teamai] Initialize single-repo mode" committed on main

gitlab was exercised against a live instance with GITLAB_TOKEN. Not run: the tgit / cnb logins against their real CLIs — their guards sit in front of the spawn, the TGit path was driven with a fake gf that reproduces the hang, and both are covered by the unit tests above; the message text is the only thing a live run would add.

Related Issues

Closes #711

Notes for Reviewers

Door: two-way. Revert restores the previous behaviour with no data or config migration; the CLI writes nothing new.

Blast radius: prompts.

  • CI=true now makes every prompt take its default even on a TTY. GitHub Actions and most CI services export it, so a workflow that today types answers into an interactive teamai init on a pty would start getting defaults instead. That is the intended behaviour and the documented way to run unattended, but it is the one observable change for existing users.
  • Each git variable is set only when the CLI's own environment lacks it, so a caller's explicit value wins. All three name a prompt and nothing else, so one value is right for every repository a run touches. GIT_SSH_COMMAND is deliberately not among them: it overrides core.sshCommand for every later git operation, so a process-wide value would push the launch directory's key or wrapper onto the managed team repo, or suppress the one that repo configured (fix(init): refuse provider logins without a terminal instead of hanging (#711) #713 review round 3). An ssh remote that would still ask is closed per repository (git config core.sshCommand 'ssh -o BatchMode=yes') or per run (GIT_SSH_COMMAND); with stdin closed, ssh's own tty read hits EOF and fails in about a second anyway.
  • e2e.test.ts keeps its GitHub-only skip for the sandboxed init test. The comment there described a permanent hang, which this PR removes. What remains is that gf auth whoami reads only gf's own credential store, which an isolated HOME loses; the TGIT_TOKEN the test exports reaches neither (the PAT is REST-API-only, and git.woa.com's git endpoint rejects it too). A TGit run would now fail fast rather than hang, but it still cannot pass, so the skip stays with a corrected comment.
  • Two judgement calls from the review left as they are: the provider guard blocks each name their own credential (a shared helper would give one wording, but the repo has no shared provider-auth helper and TGit's remedy is not a token at all), and the isInteractive mock is added to the three test files whose code paths reach it rather than to all eleven mocks of prompt.js.
  • Docs updated: docs/usage-guide.md and .zh-CN.md (the "fully non-interactive init" paragraph now names the token variable per provider and the two environment variables), skills/teamai/references/setup-admin.md, CHANGELOG.md. README untouched.

@jeff-r2026 jeff-r2026 self-assigned this Sep 22, 2026
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/providers/tgit/gf-cli.ts:249 — Exporting the suggested TGIT_TOKEN cannot make unattended init succeed. Authentication still relies exclusively on gf auth whoami, and cloning explicitly ignores TGIT_TOKEN because the PAT is REST-only. The command therefore throws the same “Export TGIT_TOKEN” error even when it is already exported. Either implement a complete PAT-based path or instruct users to establish gf/~/.netrc authentication; also correct both usage guides.
  • [P1 blocking] src/index.ts:22 — GIT_TERMINAL_PROMPT=0 only disables Git’s own terminal username/password fallback. It does not prevent credential helpers such as Git Credential Manager from opening interactive UI, nor SSH from prompting through /dev/tty. Generic-provider clones can therefore still wait up to their 180-second timeout, contradicting the documented “never waits” guarantee. Suppress those interactive paths too or narrow the guarantee and coverage.

The PR description includes a detailed test plan and real-CLI end-to-end record, so no testing-description issue.

SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 22, 2026
…at works

Review follow-up on Tencent#713.

- utils/git-env.ts: GIT_TERMINAL_PROMPT=0 only closed git's own terminal
  question. The askpass chain (GUI dialog), ssh's passphrase / unknown-host
  question through /dev/tty, and Git Credential Manager's window each still
  parked an unattended clone until the 180s timeout. All four are now closed
  together (GIT_ASKPASS=echo, GIT_SSH_COMMAND='ssh -o BatchMode=yes',
  GCM_INTERACTIVE=never), each only where the caller set nothing.
- tgit: the guard suggested exporting TGIT_TOKEN, which cannot make an
  unattended run succeed — the PAT is REST-API-only and git.woa.com's git
  endpoint rejects it, so `gf auth whoami` still fails and the clone still has
  no credential. The message now names `gf auth login` (whose stored credential
  is the one that works) and says why the token is not it. Docs follow.
- local-agent: keep askViaTty's non-interactive decline synchronous. Awaiting
  the prompt module's import before declining shifted hook-path timing enough
  to break the once-per-session binding hint (local-agent.test.ts).
@SaulMoro
SaulMoro force-pushed the fix/711-init-unattended branch from c2b0872 to 2dc23c2 Compare September 22, 2026 10:58
@SaulMoro

Copy link
Copy Markdown
Collaborator Author

Both P1 findings fixed, plus a regression this PR had introduced that the full suite caught. Rebased on origin/main (9ce8a0e) — one conflict in src/index.ts against the new notifyWebhook block, resolved by keeping both.

P1 — TGIT_TOKEN cannot make an unattended run succeed

Correct, and the message was sending people after a credential that provably cannot work: the PAT is REST-API-only (rest-auth.ts tgitGitCloneUrl rejects it for clone by design), and ensureAuthenticated only ever reads gf auth whoami. So the guard now names the only credential that does work — the one gf auth login stores — and says why the token is not it.

before  Export TGIT_TOKEN (a TGit Personal Access Token), or run `gf auth login` …
after   Run `gf auth login` in an interactive shell first — this machine then reuses
        the credential it stores (TGIT_TOKEN is REST-API-only and cannot clone).

Corrected everywhere the same claim appeared: docs/usage-guide.md, docs/usage-guide.zh-CN.md, skills/teamai/references/setup-admin.md (both the non-interactive paragraph and the "Headless/CI only" aside in Step 3), skills/teamai/references/join-member.md, CHANGELOG.md, and the comment in e2e.test.ts that repeated it. gf-cli.test.ts now asserts the message points at gf auth login, not at a token.

P1 — GIT_TERMINAL_PROMPT=0 leaves three other doors open

Also correct. Rather than narrow the guarantee, all four are closed together in the new src/utils/git-env.ts, each only where the caller set nothing:

variable door it closes
GIT_TERMINAL_PROMPT=0 git's own username/password question on the terminal
GIT_ASKPASS=echo the askpass chain (GIT_ASKPASS → core.askPass → SSH_ASKPASS) — on a desktop, a GUI dialog
GIT_SSH_COMMAND='ssh -o BatchMode=yes' ssh's key passphrase and unknown-host questions, asked through /dev/tty past every git setting
GCM_INTERACTIVE=never Git Credential Manager's own window, which no git variable reaches

src/index.ts now calls applyNonInteractiveGitEnv() in place of the single assignment. One trade-off worth naming: GIT_SSH_COMMAND takes precedence over core.sshCommand, so an unattended run that needs its own ssh invocation must export GIT_SSH_COMMAND (the guard then leaves it alone). ~/.ssh/config is unaffected — ssh still reads it.

New git-env.test.ts covers the four cases: no terminal → all four set; TTY under CI → set; a person at a terminal → nothing set, so their credential helper still works; a caller's explicit value survives variable by variable.

Regression this PR introduced (found while verifying)

local-agent.test.ts "emits hint only once per sessionId" failed 3/3 isolated runs at c2b0872 and passed 3/3 at its parent. Cause: askViaTty awaited import('./utils/prompt.js') before declining, where the old code returned null synchronously on the hook path. Restoring the synchronous decline (static isInteractive import, dynamic import only when it will actually ask) makes it pass 4/4 isolated and 3/3 full-suite. The test remains timing-sensitive — I saw it flake once in ~5 full runs after the fix and could not reproduce it in six subsequent full runs, on this branch or with main's local-agent.ts — so it is worth hardening separately, not in this PR.

Test plan (re-run after the rebase)

  • npx tsc --noEmit
  • CI=true npx vitest run — 3781 pass, 1 skipped. The 3 failures are shell-profile.test.ts Windows cases on this macOS host (/var vs /private/var, zsh default), in a file this PR does not touch; same 3 on origin/main.
  • npx vitest run --config vitest.e2e.config.ts — 40 files, 174 tests pass, 26 skipped.

Real CLI, end to end (built from 2dc23c2)

Isolated HOME, a git wrapper on PATH recording the environment every git child inherited, throwaway private GitHub repo, ambient GIT_ASKPASS / SSH_ASKPASS stripped first. Paths redacted.

1. init, GitHub, no credential, stdin closed   exit=1 in 1s, message names GITHUB_TOKEN
2. init, GITHUB_TOKEN exported                 exit=0 in 8s, repo cloned, hooks written
3. init on a pseudo-terminal, CI=true          exit=0 in 12s  ← pty + CI still counts as unattended
4. pull / doctor / uninstall --force in (2)    exit=0 / exit=1 (sandbox has no project-scope
                                               .claude dir — unrelated to this PR) / exit=0
5. init, TGit, fake gf (login sleeps 300s),
   TGIT_TOKEN exported                         exit=1 in 0s, message names `gf auth login`,
                                               login never spawned
6. git children in 1–5                         GIT_TERMINAL_PROMPT=0 GIT_ASKPASS=echo
                                               GIT_SSH_COMMAND='ssh -o BatchMode=yes'
                                               GCM_INTERACTIVE=never
7. interactive control (pty, no CI)            teamai sets none of the four

Real clones under exactly that environment, to show the new variables close prompts without breaking credentials that work:

https, osxkeychain helper, private repo   exit=0 in 1s   ← helper path untouched
ssh, BatchMode, private repo              exit=0 in 2s   ← agent/key auth untouched
https, wrong token                        exit=128 in 1s
https, no credential at all               exit=128 in 0s  "fatal: unable to get password from user"

Not run: gitlab and the cnb / tgit logins against their real CLIs (no credentials for a live instance here). Their guards sit in front of the spawn and are covered by unit tests; the TGit path above used a fake gf that reproduces the hang.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/utils/git-env.ts:32 unconditionally sets GIT_SSH_COMMAND for unattended runs, which overrides existing global/repository core.sshCommand configuration. Valid setups relying on a custom key, SSH binary, or wrapper will fail during init, pull, or push. Add batch mode without replacing the configured SSH command.
  • [P1 blocking] PR description, Test Plan: CI=true npx vitest run is checked as passing but explicitly reports three failures. The trusted repository rules require every Test Plan item to actually pass before PR; provide a clean full-suite run or fix the failures.

The description otherwise contains a sufficiently detailed real-CLI end-to-end verification record.

SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 22, 2026
…ng it

Review follow-up on Tencent#713.

GIT_SSH_COMMAND overrides core.sshCommand rather than extending it, so setting
it blindly dropped a configured custom key, ssh binary or wrapper and left the
run unable to authenticate at all. The value is now composed: read
core.sshCommand and append `-o BatchMode=yes`, or use plain `ssh` when nothing
is configured. A command that already decides BatchMode is left alone, and the
config read is skipped entirely when the caller set GIT_SSH_COMMAND.

Test isolation, so the suite's own result can be trusted:

- shell-profile.test.ts: the three Windows cases never stubbed SHELL, and
  detectShellProfile reads it before the platform branch — a suite run from a
  zsh login shell resolved .zshrc and failed them without ever reaching the
  Windows branch. CI runners use bash, which is why only local runs saw it.
- local-agent.test.ts: the once-per-session binding-hint markers live in
  os.tmpdir() under one shared key, so a leftover marker decided whether the
  next test emitted a hint, and concurrent runs competed for the same paths.
  Each test now gets its own temp directory, which makes the markers per-test
  by construction.

Full suite: 3788 pass, 0 failures, five consecutive runs.
@SaulMoro

Copy link
Copy Markdown
Collaborator Author

Round 2 — both findings fixed in 9c22d74. Branch is already on top of origin/main (9ce8a0e), no rebase needed.

P1 — GIT_SSH_COMMAND replaced core.sshCommand

Right, and the note I left in round 1 ("callers who need their own ssh must export GIT_SSH_COMMAND") put the burden on the wrong side: a deploy key configured in core.sshCommand is a working setup that this PR would have broken. Batch mode is now appended to what is configured instead of replacing it.

core.sshCommand unset                          → ssh -o BatchMode=yes
core.sshCommand = ssh -i ~/.ssh/deploy_key -F … → ssh -i ~/.ssh/deploy_key -F … -o BatchMode=yes
core.sshCommand already names BatchMode         → left exactly as configured (yes or no is the caller's call)
GIT_SSH_COMMAND exported by the caller          → untouched, and the config read is skipped entirely

Cost: one git config --get core.sshCommand per unattended run, and only when GIT_SSH_COMMAND is unset. It is the last thing applyNonInteractiveGitEnv() does, so the three cheap variables are set even if git is missing from PATH (then there is no git child to protect either, and the fallback is plain ssh).

Verified with the real CLI — a git wrapper on PATH recording what each git child inherited, isolated HOME carrying the core.sshCommand under test:

none        SSH_COMMAND=ssh -o BatchMode=yes
configured  SSH_COMMAND=ssh -i ~/.ssh/deploy_key -F /etc/ci -o BatchMode=yes
caller-wins SSH_COMMAND=ssh

(The probe itself — git config --get core.sshCommand — is the one git child that runs before the variable exists. It opens no connection.)

P1 — the Test Plan claimed a pass while reporting three failures

Fair, and the explanation I gave for those three was wrong on top of it. They were not a /var vs /private/var comparison: detectShellProfile checks SHELL before the platform branch (deliberately — MSYS2/Cygwin zsh on Windows), and those three Windows cases were the only ones in the file that never stubbed SHELL. Run from a zsh login shell they resolved .zshrc and never reached the Windows branch at all. CI runners use bash, which is why all four matrix legs were green. They stub SHELL now, exactly as their siblings already did.

That left one more source of noise: the once-per-session binding-hint markers in local-agent.test.ts live in os.tmpdir() under a single shared key, so a leftover marker from one test decided whether the next test emitted a hint, and parallel runs competed for the same two paths. Each test now gets its own temp directory, which makes the markers per-test by construction.

CI=true npx vitest run → 3788 passed, 1 skipped, 0 failures, five consecutive runs, exit 0. npx tsc --noEmit clean, npx vitest run --config vitest.e2e.config.ts → 174 passed / 26 skipped. The PR description's Test Plan is updated to say this, and the stale round-1 claims in it (the TGIT_TOKEN line in the summary diff, the GIT_TERMINAL_PROMPT-only note) are corrected.

Docs updated for the composed ssh command: docs/usage-guide.md, docs/usage-guide.zh-CN.md, CHANGELOG.md.

One disclosure: shell-profile.test.ts and local-agent.test.ts are test-isolation fixes outside this PR's subject. I made them because the alternative was asking you to trust a suite that fails for reasons unrelated to the change — both are test-only, and neither weakens an assertion.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/utils/git-env.ts:47 — Do not derive a process-wide GIT_SSH_COMMAND from the launch directory. git config --get core.sshCommand reads the current repository’s local config, then the environment variable overrides core.sshCommand for every later Git operation. Running teamai pull from repo A can therefore force repo A’s SSH key/wrapper onto the managed team repo; conversely, the default value can suppress the managed repo’s own core.sshCommand. Set batch mode per Git child/repository instead of globally.
  • [P1 blocking] src/__tests__/shell-profile.test.ts:52 — The required unit-test run is not green, and the explanation is inaccurate. The PR description reports three failures while claiming shell-profile.test.ts is unchanged, but this PR modifies that file. The repository requires test-plan items to actually pass; provide a successful run or fix/revert the relevant changes before merge.
  • [P1 blocking] src/__tests__/local-agent.test.ts:28 — Remove unrelated test cleanup changes. The temp-directory isolation rewrite and the shell-profile test edits do not trace to preventing unattended provider-login hangs, violating the surgical-change rule. Move them to a separate PR unless they are demonstrably required for this fix.

The description does include substantial unit and real-CLI end-to-end evidence, so it is not missing an e2e record.

SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 22, 2026
…at works

Review follow-up on Tencent#713.

- utils/git-env.ts: GIT_TERMINAL_PROMPT=0 only closed git's own terminal
  question. The askpass chain (GUI dialog), ssh's passphrase / unknown-host
  question through /dev/tty, and Git Credential Manager's window each still
  parked an unattended clone until the 180s timeout. All four are now closed
  together (GIT_ASKPASS=echo, GIT_SSH_COMMAND='ssh -o BatchMode=yes',
  GCM_INTERACTIVE=never), each only where the caller set nothing.
- tgit: the guard suggested exporting TGIT_TOKEN, which cannot make an
  unattended run succeed — the PAT is REST-API-only and git.woa.com's git
  endpoint rejects it, so `gf auth whoami` still fails and the clone still has
  no credential. The message now names `gf auth login` (whose stored credential
  is the one that works) and says why the token is not it. Docs follow.
- local-agent: keep askViaTty's non-interactive decline synchronous. Awaiting
  the prompt module's import before declining shifted hook-path timing enough
  to break the once-per-session binding hint (local-agent.test.ts).
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 22, 2026
…ng it

Review follow-up on Tencent#713.

GIT_SSH_COMMAND overrides core.sshCommand rather than extending it, so setting
it blindly dropped a configured custom key, ssh binary or wrapper and left the
run unable to authenticate at all. The value is now composed: read
core.sshCommand and append `-o BatchMode=yes`, or use plain `ssh` when nothing
is configured. A command that already decides BatchMode is left alone, and the
config read is skipped entirely when the caller set GIT_SSH_COMMAND.

Test isolation, so the suite's own result can be trusted:

- shell-profile.test.ts: the three Windows cases never stubbed SHELL, and
  detectShellProfile reads it before the platform branch — a suite run from a
  zsh login shell resolved .zshrc and failed them without ever reaching the
  Windows branch. CI runners use bash, which is why only local runs saw it.
- local-agent.test.ts: the once-per-session binding-hint markers live in
  os.tmpdir() under one shared key, so a leftover marker decided whether the
  next test emitted a hint, and concurrent runs competed for the same paths.
  Each test now gets its own temp directory, which makes the markers per-test
  by construction.

Full suite: 3788 pass, 0 failures, five consecutive runs.
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 22, 2026
…override

Review follow-up on Tencent#713.

GIT_SSH_COMMAND is the only way to reach ssh's batch flag, and it overrides
`core.sshCommand` for *every* later git operation, not just the one the value
was derived from. Reading the launch directory's config and exporting it
process-wide therefore pushed that repo's key or wrapper onto the managed team
repo, and the plain default suppressed a `core.sshCommand` the managed repo had
configured for itself. Prompt suppression must not reach a repository's
transport, so the variable and the `git config` read are gone.

What remains is the three variables that name a prompt and nothing else, so one
value is right for every repository a run touches: GIT_TERMINAL_PROMPT=0,
GIT_ASKPASS=echo and GCM_INTERACTIVE=never.

An ssh remote that would still ask is now documented as the caller's to close,
per repository (`git config core.sshCommand 'ssh -o BatchMode=yes'`) or per run
(`GIT_SSH_COMMAND`). Measured first: with stdin closed, ssh's own tty read hits
EOF and fails in about a second, so the unattended paths this PR is about do
not depend on the flag.

Also reverts the shell-profile.test.ts and local-agent.test.ts isolation edits
from the previous round: neither traces to the unattended-login fix, so they
belong in their own PR.
@SaulMoro
SaulMoro force-pushed the fix/711-init-unattended branch from 9c22d74 to b7ae874 Compare September 22, 2026 11:56
@SaulMoro

Copy link
Copy Markdown
Collaborator Author

Round 3 — all three findings addressed in b7ae874, and the branch is rebased on origin/main (109b334), which also clears the conflict banner: #724 moved the TGit login section into provider-tgit.md, so my correction moved with it.

P1 — a process-wide GIT_SSH_COMMAND derived from the launch directory

Correct, and worse than the round-2 finding it was meant to fix. GIT_SSH_COMMAND overrides core.sshCommand for every later git operation, not only the repository the value came from, so teamai pull in repo A would have pushed A's key or wrapper onto the managed team repo, and the plain default would have suppressed a core.sshCommand the team repo configured for itself. Prompt suppression has no business reaching a repository's transport.

The variable and the git config read are gone. What stays are the three that name a prompt and nothing else, so one value is right for every repository a run touches:

GIT_TERMINAL_PROMPT=0   git's own username/password question on the terminal
GIT_ASKPASS=echo        the askpass chain, a GUI dialog on a desktop session
GCM_INTERACTIVE=never   Git Credential Manager's own window

ssh is now documented as the caller's to close, where the setting belongs: git config core.sshCommand 'ssh -o BatchMode=yes' on the repository, or GIT_SSH_COMMAND for one run. I measured the exposure before narrowing the promise — with stdin closed, ssh's /dev/tty read hits EOF and the connection fails in about a second, so the unattended paths this PR is about never depended on the flag:

ssh -i <passphrase-protected key> … on a pty, stdin closed   exited in 1s

End to end, with a git wrapper on PATH recording what each git child inherited and a core.sshCommand planted in the sandbox HOME:

core.sshCommand unset       GIT_SSH_COMMAND=<unset>   TERMINAL_PROMPT=0 ASKPASS=echo GCM=never
core.sshCommand = ssh -i …  GIT_SSH_COMMAND=<unset>   ← the repo's own setting governs
GIT_SSH_COMMAND=ssh         GIT_SSH_COMMAND=ssh       ← caller's value passes through

P1 — unrelated test changes

Agreed, and removed: the shell-profile.test.ts and local-agent.test.ts isolation edits are reverted. Neither traces to preventing an unattended provider-login hang, and I should not have folded them in.

P1 — the unit-test run

The description's Test Plan is rewritten to a run that is actually green: 3786 pass, 1 skipped, 0 failures, three consecutive runs on the rebased branch, launched the way CI launches it (SHELL=/bin/bash). CI's four matrix legs are green on the same source.

On the earlier failures, so the record is straight: they were never a /var vs /private/var comparison, and shell-profile.test.ts was unchanged when the first description claimed so — the round-2 edit came later, and is now reverted. The real cause is that detectShellProfile reads SHELL before its platform branch, so under a zsh login shell three win32 cases resolve .zshrc and never reach the branch they test. That is a pre-existing dependency in a file this PR does not touch; it is noted in the Test Plan and left for its own PR rather than widened into this one.

Coverage in this round

tsc --noEmit clean; unit suite as above; vitest run --config vitest.e2e.config.ts 174 pass / 26 skipped on this exact source (the rebase that followed carried only skill documentation). Real CLI, built from this branch: GitHub with no credential fails in 1s naming GITHUB_TOKEN; GitHub with GITHUB_TOKEN clones and completes; a pseudo-terminal with CI=true behaves as unattended; TGit with a fake gf whose login sleeps fails in 0s naming gf auth login; GitLab was exercised against a live instance with GITLAB_TOKEN; and the three ssh cases above. The cnb login against its real CLI is the one guard still covered only by unit tests — it sits in front of the spawn, like the others.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] Update the provider authentication reference. docs/providers.md:122 still states that teamai init runs gf auth login unconditionally, but this PR now refuses to do so without an interactive terminal. This violates the repository rule requiring every affected document to reflect behavior changes. The GitHub/CNB auto-login wording at docs/providers.md:77 and docs/providers.md:152 should likewise clarify the interactive-only condition.
  • [P2 non-blocking] Correct the PR summary’s claim that SSH BatchMode is closed. src/utils/git-env.ts:16 explicitly leaves SSH prompts enabled, so an unattended SSH clone can still wait for a key passphrase or host confirmation. The later Notes section describes this accurately, but the opening summary and “every git prompt” framing do not.

The PR description contains a detailed test plan and real-CLI end-to-end verification record; no testing-description blocker.

…ng (Tencent#711)

`teamai init` with no session spawned `gh auth login --web` (or `gf auth
login`, `cnb login`) with inherited stdio and waited for a browser device
flow nobody could complete, about five minutes for GitHub, then exited
with the provider's error and no hint of the missing credential.

Cause: the non-TTY guard lived only in utils/prompt.ts. A login is a child
process that owns the terminal, so it never went through that guard.

- `isInteractive()` in utils/prompt.ts: stdin is a TTY and neither `CI`
  nor `TEAMAI_NONINTERACTIVE` is set. Every prompt and the four
  prompt-semantics `isTTY` checks use it; the six hook-payload checks are
  untouched.
- github, tgit and cnb logins throw before spawning when not interactive,
  naming the token variable, the way gitcode already did.
- index.ts exports GIT_TERMINAL_PROMPT=0 when not interactive, so a
  missing clone credential fails at once instead of prompting or opening
  a credential helper dialog. An explicit caller value wins.
- e2e test with a fake `gh` whose `auth login` sleeps: exit 1 in under a
  second naming GITHUB_TOKEN, also under CI=true.

Closes Tencent#711
…at works

Review follow-up on Tencent#713.

- utils/git-env.ts: GIT_TERMINAL_PROMPT=0 only closed git's own terminal
  question. The askpass chain (GUI dialog), ssh's passphrase / unknown-host
  question through /dev/tty, and Git Credential Manager's window each still
  parked an unattended clone until the 180s timeout. All four are now closed
  together (GIT_ASKPASS=echo, GIT_SSH_COMMAND='ssh -o BatchMode=yes',
  GCM_INTERACTIVE=never), each only where the caller set nothing.
- tgit: the guard suggested exporting TGIT_TOKEN, which cannot make an
  unattended run succeed — the PAT is REST-API-only and git.woa.com's git
  endpoint rejects it, so `gf auth whoami` still fails and the clone still has
  no credential. The message now names `gf auth login` (whose stored credential
  is the one that works) and says why the token is not it. Docs follow.
- local-agent: keep askViaTty's non-interactive decline synchronous. Awaiting
  the prompt module's import before declining shifted hook-path timing enough
  to break the once-per-session binding hint (local-agent.test.ts).
…ng it

Review follow-up on Tencent#713.

GIT_SSH_COMMAND overrides core.sshCommand rather than extending it, so setting
it blindly dropped a configured custom key, ssh binary or wrapper and left the
run unable to authenticate at all. The value is now composed: read
core.sshCommand and append `-o BatchMode=yes`, or use plain `ssh` when nothing
is configured. A command that already decides BatchMode is left alone, and the
config read is skipped entirely when the caller set GIT_SSH_COMMAND.

Test isolation, so the suite's own result can be trusted:

- shell-profile.test.ts: the three Windows cases never stubbed SHELL, and
  detectShellProfile reads it before the platform branch — a suite run from a
  zsh login shell resolved .zshrc and failed them without ever reaching the
  Windows branch. CI runners use bash, which is why only local runs saw it.
- local-agent.test.ts: the once-per-session binding-hint markers live in
  os.tmpdir() under one shared key, so a leftover marker decided whether the
  next test emitted a hint, and concurrent runs competed for the same paths.
  Each test now gets its own temp directory, which makes the markers per-test
  by construction.

Full suite: 3788 pass, 0 failures, five consecutive runs.
…override

Review follow-up on Tencent#713.

GIT_SSH_COMMAND is the only way to reach ssh's batch flag, and it overrides
`core.sshCommand` for *every* later git operation, not just the one the value
was derived from. Reading the launch directory's config and exporting it
process-wide therefore pushed that repo's key or wrapper onto the managed team
repo, and the plain default suppressed a `core.sshCommand` the managed repo had
configured for itself. Prompt suppression must not reach a repository's
transport, so the variable and the `git config` read are gone.

What remains is the three variables that name a prompt and nothing else, so one
value is right for every repository a run touches: GIT_TERMINAL_PROMPT=0,
GIT_ASKPASS=echo and GCM_INTERACTIVE=never.

An ssh remote that would still ask is now documented as the caller's to close,
per repository (`git config core.sshCommand 'ssh -o BatchMode=yes'`) or per run
(`GIT_SSH_COMMAND`). Measured first: with stdin closed, ssh's own tty read hits
EOF and fails in about a second, so the unattended paths this PR is about do
not depend on the flag.

Also reverts the shell-profile.test.ts and local-agent.test.ts isolation edits
from the previous round: neither traces to the unattended-login fix, so they
belong in their own PR.
docs/providers.md still described `teamai init` as running `gh auth login`,
`gf auth login` and `cnb login` unconditionally. Each now happens only in an
interactive terminal; an unattended run fails at once naming the credential to
prepare (a token for GitHub and CNB, a prior `gf auth login` for TGit, since a
TGIT_TOKEN PAT is REST-API-only and cannot clone).
@SaulMoro
SaulMoro force-pushed the fix/711-init-unattended branch from b7ae874 to 5f33142 Compare September 22, 2026 12:02
@SaulMoro

Copy link
Copy Markdown
Collaborator Author

Round 4, both fixed in 3b4d24c (branch rebased on origin/main 9d8ed1f, so the conflict banner is clear again).

P1 — docs/providers.md. Missed file, and the rule is clear that it should have moved with the behaviour. All three auto-login descriptions now say the login only happens in an interactive terminal, and what an unattended run gets instead: GitHub and CNB name the token to export, TGit names a prior gf auth login and says why no token substitutes for it (TGIT_TOKEN is REST-API-only and git.woa.com's git endpoint rejects it). GitLab and GitCode already described token auth, so they are unchanged.

P2 — the summary's framing. Corrected. The opening paragraph now states the limit where a reader meets it, instead of leaving it to the Notes: the git environment closes git's own questions (terminal prompt, askpass dialog, credential manager window), and ssh is explicitly not covered, because its batch flag is only reachable through GIT_SSH_COMMAND and that would override each repository's core.sshCommand. The diff block in the summary says the same.

Docs-only change since the last round; npx tsc --noEmit clean.

@github-actions

Copy link
Copy Markdown

Findings

  • [P2 non-blocking] src/providers/github/gh-cli.ts:229 and src/providers/cnb/cnb-cli.ts:179 misdiagnose invalid exported tokens as missing credentials. When a token exists but verification fails, the new guard tells users to export the same variable again. Detect token presence and report that it was rejected, similar to the GitCode provider.
  • [P2 non-blocking] src/utils/git-env.ts:17 deliberately leaves SSH interactive. An initial SSH clone cannot rely on repository-local core.sshCommand because the repository does not exist yet; with a pseudo-TTY plus TEAMAI_NONINTERACTIVE=1, host-key or passphrase prompts can still hang. Apply -c core.sshCommand="ssh -o BatchMode=yes" to the individual clone invocation rather than globally overriding later operations.

The PR description includes a detailed test plan and real-CLI end-to-end verification, so no testing-description blocker.

@jeff-r2026
jeff-r2026 merged commit 94eb148 into Tencent:main Sep 23, 2026
10 checks passed
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 23, 2026
…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.
@SaulMoro
SaulMoro deleted the fix/711-init-unattended branch September 23, 2026 05:44
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 23, 2026
…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.
jeff-r2026 pushed a commit that referenced this pull request Sep 23, 2026
…very stub (#699)

* feat(skill): serve packaged skill content from the CLI

Add `teamai skill get <names...> [--full] [--all]` and `teamai skill path
[name]`, so an agent can read built-in skill content that always matches the
installed CLI version instead of a copy deployed into its skills directory.

`get` prints SKILL.md byte for byte, frontmatter included, with {SKILL_DIR}
resolved to the absolute packaged directory so documented script invocations
run as-is. `--full` appends references/ and templates/, walked recursively and
sorted by relative path, because our references nest one level deeper than the
flat layout agent-browser assumes.

Content goes to stdout and every diagnostic to stderr, so the output stays
byte-exact when piped. An unknown flag warns and continues; an unknown name is
fatal, since acting on the wrong skill is worse than a retry.

`skill list` gains the served catalog and `--json`; `skill show` resolves
packaged skills before the installed-agent fallback, which is what keeps it
working once the deployed unit becomes a stub. Legacy directory names resolve
as aliases.

Refs #678

* refactor(skills): move content to skill-data and deploy a single stub

Agents now receive one file: `skills/teamai/SKILL.md`, a discovery stub of
about 2 KB whose description carries the triggers of every workflow and whose
body holds the commands that load them. The workflow content moves to
skill-data/{core,share,wiki}, which is never deployed and is printed by
`teamai skill get`.

Before this, `deployBuiltinSkills` copied three whole trees — 176 KB — into
every installed agent on every pull, so the text an agent read could disagree
with the CLI it documented until the member ran a pull, and a machine with ten
agents held ten copies. skills/ keeps its meaning ("everything here is
deployed"), which is what lets BUILTIN_SKILL_NAMES collapse to one name.

The stub is copied verbatim: no ensureSkillFrontmatter on the way out, so a
deployed copy that differs from the packaged one is a bug rather than a
variant. Recall no longer gates deployment, since the stub routes to every
workflow; the run-time gate for share lands with the pruning pass.

Uninstall learns the legacy directory names, which it would otherwise leave
behind on every machine that upgraded.

"skill-data" is added to package.json files, with a test that asserts it
through `npm pack`: without that entry every test still passes against the
repo and `skill get` serves nothing once installed from the registry.

Refs #678

* fix(skills): repair stale commands, broken refs and frontmatter

An audit of the three builtin skills found 60 defects. This fixes the ones that
survive the move to skill-data, and splits the two skills that were carrying
more than one job.

Stale CLI surface. The wiki skill advertised `teamai extract graph`, a command
that has never existed. The hand-written "ground truth" cheat sheet in the
teamai skill omitted 19 real commands while telling the agent that anything
missing from it could be checked with `--help` — which fails for the flags
`--help` hides. The cheat sheet is replaced by
`skill-data/core/references/commands.md`, rendered from the CLI's own command
table, with hidden flags marked as such. Two tests guard it: one regenerates
the file and diffs, the other resolves every `teamai …` string written anywhere
in skill-data against the command table and fails on an unknown command or
flag. That second test is the one that would have caught e151d43, 1ca43ac,
8bb0548 and 2ddb546 before they shipped; it carries a case proving it catches
`teamai extract graph`.

Paths. Everything the skills told an agent to read or execute assumed the skill
sat in the agent's own directory: `python3 scripts/scan_repo.py` from a cwd that
is the target repo, references cited by bare filename in two different
conventions, methodology paths handed to sub-agents inside input packets. All of
them now go through {SKILL_DIR}, which `skill get` resolves. The README template
nobody referenced is wired into the step that writes the knowledge-base README.

Frontmatter. None of the three skills declared allowed-tools, so the first
command of every flow hit a permission prompt. The wiki skill kept its trigger
words and prerequisites inside the description text; both move into the body.

Splits. `core` keeps what a daily user needs and `setup` takes day 0 and the
repo lifecycle, so the common path no longer carries ~500 lines of repo
creation. The wiki skill's phase procedures move into references/phases/, taking
its SKILL.md from 38.7 KB — larger than agent-browser's entire core — to 17 KB
with an index that says when to load each phase.

One contradiction is resolved in the author's text: the share skill mandated
that every generated document be written in Chinese, against global rule 1
("reply in the user's language") and this repo's own English rule. It now
follows rule 1.

Refs #678

* feat(pull): prune legacy builtin skill directories, gate recall at run time

Upgrading the CLI used to leave the pre-stub trees in place: cleanup skips
builtin names, and nothing else knew about them, so `team-wiki-codebase` and
`teamai-share-learnings` would sit in every agent directory on the machine
forever. Deployment now removes them first, in both the configured skills path
and Codex's shared `.agents/skills`. Unconditional, because those trees were
overwritten on every pull, so no local edit ever survived in them.

Recall moves from deploy time to run time. Before, `skipRecall` decided whether
the share skill reached the agent at all; with one stub routing to everything,
there is no directory to withhold, so `teamai skill get share` checks instead
and says what to enable. `--all` is exempt: an inventory dump is not an attempt
to run the workflow. With no team config to consult the gate fails open — a
fresh machine reading the docs gets the content rather than a refusal it cannot
act on.

deployBuiltinSkills drops its `skipRecall` option rather than keeping one that
no longer decides anything, and recall-toggle stops deleting a skill directory
it no longer owns.

Refs #678

* docs: align the nudge and the guides with CLI-served skills

`/teamai-share-learnings` was never a slash command of its own — it existed
because the directory was installed. The Stop-hook nudge now names `/teamai` and
carries `teamai skill get share` literally, so an agent can act on it without
having to infer the intent from the conversation. The five READMEs and both
usage guides follow.

Both guides gain the `skill get` / `skill path` commands and a short section on
why built-in skills are served rather than copied. `docs/designs/skill-serving.md`
records the contracts that are easy to break later: byte-for-byte output,
{SKILL_DIR} substitution, stdout/stderr discipline, recursive `--full`, the
run-time recall gate, the three drift guards, and when to retire
LEGACY_BUILTIN_SKILL_NAMES and the long-name aliases.

AGENTS.md and CLAUDE.md gain the rule that keeps this from rotting: skill-data
is treated like documentation, a behaviour change updates the affected skill,
commands.md is regenerated rather than edited, and new workflows go under
skill-data instead of into the stub.

Refs #678

* fix(skills): apply standards review findings

`teamai init` printed "Built-in skills (e.g. team-wiki-codebase) are ready to
use in your IDE now" seconds after deployment deleted that very directory. The
message now names the teamai skill and how it loads its workflows. Two comments
carrying the same stale name follow.

The five READMEs said different things: only the English one named the share
workflow and its command. All five now do.

`collectSupplementaryFiles` hand-rolled a recursive walk that
`listFilesRecursive` already does, including the ignore list that skips `.pyc`
and `__pycache__` next to the wiki's Python scripts. It calls the helper
instead. `listServableSkills` drops its fallback to `skills/`: a package without
`skill-data/` is broken, and serving the stub as if it were the content hides
that from the one error message built to report it.

Tests drop six non-null assertions for a helper that throws, per the repo's rule
against moving a compile-time error to run time.

AGENTS.md and CLAUDE.md record the exemption the branch created: skill content
printed by `skill get` keeps the language its author wrote it in, while the
command's own prompts, errors and listings stay English.

Refs #678

* fix(skills): apply spec review findings

Upgrading left the old references in place. Releases before the stub deployed
`skills/teamai/` with six reference files beside SKILL.md, and `teamai` is not a
legacy name to prune, so copying one file over that directory kept ~39 KB of
pre-stub instructions next to the new stub for good. Deployment now clears
everything the deployed unit does not contain before writing it, and a test
seeds the old layout to prove it.

Pruning reached neither reporting-only teams nor the Codex shared directory in
any test. The prune now runs before the reporting-only return, so a team that
switched to reporting-only still loses the stale trees, and the Codex
`.agents/skills` path is covered by a test. Excluded agents stay untouched, as
the enabledAgents whitelist documents.

The served content still routed to skills that no longer exist: five mentions of
`teamai-share-learnings` and one `/team-wiki-codebase --update`, which is the
rule this branch itself added being broken on arrival. One second-hop path
inside a sub-agent input packet was still relative.

The share skill's document template, frontmatter table and tag taxonomy move to
`references/doc-template.md`, taking the always-read body from 3 701 to 2 471
bytes. Two tests now assert what nothing guarded: every served skill's
frontmatter name matches its directory and declares allowed-tools.

A new e2e file runs the built CLI the way an agent does: every listed skill is
servable and byte-identical bar the resolved placeholder, the wiki scripts run
from the directory `skill path` prints, an unknown name exits 1 with empty
stdout, a hallucinated flag warns and still serves, and `--full` appends the
nested references in sorted order.

Refs #678

* fix(skills): prune only directories the CLI owned, gate every content path on recall

- LEGACY_BUILTIN_SKILL_NAMES drops teamai-workflow and teamai-import: they were
  reserved in the old guard set but never packaged, so a directory by either
  name is the user's own skill. Test: user-created skills with those names
  survive pull.
- The recall gate now covers skill get --all (blocked skill skipped, named on
  stderr), skill path (refused) and skill list --json (blockedByRecall, path
  null). skill list reads the flag from the catalog instead of re-checking.
- skill get [names...]: the positional is optional so --all is reachable from
  the real CLI; Commander used to fail with 'missing required argument'.
  Covered by the skill-serving e2e.
- commands-reference renders Commander's variadic marker (<names...>); the
  snapshot is regenerated.

* test(skills): drive the recall gate through the real CLI, guard the stub description budget

- skill-serving e2e: a HOME with a team whose recall is off; skill get share,
  --all, skill path share and skill list --json each withhold share, and all
  serve it after recall enable. The earlier HOME has no team config and fails
  open, so the gate was never exercised through dist/index.js.
- skill-content test: the stub description stays within 1024 characters.
- skill-commands-exist also scans the deployed stub.
- Docs and PR lead with content versioned with the CLI; the size numbers are
  measured (stub description 0.8 KB, body 1.3 KB; --full 32/36/115 KB).
- Content audit against origin/main: every file has a counterpart. Fixes:
  {SKILL_DIR} defined in core/setup/share where the references are listed, the
  wiki overview draws the served layout, team-wiki-codebase kept as a trigger
  word in the stub description.

* fix(skills): gate skill show on recall, prune Codex's shared dir only from Codex

Review follow-up on #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.

* fix(skills): one resolver for served skills, legacy names kept out of push, wiki in English

Review follow-up on #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.

* fix(skills): prune only files the CLI packaged, let local skills win by name

Review follow-up on #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.

* fix(skills): close the last recall bypass, uninstall Codex's shared stub

Review follow-up on #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`.

* fix(skills): uninstall by the same ownership rule as pull, quote {SKILL_DIR}

Review follow-up on #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.

* fix(skills): deploy the stub in reporting-only mode, drop the stale list alias

Review follow-up on #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 #678

* fix(skills): carry the TGit provider guide into the served setup skill

#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

* fix(skills): make the blocked catalog entry unrepresentable, drop unsafe 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.

* fix(skills): keep publishing a skill reachable when recall is off

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`.

* fix(skills): back up what the prune removes, so no edit is a one-way 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.

* fix(skills): let no backup failure authorise a delete, give each root 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.

* fix(skills): stop at a symlinked root, archive only what is retired

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.

* fix(skills): stop at the first link above a skill dir, report a half 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.

* fix(skills): route the nudge and skill publishing where they land, classify 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.

* fix(skills): pre-push review — gate the nudge on recall, keep bytecode out of the tarball, report a failed uninstall delete

A review of the whole branch against #678, #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 (#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.

* fix(skills): English-only served content, one link guard for every caller, 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.

* fix(skills): check every path component below the base for a link, in 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.

* fix(skills): withhold the share hint on read-only sources, route legacy 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.

* fix(skills): deploy and prune built-ins where the tool keeps its skills

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.

* fix(skills): check an external skills root for a link, keep config-load 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.

* fix(skills): walk the link guard from the scope root, so a linked COPILOT_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.

* fix(skills): keep generated documents in Simplified Chinese, fail on 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.

* fix(config): raise NotInitializedError only when the config file is missing

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.

* fix(skills): prove ownership by content, not path alone; retire the second 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.

* chore(skills): carry main's skill edits into the served copies after the rebase

#713 and #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 (#713).
- setup-admin.md: the /teamai share entry publishes a reusable skill; a
  session's learnings are automatic (#736), in English as the served text is.
#739's partial config mock is restored in skip-uninstalled-tools.test.ts.

* fix(skills): deploy before pruning, block share on an unloadable config, 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.

* fix(skills): whole-file ownership, stub before its references, no side 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.

* fix(config): report a broken higher-priority project config even when 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] teamai init cannot run unattended: provider login blocks for minutes when there is no terminal

2 participants