feat: block First Tree chat tools in Feishu-bridged chats

[liuchao-001]

What changed

A First Tree chat bridged to a Feishu conversation lives in Feishu — the humans in it read the Feishu group, not the web app. An agent could still answer with chat send / chat ask / chat invite, and those writes landed where nobody on the other side could see them. The reply was silently lost.

This adds the agent-scope counterpart of assertWebMutableChat, which has covered the Web/user scope for a while.

Server (blocking, 403 + code: "FEISHU_CHAT_AGENT_WRITE_FORBIDDEN")

• POST /api/v1/agent/chats/:chatId/messages — covers chat send and chat ask (same route; chat ask is just format: "request").
• POST /api/v1/agent/chats/:chatId/participants — covers chat invite.

The shared helper is packages/server/src/api/agent/feishu-chat-guard.ts. Authority is im_chat_bindings filtered to status = 'active', not chats.metadata.source — that label stays "feishu" after a binding detaches. The Web helper predates the status column and does not filter; this one does, so a detached chat becomes an ordinary First Tree chat again.

The error message names the alternative rather than only refusing: record the delivery with feishu intent, then send with the official lark-cli --as bot.

Client-side preconditions (advisory, exit code 2, FEISHU_CHAT_CONTEXT)

chat create and chat open are refused from inside a bridged session, in apps/cli/src/core/feishu-chat-context.ts.

The bridge-collision hazard

This is the part worth reviewing closely. The Feishu bridge's own outbound delivery (POST /api/v1/agent/feishu/intents) calls the same messageService.sendMessage that chat send calls, with the identical source: MESSAGE_SOURCES.CLI and the agent's own senderId. A guard placed inside sendMessage would have broken the bot's own replies — the exact behaviour this PR exists to protect.

The two are distinguishable only by their route and by trusted in-process options (allowFeishuMetadata / allowRecipientlessSend) that never cross HTTP. So the guard lives in the route/adapter layer and is applied per-route, and the module header says so.

feishu-cli-preflight.test.ts gains the regression that pins this: on one bridged chat, the intent route still delivers (message stored, silent notify=false fan-out intact) while the agent chat route on that same chat returns 403.

The runtime-notice exemption

runtime/runtime-notice.ts::postProviderFailureRuntimeNotice posts through the same agent message route when a provider terminally fails. That row is the only in-product signal an operator gets that the agent could not run at all; suppressing it on a Feishu chat would make the chat look merely idle. It is exempt.

The exemption requires both purpose: "agent-final-text" and metadata.runtimeNotice === true. The silent delivery profile alone is not sufficient, because deliberate agent sends may also carry purpose — there is a test for that.

Note this is a confusion rail, not an authorization boundary: runtimeNotice is client-supplied metadata that the server does not strip, so an agent determined to spoof it could. That is already true of the whole agent surface and is not made worse here.

Why chat create is CLI-side only

The server never learns which chat the caller is sitting in: there is no field for it in createTaskChatSchema, no header carries it, and chat/create.ts does not read FIRST_TREE_CHAT_ID. There is nothing to gate on server-side. chat open is worse — it runs on the user scope and starts an interactive REPL, so the server cannot tell an operator terminal from an agent session.

Rather than sniff the stale metadata.source label, both read a new ChatDetail.externalChannel field, populated by the agent chat-detail route from the same live im_chat_bindings state the write boundary enforces. That keeps the advisory rail and the real boundary from disagreeing — with metadata.source, a detached chat would have been refused locally while the server happily accepted it.

Both checks fail open: no chat context, an older server that omits the field, or a failed lookup all let the command proceed. The server stays the boundary for every route that can carry a chat id. chat open additionally requires FIRST_TREE_AGENT_ID so a human operator's machine with no agent configured is never touched.

Deliberately left allowed

• chat update / set-topic — the agent briefing requires it to keep topic/description current, and neither is a message to a human.
• chat list, chat history, participant reads, feishu intent, feishu credential-env, cron, github/gitlab follow, doc commands.
• POST /agent/chats/:chatId/archive — this is a deviation from the original scope, called out for review. The route does exist on the agent scope, but it writes the calling human's private engagement row, i.e. personal view state. That is the same class the Web boundary deliberately keeps working on Feishu chats (/read, /unread, /pin are all unguarded there), and packages/qa/cases/cross-surface/feishu-agent-channel.md already pins "personal read, pin and archive state must continue to work." Blocking it would have made the agent scope stricter than Web for no delivery-visibility reason. Happy to add it if reviewers disagree.

Tests

