diff --git a/apps/cli/src/__tests__/chat-attention-commands-extra.test.ts b/apps/cli/src/__tests__/chat-attention-commands-extra.test.ts index c45da54e8..d6260d265 100644 --- a/apps/cli/src/__tests__/chat-attention-commands-extra.test.ts +++ b/apps/cli/src/__tests__/chat-attention-commands-extra.test.ts @@ -65,6 +65,7 @@ vi.mock("../core/output.js", () => ({ vi.mock("node:readline", () => readlineMocks); const originalChatId = process.env.FIRST_TREE_CHAT_ID; +const originalAgentId = process.env.FIRST_TREE_AGENT_ID; const originalExit = process.exit; const originalSetInterval = globalThis.setInterval; const originalClearInterval = globalThis.clearInterval; @@ -89,7 +90,12 @@ async function runChat(args: string[]): Promise { beforeEach(() => { vi.clearAllMocks(); + // A real agent session always carries BOTH: the daemon injects them + // together. `chat create` / `chat open` now refuse a session that names a + // chat with no agent able to read it, so a fixture with only the chat id + // would be testing an environment that cannot occur. process.env.FIRST_TREE_CHAT_ID = "chat-env"; + process.env.FIRST_TREE_AGENT_ID = "agent-self"; bootstrapMocks.ensureFreshAccessToken.mockResolvedValue("user-token"); bootstrapMocks.resolveServerUrl.mockReturnValue("https://hub.example"); resolveAgentMock.mockResolvedValue({ uuid: "agent-1", name: "nova", displayName: "Nova" }); @@ -102,6 +108,11 @@ beforeEach(() => { localAgentMocks.createSdk.mockReturnValue({ agentId: "agent-self", attention: { raise: vi.fn() }, + // `chat create` / `chat open` resolve the session chat's bridge state + // before doing anything, and an unresolvable answer now refuses instead of + // proceeding. Answer "ordinary chat" so these cases exercise the command, + // not the Feishu precondition (which owns its own test file). + getChatDetail: vi.fn(async () => ({ externalChannel: null })), createTaskChat: vi.fn(async () => ({ chatId: "chat-created", messageId: "msg-created", @@ -127,6 +138,11 @@ afterEach(() => { } else { process.env.FIRST_TREE_CHAT_ID = originalChatId; } + if (originalAgentId === undefined) { + delete process.env.FIRST_TREE_AGENT_ID; + } else { + process.env.FIRST_TREE_AGENT_ID = originalAgentId; + } process.exit = originalExit; globalThis.setInterval = originalSetInterval; globalThis.clearInterval = originalClearInterval; diff --git a/apps/cli/src/__tests__/chat-feishu-context-guard.test.ts b/apps/cli/src/__tests__/chat-feishu-context-guard.test.ts new file mode 100644 index 000000000..cf6a3a146 --- /dev/null +++ b/apps/cli/src/__tests__/chat-feishu-context-guard.test.ts @@ -0,0 +1,336 @@ +import type { ChatExternalChannel } from "@first-tree/shared"; +import { Command } from "commander"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const localAgentMocks = vi.hoisted(() => ({ + createSdk: vi.fn(), + handleSdkError: vi.fn((error: unknown) => { + throw error; + }), +})); + +const docCaptureMock = vi.hoisted(() => ({ + captureOutboundDocs: vi.fn(async (content: string) => ({ content })), +})); + +const outputMocks = vi.hoisted(() => ({ + fail: vi.fn((code: string, message: string, exitCode = 1) => { + throw Object.assign(new Error(message), { code, exitCode }); + }), + success: vi.fn(), +})); + +vi.mock("../commands/_shared/local-agent.js", () => localAgentMocks); +vi.mock("../core/doc-capture.js", () => docCaptureMock); +vi.mock("../cli/output.js", () => outputMocks); + +import { + checkFeishuChatContext, + FEISHU_CHAT_CONTEXT_CODE, + FEISHU_CHAT_CONTEXT_UNKNOWN_CODE, + feishuChatContextMessage, + resolveFeishuChatContext, +} from "../core/feishu-chat-context.js"; + +/** + * Pins the two CLI-side preconditions the server cannot enforce. + * + * `chat create` never transmits the originating chat (`createTaskChatSchema` + * has no field for it, and there is no header), and `chat open` runs on the + * user scope and starts an interactive REPL. Both are therefore refused here, + * from `FIRST_TREE_CHAT_ID` plus the live `externalChannel` signal — and both + * must fail CLOSED, because there is no server-side boundary behind them to + * catch a wrong guess. + */ + +type DetailRow = { externalChannel?: ChatExternalChannel | null }; + +function reader(impl: () => Promise) { + return { getChatDetail: vi.fn(impl) }; +} + +describe("resolveFeishuChatContext", () => { + it("reports a bridged chat", async () => { + const sdk = reader(async () => ({ externalChannel: "feishu" })); + expect(await resolveFeishuChatContext(sdk, "chat-1")).toEqual({ kind: "bridged" }); + expect(sdk.getChatDetail).toHaveBeenCalledWith("chat-1"); + }); + + it("reports an ordinary chat", async () => { + expect( + await resolveFeishuChatContext( + reader(async () => ({ externalChannel: null })), + "chat-1", + ), + ).toEqual({ kind: "unbridged" }); + }); + + /** + * The fail-open edge two reviewers landed on. A server older than + * `externalChannel` omits it, which is a normal mid-deploy state — and + * reading "absent" as "ordinary chat" silently switches the guard off for + * the entire rollout. + */ + it("reports `unknown` for a server that predates the field, never `unbridged`", async () => { + const state = await resolveFeishuChatContext( + reader(async () => ({})), + "chat-1", + ); + expect(state.kind).toBe("unknown"); + expect(state.kind === "unknown" && state.reason).toContain("did not report"); + }); + + it("reports `unknown` for a value this CLI does not recognise", async () => { + const state = await resolveFeishuChatContext( + reader(async () => ({ externalChannel: "slack" }) as unknown as DetailRow), + "chat-1", + ); + expect(state.kind).toBe("unknown"); + expect(state.kind === "unknown" && state.reason).toContain("slack"); + }); + + it("treats ONLY an explicit null as unbridged", async () => { + expect( + await resolveFeishuChatContext( + reader(async () => ({ externalChannel: null })), + "chat-1", + ), + ).toEqual({ kind: "unbridged" }); + }); + + it("reports `unknown` — never `unbridged` — when the lookup throws", async () => { + const state = await resolveFeishuChatContext( + reader(async () => { + throw new Error("connection refused"); + }), + "chat-1", + ); + expect(state).toEqual({ kind: "unknown", reason: "connection refused" }); + }); +}); + +describe("checkFeishuChatContext", () => { + /** A session that is fully configured, as an agent runtime exports it. */ + const SESSION = { chatId: "chat-1", agentId: "agent-1" }; + + it("refuses `chat create` inside a bridged chat and names the Feishu path", async () => { + const refusal = await checkFeishuChatContext( + () => reader(async () => ({ externalChannel: "feishu" })), + SESSION, + "create", + ); + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_CODE); + expect(refusal?.message).toContain("chat create"); + expect(refusal?.message).toContain("feishu intent"); + expect(refusal?.message).toContain("lark-cli"); + }); + + it("refuses `chat open` inside a bridged chat", async () => { + const refusal = await checkFeishuChatContext( + () => reader(async () => ({ externalChannel: "feishu" })), + SESSION, + "open", + ); + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_CODE); + expect(refusal?.message).toContain("chat open"); + }); + + /** + * Regression for the `--agent ` bypass: the overridden agent is not a + * member of the origin chat, so the lookup 403s. Treating that as "not a + * Feishu chat" is what let the create through. + */ + it("refuses with a distinct code when the origin lookup is inconclusive", async () => { + const refusal = await checkFeishuChatContext( + () => + reader(async () => { + throw Object.assign(new Error("Not a participant of this chat"), { statusCode: 403 }); + }), + SESSION, + "create", + ); + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE); + expect(refusal?.message).toContain("Could not determine"); + expect(refusal?.message).toContain("Not a participant of this chat"); + }); + + it("refuses when the server omits the field, instead of assuming the chat is ordinary", async () => { + for (const command of ["create", "open"] as const) { + const refusal = await checkFeishuChatContext(() => reader(async () => ({})), SESSION, command); + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE); + } + }); + + it("allows both commands in an ordinary chat", async () => { + for (const command of ["create", "open"] as const) { + expect( + await checkFeishuChatContext(() => reader(async () => ({ externalChannel: null })), SESSION, command), + ).toBeNull(); + } + }); + + /** + * THE OPERATOR CASE, which must keep working: `chat open` is run from a human + * terminal with no chat context and possibly no agent configured at all. No + * chat id means the command is not running inside a chat, so there is nothing + * to check — and the reader is never even constructed, because building an + * SDK on that machine can legitimately fail. + */ + it("allows, without any lookup, when there is no chat context at all", async () => { + const factory = vi.fn(() => reader(async () => ({ externalChannel: "feishu" }))); + for (const session of [ + { chatId: undefined, agentId: undefined }, + { chatId: undefined, agentId: "agent-1" }, + { chatId: "", agentId: "agent-1" }, + ]) { + expect(await checkFeishuChatContext(factory, session, "open")).toBeNull(); + } + expect(factory).not.toHaveBeenCalled(); + }); + + /** + * The mirror image, and the second half of the reported fail-open: a chat + * context EXISTS but nothing can read it as the session agent. Skipping the + * check there made a half-configured environment the cheapest way past the + * guard. + */ + it("refuses when a chat id is present but no session agent can read it", async () => { + const factory = vi.fn(() => reader(async () => ({ externalChannel: "feishu" }))); + const refusal = await checkFeishuChatContext(factory, { chatId: "chat-1", agentId: undefined }, "create"); + + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE); + expect(refusal?.message).toContain("FIRST_TREE_AGENT_ID"); + expect(factory).not.toHaveBeenCalled(); + }); + + it("refuses when the reader cannot be constructed at all", async () => { + const refusal = await checkFeishuChatContext( + () => { + throw new Error("No agent configured on this machine"); + }, + SESSION, + "create", + ); + expect(refusal?.code).toBe(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE); + expect(refusal?.message).toContain("No agent configured"); + }); +}); + +describe("feishuChatContextMessage", () => { + it("explains why each command is wrong here, not just that it is refused", () => { + expect(feishuChatContextMessage("create")).toContain("nobody in the Feishu group can see"); + expect(feishuChatContextMessage("open")).toContain("interactive REPL"); + }); +}); + +/** + * Command-level wiring, where the actual bypass lived: the origin-chat lookup + * has to run under the SESSION identity, not under `--agent`. + */ +describe("`chat create` origin-chat resolution", () => { + const originalChatId = process.env.FIRST_TREE_CHAT_ID; + const originalAgentId = process.env.FIRST_TREE_AGENT_ID; + + /** Session agent sees the bridged origin chat; `other` is not a member of it. */ + function wireAgents(): { createTaskChat: ReturnType } { + const createTaskChat = vi.fn(async () => ({ chatId: "new-chat", messageId: "m1" })); + const sessionSdk = { + getChatDetail: vi.fn(async () => ({ externalChannel: "feishu" as const })), + createTaskChat, + }; + const overriddenSdk = { + getChatDetail: vi.fn(async () => { + throw Object.assign(new Error("Not a participant of this chat"), { statusCode: 403 }); + }), + createTaskChat, + }; + localAgentMocks.createSdk.mockImplementation((agentName?: string) => + agentName === undefined ? sessionSdk : overriddenSdk, + ); + return { createTaskChat }; + } + + async function runCreate(args: string[]): Promise { + const { registerChatCommands } = await import("../commands/chat/index.js"); + const program = new Command(); + program.exitOverride(); + program.configureOutput({ writeErr: () => undefined, writeOut: () => undefined }); + registerChatCommands(program); + await program.parseAsync(["node", "test", "chat", "create", ...args]); + } + + beforeEach(() => { + vi.clearAllMocks(); + docCaptureMock.captureOutboundDocs.mockImplementation(async (content: string) => ({ content })); + process.env.FIRST_TREE_CHAT_ID = "origin-chat"; + process.env.FIRST_TREE_AGENT_ID = "agent-session"; + }); + + afterEach(() => { + if (originalChatId === undefined) delete process.env.FIRST_TREE_CHAT_ID; + else process.env.FIRST_TREE_CHAT_ID = originalChatId; + if (originalAgentId === undefined) delete process.env.FIRST_TREE_AGENT_ID; + else process.env.FIRST_TREE_AGENT_ID = originalAgentId; + }); + + it("refuses `--agent ` from a bridged session instead of creating", async () => { + const { createTaskChat } = wireAgents(); + + await expect(runCreate(["hello", "--to", "someone", "--agent", "other"])).rejects.toThrow( + /bridged to a Feishu conversation/, + ); + expect(createTaskChat).not.toHaveBeenCalled(); + expect(outputMocks.fail).toHaveBeenCalledWith(FEISHU_CHAT_CONTEXT_CODE, expect.any(String), 2); + }); + + it("refuses rather than creating when the session lookup itself is inconclusive", async () => { + const createTaskChat = vi.fn(async () => ({ chatId: "new-chat", messageId: "m1" })); + localAgentMocks.createSdk.mockImplementation(() => ({ + getChatDetail: vi.fn(async () => { + throw new Error("connection refused"); + }), + createTaskChat, + })); + + await expect(runCreate(["hello", "--to", "someone"])).rejects.toThrow(/Could not determine/); + expect(createTaskChat).not.toHaveBeenCalled(); + expect(outputMocks.fail).toHaveBeenCalledWith(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE, expect.any(String), 2); + }); + + /** + * A session that names a chat but exports no agent id used to skip the check + * outright, so an incomplete environment was the cheapest way past it. + */ + it("refuses when the session names a chat but exports no agent id", async () => { + const { createTaskChat } = wireAgents(); + delete process.env.FIRST_TREE_AGENT_ID; + + await expect(runCreate(["hello", "--to", "someone"])).rejects.toThrow(/FIRST_TREE_AGENT_ID/); + expect(createTaskChat).not.toHaveBeenCalled(); + expect(outputMocks.fail).toHaveBeenCalledWith(FEISHU_CHAT_CONTEXT_UNKNOWN_CODE, expect.any(String), 2); + }); + + it("still creates under `--agent ` when the session chat is not bridged", async () => { + const createTaskChat = vi.fn(async () => ({ chatId: "new-chat", messageId: "m1" })); + const sessionSdk = { + getChatDetail: vi.fn(async () => ({ externalChannel: null })), + createTaskChat, + }; + // The overridden agent is still not a member of the origin chat; its + // inability to see that chat must not matter to an ordinary create. + const overriddenSdk = { + getChatDetail: vi.fn(async () => { + throw new Error("should never be consulted"); + }), + createTaskChat, + }; + localAgentMocks.createSdk.mockImplementation((agentName?: string) => + agentName === undefined ? sessionSdk : overriddenSdk, + ); + + await runCreate(["hello", "--to", "someone", "--agent", "other"]); + expect(createTaskChat).toHaveBeenCalledTimes(1); + expect(overriddenSdk.getChatDetail).not.toHaveBeenCalled(); + expect(sessionSdk.getChatDetail).toHaveBeenCalledWith("origin-chat"); + }); +}); diff --git a/apps/cli/src/commands/chat/create.ts b/apps/cli/src/commands/chat/create.ts index 41c1d0a7c..99779d829 100644 --- a/apps/cli/src/commands/chat/create.ts +++ b/apps/cli/src/commands/chat/create.ts @@ -3,6 +3,7 @@ import type { MessageFormat } from "@first-tree/shared"; import type { Command } from "commander"; import { fail, success } from "../../cli/output.js"; import { captureOutboundDocs } from "../../core/doc-capture.js"; +import { checkFeishuChatContext } from "../../core/feishu-chat-context.js"; import { createSdk, handleSdkError } from "../_shared/local-agent.js"; import { guardInlineDescription, readStdin } from "./_shared/io.js"; import { buildRequestMetadata } from "./_shared/request.js"; @@ -139,6 +140,33 @@ export function registerChatCreateCommand(chat: Command): void { } const sdk = createSdk(options.agent); + + // The server cannot enforce this one: `POST /agent/chats` never + // receives the originating chat. Refuse client-side so a Feishu-bound + // session does not spawn a First Tree chat its humans cannot see. + // + // The origin lookup deliberately runs under the SESSION identity + // (`createSdk()` resolves from FIRST_TREE_AGENT_ID), never under + // `--agent`. `--agent` picks who creates the new chat; it must not pick + // who answers "is the chat I am sitting in bridged?" — an overridden + // agent that is not a member of the origin chat gets a 403, and reading + // that as "not a Feishu chat" was a straight bypass of this refusal. + // Requiring the session agent here also keeps an unrelated agent's + // membership from becoming a precondition for ordinary creates. + // + // The whole decision lives in `checkFeishuChatContext`: no chat id at + // all is an operator terminal and proceeds without a lookup, while a + // chat id whose bridged-ness cannot be established — no session agent + // to read it as, an unreachable or older server, an unrecognised + // value — refuses. A half-configured environment must not be the + // cheapest way past the guard. + const refusal = await checkFeishuChatContext( + () => createSdk(), + { chatId: process.env.FIRST_TREE_CHAT_ID, agentId: process.env.FIRST_TREE_AGENT_ID }, + "create", + ); + if (refusal) fail(refusal.code, refusal.message, 2); + // KNOWN GAP (follow-up #1069), out of scope for this PR: no chat exists // yet, so the upload org can't be resolved from a chat — doc capture is a // pass-through for `chat create`'s initial message (doc mentions render as diff --git a/apps/cli/src/commands/chat/open.ts b/apps/cli/src/commands/chat/open.ts index ac8b4f094..c8c57be63 100644 --- a/apps/cli/src/commands/chat/open.ts +++ b/apps/cli/src/commands/chat/open.ts @@ -2,7 +2,9 @@ import type { Command } from "commander"; import { fail } from "../../cli/output.js"; import { ensureFreshAccessToken, resolveServerUrl } from "../../core/bootstrap.js"; import { cliFetch } from "../../core/cli-fetch.js"; +import { checkFeishuChatContext } from "../../core/feishu-chat-context.js"; import { print } from "../../core/output.js"; +import { createSdk } from "../_shared/local-agent.js"; import { resolveAgent } from "../_shared/resolve-agent.js"; export function registerChatOpenCommand(chat: Command): void { @@ -12,6 +14,21 @@ export function registerChatOpenCommand(chat: Command): void { .option("--server ", "First Tree server URL") .action(async (agentName: string, options: { server?: string }) => { try { + // Agent-session precondition. `chat open` runs on the user scope and + // drives an interactive REPL, so the server has no chat id to gate on + // and no way to tell an operator terminal from an agent. + // + // A human operator's terminal exports no FIRST_TREE_CHAT_ID and may + // have no agent configured at all: that case allows without building + // an SDK. A session that DOES name a chat is checked, and refuses when + // the answer cannot be established. + const refusal = await checkFeishuChatContext( + () => createSdk(), + { chatId: process.env.FIRST_TREE_CHAT_ID, agentId: process.env.FIRST_TREE_AGENT_ID }, + "open", + ); + if (refusal) fail(refusal.code, refusal.message, 2); + const serverUrl = resolveServerUrl(options.server); const adminToken = await ensureFreshAccessToken(); const headers = { diff --git a/apps/cli/src/core/feishu-chat-context.ts b/apps/cli/src/core/feishu-chat-context.ts new file mode 100644 index 000000000..51a8a5428 --- /dev/null +++ b/apps/cli/src/core/feishu-chat-context.ts @@ -0,0 +1,220 @@ +import type { ChatExternalChannel } from "@first-tree/shared"; + +/** + * Agent-session preconditions for the two chat commands the server cannot gate. + * + * `POST /agent/chats` never learns which chat the caller is sitting in — the + * originating chat is not in `createTaskChatSchema`, not a header, and the + * route has nothing to look up. `chat open` is worse: it runs on the user scope + * and starts an interactive REPL, which is meaningless in a non-interactive + * agent session. So both are refused client-side, before anything is created. + * + * The signal is `ChatDetail.externalChannel`, the same live `im_chat_bindings` + * state the server-side guard enforces — not `metadata.source`, which stays + * `"feishu"` after a binding detaches and would refuse commands the server + * would happily accept. + * + * FAIL CLOSED. An earlier revision treated any lookup failure as "not a Feishu + * chat" and proceeded. That was a real bypass, not just a rough edge: `chat + * create --agent ` ran the origin lookup as the overridden agent, and an + * agent that is not a member of the origin chat gets a 403 — which the + * fail-open path read as permission to create. Three changes close it: + * + * 1. The origin chat is resolved under the SESSION identity (see + * `chat create`), so the lookup is performed by an agent that can + * actually see the chat. `--agent` still chooses who creates the new + * chat; it no longer decides who is allowed to answer the origin + * question. An unrelated agent's membership never becomes a requirement + * for an ordinary create. + * 2. An inconclusive answer refuses instead of allowing. + * 3. Only an EXPLICIT `null` counts as "not bridged". A missing field, a + * value this CLI does not recognise, or a session that carries a chat id + * but no agent id are all `unknown`, because each of them is a state a + * rolling deploy actually produces: a server older than + * `externalChannel` omits it, and a half-configured session cannot read + * the chat as the session agent. Reading any of those as "ordinary chat" + * is how the guard silently switches itself off mid-deploy. + * + * Refusing on an inconclusive lookup costs nothing in practice: the lookup and + * the create talk to the same server with the same credentials, so a failure + * here means the create was going to fail anyway. All the refusal changes is + * that the operator gets a precise reason instead of a confusing downstream + * error — and in the one case where the lookup fails but the create would have + * succeeded, guessing is exactly what produced this bug. + * + * THE ONE ALLOWED SILENCE is "no chat context at all". `chat open` is a human + * operator's command; that terminal exports neither variable and may have no + * agent configured. Absent `FIRST_TREE_CHAT_ID` therefore means "not running + * inside a chat" and is allowed without any lookup. Present-but-unresolvable + * is the opposite case and refuses. + */ + +export const FEISHU_CHAT_CONTEXT_CODE = "FEISHU_CHAT_CONTEXT"; +export const FEISHU_CHAT_CONTEXT_UNKNOWN_CODE = "FEISHU_CHAT_CONTEXT_UNKNOWN"; + +export const FEISHU_GUARDED_COMMANDS = ["create", "open"] as const; +export type FeishuGuardedCommand = (typeof FEISHU_GUARDED_COMMANDS)[number]; + +/** + * Minimal SDK surface this check needs, so tests can supply a stub. + * `externalChannel` is optional here even though `ChatDetail` declares it: + * a server older than the field simply omits it from the JSON body, and the + * SDK does not re-parse the response through Zod. That is exactly why the + * resolver below treats "absent" as `unknown` rather than as `null`. + */ +export type ChatDetailReader = { + getChatDetail(chatId: string): Promise<{ externalChannel?: ChatExternalChannel | null }>; +}; + +/** + * Built lazily, because an operator terminal running `chat open` may have no + * agent configured at all — constructing an SDK there would fail on a machine + * where the command is perfectly legitimate. The factory runs only once a chat + * id proves there is a session to check. + */ +export type ChatDetailReaderFactory = () => ChatDetailReader; + +/** The agent-session environment the check reads its context from. */ +export type FeishuSessionContext = { + /** `FIRST_TREE_CHAT_ID` — the chat this session is running inside. */ + chatId: string | undefined; + /** `FIRST_TREE_AGENT_ID` — the identity that can read that chat. */ + agentId: string | undefined; +}; + +export type FeishuChatContextRefusal = { + code: string; + message: string; +}; + +/** + * Tri-state on purpose. Collapsing `unknown` into `unbridged` is precisely the + * fail-open that let `--agent ` through. + */ +export type FeishuChatContextState = { kind: "bridged" } | { kind: "unbridged" } | { kind: "unknown"; reason: string }; + +const REASONS: Record = { + create: + "`chat create` would open a First Tree task chat nobody in the Feishu group can see, and the new chat would " + + "carry no way back to them.", + open: "`chat open` starts an interactive REPL against a First Tree chat, which an agent session cannot drive.", +}; + +/** Build the refusal text for one guarded command in a confirmed bridged chat. */ +export function feishuChatContextMessage(command: FeishuGuardedCommand): string { + return ( + `This agent session is running inside a chat bridged to a Feishu conversation. ${REASONS[command]} ` + + "Reply in the Feishu conversation instead — record the delivery with `feishu intent`, then send it with the " + + "official `lark-cli --as bot`. To reach a First Tree teammate about this work, hand off from a chat that is " + + "not bridged." + ); +} + +/** Build the refusal text for an origin check that could not be completed. */ +export function feishuChatContextUnknownMessage(command: FeishuGuardedCommand, reason: string): string { + return ( + `Could not determine whether this agent session's chat is bridged to a Feishu conversation (${reason}). ` + + `\`chat ${command}\` is refused rather than guessed, because ${REASONS[command]} ` + + "Retry once the server is reachable and up to date; export FIRST_TREE_AGENT_ID so the chat can be read as the " + + "session agent; if this session is not attached to a chat at all, unset FIRST_TREE_CHAT_ID; or run the command " + + "from a session whose chat is not bridged." + ); +} + +function describeError(error: unknown): string { + if (error instanceof Error && error.message.length > 0) return error.message; + return String(error); +} + +/** + * Resolve whether the current session's chat is bridged to Feishu. + * + * The caller must pass an SDK bound to an identity that can actually read the + * chat — in practice the session agent, never a `--agent` override. + */ +export async function resolveFeishuChatContext(sdk: ChatDetailReader, chatId: string): Promise { + let detail: { externalChannel?: ChatExternalChannel | null }; + try { + detail = await sdk.getChatDetail(chatId); + } catch (error) { + return { kind: "unknown", reason: describeError(error) }; + } + if (detail === null || typeof detail !== "object") { + return { kind: "unknown", reason: "the server returned a chat detail this CLI cannot read" }; + } + const channel: unknown = detail.externalChannel; + // ONLY an explicit null is "this chat is not bridged". Everything else is a + // state we cannot interpret, and a guard that guesses in that state is not a + // guard. + if (channel === null) return { kind: "unbridged" }; + if (channel === "feishu") return { kind: "bridged" }; + if (channel === undefined) { + return { + kind: "unknown", + reason: + "the server did not report this chat's externalChannel — it is probably older than the field, " + + "which a rolling deploy makes temporary", + }; + } + return { + kind: "unknown", + reason: `the server reported an externalChannel this CLI does not recognise (${JSON.stringify(channel)})`, + }; +} + +/** + * Full precondition for a guarded command: returns the refusal to report, or + * `null` when the command may proceed. + * + * The distinction that matters is NOT "is an agent configured" but "is there a + * chat context at all": + * + * - no `FIRST_TREE_CHAT_ID` → the command is not running inside a chat. + * Ordinary operator terminal; allowed without a lookup, and the reader is + * never even constructed. + * - `FIRST_TREE_CHAT_ID` with no `FIRST_TREE_AGENT_ID` → there IS a chat + * context, but nothing can read it as the session agent, so its + * bridged-ness cannot be established. Refused as `unknown`, because + * skipping the check here used to make an incomplete environment the + * easiest way around the guard. + */ +export async function checkFeishuChatContext( + readerFactory: ChatDetailReaderFactory, + session: FeishuSessionContext, + command: FeishuGuardedCommand, +): Promise { + if (!session.chatId) return null; + + const state = await resolveSessionState(readerFactory, session); + if (state.kind === "unbridged") return null; + if (state.kind === "bridged") { + return { code: FEISHU_CHAT_CONTEXT_CODE, message: feishuChatContextMessage(command) }; + } + return { + code: FEISHU_CHAT_CONTEXT_UNKNOWN_CODE, + message: feishuChatContextUnknownMessage(command, state.reason), + }; +} + +async function resolveSessionState( + readerFactory: ChatDetailReaderFactory, + session: FeishuSessionContext, +): Promise { + const chatId = session.chatId; + if (!chatId) return { kind: "unbridged" }; + if (!session.agentId) { + return { + kind: "unknown", + reason: + "FIRST_TREE_CHAT_ID names a chat but FIRST_TREE_AGENT_ID is unset, so this CLI cannot read that chat " + + "as the session agent", + }; + } + let reader: ChatDetailReader; + try { + reader = readerFactory(); + } catch (error) { + return { kind: "unknown", reason: describeError(error) }; + } + return resolveFeishuChatContext(reader, chatId); +} diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 5256d5ff1..98f3d0308 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -649,6 +649,64 @@ If a non-human agent includes itself in `chat create --to`, the server records the originating agent in metadata and uses that agent's manager human as the effective sender so the first message can wake the agent normally. +### Chats bridged to a Feishu conversation + +A chat bound to a Feishu conversation is mirrored into that conversation, and +the people in it read Feishu — not the First Tree web app. A First Tree +*message* written into such a chat therefore reaches nobody on the other side. +To make that failure loud instead of silent, **messages and membership changes +are blocked** in a bridged chat: + +| Command | In a bridged chat | +| --- | --- | +| `chat send`, `chat ask` | refused, HTTP 403 `FEISHU_CHAT_AGENT_WRITE_FORBIDDEN` | +| `chat invite` | refused, same code | +| message edit, participant removal | refused, same code — same class as the two above | +| `chat create`, `chat open` | refused locally before anything is created | +| `chat update --topic/--description` | **works** — chat self-description, not a message | +| `chat archive` | **works** — private per-user view state | +| `chat list`, `chat history` | **works** — reads are unaffected | +| `feishu intent`, `feishu credential-env` (+ official `lark-cli --as bot`) | **works** — this is the delivery path | + +The Web app's own read, pin and archive state on a bridged chat keeps working +too; it is personal view state, not a change anyone else sees. + +This is not a blanket read-only mode: it blocks the operations that would +strand a human — a message no one receives, and a membership change to a room +no one can see — and leaves everything else alone. + +To answer a bridged conversation, record the delivery with `feishu intent` and +send it with the official `lark-cli --as bot`. To reach a First Tree teammate +about the work, hand off from a chat that is not bridged. + +The boundary follows the **live** binding, not the chat's origin label: once the +binding is detached the chat is no longer mirrored anywhere, and every command +above works normally again. The Web app applies the same rule to its own +structural writes. + +Refusals happen only after the caller's chat membership is verified, so the +error cannot be used to discover which chats are Feishu-bound. + +Operator-facing runtime notices ("the provider failed", "usage limit reached") +are exempt and still land in a bridged chat's First Tree history — an agent that +cannot run at all must not also go silent. The exemption belongs to a dedicated +server endpoint the client runtime posts to, so an ordinary `chat send` cannot +obtain it by describing itself as a notice. + +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. It is gated on chat membership and nothing more — the same gate as an +ordinary send — so it prevents the accidental and the careless, not the +determined. Its separate existence is still worth having: the ordinary send path +stays uniformly blocked, and the server authors the whole stored row so a notice +cannot quietly become an addressed message. Narrowing the capability further is +an open question, not something this boundary already does. + +During a rolling deploy the endpoint degrades in both directions rather than +dropping notices: a newer client falls back to the older wire shape when the +server has no such route, and a newer server still recognises that older shape +from a client that predates the endpoint. + --- ## doc @@ -2023,11 +2081,31 @@ agent process can talk to the server without extra setup: | Variable | Purpose | |---|---| | `FIRST_TREE_ACCESS_TOKEN` | The signed-in member's access JWT (short-lived). | -| `FIRST_TREE_AGENT_ID` | The agent's own UUID — the CLI uses it to identify the sender. | +| `FIRST_TREE_AGENT_ID` | The agent's own UUID — the CLI uses it to identify the sender. Also the identity that reads the session's own chat for the Feishu origin check below. | | `FIRST_TREE_CLIENT_ID` | The client (machine) the agent is bound to. | -| `FIRST_TREE_CHAT_ID` | The chat the current session is bound to. Used by `chat send` / `chat invite`, and by every `cron` command (including `preview` / `list` / `show`). | +| `FIRST_TREE_CHAT_ID` | The chat the current session is bound to. Used by `chat send` / `chat invite`, by every `cron` command (including `preview` / `list` / `show`), and by the Feishu origin check below. | | `FIRST_TREE_SERVER_URL` | Server URL override; falls back to client config. | +**The Feishu origin check on `chat create` / `chat open`.** These two commands +cannot be gated server-side — `chat create` never transmits the chat it is +being run from, and `chat open` runs on the user scope — so the CLI reads +`FIRST_TREE_CHAT_ID` and `FIRST_TREE_AGENT_ID` and asks the server whether that +chat is bridged to a Feishu conversation ([see above](#chats-bridged-to-a-feishu-conversation)). + +The check is deliberately conservative about what it treats as "not bridged": + +| Session | Behavior | +|---|---| +| No `FIRST_TREE_CHAT_ID` | Allowed without any lookup — an operator terminal is not running inside a chat, and needs no agent configured. | +| Both variables set, chat not bridged | Allowed. | +| Both variables set, chat bridged | Refused with `FEISHU_CHAT_CONTEXT`. | +| `FIRST_TREE_CHAT_ID` set, `FIRST_TREE_AGENT_ID` unset | Refused with `FEISHU_CHAT_CONTEXT_UNKNOWN` — there is a chat context, but nothing that can read it. | +| Lookup failed, or the server did not report the chat's channel | Refused with `FEISHU_CHAT_CONTEXT_UNKNOWN` — including against a server older than the field, so a rolling deploy cannot silently disable the check. | + +An `unknown` refusal names what to fix: export `FIRST_TREE_AGENT_ID`, unset +`FIRST_TREE_CHAT_ID` if the session is not attached to a chat, retry once the +server is reachable, or run from a chat that is not bridged. + ### Server (SaaS internal) These configure the SaaS server image (`packages/server/dist/index.mjs`) diff --git a/packages/client/src/__tests__/agent-slot-shutdown-settlement.test.ts b/packages/client/src/__tests__/agent-slot-shutdown-settlement.test.ts index cac50110c..0d846887f 100644 --- a/packages/client/src/__tests__/agent-slot-shutdown-settlement.test.ts +++ b/packages/client/src/__tests__/agent-slot-shutdown-settlement.test.ts @@ -138,7 +138,9 @@ describe("AgentSlot/SessionRuntime shutdown settlement authority", () => { const { server, baseUrl } = await listen(async (req, res) => { const runtimeSessionToken = req.headers[AGENT_RUNTIME_SESSION_HEADER.toLowerCase()] as string | undefined; - if (req.method === "POST" && req.url?.includes("/messages")) { + // Runtime notices have their own route; the server authors the delivery + // profile and the stored marker, so only the text is posted here. + if (req.method === "POST" && req.url?.includes("/runtime-notices")) { await readBody(req); const record = { runtimeSessionToken, path: req.url ?? "" }; if (runtimeSessionToken !== CURRENT) { @@ -275,7 +277,9 @@ describe("AgentSlot/SessionRuntime shutdown settlement authority", () => { const { server, baseUrl } = await listen(async (req, res) => { const runtimeSessionToken = req.headers[AGENT_RUNTIME_SESSION_HEADER.toLowerCase()] as string | undefined; - if (req.method === "POST" && req.url?.includes("/messages")) { + // Runtime notices have their own route; the server authors the delivery + // profile and the stored marker, so only the text is posted here. + if (req.method === "POST" && req.url?.includes("/runtime-notices")) { await readBody(req); const record = { runtimeSessionToken, path: req.url ?? "" }; if (runtimeSessionToken !== CURRENT) { diff --git a/packages/client/src/__tests__/chat-context.test.ts b/packages/client/src/__tests__/chat-context.test.ts index d00f42830..6ac75b4cc 100644 --- a/packages/client/src/__tests__/chat-context.test.ts +++ b/packages/client/src/__tests__/chat-context.test.ts @@ -39,6 +39,7 @@ function mkChatDetail(overrides?: Partial): ChatDetail { viewerMembershipKind: "participant", descriptionUpdatedAt: null, lastReadAt: null, + externalChannel: null, ...overrides, }; } diff --git a/packages/client/src/__tests__/runtime-notice.test.ts b/packages/client/src/__tests__/runtime-notice.test.ts index 88562b29c..1b6866a60 100644 --- a/packages/client/src/__tests__/runtime-notice.test.ts +++ b/packages/client/src/__tests__/runtime-notice.test.ts @@ -1,6 +1,6 @@ -import { type ProviderRetryEventPayload, RUNTIME_NOTICE_METADATA_KEY } from "@first-tree/shared"; -import { describe, expect, it, vi } from "vitest"; -import { FirstTreeHubSDK } from "../cloud/sdk.js"; +import type { ProviderRetryEventPayload } from "@first-tree/shared"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstTreeHubSDK, SdkError } from "../cloud/sdk.js"; import { formatProviderFailureRuntimeNotice, isEgressForbiddenText, @@ -153,9 +153,9 @@ describe("runtime notice formatting", () => { expect(isEgressForbiddenText("Request not allowed")).toBe(false); }); - it("sends the formatted notice as final API text with runtime metadata", async () => { + it("publishes the formatted notice through the dedicated runtime-notice endpoint", async () => { const sdk = new FirstTreeHubSDK({ serverUrl: "https://first-tree.test", getAccessToken: () => "token" }); - const sendMessage = vi.spyOn(sdk, "sendMessage").mockResolvedValue({ + const postRuntimeNotice = vi.spyOn(sdk, "postRuntimeNotice").mockResolvedValue({ id: "msg-1", chatId: "chat-1", senderId: "agent-1", @@ -171,12 +171,88 @@ describe("runtime notice formatting", () => { await postProviderFailureRuntimeNotice(sdk, "chat-1", payload({ messagePreview: "refresh token revoked" })); - expect(sendMessage).toHaveBeenCalledWith("chat-1", { + // Only the text travels: the server authors source/format/purpose and the + // stored runtimeNotice marker, so a notice cannot quietly become an + // addressed message. + expect(postRuntimeNotice).toHaveBeenCalledWith("chat-1", expect.stringContaining("refresh token revoked")); + }); +}); + +/** + * ROLLING DEPLOY, new client → old server. The runtime is upgraded + * independently of the server it talks to, so a runtime that knows the + * dedicated endpoint will meet servers that do not. A provider-failure notice + * is most valuable precisely then, so a 404 must degrade to the older wire + * shape rather than drop the notice. + */ +describe("runtime notice endpoint compatibility", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function jsonResponse(data: unknown, status = 200): Response { + return new Response(JSON.stringify(data), { status, headers: { "content-type": "application/json" } }); + } + + function storedMessage(): Record { + return { + id: "msg-1", + chatId: "chat-1", + senderId: "agent-1", + senderKind: "member", + senderProvider: null, + format: "text", + content: "notice", + metadata: {}, + inReplyTo: null, + source: "api", + createdAt: "2026-07-09T00:00:00.000Z", + }; + } + + function makeSdk(): FirstTreeHubSDK { + return new FirstTreeHubSDK({ + serverUrl: "https://first-tree.example", + agentId: "agent-1", + getAccessToken: () => "access-token", + }); + } + + it("falls back to the legacy send shape when the server has no runtime-notice route", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ error: "Route POST:/api/v1/... not found" }, 404)) + .mockResolvedValueOnce(jsonResponse(storedMessage(), 201)); + vi.stubGlobal("fetch", fetchMock); + + const message = await makeSdk().postRuntimeNotice("chat-1", "provider failed"); + + expect(message.id).toBe("msg-1"); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(String(fetchMock.mock.calls[0]?.[0])).toContain("/runtime-notices"); + const fallbackUrl = String(fetchMock.mock.calls[1]?.[0]); + expect(fallbackUrl).toContain("/chats/chat-1/messages"); + // Exactly the body the server recognises as a legacy runtime notice; the + // two sides share `legacyRuntimeNoticeSendBody` so they cannot drift. + expect(JSON.parse(String(fetchMock.mock.calls[1]?.[1]?.body))).toEqual({ source: "api", format: "text", - content: expect.stringContaining("refresh token revoked"), - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, + content: "provider failed", + metadata: { runtimeNotice: true }, purpose: "agent-final-text", }); }); + + it("does not reshape a genuine refusal into an ordinary send", async () => { + const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ error: "Not a participant of this chat" }, 403)); + vi.stubGlobal("fetch", fetchMock); + + await expect(makeSdk().postRuntimeNotice("chat-1", "provider failed")).rejects.toBeInstanceOf(SdkError); + // Only 404 means "this server predates the route". Anything else must not + // be retried as a plain message, which in a bridged chat would be refused + // anyway and elsewhere would land mislabelled. + for (const call of fetchMock.mock.calls) { + expect(String(call[0])).toContain("/runtime-notices"); + } + }); }); diff --git a/packages/client/src/__tests__/session-manager-edge-coverage.test.ts b/packages/client/src/__tests__/session-manager-edge-coverage.test.ts index 4631e73cf..87cc2c73e 100644 --- a/packages/client/src/__tests__/session-manager-edge-coverage.test.ts +++ b/packages/client/src/__tests__/session-manager-edge-coverage.test.ts @@ -183,6 +183,7 @@ function mockSdk(): FirstTreeHubSDK { serverUrl: "https://first-tree.example.test", register: vi.fn(), sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice: vi.fn().mockResolvedValue({ id: "runtime-notice" }), sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), listChatParticipants: vi.fn().mockResolvedValue([ { agentId: "sender-1", role: "member", mode: "full", name: "alice", displayName: "Alice", type: "human" }, @@ -1881,7 +1882,7 @@ describe("SessionRuntime edge coverage", () => { let enteredToken: Parameters[1] | undefined; let enteredMessage: SessionMessage | undefined; let injectCount = 0; - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-inject-tail" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-inject-tail" }); const activeHandler = handler({ start: vi.fn().mockImplementation(async (message, ctx) => { initialCtx = ctx; @@ -1929,7 +1930,7 @@ describe("SessionRuntime edge coverage", () => { }); const ackEntry = vi.fn<(entryId: number) => Promise>().mockResolvedValue(undefined); const sdk = mockSdk(); - vi.mocked(sdk.sendMessage).mockImplementation(sendMessage); + vi.mocked(sdk.postRuntimeNotice).mockImplementation(postRuntimeNotice); const sm = makeRuntime({ handlers: [activeHandler], ackEntry, @@ -1970,7 +1971,7 @@ describe("SessionRuntime edge coverage", () => { reason: "unsafe_replay", }); expect(disposition).toBe("settled"); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry.mock.calls.filter((call) => call[0] === 2)).toHaveLength(1); await sm.handleCommand(chatId, "session:suspend"); @@ -2163,7 +2164,7 @@ describe("SessionRuntime edge coverage", () => { }); const ackEntry = vi.fn<(entryId: number) => Promise>().mockResolvedValue(undefined); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockImplementation(async () => { + const postRuntimeNotice = vi.fn().mockImplementation(async () => { signalNoticeStarted?.(); await noticeGate; return { id: "runtime-notice-message" }; @@ -2172,7 +2173,7 @@ describe("SessionRuntime edge coverage", () => { handlers: [routedHandler], ackEntry, recoverChat, - sdk: { ...mockSdk(), sendMessage } as unknown as FirstTreeHubSDK, + sdk: { ...mockSdk(), postRuntimeNotice } as unknown as FirstTreeHubSDK, }); const i = internals(sm); const chatId = "chat-stale-notice-post"; @@ -3464,8 +3465,8 @@ describe("SessionRuntime edge coverage", () => { }); const ackEntry = vi.fn<(entryId: number) => Promise>().mockResolvedValue(undefined); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-after-timeout" }); - const sdk = { ...mockSdk(), sendMessage } as unknown as FirstTreeHubSDK; + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-after-timeout" }); + const sdk = { ...mockSdk(), postRuntimeNotice } as unknown as FirstTreeHubSDK; const sm = makeRuntime({ handlers: [oldHandler], ackEntry, recoverChat, sdk }); const i = internals(sm); const chatId = "chat-timeout-terminal-notice-debt"; @@ -3486,16 +3487,16 @@ describe("SessionRuntime edge coverage", () => { // the retained notice debt without re-entering the provider. await sm.dispatch(headEntry); expect(recoverChat).toHaveBeenCalledTimes(1); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(ackEntry).not.toHaveBeenCalled(); await sm.dispatch(headEntry); await vi.waitFor(() => expect(ackEntry).toHaveBeenCalledTimes(1)); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(9110); expect(oldHandler.start).toHaveBeenCalledTimes(1); - const noticeOrder = sendMessage.mock.invocationCallOrder[0]; + const noticeOrder = postRuntimeNotice.mock.invocationCallOrder[0]; const ackOrder = ackEntry.mock.invocationCallOrder[0]; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ACK order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -7402,8 +7403,8 @@ describe("SessionRuntime edge coverage", () => { it("retries consumed error completions when runtime notice delivery and failure-event emit both fail", async () => { const ackEntry = vi.fn<(entryId: number) => Promise>().mockResolvedValue(undefined); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockRejectedValue(new Error("notice store offline")); - const sdk = { ...mockSdk(), sendMessage } as unknown as FirstTreeHubSDK; + const postRuntimeNotice = vi.fn().mockRejectedValue(new Error("notice store offline")); + const sdk = { ...mockSdk(), postRuntimeNotice } as unknown as FirstTreeHubSDK; let capturedToken: Parameters[2] | undefined; let capturedMessage: SessionMessage | undefined; const started = handler({ @@ -7444,7 +7445,7 @@ describe("SessionRuntime edge coverage", () => { reason: "provider_failed", }); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).not.toHaveBeenCalled(); await vi.waitFor(() => expect(recoverChat).toHaveBeenCalledWith("chat-notice-emit-fail")); await sm.shutdown(); diff --git a/packages/client/src/__tests__/session-manager-more-coverage.test.ts b/packages/client/src/__tests__/session-manager-more-coverage.test.ts index ae54486fa..5d9a98f67 100644 --- a/packages/client/src/__tests__/session-manager-more-coverage.test.ts +++ b/packages/client/src/__tests__/session-manager-more-coverage.test.ts @@ -33,6 +33,7 @@ function mockSdk(overrides: Record = {}): FirstTreeHubSDK { return { register: vi.fn(), sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice: vi.fn().mockResolvedValue({ id: "runtime-notice" }), sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatDetail: vi.fn().mockResolvedValue({ organizationId: "org-1" }), ...overrides, @@ -617,8 +618,8 @@ describe("SessionRuntime additional delivery token and payload coverage", () => it("retries terminalRejected instead of ACKing when the durable runtime notice cannot be posted", async () => { const ackEntry = mockAckEntry(); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockRejectedValue(new Error("notice write failed")); - const sdk = mockSdk({ sendMessage }); + const postRuntimeNotice = vi.fn().mockRejectedValue(new Error("notice write failed")); + const sdk = mockSdk({ postRuntimeNotice }); let capturedCtx: SessionContext | undefined; let capturedToken: DeliveryToken | undefined; let capturedMessage: SessionMessage | undefined; @@ -664,7 +665,7 @@ describe("SessionRuntime additional delivery token and payload coverage", () => messageId: "runtime-notice-error", }); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).not.toHaveBeenCalled(); await vi.waitFor(() => expect(recoverChat).toHaveBeenCalledWith("chat-terminal-notice-fail")); expect( @@ -681,8 +682,8 @@ describe("SessionRuntime additional delivery token and payload coverage", () => it("ignores malformed provider retry event payloads when completing a consumed error", async () => { const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); - const sdk = mockSdk({ sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const sdk = mockSdk({ postRuntimeNotice }); let capturedCtx: SessionContext | undefined; let capturedToken: DeliveryToken | undefined; let capturedMessage: SessionMessage | undefined; @@ -707,7 +708,7 @@ describe("SessionRuntime additional delivery token and payload coverage", () => reason: "provider_clean_error", }); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(ackEntry).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(403); diff --git a/packages/client/src/__tests__/session-manager.test.ts b/packages/client/src/__tests__/session-manager.test.ts index 1bdced051..43aaf902f 100644 --- a/packages/client/src/__tests__/session-manager.test.ts +++ b/packages/client/src/__tests__/session-manager.test.ts @@ -6,7 +6,6 @@ import { type AgentRuntimeConfig, encodeProviderRetryEventMessage, parseProviderRetryEventMessage, - RUNTIME_NOTICE_METADATA_KEY, type RuntimeState, type SessionEvent, } from "@first-tree/shared"; @@ -38,6 +37,7 @@ function mockSdk(): FirstTreeHubSDK { return { register: vi.fn(), sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice: vi.fn().mockResolvedValue({ id: "runtime-notice" }), sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; } @@ -2212,10 +2212,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable runtime notice before ACKing a terminal provider failure", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2251,22 +2252,17 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { }); expect(completionDisposition).toBe("settled"); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage).toHaveBeenCalledWith( - "chat-provider-terminal", - expect.objectContaining({ - source: "api", - format: "text", - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", - }), - ); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + // The notice goes out through the dedicated runtime-notice endpoint, which + // authors the delivery profile and the stored marker server-side; only the + // chat id and the text travel from here. + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledWith("chat-provider-terminal", expect.any(String)); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("Codex could not run this turn"); expect(notice).toContain("credentials need attention"); expect(notice).toContain("refresh token was revoked"); expect(ackEntry).toHaveBeenCalledWith(21); - const [noticeOrder] = sendMessage.mock.invocationCallOrder; + const [noticeOrder] = postRuntimeNotice.mock.invocationCallOrder; const [ackOrder] = ackEntry.mock.invocationCallOrder; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ack order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -2276,10 +2272,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable Pi auth/capability notice before ACKing terminal failure", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2312,15 +2309,15 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { }); expect(completionDisposition).toBe("settled"); - expect(sendMessage).toHaveBeenCalledTimes(1); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("Pi could not run this turn"); expect(notice).toContain("credentials need attention"); expect(notice).toContain("run `pi`"); expect(notice).toContain("`/login`"); expect(notice).toContain("missing credentials"); expect(ackEntry).toHaveBeenCalledWith(31); - const [noticeOrder] = sendMessage.mock.invocationCallOrder; + const [noticeOrder] = postRuntimeNotice.mock.invocationCallOrder; const [ackOrder] = ackEntry.mock.invocationCallOrder; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ack order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -2330,10 +2327,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable runtime notice before ACKing a Codex retry-exhausted turn once", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2367,10 +2365,10 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { reason: "provider_retry_exhausted", }); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(27); - const [noticeOrder] = sendMessage.mock.invocationCallOrder; + const [noticeOrder] = postRuntimeNotice.mock.invocationCallOrder; const [ackOrder] = ackEntry.mock.invocationCallOrder; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ack order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -2380,10 +2378,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable runtime notice for Claude provider-turn terminal failures", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2420,14 +2419,14 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { }); expect(completionDisposition).toBe("settled"); - expect(sendMessage).toHaveBeenCalledTimes(1); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("Claude Code could not run this turn"); expect(notice).toContain("before authentication"); expect(notice).toContain("daemon.env"); expect(notice).not.toContain("rejected the local Claude authentication"); expect(ackEntry).toHaveBeenCalledWith(24); - const [noticeOrder] = sendMessage.mock.invocationCallOrder; + const [noticeOrder] = postRuntimeNotice.mock.invocationCallOrder; const [ackOrder] = ackEntry.mock.invocationCallOrder; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ack order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -2437,10 +2436,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable runtime notice for Claude retry-exhausted provider-turn failures", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2479,8 +2479,8 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { reason: "retry_exhausted_notice_posted", }); - expect(sendMessage).toHaveBeenCalledTimes(1); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("provider API connection failed after retry handling"); expect(notice).toContain("socket connection was closed unexpectedly"); expect(ackEntry).toHaveBeenCalledWith(25); @@ -2490,10 +2490,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("posts a durable runtime notice before ACKing Claude auto-resume failures", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2531,13 +2532,13 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { reason: "auto_resume_failed_notice_posted", }); - expect(sendMessage).toHaveBeenCalledTimes(1); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("provider API connection failed after retry handling"); expect(notice).toContain("initial sdk transport crash"); expect(notice).toContain("respawn build failed"); expect(ackEntry).toHaveBeenCalledWith(26); - const [noticeOrder] = sendMessage.mock.invocationCallOrder; + const [noticeOrder] = postRuntimeNotice.mock.invocationCallOrder; const [ackOrder] = ackEntry.mock.invocationCallOrder; if (noticeOrder === undefined || ackOrder === undefined) throw new Error("expected notice and ack order"); expect(noticeOrder).toBeLessThan(ackOrder); @@ -2549,10 +2550,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { const ackEntry = vi.fn().mockResolvedValue(undefined); const recoverChat = vi.fn().mockResolvedValue(undefined); const recoverRuntimeSessionProof = vi.fn().mockRejectedValue(new Error("bind temporarily rejected")); - const sendMessage = vi.fn().mockResolvedValue({ id: "must-not-post" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "must-not-post" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2593,7 +2595,7 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { // for bind recovery), so the completion disposition must be "retry", // never "settled". expect(heldDisposition).toBe("retry"); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(ackEntry).not.toHaveBeenCalled(); expect(recoverChat).not.toHaveBeenCalled(); expect(recoverRuntimeSessionProof).toHaveBeenCalledTimes(1); @@ -2648,7 +2650,7 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { expect(recoverRuntimeSessionProof).toHaveBeenCalledWith("runtime_session_invalid"); expect(recoverChat).not.toHaveBeenCalled(); expect(ackEntry).not.toHaveBeenCalled(); - expect(vi.mocked(sdk.sendMessage)).not.toHaveBeenCalled(); + expect(vi.mocked(sdk.postRuntimeNotice)).not.toHaveBeenCalled(); await sm.shutdown(); }); @@ -2657,14 +2659,15 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { const ackEntry = vi.fn().mockResolvedValue(undefined); const recoverChat = vi.fn().mockResolvedValue(undefined); const recoverRuntimeSessionProof = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockRejectedValue( + const postRuntimeNotice = vi.fn().mockRejectedValue( new SdkError(403, "Missing x-agent-runtime-session header", { code: AGENT_RUNTIME_SESSION_ERROR_CODES.MISSING, }), ); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2700,7 +2703,7 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { reason: "provider_credential_required", }); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(recoverRuntimeSessionProof).toHaveBeenCalledWith("runtime_session_missing"); expect(recoverChat).not.toHaveBeenCalled(); expect(ackEntry).not.toHaveBeenCalled(); @@ -2711,10 +2714,11 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { it("does not ACK a terminal provider failure when the durable runtime notice cannot be posted", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); const recoverChat = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockRejectedValue(new Error("send failed")); + const postRuntimeNotice = vi.fn().mockRejectedValue(new Error("notice post failed")); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; const emitted: SessionEvent[] = []; @@ -2752,7 +2756,7 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { }); expect(failedNoticeCompletionDisposition).toBe("retry"); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).not.toHaveBeenCalled(); expect(recoverChat).toHaveBeenCalledWith("chat-provider-notice-fail"); expect( @@ -2763,25 +2767,26 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { event.payload.message.includes("runtime failure notice delivery failed"), ), ).toBe(true); - sendMessage.mockReset(); - sendMessage.mockResolvedValue({ id: "later-runtime-notice" }); + postRuntimeNotice.mockReset(); + postRuntimeNotice.mockResolvedValue({ id: "later-runtime-notice" }); await capturedCtx.finishTurn(capturedMessage, { status: "error", terminal: true, completion: "consumed", reason: "forward_failed", }); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); await sm.shutdown(); }); it("clears stale terminal provider notices when the delivery is retried instead of consumed", async () => { const ackEntry = vi.fn().mockResolvedValue(undefined); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const sdk = { register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), } as unknown as FirstTreeHubSDK; let capturedCtx: SessionContext | undefined; @@ -2815,7 +2820,7 @@ describe("SessionRuntime ackEntry callback (deferred ack)", () => { reason: "forward_failed", }); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(ackEntry).not.toHaveBeenCalled(); await sm.shutdown(); diff --git a/packages/client/src/__tests__/session-start-error-signaling.test.ts b/packages/client/src/__tests__/session-start-error-signaling.test.ts index 81f563fce..12e4c79c1 100644 --- a/packages/client/src/__tests__/session-start-error-signaling.test.ts +++ b/packages/client/src/__tests__/session-start-error-signaling.test.ts @@ -37,8 +37,10 @@ import { mockEntry } from "./test-helpers.js"; function mockSdk(): { sdk: FirstTreeHubSDK; sendMessage: ReturnType; + postRuntimeNotice: ReturnType; } { const sendMessage = vi.fn().mockResolvedValue({ id: "msg-reply" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice" }); const listChatParticipants = vi.fn().mockResolvedValue([ { agentId: "agent-1", role: "member", mode: "full", name: "agent", displayName: "Agent", type: "agent" }, { agentId: "user-1", role: "member", mode: "full", name: "user", displayName: "User", type: "human" }, @@ -47,10 +49,12 @@ function mockSdk(): { sdk: { register: vi.fn(), sendMessage, + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), listChatParticipants, } as unknown as FirstTreeHubSDK, sendMessage, + postRuntimeNotice, }; } diff --git a/packages/client/src/__tests__/test-helpers.ts b/packages/client/src/__tests__/test-helpers.ts index afa47b4fd..5e50b57ed 100644 --- a/packages/client/src/__tests__/test-helpers.ts +++ b/packages/client/src/__tests__/test-helpers.ts @@ -11,7 +11,7 @@ import type { SessionMessage } from "../runtime/handler.js"; * chat — it only closes the turn trigger. The agent's text is captured via * `assistant_text` events, not this hook. A handler test that needs to assert * an EXPLICIT chat write (e.g. the codex usage-limit runtime notice) mocks - * `sdk.sendMessage` on the ctx directly, not through here. + * `sdk.postRuntimeNotice` on the ctx directly, not through here. * * The stubbed name-resolution path returns the raw senderId — the production * `[From: ]` path is covered separately in `agent-io.test.ts`. diff --git a/packages/client/src/cloud/sdk.ts b/packages/client/src/cloud/sdk.ts index 54f55a439..6a2c1bf95 100644 --- a/packages/client/src/cloud/sdk.ts +++ b/packages/client/src/cloud/sdk.ts @@ -74,6 +74,7 @@ import { type ListDocCommentsResponse, type ListDocsResponse, legacyContextActivationResponseSchema, + legacyRuntimeNoticeSendBody, listMeChatsResponseSchema, type Message, type OrgContextTreeFeaturesOutput, @@ -83,6 +84,7 @@ import { orgContextTreeOutputSchema, type PublishDocRequest, type PublishDocResponse, + type RuntimeNoticeRequest, type RuntimeProvider, type SendMessage, type UnfollowChatGitlabEntityResponse, @@ -440,6 +442,35 @@ export class FirstTreeHubSDK { }); } + /** + * Publish an operator-facing runtime notice ("the provider failed", "the + * usage limit is reached"). + * + * Its own endpoint rather than a decorated `sendMessage`: a runtime notice is + * the one agent write that must still land in a Feishu-bridged chat, and + * putting it on its own route keeps the ordinary send path uniformly guarded + * while letting the server author the whole delivery profile. Only the text + * travels — the endpoint rejects an attempt to pass `purpose` or `metadata`. + * + * ROLLING DEPLOY. A client is upgraded independently of the server it talks + * to, so this runtime may well be newer than the server. An older server has + * no such route and answers 404; falling back to the legacy send shape keeps + * the notice from being dropped in exactly the window where an operator most + * needs to see it. The fallback narrows to 404 so a real failure (403, 5xx) + * still surfaces rather than being retried as something else. + */ + async postRuntimeNotice(chatId: string, content: string): Promise { + try { + return await this.requestJson(`/api/v1/agent/chats/${encodeURIComponent(chatId)}/runtime-notices`, { + method: "POST", + body: JSON.stringify({ content } satisfies RuntimeNoticeRequest), + }); + } catch (error) { + if (!(error instanceof SdkError) || error.statusCode !== 404) throw error; + return this.sendMessage(chatId, legacyRuntimeNoticeSendBody(content)); + } + } + async createAgentOutboxToken(chatId: string): Promise<{ accessToken: string; expiresIn: number }> { return this.requestJson(`/api/v1/agent/chats/${encodeURIComponent(chatId)}/outbox-token`, { method: "POST", diff --git a/packages/client/src/providers/codex/__tests__/app-server/extra-coverage.test.ts b/packages/client/src/providers/codex/__tests__/app-server/extra-coverage.test.ts index 7deda2730..e5ed5106e 100644 --- a/packages/client/src/providers/codex/__tests__/app-server/extra-coverage.test.ts +++ b/packages/client/src/providers/codex/__tests__/app-server/extra-coverage.test.ts @@ -293,6 +293,7 @@ function makeContext( const sendMessage = vi .fn<(chatId: string, body: Record) => Promise>() .mockResolvedValue(undefined); + const postRuntimeNotice = vi.fn<(chatId: string, content: string) => Promise>().mockResolvedValue({}); const createAgentOutboxToken = vi .fn<(chatId: string) => Promise<{ accessToken: string; expiresIn: number }>>() .mockResolvedValue({ accessToken: "scoped-outbox-token", expiresIn: 900 }); @@ -310,6 +311,7 @@ function makeContext( sdk: { serverUrl: "http://test", sendMessage, + postRuntimeNotice, createAgentOutboxToken, getAgentContextTreeConfig: async () => opts.contextTreeRepoUrl @@ -999,8 +1001,8 @@ describe("codex app-server handler extra branches", () => { const successHandler = makeHandler(successFake); const successLog = vi.fn<(message: string) => void>(); const successCtx = makeContext({ log: successLog }); - const successSendMessage = vi.fn().mockResolvedValue(sentMessageResponse()); - successCtx.sdk.sendMessage = successSendMessage; + const successNotice = vi.fn().mockResolvedValue(sentMessageResponse()); + successCtx.sdk.postRuntimeNotice = successNotice; const successStart = successHandler.start(makeMessage("m1", "first"), successCtx, successToken); await waitFor(() => successFake.requests.some((request) => request.method === "turn/start"), "usage turn/start"); @@ -1015,15 +1017,9 @@ describe("codex app-server handler extra branches", () => { }); await successStart; - expect(successSendMessage).toHaveBeenCalledWith( - "chat-app-server-extra", - expect.objectContaining({ - source: "api", - format: "text", - purpose: "agent-final-text", - metadata: { runtimeNotice: true }, - }), - ); + // The notice takes the dedicated runtime-notice route, which authors the + // delivery profile and the stored marker server-side. + expect(successNotice).toHaveBeenCalledWith("chat-app-server-extra", expect.stringContaining("usage limit")); expect(successToken.complete).toHaveBeenCalledWith([makeMessage("m1", "first")], { status: "error", terminal: true, @@ -1043,7 +1039,7 @@ describe("codex app-server handler extra branches", () => { const failureToken = makeDeliveryToken(); const failureHandler = makeHandler(failureFake); const failureCtx = makeContext(); - failureCtx.sdk.sendMessage = vi.fn(async () => { + failureCtx.sdk.postRuntimeNotice = vi.fn(async () => { throw new Error("chat write failed"); }); diff --git a/packages/client/src/providers/codex/__tests__/usage-limit.test.ts b/packages/client/src/providers/codex/__tests__/usage-limit.test.ts index 4f3fefa7f..5a9008d2c 100644 --- a/packages/client/src/providers/codex/__tests__/usage-limit.test.ts +++ b/packages/client/src/providers/codex/__tests__/usage-limit.test.ts @@ -1,7 +1,7 @@ import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { RUNTIME_NOTICE_METADATA_KEY, type SessionEvent } from "@first-tree/shared"; +import type { SessionEvent } from "@first-tree/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { mockCtxPlumbing } from "../../../__tests__/test-helpers.js"; import type { ChatContext } from "../../../runtime/chat-context.js"; @@ -144,6 +144,7 @@ const trialAgentMetadata = { }; type SendMessageMock = ReturnType) => Promise>>; +type RuntimeNoticeMock = ReturnType Promise>>; function makeMessage(id: string, content: string, inboxEntryId?: number): SessionMessage { return { @@ -161,6 +162,7 @@ function makeContext( onFinishTurn: (count?: number, outcome?: { status: "success" | "error"; reason?: string }) => void, opts: { sendMessage?: SendMessageMock; + postRuntimeNotice?: RuntimeNoticeMock; emitEvent?: SessionContext["emitEvent"]; emitEventConfirmed?: SessionContext["emitEventConfirmed"]; failSessionForRecovery?: SessionContext["failSessionForRecovery"]; @@ -172,6 +174,8 @@ function makeContext( const sendMessage = opts.sendMessage ?? vi.fn<(chatId: string, body: Record) => Promise>().mockResolvedValue(undefined); + const postRuntimeNotice = + opts.postRuntimeNotice ?? vi.fn<(chatId: string, content: string) => Promise>().mockResolvedValue({}); return { agent: { agentId: AGENT_ID, @@ -182,7 +186,7 @@ function makeContext( delegateMention: null, metadata: opts.agentMetadata ?? {}, }, - sdk: { serverUrl: "http://test", sendMessage } as unknown as SessionContext["sdk"], + sdk: { serverUrl: "http://test", sendMessage, postRuntimeNotice } as unknown as SessionContext["sdk"], chatId: "chat-usage-limit", log: opts.log ?? (() => {}), recordProviderActivity: () => {}, @@ -192,8 +196,8 @@ function makeContext( ...mockCtxPlumbing({ sendMessage }, "chat-usage-limit"), // Production-faithful: the final-text forward is retired, so it delivers // nothing. (mockCtxPlumbing's stub would proxy to sendMessage and mask - // that — the usage-limit notice is delivered by an EXPLICIT sdk.sendMessage - // in the handler, NOT through this path.) + // that — the usage-limit notice is delivered by an EXPLICIT + // sdk.postRuntimeNotice in the handler, NOT through this path.) forwardResult: async () => {}, retryTurn: opts.retryTurn ?? (() => {}), finishTurn: async (messages, outcome) => { @@ -225,6 +229,7 @@ describe("codex usage-limit empty-turn (issue #971)", () => { const sendMessage = vi .fn<(chatId: string, body: Record) => Promise>() .mockResolvedValue(undefined); + const postRuntimeNotice = vi.fn<(chatId: string, content: string) => Promise>().mockResolvedValue({}); const emitEvent = vi.fn<(event: SessionEvent) => void>(); const handler = createCodexHandler({ runtimeProvider: "codex", @@ -233,6 +238,7 @@ describe("codex usage-limit empty-turn (issue #971)", () => { }); const ctx = makeContext((count) => completedCounts.push(count), { sendMessage, + postRuntimeNotice, emitEvent, log: (message) => logs.push(message), }); @@ -241,13 +247,14 @@ describe("codex usage-limit empty-turn (issue #971)", () => { const events = emitEvent.mock.calls.map(([event]) => event); - // Layer 1-A: a chat-visible notice is posted by an EXPLICIT sdk.sendMessage - // (NOT the retired final-text forward), carrying the agent-final-text - // delivery profile so it lands recipientless without waking anyone. - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(String(sendMessage.mock.calls[0]?.[1].content)).toContain("usage limit"); - expect(sendMessage.mock.calls[0]?.[1].purpose).toBe("agent-final-text"); - expect(sendMessage.mock.calls[0]?.[1].metadata).toMatchObject({ [RUNTIME_NOTICE_METADATA_KEY]: true }); + // Layer 1-A: a chat-visible notice is posted by an EXPLICIT + // sdk.postRuntimeNotice (NOT the retired final-text forward, and NOT a + // decorated sendMessage). The dedicated endpoint authors the delivery + // profile server-side so it lands recipientless without waking anyone. + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice.mock.calls[0]?.[0]).toBe("chat-usage-limit"); + expect(String(postRuntimeNotice.mock.calls[0]?.[1])).toContain("usage limit"); // Layer 2: an `error` event is emitted (daemon log + admin stream), and a // warn-style log line is recorded — not a phantom success. @@ -287,6 +294,7 @@ describe("codex usage-limit empty-turn (issue #971)", () => { const sendMessage = vi .fn<(chatId: string, body: Record) => Promise>() .mockResolvedValue(undefined); + const postRuntimeNotice = vi.fn<(chatId: string, content: string) => Promise>().mockResolvedValue({}); const emitEvent = vi.fn<(event: SessionEvent) => void>(); const handler = createCodexHandler({ runtimeProvider: "codex", @@ -295,6 +303,7 @@ describe("codex usage-limit empty-turn (issue #971)", () => { }); const ctx = makeContext((count) => completedCounts.push(count), { sendMessage, + postRuntimeNotice, emitEvent, log: (message) => logs.push(message), }); @@ -304,6 +313,7 @@ describe("codex usage-limit empty-turn (issue #971)", () => { const events = emitEvent.mock.calls.map(([event]) => event); // No notice, no usage-limit error/log — a chosen silence is left alone. + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(sendMessage).not.toHaveBeenCalled(); expect( events.some((event) => event.kind === "error" && event.payload.message.includes("codex usage limit reached")), diff --git a/packages/client/src/providers/codex/app-server/index.ts b/packages/client/src/providers/codex/app-server/index.ts index 7b1c51aa3..3d317a1d2 100644 --- a/packages/client/src/providers/codex/app-server/index.ts +++ b/packages/client/src/providers/codex/app-server/index.ts @@ -4,7 +4,6 @@ import { type AgentRuntimeConfigPayload, encodeProviderRetryEventMessage, isLandingCampaignTrialAgentMetadata, - RUNTIME_NOTICE_METADATA_KEY, runtimeProviderSchema, type SessionEvent, type ToolFileRef, @@ -1399,17 +1398,11 @@ export const createCodexAppServerHandler: HandlerFactory = (config: HandlerConfi ); // Post the usage-limit notice as a deliberate, chat-visible runtime // message — an EXPLICIT send, not the retired final-text forward - // (`forwardResult` no longer delivers). It rides the `agent-final-text` - // purpose only for its delivery profile (recipientless, notify=false, - // bypasses the group @mention guard). + // (`forwardResult` no longer delivers). It goes through the dedicated + // runtime-notice endpoint, which owns the recipientless notify=false + // delivery profile and the server-side `runtimeNotice` marker. try { - await sessionCtx.sdk.sendMessage(sessionCtx.chatId, { - source: "api", - format: "text", - content: USAGE_LIMIT_NOTICE, - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", - }); + await sessionCtx.sdk.postRuntimeNotice(sessionCtx.chatId, USAGE_LIMIT_NOTICE); consumedErrorReason = "usage_limit_notice_posted"; } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/packages/client/src/providers/codex/sdk.ts b/packages/client/src/providers/codex/sdk.ts index b5ad74726..3cd04f138 100644 --- a/packages/client/src/providers/codex/sdk.ts +++ b/packages/client/src/providers/codex/sdk.ts @@ -5,7 +5,6 @@ import { deriveRepoLocalPath, encodeProviderRetryEventMessage, isLandingCampaignTrialAgentMetadata, - RUNTIME_NOTICE_METADATA_KEY, runtimeProviderSchema, type SessionEvent, type ToolFileRef, @@ -1129,16 +1128,11 @@ export const createCodexSdkHandler: HandlerFactory = (config) => { // sees WHY their message got no reply, rather than digging through codex // rollout files. This is a deliberate, EXPLICIT send — NOT the retired // final-text forward (`forwardResult` no longer delivers anything). It - // rides the `agent-final-text` purpose only for its delivery profile - // (recipientless, notify=false, bypasses the group @mention guard). + // goes through the dedicated runtime-notice endpoint, which owns the + // recipientless notify=false delivery profile and the server-side + // `runtimeNotice` marker. try { - await sessionCtx.sdk.sendMessage(sessionCtx.chatId, { - source: "api", - format: "text", - content: USAGE_LIMIT_NOTICE, - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", - }); + await sessionCtx.sdk.postRuntimeNotice(sessionCtx.chatId, USAGE_LIMIT_NOTICE); consumedErrorReason = "usage_limit_notice_posted"; } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/packages/client/src/providers/opencode/__tests__/handler.test.ts b/packages/client/src/providers/opencode/__tests__/handler.test.ts index 47f492fa1..e660f7d7f 100644 --- a/packages/client/src/providers/opencode/__tests__/handler.test.ts +++ b/packages/client/src/providers/opencode/__tests__/handler.test.ts @@ -1067,10 +1067,11 @@ describe("OpenCode V1 handler", () => { ); const ackEntry = vi.fn<(entryId: number) => Promise>(async () => {}); const recoverChat = vi.fn<(chatId: string) => Promise>(async () => {}); - const sendMessage = vi.fn(async () => ({ id: "runtime-notice" })); + const postRuntimeNotice = vi.fn(async () => ({ id: "runtime-notice" })); const sdk = { serverUrl: "https://first-tree.test", - sendMessage, + sendMessage: vi.fn(async () => ({ id: "msg-reply" })), + postRuntimeNotice, getChatDetail: vi.fn(async () => ({ id: "chat-sm-retry", title: "Retry chat", @@ -1142,7 +1143,7 @@ describe("OpenCode V1 handler", () => { } expect(sleep.mock.calls.map(([delay]) => delay)).toEqual([5_000, 15_000]); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(802); await manager.shutdown(); }); @@ -1208,10 +1209,11 @@ describe("OpenCode V1 handler", () => { ]); const ackEntry = vi.fn<(entryId: number) => Promise>(async () => {}); const recoverChat = vi.fn<(chatId: string) => Promise>(async () => {}); - const sendMessage = vi.fn(async () => ({ id: "runtime-notice" })); + const postRuntimeNotice = vi.fn(async () => ({ id: "runtime-notice" })); const sdk = { serverUrl: "https://first-tree.test", - sendMessage, + sendMessage: vi.fn(async () => ({ id: "msg-reply" })), + postRuntimeNotice, getChatDetail: vi.fn(async (chatId: string) => ({ id: chatId, title: "Retry preemption chat", @@ -1278,7 +1280,7 @@ describe("OpenCode V1 handler", () => { await vi.waitFor(() => expect(ackEntry).toHaveBeenCalledWith(811)); expect(sleep.mock.calls.map(([delay]) => delay)).toEqual([5_000, 15_000]); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); await manager.shutdown(); }); @@ -1330,13 +1332,14 @@ describe("OpenCode V1 handler", () => { const supervisor = createProtocolSupervisor([], [`${credentialOutput}\n`, `${successfulTurn()}\n`], inputs); const ackEntry = vi.fn<(entryId: number) => Promise>(async () => {}); const recoverChat = vi.fn<(chatId: string) => Promise>(async () => {}); - const sendMessage = vi + const postRuntimeNotice = vi .fn() .mockRejectedValueOnce(new Error("runtime notice write failed")) .mockResolvedValue({ id: "runtime-notice" }); const sdk = { serverUrl: "https://first-tree.test", - sendMessage, + sendMessage: vi.fn(async () => ({ id: "msg-reply" })), + postRuntimeNotice, getChatDetail: vi.fn(async () => ({ id: "chat-sm-notice", title: "Notice chat", @@ -1392,8 +1395,8 @@ describe("OpenCode V1 handler", () => { await manager.dispatch(entry); await vi.waitFor(() => expect(ackEntry).toHaveBeenCalledWith(802)); - expect(sendMessage).toHaveBeenCalledTimes(2); - expect(sendMessage.mock.invocationCallOrder[1] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(2); + expect(postRuntimeNotice.mock.invocationCallOrder[1] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[0] as number, ); expect(inputs).toHaveLength(1); diff --git a/packages/client/src/providers/pi/__tests__/session-custody.test.ts b/packages/client/src/providers/pi/__tests__/session-custody.test.ts index c836f40a9..bf84104c2 100644 --- a/packages/client/src/providers/pi/__tests__/session-custody.test.ts +++ b/packages/client/src/providers/pi/__tests__/session-custody.test.ts @@ -3,7 +3,7 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "no import { tmpdir } from "node:os"; import { join } from "node:path"; import type { AgentRuntimeConfig, SessionEvent } from "@first-tree/shared"; -import { encodeProviderRetryEventMessage, RUNTIME_NOTICE_METADATA_KEY } from "@first-tree/shared"; +import { encodeProviderRetryEventMessage } from "@first-tree/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { silentLogger } from "../../../__tests__/_logger-helpers.js"; import { mockEntry } from "../../../__tests__/test-helpers.js"; @@ -360,11 +360,12 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("exhausted_retry"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi" }); const sdk = { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK; @@ -407,22 +408,16 @@ describe("Pi handler → SessionRuntime custody", () => { await sm.dispatch(mockEntry({ id: 77, chatId: "chat-pi-exhausted", messageId: "msg-pi-exhausted", content: "go" })); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(1); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage).toHaveBeenCalledWith( - "chat-pi-exhausted", - expect.objectContaining({ - source: "api", - format: "text", - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", - }), - ); - const notice = String(sendMessage.mock.calls[0]?.[1].content); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + // The dedicated runtime-notice endpoint carries only the chat id and the + // text; the server authors the delivery profile and the stored marker. + expect(postRuntimeNotice).toHaveBeenCalledWith("chat-pi-exhausted", expect.any(String)); + const notice = String(postRuntimeNotice.mock.calls[0]?.[1]); expect(notice).toContain("Pi could not run this turn"); expect(notice).not.toContain("provider overloaded"); expect(notice).not.toContain("temporary provider blip"); expect(ackEntry).toHaveBeenCalledWith(77); - const noticeOrder = sendMessage.mock.invocationCallOrder[0]; + const noticeOrder = postRuntimeNotice.mock.invocationCallOrder[0]; const ackOrder = ackEntry.mock.invocationCallOrder[0]; expect(noticeOrder).toBeTypeOf("number"); expect(ackOrder).toBeTypeOf("number"); @@ -434,7 +429,7 @@ describe("Pi handler → SessionRuntime custody", () => { function makePiSessionRuntime(input: { specs: ProviderProcessSpec[]; ackEntry: ReturnType; - sendMessage: ReturnType; + postRuntimeNotice: ReturnType; onHandler?: (handler: ReturnType) => void; recoverChat?: (chatId: string) => Promise; registryPath?: string; @@ -442,7 +437,8 @@ describe("Pi handler → SessionRuntime custody", () => { const sdk = { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage: input.sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice: input.postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK; @@ -494,13 +490,13 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("bash_hold_until_abort"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-shutdown" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-shutdown" }); const shutdownCalls: Array<{ reason?: string; opts?: { settleProviderEntered?: boolean } }> = []; const sm = makePiSessionRuntime({ specs, ackEntry, - sendMessage, + postRuntimeNotice, onHandler: (handler) => { const original = handler.shutdown.bind(handler); handler.shutdown = async (reason, opts) => { @@ -534,8 +530,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(Number(readFileSync(bashStartCountFile, "utf8")) || 0).toBe(1); expect(ackEntry).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(88); - expect(sendMessage).toHaveBeenCalledTimes(1); - const noticeOrder = sendMessage.mock.invocationCallOrder[0]; + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const noticeOrder = postRuntimeNotice.mock.invocationCallOrder[0]; const ackOrder = ackEntry.mock.invocationCallOrder[0]; expect(noticeOrder as number).toBeLessThan(ackOrder as number); // Deferred start receipt must not adopt/resurrect after manager drain. @@ -546,8 +542,8 @@ describe("Pi handler → SessionRuntime custody", () => { it("deferred resume race: manager shutdown settles accepted token without resume adoption", async () => { const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-resume" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-resume" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); await sm.dispatch(mockEntry({ id: 70, chatId: "chat-pi-resume-race", messageId: "msg-first", content: "first" })); expect(ackEntry).toHaveBeenCalledWith(70); @@ -569,8 +565,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(ackEntry).toHaveBeenCalledWith(71); expect(ackEntry.mock.calls.filter((call) => call[0] === 71)).toHaveLength(1); - expect(sendMessage).toHaveBeenCalledTimes(1); - const noticeOrder = sendMessage.mock.invocationCallOrder[0]; + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const noticeOrder = postRuntimeNotice.mock.invocationCallOrder[0]; const ack71Index = ackEntry.mock.calls.findIndex((call) => call[0] === 71); const ack71Order = ackEntry.mock.invocationCallOrder[ack71Index]; expect(noticeOrder as number).toBeLessThan(ack71Order as number); @@ -583,11 +579,12 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("preflight_capacity"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); const sdk = { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK; @@ -673,7 +670,7 @@ describe("Pi handler → SessionRuntime custody", () => { const sessionEvents: SessionEvent[] = []; const registryPath = join(workspaceRoot, "sessions-deferred.json"); const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-deferred" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-deferred" }); const forwardCalls: string[] = []; const mutationCalls: string[] = []; @@ -766,7 +763,8 @@ describe("Pi handler → SessionRuntime custody", () => { sdk: { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK, @@ -785,8 +783,8 @@ describe("Pi handler → SessionRuntime custody", () => { await dispatchPromise; expect(ackEntry).toHaveBeenCalledWith(92); - expect(sendMessage).toHaveBeenCalledTimes(1); - const noticeOrder = sendMessage.mock.invocationCallOrder[0]; + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + const noticeOrder = postRuntimeNotice.mock.invocationCallOrder[0]; const ackOrder = ackEntry.mock.invocationCallOrder[0]; expect(noticeOrder as number).toBeLessThan(ackOrder as number); // Non-terminal session events must not cross the drain fence. @@ -816,8 +814,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("prompt_write_tool_no_response"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-write-gap" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-write-gap" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const dispatchPromise = sm.dispatch( mockEntry({ @@ -837,8 +835,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(Number(readFileSync(bashStartCountFile, "utf8")) || 0).toBe(1); expect(ackEntry).toHaveBeenCalledTimes(1); expect(ackEntry).toHaveBeenCalledWith(93); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage.mock.invocationCallOrder[0] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice.mock.invocationCallOrder[0] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[0] as number, ); expect(sm.totalCount).toBe(0); @@ -849,8 +847,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("prompt_accepted_no_events"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-no-events" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-no-events" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const dispatchPromise = sm.dispatch( mockEntry({ @@ -868,8 +866,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(1); expect(ackEntry).toHaveBeenCalledWith(94); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage.mock.invocationCallOrder[0] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice.mock.invocationCallOrder[0] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[0] as number, ); expect(sm.totalCount).toBe(0); @@ -889,7 +887,7 @@ describe("Pi handler → SessionRuntime custody", () => { const sessionEvents: SessionEvent[] = []; const registryPath = join(workspaceRoot, "sessions-operator-suspend-deferred.json"); const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-operator-deferred" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-operator-deferred" }); const forwardCalls: string[] = []; const mutationCalls: string[] = []; @@ -986,7 +984,8 @@ describe("Pi handler → SessionRuntime custody", () => { sdk: { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK, @@ -1010,8 +1009,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(ackEntry).toHaveBeenCalledWith(97); expect(ackEntry.mock.calls.filter((call) => call[0] === 97)).toHaveLength(1); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage.mock.invocationCallOrder[0] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice.mock.invocationCallOrder[0] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[0] as number, ); expect(sessionEvents.some((event) => event.kind === "assistant_text")).toBe(false); @@ -1040,13 +1039,13 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("prompt_write_tool_no_response"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-operator-suspend" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-operator-suspend" }); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); const suspendCalls: Array<{ reason?: string; opts?: { settleProviderEntered?: boolean } }> = []; const sm = makePiSessionRuntime({ specs, ackEntry, - sendMessage, + postRuntimeNotice, recoverChat, onHandler: (handler) => { const original = handler.suspend.bind(handler); @@ -1079,8 +1078,8 @@ describe("Pi handler → SessionRuntime custody", () => { expect(suspendCalls.some((call) => call.opts?.settleProviderEntered === true)).toBe(true); expect(ackEntry).toHaveBeenCalledWith(96); expect(ackEntry.mock.calls.filter((call) => call[0] === 96)).toHaveLength(1); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(sendMessage.mock.invocationCallOrder[0] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice.mock.invocationCallOrder[0] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[0] as number, ); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(1); @@ -1123,7 +1122,7 @@ describe("Pi handler → SessionRuntime custody", () => { }; const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "unused" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "unused" }); const sm = new SessionRuntime({ session: { idle_timeout: 300, @@ -1155,7 +1154,8 @@ describe("Pi handler → SessionRuntime custody", () => { sdk: { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK, @@ -1182,7 +1182,7 @@ describe("Pi handler → SessionRuntime custody", () => { expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(0); expect(ackEntry).not.toHaveBeenCalled(); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); await sm.shutdown(); }); @@ -1194,9 +1194,9 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("happy"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-active-inject" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-active-inject" }); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, recoverChat }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, recoverChat }); const chatId = "chat-pi-active-inject-suspend"; await sm.dispatch( @@ -1233,10 +1233,10 @@ describe("Pi handler → SessionRuntime custody", () => { ).projection.sessions.get(chatId)?.suspending; await Promise.all([suspending ?? Promise.resolve(), injectPromise]); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry.mock.calls.filter((call) => call[0] === 111)).toHaveLength(1); const injectAckIndex = ackEntry.mock.calls.findIndex((call) => call[0] === 111); - expect(sendMessage.mock.invocationCallOrder[0] as number).toBeLessThan( + expect(postRuntimeNotice.mock.invocationCallOrder[0] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[injectAckIndex] as number, ); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(promptsBeforeInject + 1); @@ -1263,11 +1263,11 @@ describe("Pi handler → SessionRuntime custody", () => { const recoverChat = vi.fn<(chatId: string) => Promise>().mockImplementation(async () => { await recoverGate; }); - const sendMessage = vi + const postRuntimeNotice = vi .fn() .mockRejectedValueOnce(new Error("runtime notice store offline")) .mockResolvedValue({ id: "runtime-notice-after-recovery" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, recoverChat }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, recoverChat }); const chatId = "chat-pi-active-inject-notice-recover"; const injectEntry = mockEntry({ id: 113, @@ -1299,7 +1299,7 @@ describe("Pi handler → SessionRuntime custody", () => { ).projection.sessions.get(chatId)?.suspending; await Promise.all([suspending ?? Promise.resolve(), injectPromise]); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry.mock.calls.filter((call) => call[0] === 113)).toHaveLength(0); expect(recoverChat).toHaveBeenCalledWith(chatId); expect( @@ -1321,8 +1321,8 @@ describe("Pi handler → SessionRuntime custody", () => { await sm.dispatch(injectEntry); await vi.waitFor(() => expect(ackEntry.mock.calls.filter((call) => call[0] === 113)).toHaveLength(1)); - expect(sendMessage).toHaveBeenCalledTimes(2); - expect(sendMessage.mock.invocationCallOrder[1] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(2); + expect(postRuntimeNotice.mock.invocationCallOrder[1] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[ackEntry.mock.calls.findIndex((call) => call[0] === 113)] as number, ); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(promptsBeforeInject + 1); @@ -1343,12 +1343,12 @@ describe("Pi handler → SessionRuntime custody", () => { await recoverGate; }); // Notice #1 (suspend) fails; #2 (after recovery #1) fails; #3 (after recovery #2) succeeds. - const sendMessage = vi + const postRuntimeNotice = vi .fn() .mockRejectedValueOnce(new Error("runtime notice store offline")) .mockRejectedValueOnce(new Error("runtime notice store still offline")) .mockResolvedValue({ id: "runtime-notice-after-recovery-2" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, recoverChat }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, recoverChat }); const chatId = "chat-pi-active-inject-notice-recover-twice"; const injectEntry = mockEntry({ id: 119, @@ -1383,7 +1383,7 @@ describe("Pi handler → SessionRuntime custody", () => { ).projection.sessions.get(chatId)?.suspending; await Promise.all([suspending ?? Promise.resolve(), injectPromise]); - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(postRuntimeNotice).toHaveBeenCalledTimes(1); expect(ackEntry.mock.calls.filter((call) => call[0] === 119)).toHaveLength(0); expect(recoverChat).toHaveBeenCalledTimes(1); expect(hasDebt()).toBe(true); @@ -1393,7 +1393,7 @@ describe("Pi handler → SessionRuntime custody", () => { // Redelivery after recovery #1 — notice #2 fails; must not ACK or re-enter Pi. await sm.dispatch(injectEntry); - await vi.waitFor(() => expect(sendMessage).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => expect(postRuntimeNotice).toHaveBeenCalledTimes(2)); await Promise.resolve(); await Promise.resolve(); expect(ackEntry.mock.calls.filter((call) => call[0] === 119)).toHaveLength(0); @@ -1416,8 +1416,8 @@ describe("Pi handler → SessionRuntime custody", () => { // Redelivery after recovery #2 — notice #3 succeeds; sole ACK after notice. await sm.dispatch(injectEntry); await vi.waitFor(() => expect(ackEntry.mock.calls.filter((call) => call[0] === 119)).toHaveLength(1)); - expect(sendMessage).toHaveBeenCalledTimes(3); - expect(sendMessage.mock.invocationCallOrder[2] as number).toBeLessThan( + expect(postRuntimeNotice).toHaveBeenCalledTimes(3); + expect(postRuntimeNotice.mock.invocationCallOrder[2] as number).toBeLessThan( ackEntry.mock.invocationCallOrder[ackEntry.mock.calls.findIndex((call) => call[0] === 119)] as number, ); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(promptsBeforeInject + 1); @@ -1438,8 +1438,8 @@ describe("Pi handler → SessionRuntime custody", () => { const recoverChat = vi.fn<(chatId: string) => Promise>().mockImplementation(async () => { await recoverGate; }); - const sendMessage = vi.fn().mockRejectedValue(new Error("runtime notice store offline")); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, recoverChat }); + const postRuntimeNotice = vi.fn().mockRejectedValue(new Error("runtime notice store offline")); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, recoverChat }); const chatId = "chat-pi-active-inject-notice-fail-persist"; const injectEntry = mockEntry({ id: 121, @@ -1481,7 +1481,7 @@ describe("Pi handler → SessionRuntime custody", () => { // After recovery #1, redelivery notice fails again → debt, no ACK, no Pi replay. await sm.dispatch(injectEntry); - await vi.waitFor(() => expect(sendMessage.mock.calls.length).toBeGreaterThanOrEqual(2)); + await vi.waitFor(() => expect(postRuntimeNotice.mock.calls.length).toBeGreaterThanOrEqual(2)); await Promise.resolve(); await Promise.resolve(); expect(ackEntry.mock.calls.filter((call) => call[0] === 121)).toHaveLength(0); @@ -1503,7 +1503,7 @@ describe("Pi handler → SessionRuntime custody", () => { // Recovery #2 completes; next redelivery notice still fails → still requestable. await sm.dispatch(injectEntry); - await vi.waitFor(() => expect(sendMessage.mock.calls.length).toBeGreaterThanOrEqual(3)); + await vi.waitFor(() => expect(postRuntimeNotice.mock.calls.length).toBeGreaterThanOrEqual(3)); await Promise.resolve(); await Promise.resolve(); expect(ackEntry.mock.calls.filter((call) => call[0] === 121)).toHaveLength(0); @@ -1542,7 +1542,7 @@ describe("Pi handler → SessionRuntime custody", () => { }; const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "unused" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "unused" }); const recoverChat = vi.fn<(chatId: string) => Promise>().mockResolvedValue(undefined); const sm = new SessionRuntime({ session: { @@ -1575,7 +1575,8 @@ describe("Pi handler → SessionRuntime custody", () => { sdk: { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK, @@ -1610,7 +1611,10 @@ describe("Pi handler → SessionRuntime custody", () => { expect(refreshCount).toBe(1); expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(promptsAfterEstablish + 1); - expect(sendMessage).not.toHaveBeenCalled(); + // `main` rewrote this case; the "no operator-facing write happened" leg now + // watches the dedicated runtime-notice endpoint, which is where a notice + // goes on this branch. + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(recoverChat).not.toHaveBeenCalled(); await sm.handleCommand(chatId, "session:suspend"); @@ -1642,7 +1646,7 @@ describe("Pi handler → SessionRuntime custody", () => { }; const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "unused" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "unused" }); const sm = new SessionRuntime({ session: { idle_timeout: 300, @@ -1674,7 +1678,8 @@ describe("Pi handler → SessionRuntime custody", () => { sdk: { serverUrl: "https://first-tree.test", register: vi.fn(), - sendMessage, + sendMessage: vi.fn().mockResolvedValue({ id: "msg-reply" }), + postRuntimeNotice, sendToAgent: vi.fn().mockResolvedValue({ id: "msg-dm" }), getChatContext: vi.fn().mockResolvedValue(null), } as unknown as FirstTreeHubSDK, @@ -1700,7 +1705,7 @@ describe("Pi handler → SessionRuntime custody", () => { expect(Number(readFileSync(promptCountFile, "utf8")) || 0).toBe(0); expect(ackEntry).not.toHaveBeenCalled(); - expect(sendMessage).not.toHaveBeenCalled(); + expect(postRuntimeNotice).not.toHaveBeenCalled(); expect(sm.totalCount).toBe(0); expect(sm.activeCount).toBe(0); }); @@ -1709,8 +1714,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("bash_hold_until_abort"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-steer" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-steer" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const headPromise = sm.dispatch( mockEntry({ @@ -1756,8 +1761,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("hold_get_state_until_gate"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-fifo" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-fifo" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const headPromise = sm.dispatch( mockEntry({ @@ -1801,8 +1806,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("get_state_fail"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-prereadiness" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-prereadiness" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const headPromise = sm.dispatch( mockEntry({ @@ -1840,8 +1845,8 @@ describe("Pi handler → SessionRuntime custody", () => { setPiTestMode("hold_get_state_until_gate"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-suspend-defer" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-suspend-defer" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const headPromise = sm.dispatch( mockEntry({ @@ -1880,8 +1885,8 @@ describe("Pi handler → SessionRuntime custody", () => { writeFileSync(getStateFailRemainFile, "1"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-retry-steer" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-retry-steer" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); // First dispatch returns after the transient failure schedules retry — it // does not await the winning retry turn. Observe custody via wire counts/ACK. @@ -1926,8 +1931,8 @@ describe("Pi handler → SessionRuntime custody", () => { writeFileSync(promptGateFile, "0"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-retry-fifo" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-retry-fifo" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); void sm.dispatch( mockEntry({ @@ -1968,8 +1973,8 @@ describe("Pi handler → SessionRuntime custody", () => { it("suspend/resume keeps the persisted Pi session identity across spawns", async () => { const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); await sm.dispatch(mockEntry({ id: 301, chatId: "chat-pi-continuity", messageId: "msg-cont-1", content: "first" })); expect(ackEntry).toHaveBeenCalledWith(301); @@ -1990,8 +1995,8 @@ describe("Pi handler → SessionRuntime custody", () => { const registryPath = join(workspaceRoot, "sessions-reset.json"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, registryPath }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, registryPath }); await sm.dispatch( mockEntry({ id: 311, chatId: "chat-pi-reset", messageId: "msg-reset-1", content: "before reset" }), @@ -2029,8 +2034,8 @@ describe("Pi handler → SessionRuntime custody", () => { writeFileSync(getStateFailRemainFile, "1"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-reidentity" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-pi-reidentity" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice }); const dispatchPromise = sm.dispatch( mockEntry({ id: 321, chatId: "chat-pi-reidentity", messageId: "msg-reidentity", content: "run sleep 60" }), @@ -2054,8 +2059,8 @@ describe("Pi handler → SessionRuntime custody", () => { const registryPath = join(workspaceRoot, "sessions-reset-flush.json"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); - const sm = makePiSessionRuntime({ specs, ackEntry, sendMessage, registryPath }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const sm = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, registryPath }); await sm.dispatch( mockEntry({ id: 331, chatId: "chat-pi-reset-flush", messageId: "msg-flush-1", content: "before reset" }), @@ -2102,8 +2107,8 @@ describe("Pi handler → SessionRuntime custody", () => { const specs: ProviderProcessSpec[] = []; // Persistent ACK failure leaves the settled first row as recovery debt. const ackEntry = vi.fn<(entryId: number) => Promise>().mockRejectedValue(new Error("ack offline")); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); - const sm1 = makePiSessionRuntime({ specs, ackEntry, sendMessage, registryPath }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const sm1 = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, registryPath }); const chatId = "chat-pi-reset-redelivery"; const messageId = "msg-reset-redelivery"; @@ -2147,7 +2152,7 @@ describe("Pi handler → SessionRuntime custody", () => { // New SessionRuntime (client restart): in-memory terminal ledger is gone. // Same durable inbox row is redelivered; ACK is available this time. const ackEntry2 = mockAckEntry(); - const sm2 = makePiSessionRuntime({ specs, ackEntry: ackEntry2, sendMessage, registryPath }); + const sm2 = makePiSessionRuntime({ specs, ackEntry: ackEntry2, postRuntimeNotice, registryPath }); await sm2.dispatch(mockEntry({ id: entryId, chatId, messageId, content: "settled but unacked" })); expect(ackEntry2).toHaveBeenCalledWith(entryId); @@ -2165,7 +2170,7 @@ describe("Pi handler → SessionRuntime custody", () => { const registryPath = join(workspaceRoot, "sessions-reset-fence.json"); const specs: ProviderProcessSpec[] = []; const ackEntry = mockAckEntry(); - const sendMessage = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); + const postRuntimeNotice = vi.fn().mockResolvedValue({ id: "runtime-notice-unused" }); let consecutiveNoProgressResets = 0; let lastResetSignature: string | null = null; let noProgressCircuitOpen = false; @@ -2179,7 +2184,7 @@ describe("Pi handler → SessionRuntime custody", () => { throw new Error("recover_failed: no-progress circuit open"); } }); - const sm1 = makePiSessionRuntime({ specs, ackEntry, sendMessage, registryPath, recoverChat }); + const sm1 = makePiSessionRuntime({ specs, ackEntry, postRuntimeNotice, registryPath, recoverChat }); const chatId = "chat-pi-reset-fence"; await sm1.dispatch(mockEntry({ id: 351, chatId, messageId: "msg-fence-old", content: "establish identity" })); diff --git a/packages/client/src/runtime/runtime-notice.ts b/packages/client/src/runtime/runtime-notice.ts index e4c37683f..21475c430 100644 --- a/packages/client/src/runtime/runtime-notice.ts +++ b/packages/client/src/runtime/runtime-notice.ts @@ -1,7 +1,6 @@ import { type ProviderRetryEventPayload, type ProviderRetryScope, - RUNTIME_NOTICE_METADATA_KEY, type RuntimeProvider, runtimeProviderLabel, } from "@first-tree/shared"; @@ -26,18 +25,21 @@ export function formatProviderFailureRuntimeNotice(payload: ProviderRetryEventPa return detail.length > 0 ? `${lead} Original provider message: ${detail}` : lead; } +/** + * Publish the notice through the dedicated runtime-notice endpoint. + * + * Not `sendMessage` with a `runtimeNotice` metadata flag any more: that flag is + * what exempts the write from the Feishu-bridged chat boundary, and a request + * body is not a place to keep a capability — any agent credential could set the + * same fields. The server now owns the marker and the delivery profile, and + * grants the exemption to this route rather than to a shape of body. + */ export async function postProviderFailureRuntimeNotice( sdk: FirstTreeHubSDK, chatId: string, payload: ProviderRetryEventPayload, ): Promise { - await sdk.sendMessage(chatId, { - source: "api", - format: "text", - content: formatProviderFailureRuntimeNotice(payload), - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", - }); + await sdk.postRuntimeNotice(chatId, formatProviderFailureRuntimeNotice(payload)); } function actionLabel(scope: ProviderRetryScope): string { diff --git a/packages/qa/cases/cross-surface/feishu-agent-channel.md b/packages/qa/cases/cross-surface/feishu-agent-channel.md index af7a96096..f59bd9ea3 100644 --- a/packages/qa/cases/cross-surface/feishu-agent-channel.md +++ b/packages/qa/cases/cross-surface/feishu-agent-channel.md @@ -1,6 +1,6 @@ --- id: feishu-agent-channel -description: Validate a Bot-bound Agent's Feishu registration, inbound message and attachment projection, agentic official lark-cli egress, and read-only Web task end to end. +description: Validate a Bot-bound Agent's Feishu registration, inbound message and attachment projection, agentic official lark-cli egress, the agent-side First Tree chat-tool boundary, and the Web write boundary end to end. areas: [cross-surface] surfaces: [server, client, cli, web] --- @@ -11,7 +11,8 @@ surfaces: [server, client, cli, web] Confirm that one disposable Feishu Bot belongs to exactly one First Tree Agent and that a real Feishu conversation uses the canonical First Tree message, Inbox, attachment, and chat-history paths. The run must also prove that internal -collaborators cannot borrow the primary Agent's Bot identity and that the Web projection remains read-only. +collaborators cannot borrow the primary Agent's Bot identity, that the Agent cannot answer the conversation through +First Tree's own chat tools, and that Web structural writes stay blocked while personal view state keeps working. ## Preconditions @@ -69,17 +70,54 @@ collaborators cannot borrow the primary Agent's Bot identity and that the Web pr materialized paths. Partial failure, unsupported cards/merged forwards, >10 refs and >10 MiB resources must preserve the message with explicit unavailable placeholders. Confirm the uploader actor is the Bot-scoped Integration, while the displayed author remains the Feishu human. -- Invite Agent B through the ordinary Agent collaboration path. Confirm normal bounded history/backfill applies and B - can inspect the same canonical messages and attachments, but cannot obtain A's App Secret, record external intent, or send - to this Feishu conversation. +- Put Agent B into the bound task as an ordinary collaborator. Note that `chat invite` into a chat with an ACTIVE + binding is refused — that refusal is itself checked below — so B's membership has to be established while no active + binding covers the chat, not by inviting into a live one. With B a speaker, confirm normal bounded history/backfill + applies and B can inspect the same canonical messages and attachments, but cannot obtain A's App Secret, record + external intent, or send to this Feishu conversation. - From Agent A, first record an outbound intent, then call the official `lark-cli` directly for a new message, reply, thread reply, Markdown/card and attachment. Confirm each first attempt creates exactly one immutable recipientless First Tree message through shared `sendMessage`, gives other speakers only `notify=false` context, and uses that message id as the Feishu idempotency key. Reusing the same message id with changed content, target, or media bytes must be rejected. Confirm the temporary credential environment is private, is available only to A, and is deleted after use. +- From Agent A's session inside the bound task, attempt every First Tree chat tool. The boundary blocks messages and + membership changes, not all writes: `chat send`, `chat ask` and `chat invite` must be refused before anything is + written, each naming the Feishu reply path (record the delivery with `feishu intent`, then send with the official + `lark-cli --as bot`) rather than only refusing; `chat create` and `chat open` must be refused locally with the same + guidance. Removing a participant and editing an existing message must be refused identically — the same class of + change, so a gap in either is a gap in the boundary. `chat update --topic/--description`, `chat list`, `chat history`, + `feishu intent`, `feishu credential-env` and the agent's own archive/read state must keep working. Confirm no refused + command left a message, participant or chat behind. Repeat from Agent B and confirm the boundary is a property of the + chat, not of the Bot-owning Agent. +- Point the session's `FIRST_TREE_CHAT_ID` at the bound chat with `FIRST_TREE_AGENT_ID` unset, and separately against a + Server that does not report a chat's external channel. `chat create` and `chat open` must refuse as UNDETERMINED in + both cases rather than proceeding, and must say what to fix. From a plain operator terminal with neither variable set + and no Agent configured, `chat open` must still work — "no chat context" and "chat context we cannot resolve" are + different answers. +- Confirm a refusal requires membership first: from an Agent that is not a participant, target the bound chat's UUID and + confirm the error is indistinguishable from the same attempt against an ordinary chat it also does not belong to, so + the boundary cannot be used to discover which chats are Feishu-bound. +- Run `chat create --agent ` from inside the bound session. It must refuse without creating + anything, even though that other Agent cannot see the originating chat — the origin check runs as the session Agent, + and an inconclusive answer must refuse rather than proceed. +- With the guard active, force a provider terminal failure for Agent A in the bound task (for example, invalid provider + credentials). Confirm the operator-facing runtime notice still lands in First Tree chat history — an agent that cannot + run at all must not also go silent — while ordinary agent sends in the same chat remain refused. Then confirm the + exemption is not casually borrowed: an ordinary agent send that decorates itself with the runtime-notice metadata + while addressing a teammate must still be refused, and must not persist that marker even in an unbridged chat. Treat + the dedicated notice endpoint as a misuse-prevention rail, not an authorization boundary — it is membership-gated + exactly like an ordinary send, so this step is checking that the ordinary path stays closed, not that the notice path + is unforgeable. +- Exercise the runtime notice across a version skew in BOTH directions, since a provider failure is most likely during a + deploy. Point the current Client at a Server without the runtime-notice endpoint and confirm the notice still reaches + chat history through the older wire shape. Then have a Client that predates the endpoint publish into the current + Server and confirm the notice lands there too, stored the same way — including in the bound chat. +- Detach the chat binding, then retry `chat send` in the same chat. It must succeed: the boundary follows the live + binding, not the chat's `feishu` origin label, so a detached conversation returns to being an ordinary First Tree chat. - Open the bound task in Web. Confirm messages, author attribution and attachments remain readable, while direct message, rename, membership, join/leave and other structural mutations are absent and rejected by direct Web API calls. Personal - read, pin and archive state must continue to work. + read, pin and archive state must continue to work. After detaching the binding, confirm Web structural writes are + accepted again — Web and the agent scope must release the chat at the same moment. - Revoke the binding and confirm credentials are cleared, the Channel disconnects, chat bindings detach, and later ingress/resource/CLI operations fail closed without deleting historical canonical messages or attachments. - Delete every disposable Feishu document, spreadsheet, Base app, calendar, event, task and attachment created by the @@ -89,14 +127,17 @@ collaborators cannot borrow the primary Agent's Bot identity and that the Web pr ## Expected Result `PASS`: all real provider, permission, runtime, canonical history, attachment, authorization, idempotency and Web -read-only branches above are observed on the exact target with no user scope, cross-Bot, cross-Agent or duplicate +write-boundary branches above are observed on the exact target with no user scope, cross-Bot, cross-Agent or duplicate delivery, and all disposable provider resources are removed. `FAIL`: a reproducible product defect creates/wakes on an unrelated unmentioned group message, fails to wake on a verified Bot reply or activated-thread continuation, persists provider reference context as canonical history, attributes an external human as a First Tree member, loses a triggered message when one resource fails, exposes A's Bot credential to -B, bypasses canonical message creation, duplicates a same-id send inside the provider window, or permits a Web structural -mutation. +B, bypasses canonical message creation, duplicates a same-id send inside the provider window, permits a Web structural +mutation, lets an Agent answer a bridged conversation through First Tree's own chat tools, lets a request body mint its +own runtime-notice exemption, reveals a chat's Feishu binding to a non-member through the refusal, blocks the Bot's own +outbound delivery or the provider-failure runtime notice, blocks `chat update` or personal state, or keeps refusing in +either scope after the binding detaches. `BLOCKED`: official QR creation, disposable tenant/chat, inbound provider connectivity, official `lark-cli`, a provider-backed Agent turn, or the two-replica environment cannot be established. Deterministic product tests alone do diff --git a/packages/server/src/__tests__/agent-final-text-purpose.test.ts b/packages/server/src/__tests__/agent-final-text-purpose.test.ts index 8ee85bffe..c395e8187 100644 --- a/packages/server/src/__tests__/agent-final-text-purpose.test.ts +++ b/packages/server/src/__tests__/agent-final-text-purpose.test.ts @@ -114,17 +114,51 @@ describe("sendMessage — agent-final-text bypass (v1 §四 改造 4 b)", () => participantIds: [peerA.agent.uuid, peerB.agent.uuid], }); + // The marker is a trusted OPTION now, not a request field — see the + // smuggling test below. + const r = await sendMessage( + app.db, + chat.id, + peerA.agent.uuid, + { + source: "api", + format: "text", + content: "provider failed after retry handling", + purpose: "agent-final-text", + }, + { runtimeNotice: true }, + ); + + expect(r.recipients).toEqual([]); + expect(r.message.metadata[RUNTIME_NOTICE_METADATA_KEY]).toBe(true); + expect(r.message.metadata.agentFinalText).toBeUndefined(); + }); + + /** + * The stored marker decides whether a row counts as an agent final-text + * mirror, which the staging view toggle filters on. Keeping it server-stamped + * means the classification always reflects which endpoint was called rather + * than what a body claimed — the route layer decides that, never the service. + */ + it("strips a client-smuggled runtimeNotice flag so the marker stays server-owned", async () => { + const app = getApp(); + const owner = await createTestAgent(app, { type: "human" }); + const peerA = await createTestAgent(app, { type: "agent" }); + const peerB = await createTestAgent(app, { type: "agent" }); + + const chat = await createChat(app.db, owner.agent.uuid, { + type: "group", + participantIds: [peerA.agent.uuid, peerB.agent.uuid], + }); + const r = await sendMessage(app.db, chat.id, peerA.agent.uuid, { source: "api", format: "text", - content: "provider failed after retry handling", - metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, - purpose: "agent-final-text", + content: "pretending to be a runtime notice", + metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true, mentions: [peerB.agent.uuid] }, }); - expect(r.recipients).toEqual([]); - expect(r.message.metadata[RUNTIME_NOTICE_METADATA_KEY]).toBe(true); - expect(r.message.metadata.agentFinalText).toBeUndefined(); + expect(r.message.metadata[RUNTIME_NOTICE_METADATA_KEY]).toBeUndefined(); }); it("does NOT mark a normal agent send, and strips a client-smuggled agentFinalText flag", async () => { diff --git a/packages/server/src/__tests__/feishu-agent-readonly.test.ts b/packages/server/src/__tests__/feishu-agent-readonly.test.ts new file mode 100644 index 000000000..fd22fe058 --- /dev/null +++ b/packages/server/src/__tests__/feishu-agent-readonly.test.ts @@ -0,0 +1,395 @@ +import { legacyRuntimeNoticeSendBody, RUNTIME_NOTICE_METADATA_KEY } from "@first-tree/shared"; +import { and, eq } from "drizzle-orm"; +import { describe, expect, it } from "vitest"; +import { FEISHU_AGENT_CHAT_WRITE_CODE, FEISHU_AGENT_CHAT_WRITE_MESSAGE } from "../api/agent/feishu-chat-guard.js"; +import { chatMembership } from "../db/schema/chat-membership.js"; +import { imBotBindings } from "../db/schema/im-bot-bindings.js"; +import { imChatBindings } from "../db/schema/im-chat-bindings.js"; +import { messages } from "../db/schema/messages.js"; +import { serverInstances } from "../db/schema/server-instances.js"; +import { createChat } from "../services/chat/conversation.js"; +import { sendMessage } from "../services/chat/message.js"; +import { createTestAgent, useTestApp } from "./helpers.js"; + +/** + * The complete refusal body, asserted with `toEqual` rather than a status + + * code spot-check. The message is the actionable half of this boundary — it is + * what tells an agent to answer through Feishu instead — so a route that + * refuses with the right code and the wrong (or missing) guidance is still a + * regression, and only a full-body comparison catches it. + */ +const FEISHU_REFUSAL_BODY = { + error: FEISHU_AGENT_CHAT_WRITE_MESSAGE, + code: FEISHU_AGENT_CHAT_WRITE_CODE, +}; + +/** What a non-member sees on any of these routes, bridged chat or not. */ +const NOT_A_PARTICIPANT_BODY = { error: "Not a participant of this chat" }; + +/** + * Agent-scope mirror of `feishu-web-readonly.test.ts`. The Web boundary keeps + * a Feishu-bridged chat readable but structurally immutable for the signed-in + * user; this pins the symmetric boundary for the agent's own chat tools, whose + * writes would otherwise land where no Feishu human can see them. + */ +describe("Feishu agent chat-tool boundary", () => { + const getApp = useTestApp(); + + async function setup() { + const app = getApp(); + const a = await createTestAgent(app, { displayName: "Agent A" }); + const b = await createTestAgent(app, { displayName: "Agent B" }); + const c = await createTestAgent(app, { displayName: "Agent C" }); + const chat = await createChat(app.db, a.agent.uuid, { type: "group", participantIds: [b.agent.uuid] }); + const foreignInstanceId = `foreign-${crypto.randomUUID()}`; + await app.db.insert(serverInstances).values({ instanceId: foreignInstanceId, lastHeartbeat: new Date() }); + const [botBinding] = await app.db + .insert(imBotBindings) + .values({ + id: `binding-${crypto.randomUUID()}`, + organizationId: a.organizationId, + agentId: a.agent.uuid, + appId: `cli_${crypto.randomUUID()}`, + botOpenId: "ou_bot", + tenantKey: "tenant-a", + appSecretCipher: "encrypted-test-secret", + status: "active", + connectionStatus: "connected", + connectionOwnerInstanceId: foreignInstanceId, + connectionLeaseExpiresAt: new Date(Date.now() + 60 * 60 * 1_000), + }) + .returning(); + if (!botBinding) throw new Error("binding setup failed"); + const chatBindingId = `chat-binding-${crypto.randomUUID()}`; + await app.db.insert(imChatBindings).values({ + id: chatBindingId, + botBindingId: botBinding.id, + feishuChatId: "oc_feishu", + chatId: chat.id, + feishuChatType: "group", + status: "active", + }); + // An ordinary agent message that exists in the bridged chat. Seeded through + // the service, which is deliberately unguarded (the Feishu bridge reuses + // it), so the edit route has a row that is NOT bridge-authored to aim at — + // `editMessage`'s own Feishu-history rule would otherwise mask the gap. + const seeded = await sendMessage(app.db, chat.id, a.agent.uuid, { + source: "cli", + format: "text", + content: "an ordinary agent message", + metadata: { mentions: [b.agent.uuid] }, + }); + return { app, a, b, c, chat, chatBindingId, seededMessageId: seeded.message.id }; + } + + it("rejects `chat send`, `chat ask` and `chat invite` with an actionable code", async () => { + const { a, b, c, chat } = await setup(); + + const send = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "this would vanish", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(send.statusCode).toBe(403); + const sendBody = send.json<{ code?: string; error: string }>(); + expect(sendBody.code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + // The refusal must name the path that actually delivers, not just refuse. + expect(sendBody.error).toContain("feishu intent"); + expect(sendBody.error).toContain("lark-cli"); + + // `chat ask` is the same route with `format: "request"` — one guard covers both. + const ask = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "request", + content: "should I proceed?", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(ask.statusCode).toBe(403); + expect(ask.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + const invite = await a.request("POST", `/api/v1/agent/chats/${chat.id}/participants`, { + agentIds: [c.agent.uuid], + }); + expect(invite.statusCode).toBe(403); + expect(invite.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + }); + + /** + * The documented boundary is "messages AND MEMBERSHIP CHANGES". Adding a + * participant was guarded from the start; removing one mutates the same + * shared membership of the same invisible room, and editing a message + * rewrites the same unreadable history — a boundary that stops one and not + * the others is just a differently-shaped hole. + */ + it("blocks membership removal and message edits with the same actionable refusal", async () => { + const { app, a, b, chat, seededMessageId } = await setup(); + + const removal = await a.request("DELETE", `/api/v1/agent/chats/${chat.id}/participants/${b.agent.uuid}`); + expect(removal.statusCode).toBe(403); + expect(removal.json()).toEqual(FEISHU_REFUSAL_BODY); + + const edit = await a.request("PATCH", `/api/v1/agent/chats/${chat.id}/messages/${seededMessageId}`, { + content: "rewritten after the fact", + }); + expect(edit.statusCode).toBe(403); + expect(edit.json()).toEqual(FEISHU_REFUSAL_BODY); + + // A refusal that still mutated would be the worst of both worlds. + const [stillMember] = await app.db + .select({ agentId: chatMembership.agentId }) + .from(chatMembership) + .where(and(eq(chatMembership.chatId, chat.id), eq(chatMembership.agentId, b.agent.uuid))); + expect(stillMember).toBeDefined(); + + const [stored] = await app.db.select().from(messages).where(eq(messages.id, seededMessageId)); + expect(stored?.content).toBe("an ordinary agent message"); + }); + + /** The send and invite refusals carry the same complete body. */ + it("gives `chat send` and `chat invite` the identical full refusal body", async () => { + const { a, b, c, chat } = await setup(); + + const send = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "this would vanish", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(send.json()).toEqual(FEISHU_REFUSAL_BODY); + + const invite = await a.request("POST", `/api/v1/agent/chats/${chat.id}/participants`, { + agentIds: [c.agent.uuid], + }); + expect(invite.json()).toEqual(FEISHU_REFUSAL_BODY); + }); + + it("keeps reads, `chat update` and the bridge signal working", async () => { + const { a, chat } = await setup(); + + const detail = await a.request("GET", `/api/v1/agent/chats/${chat.id}`); + expect(detail.statusCode).toBe(200); + expect(detail.json<{ externalChannel: string | null }>().externalChannel).toBe("feishu"); + + const history = await a.request("GET", `/api/v1/agent/chats/${chat.id}/messages`); + expect(history.statusCode).toBe(200); + + const participants = await a.request("GET", `/api/v1/agent/chats/${chat.id}/participants`); + expect(participants.statusCode).toBe(200); + + // Deliberately still allowed: the agent briefing requires it to keep the + // chat's topic/description current, and neither is a message to a human. + const update = await a.request("PATCH", `/api/v1/agent/chats/${chat.id}`, { + topic: "Feishu bridge triage", + description: "Answering in the Feishu group.", + }); + expect(update.statusCode).toBe(200); + }); + + /** + * The exemption is a property of the ROUTE, so the genuine runtime notice + * has to keep landing: an agent that cannot run at all must not also go + * silent on the operator watching the chat. + */ + it("delivers a genuine runtime notice through the dedicated route and marks it server-side", async () => { + const { app, a, chat } = await setup(); + + const notice = await a.request("POST", `/api/v1/agent/chats/${chat.id}/runtime-notices`, { + content: "Claude Code could not run this turn: credentials need attention.", + }); + expect(notice.statusCode).toBe(201); + + const [stored] = await app.db.select().from(messages).where(eq(messages.id, notice.json<{ id: string }>().id)); + expect(stored?.metadata).toMatchObject({ [RUNTIME_NOTICE_METADATA_KEY]: true }); + }); + + /** + * Regression for the old blanket body exemption: ANY send that carried the + * final-text purpose plus the marker used to pass, which made the boundary + * depend on what the caller said it was doing. Only the exact legacy wire + * shape is honoured now (see the rolling-deploy test above), and these + * decorated ordinary sends are not it. + */ + it("rejects a forged runtime notice from an ordinary agent credential", async () => { + const { app, a, b, chat } = await setup(); + + const forged = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "arbitrary content wearing a runtime-notice costume", + source: "cli", + purpose: "agent-final-text", + metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, + }); + expect(forged.statusCode).toBe(403); + expect(forged.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + // The silent delivery profile alone never opened the door either. + const bareFinalText = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "not a runtime notice", + source: "cli", + purpose: "agent-final-text", + }); + expect(bareFinalText.statusCode).toBe(403); + expect(bareFinalText.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + // And even in an ORDINARY chat, where the send succeeds, the smuggled + // marker must not survive onto the stored row — otherwise the forgery just + // moves one chat over. + const plain = await createChat(app.db, a.agent.uuid, { type: "group", participantIds: [b.agent.uuid] }); + const smuggled = await a.request("POST", `/api/v1/agent/chats/${plain.id}/messages`, { + format: "text", + content: "ordinary send carrying the marker", + source: "cli", + metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true, mentions: [b.agent.uuid] }, + }); + expect(smuggled.statusCode).toBe(201); + const [stored] = await app.db.select().from(messages).where(eq(messages.id, smuggled.json<{ id: string }>().id)); + expect(stored?.metadata).not.toHaveProperty(RUNTIME_NOTICE_METADATA_KEY); + }); + + /** + * The notice route is a narrower capability, not an open door: it still + * requires membership, and it refuses to let the caller shape the stored row. + */ + it("keeps the runtime-notice route membership-gated and strict about its body", async () => { + const { a, c, chat } = await setup(); + + const outsider = await c.request("POST", `/api/v1/agent/chats/${chat.id}/runtime-notices`, { + content: "not my chat", + }); + expect(outsider.statusCode).toBe(403); + expect(outsider.json<{ code?: string }>().code).not.toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + const overreaching = await a.request("POST", `/api/v1/agent/chats/${chat.id}/runtime-notices`, { + content: "trying to address a teammate", + metadata: { mentions: [c.agent.uuid] }, + }); + expect(overreaching.statusCode).toBe(400); + }); + + /** + * The guard must not become an oracle: a non-member who guesses a chat UUID + * should not be able to tell a Feishu-bound chat from an ordinary one by the + * difference in error. + */ + it("authorizes membership before the boundary, so the 403 cannot be probed", async () => { + const { app, a, b, c, chat, seededMessageId } = await setup(); + const plain = await createChat(app.db, a.agent.uuid, { type: "group", participantIds: [b.agent.uuid] }); + + // Full-body equality across BOTH targets is the actual property under test: + // the bridged and the ordinary chat must be indistinguishable to a + // non-member, and comparing whole bodies leaves no field free to leak the + // difference. + for (const target of [chat, plain]) { + const invite = await c.request("POST", `/api/v1/agent/chats/${target.id}/participants`, { + agentIds: [c.agent.uuid], + }); + expect(invite.statusCode).toBe(403); + expect(invite.json()).toEqual(NOT_A_PARTICIPANT_BODY); + + const send = await c.request("POST", `/api/v1/agent/chats/${target.id}/messages`, { + format: "text", + content: "probing", + source: "cli", + metadata: { mentions: [a.agent.uuid] }, + }); + expect(send.statusCode).toBe(403); + expect(send.json()).toEqual(NOT_A_PARTICIPANT_BODY); + + const removal = await c.request("DELETE", `/api/v1/agent/chats/${target.id}/participants/${b.agent.uuid}`); + expect(removal.statusCode).toBe(403); + expect(removal.json()).toEqual(NOT_A_PARTICIPANT_BODY); + + // The message id only exists in the bridged chat; a non-member must not + // learn even that much, so the membership check has to come first. + const edit = await c.request("PATCH", `/api/v1/agent/chats/${target.id}/messages/${seededMessageId}`, { + content: "probing", + }); + expect(edit.statusCode).toBe(403); + expect(edit.json()).toEqual(NOT_A_PARTICIPANT_BODY); + } + }); + + /** + * ROLLING DEPLOY, old client → new server. A client that predates + * `/runtime-notices` publishes the same notice as a decorated send, and + * clients upgrade on their own schedule. Dropping it would silence exactly + * the operator signal a deploy is most likely to produce. + */ + it("still delivers a runtime notice sent in the legacy shape by an older client", async () => { + const { app, a, chat } = await setup(); + + const legacy = await a.request( + "POST", + `/api/v1/agent/chats/${chat.id}/messages`, + legacyRuntimeNoticeSendBody("Claude Code could not run this turn: credentials need attention."), + ); + expect(legacy.statusCode).toBe(201); + + // Same stored shape as the dedicated route produces: the marker is stamped + // by the server, not carried over from the request metadata. + const [stored] = await app.db.select().from(messages).where(eq(messages.id, legacy.json<{ id: string }>().id)); + expect(stored?.metadata).toMatchObject({ [RUNTIME_NOTICE_METADATA_KEY]: true }); + expect(stored?.metadata).not.toHaveProperty("agentFinalText"); + }); + + /** + * The compatibility path is an EXACT shape match for what older clients + * emit, not a general "say it is a notice and the boundary lifts" escape. + */ + it("does not extend the legacy shape to near-misses", async () => { + const { a, b, chat } = await setup(); + + const nearMisses = [ + // A different source: the legacy call sites all sent `api`. + { ...legacyRuntimeNoticeSendBody("wrong source"), source: "cli" as const }, + // Extra metadata — a notice addresses nobody. + { + ...legacyRuntimeNoticeSendBody("addressed"), + metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true, mentions: [b.agent.uuid] }, + }, + // The silent delivery purpose on its own never meant "runtime notice". + { ...legacyRuntimeNoticeSendBody("no marker"), metadata: {} }, + ]; + + for (const body of nearMisses) { + const res = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, body); + expect(res.statusCode).toBe(403); + expect(res.json()).toEqual(FEISHU_REFUSAL_BODY); + } + }); + + it("releases the boundary once the Feishu binding detaches", async () => { + const { app, a, b, chat, chatBindingId } = await setup(); + + await app.db.update(imChatBindings).set({ status: "detached" }).where(eq(imChatBindings.id, chatBindingId)); + + const detail = await a.request("GET", `/api/v1/agent/chats/${chat.id}`); + expect(detail.json<{ externalChannel: string | null }>().externalChannel).toBeNull(); + + const send = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "the bridge is gone; this is an ordinary chat again", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(send.statusCode).toBe(201); + }); + + it("leaves an unbridged chat untouched", async () => { + const { app, a, b } = await setup(); + const plain = await createChat(app.db, a.agent.uuid, { type: "group", participantIds: [b.agent.uuid] }); + + const detail = await a.request("GET", `/api/v1/agent/chats/${plain.id}`); + expect(detail.json<{ externalChannel: string | null }>().externalChannel).toBeNull(); + + const send = await a.request("POST", `/api/v1/agent/chats/${plain.id}/messages`, { + format: "text", + content: "ordinary send", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(send.statusCode).toBe(201); + }); +}); diff --git a/packages/server/src/__tests__/feishu-cli-preflight.test.ts b/packages/server/src/__tests__/feishu-cli-preflight.test.ts index 54811f8f4..39a2187e0 100644 --- a/packages/server/src/__tests__/feishu-cli-preflight.test.ts +++ b/packages/server/src/__tests__/feishu-cli-preflight.test.ts @@ -1,5 +1,6 @@ import { eq } from "drizzle-orm"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { FEISHU_AGENT_CHAT_WRITE_CODE } from "../api/agent/feishu-chat-guard.js"; import { imBotBindings } from "../db/schema/im-bot-bindings.js"; import { imChatBindings } from "../db/schema/im-chat-bindings.js"; import { inboxEntries } from "../db/schema/inbox-entries.js"; @@ -111,10 +112,26 @@ describe("controlled Feishu CLI preflight", () => { }); expect((stored?.metadata as { mentions?: unknown } | undefined)?.mentions).toEqual([]); + // Two independent rules refuse this edit; while the binding is active the + // chat-level boundary is the one that answers. const edit = await a.request("PATCH", `/api/v1/agent/chats/${chat.id}/messages/${grant.canonicalMessageId}`, { content: "edited after provider delivery", }); expect(edit.statusCode).toBe(403); + expect(edit.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + // Detaching releases the chat-level boundary, and the delivered provider + // row must STILL be immutable — First Tree cannot retract what Feishu has + // already shown, whatever the binding's current state. + await app.db.update(imChatBindings).set({ status: "detached" }).where(eq(imChatBindings.chatId, chat.id)); + const editAfterDetach = await a.request( + "PATCH", + `/api/v1/agent/chats/${chat.id}/messages/${grant.canonicalMessageId}`, + { content: "edited after the binding detached" }, + ); + expect(editAfterDetach.statusCode).toBe(403); + expect(editAfterDetach.json<{ error: string }>().error).toContain("Feishu message history cannot be edited"); + await app.db.update(imChatBindings).set({ status: "active" }).where(eq(imChatBindings.chatId, chat.id)); const inbox = await app.db.select().from(inboxEntries).where(eq(inboxEntries.messageId, grant.canonicalMessageId)); expect(inbox).toHaveLength(1); @@ -248,6 +265,45 @@ describe("controlled Feishu CLI preflight", () => { expect(changed.statusCode).toBe(403); }); + /** + * The bridge's own delivery reuses `messageService.sendMessage` with the same + * `source: "cli"` and the same agent `senderId` that `chat send` uses, so a + * Feishu boundary placed in the service layer would silence the bot itself. + * This pins the discriminator that keeps them apart: the intent route stays + * open while the agent chat route on the very same chat is refused. + */ + it("still delivers through the bridge while the agent chat tools are blocked on the same chat", async () => { + const { app, a, b, chat } = await setup(); + + const blocked = await a.request("POST", `/api/v1/agent/chats/${chat.id}/messages`, { + format: "text", + content: "chat send must not reach the Feishu group", + source: "cli", + metadata: { mentions: [b.agent.uuid] }, + }); + expect(blocked.statusCode).toBe(403); + expect(blocked.json<{ code?: string }>().code).toBe(FEISHU_AGENT_CHAT_WRITE_CODE); + + const delivered = await a.request("POST", "/api/v1/agent/feishu/intents", { + chatId: chat.id, + operation: "send", + targetChatId: "oc_feishu", + replyInThread: false, + format: "markdown", + content: "**the bridge still works**", + }); + expect(delivered.statusCode).toBe(200); + const canonicalMessageId = delivered.json<{ canonicalMessageId: string }>().canonicalMessageId; + + const [stored] = await app.db.select().from(messages).where(eq(messages.id, canonicalMessageId)); + expect(stored).toMatchObject({ chatId: chat.id, senderId: a.agent.uuid, content: "**the bridge still works**" }); + + // …and the silent context fan-out to other speakers is unaffected. + const inbox = await app.db.select().from(inboxEntries).where(eq(inboxEntries.messageId, canonicalMessageId)); + expect(inbox).toHaveLength(1); + expect(inbox[0]).toMatchObject({ inboxId: b.agent.inboxId, notify: false }); + }); + it("returns Bot credentials only to the bound primary Agent", async () => { const { a, b } = await setup(); const allowed = await a.request("POST", "/api/v1/agent/feishu/credentials"); diff --git a/packages/server/src/__tests__/feishu-web-readonly.test.ts b/packages/server/src/__tests__/feishu-web-readonly.test.ts index d89c598a4..53f1e5e7c 100644 --- a/packages/server/src/__tests__/feishu-web-readonly.test.ts +++ b/packages/server/src/__tests__/feishu-web-readonly.test.ts @@ -1,3 +1,4 @@ +import { eq } from "drizzle-orm"; import { describe, expect, it } from "vitest"; import { imBotBindings } from "../db/schema/im-bot-bindings.js"; import { imChatBindings } from "../db/schema/im-chat-bindings.js"; @@ -5,7 +6,12 @@ import { serverInstances } from "../db/schema/server-instances.js"; import { createChat } from "../services/chat/conversation.js"; import { createTestAgent, useTestApp } from "./helpers.js"; -describe("Feishu Web read-only boundary", () => { +/** + * "Web write boundary", not "Web read-only": personal view state (read, pin, + * archive) is deliberately still writable, which is why the 403 text below + * names the blocked class instead of claiming the whole chat is read-only. + */ +describe("Feishu Web write boundary", () => { const getApp = useTestApp(); async function setup() { @@ -31,15 +37,17 @@ describe("Feishu Web read-only boundary", () => { }) .returning(); if (!binding) throw new Error("binding setup failed"); + const chatBindingId = `chat-binding-${crypto.randomUUID()}`; await app.db.insert(imChatBindings).values({ - id: `chat-binding-${crypto.randomUUID()}`, + id: chatBindingId, botBindingId: binding.id, feishuChatId: "oc_feishu", chatId: chat.id, feishuChatType: "group", + status: "active", }); const headers = { authorization: `Bearer ${a.accessToken}` }; - return { app, a, chat, headers }; + return { app, a, chat, headers, chatBindingId }; } it("allows reads and private view state but rejects structural Web writes", async () => { @@ -64,6 +72,13 @@ describe("Feishu Web read-only boundary", () => { payload: { topic: "Web must not rename Feishu" }, }); expect(rename.statusCode).toBe(403); + // The refusal must not call the chat read-only: personal state above just + // succeeded, so that wording would be actively misleading. + const renameBody = rename.json<{ error: string }>(); + expect(renameBody.error).toContain("structural changes are blocked"); + expect(renameBody.error).toContain("read/pin/archive"); + expect(renameBody.error).not.toContain("read-only"); + const send = await app.inject({ method: "POST", url: `/api/v1/chats/${chat.id}/messages`, @@ -95,4 +110,33 @@ describe("Feishu Web read-only boundary", () => { expect(unfollow.statusCode).toBe(403); } }); + + /** + * BEHAVIOR CHANGE. The Web guard used to match ANY `im_chat_bindings` row, + * detached ones included, so a detached chat stayed Web-read-only forever + * while the agent scope had already released it. Both scopes now share one + * active-only predicate: once the binding detaches the chat is no longer + * mirrored into any Feishu conversation, so Web writes are legitimate again. + */ + it("releases the Web boundary once the binding detaches, matching the agent scope", async () => { + const { app, a, chat, headers, chatBindingId } = await setup(); + + await app.db.update(imChatBindings).set({ status: "detached" }).where(eq(imChatBindings.id, chatBindingId)); + + const rename = await app.inject({ + method: "PATCH", + url: `/api/v1/chats/${chat.id}`, + headers, + payload: { topic: "detached, so writable again" }, + }); + expect(rename.statusCode).toBe(200); + + const send = await app.inject({ + method: "POST", + url: `/api/v1/chats/${chat.id}/messages`, + headers, + payload: { format: "text", content: "the bridge is gone", metadata: { mentions: [a.agent.uuid] } }, + }); + expect(send.statusCode).toBe(201); + }); }); diff --git a/packages/server/src/api/agent/chats.ts b/packages/server/src/api/agent/chats.ts index 37a302c55..8adf802b1 100644 --- a/packages/server/src/api/agent/chats.ts +++ b/packages/server/src/api/agent/chats.ts @@ -33,6 +33,7 @@ import { } from "../../services/scm/gitlab/entity-follow.js"; import { resolveAgentScmBindingPair } from "../../services/scm/shared/attention-line.js"; import { sendFollowResult } from "../github-entity-reply.js"; +import { assertAgentMutableChat, isFeishuBridgedChat } from "./feishu-chat-guard.js"; const log = createLogger("AgentChatsRoute"); @@ -172,6 +173,10 @@ export async function agentChatRoutes(app: FastifyInstance): Promise { const detail = await chatService.getChatDetail(app.db, request.params.chatId, identity.uuid); return { ...serializeChat(detail), + // Live bridge state, so an agent-side precondition (`chat create` / + // `chat open`) can consult the same authority the write boundary uses + // instead of the stale `metadata.source` label. + externalChannel: (await isFeishuBridgedChat(app.db, request.params.chatId)) ? "feishu" : null, participants: detail.participants.map((p) => ({ ...p, joinedAt: p.joinedAt.toISOString(), @@ -255,6 +260,18 @@ export async function agentChatRoutes(app: FastifyInstance): Promise { }); } + // Authorize BEFORE the Feishu boundary. `addParticipant` below re-checks + // membership inside the invite service, but that is too late for this + // route: running the bridged-chat guard first would answer "is this chat + // bound to Feishu?" to a caller who is not even a participant, turning the + // 403 into an oracle over guessed chat UUIDs. Checking membership here + // makes the two failures indistinguishable to an outsider. + await chatService.assertParticipant(app.db, request.params.chatId, identity.uuid); + + // Feishu boundary for `chat invite`: pulling another agent into a bridged + // chat only widens a room the Feishu humans cannot see. + await assertAgentMutableChat(app.db, request.params.chatId); + const body = addParticipantSchema.parse(request.body); const participants = await chatService.addParticipant(app.db, request.params.chatId, identity.uuid, body); return reply.status(201).send( @@ -269,6 +286,17 @@ export async function agentChatRoutes(app: FastifyInstance): Promise { "/:chatId/participants/:agentId", async (request, reply) => { const identity = requireAgent(request); + + // Same two-step as the invite route above, and for the same two reasons. + // `removeParticipant` re-checks membership inside its transaction, but a + // removal mutates SHARED membership of a chat the Feishu humans cannot + // see, so the boundary has to apply here too — adding a participant and + // dropping one are the same class of change. Membership is authorized + // FIRST so the 403 cannot be used to probe which guessed chat UUIDs are + // Feishu-bound. + await chatService.assertParticipant(app.db, request.params.chatId, identity.uuid); + await assertAgentMutableChat(app.db, request.params.chatId); + await chatService.removeParticipant(app.db, request.params.chatId, identity.uuid, request.params.agentId); return reply.status(204).send(); }, diff --git a/packages/server/src/api/agent/feishu-chat-guard.ts b/packages/server/src/api/agent/feishu-chat-guard.ts new file mode 100644 index 000000000..37e370b8b --- /dev/null +++ b/packages/server/src/api/agent/feishu-chat-guard.ts @@ -0,0 +1,95 @@ +import type { Database } from "../../db/connection.js"; +import { ForbiddenError } from "../../errors.js"; +import { isFeishuBridgedChat } from "../../services/integrations/feishu/chat-binding.js"; + +/** + * Agent-scope counterpart of `assertWebMutableChat` in `api/chats.ts`. + * + * A chat bridged to a Feishu conversation lives in Feishu: the humans in it + * read the Feishu group, not the First Tree web app. An agent that answers + * with `chat send` / `chat ask` / `chat invite` writes into a surface nobody + * on the other side can see, so the reply is silently lost. These routes fail + * fast instead and name the path that actually delivers. + * + * GUARDED ROUTES, matching the documented "messages and membership changes" + * boundary — a partial application would just be a differently-shaped hole: + * - `POST /agent/chats/:chatId/messages` (`chat send`, `chat ask`) + * - `PATCH /agent/chats/:chatId/messages/:messageId` (message edit) + * - `POST /agent/chats/:chatId/participants` (`chat invite`) + * - `DELETE /agent/chats/:chatId/participants/:agentId` (membership removal) + * + * Authority is the shared `isFeishuBridgedChat` predicate — live + * `im_chat_bindings` state restricted to `status = 'active'`, NOT + * `chats.metadata.source`, which is a soft label that stays `"feishu"` after a + * binding detaches. The Web boundary uses the very same predicate so the two + * scopes cannot drift apart on what "bridged" means. + * + * DELIBERATELY NOT GUARDED HERE: + * - `messageService.sendMessage` itself. The Feishu bridge's own outbound + * delivery (`POST /agent/feishu/intents`) reuses that exact service call + * with the same `source: "cli"` and the same agent `senderId`; a guard in + * the service layer would break the bot's own replies. The bridge is + * distinguishable only by its route, which is why this lives in the + * route/adapter layer and is applied per-route. + * - `POST /agent/chats/:chatId/runtime-notices`. Operator-facing runtime + * notices must survive the boundary — an agent that cannot run at all must + * not also go silent. That route is exempt because of WHICH ROUTE IT IS. + * Be honest about what that buys: the route is membership-gated exactly + * like an ordinary send, so it is a misuse-prevention rail around a + * client-runtime-reported notice, not an unforgeable authorization + * boundary. See the note below. + * - `PATCH /agent/chats/:chatId` (`chat update`). Topic/description are + * First-Tree-side metadata the agent briefing requires it to maintain; + * they are not a message to a human in the Feishu group. + * - `POST /agent/chats/:chatId/archive`. That writes the calling human's + * private engagement row, i.e. personal view state — the same class the + * Web boundary deliberately keeps working on Feishu chats (`/read`, + * `/unread`, `/pin` are all unguarded there). + * + * NO GENERAL CONTENT-DERIVED EXEMPTION. An earlier revision let ANY send + * decorated with `purpose: "agent-final-text"` plus `metadata.runtimeNotice` + * through, which made the boundary depend on what a caller claimed to be + * sending. Runtime notices now have their own route, the stored marker is + * server-stamped (`stripUntrustedMetadataKeys` removes any inbound copy), and + * `POST /messages` is guarded regardless of body. + * + * The single remaining body-shaped path is the ROLLING-DEPLOY COMPATIBILITY + * one in `api/agent/messages.ts`: a body that matches the exact legacy + * runtime-notice wire shape is handled as the notice it is, because clients + * upgrade independently of the server and a provider-failure notice matters + * most mid-deploy. It is not a privilege escalation — the runtime-notice route + * is membership-gated exactly like the send route, so that body buys a caller + * nothing it could not get by calling the endpoint directly. It should be + * deleted once no supported client predates the endpoint. + */ + +/** Machine-readable code surfaced to the CLI through `AppError.attrs.code`. */ +export const FEISHU_AGENT_CHAT_WRITE_CODE = "FEISHU_CHAT_AGENT_WRITE_FORBIDDEN"; + +/** + * Wording matters here: the boundary blocks MESSAGES AND MEMBERSHIP CHANGES, + * not every write. `chat update`, `chat archive` and the agent's own read/view + * state all keep working, and saying "read-only" would send an agent hunting + * for a workaround it does not need. + */ +export const FEISHU_AGENT_CHAT_WRITE_MESSAGE = + "This chat is bridged to a Feishu conversation, so messages and membership changes are blocked here: " + + "a First Tree message reaches nobody, because the humans in this chat only ever see the Feishu group. " + + "Reply through the Feishu path instead — record the delivery with `feishu intent`, then send it with the " + + "official `lark-cli --as bot`. Reads, `chat update` and your own archive/read state still work normally."; + +export { isFeishuBridgedChat }; + +/** + * Reject an agent-scope chat write that would land outside the Feishu group. + * + * Call this only AFTER the route has authorized the caller's membership. + * Running it first would turn the boundary into an oracle: a non-member who + * guesses a chat UUID could tell bridged chats from ordinary ones by the + * difference between this 403 and the ordinary not-a-participant error. + */ +export async function assertAgentMutableChat(db: Database, chatId: string): Promise { + if (await isFeishuBridgedChat(db, chatId)) { + throw new ForbiddenError(FEISHU_AGENT_CHAT_WRITE_MESSAGE, { code: FEISHU_AGENT_CHAT_WRITE_CODE }); + } +} diff --git a/packages/server/src/api/agent/messages.ts b/packages/server/src/api/agent/messages.ts index 07c1c0ca5..44d16e22e 100644 --- a/packages/server/src/api/agent/messages.ts +++ b/packages/server/src/api/agent/messages.ts @@ -1,4 +1,9 @@ -import { paginationQuerySchema, sendMessageSchema } from "@first-tree/shared"; +import { + isLegacyRuntimeNoticeSend, + paginationQuerySchema, + runtimeNoticeRequestSchema, + sendMessageSchema, +} from "@first-tree/shared"; import type { FastifyInstance } from "fastify"; import { z } from "zod"; import { requireAgent } from "../../middleware/require-identity.js"; @@ -7,6 +12,7 @@ import { expiryToSeconds, signAgentOutboxToken } from "../../services/auth/token import * as chatService from "../../services/chat/conversation.js"; import * as messageService from "../../services/chat/message.js"; import { notifyRecipients } from "../../services/notifier.js"; +import { assertAgentMutableChat } from "./feishu-chat-guard.js"; const editMessageSchema = z.object({ format: z.string().optional(), @@ -45,12 +51,43 @@ export async function agentMessageRoutes(app: FastifyInstance): Promise { // to require explicit source — it would break unaudited third- // party integrations. const body = sendMessageSchema.parse(request.body); + + // ROLLING-DEPLOY COMPATIBILITY. A client older than `/runtime-notices` + // publishes its provider-failure and usage-limit notices as this exact + // send shape, and clients upgrade on their own schedule, so "old client, + // new server" is a normal steady state rather than a brief window. + // Recognising the legacy shape routes it to the same handling the + // dedicated endpoint gets: the notice still lands, and the server — not + // the body — stamps the stored marker. + // + // This is not an authorization decision, and it grants nothing: the + // runtime-notice endpoint is membership-gated exactly like this route, so + // any caller that could assemble this body could equally have called that + // endpoint. Both are misuse-prevention rails around a notice the client + // runtime reports, not a security boundary. `isLegacyRuntimeNoticeSend` + // is deliberately an exact shape match; remove it once no supported + // client predates the endpoint. + const legacyRuntimeNotice = isLegacyRuntimeNoticeSend(body); + + // Feishu boundary for `chat send` AND `chat ask` (same route; `chat ask` + // is just `format: "request"`). Applied here rather than inside + // `messageService.sendMessage`, which the Feishu bridge itself reuses — + // see `feishu-chat-guard.ts` for that collision. + // + // Ordered after `assertParticipant` above so the 403 cannot be probed by + // a non-member. + if (!legacyRuntimeNotice) { + await assertAgentMutableChat(app.db, request.params.chatId); + } const { message: msg, recipients } = await messageService.sendMessage( app.db, request.params.chatId, identity.uuid, body, { + // Legacy spelling of the dedicated endpoint; the marker is still + // server-stamped, never carried over from the request metadata. + runtimeNotice: legacyRuntimeNotice, // Explicit-recipient enforcement is the default in `sendMessage()`; // this route carries no business flag. Agent SDK callers (CLI // `chat send`, result-sink, etc.) declare routing via `receiverNames` @@ -75,12 +112,85 @@ export async function agentMessageRoutes(app: FastifyInstance): Promise { }, ); + /** + * Operator-facing runtime notice — "the provider failed", "the usage limit is + * reached". Its own route on purpose. + * + * A runtime notice is exempt from the Feishu-bridged chat write boundary, + * because an agent that could not run at all must not also go silent: the + * operator needs that row in First Tree history even when ordinary agent + * sends into the chat are refused. + * + * WHAT THIS ROUTE IS, PRECISELY. It is a MISUSE-PREVENTION RAIL carrying a + * notice the client runtime reports about itself — NOT a security or + * authorization boundary, and the exemption it grants is NOT unforgeable. + * The route is gated on chat membership and nothing else, exactly like + * `POST /messages`, so every credential that can reach one can reach the + * other; an agent determined to write into a bridged chat can simply call + * this endpoint and label the text a runtime notice. Nothing here verifies + * that a provider actually failed. + * + * What the separate route DOES buy is worth having anyway: the ordinary send + * path stays uniformly guarded with no shape of body that opens it, the + * server authors the entire stored row so a notice cannot quietly become an + * addressed message, and the narrow surface makes accidental misuse visible + * in review instead of plausible. Whether the capability should be narrowed + * further — to the daemon, or scoped to a chat/turn — is an open posture + * question, deliberately not settled here. + * + * The server authors everything that carries meaning: `source`, `format`, + * the silent recipientless delivery profile, and the `runtimeNotice` marker + * (a trusted `sendMessage` option — `stripUntrustedMetadataKeys` deletes any + * inbound copy). The request contributes only the notice text, and + * `runtimeNoticeRequestSchema` is strict, so a caller that tries to attach + * `purpose` or `metadata` gets a 400 instead of a quietly ignored field. + */ + app.post<{ Params: { chatId: string } }>( + "/:chatId/runtime-notices", + { config: { otelRecordBody: true } }, + async (request, reply) => { + const identity = requireAgent(request); + await chatService.assertParticipant(app.db, request.params.chatId, identity.uuid); + const body = runtimeNoticeRequestSchema.parse(request.body); + + const { message: msg } = await messageService.sendMessage( + app.db, + request.params.chatId, + identity.uuid, + { + source: "api", + format: "text", + content: body.content, + // Server-authored delivery profile: recipientless and silent, so a + // notice wakes nobody and cannot be used to address a teammate. + purpose: "agent-final-text", + }, + { runtimeNotice: true }, + ); + + return reply.status(201).send({ + ...msg, + createdAt: msg.createdAt.toISOString(), + }); + }, + ); + app.patch<{ Params: { chatId: string; messageId: string } }>( "/:chatId/messages/:messageId", { config: { otelRecordBody: true } }, async (request) => { const identity = requireAgent(request); + // Membership first, boundary second — the same ordering the send and + // participant routes use, so neither error reveals a chat's binding + // state to a non-member. await chatService.assertParticipant(app.db, request.params.chatId, identity.uuid); + // An edit is a message write. `editMessage` already refuses to touch a + // bridge-authored row, but ordinary agent messages and runtime notices in + // a bridged chat are editable without this, and rewriting First Tree + // history that the Feishu humans cannot see is exactly what the boundary + // exists to stop. Feishu carries no edit, so the two sides would also + // silently diverge. + await assertAgentMutableChat(app.db, request.params.chatId); const body = editMessageSchema.parse(request.body); const msg = await messageService.editMessage( app.db, diff --git a/packages/server/src/api/chats.ts b/packages/server/src/api/chats.ts index cafed2ff4..33d5ff367 100644 --- a/packages/server/src/api/chats.ts +++ b/packages/server/src/api/chats.ts @@ -17,7 +17,6 @@ import type { FastifyInstance } from "fastify"; import { agents } from "../db/schema/agents.js"; import { chatMembership } from "../db/schema/chat-membership.js"; import { chatUserState } from "../db/schema/chat-user-state.js"; -import { imChatBindings } from "../db/schema/im-chat-bindings.js"; import { inboxEntries } from "../db/schema/inbox-entries.js"; import { members } from "../db/schema/members.js"; import { messages } from "../db/schema/messages.js"; @@ -48,6 +47,7 @@ import { setChatEngagement, } from "../services/chat/workspace/me-chat.js"; import { listRequestThread } from "../services/chat/workspace/need-you.js"; +import { isFeishuBridgedChat } from "../services/integrations/feishu/chat-binding.js"; import { hasRemainingLandingCampaignTrialBudget, normalizeLandingCampaignTrialChatMetadataForRead, @@ -77,13 +77,31 @@ import { sendFollowResult } from "./github-entity-reply.js"; * and gates participation/supervision. */ export async function chatRoutes(app: FastifyInstance): Promise { + /** + * Web-scope Feishu boundary. Shares `isFeishuBridgedChat` with the agent + * scope so both answer "is this chat mirrored to Feishu right now?" the same + * way; see that module for why the answer is active-bindings-only. + * + * BEHAVIOR CHANGE: this used to match ANY binding row, including detached + * ones, which left a detached chat permanently blocked in Web while the agent + * scope had already let go of it. + * + * WORDING: not "read-only". The signed-in user's own view state — + * read/unread, pin, archive — deliberately keeps working, so "read-only" + * describes a stricter product than the one we ship and sends people hunting + * for a workaround they do not need. Name the blocked class instead. (The + * Web scope blocks more than the agent scope does: a rename is a structural + * write here, while an agent is required to keep topic/description current.) + */ async function assertWebMutableChat(chatId: string): Promise { - const [binding] = await app.db - .select({ id: imChatBindings.id }) - .from(imChatBindings) - .where(eq(imChatBindings.chatId, chatId)) - .limit(1); - if (binding) throw new ForbiddenError("Feishu chats are read-only in the Web app"); + if (await isFeishuBridgedChat(app.db, chatId)) { + throw new ForbiddenError( + "This chat is bridged to a Feishu conversation, so structural changes are blocked in the Web app: " + + "messages, membership, rename and entity follows all land where the humans in this chat — who only ever " + + "see the Feishu group — cannot see them. Reply in the Feishu conversation instead. Reading the chat, and " + + "your own read/pin/archive state, keep working normally.", + ); + } } async function requireDirectHumanChatMembership(chatId: string, humanAgentId: string): Promise { diff --git a/packages/server/src/services/chat/message.ts b/packages/server/src/services/chat/message.ts index 48217c618..30bb6a2e7 100644 --- a/packages/server/src/services/chat/message.ts +++ b/packages/server/src/services/chat/message.ts @@ -97,6 +97,12 @@ function stripUntrustedMetadataKeys( const shouldStripFirstChatOrientation = !options.allowFirstChatOrientation && FIRST_CHAT_ORIENTATION_METADATA_KEY in meta; const shouldStripFeishu = !options.allowFeishuMetadata && "feishu" in meta; + // Always stripped, never allow-listed: the runtime-notice marker is re-stamped + // below from `options.runtimeNotice`, which only the dedicated runtime-notice + // route can set. The key grants an exemption from the Feishu-bridged chat + // write boundary, so accepting it from a request body would let any agent + // credential mint that exemption for itself. + const shouldStripRuntimeNotice = RUNTIME_NOTICE_METADATA_KEY in meta; if ( !shouldStripSystemSender && !shouldStripAddressedAgentIds && @@ -105,7 +111,8 @@ function stripUntrustedMetadataKeys( !shouldStripEditedAt && !shouldStripFirstChatOrientationContinuation && !shouldStripFirstChatOrientation && - !shouldStripFeishu + !shouldStripFeishu && + !shouldStripRuntimeNotice ) { return meta; } @@ -117,6 +124,7 @@ function stripUntrustedMetadataKeys( key !== CLI_BODY_ORIGIN_METADATA_KEY && key !== "editedAt" && key !== FIRST_CHAT_ORIENTATION_CONTINUATION_METADATA_KEY && + key !== RUNTIME_NOTICE_METADATA_KEY && (options.allowFeishuMetadata || key !== "feishu") && (options.allowFirstChatOrientation || key !== FIRST_CHAT_ORIENTATION_METADATA_KEY) && (options.allowSystemSender || key !== "systemSender"), @@ -437,6 +445,17 @@ export type SendMessageOptions = { }; /** Allow the trusted integration/CLI bridge to persist server-authored `metadata.feishu`. */ allowFeishuMetadata?: boolean; + /** + * Trusted runtime-notice write, set only by + * `POST /agent/chats/:chatId/runtime-notices`. The service stamps + * `metadata.runtimeNotice` itself; ordinary sends cannot mint the marker + * because `stripUntrustedMetadataKeys` always removes an inbound copy. + * + * The marker is what exempts a message from the Feishu-bridged chat write + * boundary, so it is a capability, not a label — which is exactly why it is + * a trusted option rather than a request field. + */ + runtimeNotice?: boolean; /** * Trusted internal delivery mode that persists an explicitly addressed * message as replayable context without waking any recipient. The ordinary @@ -822,21 +841,27 @@ export function preflightMessageSendIntent(input: { // The flag is SERVER-OWNED: // 1. strip any inbound client-supplied value, then // 2. set it true ONLY for a genuine mirror — a NON-HUMAN sender with the - // final-text purpose, excluding `metadata.runtimeNotice=true`. + // final-text purpose, excluding a runtime notice. // `purpose` rides the shared send schema, so a human/web send can carry it // (and gets the silent enforcement profile above) — but it must never be // persisted as a mirror, matching the unread-projection's // `senderRow.type !== "human"` gate. The staging-only "hide agent final // text" toggle filters on this flag. - const isRuntimeNotice = metadataToStore[RUNTIME_NOTICE_METADATA_KEY] === true; + // Server-owned too — `stripUntrustedMetadataKeys` has already removed any + // inbound copy, so the only way this is true is the dedicated runtime-notice + // route asking for it. + const isRuntimeNotice = options.runtimeNotice === true; const isAgentFinalTextMirror = isAgentFinalText && senderType !== "human" && !isRuntimeNotice; const metadataSansFlag = AGENT_FINAL_TEXT_METADATA_KEY in metadataToStore ? Object.fromEntries(Object.entries(metadataToStore).filter(([key]) => key !== AGENT_FINAL_TEXT_METADATA_KEY)) : metadataToStore; - const storedMetadata = isAgentFinalTextMirror + const metadataWithFinalTextFlag = isAgentFinalTextMirror ? { ...metadataSansFlag, [AGENT_FINAL_TEXT_METADATA_KEY]: true } : metadataSansFlag; + const storedMetadata = isRuntimeNotice + ? { ...metadataWithFinalTextFlag, [RUNTIME_NOTICE_METADATA_KEY]: true } + : metadataWithFinalTextFlag; return { content: outboundContent, diff --git a/packages/server/src/services/integrations/feishu/chat-binding.ts b/packages/server/src/services/integrations/feishu/chat-binding.ts new file mode 100644 index 000000000..ced5ef9ae --- /dev/null +++ b/packages/server/src/services/integrations/feishu/chat-binding.ts @@ -0,0 +1,35 @@ +import { and, eq } from "drizzle-orm"; +import type { Database } from "../../../db/connection.js"; +import { imChatBindings } from "../../../db/schema/im-chat-bindings.js"; + +/** + * The single "is this chat mirrored to a Feishu conversation right now?" + * predicate. Both write boundaries — the Web one in `api/chats.ts` and the + * agent one in `api/agent/feishu-chat-guard.ts` — must answer this question + * identically, so they share this function rather than each writing their own + * query. + * + * ACTIVE-ONLY is the rule. `im_chat_bindings.status` is `'active' | 'detached'`; + * a detached row is history, not a live mirror. Once a binding detaches the + * chat is no longer projected into any Feishu conversation, so writing to it + * reaches the same people it always did and the boundary has nothing left to + * protect. Treating a detached row as still-bridged would strand the chat + * permanently read-only with no way back. + * + * The two scopes previously disagreed here — the agent scope filtered on + * `status`, the Web scope matched any row — which left a detached chat + * agent-writable but Web-read-only. This module exists so that cannot recur. + */ + +/** The one binding status that means "currently mirrored to Feishu". */ +export const FEISHU_ACTIVE_CHAT_BINDING_STATUS = "active"; + +/** True when the chat currently has an active Feishu conversation binding. */ +export async function isFeishuBridgedChat(db: Database, chatId: string): Promise { + const [binding] = await db + .select({ id: imChatBindings.id }) + .from(imChatBindings) + .where(and(eq(imChatBindings.chatId, chatId), eq(imChatBindings.status, FEISHU_ACTIVE_CHAT_BINDING_STATUS))) + .limit(1); + return binding !== undefined; +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index fb524944d..c005a740f 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -323,10 +323,12 @@ export { addParticipantSchema, archiveChatResponseSchema, CHAT_ENGAGEMENT_STATUSES, + CHAT_EXTERNAL_CHANNELS, CHAT_TYPES, type Chat, type ChatDetail, type ChatEngagementStatus, + type ChatExternalChannel, type ChatParticipant, type ChatParticipantDetail, type ChatType, @@ -335,6 +337,7 @@ export { type CreateWebTaskChat, chatDetailSchema, chatEngagementStatusSchema, + chatExternalChannelSchema, chatParticipantDetailSchema, chatParticipantSchema, chatSchema, @@ -1153,7 +1156,9 @@ export { firstChatOrientationContinuationMessageMetadataSchema, firstChatOrientationMessageMetadataSchema, isAgentFinalTextMetadata, + isLegacyRuntimeNoticeSend, isRuntimeNoticeMetadata, + legacyRuntimeNoticeSendBody, MESSAGE_FORMATS, MESSAGE_SENDER_KINDS, MESSAGE_SENDER_PROVIDERS, @@ -1175,12 +1180,15 @@ export { participantModeSchema, precedingMessageSchema, type RequestResolution, + RUNTIME_NOTICE_MAX_LENGTH, RUNTIME_NOTICE_METADATA_KEY, + type RuntimeNoticeRequest, readAskAgentMessageMetadata, readFeishuMessageMetadata, readFirstChatOrientationContinuationMessageMetadata, readFirstChatOrientationMessageMetadata, requestResolutionSchema, + runtimeNoticeRequestSchema, type SendMessage, sendMessageSchema, } from "./schemas/message.js"; diff --git a/packages/shared/src/schemas/chat.ts b/packages/shared/src/schemas/chat.ts index 36461cce2..c1806b997 100644 --- a/packages/shared/src/schemas/chat.ts +++ b/packages/shared/src/schemas/chat.ts @@ -135,6 +135,16 @@ export const chatParticipantDetailSchema = chatParticipantSchema.extend({ }); export type ChatParticipantDetail = z.infer; +/** + * External IM conversations a First Tree chat can be bridged to. Distinct from + * `ChatSource` (a conversation-list origin label projected from + * `chats.metadata`): this one is a live binding state, so it goes back to + * `null` once the binding detaches. + */ +export const CHAT_EXTERNAL_CHANNELS = ["feishu"] as const; +export const chatExternalChannelSchema = z.enum(CHAT_EXTERNAL_CHANNELS); +export type ChatExternalChannel = z.infer; + export const chatSchema = z.object({ id: z.string(), organizationId: z.string(), @@ -201,6 +211,18 @@ export const chatDetailSchema = chatSchema.extend({ * payload. */ lastReadAt: z.string().nullable().default(null), + /** + * The external IM conversation this chat is currently bridged to, or NULL. + * Authoritative live binding state (`im_chat_bindings` filtered to + * `status = 'active'`), not the `metadata.source` label — a detached chat + * reports NULL here while its metadata still says `"feishu"`. + * + * The agent CLI reads it to refuse `chat create` / `chat open` from inside a + * bridged chat before spending a write, using the exact same signal the + * server's own agent-scope boundary enforces. `.default(null)`: only the + * agent chat-detail route populates it. + */ + externalChannel: chatExternalChannelSchema.nullable().default(null), }); export type ChatDetail = z.infer; diff --git a/packages/shared/src/schemas/message.ts b/packages/shared/src/schemas/message.ts index dc6bc47a4..b8837bb54 100644 --- a/packages/shared/src/schemas/message.ts +++ b/packages/shared/src/schemas/message.ts @@ -415,8 +415,100 @@ export type MessagePurpose = z.infer; * false on every other message. */ export const AGENT_FINAL_TEXT_METADATA_KEY = "agentFinalText"; + +/** + * Metadata flag marking a STORED message as an operator-facing runtime notice + * ("the provider failed", "the usage limit is reached") rather than anything + * the agent chose to say. + * + * SERVER-OWNED, like `AGENT_FINAL_TEXT_METADATA_KEY`: the server strips any + * inbound copy on every write path and re-stamps the flag itself, so the stored + * value always reflects which endpoint was called rather than what a body + * claimed. That keeps the classification honest — the flag decides whether a + * row counts as an agent final-text mirror, which the staging view toggle + * filters on. + * + * It is a CLASSIFICATION LABEL, not a capability. It confers no authority a + * caller does not already have: the dedicated runtime-notice endpoint is gated + * on chat membership exactly like an ordinary send, so any credential that can + * reach one can reach the other. + */ export const RUNTIME_NOTICE_METADATA_KEY = "runtimeNotice"; +/** + * Upper bound on a runtime notice's text. The longest notice the runtime + * composes today is a provider-failure lead plus a 500-character redacted + * provider preview; this leaves generous headroom while keeping the dedicated + * route from becoming a general-purpose writing surface. + */ +export const RUNTIME_NOTICE_MAX_LENGTH = 4_000; + +/** + * Body of `POST /api/v1/agent/chats/:chatId/runtime-notices`. + * + * Deliberately carries ONLY the notice text: `source`, `format`, `purpose` and + * every metadata marker are authored by the server. `.strict()` makes an + * attempt to smuggle those fields a 400 rather than a silent drop, so a caller + * that still believes it can shape the stored row fails loudly. + */ +export const runtimeNoticeRequestSchema = z + .object({ + content: z.string().min(1).max(RUNTIME_NOTICE_MAX_LENGTH), + }) + .strict(); +export type RuntimeNoticeRequest = z.infer; + +/** + * The wire shape a client OLDER than the runtime-notice endpoint uses to + * publish the same notice: an ordinary agent send decorated with the marker and + * the silent final-text delivery purpose. Every pre-endpoint call site — the + * provider-failure notice and both Codex usage-limit notices — emitted exactly + * these five fields. + * + * It exists in shared so the two halves of the rolling-deploy story cannot + * drift: the SDK falls back to this body when the new endpoint 404s on an older + * server, and the server recognises this body with + * `isLegacyRuntimeNoticeSend()` when an older client posts to a new server. A + * provider-failure notice is most valuable exactly during a deploy, so neither + * direction may drop it. + */ +export function legacyRuntimeNoticeSendBody(content: string): SendMessage { + return { + source: "api", + format: "text", + content, + metadata: { [RUNTIME_NOTICE_METADATA_KEY]: true }, + purpose: "agent-final-text", + }; +} + +/** + * True when a send body is exactly the legacy runtime-notice shape above. + * + * SHAPE MATCHING, NOT AUTHORIZATION. The match is deliberately exact — the + * final-text purpose, `format: "text"`, `source: "api"`, and `runtimeNotice` + * as the sole metadata key — so it recognises the bodies real older clients + * emit and nothing else. It is not a permission check and must not be read as + * one: any caller could assemble this body, just as any caller could POST to + * the runtime-notice endpoint directly. Both are membership-gated and neither + * is a security boundary; matching here only preserves the delivery an older + * client already had. + */ +export function isLegacyRuntimeNoticeSend(body: { + format?: unknown; + source?: unknown; + purpose?: unknown; + metadata?: Record | null; +}): boolean { + if (body.purpose !== "agent-final-text") return false; + if (body.format !== "text") return false; + if (body.source !== "api") return false; + const metadata = body.metadata; + if (!metadata) return false; + const keys = Object.keys(metadata); + return keys.length === 1 && keys[0] === RUNTIME_NOTICE_METADATA_KEY && metadata[RUNTIME_NOTICE_METADATA_KEY] === true; +} + /** True when a stored message's metadata marks it as an agent final-text mirror. */ export function isAgentFinalTextMetadata(metadata: Record | null | undefined): boolean { return metadata?.[AGENT_FINAL_TEXT_METADATA_KEY] === true; diff --git a/packages/web/src/pages/__tests__/page-ssr-smoke.test.tsx b/packages/web/src/pages/__tests__/page-ssr-smoke.test.tsx index 67788dfb0..33b44a4f0 100644 --- a/packages/web/src/pages/__tests__/page-ssr-smoke.test.tsx +++ b/packages/web/src/pages/__tests__/page-ssr-smoke.test.tsx @@ -310,6 +310,7 @@ function chatDetail(overrides: Partial = {}): ChatDetail { description: overrides.description ?? null, descriptionUpdatedAt: overrides.descriptionUpdatedAt ?? null, lastReadAt: overrides.lastReadAt ?? null, + externalChannel: overrides.externalChannel ?? null, lifecyclePolicy: overrides.lifecyclePolicy ?? null, metadata: overrides.metadata ?? diff --git a/packages/web/src/pages/workspace/center/__tests__/chat-by-id-center-dom.test.tsx b/packages/web/src/pages/workspace/center/__tests__/chat-by-id-center-dom.test.tsx index 88835decc..332fa48cc 100644 --- a/packages/web/src/pages/workspace/center/__tests__/chat-by-id-center-dom.test.tsx +++ b/packages/web/src/pages/workspace/center/__tests__/chat-by-id-center-dom.test.tsx @@ -136,6 +136,7 @@ function chatDetail(overrides: Partial = {}): ChatDetail { description: overrides.description ?? null, descriptionUpdatedAt: overrides.descriptionUpdatedAt ?? null, lastReadAt: overrides.lastReadAt ?? null, + externalChannel: overrides.externalChannel ?? null, lifecyclePolicy: overrides.lifecyclePolicy ?? null, metadata: overrides.metadata ?? {}, createdAt: overrides.createdAt ?? NOW, diff --git a/packages/web/src/pages/workspace/center/__tests__/chat-view-dom.test.tsx b/packages/web/src/pages/workspace/center/__tests__/chat-view-dom.test.tsx index ea2e678db..217caca90 100644 --- a/packages/web/src/pages/workspace/center/__tests__/chat-view-dom.test.tsx +++ b/packages/web/src/pages/workspace/center/__tests__/chat-view-dom.test.tsx @@ -269,6 +269,7 @@ function chatDetail(overrides: Partial = {}): ChatDetail { description: overrides.description ?? null, descriptionUpdatedAt: overrides.descriptionUpdatedAt ?? null, lastReadAt: overrides.lastReadAt ?? null, + externalChannel: overrides.externalChannel ?? null, lifecyclePolicy: overrides.lifecyclePolicy ?? null, metadata: overrides.metadata ?? { source: "github", entityUrl: "https://github.com/acme/web/pull/42" }, createdAt: overrides.createdAt ?? NOW, diff --git a/packages/web/src/pages/workspace/center/__tests__/workspace-center-extra-dom.test.tsx b/packages/web/src/pages/workspace/center/__tests__/workspace-center-extra-dom.test.tsx index 10c29d307..002bf414d 100644 --- a/packages/web/src/pages/workspace/center/__tests__/workspace-center-extra-dom.test.tsx +++ b/packages/web/src/pages/workspace/center/__tests__/workspace-center-extra-dom.test.tsx @@ -240,6 +240,7 @@ function chatDetail(overrides: Partial = {}): ChatDetail { description: overrides.description ?? null, descriptionUpdatedAt: overrides.descriptionUpdatedAt ?? null, lastReadAt: overrides.lastReadAt ?? null, + externalChannel: overrides.externalChannel ?? null, lifecyclePolicy: overrides.lifecyclePolicy ?? null, metadata: overrides.metadata ?? { source: "github", entityUrl: "https://github.com/acme/web/pull/42" }, createdAt: overrides.createdAt ?? NOW,