From 6a5436db7061fbbba4aaa29d5b320eb9f797b288 Mon Sep 17 00:00:00 2001 From: Gabriel De Andrade Date: Mon, 2 Nov 2026 20:58:51 -0400 Subject: [PATCH 1/5] feat(server): agents ask for secrets through request_secret instead of chat - Claude now loads request_secret up front (`anthropic/alwaysLoad`), so it is in reach without a ToolSearch. - The instruction line tells agents to open the card rather than offer it or ask in chat, and to call it when asked how to give a secret. - request_secret takes `value`, a secret the user pasted in this thread, and saves its exact bytes to an owner-only file with no card. delegate_task says to keep secrets out of `task` and give the child that file instead. - A declined, cancelled or timed-out result carries a note on what to do next. Co-Authored-By: Claude Opus 5.5 --- .../OrchestratorMcpService.activity.test.ts | 166 ++++++++++++++++++ apps/server/src/mcp/OrchestratorMcpService.ts | 22 ++- .../src/mcp/toolkits/orchestrator/tools.ts | 8 +- .../src/provider/AgentContextBudget.test.ts | 25 ++- .../src/provider/agentContextBudget.json | 6 +- .../server/src/secrets/SecretRequests.test.ts | 101 +++++++++++ apps/server/src/secrets/SecretRequests.ts | 52 ++++-- packages/contracts/src/orchestratorMcp.ts | 19 +- packages/contracts/src/secretRequest.ts | 9 + .../src/server/orchestrationInstructions.ts | 2 +- 10 files changed, 374 insertions(+), 36 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index f260093f3f161..2ab25d5c7dccc 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -8,12 +8,14 @@ import { ProviderInstanceId, RunId, ScheduledTaskId, + SECRET_REQUEST_UNSAVED_NOTE, ThreadId, TurnItemId, } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Metric from "effect/Metric"; import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import { expect, it } from "vite-plus/test"; @@ -655,3 +657,167 @@ it("readThread and sendToThread reach threads in other projects", async () => { expect(stopped.code).toBe("parent_not_active"); }).pipe(Effect.provide(layer), Effect.runPromise); }); + +it("requestSecret saves a pasted value to a file without opening a card", async () => { + const projection = { + thread: baseThread({ + threadId: parentThreadId, + title: "Parent", + instanceId: parentInstanceId, + model: "gpt-5.4", + }), + runs: [{ ...makeRun({ id: activeRunId, ordinal: 1, status: "running" }), rootNodeId: taskId }], + visibleTurnItems: [], + runtimeRequests: [], + messages: [], + contextTransfers: [], + subagents: [], + updatedAt: now, + } as unknown as OrchestrationV2ThreadProjection; + const dispatched: Array = []; + const saved: Array<{ readonly requestKey: string; readonly value: string }> = []; + const file = "/home/user/.t3/userdata/secrets/files/0123456789abcdef0123456789abcdef"; + + const layer = layerOrchestratorService.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: (threadId) => + threadId === parentThreadId + ? Effect.succeed(projection) + : Effect.die(`unexpected thread ${threadId}`), + getThreadShell: (threadId) => + Effect.succeed( + threadId === parentThreadId + ? (projection.thread as unknown as OrchestrationV2ThreadShell) + : null, + ), + dispatch: (command) => Effect.sync(() => void dispatched.push(command) as never), + } satisfies Partial), + ServerSettings.layerTest(), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + getProviders: Effect.succeed([]), + } satisfies Partial), + Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({ + savePasted: (input) => + Effect.sync(() => { + saved.push({ requestKey: input.requestKey, value: input.value }); + return file; + }), + }), + Layer.mock(RemoteDelegation.RemoteDelegation)({}), + Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({}), + NodeCrypto.layer, + ), + ), + ); + const counted = Metric.snapshot.pipe( + Effect.map((snapshots) => { + const found = snapshots.find( + (snapshot) => + snapshot.id === "t3_secret_requests_total" && + snapshot.attributes?.status === "saved_pasted", + ); + return found?.type === "Counter" ? Number(found.state.count) : 0; + }), + ); + + await Effect.gen(function* () { + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + const before = yield* counted; + const result = yield* service.requestSecret(makeScope(), { + label: "Pixel PIN", + reason: "Unlocks the test phone.", + value: "482916", + clientRequestId: "pin-1", + }); + expect(result).toEqual({ status: "saved", file }); + expect(JSON.stringify(result)).not.toContain("482916"); + expect(saved).toEqual([{ requestKey: "pin-1", value: "482916" }]); + expect(dispatched).toEqual([]); + expect((yield* counted) - before).toBe(1); + + const asRef = yield* service + .requestSecret(makeScope(), { + label: "Pixel PIN", + reason: "Unlocks the test phone.", + value: "482916", + delivery: "secretRef", + }) + .pipe(Effect.flip); + expect(asRef.code).toBe("invalid_request"); + expect(asRef.message).not.toContain("482916"); + expect(saved).toHaveLength(1); + expect(dispatched).toEqual([]); + }).pipe(Effect.provide(layer), Effect.runPromise); +}); + +it("requestSecret returns the unsaved note for a declined, cancelled or timed-out card", async () => { + const outcomes = ["declined", "cancelled", "pending"] as const; + const results = await Effect.forEach(outcomes, (outcome) => { + const dispatched: Array<{ readonly type: string; readonly turnItemId?: string }> = []; + const projection = () => { + const card = dispatched.find((command) => command.type === "secret_request.record"); + return { + thread: baseThread({ + threadId: parentThreadId, + title: "Parent", + instanceId: parentInstanceId, + model: "gpt-5.4", + }), + runs: [ + { ...makeRun({ id: activeRunId, ordinal: 1, status: "running" }), rootNodeId: taskId }, + ], + visibleTurnItems: [], + turnItems: + card === undefined + ? [] + : [{ type: "secret_request", id: card.turnItemId, secretStatus: outcome }], + runtimeRequests: [], + messages: [], + contextTransfers: [], + subagents: [], + updatedAt: now, + } as unknown as OrchestrationV2ThreadProjection; + }; + const layer = layerOrchestratorService.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: () => Effect.sync(projection), + getThreadShell: () => + Effect.sync(() => projection().thread as unknown as OrchestrationV2ThreadShell), + dispatch: (command) => Effect.sync(() => void dispatched.push(command) as never), + } satisfies Partial), + ServerSettings.layerTest(), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + getProviders: Effect.succeed([]), + } satisfies Partial), + Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), + Layer.mock(RemoteDelegation.RemoteDelegation)({}), + Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({}), + NodeCrypto.layer, + ), + ), + ); + return Effect.gen(function* () { + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + return yield* service.requestSecret(makeScope(), { + label: "Staging password", + reason: "Signs in the QA account.", + delivery: "file", + timeoutMs: 1_000, + }); + }).pipe(Effect.provide(layer)); + }).pipe(Effect.runPromise); + + expect(results).toEqual([ + { status: "declined", note: SECRET_REQUEST_UNSAVED_NOTE.declined }, + { status: "cancelled", note: SECRET_REQUEST_UNSAVED_NOTE.cancelled }, + { status: "timed_out", note: SECRET_REQUEST_UNSAVED_NOTE.timed_out }, + ]); +}); diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index e84212cab0023..c21305479aff0 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -63,6 +63,7 @@ import { ThreadId, isOrchestrationV2WorkActive, resolveProjectProviderInstanceEnabled, + SECRET_REQUEST_UNSAVED_NOTE, } from "@t3tools/contracts"; import { getProviderOptionCurrentValue } from "@t3tools/shared/model"; import { runRanAfter } from "@t3tools/shared/orchestrationV2ThreadError"; @@ -2135,6 +2136,25 @@ const make = Effect.gen(function* () { "Asking for a secret requires an active run owned by this MCP provider session.", ); } + // A secret the user already pasted needs no card: it goes straight to a file. + if (input.value !== undefined) { + if (input.delivery === "secretRef") { + return yield* failure( + "invalid_request", + 'A pasted `value` is saved as a file. Leave out `delivery: "secretRef"`.', + ); + } + const file = yield* secretRequests + .savePasted({ + threadId, + requestKey: yield* requestKey(input.clientRequestId), + value: input.value, + }) + .pipe(Effect.mapError((error) => failure("orchestration_error", error.message))); + yield* Effect.annotateCurrentSpan({ "secret_request.status": "saved_pasted" }); + yield* Metrics.increment(Metrics.secretRequestsTotal, { status: "saved_pasted" }); + return { status: "saved" as const, file }; + } const runId = run.id; const nodeId = run.rootNodeId; const key = yield* requestKey(input.clientRequestId); @@ -2235,7 +2255,7 @@ const make = Effect.gen(function* () { ); yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); yield* Metrics.increment(Metrics.secretRequestsTotal, { status }); - if (status !== "saved") return { status }; + if (status !== "saved") return { status, note: SECRET_REQUEST_UNSAVED_NOTE[status] }; if (input.delivery === "file") { const file = yield* secretRequests .saveToFile({ threadId, turnItemId }) diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 7c05f5c8d8e25..6016622936fd2 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -85,7 +85,7 @@ const OrchestratorCapabilitiesTool = Tool.make("orchestrator_capabilities", { const DelegateTaskTool = Tool.make("delegate_task", { description: - "Start one child agent with only `task`. No parent history. Separate checkout or PR per child: `isolation:'worktree'`. Keep `taskId` for `task_status` and `task_cancel`.", + "Start one child agent with only `task`. No parent history. Separate checkout or PR per child: `isolation:'worktree'`. Keep `taskId` for `task_status` and `task_cancel`. No secrets in `task` or temp files: `request_secret` `value`.", parameters: OrchestratorMcpDelegateTaskInput, success: OrchestratorMcpDelegateTaskResult, failure: OrchestratorMcpFailure, @@ -197,7 +197,7 @@ const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { const RequestSecretTool = Tool.make("request_secret", { description: - 'Ask the user to enter a secret in a private card and wait. You never see the value. Returns a one-use `secretRef` for a tool that accepts it, or with `delivery: "file"` a private file path to read inside a shell command. Asked how to provide a secret: call this now, do not explain instead.', + 'Ask for a secret in a private card and wait; open it, don\'t offer it. Returns a one-use `secretRef`, or with `delivery: "file"` a file to read only in its command, e.g. `adb pair "$(cat )"`. Asked how to give one: call now. Pasted secret for a child or script: `value`. Never reuse old ones.', parameters: OrchestratorMcpRequestSecretInput, success: OrchestratorMcpRequestSecretResult, failure: OrchestratorMcpFailure, @@ -205,7 +205,9 @@ const RequestSecretTool = Tool.make("request_secret", { dependencies, }) .annotate(Tool.Title, "Request a secret from the user") - .annotate(Tool.Destructive, false); + .annotate(Tool.Destructive, false) + // Claude Code defers MCP tools behind ToolSearch; agents that never load this one ask in chat. + .annotate(Tool.Meta, { "anthropic/alwaysLoad": true }); const CreateThreadsTool = Tool.make("create_threads", { description: diff --git a/apps/server/src/provider/AgentContextBudget.test.ts b/apps/server/src/provider/AgentContextBudget.test.ts index c4e3b812408f8..08bfd011514fc 100644 --- a/apps/server/src/provider/AgentContextBudget.test.ts +++ b/apps/server/src/provider/AgentContextBudget.test.ts @@ -106,6 +106,22 @@ const codexAdditionalContext = () => .map((entry) => entry.value) .join(""); +const layerAdvertisedTools = McpHttpServer.layerTools.pipe( + Layer.provideMerge(McpHttpServer.layerInProcessMcpServer), + Layer.provide(toolHandlerServices), +); + +// Claude Code loads these tools up front instead of behind ToolSearch, outside this budget. +it.effect("only request_secret asks Claude Code to load it up front", () => + Effect.gen(function* () { + const mcp = yield* McpServer.McpServer; + const alwaysLoad = mcp.tools + .filter(({ tool }) => tool._meta?.["anthropic/alwaysLoad"] !== undefined) + .map(({ tool }) => [tool.name, tool._meta]); + expect(alwaysLoad).toEqual([["request_secret", { "anthropic/alwaysLoad": true }]]); + }).pipe(Effect.provide(layerAdvertisedTools)), +); + // CI rejects a PR that raises any value above main's agentContextBudget.json, so growth // needs the agent-context-increase label. Lower the values when the text shrinks. it.effect("agent context matches the committed budget", () => @@ -130,12 +146,5 @@ it.effect("agent context matches the committed budget", () => measured, "Set agentContextBudget.json to the measured values. Before you cut text to fit, run the evals that cover it (evals/README.md).", ).toEqual(budget); - }).pipe( - Effect.provide( - McpHttpServer.layerTools.pipe( - Layer.provideMerge(McpHttpServer.layerInProcessMcpServer), - Layer.provide(toolHandlerServices), - ), - ), - ), + }).pipe(Effect.provide(layerAdvertisedTools)), ); diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index 1719d3ca6bd19..bff541d7ba947 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { - "claudeSystemAppendBytes": 5153, - "codexAdditionalContextBytes": 6883, - "mcpToolListJsonBytes": 47538 + "claudeSystemAppendBytes": 5148, + "codexAdditionalContextBytes": 6878, + "mcpToolListJsonBytes": 47534 } diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index 8962e5d4103d0..be24f356b5a9e 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -3,6 +3,7 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { + OrchestratorMcpRequestSecretInput, ProjectId, type OrchestrationV2ServerCommand, SecretRef, @@ -10,6 +11,7 @@ import { TurnItemId, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Metric from "effect/Metric"; @@ -17,6 +19,7 @@ import * as Tracer from "effect/Tracer"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; +import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; @@ -26,6 +29,8 @@ 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"); +const decodeSecretInput = Schema.decodeUnknownEffect(OrchestratorMcpRequestSecretInput); +const decodeSecretInputExit = Schema.decodeUnknownExit(OrchestratorMcpRequestSecretInput); const withService = ( body: (input: { @@ -456,3 +461,99 @@ it.effect("a secret the card promised to keep from the agent is never written to }), ), ); + +it.effect("a pasted secret goes straight to an owner-only file, with no card", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "secret-files-test-" }); + const spans: Array = []; + const tracer = Tracer.make({ + span: (options) => { + const span = new Tracer.NativeSpan(options); + spans.push(span); + return span; + }, + }); + const pasted = "4829'$(id) 16"; + const file = yield* withService( + ({ service, stored, dispatched }) => + Effect.gen(function* () { + const file = yield* service.savePasted({ threadId, requestKey: "key-1", value: pasted }); + assert.equal(path.dirname(file), path.join(directory, "files")); + assert.match(path.basename(file), /^[0-9a-f]{32}$/); + assert.equal(yield* fs.readFileString(file), pasted); + if ((yield* HostProcess.Platform) !== "win32") { + assert.equal((yield* fs.stat(file)).mode & 0o777, 0o600); + assert.equal((yield* fs.stat(path.dirname(file))).mode & 0o777, 0o700); + } + // No card and no stored copy: the thread never hears of it. + assert.deepEqual(dispatched, []); + assert.deepEqual(valuesOf(stored), []); + // A retry with the same key writes the same file; another key gets its own. + assert.equal( + yield* service.savePasted({ threadId, requestKey: "key-1", value: pasted }), + file, + ); + const other = yield* service.savePasted({ threadId, requestKey: "key-2", value: "x" }); + assert.notEqual(other, file); + // Its name is not a ref to the value. + const ref = SecretRef.make(`secret-ref:${path.basename(file)}`); + const reused = yield* service.consume({ ref, projectId }).pipe(Effect.flip); + assert.equal(reused.reason, "ref_unavailable"); + return file; + }).pipe(Effect.withTracer(tracer)), + { directory }, + ); + const recorded = spans.flatMap((span) => [ + span.name, + ...Array.from(span.attributes.values(), String), + ]); + assert.include(recorded, "SecretRequests.savePasted"); + assert.isFalse(recorded.some((value) => value.includes("4829"))); + + const { mtime } = yield* fs.stat(file); + yield* TestClock.setTime(Option.getOrThrow(mtime).getTime() + SECRET_FILE_TTL_MS); + yield* withService(() => Effect.void, { directory }); + assert.isTrue(yield* fs.exists(file)); + yield* TestClock.setTime(Option.getOrThrow(mtime).getTime() + SECRET_FILE_TTL_MS + 1); + yield* withService(() => Effect.void, { directory }); + assert.isFalse(yield* fs.exists(file)); + }).pipe(Effect.provide(NodeServices.layer)), +); + +it.effect("a pasted secret keeps its exact bytes from the tool input to the file", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "secret-files-test-" }); + const pasted = " pa ss\tword\t "; + const input = yield* decodeSecretInput({ + label: "Staging password", + reason: "Signs in the QA account.", + value: pasted, + }); + assert.equal(input.value, pasted); + const empty = decodeSecretInputExit({ + label: "Staging password", + reason: "Signs in the QA account.", + value: "", + }); + assert.isTrue(Exit.isFailure(empty)); + const file = yield* withService( + ({ service }) => service.savePasted({ threadId, requestKey: "key-1", value: input.value! }), + { directory }, + ); + assert.equal(yield* fs.readFileString(file), pasted); + }).pipe(Effect.provide(NodeServices.layer)), +); + +it.effect("a pasted secret is refused without a secrets directory", () => + withService(({ service }) => + Effect.gen(function* () { + const failed = yield* service + .savePasted({ threadId, requestKey: "key-1", value: "4829" }) + .pipe(Effect.flip); + assert.equal(failed.reason, "store_failed"); + }), + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 9e3a7c033c220..3b2e067b6c123 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -1,6 +1,7 @@ // Answers stay in the secret store behind one-use refs; orchestration records only request status. // Consuming tools keep values out of transcripts, projections, clients and model context. // A `file` request instead moves its value to an owner-only file for the agent's own shell. +// A secret the user pasted in chat goes straight to such a file, with no card. import { pipe } from "effect/Function"; import { CommandId, @@ -66,6 +67,12 @@ export class SecretRequests extends Context.Service< readonly threadId: ThreadId; readonly turnItemId: string; }) => Effect.Effect; + /** Writes a secret the user pasted in chat to an owner-only file and returns its path. */ + readonly savePasted: (input: { + readonly threadId: ThreadId; + readonly requestKey: string; + readonly value: string; + }) => Effect.Effect; } >()("t3/secrets/SecretRequests") {} @@ -232,6 +239,23 @@ const make = Effect.gen(function* () { return value; }); + const writeSecretFile = (dir: string, file: string, value: string) => { + const partial = `${file}.partial`; + return fileSystem.makeDirectory(dir, { recursive: true, mode: 0o700 }).pipe( + Effect.andThen(fileSystem.chmod(dir, 0o700)), + Effect.andThen(fileSystem.remove(partial, { force: true })), + // `wx` never follows or replaces an existing path. + Effect.andThen( + fileSystem.writeFile(partial, new TextEncoder().encode(value), { + flag: "wx", + mode: 0o600, + }), + ), + Effect.andThen(fileSystem.rename(partial, file)), + Effect.mapError((cause) => new SecretRequestError({ reason: "store_failed", cause })), + ); + }; + const saveToFile: SecretRequests["Service"]["saveToFile"] = Effect.fn( "SecretRequests.saveToFile", )(function* (input) { @@ -266,24 +290,22 @@ const make = Effect.gen(function* () { } // The stored value goes only after the file is complete, so a failed write can retry. const value = yield* readRef({ ref, projectId: records.thread.projectId }); - const partial = `${file}.partial`; - yield* fileSystem.makeDirectory(filesDir, { recursive: true, mode: 0o700 }).pipe( - Effect.andThen(fileSystem.chmod(filesDir, 0o700)), - Effect.andThen(fileSystem.remove(partial, { force: true })), - // `wx` never follows or replaces an existing path. - Effect.andThen( - fileSystem.writeFile(partial, new TextEncoder().encode(value), { - flag: "wx", - mode: 0o600, - }), - ), - Effect.andThen(fileSystem.rename(partial, file)), - Effect.mapError((cause) => new SecretRequestError({ reason: "store_failed", cause })), - ); + yield* writeSecretFile(filesDir, file, value); return yield* removeStored; }).pipe(consumeLock.withPermits(1)); }); + const savePasted: SecretRequests["Service"]["savePasted"] = Effect.fn( + "SecretRequests.savePasted", + )(function* (input) { + if (filesDir === undefined) return yield* new SecretRequestError({ reason: "store_failed" }); + // A retry with the same clientRequestId writes the same file. + const ref = refFor(salt, input.threadId, `pasted\u0000${input.requestKey}`); + const file = path.join(filesDir, ref.slice(SECRET_REF_PREFIX.length)); + yield* writeSecretFile(filesDir, file, input.value).pipe(consumeLock.withPermits(1)); + return file; + }); + const sweepExpired = Effect.gen(function* () { if (store.directory === undefined) return; const now = yield* Clock.currentTimeMillis; @@ -330,7 +352,7 @@ const make = Effect.gen(function* () { Effect.forkScoped, ); - return SecretRequests.of({ answer, savedRef, consume, saveToFile }); + return SecretRequests.of({ answer, savedRef, consume, saveToFile, savePasted }); }); export const layer = Layer.effect(SecretRequests, make); diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 1d6d7f3d67908..6756225666fd4 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -728,6 +728,11 @@ export const OrchestratorMcpUpdateScheduledTaskInput = Schema.Struct({ export type OrchestratorMcpUpdateScheduledTaskInput = typeof OrchestratorMcpUpdateScheduledTaskInput.Type; +// Not trimmed: whitespace in a password matters. The check stays out of the advertised schema. +const PastedSecretValue = Schema.String.check( + Schema.makeFilter((value: string) => value.length > 0, { expected: "a non-empty string" }), +); + export const OrchestratorMcpRequestSecretInput = Schema.Struct({ label: TrimmedNonEmptyString.annotate({ description: "Card title, such as 'GitHub webhook secret'.", @@ -737,14 +742,17 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ }), placeholder: Schema.optional(TrimmedNonEmptyString), delivery: Schema.optional(Schema.Literals(["secretRef", "file"])).annotate({ - description: - 'secretRef (default) for a T3 tool. file for a shell CLI: read the path in your command, e.g. TOKEN="$(cat )" wrangler whoami.', + description: "secretRef (default) for a T3 tool; file for a command.", }), + value: Schema.optional(PastedSecretValue).annotate({ + description: "Pasted in this thread; no card.", + }), + /** Default 10 minutes. */ timeoutMs: Schema.optional( Schema.Int.check(Schema.isBetween({ minimum: 1_000, maximum: 60 * 60 * 1_000 })), - ).annotate({ description: "Default 10 min." }), + ), clientRequestId: Schema.optional(OrchestratorMcpClientRequestId).annotate({ - description: "Retry key for this ask. New key after a timeout or cancel.", + description: "Retry key.", }), }); export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSecretInput.Type; @@ -766,8 +774,9 @@ export const OrchestratorMcpRequestSecretResult = Schema.Union([ Schema.Struct({ status: Schema.Literals(["declined", "cancelled", "timed_out"]).annotate({ description: - "declined: user chose not to. cancelled: ended with the run. timed_out: no answer in time. Ask again with a new `clientRequestId` if still needed.", + "declined: user chose not to; don't ask again. cancelled: ended with the run. timed_out: no answer in time. After cancelled or timed_out, ask again with a new `clientRequestId` only if still needed.", }), + note: Schema.optional(Schema.String).annotate({ description: "What to do next." }), }), ]); export type OrchestratorMcpRequestSecretResult = typeof OrchestratorMcpRequestSecretResult.Type; diff --git a/packages/contracts/src/secretRequest.ts b/packages/contracts/src/secretRequest.ts index 2506d064b533c..374592722eb1c 100644 --- a/packages/contracts/src/secretRequest.ts +++ b/packages/contracts/src/secretRequest.ts @@ -13,6 +13,15 @@ export const SecretRequestAnswerInput = Schema.Struct({ }); export type SecretRequestAnswerInput = typeof SecretRequestAnswerInput.Type; +/** What the agent reads when its card ends without a secret. */ +export const SECRET_REQUEST_UNSAVED_NOTE = { + declined: + "The user declined. Don't ask for it in chat or another way; say what's blocked and go on without it.", + cancelled: "The card closed with the run. Ask again only if the secret is still needed.", + timed_out: + "No answer in time. Don't ask in chat; ask again with a new clientRequestId only if it is still needed.", +} as const; + const SECRET_REQUEST_FAILURE_MESSAGES = { load_failed: "Could not load the secret request.", not_found: "This secret request no longer exists.", diff --git a/packages/provider-core/src/server/orchestrationInstructions.ts b/packages/provider-core/src/server/orchestrationInstructions.ts index 90ed18e9b3398..e36f04abf9828 100644 --- a/packages/provider-core/src/server/orchestrationInstructions.ts +++ b/packages/provider-core/src/server/orchestrationInstructions.ts @@ -16,7 +16,7 @@ Ordinary coding tasks need no T3 tools. - Another machine: \`t3_environment_links\` lists linked ones. Pass its \`environmentId\` to a T3 tool to act there. Ids are then that machine's, so pass \`threadId\`. - A machine the user names that is not linked ("delegate this to my vps"): call \`t3_environment_link\` with a \`hint\` from their words. Never ask for a pairing code in chat. - In your reply, mention a thread by its \`link\`, verbatim. Never a plain title or bare id. -- A secret: call \`request_secret\` yourself. Pass its \`secretRef\` to a T3 tool, or use \`delivery: "file"\` for a shell command. Never ask in chat or invent one. When asked only to schedule, \`schedule_task\` without running its prompt. +- A login, token, PIN or pairing code only the user can give: call \`request_secret\`, not a chat ask or a handoff. Not for sudo, OAuth or access you have. When asked only to schedule, \`schedule_task\` without running its prompt. - T3 tools may load late. Try the name once before you call it absent.`, `### Visuals and preview From 166fe4da39f78a5b13befe6010150831407b6a3c Mon Sep 17 00:00:00 2001 From: Gabriel De Andrade Date: Tue, 3 Nov 2026 12:56:11 -0400 Subject: [PATCH 2/5] fix(server): keep a pasted secret's file per session and unchanged on retry - savePasted derives the file from the provider session's request namespace as well as the clientRequestId, so two sessions in one thread never share a file. - A retry returns the file the first call wrote without rewriting it, so a command holding that path never reads another value. - The cancelled note says to ask again with a new clientRequestId. Co-Authored-By: Claude Opus 5.5 --- .../OrchestratorMcpService.activity.test.ts | 20 +++++++-- apps/server/src/mcp/OrchestratorMcpService.ts | 1 + .../server/src/secrets/SecretRequests.test.ts | 43 ++++++++++++++++--- apps/server/src/secrets/SecretRequests.ts | 18 ++++++-- packages/contracts/src/secretRequest.ts | 3 +- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index 2ab25d5c7dccc..67f430a9ce9dc 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -675,7 +675,11 @@ it("requestSecret saves a pasted value to a file without opening a card", async updatedAt: now, } as unknown as OrchestrationV2ThreadProjection; const dispatched: Array = []; - const saved: Array<{ readonly requestKey: string; readonly value: string }> = []; + const saved: Array<{ + readonly requestNamespace: string; + readonly requestKey: string; + readonly value: string; + }> = []; const file = "/home/user/.t3/userdata/secrets/files/0123456789abcdef0123456789abcdef"; const layer = layerOrchestratorService.pipe( @@ -702,7 +706,11 @@ it("requestSecret saves a pasted value to a file without opening a card", async Layer.mock(SecretRequests.SecretRequests)({ savePasted: (input) => Effect.sync(() => { - saved.push({ requestKey: input.requestKey, value: input.value }); + saved.push({ + requestNamespace: input.requestNamespace, + requestKey: input.requestKey, + value: input.value, + }); return file; }), }), @@ -735,7 +743,13 @@ it("requestSecret saves a pasted value to a file without opening a card", async }); expect(result).toEqual({ status: "saved", file }); expect(JSON.stringify(result)).not.toContain("482916"); - expect(saved).toEqual([{ requestKey: "pin-1", value: "482916" }]); + expect(saved).toEqual([ + { + requestNamespace: "provider-session-mcp-orchestrator-detail", + requestKey: "pin-1", + value: "482916", + }, + ]); expect(dispatched).toEqual([]); expect((yield* counted) - before).toBe(1); diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index c21305479aff0..e53ed03a7c6fa 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -2147,6 +2147,7 @@ const make = Effect.gen(function* () { const file = yield* secretRequests .savePasted({ threadId, + requestNamespace: scope.requestNamespace, requestKey: yield* requestKey(input.clientRequestId), value: input.value, }) diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index be24f356b5a9e..c362a1b0ed7e0 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -479,7 +479,12 @@ it.effect("a pasted secret goes straight to an owner-only file, with no card", ( const file = yield* withService( ({ service, stored, dispatched }) => Effect.gen(function* () { - const file = yield* service.savePasted({ threadId, requestKey: "key-1", value: pasted }); + const file = yield* service.savePasted({ + threadId, + requestNamespace: "session-a", + requestKey: "key-1", + value: pasted, + }); assert.equal(path.dirname(file), path.join(directory, "files")); assert.match(path.basename(file), /^[0-9a-f]{32}$/); assert.equal(yield* fs.readFileString(file), pasted); @@ -490,13 +495,33 @@ it.effect("a pasted secret goes straight to an owner-only file, with no card", ( // No card and no stored copy: the thread never hears of it. assert.deepEqual(dispatched, []); assert.deepEqual(valuesOf(stored), []); - // A retry with the same key writes the same file; another key gets its own. + // A retry with the same key gets the same file, unchanged even if the value differs. assert.equal( - yield* service.savePasted({ threadId, requestKey: "key-1", value: pasted }), + yield* service.savePasted({ + threadId, + requestNamespace: "session-a", + requestKey: "key-1", + value: "other", + }), file, ); - const other = yield* service.savePasted({ threadId, requestKey: "key-2", value: "x" }); + assert.equal(yield* fs.readFileString(file), pasted); + // Another key, or the same key from another provider session, gets its own file. + const other = yield* service.savePasted({ + threadId, + requestNamespace: "session-a", + requestKey: "key-2", + value: "x", + }); assert.notEqual(other, file); + const otherSession = yield* service.savePasted({ + threadId, + requestNamespace: "session-b", + requestKey: "key-1", + value: "y", + }); + assert.notEqual(otherSession, file); + assert.equal(yield* fs.readFileString(file), pasted); // Its name is not a ref to the value. const ref = SecretRef.make(`secret-ref:${path.basename(file)}`); const reused = yield* service.consume({ ref, projectId }).pipe(Effect.flip); @@ -540,7 +565,13 @@ it.effect("a pasted secret keeps its exact bytes from the tool input to the file }); assert.isTrue(Exit.isFailure(empty)); const file = yield* withService( - ({ service }) => service.savePasted({ threadId, requestKey: "key-1", value: input.value! }), + ({ service }) => + service.savePasted({ + threadId, + requestNamespace: "session-a", + requestKey: "key-1", + value: input.value!, + }), { directory }, ); assert.equal(yield* fs.readFileString(file), pasted); @@ -551,7 +582,7 @@ it.effect("a pasted secret is refused without a secrets directory", () => withService(({ service }) => Effect.gen(function* () { const failed = yield* service - .savePasted({ threadId, requestKey: "key-1", value: "4829" }) + .savePasted({ threadId, requestNamespace: "session-a", requestKey: "key-1", value: "4829" }) .pipe(Effect.flip); assert.equal(failed.reason, "store_failed"); }), diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index 3b2e067b6c123..4d713f9bf7ee6 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -70,6 +70,8 @@ export class SecretRequests extends Context.Service< /** Writes a secret the user pasted in chat to an owner-only file and returns its path. */ readonly savePasted: (input: { readonly threadId: ThreadId; + /** The provider session's namespace, so two sessions' keys never share a file. */ + readonly requestNamespace: string; readonly requestKey: string; readonly value: string; }) => Effect.Effect; @@ -299,11 +301,19 @@ const make = Effect.gen(function* () { "SecretRequests.savePasted", )(function* (input) { if (filesDir === undefined) return yield* new SecretRequestError({ reason: "store_failed" }); - // A retry with the same clientRequestId writes the same file. - const ref = refFor(salt, input.threadId, `pasted\u0000${input.requestKey}`); + const ref = refFor( + salt, + input.threadId, + `pasted\u0000${input.requestNamespace}\u0000${input.requestKey}`, + ); const file = path.join(filesDir, ref.slice(SECRET_REF_PREFIX.length)); - yield* writeSecretFile(filesDir, file, input.value).pipe(consumeLock.withPermits(1)); - return file; + // A retry with the same clientRequestId gets the file the first call wrote, unchanged, so a + // command already holding that path never reads another value. + return yield* Effect.gen(function* () { + if (yield* fileSystem.exists(file).pipe(Effect.orElseSucceed(() => false))) return file; + yield* writeSecretFile(filesDir, file, input.value); + return file; + }).pipe(consumeLock.withPermits(1)); }); const sweepExpired = Effect.gen(function* () { diff --git a/packages/contracts/src/secretRequest.ts b/packages/contracts/src/secretRequest.ts index 374592722eb1c..b449a17643d40 100644 --- a/packages/contracts/src/secretRequest.ts +++ b/packages/contracts/src/secretRequest.ts @@ -17,7 +17,8 @@ export type SecretRequestAnswerInput = typeof SecretRequestAnswerInput.Type; export const SECRET_REQUEST_UNSAVED_NOTE = { declined: "The user declined. Don't ask for it in chat or another way; say what's blocked and go on without it.", - cancelled: "The card closed with the run. Ask again only if the secret is still needed.", + cancelled: + "The card closed with the run. Ask again with a new clientRequestId only if the secret is still needed.", timed_out: "No answer in time. Don't ask in chat; ask again with a new clientRequestId only if it is still needed.", } as const; From bcab2e1e0bec70907ea639699f6517e3527d2a80 Mon Sep 17 00:00:00 2001 From: Gabriel De Andrade Date: Tue, 3 Nov 2026 12:59:17 -0400 Subject: [PATCH 3/5] test(server): time out the unsaved-note card on a test clock Co-Authored-By: Claude Opus 5.5 --- .../OrchestratorMcpService.activity.test.ts | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index 67f430a9ce9dc..c35b605dca4fb 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -14,8 +14,10 @@ import { } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Metric from "effect/Metric"; +import * as TestClock from "effect/testing/TestClock"; import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import { expect, it } from "vite-plus/test"; @@ -820,13 +822,17 @@ it("requestSecret returns the unsaved note for a declined, cancelled or timed-ou ); return Effect.gen(function* () { const service = yield* OrchestratorMcpService.OrchestratorMcpService; - return yield* service.requestSecret(makeScope(), { - label: "Staging password", - reason: "Signs in the QA account.", - delivery: "file", - timeoutMs: 1_000, - }); - }).pipe(Effect.provide(layer)); + const request = yield* service + .requestSecret(makeScope(), { + label: "Staging password", + reason: "Signs in the QA account.", + delivery: "file", + timeoutMs: 1_000, + }) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust("2 seconds"); + return yield* Fiber.join(request); + }).pipe(Effect.provide(Layer.merge(layer, TestClock.layer()))); }).pipe(Effect.runPromise); expect(results).toEqual([ From 03afb6596c1a13d51adf25b5362f089674c042f2 Mon Sep 17 00:00:00 2001 From: Gabriel De Andrade Date: Tue, 3 Nov 2026 20:21:23 -0400 Subject: [PATCH 4/5] fix(server): let the model choose a card for sudo and OAuth, and keep old-thread secrets out Drop "sudo, OAuth" from the request_secret instruction line, so whether a card fits a root password or a missing scope is the model's call. Narrow "Never reuse old ones" to "Not one from an old thread", which keeps transcript recovery at 6/6 on both GPT models (2/6 without it). Co-Authored-By: Claude Opus 5.5 --- apps/server/src/mcp/toolkits/orchestrator/tools.ts | 2 +- apps/server/src/provider/agentContextBudget.json | 6 +++--- .../provider-core/src/server/orchestrationInstructions.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 6016622936fd2..6995f5f6608fe 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -197,7 +197,7 @@ const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { const RequestSecretTool = Tool.make("request_secret", { description: - 'Ask for a secret in a private card and wait; open it, don\'t offer it. Returns a one-use `secretRef`, or with `delivery: "file"` a file to read only in its command, e.g. `adb pair "$(cat )"`. Asked how to give one: call now. Pasted secret for a child or script: `value`. Never reuse old ones.', + 'Ask for a secret in a private card and wait; open it, don\'t offer it. Returns a one-use `secretRef`, or with `delivery: "file"` a file to read only in its command, e.g. `adb pair "$(cat )"`. Asked how to give one: call now. Pasted secret for child or script: `value`. Not one from an old thread.', parameters: OrchestratorMcpRequestSecretInput, success: OrchestratorMcpRequestSecretResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index bff541d7ba947..d8360f1a1ec9d 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { - "claudeSystemAppendBytes": 5148, - "codexAdditionalContextBytes": 6878, - "mcpToolListJsonBytes": 47534 + "claudeSystemAppendBytes": 5133, + "codexAdditionalContextBytes": 6863, + "mcpToolListJsonBytes": 47538 } diff --git a/packages/provider-core/src/server/orchestrationInstructions.ts b/packages/provider-core/src/server/orchestrationInstructions.ts index e36f04abf9828..efe3fc242d9a5 100644 --- a/packages/provider-core/src/server/orchestrationInstructions.ts +++ b/packages/provider-core/src/server/orchestrationInstructions.ts @@ -16,7 +16,7 @@ Ordinary coding tasks need no T3 tools. - Another machine: \`t3_environment_links\` lists linked ones. Pass its \`environmentId\` to a T3 tool to act there. Ids are then that machine's, so pass \`threadId\`. - A machine the user names that is not linked ("delegate this to my vps"): call \`t3_environment_link\` with a \`hint\` from their words. Never ask for a pairing code in chat. - In your reply, mention a thread by its \`link\`, verbatim. Never a plain title or bare id. -- A login, token, PIN or pairing code only the user can give: call \`request_secret\`, not a chat ask or a handoff. Not for sudo, OAuth or access you have. When asked only to schedule, \`schedule_task\` without running its prompt. +- A login, token, PIN or pairing code only the user can give: call \`request_secret\`, not a chat ask or a handoff. Not for access you have. When asked only to schedule, \`schedule_task\` without running its prompt. - T3 tools may load late. Try the name once before you call it absent.`, `### Visuals and preview From 2c52acf685081eceaae1f26ff2b874ff6ec0764d Mon Sep 17 00:00:00 2001 From: Gabriel De Andrade Date: Wed, 4 Nov 2026 02:54:42 -0400 Subject: [PATCH 5/5] fix(server): shorten delegate_task's secret rule to "No secrets in `task`." request_secret's description already routes a pasted secret for a child to `value`, so delegate_task keeps only the rule. pasted-pin-delegation stays 6/6 on GPT-6.1 Sol and GPT-6 Astra, and the tool list is 40 bytes under main. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/mcp/toolkits/orchestrator/tools.ts | 2 +- apps/server/src/provider/agentContextBudget.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 6995f5f6608fe..150962e232a13 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -85,7 +85,7 @@ const OrchestratorCapabilitiesTool = Tool.make("orchestrator_capabilities", { const DelegateTaskTool = Tool.make("delegate_task", { description: - "Start one child agent with only `task`. No parent history. Separate checkout or PR per child: `isolation:'worktree'`. Keep `taskId` for `task_status` and `task_cancel`. No secrets in `task` or temp files: `request_secret` `value`.", + "Start one child agent with only `task`. No parent history. Separate checkout or PR per child: `isolation:'worktree'`. Keep `taskId` for `task_status` and `task_cancel`. No secrets in `task`.", parameters: OrchestratorMcpDelegateTaskInput, success: OrchestratorMcpDelegateTaskResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index d8360f1a1ec9d..bb586d5b9f4b4 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { "claudeSystemAppendBytes": 5133, "codexAdditionalContextBytes": 6863, - "mcpToolListJsonBytes": 47538 + "mcpToolListJsonBytes": 47498 }