feat: block First Tree chat tools in Feishu-bridged chats - #2344
feat: block First Tree chat tools in Feishu-bridged chats#2344liuchao-001 wants to merge 4 commits into
Conversation
An agent sitting in a chat bridged to a Feishu conversation could answer with `chat send` / `chat ask` / `chat invite`. Those writes land in First Tree, which nobody in the Feishu group ever reads, so the reply was silently lost. Add the agent-scope counterpart of `assertWebMutableChat`: the agent message and participant routes now refuse a chat with an active `im_chat_bindings` row, naming the path that actually delivers. The guard lives in the route layer, never in `messageService.sendMessage` — the Feishu bridge's own outbound delivery reuses that exact service call with the same source and sender, so a service-layer guard would silence the bot itself. Provider-failure runtime notices stay exempt: an agent that cannot run at all must not also go silent on its operators. `chat create` and `chat open` cannot be gated server-side (neither transmits the originating chat), so both get an advisory CLI precondition reading the same live binding state through a new `ChatDetail.externalChannel`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: The active-binding guard is directionally correct, but detached chats get contradictory cross-surface behavior and
chat create --agentcan bypass the only available precondition.
Risk level: A
- Path baseline: includes
apps/cli/**andpackages/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
left a comment
There was a problem hiding this comment.
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:
-
Blocker —
chat create --agentbypasses the new precondition (apps/cli/src/commands/chat/create.ts:142-147). The same SDK selected byoptions.agentis used both for the new task write and for reading the current session chat.--agenttakes precedence overFIRST_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>. -
Required documentation update — this changes user-visible behavior for
chat send,chat ask,chat invite,chat create, andchat open, butdocs/cli-reference.mdstill describes all five as generally available and saysFIRST_TREE_CHAT_IDis used only by send/invite (plus cron). Please document the active-Feishu refusal, the allowed commands/exceptions, the alternativefeishu intent+lark-cli --as botpath, 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.
Address four review findings on the agent-scope Feishu guard. `chat create --agent <other>` bypass: the origin-chat lookup ran as the overridden agent, and an agent that is not a member of the origin chat gets a 403 that the fail-open path read as "not a Feishu chat". The lookup now runs under the session identity, so it is performed by an agent that can actually see the chat, and an inconclusive answer refuses instead of proceeding. `--agent` still selects who creates the new chat and never becomes a membership requirement for an ordinary create. Forgeable runtime-notice exemption: the exemption trusted `purpose` and `metadata.runtimeNotice` from the request body, which any agent credential can set. Runtime notices now post to a dedicated route that authors the delivery profile and the marker server-side, and the guard exempts by route rather than by request content — the same property that makes the Feishu bridge safe. The marker joins the server-owned metadata keys, so an inbound copy is stripped on every ordinary write path. The client runtime posts through a new SDK method. Detach semantics: the agent and Web scopes disagreed on what "bridged" means, leaving a detached chat agent-writable but Web-read-only. Both now share one active-only binding predicate. BEHAVIOR CHANGE: the Web guard previously matched any binding row, including detached ones. Web structural writes are now accepted again once a binding detaches. Probe leak: the invite guard ran before membership authorization, so the error difference revealed which chats are Feishu-bound. Membership is now enforced first. The message route already authorized before the guard. Also correct the product wording — the boundary blocks messages and membership changes, not all writes; `chat update` and personal state keep working — and document the restrictions in the CLI reference.
Review findings addressed — new head
|
| 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
left a comment
There was a problem hiding this comment.
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:
-
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 byPOST /messagescan callPOST /runtime-noticeswith any 1–4000 charactercontent, 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 ordinarychat sendcaller 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. -
Blocker — the advertised fail-closed CLI check still fails open on an omitted field (
apps/cli/src/core/feishu-chat-context.ts:104-111).ChatDetailReaderintentionally allowsexternalChannelto be absent, but the resolver maps every value except"feishu"— includingundefinedfrom an older/malformed server — tounbridged. The test explicitly pins this. That letschat create/chat openproceed even though the bridge state is unknown, contradicting the new tri-state contract and the statement that inconclusive answers refuse. Only explicitnullshould mean unbridged; an absent/unknown value should returnunknownand useFEISHU_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
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: The original detach and
--agentblockers 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/**andpackages/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.
|
Human sign-off confirmed: use the active-binding-only contract. While an This resolves only the requested product decision. The three technical blockers in the current reviews remain open: the agent-callable runtime-notice bypass, omitted |
|
Thanks for the second round — the convergence between reviewers was useful. Three of you independently flagged the Two items I would rather resolve by agreement than decide unilaterally. 1. Posture of
|
|
On item 1, I still consider the current Correcting the provenance language is necessary, but it does not change the enforced boundary: the same ordinary agent credential rejected by I do not think this forces the full daemon-capability design into this PR. A narrower in-scope fix is possible if the server accepts a closed notice kind and authors the complete user-visible text from a fixed template. If detailed caller-supplied text or error previews must remain, then the route needs authority unavailable to the ordinary agent API/tool surface. Deferring either enforcement change to a follow-up would leave this PR's advertised agent write boundary incomplete. On item 2, I agree that the separate QA App/Bot plus disposable group is the safer exact-build loopback. I am routing the provisioning decision to yuezengwu; I cannot commit the shared QA identity or a production maintenance window on the team's behalf. |
The origin check that gates `chat create` / `chat open` treated an unreadable answer as "not bridged". A server older than `ChatDetail.externalChannel` omits the field, and a session that names a chat with no agent id could not read it at all — both were silently allowed, so a rolling deploy turned the guard off. Only an explicit `null` means unbridged now; absent, unrecognised, unreachable, or unreadable all refuse with an actionable reason. The operator case is unchanged and pinned by tests: no chat context at all still allows without a lookup, and never builds an SDK on a machine with no agent. Membership removal and message edit were unguarded, so an agent could still mutate shared membership of a bridged chat or rewrite history the Feishu humans cannot see. Both now apply the boundary, after the membership check so neither can be used to probe binding state. Runtime notices lost their rolling-deploy story when they moved to a dedicated endpoint: a newer client met servers without the route, and an older client's notices were refused. The SDK now falls back to the legacy send shape on 404, and the server recognises that exact shape, sharing one definition so the halves cannot drift. Both directions are covered. Corrects the security framing throughout. The notice endpoint is membership-gated exactly like an ordinary send, so it is a misuse-prevention rail carrying a client-runtime-reported notice, not an unforgeable authorization boundary — the code, comments and docs now say so. The Web 403 no longer claims "read-only" while personal state keeps working, the CLI reference no longer documents commands that do not exist (`chat detail`, `read/unread`, `pin`), and the live-QA case no longer invites an agent into a chat where invites are refused.
Three test conflicts, all where main and this branch touched the same provider-test SDK stubs: - opencode handler, codex app-server extra-coverage: main added `serverUrl` / `getAgentContextTreeConfig` to the stub SDK while this branch added `postRuntimeNotice`. Both kept. - pi session-custody: main REWROTE the "active inject before prompt write" case into "active inject on a healthy RPC bypasses config refresh". Main's rewritten case is kept whole; its "no operator-facing write happened" assertion now watches `postRuntimeNotice`, which is where a runtime notice goes on this branch. No product code conflicted, and no guard semantics were touched.
Round 2 — per-finding dispositionHead is now A —
|
yuezengwu
left a comment
There was a problem hiding this comment.
The new head fixes the fail-closed CLI edges, covers participant removal and message edits with the correct authorization ordering, repairs the CLI/Web documentation, and implements both rolling-deploy directions. The trusted Feishu intent path remains isolated correctly.
Two enforcement blockers remain:
-
Blocker —
POST /runtime-noticesstill defeats the advertised agent write boundary (packages/server/src/api/agent/messages.ts:148-169). The route accepts any 1–4000 character caller-authored text under the same ordinary agent membership authority asPOST /messages, then stores it in the bridged chat. Renaming this a misuse-prevention rail makes the limitation honest, but does not change the behavior: the credential refused by the guarded route can still persist an arbitrary First Tree-only reply. Please either accept a closed notice kind whose complete visible text is server-authored, or require authority unavailable to the ordinary agent HTTP/tool surface. A follow-up cannot close a boundary this PR claims to enforce. -
Blocker — the legacy-body compatibility path grants authority that some callers do not have on the dedicated endpoint (
packages/server/src/api/agent/messages.ts:55-90,packages/server/src/middleware/user-auth.ts:95-105,packages/shared/src/schemas/message.ts:497-509).agent_outboxJWTs are deliberately accepted only forPOST /agent/chats/:chatId/messages; they cannot call/runtime-notices. Yet an outbox token can submit the legacy body, skipassertAgentMutableChat, and receive the trusted runtime-notice handling. That directly disproves the comments and PR description saying every such caller could call the endpoint and the body grants nothing. The matcher is also not exact: it does not rejectreceiverNamesorinReplyTo, both of which survivesendMessageSchema.parse; nor does it apply the endpoint's string/length constraint. Add an outbox-token regression on a bridged chat and close this privilege expansion. If legacy compatibility remains, it must be limited to callers that already possess the notice capability and to the actual legacy body shape.
Related comments remain internally contradictory and should be corrected with the enforcement fix: messages.ts says no request-body shape opens the ordinary path even though the legacy branch does exactly that, while services/chat/message.ts:449-456 says runtimeNotice is set only by the dedicated route and is a capability although the legacy message route sets it and the shared contract calls it a classification label.
The real-tenant exact-build Feishu loopback is still an external merge gate; the QA App/Bot provisioning decision is pending with yuezengwu.
Core contract note: this head still adds ChatDetail.externalChannel, the runtime-notice request/API shape, and now a shared legacy wire-shape contract. There is no database schema or migration change.
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: The CLI, route-coverage, authorization-ordering, and documentation gaps are fixed, but the retained runtime-notice surface still lets an ordinary agent write into an active bridged chat, and the new compatibility branch expands that bypass to a narrower token that cannot call the dedicated endpoint.
Risk level: A
- Path baseline: includes
apps/cli/**andpackages/client/**-> A - Semantic lift: changes agent message authorization and the independently deployed Client/Server protocol; no further grade above A
PR summary
- Author / repo: liuchao-001 / agent-team-foundation/first-tree
- Problem: Agents working from a Feishu-bridged conversation can otherwise write First Tree-only messages or membership changes that the humans following the conversation in Feishu never see.
- Approach: Guard every agent message/membership route against active Feishu bindings, share the live-binding predicate with Web, fail closed for CLI-only origin checks, and preserve exceptional runtime notices across mixed Client/Server versions.
- Impacted modules: CLI chat preconditions, shared chat/message contracts, Client SDK/runtime/provider handlers, agent and Web chat routes, Feishu binding services, docs, tests, and cross-surface QA guidance
Review findings
❌ 1. The original runtime-notice blocker is deliberately unchanged. POST /runtime-notices accepts arbitrary caller-authored text under the same ordinary agent membership authority as POST /messages, then stores it in the active bridged chat. Calling it a misuse-prevention rail accurately documents the limitation but does not change the user-visible effect: the credential refused by the advertised message boundary can still persist an arbitrary First Tree-only reply. A follow-up cannot complete a boundary this PR claims to enforce. Require authority unavailable to the ordinary agent HTTP/tool surface, accept a closed notice kind whose complete visible text is server-authored, or remove the bridged-chat exception until that capability exists. [R4/R5 / packages/server/src/api/agent/messages.ts:119]
❌ 2. The legacy compatibility branch is a privilege expansion, not merely another spelling of the dedicated endpoint. An agent_outbox JWT is intentionally accepted only for POST /agent/chats/:chatId/messages, so it cannot call /runtime-notices; nevertheless, it can submit the legacy body, skip assertAgentMutableChat, and receive trusted runtime-notice handling. The matcher is also not the claimed exact five-field shape: it neither constrains content to the endpoint's string/length contract nor excludes receiverNames and inReplyTo, both of which survive sendMessageSchema.parse and shape the stored row. Add the scoped-outbox regression and redesign compatibility so it cannot grant notice authority to a caller that lacks it. Securing finding 1 while retaining this unauthenticated body branch would simply reopen the same boundary. [R4/R5 / packages/server/src/api/agent/messages.ts:55, packages/server/src/middleware/user-auth.ts:95, packages/shared/src/schemas/message.ts:497]
SendMessageOptions.runtimeNotice says it is set only by the dedicated route and is a capability, while the message route now sets it and the shared schema calls the marker a classification label. Align these statements with the enforcement outcome so future callers do not treat the service option as a provenance or authorization guarantee. [R1/R5 / packages/server/src/api/agent/messages.ts:133, packages/server/src/services/chat/message.ts:98, packages/server/src/services/chat/message.ts:449]
✅ 4. The previous non-notice findings are addressed: only explicit externalChannel: null is unbridged; half-configured session context refuses; edit/removal routes authorize membership before the bridge guard; the Web wording and CLI command table are corrected; and active-only post-detach behavior has human sign-off.
Action taken
- Submitted request changes on head
ecaf3264c.
|
There is one more product-completeness blocker in the outbound path. Workspace currently does not show evidence that the Agent actually sent the message through official First Tree already has two possible observation inputs, but neither is connected to durable outbound chat history here:
The consequence is that an intent followed by a failed or omitted CLI call is indistinguishable in Workspace from a successfully delivered Feishu message. Conversely, a direct observed Please close the loop by correlating actual outbound execution/provider observation with the canonical intent (the canonical message id is already the This is separate from the existing runtime-notice authorization blockers. No database migration is present in the current head; if the delivery-state solution adds a durable field/table, that schema contract needs explicit human review. |
|
I agree that the missing provider-delivery observation is real, but it predates this PR and should not expand this guard change into a new durable delivery-state subsystem. For this PR, I would require the narrower truthful-state fix: the canonical row created by Confirmed delivery, failed/unknown state, correlation with tool/provider observation, and retry/dedupe semantics should be a separately designed follow-up with an explicit owner. If that design adds a durable field or table, it should receive its own human schema review. This scope decision does not change the two current enforcement blockers: the ordinary-agent runtime-notice write bypass and the legacy-body/outbox privilege expansion remain merge blockers for this PR. |
|
Human scope decision confirmed: accept the split proposed above. For this PR, the outbound-history requirement is now:
The full delivery-observation subsystem is therefore no longer a blocker for #2344 once those two scoped requirements are satisfied. The existing runtime-notice enforcement blockers remain unchanged: ordinary-agent arbitrary-text access and the legacy-body/ |
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. Both scopes now share one active-only predicate, and neither calls a bridged chat "read-only" any more: personal view state (read, pin, archive) deliberately keeps working, so the refusals name the blocked class instead.Server (blocking, 403 +
code: "FEISHU_CHAT_AGENT_WRITE_FORBIDDEN")POST /api/v1/agent/chats/:chatId/messages— coverschat sendandchat ask(same route;chat askis justformat: "request").PATCH /api/v1/agent/chats/:chatId/messages/:messageId— message edit; rewriting history the Feishu humans cannot see is the same class of write.POST /api/v1/agent/chats/:chatId/participants— coverschat invite.DELETE /api/v1/agent/chats/:chatId/participants/:agentId— membership removal, the other half of "membership changes".Every one of them applies the boundary after authorizing the caller's membership, so a non-member cannot tell a bridged chat from an ordinary one by the difference in error.
The shared helper is
packages/server/src/api/agent/feishu-chat-guard.ts. Authority isim_chat_bindingsfiltered tostatus = 'active', notchats.metadata.source— that label stays"feishu"after a binding detaches. The Web helper predates thestatuscolumn 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 officiallark-cli --as bot.Client-side preconditions (advisory, exit code 2,
FEISHU_CHAT_CONTEXT)chat createandchat openare refused from inside a bridged session, inapps/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 samemessageService.sendMessagethatchat sendcalls, with the identicalsource: MESSAGE_SOURCES.CLIand the agent's ownsenderId. A guard placed insidesendMessagewould 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.tsgains the regression that pins this: on one bridged chat, the intent route still delivers (message stored, silentnotify=falsefan-out intact) while the agent chat route on that same chat returns 403.The runtime-notice exemption
runtime/runtime-notice.ts::postProviderFailureRuntimeNoticeposts 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 is a property of which route is called, not of anything in a request body: notices go to a dedicated
POST /api/v1/agent/chats/:chatId/runtime-notices, whose body carries only the text. The server authorssource,format, the silent recipientless delivery profile and the storedruntimeNoticemarker, and strips any inbound copy of that marker. An ordinarychat senddecorated to look like a notice is refused.Be precise about what that endpoint is: a misuse-prevention rail carrying a notice the client runtime reports about itself — not a security or authorization boundary, and the exemption is not unforgeable. The route is gated on chat membership and nothing else, exactly like
POST /messages, so any credential that can reach one can reach the other, and nothing verifies that a provider actually failed. What the separate route does buy is still worth having: the ordinary send path stays uniformly guarded with no shape of body that opens it, and the server authors the whole stored row so a notice cannot quietly become an addressed message. Whether the capability should be narrowed further — to the daemon, or scoped to a chat/turn — is an open posture question this PR deliberately does not settle.Rolling deploy. The endpoint degrades in both directions rather than dropping notices, which matter most mid-deploy. A newer client falls back to the legacy send shape when the server answers 404; a newer server recognises that same exact legacy shape from a client that predates the endpoint. Both halves share one definition in
sharedso they cannot drift. That compatibility path is body-shaped by necessity and is scoped to an exact match on the five fields older clients emitted; it grants nothing, since the same caller could call the endpoint directly. Delete it once no supported client predates the endpoint.Why
chat createis CLI-side onlyThe server never learns which chat the caller is sitting in: there is no field for it in
createTaskChatSchema, no header carries it, andchat/create.tsdoes not readFIRST_TREE_CHAT_ID. There is nothing to gate on server-side.chat openis 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.sourcelabel, both read a newChatDetail.externalChannelfield, populated by the agent chat-detail route from the same liveim_chat_bindingsstate the write boundary enforces. That keeps the advisory rail and the real boundary from disagreeing — withmetadata.source, a detached chat would have been refused locally while the server happily accepted it.Both checks fail closed, because there is no server-side boundary behind them to catch a wrong guess. Only an explicit
nullmeans "not bridged". An absent field (a server older than it omits the field, and the SDK does not re-parse the response through Zod), an unrecognised value, a failed lookup, or a session naming a chat with noFIRST_TREE_AGENT_IDto read it as are all unknown and refuse withFEISHU_CHAT_CONTEXT_UNKNOWN, naming what to fix.The one allowed silence is no chat context at all. Absent
FIRST_TREE_CHAT_IDmeans the command is not running inside a chat: it proceeds without a lookup and never constructs an SDK, sochat openfrom a human operator's terminal with no agent configured keeps working. "No chat context" and "chat context we cannot resolve" are deliberately different answers.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 (its/read,/unread,/pinroutes are all unguarded — note these are Web routes and personal state, not CLI commands), andpackages/qa/cases/cross-surface/feishu-agent-channel.mdalready 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— every blocked route (send, ask, invite, message edit, membership removal) returns 403 with the full expected body, not just status + code; the non-member probe asserts whole-body equality between a bridged and an ordinary chat; reads,chat updateand theexternalChannelsignal still work; a genuine runtime notice lands through the dedicated route while decorated ordinary sends do not, and the legacy wire shape still lands for an older client; 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— both preconditions and every fail-closed path (field absent, unrecognised value, lookup failure, chat id with no agent id, unconstructable reader), plus the operator case that must keep working.packages/client/src/__tests__/runtime-notice.test.ts— the 404 fallback to the legacy shape, and that a non-404 refusal is never reshaped into an ordinary send.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 existingdescriptionUpdatedAt/lastReadAt.default(null)precedent onchatDetailSchema.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_bindingsrows.🤖 Generated with Claude Code