From 5c9cedbcba7bb4bf186edacb815bf871d9c047fa Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:38:49 -0700 Subject: [PATCH 01/22] feat(server): agents ask the user for a webhook signing secret privately An agent setting up a signed webhook (e.g. GitHub releases) had no way to get the signing secret without it landing in the transcript: the save failed without one, so it would invent one or ask for it in chat. - request_secret (T3 MCP) records a secret_request card in the calling thread and waits for the user. The card carries a label, reason, the target task and a status, never the value. - scheduledTasks.answerSecretRequest stores the secret on the task, then marks the card saved or declined. The agent's result is only that status; secret_request.record is refused from client dispatch. - A webhook signature can be saved with allowPendingSecret; the task rejects every request until its secret is set. - schedule_task returns webhookUrl only when it is a public URL, plus the signature state, and its description covers webhooks: placeholders, GitHub's signature settings, request_secret, and that runs posting into this thread suit an orchestrator that delegates and dedupes. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/mobile/src/lib/threadActivity.ts | 6 + apps/server/src/auth/RpcAuthorization.ts | 1 + apps/server/src/mcp/OrchestratorMcpService.ts | 124 +++++++++++++- ...OrchestratorMcpToolkit.integration.test.ts | 90 +++++++++- .../src/mcp/toolkits/orchestrator/handlers.ts | 6 + .../src/mcp/toolkits/orchestrator/tools.ts | 17 +- .../src/orchestration-v2/Orchestrator.ts | 83 +++++++++ .../testkit/OrchestratorScenario.ts | 2 + .../provider/T3OrchestrationInstructions.ts | 3 +- .../scheduledTasks/ScheduledTaskService.ts | 87 +++++++++- .../ScheduledTaskService.webhook.test.ts | 159 ++++++++++++++++++ apps/server/src/ws.ts | 68 +++++--- packages/client-runtime/src/t3ToolSummary.ts | 3 + .../contracts/src/orchestrationV2.test.ts | 2 +- packages/contracts/src/orchestrationV2.ts | 52 ++++++ packages/contracts/src/orchestratorMcp.ts | 37 +++- packages/contracts/src/rpc.ts | 11 ++ packages/contracts/src/scheduledTask.ts | 30 +++- packages/shared/src/t3McpToolPresentation.ts | 2 + 19 files changed, 748 insertions(+), 35 deletions(-) diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index 86c4184d1c1f..c9d6a58237da 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -537,6 +537,8 @@ function itemIcon(item: OrchestrationV2TurnItem): ThreadFeedActivity["icon"] { case "fork": case "thread_created": return "zap"; + case "secret_request": + return "lock"; } } @@ -590,6 +592,8 @@ function itemSummary( return "Thread forked"; case "thread_created": return "Thread created"; + case "secret_request": + return item.label; case "dynamic_tool": { const classified = classifyToolActivity({ itemType: "dynamic_tool_call", @@ -647,6 +651,8 @@ function itemPreview(item: OrchestrationV2TurnItem): string | null { case "fork": case "thread_created": return item.targetThreadId; + case "secret_request": + return item.reason || null; case "subagent": return item.result ?? item.progress ?? item.prompt; case "dynamic_tool": diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 61d7ea279bf4..88a665e035b7 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -96,6 +96,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.scheduledTasksDelete]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksRunNow]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksRotateWebhookToken]: AuthOrchestrationOperateScope, + [WS_METHODS.scheduledTasksAnswerSecretRequest]: AuthOrchestrationOperateScope, // Delivery logs hold request bodies, so they need the same scope as the URL. [WS_METHODS.scheduledTasksListWebhookDeliveries]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksGetWebhookDelivery]: AuthOrchestrationOperateScope, diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 7a18455c4d70..8f55a48933c2 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -5,6 +5,7 @@ import { MessageId, type ModelSelection, NodeId, + TurnItemId, type OrchestrationV2Run, type OrchestrationV2Subagent, type OrchestrationV2ThreadProjection, @@ -19,6 +20,8 @@ import { type OrchestratorMcpDelegateTaskResult, type OrchestratorMcpInteractionMode, type OrchestratorMcpDeleteScheduledTaskInput, + type OrchestratorMcpRequestSecretInput, + type OrchestratorMcpRequestSecretResult, type OrchestratorMcpDeleteScheduledTaskResult, type OrchestratorMcpListScheduledTasksResult, type OrchestratorMcpRuntimeMode, @@ -90,6 +93,8 @@ const TASK_WAKE_EVENTS = [ { thread: "child", eventType: "subagent.updated" }, { thread: "child", eventType: "provider-thread.updated" }, ] as const; +/** A person answers the card, so a slower poll is plenty. */ +const SECRET_REQUEST_POLL_INTERVAL_MS = 500; const DEFAULT_THREAD_LIST_LIMIT = 50; const DEFAULT_THREAD_READ_LIMIT = 50; const DEFAULT_THREAD_RUN_LIMIT = 10; @@ -140,6 +145,14 @@ export interface OrchestratorMcpServiceShape { scope: McpInvocationScope, input: OrchestratorMcpDeleteScheduledTaskInput, ) => Effect.Effect; + /** + * Asks the user for a secret through a card in the calling thread and waits + * for the answer. The value never reaches the agent: the result is a status. + */ + readonly requestSecret: ( + scope: McpInvocationScope, + input: OrchestratorMcpRequestSecretInput, + ) => Effect.Effect; readonly listThreads: ( scope: McpInvocationScope, input: OrchestratorMcpThreadListInput, @@ -220,7 +233,17 @@ function scheduledTaskSummary(task: ScheduledTask): OrchestratorMcpScheduledTask schedule: task.schedule, nextRunAt: task.nextRunAt, lastRunStatus: task.lastRunStatus, - ...(task.webhook === undefined ? {} : { webhookUrl: task.webhook.url ?? task.webhook.path }), + // A bare path is not a URL anyone can call, so agents never get one to share. + ...(task.webhook?.url == null ? {} : { webhookUrl: task.webhook.url }), + ...(task.webhook === undefined + ? {} + : { + webhookSignature: task.webhook.hasSecret + ? "set" + : task.schedule.type === "webhook" && task.schedule.signature !== null + ? "secret_pending" + : "none", + }), }; } @@ -725,6 +748,8 @@ function turnItemText(item: OrchestrationV2TurnItem): string | null { return `Forked to thread ${item.targetThreadId}.`; case "thread_created": return `Created thread ${item.targetThreadId} with ${item.targetProviderInstanceId} (${item.targetModel}).`; + case "secret_request": + return `Asked the user for ${item.label}: ${item.secretStatus}.`; case "subagent": return item.result ?? item.progress ?? item.prompt; case "dynamic_tool": @@ -1514,6 +1539,103 @@ const make = Effect.gen(function* () { ); return { scheduledTaskId: existing.id, deleted: true }; }), + requestSecret: (scope, input) => + Effect.gen(function* () { + yield* requireCapability(scope); + const parent = yield* loadProjection(scope.threadId); + const task = yield* loadScopedScheduledTask(parent.thread.projectId, input.scheduledTaskId); + if (task.schedule.type !== "webhook" || task.schedule.signature === null) { + return yield* failure( + "invalid_request", + "Only a webhook task with a signature check takes a signing secret. Set schedule.signature (with allowPendingSecret: true) first.", + ); + } + const run = ThreadManagementService.latestActiveRun(parent); + if ( + run === undefined || + run.rootNodeId === null || + run.providerInstanceId !== scope.providerInstanceId + ) { + return yield* failure( + "parent_not_active", + "Asking for a secret requires an active run owned by this MCP provider session.", + ); + } + const runId = run.id; + const nodeId = run.rootNodeId; + const key = yield* requestKey(undefined); + const turnItemId = TurnItemId.make(`turn-item:secret-request:${stablePart(key)}`); + const record = (secretStatus: "pending" | "cancelled") => + threadManagement + .dispatch({ + type: "secret_request.record", + commandId: stableCommandId({ + scope, + requestKey: key, + operation: `secret-${secretStatus}`, + }), + threadId: scope.threadId, + runId, + nodeId, + turnItemId, + label: input.label, + reason: input.reason ?? "", + target: { kind: "scheduled_task_webhook_signature", scheduledTaskId: task.id }, + secretStatus, + }) + .pipe( + Effect.mapError((error) => + failure( + "orchestration_error", + `Could not record the secret request: ${errorMessage(error)}`, + ), + ), + ); + yield* record("pending"); + + // The card is answered by the user (scheduledTasks.provideWebhookSecret) + // or ends with the run; poll it like a delegated task. + const answered = yield* Effect.gen(function* () { + while (true) { + const projection = yield* threadManagement + .getThreadRecords(scope.threadId, ["runs", "turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe( + Effect.mapError((error) => + failure( + "orchestration_error", + `Unable to read the secret request: ${errorMessage(error)}`, + ), + ), + ); + const item = projection.turnItems.find((candidate) => candidate.id === turnItemId); + if (item?.type === "secret_request" && item.secretStatus !== "pending") { + return item.secretStatus; + } + const current = projection.runs.find((candidate) => candidate.id === runId); + if ( + current === undefined || + ThreadManagementService.isTerminalRunStatus(current.status) + ) { + yield* record("cancelled"); + return "cancelled" as const; + } + yield* Effect.sleep(Duration.millis(SECRET_REQUEST_POLL_INTERVAL_MS)); + } + }).pipe( + Effect.timeoutOption( + Duration.millis( + Math.min(input.timeoutMs ?? DEFAULT_WAIT_TIMEOUT_MS, MAX_WAIT_TIMEOUT_MS), + ), + ), + ); + return { + scheduledTaskId: task.id, + status: Option.getOrElse(answered, () => "pending" as const), + }; + }), capabilities: (scope) => Effect.gen(function* () { const { parent, limits } = yield* loadCaller(scope); diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index a847c8d081f5..19313416cb46 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -44,6 +44,7 @@ import * as Option from "effect/Option"; import * as PubSub from "effect/PubSub"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; +import * as Fiber from "effect/Fiber"; import * as Stream from "effect/Stream"; import { McpSchema, McpServer } from "effect/ai"; @@ -444,7 +445,19 @@ function scheduledTaskFromUpsert(input: ScheduledTaskUpsertInput): ScheduledTask prompt: input.prompt, enabled: input.enabled, schedule: - input.schedule.type === "webhook" ? { type: "webhook", signature: null } : input.schedule, + input.schedule.type === "webhook" + ? { + type: "webhook", + signature: + input.schedule.signature == null + ? null + : { + header: input.schedule.signature.header, + encoding: input.schedule.signature.encoding, + prefix: input.schedule.signature.prefix, + }, + } + : input.schedule, projectId: input.projectId, threadId: input.threadId ?? null, workspaceStrategy: input.workspaceStrategy, @@ -473,6 +486,8 @@ const unusedScheduledTaskStubLayer = Layer.succeed( delete: () => Effect.die("ScheduledTaskService.delete is unused in this test"), runNow: () => Effect.die("ScheduledTaskService.runNow is unused in this test"), rotateWebhookToken: () => Effect.die("unused in this test"), + setWebhookSecret: () => Effect.die("unused in this test"), + answerSecretRequest: () => Effect.die("unused in this test"), listWebhookDeliveries: () => Effect.die("unused in this test"), getWebhookDelivery: () => Effect.die("unused in this test"), triggerWebhook: () => Effect.die("unused in this test"), @@ -634,6 +649,8 @@ describe("orchestrator MCP toolkit", () => { ).pipe(Effect.as({ id: input.id })), runNow: () => Effect.die("ScheduledTaskService.runNow is unused in this test"), rotateWebhookToken: () => Effect.die("unused in this test"), + setWebhookSecret: () => Effect.die("unused in this test"), + answerSecretRequest: () => Effect.die("unused in this test"), listWebhookDeliveries: () => Effect.die("unused in this test"), getWebhookDelivery: () => Effect.die("unused in this test"), triggerWebhook: () => Effect.die("unused in this test"), @@ -1472,6 +1489,77 @@ describe("orchestrator MCP toolkit", () => { }); expect(yield* Ref.get(scheduledStore)).toHaveLength(0); + // A signed webhook task waits for its secret, which the agent asks + // the user for. The tool only ever learns the answer's status. + const webhookCall = yield* invoke("schedule_task", { + prompt: + "Release {{body.release.tag_name}} was published. Delegate the release notes.", + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + allowPendingSecret: true, + }, + }, + clientRequestId: "schedule-github-releases-1", + }); + expect(webhookCall.isError).toBe(false); + const webhookTaskId = (webhookCall.structuredContent as { scheduledTaskId: string }) + .scheduledTaskId; + // Not linked to T3 Connect here, so there is no public URL to share. + expect(webhookCall.structuredContent).not.toHaveProperty("webhookUrl"); + const secretFiber = yield* invoke("request_secret", { + scheduledTaskId: webhookTaskId, + label: "GitHub webhook signing secret", + reason: "Enter the same secret in the repository's webhook settings.", + }).pipe(Effect.forkChild); + // The card is recorded before the tool starts waiting, so poll for it + // without the helper's short budget: under load the tool's own reads + // come first. + const asked = yield* Effect.gen(function* () { + while (true) { + const projection = yield* orchestrator.getThreadProjection(parentThreadId); + if ( + projection.turnItems.some( + (item) => item.type === "secret_request" && item.secretStatus === "pending", + ) + ) { + return projection; + } + yield* Effect.sleep("5 millis"); + } + }); + const card = asked.turnItems.find((item) => item.type === "secret_request"); + if (card?.type !== "secret_request" || card.runId === null || card.nodeId === null) { + return yield* Effect.die(new Error("Secret request card missing.")); + } + expect(card).toMatchObject({ + label: "GitHub webhook signing secret", + target: { kind: "scheduled_task_webhook_signature", scheduledTaskId: webhookTaskId }, + }); + // What scheduledTasks.answerSecretRequest dispatches once the secret is stored. + yield* orchestrator.dispatch({ + type: "secret_request.record", + commandId: CommandId.make("command:mcp-secret:saved"), + threadId: parentThreadId, + runId: card.runId, + nodeId: card.nodeId, + turnItemId: card.id, + label: card.label, + reason: card.reason, + target: card.target, + secretStatus: "saved", + }); + const secretCall = yield* Fiber.join(secretFiber); + expect(secretCall.isError).toBe(false); + expect(secretCall.structuredContent).toEqual({ + scheduledTaskId: webhookTaskId, + status: "saved", + }); + yield* invoke("delete_scheduled_task", { scheduledTaskId: webhookTaskId }); + const delegatedCall = yield* invoke("delegate_task", { task: delegatedPrompt, target: { diff --git a/apps/server/src/mcp/toolkits/orchestrator/handlers.ts b/apps/server/src/mcp/toolkits/orchestrator/handlers.ts index d45197b22c9c..4aaff0485226 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/handlers.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/handlers.ts @@ -54,6 +54,12 @@ const handlers = { const service = yield* OrchestratorMcpService.OrchestratorMcpService; return yield* service.deleteScheduledTask(scope, input); }), + request_secret: (input) => + Effect.gen(function* () { + const scope = yield* McpInvocationContext.McpInvocationContext; + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + return yield* service.requestSecret(scope, input); + }), create_threads: (input) => Effect.gen(function* () { const scope = yield* McpInvocationContext.McpInvocationContext; diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index a4a4ba728fec..7c68b84c6e6a 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -6,6 +6,8 @@ import { OrchestratorMcpDelegateTaskResult, OrchestratorMcpDeleteScheduledTaskInput, OrchestratorMcpDeleteScheduledTaskResult, + OrchestratorMcpRequestSecretInput, + OrchestratorMcpRequestSecretResult, OrchestratorMcpFailure, OrchestratorMcpListScheduledTasksInput, OrchestratorMcpListScheduledTasksResult, @@ -97,7 +99,7 @@ const TaskCancelTool = Tool.make("task_cancel", { export const ScheduleTaskTool = Tool.make("schedule_task", { description: - "Create persistent recurring work in the app scheduler, which runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text: {type:'interval', everyMs:3600000} means hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} means weekday mornings; {type:'webhook'} runs on each request to a generated URL (returned as webhookUrl), and its prompt may use {{body.path}}, {{headers.name}}, {{query.name}}, {{body}} or {{request}} placeholders, which are the only request data the run sees. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true); use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread. Report the returned schedule and nextRunAt after success.", + "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). For a sender that signs requests, also set signature, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', allowPendingSecret:true}}, then call request_secret so the user enters the secret privately; never ask for it in chat or invent one. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", parameters: OrchestratorMcpScheduleTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, @@ -146,6 +148,18 @@ const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { .annotate(Tool.Title, "Delete a scheduled task") .annotate(Tool.Destructive, true); +const RequestSecretTool = Tool.make("request_secret", { + description: + "Ask the user for a secret through a private card in this thread, and wait for them to answer. The value is stored by the app and NEVER returned to you or shown in the transcript; the result is only a status (saved, declined, cancelled, or pending if the wait timed out). Use it for a webhook task's signing secret after schedule_task or update_scheduled_task set a signature with allowPendingSecret:true. Tell the user to enter the same secret in the sender (e.g. GitHub's webhook Secret field). Never ask for secrets in chat.", + parameters: OrchestratorMcpRequestSecretInput, + success: OrchestratorMcpRequestSecretResult, + failure: OrchestratorMcpFailure, + failureMode: "return", + dependencies, +}) + .annotate(Tool.Title, "Request a secret from the user") + .annotate(Tool.Destructive, false); + export const CreateThreadsTool = Tool.make("create_threads", { description: "Needs an agent running inside a T3 thread. Create one or more ORDINARY TOP-LEVEL T3 conversations. This is not delegation and does not create child agents/subagents. For delegated work, choose models from orchestrator_capabilities. Prefer native subagents only when they support the chosen model; otherwise call delegate_task, including for same-provider work. Use create_threads for a batch of separate top-level threads sharing this checkout. Prefer t3_thread_launch for a single thread. Both require the user to request separate/new/top-level threads or conversations. Each entry may override provider, model, options, runtime mode, and interaction mode; omitted settings inherit. Project, branch, and worktree always inherit and cannot be overridden here. For independent implementation or a PR stack in its own worktree, use t3_thread_launch with workspaceStrategy instead of asking the agent to create a worktree in its prompt.", @@ -248,6 +262,7 @@ export const OrchestratorToolkit = Toolkit.make( ListScheduledTasksTool, UpdateScheduledTaskTool, DeleteScheduledTaskTool, + RequestSecretTool, CreateThreadsTool, ThreadListTool, ThreadReadTool, diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 7116741f8e2a..4b6d92f36058 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -439,6 +439,8 @@ function commandThreadId(command: OrchestrationV2ServerCommand): ThreadId { case "delegated_task.completion-delivery.dispose": case "thread.created.record": return command.parentThreadId; + case "secret_request.record": + return command.threadId; case "thread.fork": case "thread.merge_back": return command.targetThreadId; @@ -6889,6 +6891,84 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }, ); + /** + * Records or updates the card for a secret an agent asked the user for. The + * item carries the request and its status only; the value goes straight to + * the server's secret store and never through orchestration. + */ + const dispatchSecretRequestRecord = Effect.fn("orchestrationV2.dispatch.secretRequestRecord")( + function* ( + command: Extract, + events: Ref.Ref>, + ) { + const projection = yield* projectionStore + .getThreadRecords( + command.threadId, + ["runs", "nodes", "turnItems", "attempts", "providerTurns"], + { turnItemTypes: ["secret_request"], messageRoles: [] }, + ) + .pipe( + Effect.mapError( + (cause) => new OrchestratorProjectionError({ threadId: command.threadId, cause }), + ), + ); + const run = projection.runs.find((candidate) => candidate.id === command.runId); + if (run === undefined || run.rootNodeId !== command.nodeId) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: `Node ${command.nodeId} is not the root of run ${command.runId}.`, + }); + } + const existing = projection.turnItems.find((item) => item.id === command.turnItemId); + if (existing !== undefined && existing.type !== "secret_request") { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: `Turn item ${command.turnItemId} is not a secret request.`, + }); + } + // A request is answered once; later updates cannot reopen or change it. + if (existing !== undefined && existing.secretStatus !== "pending") return; + + const now = yield* DateTime.now; + const pending = command.secretStatus === "pending"; + const turnItem: OrchestrationV2TurnItem = { + id: command.turnItemId, + threadId: command.threadId, + runId: command.runId, + nodeId: command.nodeId, + providerThreadId: run.providerThreadId, + providerTurnId: providerTurnForRun(projection, run)?.id ?? null, + nativeItemRef: null, + parentItemId: null, + ordinal: existing?.ordinal ?? (yield* nextTurnItemOrdinal(projection)), + status: pending ? "waiting" : command.secretStatus === "saved" ? "completed" : "cancelled", + title: command.label, + startedAt: existing?.startedAt ?? now, + completedAt: pending ? null : now, + updatedAt: now, + type: "secret_request", + label: command.label, + reason: command.reason, + target: command.target, + secretStatus: command.secretStatus, + }; + yield* emit( + events, + command, + )({ + type: "turn-item.updated", + threadId: command.threadId, + runId: command.runId, + nodeId: command.nodeId, + providerInstanceId: run.providerInstanceId, + occurredAt: now, + payload: turnItem, + }); + }, + ); + const dispatchRuntimeRequestRespond = ( command: Extract, events: Ref.Ref>, @@ -10008,6 +10088,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "thread.created.record": yield* dispatchCreatedThreadRecord(command, events); break; + case "secret_request.record": + yield* dispatchSecretRequestRecord(command, events); + break; default: return yield* dispatchUnsupported(command); } diff --git a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts index 0bb2d5053b4b..7f36ee489894 100644 --- a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts +++ b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts @@ -183,6 +183,8 @@ function commandThreadIds(command: OrchestrationV2Command): ReadonlyArray Effect.Effect; + /** Sets a signed webhook task's secret without touching anything else. */ + readonly setWebhookSecret: ( + input: ScheduledTaskSetWebhookSecretInput, + ) => Effect.Effect; + /** + * Answers an agent's request for a webhook signing secret: stores the + * secret on its task, then marks the thread's card saved (or declined). + * Only the status reaches the thread. + */ + readonly answerSecretRequest: ( + input: ScheduledTaskAnswerSecretRequestInput, + ) => Effect.Effect; readonly listWebhookDeliveries: ( input: ScheduledTaskListWebhookDeliveriesInput, ) => Effect.Effect; @@ -1046,7 +1060,7 @@ export const layer = Layer.effect( signature == null ? null : (signature.secret ?? existing?.secret ?? null); const secretChanged = signature == null || signature.secret !== undefined || existing === null; - if (signature != null && secret === null) { + if (signature != null && secret === null && signature.allowPendingSecret !== true) { return yield* taskError("A webhook signature check needs a signing secret.", { taskId: id, }); @@ -1180,6 +1194,75 @@ export const layer = Layer.effect( return { task: yield* loadTask(input.id) }; }); + const setWebhookSecret: ScheduledTaskService["Service"]["setWebhookSecret"] = (input) => + Effect.gen(function* () { + const task = yield* loadTask(input.id); + if (task.schedule.type !== "webhook" || task.schedule.signature === null) { + return yield* taskError("Only a webhook task with a signature check takes a secret.", { + taskId: input.id, + }); + } + const now = yield* localNow; + const updated = yield* sql<{ task_id: string }>` + UPDATE scheduled_tasks + SET webhook_secret = ${input.secret}, updated_at = ${iso(now)} + WHERE task_id = ${input.id} AND created_at = ${task.createdAt} + RETURNING task_id + `.pipe( + Effect.mapError((cause) => + taskError("Could not save the signing secret.", { taskId: input.id, cause }), + ), + ); + if (updated.length === 0) { + return yield* taskError("Schedule task was deleted or replaced.", { taskId: input.id }); + } + yield* notifyChanged; + return { task: yield* loadTask(input.id) }; + }); + + const answerSecretRequest: ScheduledTaskService["Service"]["answerSecretRequest"] = (input) => + Effect.gen(function* () { + const records = yield* threadManagement + .getThreadRecords(input.threadId, ["turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe( + Effect.mapError((cause) => taskError("Could not load the secret request.", { cause })), + ); + const item = records.turnItems.find((candidate) => candidate.id === input.turnItemId); + if (item?.type !== "secret_request" || item.runId === null || item.nodeId === null) { + return yield* taskError("This secret request no longer exists."); + } + if (item.secretStatus !== "pending") { + return yield* taskError("This secret request was already answered."); + } + const taskId = item.target.scheduledTaskId; + // Store first: the card only says saved once the task has the secret. + if (input.answer.type === "save") { + yield* setWebhookSecret({ id: taskId, secret: input.answer.secret }); + } + const secretStatus = input.answer.type === "save" ? "saved" : "declined"; + yield* threadManagement + .dispatch({ + type: "secret_request.record", + commandId: CommandId.make(`secret-request:${input.turnItemId}:${secretStatus}`), + threadId: input.threadId, + runId: item.runId, + nodeId: item.nodeId, + turnItemId: item.id, + label: item.label, + reason: item.reason, + target: item.target, + secretStatus, + }) + .pipe( + Effect.mapError((cause) => + taskError("Saved the secret, but could not update the request.", { taskId, cause }), + ), + ); + }); + const deliveryHeaders = (row: WebhookDeliveryRow): Readonly> => Option.getOrElse(decodeHeadersJson(row.headers_json), () => ({})); const decodeDeliverySummary = (row: WebhookDeliveryRow) => ({ @@ -1646,6 +1729,8 @@ export const layer = Layer.effect( delete: deleteTask, runNow, rotateWebhookToken, + setWebhookSecret, + answerSecretRequest, listWebhookDeliveries, getWebhookDelivery, triggerWebhook, diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 5fdaf45dcf3c..35b6853a9c51 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -708,6 +708,165 @@ it.effect("deleting a task removes its delivery log", () => ), ); +const githubSignature = (secret: string) => + `sha256=${NodeCrypto.createHmac("sha256", secret).update(pullRequestBody).digest("hex")}`; + +it.effect("a signature waiting for its secret rejects every request until one is set", () => + withService(({ service, launches }) => + Effect.gen(function* () { + const { task } = yield* service.upsert( + yield* webhookTaskInput({ + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + allowPendingSecret: true, + }, + }, + }), + ); + assert.isFalse(task.webhook!.hasSecret); + + // Without a secret nothing verifies, however the request is signed. + const early = yield* service.triggerWebhook( + requestFor(task, { + headers: { + "content-type": "application/json", + "x-hub-signature-256": githubSignature(""), + }, + }), + ); + assert.equal(early._tag, "rejected_signature"); + + const { task: withSecret } = yield* service.setWebhookSecret({ + id: task.id, + secret: "github-secret", + }); + assert.isTrue(withSecret.webhook!.hasSecret); + const signed = yield* service.triggerWebhook( + requestFor(task, { + headers: { + "content-type": "application/json", + "x-hub-signature-256": githubSignature("github-secret"), + }, + }), + ); + assert.equal(signed._tag, "accepted"); + yield* Queue.take(launches); + }), + ), +); + +it.effect("a signature without a secret is still refused unless it may wait for one", () => + withService(({ service }) => + Effect.gen(function* () { + const failure = yield* service + .upsert( + yield* webhookTaskInput({ + schedule: { + type: "webhook", + signature: { header: "x-hub-signature-256", encoding: "hex", prefix: "sha256=" }, + }, + }), + ) + .pipe(Effect.flip); + assert.include(failure.message, "needs a signing secret"); + }), + ), +); + +it.effect("answering a secret request stores it on the task, and the card only says so", () => + Effect.gen(function* () { + const dispatched: Array = []; + const threadId = "thread-orchestrator"; + const turnItemId = "turn-item:secret-request:1"; + const pendingItem = (scheduledTaskId: string, secretStatus: string) => ({ + id: turnItemId, + threadId, + runId: "run-1", + nodeId: "node-root", + type: "secret_request", + label: "GitHub webhook signing secret", + reason: "Enter the same secret in GitHub's webhook settings.", + target: { kind: "scheduled_task_webhook_signature", scheduledTaskId }, + secretStatus, + }); + let status = "pending"; + let taskId = ""; + const dependencies = Layer.mergeAll( + NodePlatformCrypto.layer, + Scheduler.layer, + Layer.mock(ThreadLaunchService.ThreadLaunchService)({}), + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: () => + Effect.succeed({ turnItems: [pendingItem(taskId, status)] } as never), + dispatch: (command) => + Effect.sync(() => { + dispatched.push(command); + if (command.type === "secret_request.record") status = command.secretStatus; + return {} as never; + }), + }), + Layer.succeed( + ScheduledTaskService.ScheduledTaskWebhookOrigin, + Effect.succeed({ relayHookBaseUrl: null }), + ), + ); + yield* Effect.gen(function* () { + const service = yield* ScheduledTaskService.ScheduledTaskService; + const { task } = yield* service.upsert( + yield* webhookTaskInput({ + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + allowPendingSecret: true, + }, + }, + }), + ); + taskId = task.id; + + yield* service.answerSecretRequest({ + threadId: threadId as never, + turnItemId: turnItemId as never, + answer: { type: "save", secret: "github-secret" }, + }); + const accepted = yield* service.triggerWebhook( + requestFor(task, { + headers: { + "content-type": "application/json", + "x-hub-signature-256": githubSignature("github-secret"), + }, + }), + ); + assert.equal(accepted._tag, "accepted"); + // The thread learns only the status; the value is nowhere in what it records. + assert.equal(dispatched.length, 1); + assert.include(dispatched[0] as object, { + type: "secret_request.record", + secretStatus: "saved", + }); + assert.notInclude(Object.values(dispatched[0] as object).map(String), "github-secret"); + + // Answered once: a second answer, or a late decline, changes nothing. + const again = yield* service + .answerSecretRequest({ + threadId: threadId as never, + turnItemId: turnItemId as never, + answer: { type: "decline" }, + }) + .pipe(Effect.flip); + assert.include(again.message, "already answered"); + assert.equal(dispatched.length, 1); + }).pipe(Effect.provide(ScheduledTaskService.layer.pipe(Layer.provide(dependencies)))); + }).pipe(Effect.provide(SqlitePersistenceMemory)), +); + const signatureFor = (secret: string) => `sha256=${NodeCrypto.createHmac("sha256", secret).update(pullRequestBody).digest("hex")}`; diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index a9c2e73933a7..255d9e4118dd 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1809,34 +1809,44 @@ const makeWsRpcLayer = ( [ORCHESTRATION_V2_WS_METHODS.dispatchCommand]: (command) => observeRpcEffect( ORCHESTRATION_V2_WS_METHODS.dispatchCommand, - startup - .enqueueCommand( - // A retry also restarts the preparation work the launch owns. - (command.type === "prepared-run.retry" - ? threadLaunch.retryPreparation(command) - : ThreadMessageIntake.dispatchCommand( - ThreadManagementService.withCreationProvenance(command, { - createdBy: "user", - creationSource: - "creationSource" in command ? command.creationSource : "web", - }), - ) - ).pipe(Effect.provide(intakeContext)), - ) - .pipe( - Effect.tap(() => recordClientCommandAnalytics(command)), - Effect.map((result) => ({ sequence: result.sequence })), - Effect.mapError((cause) => { - const detail = userFacingDispatchErrorMessage(cause); - return new OrchestrationV2DispatchCommandError({ + // Secret request status is only written next to storing the + // secret (scheduledTasks.provideSecret) or by the requesting tool. + command.type === "secret_request.record" + ? Effect.fail( + new OrchestrationV2DispatchCommandError({ commandId: command.commandId, commandType: command.type, - message: detail ?? "Failed to dispatch orchestration V2 command", - ...(detail === undefined ? {} : { detail }), - cause, - }); - }), - ), + message: "Secret requests are answered through their own request.", + }), + ) + : startup + .enqueueCommand( + // A retry also restarts the preparation work the launch owns. + (command.type === "prepared-run.retry" + ? threadLaunch.retryPreparation(command) + : ThreadMessageIntake.dispatchCommand( + ThreadManagementService.withCreationProvenance(command, { + createdBy: "user", + creationSource: + "creationSource" in command ? command.creationSource : "web", + }), + ) + ).pipe(Effect.provide(intakeContext)), + ) + .pipe( + Effect.tap(() => recordClientCommandAnalytics(command)), + Effect.map((result) => ({ sequence: result.sequence })), + Effect.mapError((cause) => { + const detail = userFacingDispatchErrorMessage(cause); + return new OrchestrationV2DispatchCommandError({ + commandId: command.commandId, + commandType: command.type, + message: detail ?? "Failed to dispatch orchestration V2 command", + ...(detail === undefined ? {} : { detail }), + cause, + }); + }), + ), { "rpc.aggregate": "orchestrationV2", "orchestration_v2.command_id": command.commandId, @@ -2102,6 +2112,12 @@ const makeWsRpcLayer = ( scheduledTasks.rotateWebhookToken(input), { "rpc.aggregate": "scheduledTasks", "scheduled_task.id": input.id }, ), + [WS_METHODS.scheduledTasksAnswerSecretRequest]: (input) => + observeRpcEffect( + WS_METHODS.scheduledTasksAnswerSecretRequest, + scheduledTasks.answerSecretRequest(input), + { "rpc.aggregate": "scheduledTasks", "orchestration_v2.thread_id": input.threadId }, + ), [WS_METHODS.scheduledTasksListWebhookDeliveries]: (input) => observeRpcEffect( WS_METHODS.scheduledTasksListWebhookDeliveries, diff --git a/packages/client-runtime/src/t3ToolSummary.ts b/packages/client-runtime/src/t3ToolSummary.ts index 8d3a2fa54f06..b9a46fa65287 100644 --- a/packages/client-runtime/src/t3ToolSummary.ts +++ b/packages/client-runtime/src/t3ToolSummary.ts @@ -285,6 +285,9 @@ export function summarizeT3ToolCalls( quantity(countEntities(entityIds("requestId")), "pending question request"), ); break; + case "secret-request": + label = phrase("Asked for", "ask for", quantity(selected.length, "secret")); + break; case "worktree-handoff": label = phrase( "Handed off to", diff --git a/packages/contracts/src/orchestrationV2.test.ts b/packages/contracts/src/orchestrationV2.test.ts index 9eca08fe03ae..e8fd216de980 100644 --- a/packages/contracts/src/orchestrationV2.test.ts +++ b/packages/contracts/src/orchestrationV2.test.ts @@ -205,7 +205,7 @@ describe("orchestration V2 contracts", () => { }); const known = item("item-known", "system_notice", { message: "Hello" }); // A type no build of this client knows, standing in for a newer server's item. - const future = item("item-future", "secret_request", { secretRef: "ref-1" }); + const future = item("item-future", "hologram", { beam: "ref-1" }); const projected = (position: number, turnItem: { readonly id: string }) => ({ position, visibility: "local", diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index cb2ba540e50e..bf590dd3046a 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -1346,6 +1346,35 @@ export const OrchestrationV2WebSearchResult = Schema.Struct({ }); export type OrchestrationV2WebSearchResult = typeof OrchestrationV2WebSearchResult.Type; +/** + * What a secret an agent asked for is used for. The server stores the value + * for that purpose and never puts it in the transcript, projections, or + * model context; the turn item only ever carries this target and a status. + */ +export const OrchestrationV2SecretRequestTarget = Schema.Union([ + Schema.Struct({ + kind: Schema.Literal("scheduled_task_webhook_signature"), + scheduledTaskId: ScheduledTaskId, + }), +]); +export type OrchestrationV2SecretRequestTarget = typeof OrchestrationV2SecretRequestTarget.Type; + +export const OrchestrationV2SecretRequestStatus = Schema.Literals([ + "pending", + "saved", + "declined", + "cancelled", +]); +export type OrchestrationV2SecretRequestStatus = typeof OrchestrationV2SecretRequestStatus.Type; + +const OrchestrationV2SecretRequestFields = { + type: Schema.Literal("secret_request"), + label: TrimmedNonEmptyString, + reason: Schema.String, + target: OrchestrationV2SecretRequestTarget, + secretStatus: OrchestrationV2SecretRequestStatus, +} as const; + export const OrchestrationV2TurnItem = Schema.Union([ Schema.Struct({ ...OrchestrationV2TurnItemBaseFields, @@ -1525,6 +1554,10 @@ export const OrchestrationV2TurnItem = Schema.Union([ targetProviderInstanceId: ProviderInstanceId, targetModel: TrimmedNonEmptyString, }), + Schema.Struct({ + ...OrchestrationV2TurnItemBaseFields, + ...OrchestrationV2SecretRequestFields, + }), Schema.Struct({ ...OrchestrationV2TurnItemBaseFields, type: Schema.Literal("subagent"), @@ -2296,6 +2329,10 @@ export const OrchestrationV2TurnItemJson = Schema.Union([ targetProviderInstanceId: ProviderInstanceId, targetModel: TrimmedNonEmptyString, }), + Schema.Struct({ + ...OrchestrationV2TurnItemJsonBaseFields, + ...OrchestrationV2SecretRequestFields, + }), Schema.Struct({ ...OrchestrationV2TurnItemJsonBaseFields, type: Schema.Literal("subagent"), @@ -3013,6 +3050,21 @@ export const OrchestrationV2Command = Schema.Union([ targetThreadId: ThreadId, targetRunId: Schema.NullOr(RunId), }), + // Server-only: written by the T3 MCP secret request tool and the secret + // RPC, never by a client dispatch (which would let a client mark a request + // saved without storing anything). + Schema.Struct({ + type: Schema.Literal("secret_request.record"), + commandId: CommandId, + threadId: ThreadId, + runId: RunId, + nodeId: NodeId, + turnItemId: TurnItemId, + label: TrimmedNonEmptyString, + reason: Schema.String, + target: OrchestrationV2SecretRequestTarget, + secretStatus: OrchestrationV2SecretRequestStatus, + }), Schema.Struct({ type: Schema.Literal("provider.switch"), commandId: CommandId, diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 776cf4be52d5..42024a066e47 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -541,8 +541,15 @@ export const OrchestratorMcpScheduledTask = Schema.Struct({ schedule: ScheduledTaskSchedule, nextRunAt: Schema.NullOr(IsoDateTime), lastRunStatus: ScheduledTaskRunStatus, - /** For webhook tasks: the public T3 Connect URL, or the environment-relative path when the environment is not linked. */ - webhookUrl: Schema.optional(Schema.String), + /** For webhook tasks: the public T3 Connect URL. Absent when this environment has no managed tunnel. */ + webhookUrl: Schema.optional(Schema.String).annotate({ + description: + "Public URL to give the sender. Absent when this environment has no T3 Connect managed tunnel; the user must enable T3 Connect remote access first.", + }), + webhookSignature: Schema.optional(Schema.Literals(["none", "secret_pending", "set"])).annotate({ + description: + "Signature check state: none, secret_pending (call request_secret; requests are rejected until it is set), or set.", + }), }); export type OrchestratorMcpScheduledTask = typeof OrchestratorMcpScheduledTask.Type; @@ -577,6 +584,32 @@ export const OrchestratorMcpUpdateScheduledTaskInput = Schema.Struct({ export type OrchestratorMcpUpdateScheduledTaskInput = typeof OrchestratorMcpUpdateScheduledTaskInput.Type; +export const OrchestratorMcpRequestSecretInput = Schema.Struct({ + scheduledTaskId: ScheduledTaskId.annotate({ + description: + "Webhook task (with a signature check) whose signing secret the user should provide.", + }), + label: TrimmedNonEmptyString.annotate({ + description: "Short name shown on the card, e.g. 'GitHub webhook signing secret'.", + }), + reason: Schema.optional(Schema.String).annotate({ + description: "One sentence on what the secret is for and where the user will also enter it.", + }), + timeoutMs: Schema.optional( + Schema.Int.check(Schema.isBetween({ minimum: 1_000, maximum: 60 * 60 * 1_000 })), + ).annotate({ description: "How long to wait for the user. Default 10 minutes." }), +}); +export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; + +export const OrchestratorMcpRequestSecretResult = Schema.Struct({ + scheduledTaskId: ScheduledTaskId, + status: Schema.Literals(["saved", "declined", "cancelled", "pending"]).annotate({ + description: + "saved: the secret is stored and the task verifies requests with it. declined: the user chose not to. cancelled: the request ended with the run. pending: the wait timed out and the card is still open.", + }), +}); +export type OrchestratorMcpRequestSecretResult = typeof OrchestratorMcpRequestSecretResult.Type; + export const OrchestratorMcpDeleteScheduledTaskInput = Schema.Struct({ scheduledTaskId: ScheduledTaskId, }); diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 575ad6e32517..9fbc2ed95327 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -311,6 +311,7 @@ import { ScheduledTaskListResult, ScheduledTaskRunNowInput, ScheduledTaskRotateWebhookTokenInput, + ScheduledTaskAnswerSecretRequestInput, ScheduledTaskListWebhookDeliveriesInput, ScheduledTaskListWebhookDeliveriesResult, ScheduledTaskGetWebhookDeliveryInput, @@ -480,6 +481,7 @@ export const WS_METHODS = { scheduledTasksDelete: "scheduledTasks.delete", scheduledTasksRunNow: "scheduledTasks.runNow", scheduledTasksRotateWebhookToken: "scheduledTasks.rotateWebhookToken", + scheduledTasksAnswerSecretRequest: "scheduledTasks.answerSecretRequest", scheduledTasksListWebhookDeliveries: "scheduledTasks.listWebhookDeliveries", scheduledTasksGetWebhookDelivery: "scheduledTasks.getWebhookDelivery", @@ -1685,6 +1687,14 @@ const WsScheduledTasksRotateWebhookTokenRpc = Rpc.make( }, ); +const WsScheduledTasksAnswerSecretRequestRpc = Rpc.make( + WS_METHODS.scheduledTasksAnswerSecretRequest, + { + payload: ScheduledTaskAnswerSecretRequestInput, + error: Schema.Union([ScheduledTaskError, EnvironmentAuthorizationError]), + }, +); + const WsScheduledTasksListWebhookDeliveriesRpc = Rpc.make( WS_METHODS.scheduledTasksListWebhookDeliveries, { @@ -1789,6 +1799,7 @@ export const WsRpcGroup = RpcGroup.make( WsScheduledTasksDeleteRpc, WsScheduledTasksRunNowRpc, WsScheduledTasksRotateWebhookTokenRpc, + WsScheduledTasksAnswerSecretRequestRpc, WsScheduledTasksListWebhookDeliveriesRpc, WsScheduledTasksGetWebhookDeliveryRpc, WsServerReportClientActivityRpc, diff --git a/packages/contracts/src/scheduledTask.ts b/packages/contracts/src/scheduledTask.ts index 3140ff8a79cf..9409559c4d36 100644 --- a/packages/contracts/src/scheduledTask.ts +++ b/packages/contracts/src/scheduledTask.ts @@ -7,6 +7,7 @@ import { ProjectId, ScheduledTaskId, ThreadId, + TurnItemId, TrimmedNonEmptyString, } from "./baseSchemas.ts"; import { ModelSelection } from "./modelSelection.ts"; @@ -104,7 +105,12 @@ const ScheduledTaskUpsertWebhookSchedule = Schema.Struct({ Schema.Struct({ ...ScheduledTaskWebhookSignatureFields, secret: Schema.optional(TrimmedNonEmptyString).annotate({ - description: "Shared signing secret. Omit to keep the stored secret.", + description: + "Shared signing secret. Omit to keep the stored secret; a new task without one rejects every request until a secret is set.", + }), + allowPendingSecret: Schema.optional(Schema.Boolean).annotate({ + description: + "Save the check without a secret yet; requests are rejected until one is provided. For agents that ask the user for the secret afterwards.", }), }), ), @@ -243,6 +249,28 @@ export const ScheduledTaskRotateWebhookTokenInput = Schema.Struct({ }); export type ScheduledTaskRotateWebhookTokenInput = typeof ScheduledTaskRotateWebhookTokenInput.Type; +/** Sets only a webhook task's signing secret, leaving the rest of the task as it is. */ +export const ScheduledTaskSetWebhookSecretInput = Schema.Struct({ + id: ScheduledTaskId, + secret: TrimmedNonEmptyString, +}); +export type ScheduledTaskSetWebhookSecretInput = typeof ScheduledTaskSetWebhookSecretInput.Type; + +/** + * The user's answer to an agent's request for a webhook signing secret. The + * secret goes straight to the task; the thread only learns it was saved. + */ +export const ScheduledTaskAnswerSecretRequestInput = Schema.Struct({ + threadId: ThreadId, + turnItemId: TurnItemId, + answer: Schema.Union([ + Schema.Struct({ type: Schema.Literal("save"), secret: TrimmedNonEmptyString }), + Schema.Struct({ type: Schema.Literal("decline") }), + ]), +}); +export type ScheduledTaskAnswerSecretRequestInput = + typeof ScheduledTaskAnswerSecretRequestInput.Type; + export const ScheduledTaskWebhookDeliveryId = TrimmedNonEmptyString.pipe( Schema.brand("ScheduledTaskWebhookDeliveryId"), ); diff --git a/packages/shared/src/t3McpToolPresentation.ts b/packages/shared/src/t3McpToolPresentation.ts index ca3928112c02..2b7366392f23 100644 --- a/packages/shared/src/t3McpToolPresentation.ts +++ b/packages/shared/src/t3McpToolPresentation.ts @@ -38,6 +38,7 @@ export type T3McpToolSummaryAction = | "question-list" | "question-read" | "question-respond" + | "secret-request" | "worktree-handoff" | "worktree-list" | "worktree-status" @@ -130,6 +131,7 @@ const T3_MCP_TOOLS: Readonly> = { ["Delete", "Deleting", "Requested deletion of", "a scheduled task"], "schedule-delete", ), + request_secret: tool(["Ask for", "Asking for", "Asked for", "a secret"], "secret-request"), create_threads: tool(["Create", "Creating", "Created", "T3 threads"], "thread-create"), t3_thread_start: tool(["Start", "Starting", "Started", "a T3 thread"], "thread-create"), t3_thread_list: tool(["List", "Listing", "Listed", "T3 threads"], "thread-list"), From 5fd27540fd1c90d44523acd217ea78764eb6855c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:50:20 -0700 Subject: [PATCH 02/22] feat(client-runtime): shared command and display state for agent secret requests Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/client-runtime/package.json | 4 + .../client-runtime/src/secretRequest.test.ts | 67 ++++++++++++++ packages/client-runtime/src/secretRequest.ts | 88 +++++++++++++++++++ packages/client-runtime/src/state/server.ts | 10 +++ 4 files changed, 169 insertions(+) create mode 100644 packages/client-runtime/src/secretRequest.test.ts create mode 100644 packages/client-runtime/src/secretRequest.ts diff --git a/packages/client-runtime/package.json b/packages/client-runtime/package.json index 6f95926cd1d1..13cfabf182b8 100644 --- a/packages/client-runtime/package.json +++ b/packages/client-runtime/package.json @@ -35,6 +35,10 @@ "types": "./src/userMessage.ts", "default": "./src/userMessage.ts" }, + "./secret-request": { + "types": "./src/secretRequest.ts", + "default": "./src/secretRequest.ts" + }, "./scheduled-task-webhook": { "types": "./src/scheduledTaskWebhook.ts", "default": "./src/scheduledTaskWebhook.ts" diff --git a/packages/client-runtime/src/secretRequest.test.ts b/packages/client-runtime/src/secretRequest.test.ts new file mode 100644 index 000000000000..2c797ae95734 --- /dev/null +++ b/packages/client-runtime/src/secretRequest.test.ts @@ -0,0 +1,67 @@ +import { ScheduledTaskError, ThreadId, TurnItemId } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { + secretRequestAnswerInput, + secretRequestDisplay, + secretRequestFailureMessage, + type SecretRequestItem, +} from "./secretRequest.ts"; + +const item = { + id: TurnItemId.make("turn-item:secret-request:1"), + threadId: ThreadId.make("thread-1"), +}; + +describe("secretRequestDisplay", () => { + it("shows the form only while pending and maps every answer to its outcome copy", () => { + const display = (secretStatus: SecretRequestItem["secretStatus"]) => + secretRequestDisplay({ secretStatus } as SecretRequestItem); + expect(display("pending")).toEqual({ kind: "pending" }); + expect(display("saved")).toEqual({ + kind: "answered", + outcome: "saved", + label: "Saved securely and kept private", + }); + expect(display("declined")).toEqual({ + kind: "answered", + outcome: "declined", + label: "Declined", + }); + expect(display("cancelled")).toEqual({ + kind: "answered", + outcome: "ended", + label: "Request ended", + }); + }); +}); + +describe("secretRequestAnswerInput", () => { + it("refuses a blank save and trims the value the server will store", () => { + expect(secretRequestAnswerInput(item, { type: "save", secret: " " })).toBeNull(); + expect(secretRequestAnswerInput(item, { type: "save", secret: " whsec_1 " })).toEqual({ + threadId: item.threadId, + turnItemId: item.id, + answer: { type: "save", secret: "whsec_1" }, + }); + expect(secretRequestAnswerInput(item, { type: "decline" })).toEqual({ + threadId: item.threadId, + turnItemId: item.id, + answer: { type: "decline" }, + }); + }); +}); + +describe("secretRequestFailureMessage", () => { + it("passes through server errors but hides anything that could echo the payload", () => { + expect( + secretRequestFailureMessage( + new ScheduledTaskError({ message: "This secret request was already answered." }), + ), + ).toBe("This secret request was already answered."); + expect(secretRequestFailureMessage(new Error('Expected string, got "whsec_1"'))).toBe( + "Could not answer the request. Try again.", + ); + expect(secretRequestFailureMessage(undefined)).toBe("Could not answer the request. Try again."); + }); +}); diff --git a/packages/client-runtime/src/secretRequest.ts b/packages/client-runtime/src/secretRequest.ts new file mode 100644 index 000000000000..a9d84fa561fc --- /dev/null +++ b/packages/client-runtime/src/secretRequest.ts @@ -0,0 +1,88 @@ +import type { + OrchestrationV2TurnItem, + ScheduledTaskAnswerSecretRequestInput, +} from "@t3tools/contracts"; + +export type SecretRequestItem = Extract< + OrchestrationV2TurnItem, + { readonly type: "secret_request" } +>; + +/** What a secret request card shows: the form while pending, otherwise a one-line outcome. */ +export type SecretRequestDisplay = + | { readonly kind: "pending" } + | { + readonly kind: "answered"; + readonly outcome: "saved" | "declined" | "ended"; + readonly label: string; + }; + +const SAVED_DISPLAY: SecretRequestDisplay = { + kind: "answered", + outcome: "saved", + label: "Saved securely and kept private", +}; +const DECLINED_DISPLAY: SecretRequestDisplay = { + kind: "answered", + outcome: "declined", + label: "Declined", +}; +const ENDED_DISPLAY: SecretRequestDisplay = { + kind: "answered", + outcome: "ended", + label: "Request ended", +}; +const PENDING_DISPLAY: SecretRequestDisplay = { kind: "pending" }; + +export function secretRequestDisplay(item: SecretRequestItem): SecretRequestDisplay { + switch (item.secretStatus) { + case "pending": + return PENDING_DISPLAY; + case "saved": + return SAVED_DISPLAY; + case "declined": + return DECLINED_DISPLAY; + case "cancelled": + return ENDED_DISPLAY; + } +} + +/** + * Builds the RPC payload for an answer. A save with a blank value returns null, + * since the server rejects it; callers keep Save disabled instead. + */ +export function secretRequestAnswerInput( + item: Pick, + answer: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, +): ScheduledTaskAnswerSecretRequestInput | null { + if (answer.type === "decline") { + return { threadId: item.threadId, turnItemId: item.id, answer: { type: "decline" } }; + } + const secret = answer.secret.trim(); + if (secret.length === 0) return null; + return { threadId: item.threadId, turnItemId: item.id, answer: { type: "save", secret } }; +} + +/** Failures whose message is written for the user and never echoes the request payload. */ +const USER_FACING_FAILURE_TAGS = new Set(["ScheduledTaskError", "EnvironmentAuthorizationError"]); + +/** + * Inline error copy for a failed answer. Only known server errors pass their + * message through: anything else (transport or encoding failures) gets the + * generic copy, so the typed value can never surface in the UI. + */ +export function secretRequestFailureMessage(failure: unknown): string { + if ( + typeof failure === "object" && + failure !== null && + "_tag" in failure && + typeof failure._tag === "string" && + USER_FACING_FAILURE_TAGS.has(failure._tag) && + "message" in failure && + typeof failure.message === "string" && + failure.message.trim().length > 0 + ) { + return failure.message; + } + return "Could not answer the request. Try again."; +} diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index 1e42b6826b3f..1e53b0195815 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1299,6 +1299,16 @@ export function createServerEnvironmentAtoms( scheduler: configScheduler, concurrency: configConcurrency, }), + // Off the config lane like run-now: answering a card must not queue + // behind settings edits. One answer per card at a time. + answerScheduledTaskSecretRequest: createEnvironmentRpcCommand(runtime, { + label: "environment-data:server:scheduled-task:answer-secret-request", + tag: WS_METHODS.scheduledTasksAnswerSecretRequest, + concurrency: { + mode: "singleFlight", + key: ({ environmentId, input }) => `${environmentId}:${input.threadId}:${input.turnItemId}`, + }, + }), refreshUsageRates: createEnvironmentRpcCommand(runtime, { label: "environment-data:server:refresh-usage-rates", tag: WS_METHODS.serverRefreshUsageRates, From 0acf0680c5627304e76eb8d364971adec776d0e7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:56:07 -0700 Subject: [PATCH 03/22] feat(client-runtime): forked threads show a pending secret request without its form Co-Authored-By: Claude Opus 5.5 (1M context) --- .../client-runtime/src/secretRequest.test.ts | 12 +++++++++++- packages/client-runtime/src/secretRequest.ts | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/client-runtime/src/secretRequest.test.ts b/packages/client-runtime/src/secretRequest.test.ts index 2c797ae95734..0284dd4bcd37 100644 --- a/packages/client-runtime/src/secretRequest.test.ts +++ b/packages/client-runtime/src/secretRequest.test.ts @@ -16,7 +16,7 @@ const item = { describe("secretRequestDisplay", () => { it("shows the form only while pending and maps every answer to its outcome copy", () => { const display = (secretStatus: SecretRequestItem["secretStatus"]) => - secretRequestDisplay({ secretStatus } as SecretRequestItem); + secretRequestDisplay({ secretStatus }, "local"); expect(display("pending")).toEqual({ kind: "pending" }); expect(display("saved")).toEqual({ kind: "answered", @@ -34,6 +34,16 @@ describe("secretRequestDisplay", () => { label: "Request ended", }); }); + + it("never offers the form for a request inherited from another thread", () => { + expect(secretRequestDisplay({ secretStatus: "pending" }, "inherited")).toEqual({ + kind: "pending-elsewhere", + label: "Waiting for an answer in the original thread", + }); + expect(secretRequestDisplay({ secretStatus: "saved" }, "inherited")).toMatchObject({ + outcome: "saved", + }); + }); }); describe("secretRequestAnswerInput", () => { diff --git a/packages/client-runtime/src/secretRequest.ts b/packages/client-runtime/src/secretRequest.ts index a9d84fa561fc..43d646a93bc3 100644 --- a/packages/client-runtime/src/secretRequest.ts +++ b/packages/client-runtime/src/secretRequest.ts @@ -11,6 +11,7 @@ export type SecretRequestItem = Extract< /** What a secret request card shows: the form while pending, otherwise a one-line outcome. */ export type SecretRequestDisplay = | { readonly kind: "pending" } + | { readonly kind: "pending-elsewhere"; readonly label: string } | { readonly kind: "answered"; readonly outcome: "saved" | "declined" | "ended"; @@ -33,11 +34,22 @@ const ENDED_DISPLAY: SecretRequestDisplay = { label: "Request ended", }; const PENDING_DISPLAY: SecretRequestDisplay = { kind: "pending" }; +const PENDING_ELSEWHERE_DISPLAY: SecretRequestDisplay = { + kind: "pending-elsewhere", + label: "Waiting for an answer in the original thread", +}; -export function secretRequestDisplay(item: SecretRequestItem): SecretRequestDisplay { +/** + * `visibility` is the projected row's: a request inherited from another + * thread (a fork) can only be answered where it was asked. + */ +export function secretRequestDisplay( + item: Pick, + visibility: "local" | "inherited" | "synthetic", +): SecretRequestDisplay { switch (item.secretStatus) { case "pending": - return PENDING_DISPLAY; + return visibility === "local" ? PENDING_DISPLAY : PENDING_ELSEWHERE_DISPLAY; case "saved": return SAVED_DISPLAY; case "declined": From f4bce4551d4c813db806f606e5e72172be4bccb6 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:56:08 -0700 Subject: [PATCH 04/22] feat(web): answer an agent's secret request from a card in the timeline Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/components/chat/MessagesTimeline.tsx | 10 ++ .../src/components/chat/SecretRequestCard.tsx | 156 ++++++++++++++++++ apps/web/src/session-logic.ts | 3 + 3 files changed, 169 insertions(+) create mode 100644 apps/web/src/components/chat/SecretRequestCard.tsx diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 1417c3000f03..71cbce5a755e 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -276,6 +276,7 @@ import { V2LifecycleRow, type HandoffTimelineRun, } from "./V2LifecycleRow"; +import { SecretRequestCard } from "./SecretRequestCard"; import { TimelineSystemDivider } from "./TimelineSystemDivider"; import { SkillChipIcon, SkillInlineText } from "./SkillInlineText"; @@ -2808,6 +2809,15 @@ function V2EventTimelineRow({ row }: { row: Extract 1) { return ; } + if (item.type === "secret_request") { + return ( + + ); + } if (isV2LifecycleItem(item)) { return ( } + label={ + <> + {item.label} · {display.label} + + } + /> + ); + } + return ; +} + +function PendingSecretRequestForm(props: { + readonly environmentId: EnvironmentId; + readonly item: SecretRequestItem; +}) { + const { item } = props; + const inputId = useId(); + const errorId = useId(); + const answer = useAtomCommand(serverEnvironment.answerScheduledTaskSecretRequest, { + label: "scheduled task answer secret request", + // The failure cause holds the request; keep it out of the console. + reportFailure: false, + reportDefect: false, + }); + const [secret, setSecret] = useState(""); + const [submitting, setSubmitting] = useState(false); + const [error, setError] = useState(null); + + const send = async ( + reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, + ) => { + const input = secretRequestAnswerInput(item, reply); + if (input === null || submitting) return; + setSubmitting(true); + setError(null); + const result = await answer({ environmentId: props.environmentId, input }); + setSubmitting(false); + if (result._tag === "Success") { + // The card switches to its answered row once the item updates. + setSecret(""); + return; + } + if (!isAtomCommandInterrupted(result)) { + setError(secretRequestFailureMessage(squashAtomCommandFailure(result))); + } + }; + + const onSubmit = (event: FormEvent) => { + event.preventDefault(); + void send({ type: "save", secret }); + }; + + return ( +
+
+ +
+ + {item.reason.trim() ? ( +

{item.reason}

+ ) : null} +

+ Stored for this task only. The agent never sees it. +

+
+
+
+ setSecret(event.currentTarget.value)} + /> + + +
+ {error !== null ? ( + + ) : null} +
+ ); +} diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index 4f3e8513b5b5..984dc8743a8a 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -315,12 +315,15 @@ const STANDALONE_V2_ITEM_TYPES = new Set([ "fork", "thread_created", + // Still answerable after a steer supersedes the attempt that asked. + "secret_request", ]); export function timelineEntryIsPersistentResourceCard(entry: TimelineEntry): boolean { From 6971bd63c13e16ff8e022362abd067e66071775e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:56:09 -0700 Subject: [PATCH 05/22] feat(mobile): answer an agent's secret request from a card in the thread feed Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/SecretRequestCard.tsx | 151 ++++++++++++++++++ .../src/features/threads/ThreadFeed.tsx | 12 ++ apps/mobile/src/lib/threadActivity.ts | 15 +- 3 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 apps/mobile/src/features/threads/SecretRequestCard.tsx diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx new file mode 100644 index 000000000000..b31e045b8331 --- /dev/null +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -0,0 +1,151 @@ +import { + secretRequestAnswerInput, + secretRequestDisplay, + secretRequestFailureMessage, + type SecretRequestItem, +} from "@t3tools/client-runtime/secret-request"; +import { + isAtomCommandInterrupted, + squashAtomCommandFailure, +} from "@t3tools/client-runtime/state/runtime"; +import type { EnvironmentId, OrchestrationV2ProjectedTurnItem } from "@t3tools/contracts"; +import { useState } from "react"; +import { View, type ColorValue } from "react-native"; + +import { SymbolView, type AppSymbolName } from "../../components/AppSymbol"; +import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText"; +import { serverEnvironment } from "../../state/server"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { RequestActionButton } from "./RequestActionButton"; + +/** + * Feed card for a secret an agent asked the user for. The typed value lives + * only in this component's state and the RPC payload: it is never logged, + * alerted, or persisted, and the field clears once the answer is sent. + */ +const LOCK_SYMBOL: AppSymbolName = { ios: "lock", android: "lock" }; + +export function SecretRequestCard(props: { + readonly environmentId: EnvironmentId; + readonly projectedItem: OrchestrationV2ProjectedTurnItem; + readonly iconColor: ColorValue; +}) { + const { item, visibility } = props.projectedItem; + if (item.type !== "secret_request") return null; + const display = secretRequestDisplay(item, visibility); + if (display.kind === "pending") { + return ( + + ); + } + const icon: AppSymbolName = + display.kind === "pending-elsewhere" + ? LOCK_SYMBOL + : display.outcome === "saved" + ? "checkmark" + : "minus"; + return ( + + + + {item.label} · {display.label} + + + ); +} + +function PendingSecretRequestForm(props: { + readonly environmentId: EnvironmentId; + readonly item: SecretRequestItem; + readonly iconColor: ColorValue; +}) { + const { item } = props; + const answer = useAtomCommand(serverEnvironment.answerScheduledTaskSecretRequest, { + label: "scheduled task answer secret request", + // The failure cause holds the request; keep it out of the console. + reportFailure: false, + reportDefect: false, + }); + const [secret, setSecret] = useState(""); + const [submitting, setSubmitting] = useState(false); + const [error, setError] = useState(null); + + const send = async ( + reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, + ) => { + const input = secretRequestAnswerInput(item, reply); + if (input === null || submitting) return; + setSubmitting(true); + setError(null); + const result = await answer({ environmentId: props.environmentId, input }); + setSubmitting(false); + if (result._tag === "Success") { + // The card switches to its answered row once the item updates. + setSecret(""); + return; + } + if (!isAtomCommandInterrupted(result)) { + setError(secretRequestFailureMessage(squashAtomCommandFailure(result))); + } + }; + + return ( + + + + + + + {item.label} + {item.reason.trim() ? ( + {item.reason} + ) : null} + + Stored for this task only. The agent never sees it. + + + + void send({ type: "save", secret })} + /> + {error !== null ? ( + + {error} + + ) : null} + + + void send({ type: "decline" })} + /> + + + void send({ type: "save", secret })} + /> + + + + ); +} diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx index 7c71b4dd6c79..58191639331a 100644 --- a/apps/mobile/src/features/threads/ThreadFeed.tsx +++ b/apps/mobile/src/features/threads/ThreadFeed.tsx @@ -1,5 +1,6 @@ import { ThreadContextDivider } from "./thread-context-divider"; import { ThreadHandoffRow } from "./thread-handoff-row"; +import { SecretRequestCard } from "./SecretRequestCard"; import { WorktreeWorkingHeader, WorktreeSetupCard, @@ -162,6 +163,7 @@ import { threadFeedRunIsUnsettled, isContextCompactionActivityGroup, isContextHandoffActivityGroup, + isSecretRequestActivityGroup, type ThreadFeedEntry, type ThreadFeedLatestRun, } from "../../lib/threadActivity"; @@ -1621,6 +1623,16 @@ function renderFeedEntry( ); } + if (entry.type === "activity-group" && isSecretRequestActivityGroup(entry)) { + return ( + + ); + } + if (entry.type === "activity-group" && isContextCompactionActivityGroup(entry)) { const label = entry.activities[0]!.summary; const active = diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index c9d6a58237da..927fd59b7dec 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -321,6 +321,13 @@ export function isContextHandoffActivityGroup(entry: ThreadFeedActivityGroup): b ); } +export function isSecretRequestActivityGroup(entry: ThreadFeedActivityGroup): boolean { + return ( + entry.activities.length === 1 && + entry.activities[0]?.projectedItem.item.type === "secret_request" + ); +} + function isUserInputActivityGroup(entry: ThreadFeedActivityGroup): boolean { return entry.activities.some((activity) => activity.workEntry.questionAnswer !== undefined); } @@ -420,7 +427,13 @@ function itemIsToolLike(item: OrchestrationV2TurnItem): boolean { } function itemIsProminent(item: OrchestrationV2TurnItem): boolean { - return item.type === "fork" || item.type === "thread_created" || item.type === "system_notice"; + return ( + item.type === "fork" || + item.type === "thread_created" || + item.type === "system_notice" || + // An answerable card: it must stand alone and never fold away with the run. + item.type === "secret_request" + ); } function itemStatus(item: OrchestrationV2TurnItem): ThreadFeedActivity["status"] { From 058313c93c9e02c733ca25eb22046ed889c9f121 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 21:56:40 -0700 Subject: [PATCH 06/22] docs(server): secret request comments name the answerSecretRequest RPC Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.ts | 2 +- apps/server/src/ws.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 8f55a48933c2..37f00dd4da9a 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1593,7 +1593,7 @@ const make = Effect.gen(function* () { ); yield* record("pending"); - // The card is answered by the user (scheduledTasks.provideWebhookSecret) + // The card is answered by the user (scheduledTasks.answerSecretRequest) // or ends with the run; poll it like a delegated task. const answered = yield* Effect.gen(function* () { while (true) { diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 255d9e4118dd..520e7defafb3 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1810,7 +1810,7 @@ const makeWsRpcLayer = ( observeRpcEffect( ORCHESTRATION_V2_WS_METHODS.dispatchCommand, // Secret request status is only written next to storing the - // secret (scheduledTasks.provideSecret) or by the requesting tool. + // secret (scheduledTasks.answerSecretRequest) or by the requesting tool. command.type === "secret_request.record" ? Effect.fail( new OrchestrationV2DispatchCommandError({ From 48d72d5475e3692313ad1d5395706149b1e5247a Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 22:29:11 -0700 Subject: [PATCH 07/22] refactor: secret requests are generic and return a one-use secretRef request_secret was built around webhook tasks: it took a scheduledTaskId, its card targeted "scheduled_task_webhook_signature", and answering it was a scheduled-task RPC. Agents need secrets for more than webhooks. - request_secret takes only a label, reason and placeholder. Saving stores the value under a one-use secretRef, which the tool returns instead of the value. - A tool that needs a secret accepts a secretRef and consumes it. Webhook signatures do (signature.secretRef). A ref works once, only in the project it was entered for. - Answering is secrets.answerRequest, owned by a new SecretRequests service, not the scheduled-task service. - The pending-secret webhook state (allowPendingSecret, secret_pending) is gone: the agent asks first, then saves the task with the ref. - The card follows a title, description, field with "Save securely", then "Stored securely, never shown to the agent" layout, with Decline as a quiet action, on web and mobile. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/SecretRequestCard.tsx | 65 ++++---- apps/server/src/auth/RpcAuthorization.ts | 2 +- .../OrchestratorMcpService.activity.test.ts | 5 + .../src/mcp/OrchestratorMcpService.test.ts | 11 ++ apps/server/src/mcp/OrchestratorMcpService.ts | 48 +++--- ...OrchestratorMcpToolkit.integration.test.ts | 109 ++++++------ apps/server/src/mcp/toolkits/core.test.ts | 4 + .../src/mcp/toolkits/orchestrator/tools.ts | 4 +- .../toolkits/worktree/registration.test.ts | 2 + ...laudeAutomaticDelivery.integration.test.ts | 2 + .../src/orchestration-v2/Orchestrator.ts | 2 +- .../ThreadLaunchService.test.ts | 10 +- .../src/orchestration-v2/runtimeLayer.ts | 7 +- .../provider/T3OrchestrationInstructions.ts | 2 +- .../ScheduledTaskService.schedule.test.ts | 3 + .../ScheduledTaskService.test.ts | 3 + .../scheduledTasks/ScheduledTaskService.ts | 103 ++---------- .../ScheduledTaskService.webhook.test.ts | 154 ++++------------- .../scheduling/Scheduler.integration.test.ts | 2 + .../server/src/secrets/SecretRequests.test.ts | 137 ++++++++++++++++ apps/server/src/secrets/SecretRequests.ts | 155 ++++++++++++++++++ apps/server/src/ws.ts | 15 +- .../src/components/chat/SecretRequestCard.tsx | 92 ++++++----- .../client-runtime/src/secretRequest.test.ts | 4 +- packages/client-runtime/src/secretRequest.ts | 13 +- packages/client-runtime/src/state/server.ts | 10 +- packages/contracts/src/baseSchemas.ts | 3 + packages/contracts/src/index.ts | 1 + packages/contracts/src/orchestrationV2.ts | 21 +-- packages/contracts/src/orchestratorMcp.ts | 27 +-- packages/contracts/src/rpc.ts | 17 +- packages/contracts/src/scheduledTask.ts | 31 +--- packages/contracts/src/secretRequest.ts | 22 +++ 33 files changed, 636 insertions(+), 450 deletions(-) create mode 100644 apps/server/src/secrets/SecretRequests.test.ts create mode 100644 apps/server/src/secrets/SecretRequests.ts create mode 100644 packages/contracts/src/secretRequest.ts diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx index b31e045b8331..aec7de5444be 100644 --- a/apps/mobile/src/features/threads/SecretRequestCard.tsx +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -1,4 +1,6 @@ import { + SECRET_REQUEST_DEFAULT_PLACEHOLDER, + SECRET_REQUEST_PRIVACY_NOTE, secretRequestAnswerInput, secretRequestDisplay, secretRequestFailureMessage, @@ -24,6 +26,7 @@ import { RequestActionButton } from "./RequestActionButton"; * alerted, or persisted, and the field clears once the answer is sent. */ const LOCK_SYMBOL: AppSymbolName = { ios: "lock", android: "lock" }; +const PRIVATE_SYMBOL: AppSymbolName = { ios: "checkmark.shield", android: "lock" }; export function SecretRequestCard(props: { readonly environmentId: EnvironmentId; @@ -64,8 +67,8 @@ function PendingSecretRequestForm(props: { readonly iconColor: ColorValue; }) { const { item } = props; - const answer = useAtomCommand(serverEnvironment.answerScheduledTaskSecretRequest, { - label: "scheduled task answer secret request", + const answer = useAtomCommand(serverEnvironment.answerSecretRequest, { + label: "answer secret request", // The failure cause holds the request; keep it out of the console. reportFailure: false, reportDefect: false, @@ -93,24 +96,19 @@ function PendingSecretRequestForm(props: { } }; + // Same hierarchy as web: what is asked, why, the field, then the promise + // about where the value goes. return ( - - - - - - - {item.label} - {item.reason.trim() ? ( - {item.reason} - ) : null} - - Stored for this task only. The agent never sees it. - - + + + {item.label} + {item.reason.trim() ? ( + {item.reason} + ) : null} ) : null} - - - void send({ type: "decline" })} - /> - - - void send({ type: "save", secret })} + void send({ type: "save", secret })} + /> + + + + + {SECRET_REQUEST_PRIVACY_NOTE} + + void send({ type: "decline" })} + /> ); diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 88a665e035b7..a2988000ca78 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -96,7 +96,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.scheduledTasksDelete]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksRunNow]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksRotateWebhookToken]: AuthOrchestrationOperateScope, - [WS_METHODS.scheduledTasksAnswerSecretRequest]: AuthOrchestrationOperateScope, + [WS_METHODS.secretsAnswerRequest]: AuthOrchestrationOperateScope, // Delivery logs hold request bodies, so they need the same scope as the URL. [WS_METHODS.scheduledTasksListWebhookDeliveries]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksGetWebhookDelivery]: AuthOrchestrationOperateScope, diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index 2433eb8a704a..b179a2f45822 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -20,6 +20,7 @@ import * as ProviderAdapterRegistry from "../orchestration-v2/ProviderAdapterReg import * as ProviderRegistry from "../provider/Services/ProviderRegistry.ts"; import * as ProjectService from "../project/ProjectService.ts"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; import type * as McpInvocationContext from "./McpInvocationContext.ts"; import * as OrchestratorMcpService from "./OrchestratorMcpService.ts"; @@ -149,6 +150,7 @@ it("readThread prefers activity-run status over a newer cancelled queued run", a getProviders: Effect.succeed([]), } satisfies Partial), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({ list: () => Effect.succeed({ tasks: [] }), } satisfies Partial), @@ -213,6 +215,7 @@ it("readThread prefers waiting activity status over a newer cancelled queued run getProviders: Effect.succeed([]), } satisfies Partial), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({ list: () => Effect.succeed({ tasks: [] }), } satisfies Partial), @@ -325,6 +328,7 @@ it("taskStatus returns task.providerInstanceId rather than the driver kind", asy getProviders: Effect.succeed([]), } satisfies Partial), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({ list: () => Effect.succeed({ tasks: [] }), } satisfies Partial), @@ -447,6 +451,7 @@ it("readThread and sendToThread reach threads in other projects", async () => { getProviders: Effect.succeed([]), } satisfies Partial), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({ list: () => Effect.succeed({ diff --git a/apps/server/src/mcp/OrchestratorMcpService.test.ts b/apps/server/src/mcp/OrchestratorMcpService.test.ts index a10730200da3..d48aa125bfd2 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.test.ts @@ -24,6 +24,7 @@ import * as ProviderRegistry from "../provider/Services/ProviderRegistry.ts"; import { buildUnavailableProviderSnapshot } from "../provider/unavailableProviderSnapshot.ts"; import * as ProjectService from "../project/ProjectService.ts"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import type { McpInvocationScope } from "./McpInvocationContext.ts"; import * as OrchestratorMcpService from "./OrchestratorMcpService.ts"; @@ -120,6 +121,7 @@ describe("OrchestratorMcpService", () => { list: () => Effect.succeed([]), }), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); const scope: McpInvocationScope = { @@ -209,6 +211,7 @@ describe("OrchestratorMcpService", () => { list: () => Effect.succeed([]), }), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); const scope: McpInvocationScope = { @@ -290,6 +293,7 @@ describe("OrchestratorMcpService", () => { list: () => Effect.succeed([]), }), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); const scope: McpInvocationScope = { @@ -363,6 +367,7 @@ describe("OrchestratorMcpService", () => { list: () => Effect.succeed([]), }), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); const scope: McpInvocationScope = { @@ -444,6 +449,7 @@ describe("OrchestratorMcpService", () => { list: () => Effect.succeed([]), }), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); const scope: McpInvocationScope = { @@ -652,6 +658,7 @@ describe("OrchestratorMcpService provider resolution", () => { disabledAntigravityInstanceId, ]), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); @@ -796,6 +803,7 @@ describe("OrchestratorMcpService provider resolution", () => { }), adapterRegistryLayer([codexInstanceId, antigravityInstanceId]), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); @@ -890,6 +898,7 @@ describe("OrchestratorMcpService provider resolution", () => { }), adapterRegistryLayer([codexInstanceId, antigravityInstanceId]), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); @@ -937,6 +946,7 @@ describe("OrchestratorMcpService provider resolution", () => { ]), adapterRegistryLayer([codexInstanceId]), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); @@ -1200,6 +1210,7 @@ describe("OrchestratorMcpService provider resolution", () => { ]), adapterRegistryLayer([codexInstanceId, codexAltInstanceId]), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ); diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 37f00dd4da9a..cdec7bcab4b1 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -82,6 +82,7 @@ import { type McpThreadInvocationScope, requireThreadScope, } from "./McpInvocationContext.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; const DEFAULT_WAIT_TIMEOUT_MS = 10 * 60 * 1_000; const MAX_WAIT_TIMEOUT_MS = 60 * 60 * 1_000; @@ -237,13 +238,7 @@ function scheduledTaskSummary(task: ScheduledTask): OrchestratorMcpScheduledTask ...(task.webhook?.url == null ? {} : { webhookUrl: task.webhook.url }), ...(task.webhook === undefined ? {} - : { - webhookSignature: task.webhook.hasSecret - ? "set" - : task.schedule.type === "webhook" && task.schedule.signature !== null - ? "secret_pending" - : "none", - }), + : { webhookSignature: task.webhook.hasSecret ? "set" : "none" }), }; } @@ -831,6 +826,7 @@ const make = Effect.gen(function* () { ), ) : Effect.succeed(project.defaultModelSelection); + const secretRequests = yield* SecretRequests.SecretRequests; const requireCapability = (scope: McpInvocationScope) => scope.capabilities.has("orchestration") @@ -1541,20 +1537,14 @@ const make = Effect.gen(function* () { }), requestSecret: (scope, input) => Effect.gen(function* () { - yield* requireCapability(scope); - const parent = yield* loadProjection(scope.threadId); - const task = yield* loadScopedScheduledTask(parent.thread.projectId, input.scheduledTaskId); - if (task.schedule.type !== "webhook" || task.schedule.signature === null) { - return yield* failure( - "invalid_request", - "Only a webhook task with a signature check takes a signing secret. Set schedule.signature (with allowPendingSecret: true) first.", - ); - } + // The card is shown in, and answered from, the caller's own thread. + const { scope: threadScope, parent } = yield* loadThreadCaller(scope, "request_secret"); + const threadId = threadScope.thread.threadId; const run = ThreadManagementService.latestActiveRun(parent); if ( run === undefined || run.rootNodeId === null || - run.providerInstanceId !== scope.providerInstanceId + run.providerInstanceId !== threadScope.thread.providerInstanceId ) { return yield* failure( "parent_not_active", @@ -1574,13 +1564,13 @@ const make = Effect.gen(function* () { requestKey: key, operation: `secret-${secretStatus}`, }), - threadId: scope.threadId, + threadId: threadId, runId, nodeId, turnItemId, label: input.label, - reason: input.reason ?? "", - target: { kind: "scheduled_task_webhook_signature", scheduledTaskId: task.id }, + reason: input.reason, + ...(input.placeholder === undefined ? {} : { placeholder: input.placeholder }), secretStatus, }) .pipe( @@ -1593,12 +1583,12 @@ const make = Effect.gen(function* () { ); yield* record("pending"); - // The card is answered by the user (scheduledTasks.answerSecretRequest) - // or ends with the run; poll it like a delegated task. + // The user answers the card (secrets.answerRequest), or it ends with + // the run; poll it like a delegated task. const answered = yield* Effect.gen(function* () { while (true) { const projection = yield* threadManagement - .getThreadRecords(scope.threadId, ["runs", "turnItems"], { + .getThreadRecords(threadId, ["runs", "turnItems"], { turnItemTypes: ["secret_request"], messageRoles: [], }) @@ -1631,10 +1621,13 @@ const make = Effect.gen(function* () { ), ), ); - return { - scheduledTaskId: task.id, - status: Option.getOrElse(answered, () => "pending" as const), - }; + const status = Option.getOrElse(answered, () => "pending" as const); + if (status !== "saved") return { status }; + const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); + return Option.match(secretRef, { + onNone: () => ({ status }), + onSome: (ref) => ({ status, secretRef: ref }), + }); }), capabilities: (scope) => Effect.gen(function* () { @@ -2314,4 +2307,5 @@ export const layer: Layer.Layer< | ProviderAdapterRegistry.ProviderAdapterRegistryV2 | ScheduledTaskService.ScheduledTaskService | ProjectService.ProjectService + | SecretRequests.SecretRequests > = Layer.effect(OrchestratorMcpService, make); diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index 19313416cb46..4e28e3356eef 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -74,6 +74,8 @@ import { import { makeProviderRegistryLayer } from "../provider/testUtils/providerRegistryMock.ts"; import * as ProjectService from "../project/ProjectService.ts"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; +import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as McpHttpServer from "./McpHttpServer.ts"; import * as McpInvocationContext from "./McpInvocationContext.ts"; import { delegatedTaskRun, hasPendingChildRuns } from "./OrchestratorMcpService.ts"; @@ -476,6 +478,18 @@ function scheduledTaskFromUpsert(input: ScheduledTaskUpsertInput): ScheduledTask }; } +/** In-memory server secret store for tests that exercise secret requests. */ +const memorySecretStoreLayer = Layer.sync(ServerSecretStore.ServerSecretStore, () => { + const stored = new Map(); + return ServerSecretStore.ServerSecretStore.of({ + get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))), + set: (name, value) => Effect.sync(() => void stored.set(name, value)), + create: (name, value) => Effect.sync(() => void stored.set(name, value)), + getOrCreateRandom: () => Effect.die("unused in this test"), + remove: (name) => Effect.sync(() => void stored.delete(name)), + }); +}); + const unusedScheduledTaskStubLayer = Layer.succeed( ScheduledTaskService.ScheduledTaskService, ScheduledTaskService.ScheduledTaskService.of({ @@ -486,8 +500,6 @@ const unusedScheduledTaskStubLayer = Layer.succeed( delete: () => Effect.die("ScheduledTaskService.delete is unused in this test"), runNow: () => Effect.die("ScheduledTaskService.runNow is unused in this test"), rotateWebhookToken: () => Effect.die("unused in this test"), - setWebhookSecret: () => Effect.die("unused in this test"), - answerSecretRequest: () => Effect.die("unused in this test"), listWebhookDeliveries: () => Effect.die("unused in this test"), getWebhookDelivery: () => Effect.die("unused in this test"), triggerWebhook: () => Effect.die("unused in this test"), @@ -649,8 +661,6 @@ describe("orchestrator MCP toolkit", () => { ).pipe(Effect.as({ id: input.id })), runNow: () => Effect.die("ScheduledTaskService.runNow is unused in this test"), rotateWebhookToken: () => Effect.die("unused in this test"), - setWebhookSecret: () => Effect.die("unused in this test"), - answerSecretRequest: () => Effect.die("unused in this test"), listWebhookDeliveries: () => Effect.die("unused in this test"), getWebhookDelivery: () => Effect.die("unused in this test"), triggerWebhook: () => Effect.die("unused in this test"), @@ -675,6 +685,12 @@ describe("orchestrator MCP toolkit", () => { ), }), ), + Layer.provideMerge( + SecretRequests.layer.pipe( + Layer.provide(memorySecretStoreLayer), + Layer.provide(orchestrationLayer), + ), + ), Layer.provide(NodeServices.layer), ); @@ -1489,35 +1505,17 @@ describe("orchestrator MCP toolkit", () => { }); expect(yield* Ref.get(scheduledStore)).toHaveLength(0); - // A signed webhook task waits for its secret, which the agent asks - // the user for. The tool only ever learns the answer's status. - const webhookCall = yield* invoke("schedule_task", { - prompt: - "Release {{body.release.tag_name}} was published. Delegate the release notes.", - schedule: { - type: "webhook", - signature: { - header: "x-hub-signature-256", - encoding: "hex", - prefix: "sha256=", - allowPendingSecret: true, - }, - }, - clientRequestId: "schedule-github-releases-1", - }); - expect(webhookCall.isError).toBe(false); - const webhookTaskId = (webhookCall.structuredContent as { scheduledTaskId: string }) - .scheduledTaskId; - // Not linked to T3 Connect here, so there is no public URL to share. - expect(webhookCall.structuredContent).not.toHaveProperty("webhookUrl"); + // The agent asks for a secret; the tool waits for the user and + // returns a one-use ref, never the value, which a signed webhook + // task then consumes. + const secretRequests = yield* SecretRequests.SecretRequests; const secretFiber = yield* invoke("request_secret", { - scheduledTaskId: webhookTaskId, - label: "GitHub webhook signing secret", - reason: "Enter the same secret in the repository's webhook settings.", + label: "GitHub webhook secret", + reason: "Signs release webhooks. Enter the same value in GitHub's webhook settings.", + placeholder: "Paste the webhook secret", }).pipe(Effect.forkChild); - // The card is recorded before the tool starts waiting, so poll for it - // without the helper's short budget: under load the tool's own reads - // come first. + // Polled without the helper's short budget: under load the tool's + // own reads come first. const asked = yield* Effect.gen(function* () { while (true) { const projection = yield* orchestrator.getThreadProjection(parentThreadId); @@ -1532,33 +1530,40 @@ describe("orchestrator MCP toolkit", () => { } }); const card = asked.turnItems.find((item) => item.type === "secret_request"); - if (card?.type !== "secret_request" || card.runId === null || card.nodeId === null) { + if (card?.type !== "secret_request") { return yield* Effect.die(new Error("Secret request card missing.")); } expect(card).toMatchObject({ - label: "GitHub webhook signing secret", - target: { kind: "scheduled_task_webhook_signature", scheduledTaskId: webhookTaskId }, + label: "GitHub webhook secret", + placeholder: "Paste the webhook secret", }); - // What scheduledTasks.answerSecretRequest dispatches once the secret is stored. - yield* orchestrator.dispatch({ - type: "secret_request.record", - commandId: CommandId.make("command:mcp-secret:saved"), + // What the card's Save sends (secrets.answerRequest). + yield* secretRequests.answer({ threadId: parentThreadId, - runId: card.runId, - nodeId: card.nodeId, turnItemId: card.id, - label: card.label, - reason: card.reason, - target: card.target, - secretStatus: "saved", + answer: { type: "save", secret: "github-webhook-secret" }, }); const secretCall = yield* Fiber.join(secretFiber); expect(secretCall.isError).toBe(false); - expect(secretCall.structuredContent).toEqual({ - scheduledTaskId: webhookTaskId, - status: "saved", - }); - yield* invoke("delete_scheduled_task", { scheduledTaskId: webhookTaskId }); + const secretResult = secretCall.structuredContent as { + status: string; + secretRef?: string; + }; + expect(secretResult.status).toBe("saved"); + expect(secretResult.secretRef).toMatch(/^secret-ref:[0-9a-f]{32}$/); + expect(JSON.stringify(secretCall)).not.toContain("github-webhook-secret"); + + // The ref is the secret for exactly one consumer. + expect( + yield* secretRequests.consume({ + ref: secretResult.secretRef as never, + projectId, + }), + ).toBe("github-webhook-secret"); + const reused = yield* secretRequests + .consume({ ref: secretResult.secretRef as never, projectId }) + .pipe(Effect.flip); + expect(reused.message).toContain("already used"); const delegatedCall = yield* invoke("delegate_task", { task: delegatedPrompt, @@ -3658,6 +3663,12 @@ describe("orchestrator MCP toolkit", () => { Layer.provide(providerRegistryLayer), Layer.provide(unusedScheduledTaskStubLayer), Layer.provide(Layer.mock(ProjectService.ProjectService)({})), + Layer.provideMerge( + SecretRequests.layer.pipe( + Layer.provide(memorySecretStoreLayer), + Layer.provide(orchestrationLayer), + ), + ), Layer.provide(NodeServices.layer), ); diff --git a/apps/server/src/mcp/toolkits/core.test.ts b/apps/server/src/mcp/toolkits/core.test.ts index cb26b1db25e6..feacf73e7003 100644 --- a/apps/server/src/mcp/toolkits/core.test.ts +++ b/apps/server/src/mcp/toolkits/core.test.ts @@ -23,6 +23,7 @@ import * as ProviderAdapterRegistry from "../../orchestration-v2/ProviderAdapter import * as ThreadManagement from "../../orchestration-v2/ThreadManagementService.ts"; import * as ProjectService from "../../project/ProjectService.ts"; import * as ProviderRegistry from "../../provider/Services/ProviderRegistry.ts"; +import * as SecretRequests from "../../secrets/SecretRequests.ts"; import * as ScheduledTaskService from "../../scheduledTasks/ScheduledTaskService.ts"; import * as McpHttpServer from "../McpHttpServer.ts"; import * as McpInvocationContext from "../McpInvocationContext.ts"; @@ -386,6 +387,7 @@ it.effect("refuses act-as-caller tools to a client caller", () => Layer.provide(Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({})), Layer.provide(Layer.mock(ScheduledTaskService.ScheduledTaskService)({})), Layer.provide(Layer.mock(ProjectService.ProjectService)({})), + Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), ), ), ), @@ -437,6 +439,7 @@ it.effect("a caller cannot rewrite a scheduled task that runs above its own mode }), ), Layer.provide(Layer.mock(ProjectService.ProjectService)({})), + Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), ), ), ), @@ -505,6 +508,7 @@ it.effect("a caller cannot interrupt a thread that runs above its own modes", () Layer.provide(Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({})), Layer.provide(Layer.mock(ScheduledTaskService.ScheduledTaskService)({})), Layer.provide(Layer.mock(ProjectService.ProjectService)({})), + Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), ), ), ), diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 7c68b84c6e6a..6d4f0366d816 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -99,7 +99,7 @@ const TaskCancelTool = Tool.make("task_cancel", { export const ScheduleTaskTool = Tool.make("schedule_task", { description: - "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). For a sender that signs requests, also set signature, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', allowPendingSecret:true}}, then call request_secret so the user enters the secret privately; never ask for it in chat or invent one. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", + "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). For a sender that signs requests, first call request_secret so the user enters the secret privately (never ask for it in chat or invent one), then set signature with the returned secretRef, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', secretRef}}. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", parameters: OrchestratorMcpScheduleTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, @@ -150,7 +150,7 @@ const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { const RequestSecretTool = Tool.make("request_secret", { description: - "Ask the user for a secret through a private card in this thread, and wait for them to answer. The value is stored by the app and NEVER returned to you or shown in the transcript; the result is only a status (saved, declined, cancelled, or pending if the wait timed out). Use it for a webhook task's signing secret after schedule_task or update_scheduled_task set a signature with allowPendingSecret:true. Tell the user to enter the same secret in the sender (e.g. GitHub's webhook Secret field). Never ask for secrets in chat.", + "Ask the user for a secret (a token, API key, signing secret, password) through a private card in this thread, and wait for them to answer. The value is kept by the app and NEVER returned to you or shown in the transcript. When saved, the result carries a secretRef: pass it to a tool that accepts one (e.g. schedule_task's signature.secretRef). It works once. Never ask for secrets in chat, and never invent one.", parameters: OrchestratorMcpRequestSecretInput, success: OrchestratorMcpRequestSecretResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/mcp/toolkits/worktree/registration.test.ts b/apps/server/src/mcp/toolkits/worktree/registration.test.ts index 452302095eaf..1878fa87cc74 100644 --- a/apps/server/src/mcp/toolkits/worktree/registration.test.ts +++ b/apps/server/src/mcp/toolkits/worktree/registration.test.ts @@ -19,6 +19,7 @@ import * as ProjectService from "../../../project/ProjectService.ts"; import * as ProjectSetupScriptRunner from "../../../project/ProjectSetupScriptRunner.ts"; import * as ProviderRegistry from "../../../provider/Services/ProviderRegistry.ts"; import * as ScheduledTaskService from "../../../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../../../secrets/SecretRequests.ts"; import * as ServerSettings from "../../../serverSettings.ts"; import * as VcsStatusBroadcaster from "../../../vcs/VcsStatusBroadcaster.ts"; import * as McpHttpServer from "../../McpHttpServer.ts"; @@ -33,6 +34,7 @@ const StubServicesLive = Layer.mergeAll( Layer.mock(ProviderRegistry.ProviderRegistry)({}), Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), Layer.mock(ProjectService.ProjectService)({}), ServerSettings.layerTest({}), Layer.mock(GitWorkflowService.GitWorkflowService)({}), diff --git a/apps/server/src/orchestration-v2/ClaudeAutomaticDelivery.integration.test.ts b/apps/server/src/orchestration-v2/ClaudeAutomaticDelivery.integration.test.ts index b13a8b5d90c1..5c8e6e689ef7 100644 --- a/apps/server/src/orchestration-v2/ClaudeAutomaticDelivery.integration.test.ts +++ b/apps/server/src/orchestration-v2/ClaudeAutomaticDelivery.integration.test.ts @@ -24,6 +24,7 @@ import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import * as Scheduler from "../scheduling/Scheduler.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as ClaudeAdapterV2 from "./Adapters/ClaudeAdapterV2.ts"; @@ -315,6 +316,7 @@ it.effect.each(["child completion", "scheduled message", "user steering"] as con }), ), Layer.provide(Layer.mock(ThreadLaunchService.ThreadLaunchService)({})), + Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), Layer.provide( Layer.mergeAll(NodeCrypto.layer, Scheduler.layer, SqlitePersistenceMemory), ), diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 4b6d92f36058..ee55a9aa310f 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -6951,7 +6951,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio type: "secret_request", label: command.label, reason: command.reason, - target: command.target, + ...(command.placeholder === undefined ? {} : { placeholder: command.placeholder }), secretStatus: command.secretStatus, }; yield* emit( diff --git a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts index 9b986f9fe860..82d66edb132f 100644 --- a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts @@ -48,6 +48,7 @@ import * as ManagedProjectFolders from "../project/ManagedProjectFolders.ts"; import { makeProviderRegistryLayer } from "../provider/testUtils/providerRegistryMock.ts"; import * as ServerSettings from "../serverSettings.ts"; import * as ScheduledTasks from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import * as TextGeneration from "../textGeneration/TextGeneration.ts"; import { CodexProviderCapabilitiesV2 } from "./Adapters/CodexAdapterV2.ts"; import * as CommandReceiptStore from "./CommandReceiptStore.ts"; @@ -288,7 +289,14 @@ it.effect.each( ({ target, createdBy }) => { const harness = makeHarness(); const scheduledTasks = ScheduledTasks.layer.pipe( - Layer.provide(Layer.mergeAll(harness.layer, NodeCrypto.layer, Scheduler.layer)), + Layer.provide( + Layer.mergeAll( + harness.layer, + NodeCrypto.layer, + Scheduler.layer, + Layer.mock(SecretRequests.SecretRequests)({}), + ), + ), ); return Effect.gen(function* () { const tasks = yield* ScheduledTasks.ScheduledTaskService; diff --git a/apps/server/src/orchestration-v2/runtimeLayer.ts b/apps/server/src/orchestration-v2/runtimeLayer.ts index 8573c6570795..81807e4821af 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.ts @@ -51,6 +51,7 @@ import { layer as threadLifecycleServiceLayer } from "./ThreadLifecycleService.t import { layer as threadForkServiceLayer } from "./ThreadForkService.ts"; import { layer as turnItemPositionStoreLayer } from "./TurnItemPositionStore.ts"; import { layer as scheduledTaskServiceLayer } from "../scheduledTasks/ScheduledTaskService.ts"; +import { layer as secretRequestsLayer } from "../secrets/SecretRequests.ts"; /** The shared application event log and its command receipts. */ export const OrchestrationEventInfrastructureLayerLive = Layer.mergeAll( @@ -256,8 +257,11 @@ const threadLaunchProvided = threadLaunchServiceLayer.pipe( const threadLifecycleProvided = threadLifecycleServiceLayer.pipe( Layer.provide(threadManagementProvided), ); +const secretRequestsProvided = secretRequestsLayer.pipe(Layer.provide(threadManagementProvided)); const scheduledTaskProvided = scheduledTaskServiceLayer.pipe( - Layer.provide(Layer.mergeAll(threadLaunchProvided, threadManagementProvided)), + Layer.provide( + Layer.mergeAll(threadLaunchProvided, threadManagementProvided, secretRequestsProvided), + ), ); const providerContinuationWorkerProvided = providerContinuationWorkerLive.pipe( Layer.provide( @@ -314,6 +318,7 @@ export const OrchestrationV2ProductionLayerLive = Layer.mergeAll( threadLaunchProvided, threadLifecycleProvided, scheduledTaskProvided, + secretRequestsProvided, UsageLimitRecoveryWorker.workerLive.pipe( Layer.provide(Layer.mergeAll(projectionStoreLayer, threadManagementProvided)), ), diff --git a/apps/server/src/provider/T3OrchestrationInstructions.ts b/apps/server/src/provider/T3OrchestrationInstructions.ts index 03a754ef92b7..edf6093fa12c 100644 --- a/apps/server/src/provider/T3OrchestrationInstructions.ts +++ b/apps/server/src/provider/T3OrchestrationInstructions.ts @@ -10,7 +10,7 @@ The \`t3-code\` MCP server provides app-owned orchestration. Treat these concept - \`t3_thread_launch\` and \`create_threads\` create ordinary top-level T3 conversations. Use them only when the user explicitly asks for separate/new/top-level threads or conversations. Never use them merely because the user said "subagent" or requested parallel delegated work. - For every T3 delegated review round, call \`delegate_task\` again. Include the original brief, prior findings, responses, and unresolved objections in each new task prompt. Track each round by its own \`taskId\`. Use a distinct \`clientRequestId\` per round, stable across retries of that round. Do not use \`t3_thread_send\` on \`childThreadId\` to continue a delegated review. - \`schedule_task\` creates persistent recurring work in the app scheduler. Pass \`schedule\` as a structured object, never as JSON text: \`{"type":"interval","everyMs":3600000}\` for an interval, or \`{"type":"fixed_time","timeOfDay":"09:00","weekdays":[1,2,3,4,5]}\` for a wall-clock schedule, or \`{"type":"webhook"}\` to run on each request to the returned \`webhookUrl\` (the run sees the request only through \`{{body.path}}\`-style placeholders in the prompt). By default runs return to the current thread, which suits orchestrating: each trigger arrives here and you delegate or dedupe; set \`bindToCurrentThread=false\` only when the user wants a fresh thread for every run. After scheduling a timer, report the returned cadence and next run time; for a webhook, report its \`webhookUrl\`, or say T3 Connect remote access is needed if it is missing. -- For a webhook from a sender that signs requests (e.g. GitHub), set \`signature\` with \`allowPendingSecret: true\`, then call \`request_secret\` so the user enters the signing secret privately. Never ask for a secret in chat, never invent one, and never repeat one. +- When you need a secret from the user (a token, API key, or webhook signing secret), call \`request_secret\` so they enter it privately, then pass the returned \`secretRef\` to the tool that needs it, e.g. \`signature.secretRef\` on a webhook task for a sender that signs requests such as GitHub. A \`secretRef\` works once. Never ask for a secret in chat, never invent one, and never repeat one. ### Choose the workspace before starting a new thread diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.schedule.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.schedule.test.ts index c4cd4a816433..b80135c3132a 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.schedule.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.schedule.test.ts @@ -10,6 +10,7 @@ import * as TestClock from "effect/testing/TestClock"; import * as ThreadLaunchService from "../orchestration-v2/ThreadLaunchService.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as ScheduledTaskService from "./ScheduledTaskService.ts"; @@ -22,6 +23,7 @@ it.effect("rejects a stale form save after deletion while preserving explicit-id Scheduler.layer, Layer.mock(ThreadLaunchService.ThreadLaunchService)({}), Layer.mock(ThreadManagementService.ThreadManagementService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), ); yield* Effect.gen(function* () { const service = yield* ScheduledTaskService.ScheduledTaskService; @@ -64,6 +66,7 @@ it.effect("preserves a due run when a save only pads the scheduled hour", () => Scheduler.layer, Layer.mock(ThreadLaunchService.ThreadLaunchService)({}), Layer.mock(ThreadManagementService.ThreadManagementService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), ); yield* Effect.gen(function* () { const service = yield* ScheduledTaskService.ScheduledTaskService; diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.test.ts index 0095d1436c17..e6e27b676141 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.test.ts @@ -17,6 +17,7 @@ import * as TestClock from "effect/testing/TestClock"; import * as ThreadLaunchService from "../orchestration-v2/ThreadLaunchService.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as ScheduledTaskService from "./ScheduledTaskService.ts"; @@ -211,6 +212,7 @@ it.effect( ), }), Layer.mock(ThreadManagementService.ThreadManagementService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), NodeCrypto.layer, Scheduler.layer, ), @@ -316,6 +318,7 @@ it.effect( ), }), Layer.mock(ThreadManagementService.ThreadManagementService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), NodeCrypto.layer, Scheduler.layer, ), diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index 11264a1b5dcd..b211b179d84a 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -14,8 +14,6 @@ import { type ScheduledTaskListWebhookDeliveriesInput, type ScheduledTaskListWebhookDeliveriesResult, type ScheduledTaskRotateWebhookTokenInput, - type ScheduledTaskSetWebhookSecretInput, - type ScheduledTaskAnswerSecretRequestInput, type ScheduledTaskWebhookDeliveryOutcome, type ScheduledTaskListResult, type ScheduledTaskMutationResult, @@ -45,6 +43,7 @@ import * as SqlClient from "effect/sql/SqlClient"; import * as ThreadLaunchService from "../orchestration-v2/ThreadLaunchService.ts"; import * as Metrics from "../observability/Metrics.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import * as Scheduler from "../scheduling/Scheduler.ts"; import { isMissedFixedTimeRun, isSameSchedule, nextScheduledRunAt } from "./Schedule.ts"; import { @@ -240,18 +239,6 @@ export class ScheduledTaskService extends Context.Service< readonly rotateWebhookToken: ( input: ScheduledTaskRotateWebhookTokenInput, ) => Effect.Effect; - /** Sets a signed webhook task's secret without touching anything else. */ - readonly setWebhookSecret: ( - input: ScheduledTaskSetWebhookSecretInput, - ) => Effect.Effect; - /** - * Answers an agent's request for a webhook signing secret: stores the - * secret on its task, then marks the thread's card saved (or declined). - * Only the status reaches the thread. - */ - readonly answerSecretRequest: ( - input: ScheduledTaskAnswerSecretRequestInput, - ) => Effect.Effect; readonly listWebhookDeliveries: ( input: ScheduledTaskListWebhookDeliveriesInput, ) => Effect.Effect; @@ -407,6 +394,7 @@ export const layer = Layer.effect( const crypto = yield* Crypto.Crypto; const threadLaunch = yield* ThreadLaunchService.ThreadLaunchService; const threadManagement = yield* ThreadManagementService.ThreadManagementService; + const secretRequests = yield* SecretRequests.SecretRequests; const scheduler = yield* Scheduler.Scheduler; const readWebhookOrigin = yield* ScheduledTaskWebhookOrigin; // Webhook deliveries for one task dispatch in arrival order rather than @@ -1056,11 +1044,19 @@ export const layer = Layer.effect( const token = existing?.token ?? (yield* newWebhookToken); const signature = input.schedule.type === "webhook" ? input.schedule.signature : null; - const secret = - signature == null ? null : (signature.secret ?? existing?.secret ?? null); + // A secretRef is a value the user entered for an agent; this + // save consumes it, so it cannot be used again. + const fromRef = + signature?.secretRef === undefined + ? undefined + : yield* secretRequests + .consume({ ref: signature.secretRef, projectId: input.projectId }) + .pipe(Effect.mapError((error) => taskError(error.message, { taskId: id }))); + const provided = fromRef ?? signature?.secret; + const secret = signature == null ? null : (provided ?? existing?.secret ?? null); const secretChanged = - signature == null || signature.secret !== undefined || existing === null; - if (signature != null && secret === null && signature.allowPendingSecret !== true) { + signature == null || provided !== undefined || existing === null; + if (signature != null && secret === null) { return yield* taskError("A webhook signature check needs a signing secret.", { taskId: id, }); @@ -1194,75 +1190,6 @@ export const layer = Layer.effect( return { task: yield* loadTask(input.id) }; }); - const setWebhookSecret: ScheduledTaskService["Service"]["setWebhookSecret"] = (input) => - Effect.gen(function* () { - const task = yield* loadTask(input.id); - if (task.schedule.type !== "webhook" || task.schedule.signature === null) { - return yield* taskError("Only a webhook task with a signature check takes a secret.", { - taskId: input.id, - }); - } - const now = yield* localNow; - const updated = yield* sql<{ task_id: string }>` - UPDATE scheduled_tasks - SET webhook_secret = ${input.secret}, updated_at = ${iso(now)} - WHERE task_id = ${input.id} AND created_at = ${task.createdAt} - RETURNING task_id - `.pipe( - Effect.mapError((cause) => - taskError("Could not save the signing secret.", { taskId: input.id, cause }), - ), - ); - if (updated.length === 0) { - return yield* taskError("Schedule task was deleted or replaced.", { taskId: input.id }); - } - yield* notifyChanged; - return { task: yield* loadTask(input.id) }; - }); - - const answerSecretRequest: ScheduledTaskService["Service"]["answerSecretRequest"] = (input) => - Effect.gen(function* () { - const records = yield* threadManagement - .getThreadRecords(input.threadId, ["turnItems"], { - turnItemTypes: ["secret_request"], - messageRoles: [], - }) - .pipe( - Effect.mapError((cause) => taskError("Could not load the secret request.", { cause })), - ); - const item = records.turnItems.find((candidate) => candidate.id === input.turnItemId); - if (item?.type !== "secret_request" || item.runId === null || item.nodeId === null) { - return yield* taskError("This secret request no longer exists."); - } - if (item.secretStatus !== "pending") { - return yield* taskError("This secret request was already answered."); - } - const taskId = item.target.scheduledTaskId; - // Store first: the card only says saved once the task has the secret. - if (input.answer.type === "save") { - yield* setWebhookSecret({ id: taskId, secret: input.answer.secret }); - } - const secretStatus = input.answer.type === "save" ? "saved" : "declined"; - yield* threadManagement - .dispatch({ - type: "secret_request.record", - commandId: CommandId.make(`secret-request:${input.turnItemId}:${secretStatus}`), - threadId: input.threadId, - runId: item.runId, - nodeId: item.nodeId, - turnItemId: item.id, - label: item.label, - reason: item.reason, - target: item.target, - secretStatus, - }) - .pipe( - Effect.mapError((cause) => - taskError("Saved the secret, but could not update the request.", { taskId, cause }), - ), - ); - }); - const deliveryHeaders = (row: WebhookDeliveryRow): Readonly> => Option.getOrElse(decodeHeadersJson(row.headers_json), () => ({})); const decodeDeliverySummary = (row: WebhookDeliveryRow) => ({ @@ -1729,8 +1656,6 @@ export const layer = Layer.effect( delete: deleteTask, runNow, rotateWebhookToken, - setWebhookSecret, - answerSecretRequest, listWebhookDeliveries, getWebhookDelivery, triggerWebhook, diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 35b6853a9c51..4b27ca32e00e 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -2,7 +2,7 @@ import * as NodeCrypto from "node:crypto"; import * as NodePlatformCrypto from "@effect/platform-node/NodeCrypto"; import { assert, it } from "@effect/vitest"; -import { ScheduledTaskUpsertInput } from "@t3tools/contracts"; +import { ScheduledTaskUpsertInput, SecretRequestError } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; @@ -16,12 +16,16 @@ import * as TestClock from "effect/testing/TestClock"; import * as ThreadLaunchService from "../orchestration-v2/ThreadLaunchService.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as Scheduler from "../scheduling/Scheduler.ts"; import * as ScheduledTaskService from "./ScheduledTaskService.ts"; const decodeUpsertInput = Schema.decodeUnknownEffect(ScheduledTaskUpsertInput); +/** Secrets the user entered for an agent, by ref; consuming one removes it. */ +const secretsByRef = new Map(); + type LaunchInput = ThreadLaunchService.ThreadLaunchInput; const webhookTaskInput = (overrides: Record = {}) => @@ -82,6 +86,15 @@ const withService = ( ), }), Layer.mock(ThreadManagementService.ThreadManagementService)({}), + Layer.mock(SecretRequests.SecretRequests)({ + consume: ({ ref }) => { + const value = secretsByRef.get(ref); + secretsByRef.delete(ref); + return value === undefined + ? Effect.fail(new SecretRequestError({ message: "That secretRef was already used." })) + : Effect.succeed(value); + }, + }), Layer.succeed( ScheduledTaskService.ScheduledTaskWebhookOrigin, Effect.succeed({ relayHookBaseUrl: options.relayHookBaseUrl ?? null }), @@ -711,40 +724,20 @@ it.effect("deleting a task removes its delivery log", () => const githubSignature = (secret: string) => `sha256=${NodeCrypto.createHmac("sha256", secret).update(pullRequestBody).digest("hex")}`; -it.effect("a signature waiting for its secret rejects every request until one is set", () => +it.effect("a signature can take the user's secret by ref, which works only once", () => withService(({ service, launches }) => Effect.gen(function* () { + secretsByRef.set("secret-ref:00000000000000000000000000000001", "github-secret"); + const githubSchedule = (secretRef: string) => ({ + type: "webhook", + signature: { header: "x-hub-signature-256", encoding: "hex", prefix: "sha256=", secretRef }, + }); const { task } = yield* service.upsert( yield* webhookTaskInput({ - schedule: { - type: "webhook", - signature: { - header: "x-hub-signature-256", - encoding: "hex", - prefix: "sha256=", - allowPendingSecret: true, - }, - }, + schedule: githubSchedule("secret-ref:00000000000000000000000000000001"), }), ); - assert.isFalse(task.webhook!.hasSecret); - - // Without a secret nothing verifies, however the request is signed. - const early = yield* service.triggerWebhook( - requestFor(task, { - headers: { - "content-type": "application/json", - "x-hub-signature-256": githubSignature(""), - }, - }), - ); - assert.equal(early._tag, "rejected_signature"); - - const { task: withSecret } = yield* service.setWebhookSecret({ - id: task.id, - secret: "github-secret", - }); - assert.isTrue(withSecret.webhook!.hasSecret); + assert.isTrue(task.webhook!.hasSecret); const signed = yield* service.triggerWebhook( requestFor(task, { headers: { @@ -755,11 +748,22 @@ it.effect("a signature waiting for its secret rejects every request until one is ); assert.equal(signed._tag, "accepted"); yield* Queue.take(launches); + + // The ref was consumed by that save. + const reused = yield* service + .upsert( + yield* webhookTaskInput({ + id: "scheduled-task:other", + schedule: githubSchedule("secret-ref:00000000000000000000000000000001"), + }), + ) + .pipe(Effect.flip); + assert.include(reused.message, "already used"); }), ), ); -it.effect("a signature without a secret is still refused unless it may wait for one", () => +it.effect("a signature without any secret is refused", () => withService(({ service }) => Effect.gen(function* () { const failure = yield* service @@ -777,96 +781,6 @@ it.effect("a signature without a secret is still refused unless it may wait for ), ); -it.effect("answering a secret request stores it on the task, and the card only says so", () => - Effect.gen(function* () { - const dispatched: Array = []; - const threadId = "thread-orchestrator"; - const turnItemId = "turn-item:secret-request:1"; - const pendingItem = (scheduledTaskId: string, secretStatus: string) => ({ - id: turnItemId, - threadId, - runId: "run-1", - nodeId: "node-root", - type: "secret_request", - label: "GitHub webhook signing secret", - reason: "Enter the same secret in GitHub's webhook settings.", - target: { kind: "scheduled_task_webhook_signature", scheduledTaskId }, - secretStatus, - }); - let status = "pending"; - let taskId = ""; - const dependencies = Layer.mergeAll( - NodePlatformCrypto.layer, - Scheduler.layer, - Layer.mock(ThreadLaunchService.ThreadLaunchService)({}), - Layer.mock(ThreadManagementService.ThreadManagementService)({ - getThreadRecords: () => - Effect.succeed({ turnItems: [pendingItem(taskId, status)] } as never), - dispatch: (command) => - Effect.sync(() => { - dispatched.push(command); - if (command.type === "secret_request.record") status = command.secretStatus; - return {} as never; - }), - }), - Layer.succeed( - ScheduledTaskService.ScheduledTaskWebhookOrigin, - Effect.succeed({ relayHookBaseUrl: null }), - ), - ); - yield* Effect.gen(function* () { - const service = yield* ScheduledTaskService.ScheduledTaskService; - const { task } = yield* service.upsert( - yield* webhookTaskInput({ - schedule: { - type: "webhook", - signature: { - header: "x-hub-signature-256", - encoding: "hex", - prefix: "sha256=", - allowPendingSecret: true, - }, - }, - }), - ); - taskId = task.id; - - yield* service.answerSecretRequest({ - threadId: threadId as never, - turnItemId: turnItemId as never, - answer: { type: "save", secret: "github-secret" }, - }); - const accepted = yield* service.triggerWebhook( - requestFor(task, { - headers: { - "content-type": "application/json", - "x-hub-signature-256": githubSignature("github-secret"), - }, - }), - ); - assert.equal(accepted._tag, "accepted"); - // The thread learns only the status; the value is nowhere in what it records. - assert.equal(dispatched.length, 1); - assert.include(dispatched[0] as object, { - type: "secret_request.record", - secretStatus: "saved", - }); - assert.notInclude(Object.values(dispatched[0] as object).map(String), "github-secret"); - - // Answered once: a second answer, or a late decline, changes nothing. - const again = yield* service - .answerSecretRequest({ - threadId: threadId as never, - turnItemId: turnItemId as never, - answer: { type: "decline" }, - }) - .pipe(Effect.flip); - assert.include(again.message, "already answered"); - assert.equal(dispatched.length, 1); - }).pipe(Effect.provide(ScheduledTaskService.layer.pipe(Layer.provide(dependencies)))); - }).pipe(Effect.provide(SqlitePersistenceMemory)), -); - const signatureFor = (secret: string) => `sha256=${NodeCrypto.createHmac("sha256", secret).update(pullRequestBody).digest("hex")}`; diff --git a/apps/server/src/scheduling/Scheduler.integration.test.ts b/apps/server/src/scheduling/Scheduler.integration.test.ts index 0984cee10874..0a9d192ad53b 100644 --- a/apps/server/src/scheduling/Scheduler.integration.test.ts +++ b/apps/server/src/scheduling/Scheduler.integration.test.ts @@ -24,6 +24,7 @@ import * as ThreadManagementService from "../orchestration-v2/ThreadManagementSe import * as UsageLimitRecoveryWorker from "../orchestration-v2/UsageLimitRecoveryWorker.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as ScheduledTasks from "../scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; import * as ServerSettings from "../serverSettings.ts"; import * as Scheduler from "./Scheduler.ts"; @@ -112,6 +113,7 @@ it.effect.each(["on time", "after restart"])( Layer.mock(ServerSettings.ServerSettingsService)({ getSettings: Effect.succeed(DEFAULT_SERVER_SETTINGS), }), + Layer.mock(SecretRequests.SecretRequests)({}), ); const workers = Layer.mergeAll( ScheduledTasks.layer, diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts new file mode 100644 index 000000000000..34eea9600484 --- /dev/null +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -0,0 +1,137 @@ +import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; +import { assert, it } from "@effect/vitest"; +import { + ProjectId, + type OrchestrationV2ServerCommand, + ThreadId, + TurnItemId, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; + +import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; +import * as SecretRequests from "./SecretRequests.ts"; + +const threadId = ThreadId.make("thread-orchestrator"); +const turnItemId = TurnItemId.make("turn-item:secret-request:1"); +const projectId = ProjectId.make("project-1"); + +/** Runs `body` against the service with an in-memory store and a thread holding one request. */ +const withService = ( + body: (input: { + readonly service: SecretRequests.SecretRequests["Service"]; + readonly stored: Map; + readonly dispatched: Array; + }) => Effect.Effect, +) => + Effect.gen(function* () { + const stored = new Map(); + const dispatched: Array = []; + let secretStatus = "pending"; + const dependencies = Layer.mergeAll( + NodeCrypto.layer, + Layer.succeed( + ServerSecretStore.ServerSecretStore, + ServerSecretStore.ServerSecretStore.of({ + get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))), + set: (name, value) => Effect.sync(() => void stored.set(name, value)), + create: (name, value) => Effect.sync(() => void stored.set(name, value)), + getOrCreateRandom: () => Effect.die("unused"), + remove: (name) => Effect.sync(() => void stored.delete(name)), + }), + ), + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: () => + Effect.succeed({ + thread: { projectId }, + turnItems: [ + { + id: turnItemId, + threadId, + runId: "run-1", + nodeId: "node-root", + type: "secret_request", + label: "GitHub token", + reason: "Used as GH_TOKEN.", + secretStatus, + }, + ], + } as never), + dispatch: (command) => + Effect.sync(() => { + dispatched.push(command); + if (command.type === "secret_request.record") secretStatus = command.secretStatus; + return {} as never; + }), + }), + ); + return yield* Effect.gen(function* () { + const service = yield* SecretRequests.SecretRequests; + return yield* body({ service, stored, dispatched }); + }).pipe(Effect.provide(SecretRequests.layer.pipe(Layer.provide(dependencies)))); + }); + +const valuesOf = (stored: Map) => + Array.from(stored.values(), (bytes) => new TextDecoder().decode(bytes)); + +it.effect("a saved answer becomes a one-use ref, and the thread only learns it was saved", () => + withService(({ service, stored, dispatched }) => + Effect.gen(function* () { + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + assert.equal(dispatched.length, 1); + assert.include(dispatched[0] as object, { + type: "secret_request.record", + secretStatus: "saved", + }); + assert.notInclude(Object.values(dispatched[0] as object).map(String), "ghp_secret"); + + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); + // Used once: the value is gone from the store and the ref fails. + assert.isFalse(valuesOf(stored).some((value) => value.includes("ghp_secret"))); + const again = yield* service.consume({ ref, projectId }).pipe(Effect.flip); + assert.include(again.message, "already used"); + }), + ), +); + +it.effect("a ref only works in the project it was entered for", () => + withService(({ service }) => + Effect.gen(function* () { + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const elsewhere = yield* service + .consume({ ref, projectId: ProjectId.make("project-other") }) + .pipe(Effect.flip); + assert.include(elsewhere.message, "does not exist"); + // A failed attempt from another project does not burn the ref. + assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); + }), + ), +); + +it.effect("declining stores nothing, and a request is answered once", () => + withService(({ service, stored, dispatched }) => + Effect.gen(function* () { + yield* service.answer({ threadId, turnItemId, answer: { type: "decline" } }); + assert.equal(stored.size, 0); + assert.isTrue(Option.isNone(yield* service.savedRef({ threadId, turnItemId }))); + const late = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) + .pipe(Effect.flip); + assert.include(late.message, "already answered"); + assert.equal(dispatched.length, 1); + assert.equal(stored.size, 0); + }), + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts new file mode 100644 index 000000000000..185daa59cd9a --- /dev/null +++ b/apps/server/src/secrets/SecretRequests.ts @@ -0,0 +1,155 @@ +/** + * SecretRequests - secrets an agent asks the user for. + * + * The user's answer goes straight to the server's secret store under a + * one-use SecretRef; orchestration only records the request and its status. + * A tool that needs the value takes the ref and consumes it, so the value + * never reaches the transcript, projections, clients, or model context. + * + * @module SecretRequests + */ +import { + CommandId, + SecretRef, + SecretRequestError, + type ProjectId, + type SecretRequestAnswerInput, + type ThreadId, +} from "@t3tools/contracts"; +import * as Context from "effect/Context"; +import * as Crypto from "effect/Crypto"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; + +import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; + +const SECRET_REF_PREFIX = "secret-ref:"; +/** Store name for a ref's value; refs are random hex, so they are safe as names. */ +const storeName = (ref: SecretRef) => `secret-request-${ref.slice(SECRET_REF_PREFIX.length)}`; +const REF_PATTERN = /^secret-ref:[0-9a-f]{32}$/; + +/** A ref's value plus the project it was entered for, stored together. */ +const StoredSecret = Schema.fromJsonString( + Schema.Struct({ projectId: Schema.String, value: Schema.String }), +); +const encodeStored = Schema.encodeEffect(StoredSecret); +const decodeStored = Schema.decodeUnknownOption(StoredSecret); + +const fail = (message: string) => new SecretRequestError({ message }); + +export class SecretRequests extends Context.Service< + SecretRequests, + { + /** + * Answers a pending request in a thread. Saving stores the value under a + * new ref that the requesting tool reads from the request's status. + */ + readonly answer: (input: SecretRequestAnswerInput) => Effect.Effect; + /** The ref minted when this request was saved, for the tool that asked. */ + readonly savedRef: (input: { + readonly threadId: ThreadId; + readonly turnItemId: string; + }) => Effect.Effect>; + /** + * Reads and deletes a ref's value. Fails for unknown or used refs, and + * for refs entered in another project. + */ + readonly consume: (input: { + readonly ref: SecretRef; + readonly projectId: ProjectId; + }) => Effect.Effect; + } +>()("t3/secrets/SecretRequests") {} + +const make = Effect.gen(function* () { + const store = yield* ServerSecretStore.ServerSecretStore; + const crypto = yield* Crypto.Crypto; + const threadManagement = yield* ThreadManagementService.ThreadManagementService; + + /** Which ref each saved request minted; the store holds the value itself. */ + const refForRequest = (threadId: ThreadId, turnItemId: string) => + `secret-request-ref-${Buffer.from(`${threadId}\u0000${turnItemId}`).toString("base64url")}`; + + const newRef = crypto.randomBytes(16).pipe( + Effect.map((bytes) => + SecretRef.make(`${SECRET_REF_PREFIX}${Buffer.from(bytes).toString("hex")}`), + ), + Effect.orDie, + ); + + const answer: SecretRequests["Service"]["answer"] = (input) => + Effect.gen(function* () { + const records = yield* threadManagement + .getThreadRecords(input.threadId, ["turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe(Effect.mapError(() => fail("Could not load the secret request."))); + const item = records.turnItems.find((candidate) => candidate.id === input.turnItemId); + if (item?.type !== "secret_request" || item.runId === null || item.nodeId === null) { + return yield* fail("This secret request no longer exists."); + } + if (item.secretStatus !== "pending") { + return yield* fail("This secret request was already answered."); + } + // Store first: the card only says saved once the value is kept. + if (input.answer.type === "save") { + const ref = yield* newRef; + const encoded = yield* encodeStored({ + projectId: records.thread.projectId, + value: input.answer.secret, + }).pipe(Effect.orDie); + yield* Effect.all([ + store.set(storeName(ref), new TextEncoder().encode(encoded)), + store.set(refForRequest(input.threadId, item.id), new TextEncoder().encode(ref)), + ]).pipe(Effect.mapError(() => fail("Could not store the secret."))); + } + const secretStatus = input.answer.type === "save" ? "saved" : "declined"; + yield* threadManagement + .dispatch({ + type: "secret_request.record", + commandId: CommandId.make(`secret-request:${item.id}:${secretStatus}`), + threadId: input.threadId, + runId: item.runId, + nodeId: item.nodeId, + turnItemId: item.id, + label: item.label, + reason: item.reason, + ...(item.placeholder === undefined ? {} : { placeholder: item.placeholder }), + secretStatus, + }) + .pipe(Effect.mapError(() => fail("Saved the secret, but could not update the request."))); + }); + + const savedRef: SecretRequests["Service"]["savedRef"] = (input) => + store.get(refForRequest(input.threadId, input.turnItemId)).pipe( + Effect.map(Option.map((bytes) => SecretRef.make(new TextDecoder().decode(bytes)))), + Effect.orElseSucceed(() => Option.none()), + ); + + const consume: SecretRequests["Service"]["consume"] = (input) => + Effect.gen(function* () { + if (!REF_PATTERN.test(input.ref)) return yield* fail("That secretRef is not valid."); + const stored = yield* store + .get(storeName(input.ref)) + .pipe(Effect.mapError(() => fail("Could not read the secret."))); + const decoded = Option.flatMap(stored, (bytes) => + decodeStored(new TextDecoder().decode(bytes)), + ); + if (Option.isNone(decoded) || decoded.value.projectId !== input.projectId) { + return yield* fail( + "That secretRef was already used or does not exist. Ask the user again with request_secret.", + ); + } + // One use: the value moves into whatever consumed it. + yield* store.remove(storeName(input.ref)).pipe(Effect.ignore); + return decoded.value.value; + }); + + return SecretRequests.of({ answer, savedRef, consume }); +}); + +export const layer = Layer.effect(SecretRequests, make); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 520e7defafb3..33a1c1d7f7cc 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -121,6 +121,7 @@ import * as ThreadLaunchService from "./orchestration-v2/ThreadLaunchService.ts" import * as ThreadMessageIntake from "./orchestration-v2/ThreadMessageIntake.ts"; import * as IdAllocator from "./orchestration-v2/IdAllocator.ts"; import * as ScheduledTasks from "./scheduledTasks/ScheduledTaskService.ts"; +import * as SecretRequests from "./secrets/SecretRequests.ts"; import { archivedShellStreamItemFromThreadShell, buildActiveShellSnapshot, @@ -1220,6 +1221,7 @@ const makeWsRpcLayer = ( const threadLaunch = yield* ThreadLaunchService.ThreadLaunchService; const providerSessionManager = yield* ProviderSessionManager.ProviderSessionManagerV2; const scheduledTasks = yield* ScheduledTasks.ScheduledTaskService; + const secretRequests = yield* SecretRequests.SecretRequests; const pullRequests = yield* PullRequestService.PullRequestService; const pullRequestSync = yield* PullRequestSyncReactor.PullRequestSyncReactor; const deviceService = yield* DeviceService.DeviceService; @@ -1810,7 +1812,7 @@ const makeWsRpcLayer = ( observeRpcEffect( ORCHESTRATION_V2_WS_METHODS.dispatchCommand, // Secret request status is only written next to storing the - // secret (scheduledTasks.answerSecretRequest) or by the requesting tool. + // secret (secrets.answerRequest) or by the requesting tool. command.type === "secret_request.record" ? Effect.fail( new OrchestrationV2DispatchCommandError({ @@ -2112,12 +2114,11 @@ const makeWsRpcLayer = ( scheduledTasks.rotateWebhookToken(input), { "rpc.aggregate": "scheduledTasks", "scheduled_task.id": input.id }, ), - [WS_METHODS.scheduledTasksAnswerSecretRequest]: (input) => - observeRpcEffect( - WS_METHODS.scheduledTasksAnswerSecretRequest, - scheduledTasks.answerSecretRequest(input), - { "rpc.aggregate": "scheduledTasks", "orchestration_v2.thread_id": input.threadId }, - ), + [WS_METHODS.secretsAnswerRequest]: (input) => + observeRpcEffect(WS_METHODS.secretsAnswerRequest, secretRequests.answer(input), { + "rpc.aggregate": "secrets", + "orchestration_v2.thread_id": input.threadId, + }), [WS_METHODS.scheduledTasksListWebhookDeliveries]: (input) => observeRpcEffect( WS_METHODS.scheduledTasksListWebhookDeliveries, diff --git a/apps/web/src/components/chat/SecretRequestCard.tsx b/apps/web/src/components/chat/SecretRequestCard.tsx index ffba2c61f0f8..31f5b0fb89e2 100644 --- a/apps/web/src/components/chat/SecretRequestCard.tsx +++ b/apps/web/src/components/chat/SecretRequestCard.tsx @@ -1,4 +1,6 @@ import { + SECRET_REQUEST_DEFAULT_PLACEHOLDER, + SECRET_REQUEST_PRIVACY_NOTE, secretRequestAnswerInput, secretRequestDisplay, secretRequestFailureMessage, @@ -9,7 +11,7 @@ import { squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, OrchestrationV2ProjectedTurnItem } from "@t3tools/contracts"; -import { CheckIcon, LockIcon, MinusIcon } from "lucide-react"; +import { CheckIcon, LockIcon, MinusIcon, ShieldCheckIcon } from "lucide-react"; import { useId, useState, type FormEvent } from "react"; import { serverEnvironment } from "../../state/server"; @@ -59,8 +61,8 @@ function PendingSecretRequestForm(props: { const { item } = props; const inputId = useId(); const errorId = useId(); - const answer = useAtomCommand(serverEnvironment.answerScheduledTaskSecretRequest, { - label: "scheduled task answer secret request", + const answer = useAtomCommand(serverEnvironment.answerSecretRequest, { + label: "answer secret request", // The failure cause holds the request; keep it out of the console. reportFailure: false, reportDefect: false, @@ -93,64 +95,66 @@ function PendingSecretRequestForm(props: { void send({ type: "save", secret }); }; + // Same hierarchy as a chat card: what is asked, why, the field, then the + // promise about where the value goes. return (
-
- -
- - {item.reason.trim() ? ( -

{item.reason}

- ) : null} -

- Stored for this task only. The agent never sees it. -

-
+
+ + {item.reason.trim() ?

{item.reason}

: null}
- setSecret(event.currentTarget.value)} - /> - +
+ {error !== null ? ( + + ) : null} +
+

+ + {SECRET_REQUEST_PRIVACY_NOTE} +

- {error !== null ? ( - - ) : null} ); } diff --git a/packages/client-runtime/src/secretRequest.test.ts b/packages/client-runtime/src/secretRequest.test.ts index 0284dd4bcd37..f176b670405d 100644 --- a/packages/client-runtime/src/secretRequest.test.ts +++ b/packages/client-runtime/src/secretRequest.test.ts @@ -1,4 +1,4 @@ -import { ScheduledTaskError, ThreadId, TurnItemId } from "@t3tools/contracts"; +import { SecretRequestError, ThreadId, TurnItemId } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; import { @@ -66,7 +66,7 @@ describe("secretRequestFailureMessage", () => { it("passes through server errors but hides anything that could echo the payload", () => { expect( secretRequestFailureMessage( - new ScheduledTaskError({ message: "This secret request was already answered." }), + new SecretRequestError({ message: "This secret request was already answered." }), ), ).toBe("This secret request was already answered."); expect(secretRequestFailureMessage(new Error('Expected string, got "whsec_1"'))).toBe( diff --git a/packages/client-runtime/src/secretRequest.ts b/packages/client-runtime/src/secretRequest.ts index 43d646a93bc3..2bee8bcaf2eb 100644 --- a/packages/client-runtime/src/secretRequest.ts +++ b/packages/client-runtime/src/secretRequest.ts @@ -1,13 +1,14 @@ -import type { - OrchestrationV2TurnItem, - ScheduledTaskAnswerSecretRequestInput, -} from "@t3tools/contracts"; +import type { OrchestrationV2TurnItem, SecretRequestAnswerInput } from "@t3tools/contracts"; export type SecretRequestItem = Extract< OrchestrationV2TurnItem, { readonly type: "secret_request" } >; +/** Shown under the field: the one promise the card makes about the value. */ +export const SECRET_REQUEST_PRIVACY_NOTE = "Stored securely, never shown to the agent"; +export const SECRET_REQUEST_DEFAULT_PLACEHOLDER = "Paste the secret"; + /** What a secret request card shows: the form while pending, otherwise a one-line outcome. */ export type SecretRequestDisplay = | { readonly kind: "pending" } @@ -66,7 +67,7 @@ export function secretRequestDisplay( export function secretRequestAnswerInput( item: Pick, answer: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, -): ScheduledTaskAnswerSecretRequestInput | null { +): SecretRequestAnswerInput | null { if (answer.type === "decline") { return { threadId: item.threadId, turnItemId: item.id, answer: { type: "decline" } }; } @@ -76,7 +77,7 @@ export function secretRequestAnswerInput( } /** Failures whose message is written for the user and never echoes the request payload. */ -const USER_FACING_FAILURE_TAGS = new Set(["ScheduledTaskError", "EnvironmentAuthorizationError"]); +const USER_FACING_FAILURE_TAGS = new Set(["SecretRequestError", "EnvironmentAuthorizationError"]); /** * Inline error copy for a failed answer. Only known server errors pass their diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index 1e53b0195815..e55d447de561 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1299,11 +1299,11 @@ export function createServerEnvironmentAtoms( scheduler: configScheduler, concurrency: configConcurrency, }), - // Off the config lane like run-now: answering a card must not queue - // behind settings edits. One answer per card at a time. - answerScheduledTaskSecretRequest: createEnvironmentRpcCommand(runtime, { - label: "environment-data:server:scheduled-task:answer-secret-request", - tag: WS_METHODS.scheduledTasksAnswerSecretRequest, + // Off the config lane: answering a card must not queue behind settings + // edits. One answer per card at a time. + answerSecretRequest: createEnvironmentRpcCommand(runtime, { + label: "environment-data:server:secrets:answer-request", + tag: WS_METHODS.secretsAnswerRequest, concurrency: { mode: "singleFlight", key: ({ environmentId, input }) => `${environmentId}:${input.threadId}:${input.turnItemId}`, diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index 00cb38a572a8..464e4ab5c912 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -349,6 +349,9 @@ export type RuntimeRequestId = typeof RuntimeRequestId.Type; export const RuntimeTaskId = makeEntityId("RuntimeTaskId"); export type RuntimeTaskId = typeof RuntimeTaskId.Type; export const ScheduledTaskId = makeEntityId("ScheduledTaskId"); +/** A one-use handle to a secret the user entered for an agent; the agent never sees the value. */ +export const SecretRef = makeEntityId("SecretRef"); +export type SecretRef = typeof SecretRef.Type; export type ScheduledTaskId = typeof ScheduledTaskId.Type; export const ApprovalRequestId = makeEntityId("ApprovalRequestId"); export type ApprovalRequestId = typeof ApprovalRequestId.Type; diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index 8690bb1b2390..bdc695a4f6ba 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -60,3 +60,4 @@ export * from "./worktreeMcp.ts"; export * from "./resourceTelemetry.ts"; export * from "./rpc.ts"; export * from "./worktreeSetup.ts"; +export * from "./secretRequest.ts"; diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index bf590dd3046a..48813a70e8dc 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -1346,19 +1346,6 @@ export const OrchestrationV2WebSearchResult = Schema.Struct({ }); export type OrchestrationV2WebSearchResult = typeof OrchestrationV2WebSearchResult.Type; -/** - * What a secret an agent asked for is used for. The server stores the value - * for that purpose and never puts it in the transcript, projections, or - * model context; the turn item only ever carries this target and a status. - */ -export const OrchestrationV2SecretRequestTarget = Schema.Union([ - Schema.Struct({ - kind: Schema.Literal("scheduled_task_webhook_signature"), - scheduledTaskId: ScheduledTaskId, - }), -]); -export type OrchestrationV2SecretRequestTarget = typeof OrchestrationV2SecretRequestTarget.Type; - export const OrchestrationV2SecretRequestStatus = Schema.Literals([ "pending", "saved", @@ -1367,11 +1354,15 @@ export const OrchestrationV2SecretRequestStatus = Schema.Literals([ ]); export type OrchestrationV2SecretRequestStatus = typeof OrchestrationV2SecretRequestStatus.Type; +/** + * A secret an agent asked the user for. The value never passes through + * orchestration: the item carries only what was asked and how it was answered. + */ const OrchestrationV2SecretRequestFields = { type: Schema.Literal("secret_request"), label: TrimmedNonEmptyString, reason: Schema.String, - target: OrchestrationV2SecretRequestTarget, + placeholder: Schema.optional(Schema.String), secretStatus: OrchestrationV2SecretRequestStatus, } as const; @@ -3062,7 +3053,7 @@ export const OrchestrationV2Command = Schema.Union([ turnItemId: TurnItemId, label: TrimmedNonEmptyString, reason: Schema.String, - target: OrchestrationV2SecretRequestTarget, + placeholder: Schema.optional(Schema.String), secretStatus: OrchestrationV2SecretRequestStatus, }), Schema.Struct({ diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 42024a066e47..3ed192d3a5c4 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -12,6 +12,7 @@ import { ProjectId, RunId, ScheduledTaskId, + SecretRef, ThreadId, TrimmedNonEmptyString, TurnItemId, @@ -546,9 +547,8 @@ export const OrchestratorMcpScheduledTask = Schema.Struct({ description: "Public URL to give the sender. Absent when this environment has no T3 Connect managed tunnel; the user must enable T3 Connect remote access first.", }), - webhookSignature: Schema.optional(Schema.Literals(["none", "secret_pending", "set"])).annotate({ - description: - "Signature check state: none, secret_pending (call request_secret; requests are rejected until it is set), or set.", + webhookSignature: Schema.optional(Schema.Literals(["none", "set"])).annotate({ + description: "Whether requests must carry a valid signature.", }), }); export type OrchestratorMcpScheduledTask = typeof OrchestratorMcpScheduledTask.Type; @@ -585,15 +585,15 @@ export type OrchestratorMcpUpdateScheduledTaskInput = typeof OrchestratorMcpUpdateScheduledTaskInput.Type; export const OrchestratorMcpRequestSecretInput = Schema.Struct({ - scheduledTaskId: ScheduledTaskId.annotate({ - description: - "Webhook task (with a signature check) whose signing secret the user should provide.", - }), label: TrimmedNonEmptyString.annotate({ - description: "Short name shown on the card, e.g. 'GitHub webhook signing secret'.", + description: "What you need, shown as the card's title, e.g. 'GitHub webhook secret'.", + }), + reason: TrimmedNonEmptyString.annotate({ + description: + "One or two sentences on what it is for and where the user gets or also enters it.", }), - reason: Schema.optional(Schema.String).annotate({ - description: "One sentence on what the secret is for and where the user will also enter it.", + placeholder: Schema.optional(TrimmedNonEmptyString).annotate({ + description: "Hint inside the input, e.g. 'Paste your GitHub token'.", }), timeoutMs: Schema.optional( Schema.Int.check(Schema.isBetween({ minimum: 1_000, maximum: 60 * 60 * 1_000 })), @@ -602,10 +602,13 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; export const OrchestratorMcpRequestSecretResult = Schema.Struct({ - scheduledTaskId: ScheduledTaskId, status: Schema.Literals(["saved", "declined", "cancelled", "pending"]).annotate({ description: - "saved: the secret is stored and the task verifies requests with it. declined: the user chose not to. cancelled: the request ended with the run. pending: the wait timed out and the card is still open.", + "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. pending: the wait timed out and the card is still open.", + }), + secretRef: Schema.optional(SecretRef).annotate({ + description: + "Present when saved. Pass it to a tool that accepts a secretRef; it works once, and you never see the value.", }), }); export type OrchestratorMcpRequestSecretResult = typeof OrchestratorMcpRequestSecretResult.Type; diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 9fbc2ed95327..74cfd8e9abc7 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -311,7 +311,6 @@ import { ScheduledTaskListResult, ScheduledTaskRunNowInput, ScheduledTaskRotateWebhookTokenInput, - ScheduledTaskAnswerSecretRequestInput, ScheduledTaskListWebhookDeliveriesInput, ScheduledTaskListWebhookDeliveriesResult, ScheduledTaskGetWebhookDeliveryInput, @@ -321,6 +320,7 @@ import { ScheduledTaskUpsertInput, ScheduledTaskMutationResult, } from "./scheduledTask.ts"; +import { SecretRequestAnswerInput, SecretRequestError } from "./secretRequest.ts"; import { ProjectCloneActionInput, ProjectCloneActionResult, @@ -481,7 +481,7 @@ export const WS_METHODS = { scheduledTasksDelete: "scheduledTasks.delete", scheduledTasksRunNow: "scheduledTasks.runNow", scheduledTasksRotateWebhookToken: "scheduledTasks.rotateWebhookToken", - scheduledTasksAnswerSecretRequest: "scheduledTasks.answerSecretRequest", + secretsAnswerRequest: "secrets.answerRequest", scheduledTasksListWebhookDeliveries: "scheduledTasks.listWebhookDeliveries", scheduledTasksGetWebhookDelivery: "scheduledTasks.getWebhookDelivery", @@ -1687,13 +1687,10 @@ const WsScheduledTasksRotateWebhookTokenRpc = Rpc.make( }, ); -const WsScheduledTasksAnswerSecretRequestRpc = Rpc.make( - WS_METHODS.scheduledTasksAnswerSecretRequest, - { - payload: ScheduledTaskAnswerSecretRequestInput, - error: Schema.Union([ScheduledTaskError, EnvironmentAuthorizationError]), - }, -); +const WsSecretsAnswerRequestRpc = Rpc.make(WS_METHODS.secretsAnswerRequest, { + payload: SecretRequestAnswerInput, + error: Schema.Union([SecretRequestError, EnvironmentAuthorizationError]), +}); const WsScheduledTasksListWebhookDeliveriesRpc = Rpc.make( WS_METHODS.scheduledTasksListWebhookDeliveries, @@ -1799,7 +1796,7 @@ export const WsRpcGroup = RpcGroup.make( WsScheduledTasksDeleteRpc, WsScheduledTasksRunNowRpc, WsScheduledTasksRotateWebhookTokenRpc, - WsScheduledTasksAnswerSecretRequestRpc, + WsSecretsAnswerRequestRpc, WsScheduledTasksListWebhookDeliveriesRpc, WsScheduledTasksGetWebhookDeliveryRpc, WsServerReportClientActivityRpc, diff --git a/packages/contracts/src/scheduledTask.ts b/packages/contracts/src/scheduledTask.ts index 9409559c4d36..126551071278 100644 --- a/packages/contracts/src/scheduledTask.ts +++ b/packages/contracts/src/scheduledTask.ts @@ -6,8 +6,8 @@ import { IsoDateTime, ProjectId, ScheduledTaskId, + SecretRef, ThreadId, - TurnItemId, TrimmedNonEmptyString, } from "./baseSchemas.ts"; import { ModelSelection } from "./modelSelection.ts"; @@ -105,12 +105,11 @@ const ScheduledTaskUpsertWebhookSchedule = Schema.Struct({ Schema.Struct({ ...ScheduledTaskWebhookSignatureFields, secret: Schema.optional(TrimmedNonEmptyString).annotate({ - description: - "Shared signing secret. Omit to keep the stored secret; a new task without one rejects every request until a secret is set.", + description: "Shared signing secret. Omit to keep the stored secret.", }), - allowPendingSecret: Schema.optional(Schema.Boolean).annotate({ + secretRef: Schema.optional(SecretRef).annotate({ description: - "Save the check without a secret yet; requests are rejected until one is provided. For agents that ask the user for the secret afterwards.", + "A secret the user entered through request_secret, used instead of secret. It is consumed by this save.", }), }), ), @@ -249,28 +248,6 @@ export const ScheduledTaskRotateWebhookTokenInput = Schema.Struct({ }); export type ScheduledTaskRotateWebhookTokenInput = typeof ScheduledTaskRotateWebhookTokenInput.Type; -/** Sets only a webhook task's signing secret, leaving the rest of the task as it is. */ -export const ScheduledTaskSetWebhookSecretInput = Schema.Struct({ - id: ScheduledTaskId, - secret: TrimmedNonEmptyString, -}); -export type ScheduledTaskSetWebhookSecretInput = typeof ScheduledTaskSetWebhookSecretInput.Type; - -/** - * The user's answer to an agent's request for a webhook signing secret. The - * secret goes straight to the task; the thread only learns it was saved. - */ -export const ScheduledTaskAnswerSecretRequestInput = Schema.Struct({ - threadId: ThreadId, - turnItemId: TurnItemId, - answer: Schema.Union([ - Schema.Struct({ type: Schema.Literal("save"), secret: TrimmedNonEmptyString }), - Schema.Struct({ type: Schema.Literal("decline") }), - ]), -}); -export type ScheduledTaskAnswerSecretRequestInput = - typeof ScheduledTaskAnswerSecretRequestInput.Type; - export const ScheduledTaskWebhookDeliveryId = TrimmedNonEmptyString.pipe( Schema.brand("ScheduledTaskWebhookDeliveryId"), ); diff --git a/packages/contracts/src/secretRequest.ts b/packages/contracts/src/secretRequest.ts new file mode 100644 index 000000000000..e843b24f79c9 --- /dev/null +++ b/packages/contracts/src/secretRequest.ts @@ -0,0 +1,22 @@ +import * as Schema from "effect/Schema"; + +import { ThreadId, TrimmedNonEmptyString, TurnItemId } from "./baseSchemas.ts"; + +/** + * The user's answer to an agent's request for a secret. A saved value is kept + * by the server under a one-use SecretRef; the thread learns only the status. + */ +export const SecretRequestAnswerInput = Schema.Struct({ + threadId: ThreadId, + turnItemId: TurnItemId, + answer: Schema.Union([ + Schema.Struct({ type: Schema.Literal("save"), secret: TrimmedNonEmptyString }), + Schema.Struct({ type: Schema.Literal("decline") }), + ]), +}); +export type SecretRequestAnswerInput = typeof SecretRequestAnswerInput.Type; + +export class SecretRequestError extends Schema.TaggedError()( + "SecretRequestError", + { message: Schema.String }, +) {} From a3a21d67711c73f346fe0d5c5e6249c3b5450245 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 23:03:25 -0700 Subject: [PATCH 08/22] feat(server): secret requests can be monitored Spans on request_secret, answering and consuming a ref, carrying only statuses. t3_secret_requests_total counts how each request ended (saved, declined, cancelled, timed_out) and t3_secret_refs_consumed_total counts refs used or rejected. A test checks that no span records the value. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.ts | 8 +++- apps/server/src/observability/Metrics.ts | 10 +++++ .../server/src/secrets/SecretRequests.test.ts | 45 +++++++++++++++++++ apps/server/src/secrets/SecretRequests.ts | 16 ++++++- docs/operations/observability.md | 4 ++ 5 files changed, 81 insertions(+), 2 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index cdec7bcab4b1..b10cd8bdebd5 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -82,6 +82,7 @@ import { type McpThreadInvocationScope, requireThreadScope, } from "./McpInvocationContext.ts"; +import * as Metrics from "../observability/Metrics.ts"; import * as SecretRequests from "../secrets/SecretRequests.ts"; const DEFAULT_WAIT_TIMEOUT_MS = 10 * 60 * 1_000; @@ -1622,13 +1623,18 @@ const make = Effect.gen(function* () { ), ); const status = Option.getOrElse(answered, () => "pending" as const); + yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); + yield* Metrics.increment(Metrics.secretRequestsTotal, { + status: status === "pending" ? "timed_out" : status, + }); if (status !== "saved") return { status }; const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); return Option.match(secretRef, { onNone: () => ({ status }), onSome: (ref) => ({ status, secretRef: ref }), }); - }), + }).pipe(Effect.withSpan("OrchestratorMcpService.requestSecret")), + capabilities: (scope) => Effect.gen(function* () { const { parent, limits } = yield* loadCaller(scope); diff --git a/apps/server/src/observability/Metrics.ts b/apps/server/src/observability/Metrics.ts index f133eda3f8b0..db36e09b2bff 100644 --- a/apps/server/src/observability/Metrics.ts +++ b/apps/server/src/observability/Metrics.ts @@ -94,6 +94,16 @@ export const webhookRunsTotal = Metric.counter("t3_webhook_runs_total", { description: "Runs started from webhook deliveries, by outcome.", }); +/** Secrets agents asked users for, by how each ended: saved, declined, cancelled, timed_out. */ +export const secretRequestsTotal = Metric.counter("t3_secret_requests_total", { + description: "Secrets agents asked users for, by how each request ended.", +}); + +/** One-use secret refs a tool tried to use, by result: used, rejected. */ +export const secretRefsConsumedTotal = Metric.counter("t3_secret_refs_consumed_total", { + description: "Secret refs tools tried to use, by result.", +}); + export const metricAttributes = ( attributes: Readonly>, ): ReadonlyArray<[string, string]> => Object.entries(compactMetricAttributes(attributes)); diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 34eea9600484..c83675545d25 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -8,6 +8,8 @@ import { } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Metric from "effect/Metric"; +import * as Tracer from "effect/Tracer"; import * as Option from "effect/Option"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; @@ -135,3 +137,46 @@ it.effect("declining stores nothing, and a request is answered once", () => }), ), ); + +it.effect("traces and counts a saved answer without ever recording the value", () => + Effect.gen(function* () { + const spans: Array = []; + const tracer = Tracer.make({ + span: (options) => { + const span = new Tracer.NativeSpan(options); + spans.push(span); + return span; + }, + }); + const counted = Metric.snapshot.pipe( + Effect.map((snapshots) => { + const found = snapshots.find( + (snapshot) => + snapshot.id === "t3_secret_refs_consumed_total" && + snapshot.attributes?.result === "used", + ); + return found?.type === "Counter" ? Number(found.state.count) : 0; + }), + ); + const before = yield* counted; + yield* withService(({ service }) => + Effect.gen(function* () { + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + yield* service.consume({ ref, projectId }); + }), + ).pipe(Effect.withTracer(tracer)); + assert.equal((yield* counted) - before, 1); + const recorded = spans.flatMap((span) => [ + span.name, + ...Array.from(span.attributes.values(), String), + ]); + assert.include(recorded, "SecretRequests.answer"); + assert.include(recorded, "SecretRequests.consume"); + assert.isFalse(recorded.some((value) => value.includes("ghp_secret"))); + }), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 185daa59cd9a..b31efbc36a2e 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -24,6 +24,7 @@ import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as Metrics from "../observability/Metrics.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; const SECRET_REF_PREFIX = "secret-ref:"; @@ -82,6 +83,10 @@ const make = Effect.gen(function* () { const answer: SecretRequests["Service"]["answer"] = (input) => Effect.gen(function* () { + yield* Effect.annotateCurrentSpan({ + "orchestration_v2.thread_id": input.threadId, + "secret_request.answer": input.answer.type, + }); const records = yield* threadManagement .getThreadRecords(input.threadId, ["turnItems"], { turnItemTypes: ["secret_request"], @@ -122,7 +127,7 @@ const make = Effect.gen(function* () { secretStatus, }) .pipe(Effect.mapError(() => fail("Saved the secret, but could not update the request."))); - }); + }).pipe(Effect.withSpan("SecretRequests.answer")); const savedRef: SecretRequests["Service"]["savedRef"] = (input) => store.get(refForRequest(input.threadId, input.turnItemId)).pipe( @@ -131,6 +136,15 @@ const make = Effect.gen(function* () { ); const consume: SecretRequests["Service"]["consume"] = (input) => + consumeRef(input).pipe( + Effect.tap(() => Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "used" })), + Effect.tapError(() => + Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "rejected" }), + ), + Effect.withSpan("SecretRequests.consume"), + ); + + const consumeRef = (input: { readonly ref: SecretRef; readonly projectId: ProjectId }) => Effect.gen(function* () { if (!REF_PATTERN.test(input.ref)) return yield* fail("That secretRef is not valid."); const stored = yield* store diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 7a39ac3dd601..3e083c269a7d 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -396,6 +396,10 @@ Webhooks have their own families: deliveries start, which happen after the sender has its answer. - `t3_webhook_held_delay` for how long requests the relay held waited before arriving. +- `t3_secret_requests_total` by `status` (`saved`, `declined`, `cancelled`, `timed_out`) for secrets + agents asked users for, and `t3_secret_refs_consumed_total` by `result` (`used`, `rejected`) for + tools redeeming them. Neither ever carries a value. + `ScheduledTaskService.triggerWebhook` spans carry the same outcome per request, and each run started from a delivery is its own `ScheduledTaskService.runWebhookDelivery` trace. For a request the relay forwarded, the span also goes to the T3 Connect trace export as a child of the relay's From 5e1d2c5d62f56a087d5b1a153bd5fb4397b4ff11 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 23:17:06 -0700 Subject: [PATCH 09/22] fix: secret requests work in every thread and never strand a value From the final review of the secret request flow: - Saving failed in delegated-subagent threads: the ref mapping's file name grew with the thread id and passed the 255-byte limit, leaving the value file behind. Each request now has one ref, derived from its thread and turn item with a server salt, so names are fixed-length and there is no second file. - A save is a create, so two racing answers cannot both store a value. - A request is closed as cancelled when the wait times out or the tool call is interrupted, and an answer is refused once the run that asked has ended, so a value is never saved where no agent will receive it. The tool reports timed_out instead of an open "pending" card. - Values nobody used expire after 24 hours. - secret_request.record is an internal orchestration command, so the client dispatch type no longer carries it and ws.ts needs no special case. - Web: typing or pasting near a pending card no longer falls through to the composer draft, a click on the card focuses its field, and the privacy note describes the field. Mobile announces errors on iOS too. - The integration test checks the tool result without JSON.stringify, which failed typecheck. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/SecretRequestCard.tsx | 6 +- apps/server/src/mcp/OrchestratorMcpService.ts | 27 +++++- ...OrchestratorMcpToolkit.integration.test.ts | 16 +++- .../src/orchestration-v2/Orchestrator.ts | 2 +- .../testkit/OrchestratorScenario.ts | 2 - .../server/src/secrets/SecretRequests.test.ts | 78 ++++++++++++++++-- apps/server/src/secrets/SecretRequests.ts | 82 +++++++++++++------ apps/server/src/ws.ts | 62 ++++++-------- apps/web/src/components/ChatView.tsx | 5 ++ .../src/components/chat/SecretRequestCard.tsx | 15 +++- packages/contracts/src/orchestrationV2.ts | 32 ++++---- packages/contracts/src/orchestratorMcp.ts | 4 +- 12 files changed, 234 insertions(+), 97 deletions(-) diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx index aec7de5444be..b37ff2af51fe 100644 --- a/apps/mobile/src/features/threads/SecretRequestCard.tsx +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -123,7 +123,11 @@ function PendingSecretRequestForm(props: { onSubmitEditing={() => void send({ type: "save", secret })} /> {error !== null ? ( - + {error} ) : null} diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index b10cd8bdebd5..46bc171fb0a4 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1583,6 +1583,9 @@ const make = Effect.gen(function* () { ), ); yield* record("pending"); + // Only this call can hand the agent its ref, so the card must not + // outlive it: a timeout or an aborted call closes it as cancelled. + const closeCard = record("cancelled").pipe(Effect.ignore); // The user answers the card (secrets.answerRequest), or it ends with // the run; poll it like a delegated task. @@ -1621,12 +1624,28 @@ const make = Effect.gen(function* () { Math.min(input.timeoutMs ?? DEFAULT_WAIT_TIMEOUT_MS, MAX_WAIT_TIMEOUT_MS), ), ), + Effect.onInterrupt(() => closeCard), ); - const status = Option.getOrElse(answered, () => "pending" as const); + if (Option.isNone(answered)) yield* closeCard; + // A save that raced the timeout still wins: the card is answered once. + const status = Option.isSome(answered) + ? answered.value + : yield* threadManagement + .getThreadRecords(threadId, ["turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe( + Effect.map((records) => { + const item = records.turnItems.find((candidate) => candidate.id === turnItemId); + return item?.type === "secret_request" && item.secretStatus === "saved" + ? ("saved" as const) + : ("timed_out" as const); + }), + Effect.orElseSucceed(() => "timed_out" as const), + ); yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); - yield* Metrics.increment(Metrics.secretRequestsTotal, { - status: status === "pending" ? "timed_out" : status, - }); + yield* Metrics.increment(Metrics.secretRequestsTotal, { status }); if (status !== "saved") return { status }; const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); return Option.match(secretRef, { diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index 4e28e3356eef..52020297399c 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -485,7 +485,14 @@ const memorySecretStoreLayer = Layer.sync(ServerSecretStore.ServerSecretStore, ( get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))), set: (name, value) => Effect.sync(() => void stored.set(name, value)), create: (name, value) => Effect.sync(() => void stored.set(name, value)), - getOrCreateRandom: () => Effect.die("unused in this test"), + getOrCreateRandom: (name, bytes) => + Effect.sync(() => { + const existing = stored.get(name); + if (existing) return existing; + const value = new Uint8Array(bytes).fill(7); + stored.set(name, value); + return value; + }), remove: (name) => Effect.sync(() => void stored.delete(name)), }); }); @@ -1551,7 +1558,12 @@ describe("orchestrator MCP toolkit", () => { }; expect(secretResult.status).toBe("saved"); expect(secretResult.secretRef).toMatch(/^secret-ref:[0-9a-f]{32}$/); - expect(JSON.stringify(secretCall)).not.toContain("github-webhook-secret"); + // The value appears nowhere in what the agent received. + const received = [ + ...secretCall.content.map((part) => ("text" in part ? part.text : "")), + ...Object.values(secretResult), + ]; + expect(received.some((value) => value.includes("github-webhook-secret"))).toBe(false); // The ref is the secret for exactly one consumer. expect( diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index ee55a9aa310f..c6a603116ad0 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -6898,7 +6898,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio */ const dispatchSecretRequestRecord = Effect.fn("orchestrationV2.dispatch.secretRequestRecord")( function* ( - command: Extract, + command: Extract, events: Ref.Ref>, ) { const projection = yield* projectionStore diff --git a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts index 7f36ee489894..0bb2d5053b4b 100644 --- a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts +++ b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts @@ -183,8 +183,6 @@ function commandThreadIds(command: OrchestrationV2Command): ReadonlyArray( readonly stored: Map; readonly dispatched: Array; }) => Effect.Effect, + options: { readonly threadId?: ThreadId; readonly runStatus?: string } = {}, ) => Effect.gen(function* () { const stored = new Map(); const dispatched: Array = []; let secretStatus = "pending"; + const requestThreadId = options.threadId ?? threadId; const dependencies = Layer.mergeAll( NodeCrypto.layer, Layer.succeed( @@ -39,8 +42,29 @@ const withService = ( ServerSecretStore.ServerSecretStore.of({ get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))), set: (name, value) => Effect.sync(() => void stored.set(name, value)), - create: (name, value) => Effect.sync(() => void stored.set(name, value)), - getOrCreateRandom: () => Effect.die("unused"), + create: (name, value) => + stored.has(name) + ? Effect.fail( + new ServerSecretStore.SecretStorePersistError({ + name, + cause: new PlatformError.PlatformError( + new PlatformError.SystemError({ + _tag: "AlreadyExists", + module: "FileSystem", + method: "open", + }), + ), + } as never), + ) + : Effect.sync(() => void stored.set(name, value)), + getOrCreateRandom: (name, bytes) => + Effect.sync(() => { + const existing = stored.get(name); + if (existing) return existing; + const value = new Uint8Array(bytes).fill(7); + stored.set(name, value); + return value; + }), remove: (name) => Effect.sync(() => void stored.delete(name)), }), ), @@ -48,10 +72,11 @@ const withService = ( getThreadRecords: () => Effect.succeed({ thread: { projectId }, + runs: [{ id: "run-1", status: options.runStatus ?? "running" }], turnItems: [ { id: turnItemId, - threadId, + threadId: requestThreadId, runId: "run-1", nodeId: "node-root", type: "secret_request", @@ -75,8 +100,11 @@ const withService = ( }).pipe(Effect.provide(SecretRequests.layer.pipe(Layer.provide(dependencies)))); }); +/** The secret values in the store, leaving out the server's own salt. */ const valuesOf = (stored: Map) => - Array.from(stored.values(), (bytes) => new TextDecoder().decode(bytes)); + Array.from(stored.entries()) + .filter(([name]) => name !== "secret-request-salt") + .map(([, bytes]) => new TextDecoder().decode(bytes)); it.effect("a saved answer becomes a one-use ref, and the thread only learns it was saved", () => withService(({ service, stored, dispatched }) => @@ -126,14 +154,14 @@ it.effect("declining stores nothing, and a request is answered once", () => withService(({ service, stored, dispatched }) => Effect.gen(function* () { yield* service.answer({ threadId, turnItemId, answer: { type: "decline" } }); - assert.equal(stored.size, 0); + assert.deepEqual(valuesOf(stored), []); assert.isTrue(Option.isNone(yield* service.savedRef({ threadId, turnItemId }))); const late = yield* service .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) .pipe(Effect.flip); assert.include(late.message, "already answered"); assert.equal(dispatched.length, 1); - assert.equal(stored.size, 0); + assert.deepEqual(valuesOf(stored), []); }), ), ); @@ -180,3 +208,41 @@ it.effect("traces and counts a saved answer without ever recording the value", ( assert.isFalse(recorded.some((value) => value.includes("ghp_secret"))); }), ); + +it.effect("works in threads with long ids, such as a delegated subagent's", () => { + const delegatedThreadId = ThreadId.make( + `thread:delegated-task:command%3Amcp%3A${"a".repeat(36)}%3Adelegate-task%3Arelease-notes-v0.2.0-${"b".repeat(40)}`, + ); + return withService( + ({ service, stored }) => + Effect.gen(function* () { + yield* service.answer({ + threadId: delegatedThreadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + // Store names never grow with the thread id, so they fit any filesystem. + assert.isTrue(Array.from(stored.keys()).every((name) => name.length < 100)); + const ref = Option.getOrThrow( + yield* service.savedRef({ threadId: delegatedThreadId, turnItemId }), + ); + assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); + }), + { threadId: delegatedThreadId }, + ); +}); + +it.effect("refuses an answer once the agent that asked has stopped", () => + withService( + ({ service, stored, dispatched }) => + Effect.gen(function* () { + const late = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) + .pipe(Effect.flip); + assert.include(late.message, "has stopped"); + assert.deepEqual(valuesOf(stored), []); + assert.equal(dispatched.length, 0); + }), + { runStatus: "completed" }, + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index b31efbc36a2e..fa50ac041fc9 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -16,8 +16,10 @@ import { type SecretRequestAnswerInput, type ThreadId, } from "@t3tools/contracts"; +import * as NodeCrypto from "node:crypto"; + +import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; -import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -28,13 +30,29 @@ import * as Metrics from "../observability/Metrics.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; const SECRET_REF_PREFIX = "secret-ref:"; -/** Store name for a ref's value; refs are random hex, so they are safe as names. */ +/** Store name for a ref's value; refs are fixed-length hex, so names stay short and safe. */ const storeName = (ref: SecretRef) => `secret-request-${ref.slice(SECRET_REF_PREFIX.length)}`; const REF_PATTERN = /^secret-ref:[0-9a-f]{32}$/; +/** A value nobody used within this long is dropped; the agent can ask again. */ +const SECRET_REF_TTL_MS = 24 * 60 * 60 * 1000; + +/** + * Each request has exactly one ref, derived from where it was asked. Its + * length never depends on the thread id, so the store's file names stay + * within filesystem limits, and the requesting tool finds it without a + * second record. Unguessable without the server's own salt. + */ +const refFor = (salt: string, threadId: ThreadId, turnItemId: string) => + SecretRef.make( + `${SECRET_REF_PREFIX}${NodeCrypto.createHmac("sha256", salt) + .update(`${threadId}\u0000${turnItemId}`) + .digest("hex") + .slice(0, 32)}`, + ); -/** A ref's value plus the project it was entered for, stored together. */ +/** A ref's value, the project it was entered for, and when it was saved. */ const StoredSecret = Schema.fromJsonString( - Schema.Struct({ projectId: Schema.String, value: Schema.String }), + Schema.Struct({ projectId: Schema.String, value: Schema.String, savedAt: Schema.Number }), ); const encodeStored = Schema.encodeEffect(StoredSecret); const decodeStored = Schema.decodeUnknownOption(StoredSecret); @@ -67,19 +85,11 @@ export class SecretRequests extends Context.Service< const make = Effect.gen(function* () { const store = yield* ServerSecretStore.ServerSecretStore; - const crypto = yield* Crypto.Crypto; const threadManagement = yield* ThreadManagementService.ThreadManagementService; - /** Which ref each saved request minted; the store holds the value itself. */ - const refForRequest = (threadId: ThreadId, turnItemId: string) => - `secret-request-ref-${Buffer.from(`${threadId}\u0000${turnItemId}`).toString("base64url")}`; - - const newRef = crypto.randomBytes(16).pipe( - Effect.map((bytes) => - SecretRef.make(`${SECRET_REF_PREFIX}${Buffer.from(bytes).toString("hex")}`), - ), - Effect.orDie, - ); + const salt = Buffer.from( + yield* store.getOrCreateRandom("secret-request-salt", 32).pipe(Effect.orDie), + ).toString("hex"); const answer: SecretRequests["Service"]["answer"] = (input) => Effect.gen(function* () { @@ -88,7 +98,7 @@ const make = Effect.gen(function* () { "secret_request.answer": input.answer.type, }); const records = yield* threadManagement - .getThreadRecords(input.threadId, ["turnItems"], { + .getThreadRecords(input.threadId, ["runs", "turnItems"], { turnItemTypes: ["secret_request"], messageRoles: [], }) @@ -100,17 +110,32 @@ const make = Effect.gen(function* () { if (item.secretStatus !== "pending") { return yield* fail("This secret request was already answered."); } - // Store first: the card only says saved once the value is kept. + // The agent is waiting inside the run that asked; once it has ended, + // nobody will ever receive the ref, so a value saved now would be lost. + const run = records.runs.find((candidate) => candidate.id === item.runId); + if (run === undefined || ThreadManagementService.isTerminalRunStatus(run.status)) { + return yield* fail("The agent that asked has stopped, so this secret can't be used."); + } + // Store first: the card only says saved once the value is kept. Create, + // not set: a second answer racing this one must not replace the value. if (input.answer.type === "save") { - const ref = yield* newRef; const encoded = yield* encodeStored({ projectId: records.thread.projectId, value: input.answer.secret, + savedAt: yield* Clock.currentTimeMillis, }).pipe(Effect.orDie); - yield* Effect.all([ - store.set(storeName(ref), new TextEncoder().encode(encoded)), - store.set(refForRequest(input.threadId, item.id), new TextEncoder().encode(ref)), - ]).pipe(Effect.mapError(() => fail("Could not store the secret."))); + yield* store + .create( + storeName(refFor(salt, input.threadId, item.id)), + new TextEncoder().encode(encoded), + ) + .pipe( + Effect.mapError((error) => + ServerSecretStore.isSecretAlreadyExistsError(error) + ? fail("This secret request was already answered.") + : fail("Could not store the secret."), + ), + ); } const secretStatus = input.answer.type === "save" ? "saved" : "declined"; yield* threadManagement @@ -130,10 +155,11 @@ const make = Effect.gen(function* () { }).pipe(Effect.withSpan("SecretRequests.answer")); const savedRef: SecretRequests["Service"]["savedRef"] = (input) => - store.get(refForRequest(input.threadId, input.turnItemId)).pipe( - Effect.map(Option.map((bytes) => SecretRef.make(new TextDecoder().decode(bytes)))), - Effect.orElseSucceed(() => Option.none()), - ); + Effect.gen(function* () { + const ref = refFor(salt, input.threadId, input.turnItemId); + const stored = yield* store.get(storeName(ref)).pipe(Effect.orElseSucceed(Option.none)); + return Option.map(stored, () => ref); + }); const consume: SecretRequests["Service"]["consume"] = (input) => consumeRef(input).pipe( @@ -158,6 +184,10 @@ const make = Effect.gen(function* () { "That secretRef was already used or does not exist. Ask the user again with request_secret.", ); } + if ((yield* Clock.currentTimeMillis) - decoded.value.savedAt > SECRET_REF_TTL_MS) { + yield* store.remove(storeName(input.ref)).pipe(Effect.ignore); + return yield* fail("That secretRef expired. Ask the user again with request_secret."); + } // One use: the value moves into whatever consumed it. yield* store.remove(storeName(input.ref)).pipe(Effect.ignore); return decoded.value.value; diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 33a1c1d7f7cc..c049ab24d32a 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1811,44 +1811,34 @@ const makeWsRpcLayer = ( [ORCHESTRATION_V2_WS_METHODS.dispatchCommand]: (command) => observeRpcEffect( ORCHESTRATION_V2_WS_METHODS.dispatchCommand, - // Secret request status is only written next to storing the - // secret (secrets.answerRequest) or by the requesting tool. - command.type === "secret_request.record" - ? Effect.fail( - new OrchestrationV2DispatchCommandError({ + startup + .enqueueCommand( + // A retry also restarts the preparation work the launch owns. + (command.type === "prepared-run.retry" + ? threadLaunch.retryPreparation(command) + : ThreadMessageIntake.dispatchCommand( + ThreadManagementService.withCreationProvenance(command, { + createdBy: "user", + creationSource: + "creationSource" in command ? command.creationSource : "web", + }), + ) + ).pipe(Effect.provide(intakeContext)), + ) + .pipe( + Effect.tap(() => recordClientCommandAnalytics(command)), + Effect.map((result) => ({ sequence: result.sequence })), + Effect.mapError((cause) => { + const detail = userFacingDispatchErrorMessage(cause); + return new OrchestrationV2DispatchCommandError({ commandId: command.commandId, commandType: command.type, - message: "Secret requests are answered through their own request.", - }), - ) - : startup - .enqueueCommand( - // A retry also restarts the preparation work the launch owns. - (command.type === "prepared-run.retry" - ? threadLaunch.retryPreparation(command) - : ThreadMessageIntake.dispatchCommand( - ThreadManagementService.withCreationProvenance(command, { - createdBy: "user", - creationSource: - "creationSource" in command ? command.creationSource : "web", - }), - ) - ).pipe(Effect.provide(intakeContext)), - ) - .pipe( - Effect.tap(() => recordClientCommandAnalytics(command)), - Effect.map((result) => ({ sequence: result.sequence })), - Effect.mapError((cause) => { - const detail = userFacingDispatchErrorMessage(cause); - return new OrchestrationV2DispatchCommandError({ - commandId: command.commandId, - commandType: command.type, - message: detail ?? "Failed to dispatch orchestration V2 command", - ...(detail === undefined ? {} : { detail }), - cause, - }); - }), - ), + message: detail ?? "Failed to dispatch orchestration V2 command", + ...(detail === undefined ? {} : { detail }), + cause, + }); + }), + ), { "rpc.aggregate": "orchestrationV2", "orchestration_v2.command_id": command.commandId, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 58d7618b76b2..656b989e8595 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -740,6 +740,8 @@ function eventPathContainsSelector(event: Event, selector: string): boolean { return path.some((target) => target instanceof Element && target.closest(selector)); } +const SECRET_REQUEST_SELECTOR = '[data-v2-item-type="secret_request"]'; + /** * Whether input that landed outside any editable or interactive element * should be redirected into the composer. Shared by type-to-focus and @@ -747,6 +749,9 @@ function eventPathContainsSelector(event: Event, selector: string): boolean { */ function shouldRedirectInputToComposer(event: Event): boolean { if (event.defaultPrevented) return false; + // Near a pending secret request, input is meant for its private field: it + // must never land in the composer draft, which is persisted and sent. + if (eventPathContainsSelector(event, SECRET_REQUEST_SELECTOR)) return false; if (eventPathContainsSelector(event, TYPE_TO_FOCUS_EDITABLE_SELECTOR)) return false; if (eventPathContainsSelector(event, TYPE_TO_FOCUS_INTERACTIVE_SELECTOR)) return false; if (document.querySelector(TYPE_TO_FOCUS_FLOATING_LAYER_SELECTOR)) return false; diff --git a/apps/web/src/components/chat/SecretRequestCard.tsx b/apps/web/src/components/chat/SecretRequestCard.tsx index 31f5b0fb89e2..90355f30663c 100644 --- a/apps/web/src/components/chat/SecretRequestCard.tsx +++ b/apps/web/src/components/chat/SecretRequestCard.tsx @@ -61,6 +61,7 @@ function PendingSecretRequestForm(props: { const { item } = props; const inputId = useId(); const errorId = useId(); + const privacyId = useId(); const answer = useAtomCommand(serverEnvironment.answerSecretRequest, { label: "answer secret request", // The failure cause holds the request; keep it out of the console. @@ -103,6 +104,13 @@ function PendingSecretRequestForm(props: { className="flex min-w-0 flex-col gap-3 rounded-xl border border-border/60 bg-card p-4" onSubmit={onSubmit} autoComplete="off" + // A click anywhere on the card targets its field, so a paste that + // follows lands here rather than with the composer. + onPointerDown={(event) => { + if (!(event.target instanceof HTMLElement)) return; + if (event.target.closest("input, button, a")) return; + document.getElementById(inputId)?.focus(); + }} >
@@ -141,7 +149,10 @@ function PendingSecretRequestForm(props: {

) : null}
-

+

{SECRET_REQUEST_PRIVACY_NOTE}

diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 48813a70e8dc..c26d5a9327e4 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -3041,21 +3041,7 @@ export const OrchestrationV2Command = Schema.Union([ targetThreadId: ThreadId, targetRunId: Schema.NullOr(RunId), }), - // Server-only: written by the T3 MCP secret request tool and the secret - // RPC, never by a client dispatch (which would let a client mark a request - // saved without storing anything). - Schema.Struct({ - type: Schema.Literal("secret_request.record"), - commandId: CommandId, - threadId: ThreadId, - runId: RunId, - nodeId: NodeId, - turnItemId: TurnItemId, - label: TrimmedNonEmptyString, - reason: Schema.String, - placeholder: Schema.optional(Schema.String), - secretStatus: OrchestrationV2SecretRequestStatus, - }), + Schema.Struct({ type: Schema.Literal("provider.switch"), commandId: CommandId, @@ -3124,6 +3110,22 @@ const OrchestrationV2InternalCommand = Schema.Union([ threadId: ThreadId, reason: Schema.optional(Schema.String), }), + /** + * Records or updates a secret an agent asked the user for. Internal so no + * client can mark a request saved without the value being stored. + */ + Schema.Struct({ + type: Schema.Literal("secret_request.record"), + commandId: CommandId, + threadId: ThreadId, + runId: RunId, + nodeId: NodeId, + turnItemId: TurnItemId, + label: TrimmedNonEmptyString, + reason: Schema.String, + placeholder: Schema.optional(Schema.String), + secretStatus: OrchestrationV2SecretRequestStatus, + }), ]); export type OrchestrationV2InternalCommand = typeof OrchestrationV2InternalCommand.Type; diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 3ed192d3a5c4..752f13afdc02 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -602,9 +602,9 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; export const OrchestratorMcpRequestSecretResult = Schema.Struct({ - status: Schema.Literals(["saved", "declined", "cancelled", "pending"]).annotate({ + status: Schema.Literals(["saved", "declined", "cancelled", "timed_out"]).annotate({ description: - "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. pending: the wait timed out and the card is still open.", + "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. timed_out: the user did not answer in time; the card is closed, so ask again if still needed.", }), secretRef: Schema.optional(SecretRef).annotate({ description: From a11a561326e950f620eaa640d8f03e1df2742385 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 23:42:10 -0700 Subject: [PATCH 10/22] feat(server): a thread waiting on a secret shows as needing input The shell reports a pending secret request the way it reports a question, so the sidebar, inbox, notifications and Live Activity show the agent is blocked on the user instead of working. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...OrchestratorMcpToolkit.integration.test.ts | 12 ++++ .../ProjectionSettlement.test.ts | 4 ++ .../src/orchestration-v2/ProjectionStore.ts | 64 +++++++++++++++++-- 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index 52020297399c..7b52c3fe468e 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -51,6 +51,7 @@ import { McpSchema, McpServer } from "effect/ai"; import { ClaudeProviderCapabilitiesV2 } from "../orchestration-v2/Adapters/ClaudeAdapterV2.ts"; import { CodexProviderCapabilitiesV2 } from "../orchestration-v2/Adapters/CodexAdapterV2.ts"; import { CodexOrchestratorReplayHarness } from "../orchestration-v2/Adapters/CodexAdapterV2.testkit.ts"; +import { threadShellFromProjection } from "../orchestration-v2/ProjectionStore.ts"; import * as EventSink from "../orchestration-v2/EventSink.ts"; import * as Orchestrator from "../orchestration-v2/Orchestrator.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; @@ -1544,6 +1545,14 @@ describe("orchestrator MCP toolkit", () => { label: "GitHub webhook secret", placeholder: "Paste the webhook secret", }); + // The agent is blocked on the user, so the thread asks for input + // like a question does, in both shell paths. + expect( + (yield* orchestrator.getThreadShell(parentThreadId))?.pendingRuntimeRequest, + ).toMatchObject({ kind: "user_input" }); + expect(threadShellFromProjection(asked).pendingRuntimeRequest).toMatchObject({ + kind: "user_input", + }); // What the card's Save sends (secrets.answerRequest). yield* secretRequests.answer({ threadId: parentThreadId, @@ -1557,6 +1566,9 @@ describe("orchestrator MCP toolkit", () => { secretRef?: string; }; expect(secretResult.status).toBe("saved"); + expect( + (yield* orchestrator.getThreadShell(parentThreadId))?.pendingRuntimeRequest ?? null, + ).toBeNull(); expect(secretResult.secretRef).toMatch(/^secret-ref:[0-9a-f]{32}$/); // The value appears nowhere in what the agent received. const received = [ diff --git a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts index 2a76341b42e1..e82e1fc4099a 100644 --- a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts @@ -521,5 +521,9 @@ it.effect("shell failure lookups stay on the thread's own turn items", () => const itemLookups = plan.filter((row) => row.detail.startsWith("SEARCH item ")); assert.lengthOf(itemLookups, 2); assert.isTrue(itemLookups.every((row) => row.detail.includes("turn_items_thread_run_idx"))); + // The pending secret request lookup is bounded the same way. + const secretLookups = plan.filter((row) => row.detail.startsWith("SEARCH secret ")); + assert.lengthOf(secretLookups, 1); + assert.include(secretLookups[0]!.detail, "turn_items_thread_run_idx"); }).pipe(Effect.provide(SqlLayer)), ); diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 93288a512ac7..a700f557cfbf 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -28,7 +28,6 @@ import type { ProviderThreadId, ProviderTurnId, RunAttemptId, - RuntimeRequestId, MessageId, } from "@t3tools/contracts"; import { @@ -50,6 +49,7 @@ import { OrchestrationV2TurnItemJson as OrchestrationV2TurnItemJsonSchema, orchestrationV2RunWorkStartedAt, RunId, + RuntimeRequestId, CheckpointScopeId, ThreadId, TurnItemId, @@ -919,6 +919,7 @@ type ShellThreadRow = { readonly blocking_run_completed_at: string | null; readonly blocking_failure_payload_json: string | null; readonly pending_request_payload_json: string | null; + readonly pending_secret_request_payload_json: string | null; readonly latest_user_message_at: string | null; readonly latest_user_authored_message_at: string | null; readonly has_actionable_proposed_plan: number; @@ -1303,6 +1304,29 @@ function buildVisibleTurnItems(input: { ]); } +/** + * An agent waiting on a secret is waiting on the user just like a question, + * so the shell reports it as pending user input. Secret requests have no + * runtime request of their own; this stands one in for the shell summary + * only, keyed by the card's turn item. + */ +function secretRequestAsPendingInput( + item: OrchestrationV2TurnItem | null, +): OrchestrationV2ThreadProjection["runtimeRequests"][number] | null { + if (item?.type !== "secret_request" || item.nodeId === null) return null; + return { + id: RuntimeRequestId.make(item.id), + nodeId: item.nodeId, + providerTurnId: item.providerTurnId, + nativeRequestRef: null, + kind: "user_input", + status: "pending", + responseCapability: { type: "message" }, + createdAt: item.startedAt ?? item.updatedAt, + resolvedAt: null, + }; +} + export function threadShellFromProjection( projection: OrchestrationV2ThreadProjection, ): OrchestrationV2ThreadShell { @@ -1327,13 +1351,28 @@ export function threadShellFromProjection( projection.runs .filter(isActivityRunForShell) .toSorted((left, right) => right.ordinal - left.ordinal)[0] ?? null; + const liveRunIds = new Set(projection.runs.filter(isActivityRunForShell).map((run) => run.id)); const pendingRuntimeRequest = projection.runtimeRequests .filter((request) => request.status === "pending") .toSorted( (left, right) => DateTime.toEpochMillis(right.createdAt) - DateTime.toEpochMillis(left.createdAt), - )[0] ?? null; + )[0] ?? + secretRequestAsPendingInput( + projection.turnItems + .filter( + (item) => + item.type === "secret_request" && + item.status === "waiting" && + item.runId !== null && + liveRunIds.has(item.runId), + ) + .toSorted( + (left, right) => + DateTime.toEpochMillis(right.updatedAt) - DateTime.toEpochMillis(left.updatedAt), + )[0] ?? null, + ); const userMessages = projection.messages .filter((message) => message.role === "user") .toSorted( @@ -4958,6 +4997,17 @@ export const layer: Layer.Layer = ORDER BY request.created_at DESC, request.runtime_request_id DESC LIMIT 1 ) AS pending_request_payload_json, + ( + SELECT secret.payload_json + FROM orchestration_v2_projection_turn_items secret + INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx + INNER JOIN orchestration_v2_projection_runs r ON r.run_id = secret.run_id + WHERE secret.thread_id = t.thread_id + AND secret.type = 'secret_request' AND secret.status = 'waiting' + AND r.status IN ('preparing', 'starting', 'running', 'waiting') + ORDER BY secret.updated_at DESC, secret.turn_item_id DESC + LIMIT 1 + ) AS pending_secret_request_payload_json, ( SELECT message.updated_at FROM orchestration_v2_projection_messages message @@ -5329,9 +5379,13 @@ export const layer: Layer.Layer = } = input; const thread = yield* decodeThreadPayload(row.payload_json); const pendingRuntimeRequest = - row.pending_request_payload_json === null - ? null - : yield* decodeRuntimeRequestPayload(row.pending_request_payload_json); + row.pending_request_payload_json !== null + ? yield* decodeRuntimeRequestPayload(row.pending_request_payload_json) + : row.pending_secret_request_payload_json !== null + ? secretRequestAsPendingInput( + yield* decodeTurnItemPayload(row.pending_secret_request_payload_json), + ) + : null; let terminalFailureItem = row.terminal_failure_payload_json === null ? null From 54bddc02c2b55f51169dbd8971b6251f451d5577 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 4 Oct 2026 23:49:35 -0700 Subject: [PATCH 11/22] fix: secret requests survive retries and stay out of password managers request_secret takes a clientRequestId, so a retried call returns the same card and ref. A retried schedule_task whose secretRef was already used by the first save keeps the secret that save stored. The web field is masked text rather than a password field, so browsers do not offer to save it, and mobile's Decline is quiet like the web card's. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/SecretRequestCard.tsx | 15 +++++--- apps/server/src/mcp/OrchestratorMcpService.ts | 8 +++-- ...OrchestratorMcpToolkit.integration.test.ts | 15 ++++++++ .../scheduledTasks/ScheduledTaskService.ts | 12 +++++-- .../ScheduledTaskService.webhook.test.ts | 35 +++++++++++++++++++ .../src/components/chat/SecretRequestCard.tsx | 5 ++- packages/contracts/src/orchestratorMcp.ts | 4 +++ 7 files changed, 84 insertions(+), 10 deletions(-) diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx index b37ff2af51fe..3999f53d35ff 100644 --- a/apps/mobile/src/features/threads/SecretRequestCard.tsx +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -12,7 +12,7 @@ import { } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, OrchestrationV2ProjectedTurnItem } from "@t3tools/contracts"; import { useState } from "react"; -import { View, type ColorValue } from "react-native"; +import { Pressable, View, type ColorValue } from "react-native"; import { SymbolView, type AppSymbolName } from "../../components/AppSymbol"; import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText"; @@ -148,12 +148,17 @@ function PendingSecretRequestForm(props: { {SECRET_REQUEST_PRIVACY_NOTE} - void send({ type: "decline" })} - /> + > + Decline + ); diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 46bc171fb0a4..4ce391d19a2f 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1554,8 +1554,12 @@ const make = Effect.gen(function* () { } const runId = run.id; const nodeId = run.rootNodeId; - const key = yield* requestKey(undefined); - const turnItemId = TurnItemId.make(`turn-item:secret-request:${stablePart(key)}`); + const key = yield* requestKey(input.clientRequestId); + // Turn item ids are global; scope the key to this thread. A retry with + // the same clientRequestId finds this card, answered or not. + const turnItemId = TurnItemId.make( + `turn-item:secret-request:${stablePart(threadId)}:${stablePart(key)}`, + ); const record = (secretStatus: "pending" | "cancelled") => threadManagement .dispatch({ diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index 7b52c3fe468e..b63827f87425 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -1521,6 +1521,7 @@ describe("orchestrator MCP toolkit", () => { label: "GitHub webhook secret", reason: "Signs release webhooks. Enter the same value in GitHub's webhook settings.", placeholder: "Paste the webhook secret", + clientRequestId: "release-webhook-secret", }).pipe(Effect.forkChild); // Polled without the helper's short budget: under load the tool's // own reads come first. @@ -1577,6 +1578,20 @@ describe("orchestrator MCP toolkit", () => { ]; expect(received.some((value) => value.includes("github-webhook-secret"))).toBe(false); + // A retry that lost the first result gets the same answer, with no + // second card for the user. + const retried = yield* invoke("request_secret", { + label: "GitHub webhook secret", + reason: "Signs release webhooks. Enter the same value in GitHub's webhook settings.", + clientRequestId: "release-webhook-secret", + }); + expect(retried.structuredContent).toEqual(secretResult); + expect( + (yield* orchestrator.getThreadProjection(parentThreadId)).turnItems.filter( + (item) => item.type === "secret_request", + ), + ).toHaveLength(1); + // The ref is the secret for exactly one consumer. expect( yield* secretRequests.consume({ diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index b211b179d84a..eb3cc91297ae 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -1045,13 +1045,21 @@ export const layer = Layer.effect( const signature = input.schedule.type === "webhook" ? input.schedule.signature : null; // A secretRef is a value the user entered for an agent; this - // save consumes it, so it cannot be used again. + // save consumes it, so it cannot be used again. A replay of a + // save that already used it keeps the secret that save stored. + const replay = input.commandId !== undefined && existing?.secret != null; const fromRef = signature?.secretRef === undefined ? undefined : yield* secretRequests .consume({ ref: signature.secretRef, projectId: input.projectId }) - .pipe(Effect.mapError((error) => taskError(error.message, { taskId: id }))); + .pipe( + Effect.catch((error) => + replay + ? Effect.succeed(undefined) + : Effect.fail(taskError(error.message, { taskId: id })), + ), + ); const provided = fromRef ?? signature?.secret; const secret = signature == null ? null : (provided ?? existing?.secret ?? null); const secretChanged = diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 4b27ca32e00e..8a21699c71c1 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -763,6 +763,41 @@ it.effect("a signature can take the user's secret by ref, which works only once" ), ); +it.effect("a retried save with an already used secretRef keeps the stored secret", () => + withService(({ service, launches }) => + Effect.gen(function* () { + secretsByRef.set("secret-ref:00000000000000000000000000000002", "github-secret"); + const save = webhookTaskInput({ + id: undefined, + commandId: "command:mcp:schedule-task:release-hook", + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + secretRef: "secret-ref:00000000000000000000000000000002", + }, + }, + }); + const first = yield* service.upsert(yield* save); + // The agent never saw the first result, so it sends the same call again. + const retried = yield* service.upsert(yield* save); + assert.equal(retried.task.id, first.task.id); + const signed = yield* service.triggerWebhook( + requestFor(retried.task, { + headers: { + "content-type": "application/json", + "x-hub-signature-256": githubSignature("github-secret"), + }, + }), + ); + assert.equal(signed._tag, "accepted"); + yield* Queue.take(launches); + }), + ), +); + it.effect("a signature without any secret is refused", () => withService(({ service }) => Effect.gen(function* () { diff --git a/apps/web/src/components/chat/SecretRequestCard.tsx b/apps/web/src/components/chat/SecretRequestCard.tsx index 90355f30663c..c80d63b2043c 100644 --- a/apps/web/src/components/chat/SecretRequestCard.tsx +++ b/apps/web/src/components/chat/SecretRequestCard.tsx @@ -122,7 +122,10 @@ function PendingSecretRequestForm(props: {
Date: Mon, 5 Oct 2026 00:17:34 -0700 Subject: [PATCH 12/22] fix(server): unused secret values are deleted once they expire A ref nobody consumed kept the user's value on disk until something tried to use it. The service now sweeps expired values at startup and hourly. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../server/src/secrets/SecretRequests.test.ts | 48 +++++++++++++++++++ apps/server/src/secrets/SecretRequests.ts | 38 +++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 2cdd9da7119b..ae849945686f 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -1,4 +1,5 @@ import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; +import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import { ProjectId, @@ -6,14 +7,17 @@ import { ThreadId, TurnItemId, } from "@t3tools/contracts"; +import * as Clock from "effect/Clock"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Metric from "effect/Metric"; +import * as TestClock from "effect/testing/TestClock"; import * as Tracer from "effect/Tracer"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as ServerConfig from "../config.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; import * as SecretRequests from "./SecretRequests.ts"; @@ -37,6 +41,7 @@ const withService = ( const requestThreadId = options.threadId ?? threadId; const dependencies = Layer.mergeAll( NodeCrypto.layer, + NodeServices.layer, Layer.succeed( ServerSecretStore.ServerSecretStore, ServerSecretStore.ServerSecretStore.of({ @@ -246,3 +251,46 @@ it.effect("refuses an answer once the agent that asked has stopped", () => { runStatus: "completed" }, ), ); + +it.effect("drops values nobody used once they expire, and keeps the rest", () => + Effect.gen(function* () { + const store = yield* ServerSecretStore.ServerSecretStore; + const encode = (savedAt: number) => + new TextEncoder().encode( + `{"projectId":"project-1","value":"ghp_secret","savedAt":${savedAt}}`, + ); + yield* TestClock.adjust("2 days"); + const now = yield* Clock.currentTimeMillis; + const stale = `secret-request-${"a".repeat(32)}`; + const fresh = `secret-request-${"b".repeat(32)}`; + yield* store.set(stale, encode(now - 25 * 60 * 60 * 1000)); + yield* store.set(fresh, encode(now)); + yield* store.set("unrelated", new Uint8Array([1])); + + // Building the service runs the first sweep. + yield* Effect.gen(function* () { + yield* SecretRequests.SecretRequests; + }).pipe( + Effect.provide( + SecretRequests.layer.pipe( + Layer.provide(Layer.mock(ThreadManagementService.ThreadManagementService)({})), + ), + ), + Effect.scoped, + ); + + assert.isTrue(Option.isNone(yield* store.get(stale))); + assert.isTrue(Option.isSome(yield* store.get(fresh))); + assert.isTrue(Option.isSome(yield* store.get("unrelated"))); + assert.isTrue(Option.isSome(yield* store.get("secret-request-salt"))); + }).pipe( + Effect.provide( + ServerSecretStore.layer.pipe( + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-secret-requests-" }), + ), + Layer.provideMerge(NodeServices.layer), + ), + ), + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index fa50ac041fc9..3a1d84a89ace 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -21,8 +21,10 @@ import * as NodeCrypto from "node:crypto"; import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; @@ -35,6 +37,7 @@ const storeName = (ref: SecretRef) => `secret-request-${ref.slice(SECRET_REF_PRE const REF_PATTERN = /^secret-ref:[0-9a-f]{32}$/; /** A value nobody used within this long is dropped; the agent can ask again. */ const SECRET_REF_TTL_MS = 24 * 60 * 60 * 1000; +const STORE_NAME_PATTERN = /^(secret-request-[0-9a-f]{32})\.bin$/; /** * Each request has exactly one ref, derived from where it was asked. Its @@ -85,6 +88,7 @@ export class SecretRequests extends Context.Service< const make = Effect.gen(function* () { const store = yield* ServerSecretStore.ServerSecretStore; + const fileSystem = yield* FileSystem.FileSystem; const threadManagement = yield* ThreadManagementService.ThreadManagementService; const salt = Buffer.from( @@ -193,6 +197,40 @@ const make = Effect.gen(function* () { return decoded.value.value; }); + /** + * Drops values nobody used before they expired, so an agent that never + * consumed its ref does not leave the user's secret on disk. + */ + const sweepExpired = Effect.gen(function* () { + if (store.directory === undefined) return; + const now = yield* Clock.currentTimeMillis; + const names = (yield* fileSystem.readDirectory(store.directory)).flatMap((file) => { + const match = STORE_NAME_PATTERN.exec(file); + return match?.[1] === undefined ? [] : [match[1]]; + }); + let removed = 0; + for (const name of names) { + const stored = yield* store.get(name).pipe(Effect.orElseSucceed(Option.none)); + const decoded = Option.flatMap(stored, (bytes) => + decodeStored(new TextDecoder().decode(bytes)), + ); + if (Option.isSome(decoded) && now - decoded.value.savedAt <= SECRET_REF_TTL_MS) continue; + yield* store.remove(name).pipe(Effect.ignore); + removed += 1; + } + yield* Effect.annotateCurrentSpan({ "secret_request.expired_removed": removed }); + }).pipe( + Effect.catchCause((cause) => Effect.logWarning("Could not sweep expired secret refs", cause)), + Effect.withSpan("SecretRequests.sweepExpired"), + ); + // Once at startup, then hourly; a value lingers at most an hour past expiry. + yield* sweepExpired; + yield* sweepExpired.pipe( + Effect.delay("1 hour"), + Effect.repeat(Schedule.spaced("1 hour")), + Effect.forkScoped, + ); + return SecretRequests.of({ answer, savedRef, consume }); }); From 1557c978bc0f838d417bad61a0122ffe18f2e943 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 00:29:03 -0700 Subject: [PATCH 13/22] fix(server): secret request retries and cleanup can't lose or move a secret - A webhook save that names a secretRef never falls back to a plain secret sent alongside it, so a replay keeps the secret it stored. - Recording a secret request again leaves an open card as it was asked, and refuses to move it to another run. - A used or expired value that cannot be deleted is logged; a used one is not handed out, so a ref is never used twice. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...OrchestratorMcpToolkit.integration.test.ts | 17 +++++ .../src/orchestration-v2/Orchestrator.ts | 63 ++++++++++++------- .../scheduledTasks/ScheduledTaskService.ts | 4 +- .../ScheduledTaskService.webhook.test.ts | 20 +++++- .../server/src/secrets/SecretRequests.test.ts | 33 +++++++++- apps/server/src/secrets/SecretRequests.ts | 24 +++++-- 6 files changed, 127 insertions(+), 34 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index b63827f87425..0a1aeed097ab 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -1546,6 +1546,23 @@ describe("orchestrator MCP toolkit", () => { label: "GitHub webhook secret", placeholder: "Paste the webhook secret", }); + // Asking again with the same id leaves the open card exactly as it was. + yield* orchestrator.dispatch({ + type: "secret_request.record", + commandId: CommandId.make("command:test:secret-request-replay"), + threadId: parentThreadId, + runId: card.runId!, + nodeId: card.nodeId!, + turnItemId: card.id, + label: "Something else", + reason: "A different reason.", + secretStatus: "pending", + }); + expect( + (yield* orchestrator.getThreadProjection(parentThreadId)).turnItems.find( + (item) => item.id === card.id, + ), + ).toMatchObject({ label: "GitHub webhook secret", runId: card.runId }); // The agent is blocked on the user, so the thread asks for input // like a question does, in both shell paths. expect( diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index c6a603116ad0..f88ebe79e799 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -6928,32 +6928,47 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: `Turn item ${command.turnItemId} is not a secret request.`, }); } - // A request is answered once; later updates cannot reopen or change it. - if (existing !== undefined && existing.secretStatus !== "pending") return; - + if (existing !== undefined && existing.runId !== command.runId) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: `Secret request ${command.turnItemId} belongs to another run.`, + }); + } const now = yield* DateTime.now; + // A request is answered once, and a retry finds its card as it was + // asked: either one records the card unchanged. + const unchanged = + existing !== undefined && + (existing.secretStatus !== "pending" || command.secretStatus === "pending"); const pending = command.secretStatus === "pending"; - const turnItem: OrchestrationV2TurnItem = { - id: command.turnItemId, - threadId: command.threadId, - runId: command.runId, - nodeId: command.nodeId, - providerThreadId: run.providerThreadId, - providerTurnId: providerTurnForRun(projection, run)?.id ?? null, - nativeItemRef: null, - parentItemId: null, - ordinal: existing?.ordinal ?? (yield* nextTurnItemOrdinal(projection)), - status: pending ? "waiting" : command.secretStatus === "saved" ? "completed" : "cancelled", - title: command.label, - startedAt: existing?.startedAt ?? now, - completedAt: pending ? null : now, - updatedAt: now, - type: "secret_request", - label: command.label, - reason: command.reason, - ...(command.placeholder === undefined ? {} : { placeholder: command.placeholder }), - secretStatus: command.secretStatus, - }; + const turnItem: OrchestrationV2TurnItem = unchanged + ? existing + : { + id: command.turnItemId, + threadId: command.threadId, + runId: command.runId, + nodeId: command.nodeId, + providerThreadId: run.providerThreadId, + providerTurnId: providerTurnForRun(projection, run)?.id ?? null, + nativeItemRef: null, + parentItemId: null, + ordinal: existing?.ordinal ?? (yield* nextTurnItemOrdinal(projection)), + status: pending + ? "waiting" + : command.secretStatus === "saved" + ? "completed" + : "cancelled", + title: command.label, + startedAt: existing?.startedAt ?? now, + completedAt: pending ? null : now, + updatedAt: now, + type: "secret_request", + label: command.label, + reason: command.reason, + ...(command.placeholder === undefined ? {} : { placeholder: command.placeholder }), + secretStatus: command.secretStatus, + }; yield* emit( events, command, diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index eb3cc91297ae..966d4e5ec3d1 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -1060,7 +1060,9 @@ export const layer = Layer.effect( : Effect.fail(taskError(error.message, { taskId: id })), ), ); - const provided = fromRef ?? signature?.secret; + // A ref, when given, is the only source: a plain secret sent + // alongside it must not replace what a replayed save stored. + const provided = signature?.secretRef === undefined ? signature?.secret : fromRef; const secret = signature == null ? null : (provided ?? existing?.secret ?? null); const secretChanged = signature == null || provided !== undefined || existing === null; diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 8a21699c71c1..92a527c793eb 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -781,8 +781,24 @@ it.effect("a retried save with an already used secretRef keeps the stored secret }, }); const first = yield* service.upsert(yield* save); - // The agent never saw the first result, so it sends the same call again. - const retried = yield* service.upsert(yield* save); + // The agent never saw the first result, so it sends the same call again, + // this time with a plain secret alongside the used ref. + const retried = yield* service.upsert( + yield* webhookTaskInput({ + id: undefined, + commandId: "command:mcp:schedule-task:release-hook", + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + secretRef: "secret-ref:00000000000000000000000000000002", + secret: "made-up-secret", + }, + }, + }), + ); assert.equal(retried.task.id, first.task.id); const signed = yield* service.triggerWebhook( requestFor(retried.task, { diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index ae849945686f..b2e2995ce036 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -32,7 +32,11 @@ const withService = ( readonly stored: Map; readonly dispatched: Array; }) => Effect.Effect, - options: { readonly threadId?: ThreadId; readonly runStatus?: string } = {}, + options: { + readonly threadId?: ThreadId; + readonly runStatus?: string; + readonly removeFails?: boolean; + } = {}, ) => Effect.gen(function* () { const stored = new Map(); @@ -70,7 +74,15 @@ const withService = ( stored.set(name, value); return value; }), - remove: (name) => Effect.sync(() => void stored.delete(name)), + remove: (name) => + options.removeFails + ? Effect.fail( + new ServerSecretStore.SecretStorePersistError({ + resource: name, + cause: new Error("read-only"), + }), + ) + : Effect.sync(() => void stored.delete(name)), }), ), Layer.mock(ThreadManagementService.ThreadManagementService)({ @@ -294,3 +306,20 @@ it.effect("drops values nobody used once they expire, and keeps the rest", () => ), ), ); + +it.effect("a used value that cannot be deleted is not handed out", () => + withService( + ({ service }) => + Effect.gen(function* () { + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const failed = yield* service.consume({ ref, projectId }).pipe(Effect.flip); + assert.include(failed.message, "Could not use"); + }), + { removeFails: true }, + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 3a1d84a89ace..203c045870aa 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -165,6 +165,17 @@ const make = Effect.gen(function* () { return Option.map(stored, () => ref); }); + /** Removes a stored value; a failure is logged, since the value is still on disk. */ + const removeLogged = (name: string) => + store.remove(name).pipe( + Effect.as(true), + Effect.catch((error) => + Effect.logWarning("Could not delete a secret request value", { + errorTag: error._tag, + }).pipe(Effect.as(false)), + ), + ); + const consume: SecretRequests["Service"]["consume"] = (input) => consumeRef(input).pipe( Effect.tap(() => Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "used" })), @@ -189,11 +200,15 @@ const make = Effect.gen(function* () { ); } if ((yield* Clock.currentTimeMillis) - decoded.value.savedAt > SECRET_REF_TTL_MS) { - yield* store.remove(storeName(input.ref)).pipe(Effect.ignore); + // The hourly sweep retries a removal that fails here. + yield* removeLogged(storeName(input.ref)); return yield* fail("That secretRef expired. Ask the user again with request_secret."); } - // One use: the value moves into whatever consumed it. - yield* store.remove(storeName(input.ref)).pipe(Effect.ignore); + // One use: the value moves into whatever consumed it. If it cannot be + // deleted, it is not handed out, so a ref is never used twice. + if (!(yield* removeLogged(storeName(input.ref)))) { + return yield* fail("Could not use that secretRef. Try again."); + } return decoded.value.value; }); @@ -215,8 +230,7 @@ const make = Effect.gen(function* () { decodeStored(new TextDecoder().decode(bytes)), ); if (Option.isSome(decoded) && now - decoded.value.savedAt <= SECRET_REF_TTL_MS) continue; - yield* store.remove(name).pipe(Effect.ignore); - removed += 1; + if (yield* removeLogged(name)) removed += 1; } yield* Effect.annotateCurrentSpan({ "secret_request.expired_removed": removed }); }).pipe( From 5db85a25aef4ae1880ba3c6c7ef7a53f7fd78cac Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 09:51:38 -0700 Subject: [PATCH 14/22] fix(server): a decline that races the secret request timeout is reported as declined Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 4ce391d19a2f..026062f805f2 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1631,7 +1631,7 @@ const make = Effect.gen(function* () { Effect.onInterrupt(() => closeCard), ); if (Option.isNone(answered)) yield* closeCard; - // A save that raced the timeout still wins: the card is answered once. + // An answer that raced the timeout still wins: the card is answered once. const status = Option.isSome(answered) ? answered.value : yield* threadManagement @@ -1642,8 +1642,9 @@ const make = Effect.gen(function* () { .pipe( Effect.map((records) => { const item = records.turnItems.find((candidate) => candidate.id === turnItemId); - return item?.type === "secret_request" && item.secretStatus === "saved" - ? ("saved" as const) + return item?.type === "secret_request" && + (item.secretStatus === "saved" || item.secretStatus === "declined") + ? item.secretStatus : ("timed_out" as const); }), Effect.orElseSucceed(() => "timed_out" as const), From 89964e0c11ad1837b2391c76fa8960174d3e4642 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 15:33:33 -0700 Subject: [PATCH 15/22] refactor(server): secret request errors carry a reason instead of free text SecretRequestError now has a fixed reason and keeps the underlying failure as its cause; the user-facing message comes from the reason. The runtime layer imports SecretRequests as a namespace, and the webhook skip is caught with catchTags. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/orchestration-v2/runtimeLayer.ts | 4 +-- .../scheduledTasks/ScheduledTaskService.ts | 11 +++--- .../ScheduledTaskService.webhook.test.ts | 2 +- apps/server/src/secrets/SecretRequests.ts | 30 ++++++++-------- .../client-runtime/src/secretRequest.test.ts | 4 +-- packages/contracts/src/secretRequest.ts | 34 +++++++++++++++++-- 6 files changed, 57 insertions(+), 28 deletions(-) diff --git a/apps/server/src/orchestration-v2/runtimeLayer.ts b/apps/server/src/orchestration-v2/runtimeLayer.ts index 81807e4821af..b4e262d08a70 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.ts @@ -51,7 +51,7 @@ import { layer as threadLifecycleServiceLayer } from "./ThreadLifecycleService.t import { layer as threadForkServiceLayer } from "./ThreadForkService.ts"; import { layer as turnItemPositionStoreLayer } from "./TurnItemPositionStore.ts"; import { layer as scheduledTaskServiceLayer } from "../scheduledTasks/ScheduledTaskService.ts"; -import { layer as secretRequestsLayer } from "../secrets/SecretRequests.ts"; +import * as SecretRequests from "../secrets/SecretRequests.ts"; /** The shared application event log and its command receipts. */ export const OrchestrationEventInfrastructureLayerLive = Layer.mergeAll( @@ -257,7 +257,7 @@ const threadLaunchProvided = threadLaunchServiceLayer.pipe( const threadLifecycleProvided = threadLifecycleServiceLayer.pipe( Layer.provide(threadManagementProvided), ); -const secretRequestsProvided = secretRequestsLayer.pipe(Layer.provide(threadManagementProvided)); +const secretRequestsProvided = SecretRequests.layer.pipe(Layer.provide(threadManagementProvided)); const scheduledTaskProvided = scheduledTaskServiceLayer.pipe( Layer.provide( Layer.mergeAll(threadLaunchProvided, threadManagementProvided, secretRequestsProvided), diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index 966d4e5ec3d1..51403c124a98 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -1628,11 +1628,12 @@ export const layer = Layer.effect( ) : runOutcome("started"), ), - Effect.catchTag("WebhookDeliverySkipped", (skipped) => - runOutcome("skipped").pipe( - Effect.andThen(markDeliveryFailed(deliveryId, skipped.reason)), - ), - ), + Effect.catchTags({ + WebhookDeliverySkipped: (skipped) => + runOutcome("skipped").pipe( + Effect.andThen(markDeliveryFailed(deliveryId, skipped.reason)), + ), + }), // The log is readable over RPC, so it gets a fixed reason; the // cause, which can carry request data, stays in the server log. Effect.catchCause((cause) => diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 92a527c793eb..22166d38ecdd 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -91,7 +91,7 @@ const withService = ( const value = secretsByRef.get(ref); secretsByRef.delete(ref); return value === undefined - ? Effect.fail(new SecretRequestError({ message: "That secretRef was already used." })) + ? Effect.fail(new SecretRequestError({ reason: "ref_unavailable" })) : Effect.succeed(value); }, }), diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 203c045870aa..27dae03287d0 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -12,6 +12,7 @@ import { CommandId, SecretRef, SecretRequestError, + type SecretRequestFailureReason, type ProjectId, type SecretRequestAnswerInput, type ThreadId, @@ -60,7 +61,8 @@ const StoredSecret = Schema.fromJsonString( const encodeStored = Schema.encodeEffect(StoredSecret); const decodeStored = Schema.decodeUnknownOption(StoredSecret); -const fail = (message: string) => new SecretRequestError({ message }); +const fail = (reason: SecretRequestFailureReason, cause?: unknown) => + new SecretRequestError({ reason, ...(cause === undefined ? {} : { cause }) }); export class SecretRequests extends Context.Service< SecretRequests, @@ -106,19 +108,19 @@ const make = Effect.gen(function* () { turnItemTypes: ["secret_request"], messageRoles: [], }) - .pipe(Effect.mapError(() => fail("Could not load the secret request."))); + .pipe(Effect.mapError((cause) => fail("load_failed", cause))); const item = records.turnItems.find((candidate) => candidate.id === input.turnItemId); if (item?.type !== "secret_request" || item.runId === null || item.nodeId === null) { - return yield* fail("This secret request no longer exists."); + return yield* fail("not_found"); } if (item.secretStatus !== "pending") { - return yield* fail("This secret request was already answered."); + return yield* fail("already_answered"); } // The agent is waiting inside the run that asked; once it has ended, // nobody will ever receive the ref, so a value saved now would be lost. const run = records.runs.find((candidate) => candidate.id === item.runId); if (run === undefined || ThreadManagementService.isTerminalRunStatus(run.status)) { - return yield* fail("The agent that asked has stopped, so this secret can't be used."); + return yield* fail("agent_stopped"); } // Store first: the card only says saved once the value is kept. Create, // not set: a second answer racing this one must not replace the value. @@ -136,8 +138,8 @@ const make = Effect.gen(function* () { .pipe( Effect.mapError((error) => ServerSecretStore.isSecretAlreadyExistsError(error) - ? fail("This secret request was already answered.") - : fail("Could not store the secret."), + ? fail("already_answered", error) + : fail("store_failed", error), ), ); } @@ -155,7 +157,7 @@ const make = Effect.gen(function* () { ...(item.placeholder === undefined ? {} : { placeholder: item.placeholder }), secretStatus, }) - .pipe(Effect.mapError(() => fail("Saved the secret, but could not update the request."))); + .pipe(Effect.mapError((cause) => fail("record_failed", cause))); }).pipe(Effect.withSpan("SecretRequests.answer")); const savedRef: SecretRequests["Service"]["savedRef"] = (input) => @@ -187,27 +189,25 @@ const make = Effect.gen(function* () { const consumeRef = (input: { readonly ref: SecretRef; readonly projectId: ProjectId }) => Effect.gen(function* () { - if (!REF_PATTERN.test(input.ref)) return yield* fail("That secretRef is not valid."); + if (!REF_PATTERN.test(input.ref)) return yield* fail("invalid_ref"); const stored = yield* store .get(storeName(input.ref)) - .pipe(Effect.mapError(() => fail("Could not read the secret."))); + .pipe(Effect.mapError((cause) => fail("read_failed", cause))); const decoded = Option.flatMap(stored, (bytes) => decodeStored(new TextDecoder().decode(bytes)), ); if (Option.isNone(decoded) || decoded.value.projectId !== input.projectId) { - return yield* fail( - "That secretRef was already used or does not exist. Ask the user again with request_secret.", - ); + return yield* fail("ref_unavailable"); } if ((yield* Clock.currentTimeMillis) - decoded.value.savedAt > SECRET_REF_TTL_MS) { // The hourly sweep retries a removal that fails here. yield* removeLogged(storeName(input.ref)); - return yield* fail("That secretRef expired. Ask the user again with request_secret."); + return yield* fail("ref_expired"); } // One use: the value moves into whatever consumed it. If it cannot be // deleted, it is not handed out, so a ref is never used twice. if (!(yield* removeLogged(storeName(input.ref)))) { - return yield* fail("Could not use that secretRef. Try again."); + return yield* fail("consume_failed"); } return decoded.value.value; }); diff --git a/packages/client-runtime/src/secretRequest.test.ts b/packages/client-runtime/src/secretRequest.test.ts index f176b670405d..3f8fe48885ca 100644 --- a/packages/client-runtime/src/secretRequest.test.ts +++ b/packages/client-runtime/src/secretRequest.test.ts @@ -65,9 +65,7 @@ describe("secretRequestAnswerInput", () => { describe("secretRequestFailureMessage", () => { it("passes through server errors but hides anything that could echo the payload", () => { expect( - secretRequestFailureMessage( - new SecretRequestError({ message: "This secret request was already answered." }), - ), + secretRequestFailureMessage(new SecretRequestError({ reason: "already_answered" })), ).toBe("This secret request was already answered."); expect(secretRequestFailureMessage(new Error('Expected string, got "whsec_1"'))).toBe( "Could not answer the request. Try again.", diff --git a/packages/contracts/src/secretRequest.ts b/packages/contracts/src/secretRequest.ts index e843b24f79c9..f76a84f6c628 100644 --- a/packages/contracts/src/secretRequest.ts +++ b/packages/contracts/src/secretRequest.ts @@ -16,7 +16,37 @@ export const SecretRequestAnswerInput = Schema.Struct({ }); export type SecretRequestAnswerInput = typeof SecretRequestAnswerInput.Type; +const SECRET_REQUEST_FAILURE_MESSAGES = { + load_failed: "Could not load the secret request.", + not_found: "This secret request no longer exists.", + already_answered: "This secret request was already answered.", + agent_stopped: "The agent that asked has stopped, so this secret can't be used.", + store_failed: "Could not store the secret.", + record_failed: "Saved the secret, but could not update the request.", + invalid_ref: "That secretRef is not valid.", + read_failed: "Could not read the secret.", + ref_unavailable: + "That secretRef was already used or does not exist. Ask the user again with request_secret.", + ref_expired: "That secretRef expired. Ask the user again with request_secret.", + consume_failed: "Could not use that secretRef. Try again.", +} as const; + +export const SecretRequestFailureReason = Schema.Literals( + Object.keys(SECRET_REQUEST_FAILURE_MESSAGES) as Array< + keyof typeof SECRET_REQUEST_FAILURE_MESSAGES + >, +); +export type SecretRequestFailureReason = typeof SecretRequestFailureReason.Type; + +/** Answering a request or using its ref failed; the message is shown to users and agents. */ export class SecretRequestError extends Schema.TaggedError()( "SecretRequestError", - { message: Schema.String }, -) {} + { + reason: SecretRequestFailureReason, + cause: Schema.optionalKey(Schema.Defect()), + }, +) { + override get message(): string { + return SECRET_REQUEST_FAILURE_MESSAGES[this.reason]; + } +} From 95b366282db78fb24bdf7db462278708e7094b52 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 15:37:29 -0700 Subject: [PATCH 16/22] fix(server): a secretRef is handed out once even under concurrent use consume read the stored value and deleted it in two store calls, so two concurrent calls with one ref could both read it. Consumption is now serialized. The client's single-flight key for answering a card encodes its ids structurally, so ids containing a colon can't collide. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../server/src/secrets/SecretRequests.test.ts | 23 ++++++++++++++++++- apps/server/src/secrets/SecretRequests.ts | 5 ++++ packages/client-runtime/src/state/server.ts | 3 ++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index b2e2995ce036..5fdc89ea0bbd 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -49,7 +49,8 @@ const withService = ( Layer.succeed( ServerSecretStore.ServerSecretStore, ServerSecretStore.ServerSecretStore.of({ - get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))), + // Yields like a real file read, so concurrent callers can interleave. + get: (name) => Effect.yieldNow.pipe(Effect.as(Option.fromNullishOr(stored.get(name)))), set: (name, value) => Effect.sync(() => void stored.set(name, value)), create: (name, value) => stored.has(name) @@ -148,6 +149,26 @@ it.effect("a saved answer becomes a one-use ref, and the thread only learns it w ), ); +it.effect("two concurrent uses of one ref hand the value out once", () => + withService(({ service }) => + Effect.gen(function* () { + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const results = yield* Effect.all( + [service.consume({ ref, projectId }), service.consume({ ref, projectId })].map( + Effect.result, + ), + { concurrency: "unbounded" }, + ); + assert.deepEqual(results.map((result) => result._tag).toSorted(), ["Failure", "Success"]); + }), + ), +); + it.effect("a ref only works in the project it was entered for", () => withService(({ service }) => Effect.gen(function* () { diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 27dae03287d0..05c5ecad821c 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -27,6 +27,7 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; +import * as Semaphore from "effect/Semaphore"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as Metrics from "../observability/Metrics.ts"; @@ -178,8 +179,12 @@ const make = Effect.gen(function* () { ), ); + // get and remove are separate store calls; one consumer at a time keeps two + // concurrent calls from both reading a ref before either deletes it. + const consumeLock = yield* Semaphore.make(1); const consume: SecretRequests["Service"]["consume"] = (input) => consumeRef(input).pipe( + consumeLock.withPermits(1), Effect.tap(() => Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "used" })), Effect.tapError(() => Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "rejected" }), diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index e55d447de561..17037af3c986 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1306,7 +1306,8 @@ export function createServerEnvironmentAtoms( tag: WS_METHODS.secretsAnswerRequest, concurrency: { mode: "singleFlight", - key: ({ environmentId, input }) => `${environmentId}:${input.threadId}:${input.turnItemId}`, + key: ({ environmentId, input }) => + JSON.stringify([environmentId, input.threadId, input.turnItemId]), }, }), refreshUsageRates: createEnvironmentRpcCommand(runtime, { From 0afd06f93b684f360baf05140e2fe5361460d76c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 15:53:02 -0700 Subject: [PATCH 17/22] test(server): the delegation re-probe test provides SecretRequests Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/server/src/mcp/OrchestratorMcpService.test.ts b/apps/server/src/mcp/OrchestratorMcpService.test.ts index d48aa125bfd2..2d255fda123d 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.test.ts @@ -1054,6 +1054,7 @@ describe("OrchestratorMcpService provider resolution", () => { adapterRegistryLayer([codexInstanceId, claudeInstanceId]), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), ); yield* Effect.gen(function* () { From 146ce1f1dc107e907cc66c4a5c6a654a18f89a5f Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 15:56:14 -0700 Subject: [PATCH 18/22] fix(server): a secret request card closes when the wait fails, not only on timeout A failed read while waiting returned without closing the card, so the user could still save a value nobody would receive. The card now closes on every exit except an answer, and a failed close is logged instead of ignored. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 026062f805f2..666053591105 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -63,6 +63,7 @@ import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; @@ -1588,8 +1589,14 @@ const make = Effect.gen(function* () { ); yield* record("pending"); // Only this call can hand the agent its ref, so the card must not - // outlive it: a timeout or an aborted call closes it as cancelled. - const closeCard = record("cancelled").pipe(Effect.ignore); + // outlive it: a timeout, a failed wait or an aborted call closes it as + // cancelled. If even that fails, the server still refuses an answer + // once the run ends, and an unused value expires. + const closeCard = record("cancelled").pipe( + Effect.catch((error) => + Effect.logWarning("Could not close a secret request card", { error: error.message }), + ), + ); // The user answers the card (secrets.answerRequest), or it ends with // the run; poll it like a delegated task. @@ -1628,7 +1635,7 @@ const make = Effect.gen(function* () { Math.min(input.timeoutMs ?? DEFAULT_WAIT_TIMEOUT_MS, MAX_WAIT_TIMEOUT_MS), ), ), - Effect.onInterrupt(() => closeCard), + Effect.onExit((exit) => (Exit.isSuccess(exit) ? Effect.void : closeCard)), ); if (Option.isNone(answered)) yield* closeCard; // An answer that raced the timeout still wins: the card is answered once. From f71850c9851a2a3a272f2f34b284f672084fa410 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 16:00:55 -0700 Subject: [PATCH 19/22] fix(server): a secret card closed by recovery or a racing cancel takes no value Restart recovery cancelled an open secret request but left its form open, so the user could still try to answer it. It now closes the form too. A save that lands just after the agent's wait closed the card changed nothing, but its value stayed stored until it expired; the save now checks its record and deletes the value if the card was already closed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ProviderRuntimeRecoveryService.test.ts | 65 +++++++++++++++++++ .../ProviderRuntimeRecoveryService.ts | 13 +++- .../server/src/secrets/SecretRequests.test.ts | 23 ++++++- apps/server/src/secrets/SecretRequests.ts | 15 +++++ 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.test.ts b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.test.ts index b36239de290d..cb06238395a6 100644 --- a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.test.ts @@ -578,6 +578,71 @@ it.effect("cancels a stale waiting run when no checkpoint capture can finish it" }).pipe(Effect.provide(layer)); }); +it.effect("closes a secret request card's form when its run is recovered", () => { + const threadId = ThreadId.make("thread_secret_recovery"); + const runId = RunId.make("run_secret_recovery"); + let committedInput: Parameters[0] | null = + null; + const projection = { + thread: { id: threadId }, + runtimeRequests: [], + providerSessions: [], + providerThreads: [], + providerTurns: [], + runs: [{ id: runId, status: "running", providerInstanceId: ProviderInstanceId.make("codex") }], + attempts: [], + nodes: [], + subagents: [], + messages: [], + turnItems: [ + { + id: "turn-item:secret-request:recovery", + threadId, + runId, + nodeId: null, + type: "secret_request", + status: "waiting", + secretStatus: "pending", + label: "GitHub token", + reason: "Used as GH_TOKEN.", + }, + ], + } as unknown as OrchestrationV2ThreadProjection; + const layer = ProviderRuntimeRecovery.layer.pipe( + Layer.provide(ServerSettings.layerTest()), + Layer.provide( + Layer.mergeAll( + Layer.mock(ProjectionStore.ProjectionStoreV2)({ + getRecoveryThreadIds: () => Effect.succeed([threadId]), + getRuntimeRecoveryProjection: () => Effect.succeed(projection), + }), + Layer.mock(EventSink.EventSinkV2)({ + commitCommand: (input) => { + committedInput = input; + return Effect.succeed({ committed: true, cancelledEffectCount: 0 } as never); + }, + }), + IdAllocator.layer, + Layer.mock(EffectWorker.OrchestrationEffectWorkerV2)({ + runRecoveryOnce: Effect.succeed(false), + }), + Layer.mock(EffectOutbox.EffectOutboxV2)({ + listByCommandId: () => Effect.succeed([]), + reconcileAfterProcessLoss: Effect.succeed({ requeued: 0, cancelled: 0 }), + }), + ), + ), + ); + + return Effect.gen(function* () { + yield* (yield* ProviderRuntimeRecovery.ProviderRuntimeRecoveryService).reconcile("startup"); + const itemEvent = committedInput?.events.find((event) => event.type === "turn-item.updated"); + const card = itemEvent?.type === "turn-item.updated" ? itemEvent.payload : null; + assert.equal(card?.status, "cancelled"); + assert.equal(card?.type === "secret_request" ? card.secretStatus : null, "cancelled"); + }).pipe(Effect.provide(layer)); +}); + it.effect("holds accepted queued work without cancelling its execution state after restart", () => { const threadId = ThreadId.make("thread_queued_restart"); const runId = RunId.make("run_queued_restart"); diff --git a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts index 6eb0ef12e605..27a79b9fa757 100644 --- a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts +++ b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts @@ -149,6 +149,15 @@ function resolveStaleBackgroundItemProviderInstanceId( return projection.providerThreads[0]?.providerInstanceId ?? projection.thread.providerInstanceId; } +/** An open item closed by recovery. A secret card also closes its form, so it takes no answer. */ +const cancelledItem = ( + item: OrchestrationV2ThreadProjection["turnItems"][number], + now: DateTime.Utc, +): OrchestrationV2ThreadProjection["turnItems"][number] => + item.type === "secret_request" && item.secretStatus === "pending" + ? { ...item, status: "cancelled", secretStatus: "cancelled", completedAt: now, updatedAt: now } + : { ...item, status: "cancelled", completedAt: now, updatedAt: now }; + /** * A provider thread's latest started run: the last turn that provider saw. * Restart recovery records the thread's cancelled background work on it, and @@ -426,7 +435,7 @@ export const make = Effect.gen(function* () { ...(item.nodeId === null ? {} : { nodeId: item.nodeId }), providerInstanceId: run.providerInstanceId, occurredAt: now, - payload: { ...item, status: "cancelled", completedAt: now, updatedAt: now }, + payload: cancelledItem(item, now), }); } } @@ -456,7 +465,7 @@ export const make = Effect.gen(function* () { ...(item.nodeId === null || item.nodeId === undefined ? {} : { nodeId: item.nodeId }), providerInstanceId, occurredAt: now, - payload: { ...item, status: "cancelled", completedAt: now, updatedAt: now }, + payload: cancelledItem(item, now), }); if (item.nodeId !== null && item.nodeId !== undefined) { const staleItemNode = projection.nodes.find( diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 5fdc89ea0bbd..8afc2d6dd64b 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -36,6 +36,8 @@ const withService = ( readonly threadId?: ThreadId; readonly runStatus?: string; readonly removeFails?: boolean; + /** The agent's wait closes the card just before the answer's record lands. */ + readonly closedFirst?: boolean; } = {}, ) => Effect.gen(function* () { @@ -107,7 +109,12 @@ const withService = ( dispatch: (command) => Effect.sync(() => { dispatched.push(command); - if (command.type === "secret_request.record") secretStatus = command.secretStatus; + // Like the orchestrator, a card that is no longer pending keeps its answer. + if (options.closedFirst && command.type === "secret_request.record") { + secretStatus = "cancelled"; + } else if (command.type === "secret_request.record" && secretStatus === "pending") { + secretStatus = command.secretStatus; + } return {} as never; }), }), @@ -169,6 +176,20 @@ it.effect("two concurrent uses of one ref hand the value out once", () => ), ); +it.effect("a save that loses to the card closing deletes the value and says so", () => + withService( + ({ service, stored }) => + Effect.gen(function* () { + const error = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) + .pipe(Effect.flip); + assert.equal(error.reason, "agent_stopped"); + assert.isFalse(valuesOf(stored).some((value) => value.includes("ghp_secret"))); + }), + { closedFirst: true }, + ), +); + it.effect("a ref only works in the project it was entered for", () => withService(({ service }) => Effect.gen(function* () { diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 05c5ecad821c..64624d678708 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -159,6 +159,21 @@ const make = Effect.gen(function* () { secretStatus, }) .pipe(Effect.mapError((cause) => fail("record_failed", cause))); + if (input.answer.type !== "save") return; + // A request is answered once: if the agent's wait closed the card between + // the checks above and this record, the record changed nothing. Nobody + // will receive the ref, so the value is deleted rather than left to expire. + const recorded = yield* threadManagement + .getThreadRecords(input.threadId, ["turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe(Effect.mapError((cause) => fail("record_failed", cause))); + const card = recorded.turnItems.find((candidate) => candidate.id === item.id); + if (card?.type === "secret_request" && card.secretStatus !== "saved") { + yield* removeLogged(storeName(refFor(salt, input.threadId, item.id))); + return yield* fail("agent_stopped"); + } }).pipe(Effect.withSpan("SecretRequests.answer")); const savedRef: SecretRequests["Service"]["savedRef"] = (input) => From 6eee1e72cb0b710a6fd0eded9d1472ba576c66f1 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 16:04:15 -0700 Subject: [PATCH 20/22] fix: a failed save can be retried, and the card can't be submitted twice If recording a saved answer failed, the stored value stayed behind and the user's retry was refused as already answered; the value is now removed so the card can be saved again. The secret card guards against a double submit synchronously on web and mobile. request_secret's docs say a new clientRequestId asks again after timed_out or cancelled. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/SecretRequestCard.tsx | 13 ++++--- .../ScheduledTaskService.webhook.test.ts | 12 +++---- .../server/src/secrets/SecretRequests.test.ts | 34 +++++++++++++++++-- apps/server/src/secrets/SecretRequests.ts | 11 +++++- .../src/components/chat/SecretRequestCard.tsx | 13 ++++--- packages/contracts/src/orchestratorMcp.ts | 4 +-- 6 files changed, 67 insertions(+), 20 deletions(-) diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx index 3999f53d35ff..f1cf25fde12f 100644 --- a/apps/mobile/src/features/threads/SecretRequestCard.tsx +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -11,7 +11,7 @@ import { squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, OrchestrationV2ProjectedTurnItem } from "@t3tools/contracts"; -import { useState } from "react"; +import { useRef, useState } from "react"; import { Pressable, View, type ColorValue } from "react-native"; import { SymbolView, type AppSymbolName } from "../../components/AppSymbol"; @@ -76,16 +76,21 @@ function PendingSecretRequestForm(props: { const [secret, setSecret] = useState(""); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); + // Submit then a tap can both run before a re-render; this guard is synchronous. + const inFlight = useRef(false); const send = async ( reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, ) => { const input = secretRequestAnswerInput(item, reply); - if (input === null || submitting) return; + if (input === null || inFlight.current) return; + inFlight.current = true; setSubmitting(true); setError(null); - const result = await answer({ environmentId: props.environmentId, input }); - setSubmitting(false); + const result = await answer({ environmentId: props.environmentId, input }).finally(() => { + inFlight.current = false; + setSubmitting(false); + }); if (result._tag === "Success") { // The card switches to its answered row once the item updates. setSecret(""); diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 22166d38ecdd..86c96b05901b 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -23,9 +23,6 @@ import * as ScheduledTaskService from "./ScheduledTaskService.ts"; const decodeUpsertInput = Schema.decodeUnknownEffect(ScheduledTaskUpsertInput); -/** Secrets the user entered for an agent, by ref; consuming one removes it. */ -const secretsByRef = new Map(); - type LaunchInput = ThreadLaunchService.ThreadLaunchInput; const webhookTaskInput = (overrides: Record = {}) => @@ -70,11 +67,14 @@ const withService = ( body: (input: { readonly service: ScheduledTaskService.ScheduledTaskService["Service"]; readonly launches: Queue.Queue; + /** Secrets the user entered for an agent, by ref; consuming one removes it. */ + readonly secretsByRef: Map; }) => Effect.Effect, options: { readonly gate?: Deferred.Deferred; readonly relayHookBaseUrl?: string } = {}, ) => Effect.gen(function* () { const launches = yield* Queue.unbounded(); + const secretsByRef = new Map(); const dependencies = Layer.mergeAll( NodePlatformCrypto.layer, Scheduler.layer, @@ -102,7 +102,7 @@ const withService = ( ); return yield* Effect.gen(function* () { const service = yield* ScheduledTaskService.ScheduledTaskService; - return yield* body({ service, launches }); + return yield* body({ service, launches, secretsByRef }); }).pipe(Effect.provide(ScheduledTaskService.layer.pipe(Layer.provide(dependencies)))); }).pipe(Effect.provide(SqlitePersistenceMemory)); @@ -725,7 +725,7 @@ const githubSignature = (secret: string) => `sha256=${NodeCrypto.createHmac("sha256", secret).update(pullRequestBody).digest("hex")}`; it.effect("a signature can take the user's secret by ref, which works only once", () => - withService(({ service, launches }) => + withService(({ service, launches, secretsByRef }) => Effect.gen(function* () { secretsByRef.set("secret-ref:00000000000000000000000000000001", "github-secret"); const githubSchedule = (secretRef: string) => ({ @@ -764,7 +764,7 @@ it.effect("a signature can take the user's secret by ref, which works only once" ); it.effect("a retried save with an already used secretRef keeps the stored secret", () => - withService(({ service, launches }) => + withService(({ service, launches, secretsByRef }) => Effect.gen(function* () { secretsByRef.set("secret-ref:00000000000000000000000000000002", "github-secret"); const save = webhookTaskInput({ diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 8afc2d6dd64b..7adea9711a85 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -38,12 +38,15 @@ const withService = ( readonly removeFails?: boolean; /** The agent's wait closes the card just before the answer's record lands. */ readonly closedFirst?: boolean; + /** How many record dispatches fail before one succeeds. */ + readonly failedRecords?: number; } = {}, ) => Effect.gen(function* () { const stored = new Map(); const dispatched: Array = []; let secretStatus = "pending"; + let failedRecords = options.failedRecords ?? 0; const requestThreadId = options.threadId ?? threadId; const dependencies = Layer.mergeAll( NodeCrypto.layer, @@ -106,8 +109,12 @@ const withService = ( }, ], } as never), - dispatch: (command) => - Effect.sync(() => { + dispatch: (command) => { + if (command.type === "secret_request.record" && failedRecords > 0) { + failedRecords -= 1; + return Effect.fail(new Error("orchestrator unavailable") as never); + } + return Effect.sync(() => { dispatched.push(command); // Like the orchestrator, a card that is no longer pending keeps its answer. if (options.closedFirst && command.type === "secret_request.record") { @@ -116,7 +123,8 @@ const withService = ( secretStatus = command.secretStatus; } return {} as never; - }), + }); + }, }), ); return yield* Effect.gen(function* () { @@ -190,6 +198,26 @@ it.effect("a save that loses to the card closing deletes the value and says so", ), ); +it.effect("a save whose record failed can be saved again", () => + withService( + ({ service }) => + Effect.gen(function* () { + const failed = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) + .pipe(Effect.flip); + assert.equal(failed.reason, "record_failed"); + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); + }), + { failedRecords: 1 }, + ), +); + it.effect("a ref only works in the project it was entered for", () => withService(({ service }) => Effect.gen(function* () { diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 64624d678708..5131637ba41a 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -158,7 +158,16 @@ const make = Effect.gen(function* () { ...(item.placeholder === undefined ? {} : { placeholder: item.placeholder }), secretStatus, }) - .pipe(Effect.mapError((cause) => fail("record_failed", cause))); + .pipe( + Effect.mapError((cause) => fail("record_failed", cause)), + // The card still says pending, so the user can save again; the value + // stored above would make that retry look already answered. + Effect.tapError(() => + input.answer.type === "save" + ? removeLogged(storeName(refFor(salt, input.threadId, item.id))) + : Effect.void, + ), + ); if (input.answer.type !== "save") return; // A request is answered once: if the agent's wait closed the card between // the checks above and this record, the record changed nothing. Nobody diff --git a/apps/web/src/components/chat/SecretRequestCard.tsx b/apps/web/src/components/chat/SecretRequestCard.tsx index c80d63b2043c..d11cd57568cf 100644 --- a/apps/web/src/components/chat/SecretRequestCard.tsx +++ b/apps/web/src/components/chat/SecretRequestCard.tsx @@ -12,7 +12,7 @@ import { } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, OrchestrationV2ProjectedTurnItem } from "@t3tools/contracts"; import { CheckIcon, LockIcon, MinusIcon, ShieldCheckIcon } from "lucide-react"; -import { useId, useState, type FormEvent } from "react"; +import { useId, useRef, useState, type FormEvent } from "react"; import { serverEnvironment } from "../../state/server"; import { useAtomCommand } from "../../state/use-atom-command"; @@ -71,16 +71,21 @@ function PendingSecretRequestForm(props: { const [secret, setSecret] = useState(""); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); + // Enter then a click can both run before a re-render; this guard is synchronous. + const inFlight = useRef(false); const send = async ( reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, ) => { const input = secretRequestAnswerInput(item, reply); - if (input === null || submitting) return; + if (input === null || inFlight.current) return; + inFlight.current = true; setSubmitting(true); setError(null); - const result = await answer({ environmentId: props.environmentId, input }); - setSubmitting(false); + const result = await answer({ environmentId: props.environmentId, input }).finally(() => { + inFlight.current = false; + setSubmitting(false); + }); if (result._tag === "Success") { // The card switches to its answered row once the item updates. setSecret(""); diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index bcc82beb1ceb..68f124de078b 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -600,7 +600,7 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ ).annotate({ description: "How long to wait for the user. Default 10 minutes." }), clientRequestId: Schema.optional(OrchestratorMcpClientRequestId).annotate({ description: - "Reuse when retrying, so the user sees one card and a saved answer is returned again.", + "Reuse when retrying a call that lost its result, so the user sees one card and its answer is returned again. Use a new id to ask again after timed_out or cancelled.", }), }); export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; @@ -608,7 +608,7 @@ export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSec export const OrchestratorMcpRequestSecretResult = Schema.Struct({ status: Schema.Literals(["saved", "declined", "cancelled", "timed_out"]).annotate({ description: - "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. timed_out: the user did not answer in time; the card is closed, so ask again if still needed.", + "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. timed_out: the user did not answer in time; the card is closed, so ask again with a new clientRequestId if still needed.", }), secretRef: Schema.optional(SecretRef).annotate({ description: From ba6e3d162a9219dd5b4228f621d4c06161582ae3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 16:07:29 -0700 Subject: [PATCH 21/22] fix(server): saving again finishes a save whose record and cleanup both failed If recording a save failed and its stored value couldn't be removed either, the next save found the value and was refused as already answered. A value already stored for a card that is still pending is now treated as that earlier save, and recording it finishes the save. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../server/src/secrets/SecretRequests.test.ts | 20 ++++++++++++++++++- apps/server/src/secrets/SecretRequests.ts | 9 ++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 7adea9711a85..7089c826b374 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -61,7 +61,7 @@ const withService = ( stored.has(name) ? Effect.fail( new ServerSecretStore.SecretStorePersistError({ - name, + resource: name, cause: new PlatformError.PlatformError( new PlatformError.SystemError({ _tag: "AlreadyExists", @@ -218,6 +218,24 @@ it.effect("a save whose record failed can be saved again", () => ), ); +it.effect("a save whose record and cleanup both failed is finished by saving again", () => + withService( + ({ service }) => + Effect.gen(function* () { + yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) + .pipe(Effect.flip); + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + assert.isTrue(Option.isSome(yield* service.savedRef({ threadId, turnItemId }))); + }), + { failedRecords: 1, removeFails: true }, + ), +); + it.effect("a ref only works in the project it was entered for", () => withService(({ service }) => Effect.gen(function* () { diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 5131637ba41a..8c2dacd4dcad 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -125,6 +125,8 @@ const make = Effect.gen(function* () { } // Store first: the card only says saved once the value is kept. Create, // not set: a second answer racing this one must not replace the value. + // A value already there on a card still pending is an earlier save whose + // record failed and could not be cleaned up; recording it finishes that save. if (input.answer.type === "save") { const encoded = yield* encodeStored({ projectId: records.thread.projectId, @@ -137,11 +139,8 @@ const make = Effect.gen(function* () { new TextEncoder().encode(encoded), ) .pipe( - Effect.mapError((error) => - ServerSecretStore.isSecretAlreadyExistsError(error) - ? fail("already_answered", error) - : fail("store_failed", error), - ), + Effect.catchIf(ServerSecretStore.isSecretAlreadyExistsError, () => Effect.void), + Effect.mapError((error) => fail("store_failed", error)), ); } const secretStatus = input.answer.type === "save" ? "saved" : "declined"; From cae32ed6f4f47f78746596fd6599de377c686c44 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 16:10:34 -0700 Subject: [PATCH 22/22] fix(server): request_secret's saved result always carries its secretRef The result schema allowed status saved without a secretRef, so an agent could be told a secret was saved with nothing to pass on. Saved now requires the ref, and a saved card whose value can't be read fails the call instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/mcp/OrchestratorMcpService.ts | 13 ++++++++---- packages/contracts/src/orchestratorMcp.ts | 21 ++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 666053591105..685f1be0faea 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1659,11 +1659,16 @@ const make = Effect.gen(function* () { yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); yield* Metrics.increment(Metrics.secretRequestsTotal, { status }); if (status !== "saved") return { status }; + // Saved means the value was stored before the card said so; a missing + // value is a storage fault, not an answer the agent can act on. const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); - return Option.match(secretRef, { - onNone: () => ({ status }), - onSome: (ref) => ({ status, secretRef: ref }), - }); + if (Option.isNone(secretRef)) { + return yield* failure( + "orchestration_error", + "The user saved the secret, but it could not be read. Ask again with a new clientRequestId.", + ); + } + return { status, secretRef: secretRef.value }; }).pipe(Effect.withSpan("OrchestratorMcpService.requestSecret")), capabilities: (scope) => diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 68f124de078b..0a9179c2a8f4 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -605,16 +605,21 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ }); export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; -export const OrchestratorMcpRequestSecretResult = Schema.Struct({ - status: Schema.Literals(["saved", "declined", "cancelled", "timed_out"]).annotate({ - description: - "saved: secretRef holds the value. declined: the user chose not to. cancelled: the request ended with the run. timed_out: the user did not answer in time; the card is closed, so ask again with a new clientRequestId if still needed.", +export const OrchestratorMcpRequestSecretResult = Schema.Union([ + Schema.Struct({ + status: Schema.Literal("saved").annotate({ description: "secretRef holds the value." }), + secretRef: SecretRef.annotate({ + description: + "Pass it to a tool that accepts a secretRef; it works once, and you never see the value.", + }), }), - secretRef: Schema.optional(SecretRef).annotate({ - description: - "Present when saved. Pass it to a tool that accepts a secretRef; it works once, and you never see the value.", + Schema.Struct({ + status: Schema.Literals(["declined", "cancelled", "timed_out"]).annotate({ + description: + "declined: the user chose not to. cancelled: the request ended with the run. timed_out: the user did not answer in time; the card is closed, so ask again with a new clientRequestId if still needed.", + }), }), -}); +]); export type OrchestratorMcpRequestSecretResult = typeof OrchestratorMcpRequestSecretResult.Type; export const OrchestratorMcpDeleteScheduledTaskInput = Schema.Struct({