Skip to content

fix(mcp): resolve a ${VAR} only mcp.yaml names as the member's own secret (Tencent#1011) - #8

Closed
hsuchifeng wants to merge 2 commits into
mainfrom
claude/jeffr2026-issue-1011
Closed

hsuchifeng wants to merge 2 commits into
mainfrom
claude/jeffr2026-issue-1011

Conversation

@hsuchifeng

Copy link
Copy Markdown
Owner

Summary

Implements Tencent#1011: a team mcp/mcp.yaml server that references a personal token (X-Tapd-Access-Token: ${TAPD_ACCESS_TOKEN}) with no env/ in the repo. A pull from a shell that exports the variable installed the server; the session-start pull, which runs without the member's shell exports, then deleted it from .cursor/mcp.json, and the member had no place to keep the value except their shell profile.

main already has the Tencent#875 team-secrets design (teamai env set, per-team 0600 value stores, "a missing declared secret keeps the MCP entry"), but only for keys declared in env/secrets.yaml. This PR extends it to the undeclared case, with no new command or file format:

  • withMcpReferencedSecrets (src/resources/secrets.ts): a ${VAR} the scope's MCP servers reference that no secrets file declares and no env.yaml sets becomes an implicit secret declaration, attributed to the referencing mcp.yaml. resolveTeamEnv applies it, so every consumer sees it: env set accepts the key, buildVarTable resolves it from the member's team/machine store and then their environment, desiredMcpForTarget keeps the installed entry when the value is missing, and the "tapd: TAPD_ACCESS_TOKEN is not set. Run \teamai env set TAPD_ACCESS_TOKEN``" advisory names it.
  • env set uses the same resolution (one read of the team env instead of two), and its help text names the new case; commands.md regenerated.
  • Docs: docs/designs/team-secrets.md (+ zh-CN) gain "A variable only mcp.yaml names is a secret" and the "removed, as before" bullet is updated; docs/usage-guide.md (+ zh-CN) updated where they said unresolved variables skip the server.

Design choice vs the issue text: the issue proposes process.env before the personal store. The existing Tencent#875 resolution order is store first, then environment (so a value exported for another account does not silently win for this team); the implicit secret follows that same order rather than adding a second one. The issue's other asks are already met by the existing store (~/.teamai/secrets/teams/<hash>.json, mode 0600, hidden-input env set, never rewritten by pull, redacted from logs).

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

  • npm run lint passes

  • npx vitest run passes (full unit suite; the two mcp-reconcile cases that pinned "still removes a server whose missing variable is not declared as a secret" now assert the entry is kept)

  • Real-CLI e2e, npm run build && npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/mcp-member-token-1011.test.ts → 5/5 passed. It reproduces the issue with the real binary (user scope, provider: git bare remote, Cursor, no env/):

    1. TAPD_ACCESS_TOKEN=… teamai pull --force installs tapd into ~/.cursor/mcp.json
    2. teamai pull without the export keeps the entry and prints tapd: TAPD_ACCESS_TOKEN is not set. Run \teamai env set TAPD_ACCESS_TOKEN`andtapd: the entry an earlier pull wrote stays in cursor …`
    3. teamai env list lists it under Team secrets (1) as missing
    4. printf token | teamai env set TAPD_ACCESS_TOKEN --stdin → stored in ~/.teamai/secrets/teams/<hash>.json, mode 0600, not in mcp.json
    5. teamai pull --force with no export resolves the header from the store; the store file's content and mtime are unchanged; the token appears in neither the output nor ~/.teamai/debug.log

    The same test run against the pre-change build fails steps 2–5 (step 2 removes the server), so it pins the fix.

Related Issues

Implements Tencent#1011

Notes for Reviewers

Codex/Claude and the gitlab/github providers are not exercised here; the change is in scope resolution and applies to every tool and provider the same way, so that coverage is left to CI.


Generated by Claude Code

…cret

A `${VAR}` an MCP server references that no secrets file declares and no
env.yaml sets is a personal token the team repo cannot hold. It is now
treated as a declared secret everywhere: `teamai env set VAR` stores the
member's value, a pull resolves it from their stores and environment, a
pull that cannot see the value keeps the entry an earlier pull wrote
instead of deleting the server, and the missing-secret line names it.
@hsuchifeng hsuchifeng self-assigned this Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Code Erosion Report

Informational — never blocks the merge. scb-check==0.2.0 SlopCodeBench metrics; method & caveats in docs/ci-code-erosion.md.

Metric Value Human band Agent band Reading
Verbosity* 0.073 0.15 0.33 human
Erosion 0.685 0.31 0.68 agent
Cognitive erosion 0.867 – – –

Scanned 304 files / 60655 SLOC · high-CC functions 421/4840

* On TypeScript, verbosity is partial and the bands are Python-calibrated.

Rule hits (TS verbosity layer)

Standalone ast-grep, separate from the number above.

Rule Hits
unnecessary-else-after-return 32
empty-catch-block 5
redundant-ternary-same 1

38 distinct lines flagged across src/.

Copy link
Copy Markdown
Owner Author

CI note on the Codex PR Review check: it fails before the review starts (codex-action cannot read /home/runner/.codex/<run-id>.json, "Failed to read server info"), which is the action's local proxy not coming up because this fork has no OPENAI_API_KEY / OPENAI_RESPONSES_API_ENDPOINT repository secrets. It fails the same way on every PR here and touches nothing in this diff, so there is no fix to port into the branch; it needs the secrets on the fork, or the review run on Tencent/teamai-cli.

The Lint & Test failure on 3ecb167 was the doctor-mcp-delivery test that c3d004a rewrote; the matrix is re-running on that head.


Generated by Claude Code

@hsuchifeng
hsuchifeng force-pushed the claude/jeffr2026-issue-1011 branch from c3d004a to b59a689 Compare October 9, 2026 15:08
@hsuchifeng hsuchifeng closed this Oct 13, 2026
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.

1 participant