| author | |
| committer | |
| log | cd096b9ad5a4156ffeab85de617cbb219057007f |
| tree | 82a90217847ba32e7303e3130687c2af167493e2 |
| parent | 13458e65106005ca183c02b5c84f9355b67feadb |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>9 files changed, 293 insertions(+), 25 deletions(-)
apps/server/src/provider/CodexDeveloperInstructions.ts+26-6| ... | @@ -11,7 +11,19 @@ For browser work, first call \`preview_status\`. If no automation-capable previe | ... | @@ -11,7 +11,19 @@ For browser work, first call \`preview_status\`. If no automation-capable previe |
| 11 | Do not switch to global browser skills, Chrome, Node REPL browser automation, standalone Playwright, or agent-browser merely because the preview is initially closed or a first call fails. Use an alternative browser system only when the T3 preview tools are absent, the user explicitly requests another browser, or \`preview_open\` returns an explicit unsupported/unavailable error. A failed T3 preview tool call should be inspected and retried with corrected arguments when the error is actionable. | 11 | Do not switch to global browser skills, Chrome, Node REPL browser automation, standalone Playwright, or agent-browser merely because the preview is initially closed or a first call fails. Use an alternative browser system only when the T3 preview tools are absent, the user explicitly requests another browser, or \`preview_open\` returns an explicit unsupported/unavailable error. A failed T3 preview tool call should be inspected and retried with corrected arguments when the error is actionable. |
| 12 | `; | 12 | `; |
| 13 | 13 | ||
| 14 | export const CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS = `<collaboration_mode># Plan Mode (Conversational) | 14 | /** |
| 15 | * The browser block is omitted entirely when the preview tools aren't attached. | ||
| 16 | * Describing `preview_*` tools that aren't in the turn's tool list would be | ||
| 17 | * worse than saying nothing: the instructions actively steer the model away | ||
| 18 | * from Playwright and agent-browser, so leaving them in would talk it out of | ||
| 19 | * the only browser automation it still has. | ||
| 20 | */ | ||
| 21 | const browserToolInstructions = (browserToolsAvailable: boolean): string => | ||
| 22 | browserToolsAvailable ? T3_CODE_BROWSER_TOOL_INSTRUCTIONS : ""; | ||
| 23 | |||
| 24 | export const codexPlanModeDeveloperInstructions = ( | ||
| 25 | browserToolsAvailable: boolean, | ||
| 26 | ): string => `<collaboration_mode># Plan Mode (Conversational) | ||
| 15 | 27 | ||
| 16 | You work in 3 phases, and you should *chat your way* to a great plan before finalizing it. A great plan is very detailed-intent- and implementation-wise-so that it can be handed to another engineer or agent to be implemented right away. It must be **decision complete**, where the implementer does not need to make any decisions. | 28 | You work in 3 phases, and you should *chat your way* to a great plan before finalizing it. A great plan is very detailed-intent- and implementation-wise-so that it can be handed to another engineer or agent to be implemented right away. It must be **decision complete**, where the implementer does not need to make any decisions. |
| 17 | 29 | ||
| ... | @@ -139,10 +151,12 @@ Do not ask "should I proceed?" in the final output. The user can easily switch o | ... | @@ -139,10 +151,12 @@ Do not ask "should I proceed?" in the final output. The user can easily switch o |
| 139 | Only produce at most one \`<proposed_plan>\` block per turn, and only when you are presenting a complete spec. | 151 | Only produce at most one \`<proposed_plan>\` block per turn, and only when you are presenting a complete spec. |
| 140 | 152 | ||
| 141 | If the user stays in Plan mode and asks for revisions after a prior \`<proposed_plan>\`, any new \`<proposed_plan>\` must be a complete replacement. If the user indicates that the prior plan is not acceptable but does not provide enough information to produce a complete replacement, address the concern and continue planning without producing a \`<proposed_plan>\` block. If the follow-up neither requires changes nor calls the plan into question (e.g. clarifying question), answer it before the block, then reproduce the prior \`<proposed_plan>\` unchanged. | 153 | If the user stays in Plan mode and asks for revisions after a prior \`<proposed_plan>\`, any new \`<proposed_plan>\` must be a complete replacement. If the user indicates that the prior plan is not acceptable but does not provide enough information to produce a complete replacement, address the concern and continue planning without producing a \`<proposed_plan>\` block. If the follow-up neither requires changes nor calls the plan into question (e.g. clarifying question), answer it before the block, then reproduce the prior \`<proposed_plan>\` unchanged. |
| 142 | ${T3_CODE_BROWSER_TOOL_INSTRUCTIONS} | 154 | ${browserToolInstructions(browserToolsAvailable)} |
| 143 | </collaboration_mode>`; | 155 | </collaboration_mode>`; |
| 144 | 156 | ||
| 145 | export const CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS = `<collaboration_mode># Collaboration Mode: Default | 157 | export const codexDefaultModeDeveloperInstructions = ( |
| 158 | browserToolsAvailable: boolean, | ||
| 159 | ): string => `<collaboration_mode># Collaboration Mode: Default | ||
| 146 | 160 | ||
| 147 | You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. | 161 | You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. |
| 148 | 162 | ||
| ... | @@ -153,7 +167,7 @@ Your active mode changes only when new developer instructions with a different \ | ... | @@ -153,7 +167,7 @@ Your active mode changes only when new developer instructions with a different \ |
| 153 | Use the \`request_user_input\` tool only when it is listed in the available tools for this turn. | 167 | Use the \`request_user_input\` tool only when it is listed in the available tools for this turn. |
| 154 | 168 | ||
| 155 | In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, ask the user directly with a concise plain-text question. Never write a multiple choice question as a textual assistant message. | 169 | In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, ask the user directly with a concise plain-text question. Never write a multiple choice question as a textual assistant message. |
| 156 | ${T3_CODE_BROWSER_TOOL_INSTRUCTIONS} | 170 | ${browserToolInstructions(browserToolsAvailable)} |
| 157 | </collaboration_mode>`; | 171 | </collaboration_mode>`; |
| 158 | 172 | ||
| 159 | export interface CodexRuntimeInfo { | 173 | export interface CodexRuntimeInfo { |
| ... | @@ -169,11 +183,17 @@ function toSingleLine(value: string): string { | ... | @@ -169,11 +183,17 @@ function toSingleLine(value: string): string { |
| 169 | export function buildCodexDeveloperInstructions( | 183 | export function buildCodexDeveloperInstructions( |
| 170 | interactionMode: ProviderInteractionMode, | 184 | interactionMode: ProviderInteractionMode, |
| 171 | runtime: CodexRuntimeInfo, | 185 | runtime: CodexRuntimeInfo, |
| 186 | /** | ||
| 187 | * Whether the `t3-code` MCP server is attached to this turn. Callers derive | ||
| 188 | * it from the session's actual MCP configuration rather than re-reading the | ||
| 189 | * setting, so the prompt cannot claim tools the turn doesn't have. | ||
| 190 | */ | ||
| 191 | browserToolsAvailable = true, | ||
| 172 | ): string { | 192 | ): string { |
| 173 | const base = | 193 | const base = |
| 174 | interactionMode === "plan" | 194 | interactionMode === "plan" |
| 175 | ? CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS | 195 | ? codexPlanModeDeveloperInstructions(browserToolsAvailable) |
| 176 | : CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS; | 196 | : codexDefaultModeDeveloperInstructions(browserToolsAvailable); |
| 177 | return `${base} | 197 | return `${base} |
| 178 | 198 | ||
| 179 | <runtime_info>In case you're asked: you are running in T3 Code through the Codex harness, as ${toSingleLine(runtime.model)} with ${toSingleLine(runtime.reasoningEffort)} reasoning effort. No need to mention this otherwise.</runtime_info>`; | 199 | <runtime_info>In case you're asked: you are running in T3 Code through the Codex harness, as ${toSingleLine(runtime.model)} with ${toSingleLine(runtime.reasoningEffort)} reasoning effort. No need to mention this otherwise.</runtime_info>`; |
apps/server/src/provider/Layers/CodexSessionRuntime.test.ts+32-6| ... | @@ -11,8 +11,8 @@ import * as EffectCodexSchema from "effect-codex-app-server/schema"; | ... | @@ -11,8 +11,8 @@ import * as EffectCodexSchema from "effect-codex-app-server/schema"; |
| 11 | 11 | ||
| 12 | import { | 12 | import { |
| 13 | buildCodexDeveloperInstructions, | 13 | buildCodexDeveloperInstructions, |
| 14 | CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS, | 14 | codexDefaultModeDeveloperInstructions, |
| 15 | CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS, | 15 | codexPlanModeDeveloperInstructions, |
| 16 | } from "../CodexDeveloperInstructions.ts"; | 16 | } from "../CodexDeveloperInstructions.ts"; |
| 17 | import { codexSessionAppServerArgs } from "./codexLaunchArgs.ts"; | 17 | import { codexSessionAppServerArgs } from "./codexLaunchArgs.ts"; |
| 18 | import { | 18 | import { |
| ... | @@ -255,7 +255,7 @@ describe("buildCodexDeveloperInstructions", () => { | ... | @@ -255,7 +255,7 @@ describe("buildCodexDeveloperInstructions", () => { |
| 255 | reasoningEffort: "high", | 255 | reasoningEffort: "high", |
| 256 | }); | 256 | }); |
| 257 | 257 | ||
| 258 | NodeAssert.ok(instructions.startsWith(CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS)); | 258 | NodeAssert.ok(instructions.startsWith(codexDefaultModeDeveloperInstructions(true))); |
| 259 | NodeAssert.match(instructions, /T3 Code/); | 259 | NodeAssert.match(instructions, /T3 Code/); |
| 260 | NodeAssert.match(instructions, /Codex harness/); | 260 | NodeAssert.match(instructions, /Codex harness/); |
| 261 | NodeAssert.match(instructions, /as gpt-5\.3-codex with high reasoning effort/); | 261 | NodeAssert.match(instructions, /as gpt-5\.3-codex with high reasoning effort/); |
| ... | @@ -267,7 +267,7 @@ describe("buildCodexDeveloperInstructions", () => { | ... | @@ -267,7 +267,7 @@ describe("buildCodexDeveloperInstructions", () => { |
| 267 | reasoningEffort: "medium", | 267 | reasoningEffort: "medium", |
| 268 | }); | 268 | }); |
| 269 | 269 | ||
| 270 | NodeAssert.ok(instructions.startsWith(CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS)); | 270 | NodeAssert.ok(instructions.startsWith(codexPlanModeDeveloperInstructions(true))); |
| 271 | NodeAssert.match(instructions, /as gpt-5\.3-codex with medium reasoning effort/); | 271 | NodeAssert.match(instructions, /as gpt-5\.3-codex with medium reasoning effort/); |
| 272 | }); | 272 | }); |
| 273 | 273 | ||
| ... | @@ -298,8 +298,8 @@ describe("buildCodexDeveloperInstructions", () => { | ... | @@ -298,8 +298,8 @@ describe("buildCodexDeveloperInstructions", () => { |
| 298 | describe("T3 browser developer instructions", () => { | 298 | describe("T3 browser developer instructions", () => { |
| 299 | it("prefers the product-native preview tools in both collaboration modes", () => { | 299 | it("prefers the product-native preview tools in both collaboration modes", () => { |
| 300 | for (const instructions of [ | 300 | for (const instructions of [ |
| 301 | CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS, | 301 | codexDefaultModeDeveloperInstructions(true), |
| 302 | CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS, | 302 | codexPlanModeDeveloperInstructions(true), |
| 303 | ]) { | 303 | ]) { |
| 304 | NodeAssert.match(instructions, /t3-code/); | 304 | NodeAssert.match(instructions, /t3-code/); |
| 305 | NodeAssert.match(instructions, /preview_status/); | 305 | NodeAssert.match(instructions, /preview_status/); |
| ... | @@ -307,6 +307,32 @@ describe("T3 browser developer instructions", () => { | ... | @@ -307,6 +307,32 @@ describe("T3 browser developer instructions", () => { |
| 307 | NodeAssert.match(instructions, /Do not switch to global browser skills/); | 307 | NodeAssert.match(instructions, /Do not switch to global browser skills/); |
| 308 | } | 308 | } |
| 309 | }); | 309 | }); |
| 310 | |||
| 311 | it("omits the browser block entirely when the preview tools are not attached", () => { | ||
| 312 | for (const instructions of [ | ||
| 313 | codexDefaultModeDeveloperInstructions(false), | ||
| 314 | codexPlanModeDeveloperInstructions(false), | ||
| 315 | ]) { | ||
| 316 | NodeAssert.doesNotMatch(instructions, /preview_status/); | ||
| 317 | NodeAssert.doesNotMatch(instructions, /preview_open/); | ||
| 318 | NodeAssert.doesNotMatch(instructions, /T3 Code collaborative browser/); | ||
| 319 | // Steering away from other browser automation must go with the tools; | ||
| 320 | // keeping it would leave the model talked out of its only option. | ||
| 321 | NodeAssert.doesNotMatch(instructions, /Do not switch to global browser skills/); | ||
| 322 | // The rest of the collaboration mode is untouched. | ||
| 323 | NodeAssert.match(instructions, /<collaboration_mode>/); | ||
| 324 | NodeAssert.match(instructions, /<\/collaboration_mode>/); | ||
| 325 | } | ||
| 326 | }); | ||
| 327 | |||
| 328 | it("tracks the turn's MCP configuration rather than defaulting to on", () => { | ||
| 329 | const runtime = { model: "gpt-5.3-codex", reasoningEffort: "high" }; | ||
| 330 | NodeAssert.match(buildCodexDeveloperInstructions("default", runtime, true), /preview_open/); | ||
| 331 | NodeAssert.doesNotMatch( | ||
| 332 | buildCodexDeveloperInstructions("default", runtime, false), | ||
| 333 | /preview_open/, | ||
| 334 | ); | ||
| 335 | }); | ||
| 310 | }); | 336 | }); |
| 311 | 337 | ||
| 312 | describe("hasConfiguredMcpServer", () => { | 338 | describe("hasConfiguredMcpServer", () => { |
apps/server/src/provider/Layers/CodexSessionRuntime.ts+13-4| ... | @@ -340,6 +340,7 @@ function buildCodexCollaborationMode(input: { | ... | @@ -340,6 +340,7 @@ function buildCodexCollaborationMode(input: { |
| 340 | readonly interactionMode?: ProviderInteractionMode; | 340 | readonly interactionMode?: ProviderInteractionMode; |
| 341 | readonly model?: string; | 341 | readonly model?: string; |
| 342 | readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; | 342 | readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; |
| 343 | readonly browserToolsAvailable?: boolean; | ||
| 343 | }): EffectCodexSchema.V2TurnStartParams__CollaborationMode | undefined { | 344 | }): EffectCodexSchema.V2TurnStartParams__CollaborationMode | undefined { |
| 344 | if (input.interactionMode === undefined) { | 345 | if (input.interactionMode === undefined) { |
| 345 | return undefined; | 346 | return undefined; |
| ... | @@ -351,10 +352,11 @@ function buildCodexCollaborationMode(input: { | ... | @@ -351,10 +352,11 @@ function buildCodexCollaborationMode(input: { |
| 351 | settings: { | 352 | settings: { |
| 352 | model, | 353 | model, |
| 353 | reasoning_effort: reasoningEffort, | 354 | reasoning_effort: reasoningEffort, |
| 354 | developer_instructions: buildCodexDeveloperInstructions(input.interactionMode, { | 355 | developer_instructions: buildCodexDeveloperInstructions( |
| 355 | model, | 356 | input.interactionMode, |
| 356 | reasoningEffort, | 357 | { model, reasoningEffort }, |
| 357 | }), | 358 | input.browserToolsAvailable ?? true, |
| 359 | ), | ||
| 358 | }, | 360 | }, |
| 359 | }; | 361 | }; |
| 360 | } | 362 | } |
| ... | @@ -371,6 +373,8 @@ export function buildTurnStartParams(input: { | ... | @@ -371,6 +373,8 @@ export function buildTurnStartParams(input: { |
| 371 | readonly serviceTier?: CodexServiceTier; | 373 | readonly serviceTier?: CodexServiceTier; |
| 372 | readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; | 374 | readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; |
| 373 | readonly interactionMode?: ProviderInteractionMode; | 375 | readonly interactionMode?: ProviderInteractionMode; |
| 376 | /** Defaults to true so callers that predate the agent-access gate are unchanged. */ | ||
| 377 | readonly browserToolsAvailable?: boolean; | ||
| 374 | }): Effect.Effect< | 378 | }): Effect.Effect< |
| 375 | CodexTurnStartParamsWithCollaborationMode, | 379 | CodexTurnStartParamsWithCollaborationMode, |
| 376 | CodexErrors.CodexAppServerProtocolParseError | 380 | CodexErrors.CodexAppServerProtocolParseError |
| ... | @@ -391,6 +395,7 @@ export function buildTurnStartParams(input: { | ... | @@ -391,6 +395,7 @@ export function buildTurnStartParams(input: { |
| 391 | ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), | 395 | ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), |
| 392 | ...(input.model ? { model: input.model } : {}), | 396 | ...(input.model ? { model: input.model } : {}), |
| 393 | ...(input.effort ? { effort: input.effort } : {}), | 397 | ...(input.effort ? { effort: input.effort } : {}), |
| 398 | browserToolsAvailable: input.browserToolsAvailable ?? true, | ||
| 394 | }); | 399 | }); |
| 395 | 400 | ||
| 396 | return decodeCodexTurnStartParamsWithCollaborationMode({ | 401 | return decodeCodexTurnStartParamsWithCollaborationMode({ |
| ... | @@ -1822,6 +1827,10 @@ export const makeCodexSessionRuntime = ( | ... | @@ -1822,6 +1827,10 @@ export const makeCodexSessionRuntime = ( |
| 1822 | ...(input.serviceTier ? { serviceTier: input.serviceTier } : {}), | 1827 | ...(input.serviceTier ? { serviceTier: input.serviceTier } : {}), |
| 1823 | ...(input.effort ? { effort: input.effort } : {}), | 1828 | ...(input.effort ? { effort: input.effort } : {}), |
| 1824 | ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), | 1829 | ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), |
| 1830 | // Derived from the session's own MCP configuration rather than the | ||
| 1831 | // setting, so the prompt describes the tools this turn actually | ||
| 1832 | // has even if the setting changed after the session started. | ||
| 1833 | browserToolsAvailable: hasConfiguredMcpServer(options.appServerArgs), | ||
| 1825 | }); | 1834 | }); |
| 1826 | const rawResponse = yield* client.raw.request("turn/start", params); | 1835 | const rawResponse = yield* client.raw.request("turn/start", params); |
| 1827 | const response = yield* decodeV2TurnStartResponse(rawResponse).pipe( | 1836 | const response = yield* decodeV2TurnStartResponse(rawResponse).pipe( |
apps/server/src/provider/Layers/ProviderService.test.ts+89-1| ... | @@ -12,6 +12,7 @@ import type { | ... | @@ -12,6 +12,7 @@ import type { |
| 12 | } from "@t3tools/contracts"; | 12 | } from "@t3tools/contracts"; |
| 13 | import { | 13 | import { |
| 14 | ApprovalRequestId, | 14 | ApprovalRequestId, |
| 15 | EnvironmentId, | ||
| 15 | EventId, | 16 | EventId, |
| 16 | ProviderDriverKind, | 17 | ProviderDriverKind, |
| 17 | ProviderInstanceId, | 18 | ProviderInstanceId, |
| ... | @@ -20,7 +21,7 @@ import { | ... | @@ -20,7 +21,7 @@ import { |
| 20 | TurnId, | 21 | TurnId, |
| 21 | } from "@t3tools/contracts"; | 22 | } from "@t3tools/contracts"; |
| 22 | import { createModelSelection } from "@t3tools/shared/model"; | 23 | import { createModelSelection } from "@t3tools/shared/model"; |
| 23 | import { it, assert, vi } from "@effect/vitest"; | 24 | import { it, assert, describe, vi } from "@effect/vitest"; |
| 24 | 25 | ||
| 25 | import * as Effect from "effect/Effect"; | 26 | import * as Effect from "effect/Effect"; |
| 26 | import * as Exit from "effect/Exit"; | 27 | import * as Exit from "effect/Exit"; |
| ... | @@ -1957,3 +1958,90 @@ validation.layer("ProviderServiceLive validation", (it) => { | ... | @@ -1957,3 +1958,90 @@ validation.layer("ProviderServiceLive validation", (it) => { |
| 1957 | }), | 1958 | }), |
| 1958 | ); | 1959 | ); |
| 1959 | }); | 1960 | }); |
| 1961 | |||
| 1962 | describe("agent browser access", () => { | ||
| 1963 | const revokedThreads: Array<ThreadId> = []; | ||
| 1964 | |||
| 1965 | const startSessionWith = (enableAgentBrowserAccess: boolean, threadId: ThreadId) => | ||
| 1966 | Effect.gen(function* () { | ||
| 1967 | const issued: Array<ThreadId> = []; | ||
| 1968 | const codex = makeFakeCodexAdapter(); | ||
| 1969 | const providerAdapterLayer = Layer.succeed( | ||
| 1970 | ProviderAdapterRegistry.ProviderAdapterRegistry, | ||
| 1971 | makeAdapterRegistryMock({ [CODEX_DRIVER]: codex.adapter }), | ||
| 1972 | ); | ||
| 1973 | const runtimeRepositoryLayer = ProviderSessionRuntime.layer.pipe( | ||
| 1974 | Layer.provide(SqlitePersistenceMemory), | ||
| 1975 | ); | ||
| 1976 | const directoryLayer = ProviderSessionDirectoryLive.pipe( | ||
| 1977 | Layer.provide(runtimeRepositoryLayer), | ||
| 1978 | ); | ||
| 1979 | const providerLayer = makeProviderServiceLive({ | ||
| 1980 | issueMcpCredential: (request) => | ||
| 1981 | Effect.sync(() => { | ||
| 1982 | issued.push(request.threadId); | ||
| 1983 | return undefined; | ||
| 1984 | }), | ||
| 1985 | revokeMcpCredential: (revoked) => Effect.sync(() => void revokedThreads.push(revoked)), | ||
| 1986 | }).pipe( | ||
| 1987 | Layer.provide(providerAdapterLayer), | ||
| 1988 | Layer.provide(directoryLayer), | ||
| 1989 | Layer.provide(ServerSettings.ServerSettingsService.layerTest({ enableAgentBrowserAccess })), | ||
| 1990 | Layer.provide(serverConfigTestLayer), | ||
| 1991 | Layer.provide(AnalyticsService.layerTest), | ||
| 1992 | Layer.provide( | ||
| 1993 | Layer.succeed( | ||
| 1994 | ProviderEventLoggers.ProviderEventLoggers, | ||
| 1995 | ProviderEventLoggers.NoOpProviderEventLoggers, | ||
| 1996 | ), | ||
| 1997 | ), | ||
| 1998 | ); | ||
| 1999 | |||
| 2000 | yield* Effect.gen(function* () { | ||
| 2001 | const provider = yield* ProviderService.ProviderService; | ||
| 2002 | return yield* provider.startSession(threadId, { | ||
| 2003 | provider: CODEX_DRIVER, | ||
| 2004 | providerInstanceId: codexInstanceId, | ||
| 2005 | threadId, | ||
| 2006 | runtimeMode: "full-access", | ||
| 2007 | }); | ||
| 2008 | }).pipe(Effect.provide(providerLayer)); | ||
| 2009 | |||
| 2010 | return issued; | ||
| 2011 | }); | ||
| 2012 | |||
| 2013 | // Credential issuance is the observable that matters: it is the only place a | ||
| 2014 | // credential is minted, and `/mcp` accepts nothing else, so withholding it is | ||
| 2015 | // what actually denies every provider and external MCP client. | ||
| 2016 | it.effect("requests no MCP credential when agent browser access is off", () => | ||
| 2017 | Effect.gen(function* () { | ||
| 2018 | const issued = yield* startSessionWith(false, asThreadId("thread-browser-off")); | ||
| 2019 | |||
| 2020 | assert.deepEqual(issued, []); | ||
| 2021 | }).pipe(Effect.provide(NodeServices.layer)), | ||
| 2022 | ); | ||
| 2023 | |||
| 2024 | it.effect("revokes an already-issued credential when access is off", () => | ||
| 2025 | Effect.gen(function* () { | ||
| 2026 | const threadId = asThreadId("thread-browser-revoke"); | ||
| 2027 | revokedThreads.length = 0; | ||
| 2028 | |||
| 2029 | yield* startSessionWith(false, threadId); | ||
| 2030 | |||
| 2031 | // Clearing the in-memory map is not enough: a token issued before the | ||
| 2032 | // toggle flipped stays valid against `/mcp` for its whole liveness | ||
| 2033 | // window, and later turns refresh it. | ||
| 2034 | assert.deepEqual(revokedThreads, [threadId]); | ||
| 2035 | }).pipe(Effect.provide(NodeServices.layer)), | ||
| 2036 | ); | ||
| 2037 | |||
| 2038 | it.effect("requests an MCP credential when agent browser access is on", () => | ||
| 2039 | Effect.gen(function* () { | ||
| 2040 | const threadId = asThreadId("thread-browser-on"); | ||
| 2041 | |||
| 2042 | const issued = yield* startSessionWith(true, threadId); | ||
| 2043 | |||
| 2044 | assert.deepEqual(issued, [threadId]); | ||
| 2045 | }).pipe(Effect.provide(NodeServices.layer)), | ||
| 2046 | ); | ||
| 2047 | }); |
apps/server/src/provider/Layers/ProviderService.ts+59-7| ... | @@ -57,6 +57,7 @@ import * as ProviderEventLoggers from "./ProviderEventLoggers.ts"; | ... | @@ -57,6 +57,7 @@ import * as ProviderEventLoggers from "./ProviderEventLoggers.ts"; |
| 57 | import * as AnalyticsService from "../../telemetry/AnalyticsService.ts"; | 57 | import * as AnalyticsService from "../../telemetry/AnalyticsService.ts"; |
| 58 | import * as McpProviderSession from "../../mcp/McpProviderSession.ts"; | 58 | import * as McpProviderSession from "../../mcp/McpProviderSession.ts"; |
| 59 | import * as McpSessionRegistry from "../../mcp/McpSessionRegistry.ts"; | 59 | import * as McpSessionRegistry from "../../mcp/McpSessionRegistry.ts"; |
| 60 | import * as ServerSettings from "../../serverSettings.ts"; | ||
| 60 | const isModelSelection = Schema.is(ModelSelection); | 61 | const isModelSelection = Schema.is(ModelSelection); |
| 61 | 62 | ||
| 62 | /** | 63 | /** |
| ... | @@ -66,6 +67,15 @@ const isModelSelection = Schema.is(ModelSelection); | ... | @@ -66,6 +67,15 @@ const isModelSelection = Schema.is(ModelSelection); |
| 66 | */ | 67 | */ |
| 67 | export interface ProviderServiceLiveOptions { | 68 | export interface ProviderServiceLiveOptions { |
| 68 | readonly canonicalEventLogger?: EventNdjsonLogger; | 69 | readonly canonicalEventLogger?: EventNdjsonLogger; |
| 70 | /** | ||
| 71 | * Overrides MCP credential issuance. The real issuer reads a module-global | ||
| 72 | * registry that only a running MCP server installs, which makes the | ||
| 73 | * agent-browser-access gate unobservable from a unit test; this seam lets a | ||
| 74 | * test see whether a credential was requested at all. | ||
| 75 | */ | ||
| 76 | readonly issueMcpCredential?: typeof McpSessionRegistry.issueActiveMcpCredential; | ||
| 77 | /** Same seam as `issueMcpCredential`, for observing the deny path's revoke. */ | ||
| 78 | readonly revokeMcpCredential?: typeof McpSessionRegistry.revokeActiveMcpThread; | ||
| 69 | } | 79 | } |
| 70 | 80 | ||
| 71 | type ProviderServiceMethod<Name extends keyof ProviderService.ProviderService["Service"]> = | 81 | type ProviderServiceMethod<Name extends keyof ProviderService.ProviderService["Service"]> = |
| ... | @@ -215,16 +225,58 @@ const makeProviderService = Effect.fn("makeProviderService")(function* ( | ... | @@ -215,16 +225,58 @@ const makeProviderService = Effect.fn("makeProviderService")(function* ( |
| 215 | 225 | ||
| 216 | const registry = yield* ProviderAdapterRegistry.ProviderAdapterRegistry; | 226 | const registry = yield* ProviderAdapterRegistry.ProviderAdapterRegistry; |
| 217 | const directory = yield* ProviderSessionDirectory.ProviderSessionDirectory; | 227 | const directory = yield* ProviderSessionDirectory.ProviderSessionDirectory; |
| 228 | const serverSettings = yield* ServerSettings.ServerSettingsService; | ||
| 229 | const issueMcpCredential = | ||
| 230 | options?.issueMcpCredential ?? McpSessionRegistry.issueActiveMcpCredential; | ||
| 231 | const revokeMcpCredential = | ||
| 232 | options?.revokeMcpCredential ?? McpSessionRegistry.revokeActiveMcpThread; | ||
| 218 | const runtimeEventPubSub = yield* PubSub.unbounded<ProviderRuntimeEvent>(); | 233 | const runtimeEventPubSub = yield* PubSub.unbounded<ProviderRuntimeEvent>(); |
| 219 | const nowIso = Effect.map(DateTime.now, DateTime.formatIso); | 234 | const nowIso = Effect.map(DateTime.now, DateTime.formatIso); |
| 235 | /** | ||
| 236 | * Attach the `t3-code` MCP server to the session that is about to start. | ||
| 237 | * | ||
| 238 | * This is the only place a credential is minted, so withholding one here is | ||
| 239 | * what disables agent browser access everywhere: every adapter already | ||
| 240 | * treats a missing session as "no MCP server", and the `/mcp` endpoint | ||
| 241 | * accepts nothing but tokens issued from this path. | ||
| 242 | */ | ||
| 243 | /** | ||
| 244 | * Deny on an unreadable settings file rather than letting the read failure | ||
| 245 | * escape: adding `ServerSettingsError` to `ProviderServiceError` would widen | ||
| 246 | * a union every caller handles, for a branch that only decides whether one | ||
| 247 | * optional toolset is attached. Denying is the safe direction — an explicit | ||
| 248 | * "off" silently becoming "on" would violate the user's stated choice, | ||
| 249 | * whereas the reverse costs an agent one toolset and is visible immediately. | ||
| 250 | */ | ||
| 251 | const agentBrowserAccessEnabled = serverSettings.getSettings.pipe( | ||
| 252 | Effect.map((settings) => settings.enableAgentBrowserAccess), | ||
| 253 | Effect.catch((cause) => | ||
| 254 | Effect.logWarning( | ||
| 255 | "Could not read server settings; withholding agent browser access for this session.", | ||
| 256 | { cause }, | ||
| 257 | ).pipe(Effect.as(false)), | ||
| 258 | ), | ||
| 259 | ); | ||
| 260 | |||
| 220 | const prepareMcpSession = (threadId: ThreadId, providerInstanceId: ProviderInstanceId) => | 261 | const prepareMcpSession = (threadId: ThreadId, providerInstanceId: ProviderInstanceId) => |
| 221 | McpSessionRegistry.issueActiveMcpCredential({ threadId, providerInstanceId }).pipe( | 262 | Effect.gen(function* () { |
| 222 | Effect.tap((credential) => | 263 | if (!(yield* agentBrowserAccessEnabled)) { |
| 223 | credential | 264 | // Revoke as well as clear. Every other prepare path reaches |
| 224 | ? Effect.sync(() => McpProviderSession.setMcpProviderSession(credential.config)) | 265 | // `issueActiveMcpCredential`, which revokes the thread first, so |
| 225 | : Effect.void, | 266 | // skipping it here would leave a previously issued bearer token valid |
| 226 | ), | 267 | // against `/mcp` for the rest of its liveness window — and later turns |
| 227 | ); | 268 | // would keep refreshing it. A session restart (runtime mode, cwd, |
| 269 | // model) re-prepares without stopping, so it relies on this. | ||
| 270 | yield* revokeMcpCredential(threadId); | ||
| 271 | yield* Effect.sync(() => McpProviderSession.clearMcpProviderSession(threadId)); | ||
| 272 | return undefined; | ||
| 273 | } | ||
| 274 | const credential = yield* issueMcpCredential({ threadId, providerInstanceId }); | ||
| 275 | if (credential) { | ||
| 276 | yield* Effect.sync(() => McpProviderSession.setMcpProviderSession(credential.config)); | ||
| 277 | } | ||
| 278 | return credential; | ||
| 279 | }); | ||
| 228 | const clearMcpSession = (threadId: ThreadId) => | 280 | const clearMcpSession = (threadId: ThreadId) => |
| 229 | McpSessionRegistry.revokeActiveMcpThread(threadId).pipe( | 281 | McpSessionRegistry.revokeActiveMcpThread(threadId).pipe( |
| 230 | Effect.tap(() => Effect.sync(() => McpProviderSession.clearMcpProviderSession(threadId))), | 282 | Effect.tap(() => Effect.sync(() => McpProviderSession.clearMcpProviderSession(threadId))), |
apps/web/src/components/settings/IntegrationsSettings.tsx+47-1| ... | @@ -10,6 +10,7 @@ import { | ... | @@ -10,6 +10,7 @@ import { |
| 10 | DEFAULT_BROWSER_AUTO_SHOW_FLOATING_PREVIEW, | 10 | DEFAULT_BROWSER_AUTO_SHOW_FLOATING_PREVIEW, |
| 11 | DEFAULT_BROWSER_VIEWPORT, | 11 | DEFAULT_BROWSER_VIEWPORT, |
| 12 | DEFAULT_PREVIEW_APPEARANCE, | 12 | DEFAULT_PREVIEW_APPEARANCE, |
| 13 | DEFAULT_UNIFIED_SETTINGS, | ||
| 13 | DEFAULT_PREVIEW_ZOOM_FACTOR, | 14 | DEFAULT_PREVIEW_ZOOM_FACTOR, |
| 14 | FILL_PREVIEW_VIEWPORT, | 15 | FILL_PREVIEW_VIEWPORT, |
| 15 | PREVIEW_VIEWPORT_MAX_AREA, | 16 | PREVIEW_VIEWPORT_MAX_AREA, |
| ... | @@ -39,7 +40,11 @@ import { | ... | @@ -39,7 +40,11 @@ import { |
| 39 | } from "../ui/select"; | 40 | } from "../ui/select"; |
| 40 | import { Switch } from "../ui/switch"; | 41 | import { Switch } from "../ui/switch"; |
| 41 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | 42 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 42 | import { useClientSettings, useUpdatePrimarySettings } from "~/hooks/useSettings"; | 43 | import { |
| 44 | useClientSettings, | ||
| 45 | usePrimarySettings, | ||
| 46 | useUpdatePrimarySettings, | ||
| 47 | } from "~/hooks/useSettings"; | ||
| 43 | 48 | ||
| 44 | import { | 49 | import { |
| 45 | SettingResetButton, | 50 | SettingResetButton, |
| ... | @@ -350,6 +355,44 @@ function BrowserAppearanceSetting({ disabled }: { readonly disabled: boolean }) | ... | @@ -350,6 +355,44 @@ function BrowserAppearanceSetting({ disabled }: { readonly disabled: boolean }) |
| 350 | ); | 355 | ); |
| 351 | } | 356 | } |
| 352 | 357 | ||
| 358 | function AgentBrowserAccessSetting() { | ||
| 359 | const settings = usePrimarySettings(); | ||
| 360 | const updateSettings = useUpdatePrimarySettings(); | ||
| 361 | |||
| 362 | return ( | ||
| 363 | <SettingsRow | ||
| 364 | {...searchableSetting("agent-browser-access")} | ||
| 365 | description="Let agents open and drive the preview browser. When off, the browser tools and the instructions describing them are withheld from agent sessions. Your own browser panel is unaffected." | ||
| 366 | status={ | ||
| 367 | settings.enableAgentBrowserAccess | ||
| 368 | ? undefined | ||
| 369 | : "Applies to sessions started from now on; a running agent keeps the tools it was given." | ||
| 370 | } | ||
| 371 | resetAction={ | ||
| 372 | settings.enableAgentBrowserAccess !== DEFAULT_UNIFIED_SETTINGS.enableAgentBrowserAccess ? ( | ||
| 373 | <SettingResetButton | ||
| 374 | label="agent browser access" | ||
| 375 | onClick={() => | ||
| 376 | updateSettings({ | ||
| 377 | enableAgentBrowserAccess: DEFAULT_UNIFIED_SETTINGS.enableAgentBrowserAccess, | ||
| 378 | }) | ||
| 379 | } | ||
| 380 | /> | ||
| 381 | ) : null | ||
| 382 | } | ||
| 383 | control={ | ||
| 384 | <Switch | ||
| 385 | checked={settings.enableAgentBrowserAccess} | ||
| 386 | onCheckedChange={(checked) => | ||
| 387 | updateSettings({ enableAgentBrowserAccess: Boolean(checked) }) | ||
| 388 | } | ||
| 389 | aria-label="Allow agent browser access" | ||
| 390 | /> | ||
| 391 | } | ||
| 392 | /> | ||
| 393 | ); | ||
| 394 | } | ||
| 395 | |||
| 353 | function BrowserAutoShowFloatingPreviewSetting({ disabled }: { readonly disabled: boolean }) { | 396 | function BrowserAutoShowFloatingPreviewSetting({ disabled }: { readonly disabled: boolean }) { |
| 354 | const autoShow = useClientSettings((settings) => settings.browserAutoShowFloatingPreview); | 397 | const autoShow = useClientSettings((settings) => settings.browserAutoShowFloatingPreview); |
| 355 | const updateSettings = useUpdatePrimarySettings(); | 398 | const updateSettings = useUpdatePrimarySettings(); |
| ... | @@ -425,6 +468,9 @@ export function IntegrationsSettingsPanel() { | ... | @@ -425,6 +468,9 @@ export function IntegrationsSettingsPanel() { |
| 425 | return ( | 468 | return ( |
| 426 | <SettingsPageContainer> | 469 | <SettingsPageContainer> |
| 427 | <SettingsSection id="browser" title="Browser"> | 470 | <SettingsSection id="browser" title="Browser"> |
| 471 | {/* Server-authoritative, so it stays editable on every client and sits | ||
| 472 | outside the block covering the desktop-only defaults. */} | ||
| 473 | <AgentBrowserAccessSetting /> | ||
| 428 | {previewDefaultsDisabled ? ( | 474 | {previewDefaultsDisabled ? ( |
| 429 | <DesktopOnlyBrowserDefaults>{previewDefaults}</DesktopOnlyBrowserDefaults> | 475 | <DesktopOnlyBrowserDefaults>{previewDefaults}</DesktopOnlyBrowserDefaults> |
| 430 | ) : ( | 476 | ) : ( |
apps/web/src/components/settings/SettingsPanels.tsx+8| ... | @@ -531,6 +531,9 @@ export function useSettingsRestore(onRestored?: () => void) { | ... | @@ -531,6 +531,9 @@ export function useSettingsRestore(onRestored?: () => void) { |
| 531 | : []), | 531 | : []), |
| 532 | ...(isTextGenerationModelDirty ? ["Text generation model"] : []), | 532 | ...(isTextGenerationModelDirty ? ["Text generation model"] : []), |
| 533 | ...getChangedBrowserSettingLabels(settings), | 533 | ...getChangedBrowserSettingLabels(settings), |
| 534 | ...(settings.enableAgentBrowserAccess !== DEFAULT_UNIFIED_SETTINGS.enableAgentBrowserAccess | ||
| 535 | ? ["Agent browser access"] | ||
| 536 | : []), | ||
| 534 | ], | 537 | ], |
| 535 | [ | 538 | [ |
| 536 | isTextGenerationModelDirty, | 539 | isTextGenerationModelDirty, |
| ... | @@ -539,6 +542,7 @@ export function useSettingsRestore(onRestored?: () => void) { | ... | @@ -539,6 +542,7 @@ export function useSettingsRestore(onRestored?: () => void) { |
| 539 | settings.browserDefaultZoomFactor, | 542 | settings.browserDefaultZoomFactor, |
| 540 | settings.browserDefaultAppearance, | 543 | settings.browserDefaultAppearance, |
| 541 | settings.browserAutoShowFloatingPreview, | 544 | settings.browserAutoShowFloatingPreview, |
| 545 | settings.enableAgentBrowserAccess, | ||
| 542 | settings.confirmQuit, | 546 | settings.confirmQuit, |
| 543 | settings.confirmThreadArchive, | 547 | settings.confirmThreadArchive, |
| 544 | settings.confirmThreadDelete, | 548 | settings.confirmThreadDelete, |
| ... | @@ -667,6 +671,10 @@ export function useSettingsRestore(onRestored?: () => void) { | ... | @@ -667,6 +671,10 @@ export function useSettingsRestore(onRestored?: () => void) { |
| 667 | browserDefaultZoomFactor: DEFAULT_UNIFIED_SETTINGS.browserDefaultZoomFactor, | 671 | browserDefaultZoomFactor: DEFAULT_UNIFIED_SETTINGS.browserDefaultZoomFactor, |
| 668 | browserDefaultAppearance: DEFAULT_UNIFIED_SETTINGS.browserDefaultAppearance, | 672 | browserDefaultAppearance: DEFAULT_UNIFIED_SETTINGS.browserDefaultAppearance, |
| 669 | browserAutoShowFloatingPreview: DEFAULT_UNIFIED_SETTINGS.browserAutoShowFloatingPreview, | 673 | browserAutoShowFloatingPreview: DEFAULT_UNIFIED_SETTINGS.browserAutoShowFloatingPreview, |
| 674 | // Re-granted like any other default. The confirmation dialog lists it by | ||
| 675 | // name, so a user restoring defaults is told the agent regains access | ||
| 676 | // rather than discovering it later. | ||
| 677 | enableAgentBrowserAccess: DEFAULT_UNIFIED_SETTINGS.enableAgentBrowserAccess, | ||
| 670 | }); | 678 | }); |
| 671 | onRestored?.(); | 679 | onRestored?.(); |
| 672 | }, [ | 680 | }, [ |
apps/web/src/components/settings/settingsSearch.ts+6| ... | @@ -197,6 +197,12 @@ export const SETTINGS_SEARCH_ITEMS = [ | ... | @@ -197,6 +197,12 @@ export const SETTINGS_SEARCH_ITEMS = [ |
| 197 | title: "Providers", | 197 | title: "Providers", |
| 198 | to: "/settings/providers", | 198 | to: "/settings/providers", |
| 199 | }, | 199 | }, |
| 200 | { | ||
| 201 | id: "agent-browser-access", | ||
| 202 | title: "Agent browser access", | ||
| 203 | to: "/settings/integrations", | ||
| 204 | targetId: "browser", | ||
| 205 | }, | ||
| 200 | { | 206 | { |
| 201 | id: "browser-default-viewport", | 207 | id: "browser-default-viewport", |
| 202 | title: "Default browser viewport", | 208 | title: "Default browser viewport", |
packages/contracts/src/settings.ts+13| ... | @@ -584,6 +584,18 @@ export const ServerSettings = Schema.Struct({ | ... | @@ -584,6 +584,18 @@ export const ServerSettings = Schema.Struct({ |
| 584 | Schema.withDecodingDefault(Effect.succeed(false)), | 584 | Schema.withDecodingDefault(Effect.succeed(false)), |
| 585 | ), | 585 | ), |
| 586 | enableProviderUpdateChecks: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), | 586 | enableProviderUpdateChecks: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), |
| 587 | /** | ||
| 588 | * Whether agents may drive the in-app preview browser. Turning this off | ||
| 589 | * withholds the MCP credential, so the `t3-code` server (and with it every | ||
| 590 | * `preview_*` tool) is never attached to a provider session, and the prompt | ||
| 591 | * text describing those tools is dropped along with them. The user's own | ||
| 592 | * browser panel is unaffected — this gates agent access only. | ||
| 593 | * | ||
| 594 | * Server-authoritative rather than client-local: tool injection and prompt | ||
| 595 | * construction both happen on the server, and the answer must not differ | ||
| 596 | * between a desktop window and a phone attached to the same server. | ||
| 597 | */ | ||
| 598 | enableAgentBrowserAccess: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), | ||
| 587 | backgroundActivity: BackgroundActivitySettings, | 599 | backgroundActivity: BackgroundActivitySettings, |
| 588 | // Legacy flat fields retained for old settings files and old clients. New | 600 | // Legacy flat fields retained for old settings files and old clients. New |
| 589 | // consumers should resolve `backgroundActivity` instead. | 601 | // consumers should resolve `backgroundActivity` instead. |
| ... | @@ -747,6 +759,7 @@ export const ServerSettingsPatch = Schema.Struct({ | ... | @@ -747,6 +759,7 @@ export const ServerSettingsPatch = Schema.Struct({ |
| 747 | // Server settings | 759 | // Server settings |
| 748 | enableLegacyTokenStreaming: Schema.optionalKey(Schema.Boolean), | 760 | enableLegacyTokenStreaming: Schema.optionalKey(Schema.Boolean), |
| 749 | enableProviderUpdateChecks: Schema.optionalKey(Schema.Boolean), | 761 | enableProviderUpdateChecks: Schema.optionalKey(Schema.Boolean), |
| 762 | enableAgentBrowserAccess: Schema.optionalKey(Schema.Boolean), | ||
| 750 | backgroundActivity: Schema.optionalKey( | 763 | backgroundActivity: Schema.optionalKey( |
| 751 | Schema.Struct({ | 764 | Schema.Struct({ |
| 752 | schemaVersion: Schema.optionalKey(Schema.Literal(1)), | 765 | schemaVersion: Schema.optionalKey(Schema.Literal(1)), |