From 6391be2729566a1fe1045ff8617f003580b37bfd Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Thu, 24 Sep 2026 21:29:20 -0700 Subject: [PATCH] fix(server): newer Codex models get T3 Code's instructions again (#13547) Co-authored-by: Claude Opus 5.5 (1M context) --- .../provider/CodexDeveloperInstructions.ts | 76 ++++---- .../src/provider/Drivers/CodexDriver.ts | 31 ++-- .../src/provider/Layers/CodexAdapter.ts | 4 + .../CodexCollabRuntime.integration.test.ts | 73 ++++++++ .../Layers/CodexSessionRuntime.test.ts | 172 +++++++++--------- .../provider/Layers/CodexSessionRuntime.ts | 87 +++++++-- .../src/provider/RuntimeInstructions.test.ts | 9 + .../src/provider/RuntimeInstructions.ts | 11 +- .../testFixtures/codexCollabMockPeer.mjs | 8 + 9 files changed, 322 insertions(+), 149 deletions(-) diff --git a/apps/server/src/provider/CodexDeveloperInstructions.ts b/apps/server/src/provider/CodexDeveloperInstructions.ts index 6a7fee351bced6c5ea4bc615ac5671f16fd3527a..2ed593d159bcd8b4a75dc0129018171bb1692789 100644 --- a/apps/server/src/provider/CodexDeveloperInstructions.ts +++ b/apps/server/src/provider/CodexDeveloperInstructions.ts @@ -1,23 +1,18 @@ import type { ProviderInteractionMode } from "@t3tools/contracts"; +import type { V2TurnStartParams__AdditionalContextEntry } from "effect-codex-app-server/schema"; import { buildRuntimeInstructions } from "./RuntimeInstructions.ts"; -const T3_CODE_BROWSER_TOOL_INSTRUCTIONS = ` - -## T3 Code collaborative browser +const T3_CODE_BROWSER_TOOL_INSTRUCTIONS = `## T3 Code collaborative browser You are running inside T3 Code. The \`t3-code\` MCP server is the product-native collaborative browser shared with the user. When it exposes \`preview_*\` tools, prefer those tools for browser navigation, inspection, interaction, screenshots, and recordings. For browser work, first call \`preview_status\`. If no automation-capable preview is attached, call \`preview_open\` before concluding that the browser is unavailable. Then use \`preview_navigate\`, \`preview_snapshot\`, and the focused interaction tools. Prefer snapshot-provided locators over coordinates. -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. -`; +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.`; -const T3_CODE_DEVICE_TOOL_INSTRUCTIONS = ` +const T3_CODE_DEVICE_TOOL_INSTRUCTIONS = `## T3 Code devices -## T3 Code devices - -The \`t3-code\` MCP server also exposes \`device_*\` tools for iOS Simulators and Android Emulators on this environment. For mobile verification, call \`device_list\`, then \`device_open\` so the user can watch the device in their Device panel; its result explains how to drive the device. Driving happens through the \`agent-device\` CLI, which is on PATH. Keep the host config and session flags returned by \`device_open\` on every command so concurrent devices stay independent: prefer \`agent-device snapshot -i\` refs over coordinates, and use \`device_screenshot\` when you need to see the screen. Do not call simctl, adb, xcrun, or serve-sim directly while these tools are present. If \`device_list\` reports a platform as unavailable, say so instead of trying another route. -`; +The \`t3-code\` MCP server also exposes \`device_*\` tools for iOS Simulators and Android Emulators on this environment. For mobile verification, call \`device_list\`, then \`device_open\` so the user can watch the device in their Device panel; its result explains how to drive the device. Driving happens through the \`agent-device\` CLI, which is on PATH. Keep the host config and session flags returned by \`device_open\` on every command so concurrent devices stay independent: prefer \`agent-device snapshot -i\` refs over coordinates, and use \`device_screenshot\` when you need to see the screen. Do not call simctl, adb, xcrun, or serve-sim directly while these tools are present. If \`device_list\` reports a platform as unavailable, say so instead of trying another route.`; export interface T3CodeToolAvailability { readonly browser: boolean; @@ -36,16 +31,17 @@ const normalizeAvailability = ( * from Playwright, agent-browser, and raw simctl/adb, so leaving them in would * talk it out of the only automation it still has. */ -const browserToolInstructions = (availability: boolean | T3CodeToolAvailability): string => { +const toolInstructions = (availability: boolean | T3CodeToolAvailability): string => { const tools = normalizeAvailability(availability); - return `${tools.browser ? T3_CODE_BROWSER_TOOL_INSTRUCTIONS : ""}${ - tools.device ? T3_CODE_DEVICE_TOOL_INSTRUCTIONS : "" - }`; + return [ + tools.browser ? T3_CODE_BROWSER_TOOL_INSTRUCTIONS : "", + tools.device ? T3_CODE_DEVICE_TOOL_INSTRUCTIONS : "", + ] + .filter(Boolean) + .join("\n\n"); }; -const codexPlanModeDeveloperInstructions = ( - browserToolsAvailable: boolean | T3CodeToolAvailability, -): string => `# Plan Mode (Conversational) +const CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS = `# Plan Mode (Conversational) 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. @@ -173,12 +169,9 @@ Do not ask "should I proceed?" in the final output. The user can easily switch o Only produce at most one \`\` block per turn, and only when you are presenting a complete spec. If the user stays in Plan mode and asks for revisions after a prior \`\`, any new \`\` 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 \`\` 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 \`\` unchanged. -${browserToolInstructions(browserToolsAvailable)} `; -const codexDefaultModeDeveloperInstructions = ( - browserToolsAvailable: boolean | T3CodeToolAvailability, -): string => `# Collaboration Mode: Default +const CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS = `# Collaboration Mode: Default You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. @@ -189,29 +182,46 @@ Your active mode changes only when new developer instructions with a different \ Use the \`request_user_input\` tool only when it is listed in the available tools for this turn. 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. -${browserToolInstructions(browserToolsAvailable)} `; export interface CodexRuntimeInfo { readonly model: string; + readonly modelName?: string | undefined; readonly reasoningEffort: string; } -export function buildCodexDeveloperInstructions( - interactionMode: ProviderInteractionMode, +/** Mode prompt for `turn/start.collaborationMode.settings.developer_instructions`. */ +export function buildCodexDeveloperInstructions(interactionMode: ProviderInteractionMode): string { + return interactionMode === "plan" + ? CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS + : CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS; +} + +/** + * T3 Code context for `turn/start.additionalContext`. Codex renders each entry + * as a `value` developer message and resends it only when the value + * changes. + * + * This must stay out of the collaboration mode: when the model catalog ships + * its own text for a mode, as newer models do, Codex uses that text and drops + * the client's `developer_instructions` entirely. + */ +export function buildCodexAdditionalContext( runtime: CodexRuntimeInfo, /** * Whether the `t3-code` MCP server is attached to this turn. Callers derive * it from the session's actual MCP configuration rather than re-reading the * setting, so the prompt cannot claim tools the turn doesn't have. */ - browserToolsAvailable: boolean | T3CodeToolAvailability = true, -): string { - const base = - interactionMode === "plan" - ? codexPlanModeDeveloperInstructions(browserToolsAvailable) - : codexDefaultModeDeveloperInstructions(browserToolsAvailable); - return `${base} - -${buildRuntimeInstructions({ harness: "Codex", ...runtime })}`; + toolsAvailable: boolean | T3CodeToolAvailability = true, +): Record { + const tools = toolInstructions(toolsAvailable); + // Separate keys keep each value under Codex's per-entry token cap. + return { + t3_code_runtime: { + kind: "application", + value: buildRuntimeInstructions({ harness: "Codex", ...runtime }), + }, + ...(tools ? { t3_code_tools: { kind: "application", value: tools } } : {}), + }; } diff --git a/apps/server/src/provider/Drivers/CodexDriver.ts b/apps/server/src/provider/Drivers/CodexDriver.ts index 52f7b47e1e1d01041bd61546b7978c16fc66e749..71b4a3a59bc2e6956995dd265f298d415f1983b3 100644 --- a/apps/server/src/provider/Drivers/CodexDriver.ts +++ b/apps/server/src/provider/Drivers/CodexDriver.ts @@ -172,18 +172,6 @@ export const CodexDriver: ProviderDriver = { ), ); - // `makeCodexAdapter` and `makeCodexTextGeneration` have `never` error - // channels at construction time — their failure modes are all on the - // per-operation closures they return. No `mapError` wrapper is needed - // here; the registry only has to worry about snapshot-build and - // spawner-availability failures surfaced from `checkCodexProviderStatus` - // below. - const adapter = yield* makeCodexAdapter(effectiveConfig, { - instanceId, - environment: processEnv, - ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), - }); - // Build a managed snapshot whose settings never change — mutations come // in as instance rebuilds from the registry rather than in-place // updates. Pre-provide `ChildProcessSpawner` so the check fits @@ -238,11 +226,20 @@ export const CodexDriver: ProviderDriver = { }), ), ); - const textGeneration = yield* makeCodexTextGeneration( - effectiveConfig, - processEnv, - snapshot.getSnapshot.pipe(Effect.map((value) => value.models)), - ); + const models = snapshot.getSnapshot.pipe(Effect.map((value) => value.models)); + // `makeCodexAdapter` and `makeCodexTextGeneration` have `never` error + // channels at construction time — their failure modes are all on the + // per-operation closures they return. No `mapError` wrapper is needed + // here; the registry only has to worry about snapshot-build and + // spawner-availability failures surfaced from `checkCodexProviderStatus` + // above. + const adapter = yield* makeCodexAdapter(effectiveConfig, { + instanceId, + environment: processEnv, + models, + ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), + }); + const textGeneration = yield* makeCodexTextGeneration(effectiveConfig, processEnv, models); const snapshotForCwd = (cwd: string) => !effectiveConfig.enabled ? snapshot.getSnapshot diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 0ecc9693ab04399c8eb1898f1eedaad1e421289e..baa8d846f7c65c869d6b744996d50b72168f6851 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -22,6 +22,7 @@ import { type ToolActivityNativeAppReference, type ToolActivitySource, type ProviderUserInputAnswers, + type ServerProviderModel, RuntimeItemId, RuntimeRequestId, RuntimeTaskId, @@ -89,6 +90,8 @@ const PROVIDER = ProviderDriverKind.make("codex"); export interface CodexAdapterLiveOptions { readonly instanceId?: ProviderInstanceId; readonly environment?: NodeJS.ProcessEnv; + /** The provider's model list; supplies model display names for runtime info. */ + readonly models?: Effect.Effect>; readonly makeRuntime?: ( options: CodexSessionRuntimeOptions, ) => Effect.Effect< @@ -2277,6 +2280,7 @@ export const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( providerInstanceId: boundInstanceId, cwd: input.cwd ?? process.cwd(), binaryPath: codexConfig.binaryPath, + ...(options?.models ? { models: options.models } : {}), launchArgs: resolveCodexLaunchArgs(codexConfig.launchArgs, options?.environment), ...(options?.environment ? { environment: options.environment } : {}), ...(codexConfig.homePath ? { homePath: codexConfig.homePath } : {}), diff --git a/apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts b/apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts index 2a9fb56c186a5d9e33b5d09eb0d1fc728acc8bc2..5405169926f52390cf630f6c5e13d39e14731630 100644 --- a/apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts +++ b/apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts @@ -868,3 +868,76 @@ describe("CodexSessionRuntime collab integration", () => { ); } }); + +describe("CodexSessionRuntime compaction", () => { + it.effect("restores T3 context after the root thread compacts", () => + Effect.gen(function* () { + const compacted = (threadId: string) => ({ + method: "item/completed", + params: { + threadId, + turnId: `${threadId}-turn`, + completedAtMs: 0, + item: { type: "contextCompaction", id: `compaction-${threadId}` }, + }, + }); + const script = { + rootThreadId: ROOT, + recordRequests: true, + // A child's compaction must not inject into the root thread. + notifications: [compacted(CHILD_A), compacted(ROOT)], + }; + // @effect-diagnostics-next-line preferSchemaOverJson:off + NodeFS.writeFileSync(scriptPath, JSON.stringify(script), "utf8"); + NodeFS.rmSync(`${scriptPath}.requests`, { force: true }); + yield* Effect.addFinalizer(() => + Effect.sync(() => { + NodeFS.rmSync(scriptPath, { force: true }); + NodeFS.rmSync(`${scriptPath}.requests`, { force: true }); + }), + ); + + const runtime = yield* makeCodexSessionRuntime({ + threadId: ThreadId.make("thread-compaction-context"), + binaryPath: peerPath, + cwd: NodeOS.tmpdir(), + runtimeMode: "full-access", + environment: { ...process.env, T3_CODEX_COLLAB_SCRIPT: scriptPath }, + models: Effect.succeed([ + { slug: "gpt-5.6-sol", name: "GPT-5.6 Sol", isCustom: false, capabilities: null }, + ]), + }); + const completedFiber = yield* runtime.events.pipe( + Stream.filter((event) => event.method === "turn/completed"), + Stream.take(1), + Stream.runCollect, + Effect.forkScoped, + ); + + yield* runtime.start(); + yield* runtime.sendTurn({ input: "keep going", interactionMode: "default" }); + yield* Fiber.join(completedFiber); + + // The restore is awaited before later notifications, so it has landed. + const requests = readRecordedRequests(); + assert.lengthOf(requests, 1); + const [inject] = requests; + assert.isDefined(inject); + assert.equal(inject.method, "thread/inject_items"); + assert.equal(inject.params.threadId, ROOT); + const texts = ( + inject.params.items as ReadonlyArray<{ role: string; content: [{ text: string }] }> + ).map((item) => { + assert.equal(item.role, "developer"); + return item.content[0].text; + }); + assert.lengthOf(texts, 1); + assert.match( + texts[0] ?? "", + /^.*as GPT-5\.6 Sol \(model slug: gpt-5\.6-sol\).*<\/t3_code_runtime>$/s, + ); + + yield* runtime.close; + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); +}); diff --git a/apps/server/src/provider/Layers/CodexSessionRuntime.test.ts b/apps/server/src/provider/Layers/CodexSessionRuntime.test.ts index 588c24dd11767757bf8d3fc75aef5cf9ad530af0..e315c9ab20febb45c7f6b50c58a0e87ae9a593f1 100644 --- a/apps/server/src/provider/Layers/CodexSessionRuntime.test.ts +++ b/apps/server/src/provider/Layers/CodexSessionRuntime.test.ts @@ -9,7 +9,10 @@ import * as CodexErrors from "effect-codex-app-server/errors"; import * as CodexRpc from "effect-codex-app-server/rpc"; import * as EffectCodexSchema from "effect-codex-app-server/schema"; -import { buildCodexDeveloperInstructions } from "../CodexDeveloperInstructions.ts"; +import { + buildCodexAdditionalContext, + buildCodexDeveloperInstructions, +} from "../CodexDeveloperInstructions.ts"; import { codexSessionAppServerArgs } from "./codexLaunchArgs.ts"; import { buildTurnStartParams, @@ -232,12 +235,13 @@ describe("buildTurnStartParams", () => { settings: { model: "gpt-5.3-codex", reasoning_effort: "medium", - developer_instructions: buildCodexDeveloperInstructions("plan", { - model: "gpt-5.3-codex", - reasoningEffort: "medium", - }), + developer_instructions: buildCodexDeveloperInstructions("plan"), }, }, + additionalContext: buildCodexAdditionalContext({ + model: "gpt-5.3-codex", + reasoningEffort: "medium", + }), }); }); @@ -281,12 +285,13 @@ describe("buildTurnStartParams", () => { settings: { model: "gpt-5.3-codex", reasoning_effort: "medium", - developer_instructions: buildCodexDeveloperInstructions("default", { - model: "gpt-5.3-codex", - reasoningEffort: "medium", - }), + developer_instructions: buildCodexDeveloperInstructions("default"), }, }, + additionalContext: buildCodexAdditionalContext({ + model: "gpt-5.3-codex", + reasoningEffort: "medium", + }), }); }); @@ -303,9 +308,29 @@ describe("buildTurnStartParams", () => { const settings = params.collaborationMode?.settings; NodeAssert.equal(settings?.model, DEFAULT_MODEL); NodeAssert.equal(settings?.reasoning_effort, "medium"); - NodeAssert.ok(settings?.developer_instructions?.includes(`as ${DEFAULT_MODEL} with medium`)); + NodeAssert.ok( + params.additionalContext?.t3_code_runtime?.value.includes(`as ${DEFAULT_MODEL} with medium`), + ); }); + it.effect("names the model by display name and slug in the runtime context", () => + Effect.gen(function* () { + const params = yield* buildTurnStartParams({ + threadId: "provider-thread-1", + runtimeMode: "full-access", + model: "gpt-5.3-codex", + modelName: "GPT-5.3-Codex", + effort: "high", + interactionMode: "plan", + }); + + NodeAssert.match( + params.additionalContext?.t3_code_runtime?.value ?? "", + /as GPT-5\.3-Codex \(model slug: gpt-5\.3-codex\) with high reasoning effort/, + ); + }), + ); + it.effect("routes approvals to the auto reviewer in auto mode", () => Effect.gen(function* () { const params = yield* buildTurnStartParams({ @@ -560,99 +585,82 @@ describe("Codex MCP elicitation approvals", () => { }); describe("buildCodexDeveloperInstructions", () => { - it("appends runtime info after the mode instructions", () => { - const instructions = buildCodexDeveloperInstructions("default", { - model: "gpt-5.3-codex", - reasoningEffort: "high", - }); - - NodeAssert.match(instructions, /^# Collaboration Mode: Default/); - NodeAssert.match(instructions, /T3 Code/); - NodeAssert.match(instructions, /Codex harness/); - NodeAssert.match(instructions, /as gpt-5\.3-codex with high reasoning effort/); - }); - - it("describes Markdown media support in the runtime context in both modes", () => { + it("keeps T3 context out of the mode prompt, which the model catalog can replace", () => { for (const mode of ["default", "plan"] as const) { - const instructions = buildCodexDeveloperInstructions(mode, { - model: "gpt-5.3-codex", - reasoningEffort: "high", - }); - NodeAssert.match( - instructions, - /.*embed images and videos.*Markdown.*<\/runtime_info>/, - ); + const instructions = buildCodexDeveloperInstructions(mode); + NodeAssert.match(instructions, /^[\s\S]*<\/collaboration_mode>$/); + NodeAssert.doesNotMatch(instructions, /runtime_info|pull_request_linking|preview_|device_/); } }); +}); - it("includes runtime info alongside plan mode instructions", () => { - const instructions = buildCodexDeveloperInstructions("plan", { - model: "gpt-5.3-codex", - reasoningEffort: "medium", - }); +describe("buildCodexAdditionalContext", () => { + const runtime = { model: "gpt-5.3-codex", reasoningEffort: "high" }; + const runtimeValue = (context: ReturnType) => + context.t3_code_runtime?.value ?? ""; - NodeAssert.match(instructions, /^# Plan Mode/); - NodeAssert.match(instructions, /as gpt-5\.3-codex with medium reasoning effort/); + it("describes the harness, model, effort, and Markdown media support", () => { + const context = buildCodexAdditionalContext(runtime); + + NodeAssert.equal(context.t3_code_runtime?.kind, "application"); + NodeAssert.match( + runtimeValue(context), + /.*Codex harness, as gpt-5\.3-codex with high reasoning effort.*embed images and videos.*Markdown.*<\/runtime_info>/, + ); }); it("varies with the model and effort of each turn", () => { - const first = buildCodexDeveloperInstructions("default", { - model: "gpt-5.3-codex", - reasoningEffort: "medium", - }); - const second = buildCodexDeveloperInstructions("default", { - model: "gpt-5.4", - reasoningEffort: "high", - }); - - NodeAssert.notEqual(first, second); + NodeAssert.notEqual( + runtimeValue( + buildCodexAdditionalContext({ model: "gpt-5.3-codex", reasoningEffort: "medium" }), + ), + runtimeValue(buildCodexAdditionalContext({ model: "gpt-5.4", reasoningEffort: "high" })), + ); }); it("flattens multiline metadata into single-line runtime info", () => { - const instructions = buildCodexDeveloperInstructions("default", { - model: "gpt\n5.3\ncodex", - reasoningEffort: " high\neffort ", - }); + const value = runtimeValue( + buildCodexAdditionalContext({ model: "gpt\n5.3\ncodex", reasoningEffort: " high\neffort " }), + ); - NodeAssert.match(instructions, /as gpt 5\.3 codex with high effort reasoning effort/); - NodeAssert.doesNotMatch(instructions, /[^<]*\n/); + NodeAssert.match(value, /as gpt 5\.3 codex with high effort reasoning effort/); + NodeAssert.doesNotMatch(value, /[^<]*\n/); + }); + + it("keeps every entry under Codex's 1,000 token cap per entry", () => { + const context = buildCodexAdditionalContext(runtime, { browser: true, device: true }); + for (const entry of Object.values(context)) { + // Codex estimates 4 bytes per token and truncates the middle of longer values. + NodeAssert.ok(Buffer.byteLength(entry.value) < 4_000); + } }); }); -describe("T3 browser developer instructions", () => { +describe("T3 tool instructions", () => { const runtime = { model: "gpt-5.3-codex", reasoningEffort: "high" }; - it("prefers the product-native preview tools in both collaboration modes", () => { - for (const mode of ["default", "plan"] as const) { - const instructions = buildCodexDeveloperInstructions(mode, runtime, true); - NodeAssert.match(instructions, /t3-code/); - NodeAssert.match(instructions, /preview_status/); - NodeAssert.match(instructions, /preview_open/); - NodeAssert.match(instructions, /Do not switch to global browser skills/); - } + it("prefers the product-native preview tools when they are attached", () => { + const tools = buildCodexAdditionalContext(runtime, true).t3_code_tools?.value ?? ""; + NodeAssert.match(tools, /t3-code/); + NodeAssert.match(tools, /preview_status/); + NodeAssert.match(tools, /preview_open/); + NodeAssert.match(tools, /Do not switch to global browser skills/); + NodeAssert.doesNotMatch(tools, /device_open/); }); - it("omits the browser block entirely when the preview tools are not attached", () => { - for (const mode of ["default", "plan"] as const) { - const instructions = buildCodexDeveloperInstructions(mode, runtime, false); - NodeAssert.doesNotMatch(instructions, /preview_status/); - NodeAssert.doesNotMatch(instructions, /preview_open/); - NodeAssert.doesNotMatch(instructions, /T3 Code collaborative browser/); - // Steering away from other browser automation must go with the tools; - // keeping it would leave the model talked out of its only option. - NodeAssert.doesNotMatch(instructions, /Do not switch to global browser skills/); - // The rest of the collaboration mode is untouched. - NodeAssert.match(instructions, //); - NodeAssert.match(instructions, /<\/collaboration_mode>/); - } + it("describes device tools only when the credential grants them", () => { + const tools = + buildCodexAdditionalContext(runtime, { browser: false, device: true }).t3_code_tools?.value ?? + ""; + NodeAssert.match(tools, /device_open/); + NodeAssert.doesNotMatch(tools, /preview_open/); }); - it("tracks the turn's MCP configuration rather than defaulting to on", () => { - NodeAssert.match(buildCodexDeveloperInstructions("default", runtime, true), /preview_open/); - NodeAssert.doesNotMatch( - buildCodexDeveloperInstructions("default", runtime, false), - /preview_open/, - ); + it("omits the tool entry entirely when no tools are attached", () => { + // Steering away from other browser automation must go with the tools; + // keeping it would leave the model talked out of its only option. + const context = buildCodexAdditionalContext(runtime, false); + NodeAssert.deepStrictEqual(Object.keys(context), ["t3_code_runtime"]); }); }); diff --git a/apps/server/src/provider/Layers/CodexSessionRuntime.ts b/apps/server/src/provider/Layers/CodexSessionRuntime.ts index 3e02726d7a6224a1005f7328491e1485e6f2701c..f30baf011e8af779e11a4d098fa5c1a5749bc0db 100644 --- a/apps/server/src/provider/Layers/CodexSessionRuntime.ts +++ b/apps/server/src/provider/Layers/CodexSessionRuntime.ts @@ -14,6 +14,7 @@ import { type ProviderTurnStartResult, type ProviderUserInputAnswers, RuntimeMode, + type ServerProviderModel, ThreadId, TurnId, } from "@t3tools/contracts"; @@ -40,6 +41,7 @@ import { buildCodexInitializeParams } from "./CodexProvider.ts"; import { codexSessionAppServerArgs } from "./codexLaunchArgs.ts"; import { expandHomePath } from "../../pathExpansion.ts"; import { + buildCodexAdditionalContext, buildCodexDeveloperInstructions, type T3CodeToolAvailability, } from "../CodexDeveloperInstructions.ts"; @@ -141,10 +143,13 @@ const isMcpElicitationMetadata = Schema.is(McpElicitationMetadata); const isMcpElicitationForm = Schema.is(McpElicitationForm); // TODO: Verify `packages/effect-codex-app-server/scripts/generate.ts` so the generated -// `V2TurnStartParams` schema includes `collaborationMode` directly. +// `V2TurnStartParams` schema includes its experimental fields directly. const CodexTurnStartParamsWithCollaborationMode = EffectCodexSchema.V2TurnStartParams.pipe( Schema.fieldsAssign({ collaborationMode: Schema.optionalKey(EffectCodexSchema.V2TurnStartParams__CollaborationMode), + additionalContext: Schema.optionalKey( + Schema.Record(Schema.String, EffectCodexSchema.V2TurnStartParams__AdditionalContextEntry), + ), }), ); const decodeCodexTurnStartParamsWithCollaborationMode = Schema.decodeUnknownEffect( @@ -178,6 +183,8 @@ export interface CodexSessionRuntimeOptions { readonly serviceTier?: CodexServiceTier | undefined; readonly resumeCursor?: CodexResumeCursor; readonly appServerArgs?: ReadonlyArray; + /** The provider's model list; supplies the display name for runtime info. */ + readonly models?: Effect.Effect>; /** Capabilities the session's `t3-code` MCP credential grants; drives the prompt blocks. */ readonly mcpCapabilities?: ReadonlySet; } @@ -579,28 +586,31 @@ function runtimeModeToTurnSandboxPolicy( } } -function buildCodexCollaborationMode(input: { +function buildCodexTurnInstructions(input: { readonly interactionMode?: ProviderInteractionMode; readonly model?: string; + readonly modelName?: string; readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; readonly browserToolsAvailable?: boolean | T3CodeToolAvailability; -}): EffectCodexSchema.V2TurnStartParams__CollaborationMode | undefined { +}): Pick { if (input.interactionMode === undefined) { - return undefined; + return {}; } const model = normalizeCodexModelSlug(input.model) ?? DEFAULT_MODEL; const reasoningEffort = input.effort ?? "medium"; return { - mode: input.interactionMode, - settings: { - model, - reasoning_effort: reasoningEffort, - developer_instructions: buildCodexDeveloperInstructions( - input.interactionMode, - { model, reasoningEffort }, - input.browserToolsAvailable ?? true, - ), + collaborationMode: { + mode: input.interactionMode, + settings: { + model, + reasoning_effort: reasoningEffort, + developer_instructions: buildCodexDeveloperInstructions(input.interactionMode), + }, }, + additionalContext: buildCodexAdditionalContext( + { model, modelName: input.modelName, reasoningEffort }, + input.browserToolsAvailable ?? true, + ), }; } @@ -617,6 +627,8 @@ export function buildTurnStartParams(input: { readonly path: string; }>; readonly model?: string; + /** Display name of `model`, for runtime info. */ + readonly modelName?: string; readonly serviceTier?: CodexServiceTier; readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; readonly interactionMode?: ProviderInteractionMode; @@ -638,9 +650,10 @@ export function buildTurnStartParams(input: { } const config = runtimeModeToThreadConfig(input.runtimeMode); - const collaborationMode = buildCodexCollaborationMode({ + const turnInstructions = buildCodexTurnInstructions({ ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), ...(input.model ? { model: input.model } : {}), + ...(input.modelName ? { modelName: input.modelName } : {}), ...(input.effort ? { effort: input.effort } : {}), browserToolsAvailable: input.browserToolsAvailable ?? true, }); @@ -654,7 +667,7 @@ export function buildTurnStartParams(input: { ...(input.model ? { model: input.model } : {}), ...(input.serviceTier ? { serviceTier: input.serviceTier } : {}), ...(input.effort ? { effort: input.effort } : {}), - ...(collaborationMode ? { collaborationMode } : {}), + ...turnInstructions, }).pipe( Effect.mapError((cause) => CodexErrors.CodexAppServerProtocolParseError.fromSchemaError( @@ -1305,6 +1318,9 @@ export const makeCodexSessionRuntime = ( const collabChildLiveTurnsRef = yield* Ref.make(new Map()); const suppressMemoryConsolidationNotification = makeMemoryConsolidationNotificationFilter(); const closedRef = yield* Ref.make(false); + /** The `additionalContext` of the latest `turn/start`, restored after compaction. */ + const lastAdditionalContextRef = + yield* Ref.make(undefined); // `~` is not shell-expanded when env vars are set via // `child_process.spawn`; `expandHomePath` lets a configured @@ -1856,6 +1872,35 @@ export const makeCodexSessionRuntime = ( } }); + /** + * Compaction rebuilds history from user messages and Codex's own context, + * which drops our `additionalContext` messages. Codex only resends an + * entry when its value changes, so without this the T3 context would stay + * lost until the model or effort changed. Awaited so the context is back + * before later notifications from the same turn are handled. Drop this if + * Codex enables its `retain_client_developer_messages` feature by default. + */ + const restoreAdditionalContext = (threadId: string) => + Effect.gen(function* () { + const context = yield* Ref.get(lastAdditionalContextRef); + if (!context) return; + yield* client.request("thread/inject_items", { + threadId, + items: Object.entries(context).map(([key, entry]) => ({ + type: "message", + role: "developer", + content: [{ type: "input_text", text: `<${key}>${entry.value}` }], + })), + }); + }).pipe( + Effect.timeout("10 seconds"), + Effect.catch((cause) => + Effect.logWarning("Failed to restore Codex additional context after compaction.", { + cause, + }), + ), + ); + const handleRawNotification = (notification: CodexServerNotification) => Effect.gen(function* () { const isMemoryConsolidationNotification = @@ -1942,6 +1987,14 @@ export const makeCodexSessionRuntime = ( return; } + if ( + notification.method === "item/completed" && + notification.params.item.type === "contextCompaction" && + notification.params.threadId === suppressRootId + ) { + yield* restoreAdditionalContext(notification.params.threadId); + } + let requestId: ApprovalRequestId | undefined; let requestKind: ProviderRequestKind | undefined; let turnId = childParentTurnId ?? route.turnId; @@ -2509,12 +2562,15 @@ export const makeCodexSessionRuntime = ( const normalizedModel = normalizeCodexModelSlug( input.model ?? (yield* Ref.get(sessionRef)).model, ); + const models = options.models ? yield* options.models : []; + const modelName = models.find((model) => model.slug === normalizedModel)?.name; const params = yield* buildTurnStartParams({ threadId: providerThreadId, runtimeMode: options.runtimeMode, ...(input.input ? { prompt: input.input } : {}), ...(input.attachments ? { attachments: input.attachments } : {}), ...(normalizedModel ? { model: normalizedModel } : {}), + ...(modelName ? { modelName } : {}), ...(input.serviceTier ? { serviceTier: input.serviceTier } : {}), ...(input.effort ? { effort: input.effort } : {}), ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), @@ -2526,6 +2582,7 @@ export const makeCodexSessionRuntime = ( options.mcpCapabilities, ), }); + yield* Ref.set(lastAdditionalContextRef, params.additionalContext); const rawResponse = yield* client.raw.request("turn/start", params); const response = yield* decodeV2TurnStartResponse(rawResponse).pipe( Effect.mapError((error) => diff --git a/apps/server/src/provider/RuntimeInstructions.test.ts b/apps/server/src/provider/RuntimeInstructions.test.ts index e73c50adfd6dc60e2206b7de870eb9cf6efdee90..10f8413b5a137834ebd21dba4bb7d7959be2fe2d 100644 --- a/apps/server/src/provider/RuntimeInstructions.test.ts +++ b/apps/server/src/provider/RuntimeInstructions.test.ts @@ -20,6 +20,15 @@ describe("buildRuntimeInstructions", () => { ).toContain("through the Codex harness, as custom model with high reasoning effort."); }); + it("names the model by display name and slug when they differ", () => { + expect( + buildRuntimeInstructions({ harness: "Codex", model: "gpt-5.4", modelName: "GPT-5.4" }), + ).toContain("through the Codex harness, as GPT-5.4 (model slug: gpt-5.4)."); + expect( + buildRuntimeInstructions({ harness: "Codex", model: "my-model", modelName: "my-model" }), + ).toContain("through the Codex harness, as my-model."); + }); + it.each([undefined, "", "auto", "default"])("omits unresolved model %s", (model) => { const instructions = buildRuntimeInstructions({ harness: "Cursor", model }); expect(instructions).toContain("through the Cursor harness."); diff --git a/apps/server/src/provider/RuntimeInstructions.ts b/apps/server/src/provider/RuntimeInstructions.ts index 5e72586062e531201f3b0406d4b96356365b893a..afaba915d78516b5016dc0201632f3bcd68a5d57 100644 --- a/apps/server/src/provider/RuntimeInstructions.ts +++ b/apps/server/src/provider/RuntimeInstructions.ts @@ -2,16 +2,23 @@ const PULL_REQUEST_LINKING_INSTRUCTIONS = ` When the t3-code MCP server exposes link_pull_request, you must use it to register every pull request you create or work on for this thread. Call link_pull_request with the full PR URL immediately after creating a PR or starting work on an existing PR. For a stack, call it for every layer, not just the current branch or the top PR. This applies when creating or updating PRs through gh, gh stack, another CLI, or the host API: those operations do not register the PRs with this thread. Linking an already-linked PR is safe. Before finishing PR work, call list_thread_pull_requests and link any PR from your work that is missing. Do not link unrelated PRs mentioned only as background. If a linking call fails, report that failure instead of claiming the PR is linked. `; -/** Shared runtime context; omit model and effort when the harness manages them dynamically. */ +/** + * Shared runtime context; omit model and effort when the harness manages them dynamically. + * `modelName` is the display name users see in the model picker; `model` is the slug. + */ export function buildRuntimeInstructions(runtime: { readonly harness: string; readonly model?: string | undefined; + readonly modelName?: string | undefined; readonly reasoningEffort?: string | undefined; }): string { const harness = toSingleLine(runtime.harness); const model = toSingleLine(runtime.model ?? ""); + const modelName = toSingleLine(runtime.modelName ?? ""); const effort = toSingleLine(runtime.reasoningEffort ?? ""); - const modelInfo = model && model !== "auto" && model !== "default" ? `, as ${model}` : ""; + const modelLabel = + modelName && modelName !== model ? `${modelName} (model slug: ${model})` : model; + const modelInfo = model && model !== "auto" && model !== "default" ? `, as ${modelLabel}` : ""; const effortInfo = effort ? ` with ${effort} reasoning effort` : ""; return `In case you're asked: you are running in T3 Code through the ${harness} harness${modelInfo}${effortInfo}. No need to mention this otherwise. You can embed images and videos in your response using Markdown with absolute file paths.\n\n${PULL_REQUEST_LINKING_INSTRUCTIONS}`; } diff --git a/apps/server/src/provider/testFixtures/codexCollabMockPeer.mjs b/apps/server/src/provider/testFixtures/codexCollabMockPeer.mjs index 59ebc18c5fcf9b5be8e2e2440cb4e860024a888f..5cc18af8b077c8da5ab05716a28798823ab52bad 100644 --- a/apps/server/src/provider/testFixtures/codexCollabMockPeer.mjs +++ b/apps/server/src/provider/testFixtures/codexCollabMockPeer.mjs @@ -106,6 +106,14 @@ rl.on("line", (line) => { write({ id, result: fixture.responses.threadStart }); return; } + if (method === "thread/inject_items" && script.recordRequests) { + NodeFS.appendFileSync( + `${process.env.T3_CODEX_COLLAB_SCRIPT}.requests`, + `${JSON.stringify({ method, params: message.params })}\n`, + ); + write({ id, result: {} }); + return; + } if (method === "thread/resume") { if (script.recordRequests) { NodeFS.appendFileSync( -- 2.54.0