• packages/server/src/__tests__/feishu-agent-readonly.test.ts (new, 5 cases) — blocked routes return 403 with the code and an actionable message; reads, chat update and the externalChannel signal still work; the runtime-notice exemption passes while bare purpose does not; the boundary releases on detach; an unbridged chat is untouched.
• packages/server/src/__tests__/feishu-cli-preflight.test.ts — the bridge-still-delivers regression described above.
• apps/cli/src/__tests__/chat-feishu-context-guard.test.ts (new, 9 cases) — both preconditions, the fail-open paths, and the older-server case.
• packages/qa/cases/cross-surface/feishu-agent-channel.md — new operate/observe steps for the agent-side boundary, the runtime notice under a forced provider failure, and post-detach release; FAIL criteria extended.

Four web DOM test fixtures and one client fixture gain externalChannel: null, following the existing descriptionUpdatedAt / lastReadAt .default(null) precedent on chatDetailSchema.

Checks run

• pnpm check — pass (0 errors; the remaining warnings are pre-existing).
• pnpm typecheck — pass, 9/9 packages.
• pnpm test — pass, 10/10 packages, full monorepo. Docker was available, so the server Postgres testcontainer suites ran for real; nothing was skipped.

No database change: no schema, migration, constraint, index, default, or backfill. The guard reads existing im_chat_bindings rows.

🤖 Generated with Claude Code

[baixiaohang]

Recommendation: request changes

• Rationale: The active-binding guard is directionally correct, but detached chats get contradictory cross-surface behavior and chat create --agent can bypass the only available precondition.

Risk level: A

• Path baseline: includes apps/cli/** and packages/client/** -> A
• Semantic lift: touches agent message routing and live chat-binding state; no further grade above A

PR summary

• Author / repo: liuchao-001 / agent-team-foundation/first-tree
• Problem: Agents working from Feishu-bridged chats can currently use First Tree chat commands whose output is invisible to the humans following the conversation in Feishu.
• Approach: Reject agent message/invite writes against active Feishu bindings, expose live binding state for CLI-only preconditions, and preserve the trusted Feishu delivery and provider-failure-notice paths.
• Impacted modules: CLI chat commands, shared chat DTOs, agent chat/message routes, Feishu integration regressions, and cross-surface QA guidance

Review findings
❌ 1. A detached binding is treated as an ordinary chat only on the agent surface. isFeishuBridgedChat filters status = 'active', so this PR allows agent sends after detach, but assertWebMutableChat still rejects any historical im_chat_bindings row. The result is a chat where agents can resume writing while the managing human still cannot send, rename, or manage membership in Web. This also makes the QA sequence internally contradictory: it detaches and calls the chat ordinary, then immediately requires Web structural writes to remain blocked. Please make both surfaces share the active-binding predicate (or explicitly choose and document a different post-detach contract) and pin the same post-detach behavior from agent and Web. [R1/R5 / packages/server/src/api/agent/feishu-chat-guard.ts:48, packages/server/src/api/chats.ts:80, packages/qa/cases/cross-surface/feishu-agent-channel.md:91]
❌ 2. chat create checks the current chat through the outbound --agent override rather than the session identity. resolveSenderName gives the override precedence over FIRST_TREE_AGENT_ID; if that other local agent is not a participant in the bridged chat, getChatDetail returns 403, the advisory lookup deliberately fails open, and createTaskChat then succeeds with the overridden SDK. Because the create route never receives the origin chat, there is no server guard to catch this bypass. Resolve the bridge signal with the session agent independently from the selected sender, and add a command-level regression proving --agent <other> cannot create a chat from a bridged session. [R4/R5 / apps/cli/src/commands/chat/create.ts:143]
✅ 3. Keeping the hard guard at the agent route boundary preserves the trusted Feishu intent route's reuse of sendMessage; the same-chat regression is the right blast-radius check.

Action taken

• Submitted request changes.

[yuezengwu]

This change correctly moves the Feishu boundary to the agent route layer, preserving the trusted feishu intent delivery path, and adds live binding state to ChatDetail for the two CLI-only preconditions. The runtime-notice and private archive exceptions match the existing chat semantics.

Two changes are required before approval:

1. Blocker — chat create --agent bypasses the new precondition (apps/cli/src/commands/chat/create.ts:142-147). The same SDK selected by options.agent is used both for the new task write and for reading the current session chat. --agent takes precedence over FIRST_TREE_AGENT_ID; if the selected agent is not a participant in the current Feishu-bridged chat, getChatDetail() fails, isFeishuBridgedChatContext() deliberately returns false, and the command proceeds to create the task chat. Please perform the context lookup with the current session agent (FIRST_TREE_AGENT_ID) independently of the optional sender override, and add a command-level regression covering a bridged session plus --agent <different-local-agent>.

2. Required documentation update — this changes user-visible behavior for chat send, chat ask, chat invite, chat create, and chat open, but docs/cli-reference.md still describes all five as generally available and says FIRST_TREE_CHAT_ID is used only by send/invite (plus cron). Please document the active-Feishu refusal, the allowed commands/exceptions, the alternative feishu intent + lark-cli --as bot path, and the new create/open use of session context.

Human contract check: the shared ChatDetail shape gains externalChannel; there is no database schema or migration change.

[liuchao-001]

Review findings addressed — new head 7a3756e

All four findings were re-verified against the code first. Three were real as described; one was half right (details below).

---

⚠️ BEHAVIOR CHANGE NEEDING HUMAN SIGN-OFF (from HIGH 3)

The Web app now allows structural writes on a chat whose Feishu binding has been detached.

Previously the Web guard matched any im_chat_bindings row, detached ones included, so a chat that had ever been bridged stayed Web-read-only forever. It now shares the agent scope's active-only predicate: once the binding detaches, Web rename / send / membership / entity-follow all work again.

This is a deliberate, user-visible change to Web behavior — not a refactor side effect. Rationale: a detached binding means the chat is no longer mirrored into any Feishu conversation, so a Web write reaches exactly the people it always did and the boundary has nothing left to protect. The old behavior also had no way back — a detach left the chat permanently frozen in Web.

Covered by a new case in feishu-web-readonly.test.ts. Please confirm this is the intent before merge.

---

BLOCKER 1 — chat create --agent <other> bypass — fixed

Confirmed. Two independent defects, both fixed:

1. Wrong identity. The origin-chat lookup ran on createSdk(options.agent). It now runs on createSdk(), which resolves from FIRST_TREE_AGENT_ID — the session agent, which by construction can see its own chat. --agent still chooses who creates the new chat; it no longer decides who may answer "is the chat I'm sitting in bridged?". An unrelated agent's membership is never a precondition for an ordinary create — pinned by a test asserting the overridden agent's getChatDetail is not consulted.
2. Fail-open. isFeishuBridgedChatContext collapsed every error into false. Replaced with a tri-state (bridged / unbridged / unknown); unknown refuses under a distinct FEISHU_CHAT_CONTEXT_UNKNOWN code with the underlying reason in the message.

Why fail-closed is safe here: the lookup and the create hit the same server with the same credentials, so a failed lookup means the create was going to fail anyway. The refusal replaces a confusing downstream error with a precise one — and the one case where the lookup fails but the create would have succeeded is exactly the case that produced this bug.

Gated on both FIRST_TREE_CHAT_ID and FIRST_TREE_AGENT_ID being set, matching the existing chat open precedent, so an operator terminal with no agent configured is unaffected.

BLOCKER 2 — forgeable runtime-notice exemption — fixed

Confirmed: purpose and metadata.runtimeNotice are both request fields, so the exemption was a hole straight through the 403.

Took the recommended direction — a dedicated route, POST /api/v1/agent/chats/:chatId/runtime-notices:

• The server authors source, format, the recipientless silent delivery profile, and the runtimeNotice marker. The request carries only content, and the schema is .strict(), so an attempt to smuggle purpose / metadata is a 400 rather than a silent drop.
• The guard exempts by route. Nothing a caller can write in a chat send body opens the boundary any more.
• RUNTIME_NOTICE_METADATA_KEY joins the server-owned keys in stripUntrustedMetadataKeys, alongside agentFinalText and the ask-agent / GitHub-task markers. An inbound copy is now stripped on every write path, so the forgery can't just move to an unbridged chat and be laundered back.
• The marker is set through a trusted SendMessageOptions.runtimeNotice flag, matching the existing askAgentRequestId / allowFirstChatOrientation pattern.
• The route still requires chat membership — a narrower capability, not an open door.

Client runtime updated: new sdk.postRuntimeNotice(chatId, content), used by postProviderFailureRuntimeNotice and by both codex usage-limit sites. Provider-failure notices still deliver into bridged chats.

One honest limitation. The new route accepts client-supplied notice text. That matches the trust model of the Feishu bridge route itself, which is the architecture this reuses. Making the text fully server-authored would mean relocating formatProviderFailureRuntimeNotice and redactErrorPreview into @first-tree/shared, and redactErrorPreview's module path is pinned by provider-boundary-guard.test.ts and provider-support-export-allowlists.ts — a much larger, riskier change than this review calls for. What is closed is the part that mattered: the exemption is no longer expressible in a request body, and the marker can no longer be minted by an agent credential.

MEDIUM 4 — probe leak + stale docs — fixed (message route: disputed)

• Invite route: confirmed and fixed. POST /:chatId/participants ran assertAgentMutableChat before any membership check (authz happened later, inside addParticipant → inviteParticipantsToChat). A non-member with a guessed chat UUID could distinguish bridged chats by the error. Now assertParticipant runs first. Same speaker-level check the invite service applies, so no legitimate invite is affected.
• Message route: disputed — already correct. POST /:chatId/messages calls chatService.assertParticipant at the top of the handler, before the guard. No ordering change needed. Pinned by a new test that walks a non-member through both routes against a bridged and an ordinary chat and asserts the errors are indistinguishable.
• Docs: new "Chats bridged to a Feishu conversation" section in docs/cli-reference.md with a per-command table.
• Wording corrected in the 403 message, the docs, and the QA case: the boundary blocks messages and membership changes, not all writes. chat update, chat archive, personal read/pin state and all reads keep working. "Read-only" would send an agent hunting for a workaround it doesn't need.

---

Tests

One regression per finding:

Finding	Test
1	chat-feishu-context-guard.test.ts — --agent <other> refuses and createTaskChat is never called; inconclusive lookup refuses; ordinary --agent create still works without consulting the overridden agent
2	feishu-agent-readonly.test.ts — forged runtimeNotice from an ordinary agent credential → 403; genuine notice via the dedicated route → 201 with the server-stamped marker; smuggled marker stripped even in an unbridged chat; route is membership-gated and rejects a non-strict body. Plus agent-final-text-purpose.test.ts for the strip
3	feishu-web-readonly.test.ts detach case + the existing agent-scope detach case — identical release point in both scopes
4	feishu-agent-readonly.test.ts — non-member gets an indistinguishable error on bridged vs ordinary chats, for both invite and message routes

The bridge regression in feishu-cli-preflight.test.ts still passes — chat send refused and feishu intent delivering on the same chat.

Verification

pnpm check ✅ · pnpm typecheck ✅ (9/9 packages)

Docker was available and the server testcontainer suites really ran:

Suite	Result
server	294 files, 3450 passed
client	199 files, 2558 passed, 7 skipped
web	252 files, 2365 passed
shared	77 files, 926 passed
cli	117 passed, 3 failed

The 3 CLI failures are in daemon-refresh-unit.test.ts and are environmental and pre-existing — the machine's ~/.local/bin/first-tree-dev shim is a symlink into a different, deleted worktree. That file is not in this diff and the failure is unrelated to this change.

chat-attention-commands-extra.test.ts needed a fixture update: its stub SDK had no getChatDetail, which the old fail-open silently swallowed. Adding it makes those cases deterministic regardless of ambient FIRST_TREE_AGENT_ID.

Also updated packages/qa/cases/cross-surface/feishu-agent-channel.md with the probe-oracle, --agent, forged-exemption and Web-detach branches.

[yuezengwu]

The new head resolves the previous --agent identity bug, shares the active-binding predicate across Web and agent scopes, fixes invite authorization ordering, and adds the missing CLI documentation. The route-layer placement still correctly preserves the trusted Feishu intent path.

Two blockers remain:

1. Blocker — the dedicated runtime-notice route is still an agent-callable message bypass (packages/server/src/api/agent/messages.ts:112-133). The same ordinary agent credential blocked by POST /messages can call POST /runtime-notices with any 1–4000 character content, and the server then stores that arbitrary text in the active bridged chat. Moving the exemption from body markers to a public agent route makes the marker server-authored, but it does not make the caller trusted; membership is exactly the authority an ordinary chat send caller already has. A request such as { "content": "ordinary reply" } therefore still crosses the 403 boundary. Please require authority unavailable to the agent tool/API surface (or make the notice semantics/content genuinely server-authored from a constrained event), and add a regression proving an ordinary agent credential cannot use this endpoint to publish arbitrary chat text.

2. Blocker — the advertised fail-closed CLI check still fails open on an omitted field (apps/cli/src/core/feishu-chat-context.ts:104-111). ChatDetailReader intentionally allows externalChannel to be absent, but the resolver maps every value except "feishu" — including undefined from an older/malformed server — to unbridged. The test explicitly pins this. That lets chat create/chat open proceed even though the bridge state is unknown, contradicting the new tri-state contract and the statement that inconclusive answers refuse. Only explicit null should mean unbridged; an absent/unknown value should return unknown and use FEISHU_CHAT_CONTEXT_UNKNOWN.

Documentation follow-up: the new CLI table lists chat detail, read/unread, and pin as commands, but those commands are not registered in apps/cli/src/commands/chat/index.ts. Please label the table as operations/surfaces or list only actual CLI commands. The Web 403 still says the chat is wholly “read-only” even though this change deliberately preserves personal-state and metadata writes; align that wording with the corrected contract.

Human sign-off remains required for the deliberate behavior change that restores Web structural writes after a binding detaches. The active-only rule is internally consistent and avoids permanently freezing the chat, but it should not be treated as approved until yuezengwu explicitly confirms it.

Core contract note: this head adds the dedicated runtime-notice request/API shape in addition to ChatDetail.externalChannel; there is still no database schema or migration change.

[baixiaohang]

Recommendation: request changes

• Rationale: The original detach and --agent blockers are fixed, but the new runtime-notice endpoint remains an agent-callable message bypass and the advertised fail-closed CLI check still treats an omitted signal as unbridged.

Risk level: A

• Path baseline: includes apps/cli/** and packages/client/** -> A
• Semantic lift: the follow-up adds a new agent write capability and changes the runtime/server protocol; no further grade above A

PR summary

• Author / repo: liuchao-001 / agent-team-foundation/first-tree
• Problem: Prevent agents in Feishu-bridged conversations from writing First Tree-only messages or membership changes that Feishu participants cannot observe.
• Approach: Share one active-binding predicate across Web and agent routes, resolve CLI origin context with the session identity, guard membership before bridge state, and move exceptional provider notices to a dedicated endpoint.
• Impacted modules: agent chat/message routes, Feishu binding service, CLI chat preconditions, Client runtime/provider handlers, shared message contracts, Web boundary tests, docs, and QA guidance

Review findings
❌ 1. The dedicated route does not make the runtime-notice exemption unforgeable. It uses the same agent authentication as the ordinary message route, requires only chat participation, and accepts up to 4,000 characters of caller-chosen text; postRuntimeNotice is also a public SDK method. Any speaking agent that could previously forge { purpose, metadata.runtimeNotice } can now call /runtime-notices and persist the same arbitrary First Tree-only message in a bridged chat. A route name and strict body shape do not add caller authority. This differs from feishu intent, which additionally proves Bot ownership and the exact bound conversation/target. Please gate this path with authority unavailable to the agent-authored call surface (or make the server derive a tightly closed notice from a trusted runtime event); otherwise the claimed blocker remains open. [R4 / packages/server/src/api/agent/messages.ts:112, packages/client/src/cloud/sdk.ts:447, packages/server/src/api/agent/feishu.ts:123]
❌ 2. The tri-state CLI resolver still maps an omitted externalChannel to unbridged. ChatDetailReader deliberately makes the field optional for older Servers, but line 111 treats every value except "feishu" — including undefined or a malformed value — as an affirmative unbridged answer. That lets chat create and chat open proceed precisely when bridge state is unknown, contradicting the new fail-closed contract. Only explicit null should resolve to unbridged; absence should return unknown and use FEISHU_CHAT_CONTEXT_UNKNOWN, with the existing older-server test inverted accordingly. [R4/R5 / apps/cli/src/core/feishu-chat-context.ts:104]
❌ 3. Moving every notice producer to a new endpoint also breaks the exceptional signal across independently deployed Client/Server versions. An older Client talking to this Server still posts the old decorated /messages request and is rejected by the unconditional Feishu guard; a new Client talking to an older Server posts /runtime-notices and gets 404. In both rollout directions, the provider-failure row this exception exists to preserve disappears. Please provide a staged compatibility/fencing contract and regression coverage for both version directions rather than assuming lockstep deployment. [R5 / packages/client/src/runtime/runtime-notice.ts:42, packages/server/src/api/agent/messages.ts:59]
✅ 4. The previous findings are otherwise addressed: Web and agent scopes now share the active-only binding predicate; chat create --agent resolves the origin with the session identity; invite authorization precedes bridge disclosure; and the CLI reference/QA contract now describe the intended command boundary.

Action taken

• Submitted request changes on head 7a3756e.

[yuezengwu]

Human sign-off confirmed: use the active-binding-only contract. While an im_chat_bindings row is active, Web and agent message/membership writes remain restricted; once it is detached, both surfaces restore ordinary First Tree writes.

This resolves only the requested product decision. The three technical blockers in the current reviews remain open: the agent-callable runtime-notice bypass, omitted externalChannel still resolving as unbridged, and the two-direction Client/Server rollout compatibility gap.