diff --git a/apps/server/src/coderEnvironment.ts b/apps/server/src/coderEnvironment.ts index 8d15d1d9d4e01..51d84522407a8 100644 --- a/apps/server/src/coderEnvironment.ts +++ b/apps/server/src/coderEnvironment.ts @@ -93,6 +93,8 @@ export const CODER_OMITTED_CAPABILITIES = { peerLinks: "No linked environments: they need server-to-server /mcp and pairing.", mcpModeLimitHeader: "No /mcp HTTP server; agents reach T3 tools over the workspace file bridge.", linkFence: "No linked environments, so no work starts here from another one.", + localServerStop: "No dev-server port discovery: the browser preview is not carried.", + triage: "No in-app triage: it clones upstream T3 Code to help file upstream issues.", } satisfies Partial>; const platformArch = (): ExecutionEnvironmentDescriptor["platform"]["arch"] => { diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts index 13ae2edc785fc..b600cd8c86217 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts @@ -580,6 +580,19 @@ describe("ClaudeAdapterV2 native protocol logging", () => { assert.isUndefined(logged.hasMcpServers); assert.notInclude(JSON.stringify(options), "secret-claude-token"); }); + + it("logs --settings key names without their values", () => { + const options = ClaudeAdapterV2.makeClaudeQueryOptions({ + modelSelection: CLAUDE_TEST_MODEL_SELECTION, + nativeThreadId: "native-thread-claude-settings-log", + resume: false, + cwd: "/workspace", + sdkSettings: { env: { ANTHROPIC_API_KEY: "secret-settings-key" } }, + }); + const logged = ClaudeAdapterV2.loggedClaudeQueryOptions(options); + assert.deepEqual(logged.settingKeys, ["env", "showThinkingSummaries"]); + assert.notInclude(JSON.stringify(logged), "secret-settings-key"); + }); }); describe("ClaudeAdapterV2 context usage", () => { @@ -923,6 +936,34 @@ describe("ClaudeAdapterV2 append-system-prompt-file launch argument", () => { ), ); + // regression: #15073 the SDK replaced the --settings launch argument with T3's settings + it.effect.each([ + `--settings '{"skillOverrides":{"lint":"off"},"showThinkingSummaries":false}'`, + "--settings user-settings.json", + ])("merges %s beneath T3's settings", (launchArgs) => + Effect.scoped( + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-claude-settings-" }); + yield* fileSystem.writeFileString( + NodePath.join(cwd, "user-settings.json"), + '{"skillOverrides":{"lint":"off"},"showThinkingSummaries":false}', + ); + const { exit, opened } = yield* startTurnWithLaunchArgs(launchArgs, cwd); + assert.isTrue(Exit.isSuccess(exit)); + assert.deepInclude(opened[0]!.settings, { + skillOverrides: { lint: "off" }, + showThinkingSummaries: true, + }); + assert.isUndefined(opened[0]!.extraArgs?.["settings"]); + }).pipe( + Effect.provide( + Layer.mergeAll(IdAllocator.layer, McpProviderSessions.layer, NodeServices.layer), + ), + ), + ), + ); + // regression: #20413 malformed append-system-prompt launch args fail before a query opens it.effect.each([ [ @@ -953,6 +994,19 @@ describe("ClaudeAdapterV2 append-system-prompt-file launch argument", () => { flag: "--append-system-prompt", }).message, ], + [ + "--settings missing.json", + (cwd: string) => + new ClaudeAdapterV2.ClaudeSettingsLaunchArgError({ + path: NodePath.join(cwd, "missing.json"), + }).message, + ], + ["--settings '{not json}'", () => new ClaudeAdapterV2.ClaudeSettingsLaunchArgError({}).message], + // Without a closing brace this must not become a file path that echoes the secret. + [ + `--settings '{"env":{"ANTHROPIC_API_KEY":"secret-settings-key"},'`, + () => new ClaudeAdapterV2.ClaudeSettingsLaunchArgError({}).message, + ], ] as const)("fails the turn before opening Claude for %s", ([launchArgs, expectedMessage]) => Effect.scoped( Effect.gen(function* () { @@ -3209,29 +3263,31 @@ describe("ClaudeAdapterV2 background wake turns", () => { const home = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-claude-remote-" }); const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-claude-remote-cwd-" }); const remoteFile = path.join(home, "remote-settings.json"); - const open = Effect.scoped( - Effect.gen(function* () { - const harness = yield* makeWakeHarnessWithOptions({ - settings: { ...DEFAULT_CLAUDE_SETTINGS, homePath: home }, - }); - yield* harness.runtime.startTurn( - makeClaudeTestTurnInput({ - threadId: harness.threadId, - providerThread: harness.providerThread, - now: yield* DateTime.now, - attemptId: RunAttemptId.make("attempt-cached-managed-policy"), - text: "Run git status.", - attachments: [], - runtimePolicy: ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ - runtimeMode: "full-access", - interactionMode: "default", - cwd, + const openWith = (launchArgs: string) => + Effect.scoped( + Effect.gen(function* () { + const harness = yield* makeWakeHarnessWithOptions({ + settings: { ...DEFAULT_CLAUDE_SETTINGS, homePath: home, launchArgs }, + }); + yield* harness.runtime.startTurn( + makeClaudeTestTurnInput({ + threadId: harness.threadId, + providerThread: harness.providerThread, + now: yield* DateTime.now, + attemptId: RunAttemptId.make("attempt-cached-managed-policy"), + text: "Run git status.", + attachments: [], + runtimePolicy: ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ + runtimeMode: "full-access", + interactionMode: "default", + cwd, + }), }), - }), - ); - return harness.getOpenedOptions()!; - }), - ); + ); + return harness.getOpenedOptions()!; + }), + ); + const open = openWith(""); yield* fileSystem.writeFileString( remoteFile, JSON.stringify({ @@ -3245,6 +3301,11 @@ describe("ClaudeAdapterV2 background wake turns", () => { assert.isUndefined(restricted.allowDangerouslySkipPermissions); yield* fileSystem.remove(remoteFile); assert.equal((yield* open).permissionMode, "bypassPermissions"); + const launchRestricted = yield* openWith( + `--settings '{"permissions":{"disableBypassPermissionsMode":"disable"}}'`, + ); + assert.equal(launchRestricted.permissionMode, "auto"); + assert.isUndefined(launchRestricted.allowDangerouslySkipPermissions); yield* fileSystem.writeFileString(remoteFile, "{ invalid json"); assert.equal((yield* open).permissionMode, "auto"); yield* fileSystem.remove(remoteFile); @@ -3681,12 +3742,13 @@ describe("ClaudeAdapterV2 background wake turns", () => { settingsJson: string, cwd = CLAUDE_TEST_RUNTIME_POLICY.cwd, environment?: NodeJS.ProcessEnv, + launchArgs = "", ) => Effect.scoped( Effect.gen(function* () { yield* fileSystem.writeFileString(path.join(home, "settings.json"), settingsJson); const harness = yield* makeWakeHarnessWithOptions({ - settings: { ...DEFAULT_CLAUDE_SETTINGS, homePath: home }, + settings: { ...DEFAULT_CLAUDE_SETTINGS, homePath: home, launchArgs }, ...(environment === undefined ? {} : { environment }), }); yield* harness.runtime.startTurn( @@ -3710,6 +3772,16 @@ describe("ClaudeAdapterV2 background wake turns", () => { assert.equal(yield* openedToolSearch('{"env":{"ENABLE_TOOL_SEARCH":"auto"}}'), "true"); assert.isUndefined(yield* openedToolSearch('{"env":{"ENABLE_TOOL_SEARCH":"false"}}')); assert.isUndefined(yield* openedToolSearch("{}")); + // --settings beats local, project, and user settings. + assert.equal( + yield* openedToolSearch( + '{"env":{"ENABLE_TOOL_SEARCH":"auto"}}', + CLAUDE_TEST_RUNTIME_POLICY.cwd, + undefined, + `--settings '{"env":{"ENABLE_TOOL_SEARCH":"false"}}'`, + ), + "false", + ); // Claude Code keeps the local file at the repository root, the main checkout's root in a // worktree, and that file beats the user's settings. diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts index 75fbcf6d16129..bdf21d1ac9734 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts @@ -579,6 +579,18 @@ export class ClaudeAppendSystemPromptMissingValueError extends Schema.TaggedErro } } +export class ClaudeSettingsLaunchArgError extends Schema.TaggedError()( + "ClaudeSettingsLaunchArgError", + { + path: Schema.optional(Schema.String), + cause: Schema.optional(Schema.Defect()), + }, +) { + override get message(): string { + return `Claude could not read the --settings launch argument${this.path === undefined ? "" : ` at ${this.path}`} as a JSON object. Fix it in the Claude launch arguments, then send the message again.`; + } +} + export interface ClaudeAgentSdkQueryRunnerShape { readonly allocateSessionId: Effect.Effect; readonly open: ( @@ -654,7 +666,7 @@ export interface ClaudeAgentSdkLoggedQueryOptions { readonly cwd?: ClaudeAgentSdkQueryOptions["cwd"]; readonly allowedTools?: ClaudeAgentSdkQueryOptions["allowedTools"]; readonly disallowedTools?: ClaudeAgentSdkQueryOptions["disallowedTools"]; - readonly settings?: ClaudeAgentSdkQueryOptions["settings"]; + readonly settingKeys?: ReadonlyArray; readonly effort?: ClaudeAgentSdkQueryOptions["effort"]; readonly includePartialMessages?: true; readonly pathToClaudeCodeExecutable?: ClaudeAgentSdkQueryOptions["pathToClaudeCodeExecutable"]; @@ -793,7 +805,10 @@ export function loggedClaudeQueryOptions( ...(options.cwd === undefined ? {} : { cwd: options.cwd }), ...(options.allowedTools === undefined ? {} : { allowedTools: options.allowedTools }), ...(options.disallowedTools === undefined ? {} : { disallowedTools: options.disallowedTools }), - ...(options.settings === undefined ? {} : { settings: options.settings }), + // Settings can carry the user's --settings values, such as an env block with credentials. + ...(typeof options.settings === "object" && options.settings !== null + ? { settingKeys: Object.keys(options.settings) } + : {}), ...(options.effort === undefined ? {} : { effort: options.effort }), ...(options.includePartialMessages === true ? { includePartialMessages: true } : {}), ...(options.pathToClaudeCodeExecutable === undefined @@ -876,6 +891,8 @@ export const layerQueryRunner: Layer.Layer< Stream.toAsyncIterable, ); // Send MCP credentials over stdin to keep them out of SDK argv and inherited command environments. + // Coder: no dev-server port discovery (#22542) runs here, since the browser preview is not + // carried, so jobs the agent starts get no thread marker and the agent is not tracked. const { mcpServers, ...queryOptions } = input.options; // The SDK can prepend a scope wrapper to native binaries; script entries run through node. const binaryPath = queryOptions.pathToClaudeCodeExecutable; @@ -1358,6 +1375,8 @@ export function makeClaudeQueryOptions(input: { "dangerously-skip-permissions": launchArgSkipPermissions, "append-system-prompt": launchArgAppendSystemPrompt, "append-system-prompt-file": _launchArgAppendSystemPromptFile, + // The SDK replaces this flag with options.settings, so it arrives read as sdkSettings. + settings: _launchArgSettings, ...extraArgs } = input.settings === undefined ? {} : parseCliArgs(input.settings.launchArgs).flags; // Claude Code overwrites its CLI append text with the SDK's initialize @@ -1519,6 +1538,42 @@ const readClaudeAppendSystemPromptFile = Effect.fnUntraced(function* (input: { ); }); +const decodeClaudeLaunchArgSettings = Schema.decodeUnknownEffect( + fromLenientJson(Schema.Record(Schema.String, Schema.Unknown)), +); + +/** + * Reads the --settings launch argument: a value starting with a brace is inline + * JSON, anything else is a file path relative to the cwd. makeClaudeQueryOptions + * merges the result beneath T3's own settings. + */ +const readClaudeLaunchArgSettings = Effect.fn("ClaudeAdapterV2.readLaunchArgSettings")( + function* (input: { + readonly settings: ClaudeSettings | undefined; + readonly cwd: string | null; + readonly fileSystem: FileSystem.FileSystem; + readonly path: Path.Path; + }) { + if (input.settings === undefined) return undefined; + const value = parseCliArgs(input.settings.launchArgs).flags.settings; + if (value === undefined) return undefined; + const trimmed = value?.trim() ?? ""; + if (trimmed.length === 0) return yield* new ClaudeSettingsLaunchArgError({}); + // The CLI also wants a closing brace, but then malformed JSON, secrets included, would + // become the file path in the error message. + if (trimmed.startsWith("{")) { + return (yield* decodeClaudeLaunchArgSettings(trimmed).pipe( + Effect.mapError((cause) => new ClaudeSettingsLaunchArgError({ cause })), + )) as ClaudeSdkSettings; + } + const filePath = input.path.resolve(input.cwd ?? ".", trimmed); + return (yield* input.fileSystem.readFileString(filePath).pipe( + Effect.flatMap(decodeClaudeLaunchArgSettings), + Effect.mapError((cause) => new ClaudeSettingsLaunchArgError({ path: filePath, cause })), + )) as ClaudeSdkSettings; + }, +); + export const CLAUDE_T3_MCP_TOOL_WILDCARD = "mcp__t3-code__*"; // Keep in sync with OrchestratorToolkit Tool.Readonly annotations; ClaudeAdapterV2.test.ts checks this. @@ -2422,6 +2477,7 @@ const ClaudeBypassSettings = Schema.StructWithRest( const decodeClaudeBypassSettings = Schema.decodeUnknownEffect( fromLenientJson(ClaudeBypassSettings), ); +const decodeClaudeLaunchArgBypassSettings = Schema.decodeUnknownEffect(ClaudeBypassSettings); const ClaudePromptCacheTtl = Schema.Literals(["5m", "1h"]); const decodeClaudePromptCacheTtl = Schema.decodeUnknownOption(ClaudePromptCacheTtl); @@ -4195,7 +4251,14 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( readBypassSettings, { concurrency: "unbounded" }, ); - return sources.map(claudeBypassAvailability).reduce( + const launchArgSettings = yield* readClaudeLaunchArgSettings({ + settings: adapterOptions.settings, + cwd, + fileSystem, + path, + }); + const launch = yield* decodeClaudeLaunchArgBypassSettings(launchArgSettings ?? {}); + return [...sources, launch].map(claudeBypassAvailability).reduce( (current, source) => ({ bypassDisabled: current.bypassDisabled || source.bypassDisabled, autoDisabled: current.autoDisabled || source.autoDisabled, @@ -4280,10 +4343,13 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( if (path.dirname(directory) === directory) return cwd; } }); - // Settings env beats the process env, local over project over user, and a local file an older - // Claude Code left in cwd applies under the root one. resolveSettings reads local from cwd only. + // Settings env beats the process env, --settings over local over project over user, and a local + // file an older Claude Code left in cwd applies under the root one. resolveSettings reads local + // from cwd only. const resolveToolSearchOverride = Effect.fn("ClaudeAdapterV2.resolveToolSearchOverride")( - function* (cwd: string | null) { + function* (cwd: string | null, launchArgSettings: ClaudeSdkSettings | undefined) { + const launchValue = launchArgSettings?.env?.ENABLE_TOOL_SEARCH; + if (typeof launchValue === "string") return claudeToolSearchOverride(launchValue); const localDirectory = cwd === null ? null : yield* localSettingsDirectory(cwd); const settingsFiles = [ ...(cwd === null || localDirectory === null @@ -9634,6 +9700,12 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( fileSystem, path, }); + const launchArgSettings = yield* readClaudeLaunchArgSettings({ + settings: adapterOptions.settings, + cwd: turnInput.runtimePolicy.cwd, + fileSystem, + path, + }); const closedExistingNativeThreadId = existing !== null ? existing.nativeThreadId : null; if (existing !== null) { yield* existing.query.close.pipe(Effect.ignore); @@ -9656,7 +9728,10 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( openedWithResume || hasPersistedProviderTurn ? undefined : turnInput.providerThread.nativeMetadata?.claudeFork; - const toolSearchOverride = yield* resolveToolSearchOverride(turnInput.runtimePolicy.cwd); + const toolSearchOverride = yield* resolveToolSearchOverride( + turnInput.runtimePolicy.cwd, + launchArgSettings, + ); const queryOptions = makeClaudeQueryOptions({ modelSelection: turnInput.modelSelection, nativeThreadId, @@ -9666,6 +9741,7 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( attachmentsDir, settings: adapterOptions.settings, ...(forkFrom === undefined ? {} : { forkFrom }), + ...(launchArgSettings === undefined ? {} : { sdkSettings: launchArgSettings }), environment: claudeQueryEnvironment( adapterOptions.environment, turnInput.appThread, diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts index 5d79868c09113..08b8183a6934b 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts @@ -36,6 +36,7 @@ vi.mock("../../provider/Drivers/ClaudeAgentSdk.ts", { spy: true }); // Coder: upstream's fork tests copy the transcript through the Agent SDK; the CLI transport // forks natively on the fork's first query (`ClaudeAdapterV2.forkThread`), so they are not carried. +// The runner does no port-discovery agent tracking (#22542), so that test is not carried either. const realQueryRunner = layerQueryRunner.pipe( Layer.provide( Layer.succeed(ProviderEventLoggers, { diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts index a3840aeddb4ae..82bb28abe4945 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts @@ -31,6 +31,7 @@ import { import { assert, describe, it } from "@effect/vitest"; import * as CodexClient from "effect-codex-app-server/client"; import * as CodexReplay from "effect-codex-app-server/replay"; +import * as CodexError from "effect-codex-app-server/errors"; import type * as CodexSchema from "effect-codex-app-server/schema"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; @@ -47,6 +48,11 @@ import * as Queue from "effect/Queue"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; +import * as Sink from "effect/Sink"; +import * as Stdio from "effect/Stdio"; +import * as Exit from "effect/Exit"; +import * as Cause from "effect/Cause"; +import * as PlatformError from "effect/PlatformError"; import { TestClock } from "effect/testing"; import { ChildProcess, ChildProcessSpawner } from "effect/process"; @@ -67,6 +73,9 @@ const replayTranscriptJson = Schema.fromJsonString(CodexReplay.CodexAppServerRep const encodeReplayTranscriptJson = Schema.encodeEffect(replayTranscriptJson); const decodeReplayTranscriptJson = Schema.decodeUnknownEffect(replayTranscriptJson); const encodeStringJson = Schema.encodeEffect(Schema.fromJsonString(Schema.String)); +const isProviderAdapterEventStreamError = Schema.is( + ProviderAdapter.ProviderAdapterEventStreamError, +); describe("CodexAdapterV2 context usage", () => { it("uses the current context rather than cumulative processed tokens", () => { @@ -647,6 +656,7 @@ function codexReplayPreamble(input: { thread: { id: input.nativeThreadId, sessionId: input.nativeThreadId, + projectId: null, forkedFromId: null, preview: "", ephemeral: false, @@ -740,6 +750,154 @@ function withReplayRequestId( : entry; } +describe("CodexAdapterV2 process exit", () => { + it.effect.each([ + { exit: "numeric", active: false }, + { exit: "SIGKILL", active: false }, + { exit: "numeric", active: true }, + { exit: "SIGKILL", active: true }, + ] as const)("retires the shared session after $exit (active turn: $active)", ({ exit, active }) => + Effect.gen(function* () { + const scenario = `process-exit-${exit}-${active}`; + const nativeThreadId = `native-${scenario}`; + const nativeTurnId = `turn-${scenario}`; + const input = yield* Queue.unbounded(); + const emit = (frame: unknown) => + Queue.offer(input, new TextEncoder().encode(JSON.stringify(frame) + "\n")); + const outputEnded = yield* Deferred.make(); + const turnStarted = yield* Deferred.make(); + const turnCompleted = yield* Deferred.make(); + const events: Array = []; + const preamble = codexReplayPreamble({ nativeThreadId, nativeTurnId, prompt: "work" }); + const terminationError = + exit === "numeric" + ? new CodexError.CodexAppServerProcessExitedError({ code: 17, pid: 52 }) + : new CodexError.CodexAppServerTransportError({ + operation: "read-process-exit-status", + pid: 52, + cause: PlatformError.systemError({ + _tag: "Unknown", + module: "ChildProcess", + method: "exitCode", + cause: new Error("Process terminated by SIGKILL"), + }), + }); + const client = yield* CodexClient.make( + Stdio.make({ + args: Effect.succeed([]), + stdin: Stream.fromQueue(input), + stdout: () => + Sink.forEach((chunk: string | Uint8Array) => + Effect.gen(function* () { + const frame = JSON.parse( + typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk), + ) as { readonly id?: number; readonly method: string }; + const response = preamble.find( + (entry) => + entry.type === "emit_inbound" && + entry.label === frame.method && + Predicate.isObject(entry.frame) && + "id" in entry.frame, + ); + if (response?.type === "emit_inbound") { + yield* emit(response.frame); + } + }), + ).pipe(Sink.ensuring(Deferred.succeed(outputEnded, undefined))), + stderr: () => Sink.drain, + }), + {}, + Effect.succeed(terminationError), + ); + const adapter = yield* CodexAdapterV2.makeCodexAdapterV2({ + instanceId: CodexAdapterV2.CODEX_DEFAULT_INSTANCE_ID, + settings: DEFAULT_CODEX_SETTINGS, + environment: {}, + clientFactory: { open: () => Effect.succeed(client) }, + crypto: yield* Crypto.Crypto, + fileSystem: yield* FileSystem.FileSystem, + idAllocator: yield* IdAllocator.IdAllocatorV2, + serverConfig: yield* makeReplayServerConfig(scenario).pipe(Effect.orDie), + }); + const threadId = ThreadId.make(`thread-${scenario}`); + const runtime = yield* adapter.openSession({ + threadId, + providerSessionId: ProviderSessionId.make(`session-${scenario}`), + modelSelection: CODEX_TEST_MODEL_SELECTION, + runtimePolicy: CODEX_TEST_RUNTIME_POLICY, + }); + const eventReader = yield* runtime.events.pipe( + Stream.runForEach((event) => + Effect.sync(() => events.push(event)).pipe( + Effect.andThen( + event.type === "provider_turn.updated" && event.providerTurn.status === "running" + ? Deferred.succeed(turnStarted, undefined) + : event.type === "turn.terminal" + ? Deferred.succeed(turnCompleted, undefined) + : Effect.void, + ), + ), + ), + Effect.forkScoped, + ); + const providerThread = yield* runtime.ensureThread({ + threadId, + modelSelection: CODEX_TEST_MODEL_SELECTION, + runtimePolicy: CODEX_TEST_RUNTIME_POLICY, + }); + yield* runtime.startTurn( + makeCodexTestTurnInput({ + threadId, + providerThread, + now: yield* DateTime.now, + attemptId: RunAttemptId.make(`attempt-${scenario}`), + text: "work", + }), + ); + yield* emit({ + method: "turn/started", + params: { + threadId: nativeThreadId, + turn: makeCodexReplayTurn({ id: nativeTurnId, status: "inProgress" }), + }, + }); + yield* Deferred.await(turnStarted); + if (!active) { + yield* emit({ + method: "turn/completed", + params: { + threadId: nativeThreadId, + turn: makeCodexReplayTurn({ id: nativeTurnId, status: "completed" }), + }, + }); + yield* Deferred.await(turnCompleted); + assert.isTrue( + events.some((event) => event.type === "turn.terminal" && event.status === "completed"), + ); + } + + yield* Queue.end(input); + // Output closes only after the protocol records the process termination. + yield* Deferred.await(outputEnded); + const rpcError = yield* client.request("thread/list", {}).pipe(Effect.flip); + assert.strictEqual(rpcError, terminationError); + // Without the fix the stream never ends, so this waits until the project timeout. + const readerExit = yield* Fiber.await(eventReader); + // A failure, not a clean end, carries the Codex error into the session's lastError. + assert.isTrue( + Exit.isFailure(readerExit) && + isProviderAdapterEventStreamError(Cause.squash(readerExit.cause)), + "the session manager must receive process death so it can retire the shared process", + ); + }).pipe( + Effect.scoped, + Effect.provide( + Layer.mergeAll(IdAllocator.layer, McpProviderSessions.layer, NodeServices.layer), + ), + ), + ); +}); + describe("CodexAdapterV2 session initialize", () => { const openReplaySession = ( transcript: CodexReplay.CodexAppServerReplayTranscript, diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts index 65331ac889bb2..f0e71cdac6066 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts @@ -1717,7 +1717,25 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( model: input.modelSelection.model, now, }); - const events = yield* Queue.unbounded(); + const events = yield* Queue.unbounded< + ProviderAdapter.ProviderAdapterV2Event, + ProviderAdapter.ProviderAdapterEventStreamError + >(); + // Drain final notifications before the session manager retires the dead process. + // Queue.fail lets readers take items already buffered before they see the failure. + yield* client.awaitTermination.pipe( + Effect.catch((cause) => + Queue.fail( + events, + new ProviderAdapter.ProviderAdapterEventStreamError({ + driver: CODEX_PROVIDER, + providerSessionId: input.providerSessionId, + cause, + }), + ), + ), + Effect.forkScoped, + ); const rateLimitSnapshot = yield* Ref.make(undefined); const limitedTurnItems = yield* Ref.make( new Map>(), @@ -4828,6 +4846,8 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( threadId: context.projectionThreadId, providerThreadId: context.providerThread.id, driver: CODEX_PROVIDER, + // Codex starts a new turn from this text, so nothing is buffered. + delivery: "message_text", detail: codexBackgroundCommandDetail(payload.item), notification: { ...backgroundWorkNotification([ diff --git a/apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.ts b/apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.ts index e59ad3da7cd96..ffe8203c8cbbf 100644 --- a/apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.ts +++ b/apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.ts @@ -35,6 +35,7 @@ const instanceId = ProviderInstanceId.make("codex"); const modelSelection = { instanceId, model: "test-model" }; const stopEarlierBackgroundWork = ({ + wakeAfterStop = false, failedStart = false, stopWithQueue, olderStart = false, @@ -42,6 +43,7 @@ const stopEarlierBackgroundWork = ({ stalledRun, workflowMember = false, }: { + readonly wakeAfterStop?: boolean; readonly failedStart?: boolean; readonly backgroundWorkAfterStop?: "running" | "unknown"; readonly stopWithQueue?: "thread.stop" | "run.interrupt"; @@ -192,6 +194,46 @@ const stopEarlierBackgroundWork = ({ }); yield* worker.drain(); yield* Fiber.join(running); + if (wakeAfterStop) { + const send = (id: string, provider = false) => + orchestrator.dispatch({ + type: "message.dispatch", + commandId: CommandId.make(id), + threadId, + messageId: MessageId.make(`message:${id}`), + text: id, + attachments: [], + dispatchMode: { type: "queue_after_active" }, + createdBy: provider ? "agent" : "user", + creationSource: provider ? "provider" : "web", + }); + yield* send("follow-up"); + yield* orchestrator.dispatch({ + type: "thread.stop", + commandId: CommandId.make("stop"), + threadId, + }); + // Stop is committed, but its provider interrupt has not run yet. + yield* send("Background task completed.", true); + const [stopped, followUp, wake] = (yield* orchestrator.getThreadProjection(threadId)) + .runs; + assert.equal(stopped?.status, "running"); + assert.isTrue(followUp?.queueHeld); + assert.isTrue(wake?.queueHeld); + + yield* worker.drain(); + yield* orchestrator.resumeQueuedRuns; + yield* worker.drain(); + const after = yield* orchestrator.getThreadProjection(threadId); + assert.deepEqual( + after.runs.map((run) => run.status), + ["interrupted", "queued", "queued"], + ); + assert.isTrue(after.runs.find((run) => run.id === followUp?.id)?.queueHeld); + assert.isTrue(after.runs.find((run) => run.id === wake?.id)?.queueHeld); + assert.lengthOf(started, 1); + return; + } const first = started[0]!; const codexTurn = (yield* orchestrator.getThreadProjection(threadId)).providerTurns[0]!; const devServerId = TurnItemId.make("turn-item:dev-server"); @@ -1016,3 +1058,7 @@ it.effect.each(["missing-session-terminal", "returned-interrupt", "superseded-at "Stop recovers a stalled run after %s without changing a newer attempt", (stalledRun) => stopEarlierBackgroundWork({ stalledRun }), ); + +it.effect("Stop holds a provider wake that queues before the stopped turn ends", () => + stopEarlierBackgroundWork({ wakeAfterStop: true }), +); diff --git a/apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts b/apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts index 6293d25025a92..77e4cd11ba071 100644 --- a/apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts +++ b/apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts @@ -15,11 +15,14 @@ import { ProviderInstanceId, ProviderThreadId, RunId, + RuntimeRequestId, ThreadId, TurnItemId, } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; +import * as Deferred from "effect/Deferred"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Stream from "effect/Stream"; @@ -40,6 +43,7 @@ import * as EventSink from "./EventSink.ts"; import * as Orchestrator from "./Orchestrator.ts"; import { continueRestartedRun } from "./RestartContinuation.ts"; import * as RuntimeLayer from "./runtimeLayer.ts"; +import * as ThreadCommandExecutor from "./ThreadCommandExecutor.ts"; import * as ProviderTurnStartServiceTestkit from "./ProviderTurnStartService.testkit.ts"; import type * as ProviderAdapter from "@t3tools/provider-core/server/ProviderAdapter"; import * as ProviderContinuationRequests from "@t3tools/provider-core/server/ProviderContinuationRequests"; @@ -104,7 +108,11 @@ const layerTestProviderInstanceRegistry = Layer.succeed( }, ); -const layerTest = Layer.mergeAll(RuntimeLayer.layer, RuntimeLayer.layerEventSink).pipe( +const layerTest = Layer.mergeAll( + RuntimeLayer.layer, + RuntimeLayer.layerEventSink, + ThreadCommandExecutor.layer, +).pipe( Layer.provideMerge(RuntimeLayer.layerProjectService), Layer.provide( Layer.mock(WorkspacePaths.WorkspacePaths)({ @@ -1980,6 +1988,169 @@ it.layer(Layer.merge(layerTest, ProviderContinuationRequests.layer))( } }), ); + + it.effect("serializes a recovered child answer with its parent and tracks the follow-up", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const eventSink = yield* EventSink.EventSinkV2; + const now = yield* DateTime.now; + const parentThreadId = ThreadId.make("thread:question-answer-parent"); + const projectId = ProjectId.make("project:question-answer"); + const parentRunId = RunId.make("run:question-answer-parent"); + const rootNodeId = NodeId.make("node:question-answer-parent"); + yield* seedParentWithTerminalTask({ + threadId: parentThreadId, + projectId, + runId: parentRunId, + rootNodeId, + taskId: NodeId.make("node:question-answer-seed"), + deliveryState: "acknowledged", + now, + }); + const child = yield* seedRestartCancelledChild({ + parentThreadId, + projectId, + parentRunId, + rootNodeId, + name: "question-answer-child", + completionWake: "always", + continuationPending: false, + now, + }); + // The cancelled result is published before anyone answers the question. + yield* orchestrator.recoverDelegatedTasks; + assert.isNotNull( + (yield* orchestrator.getThreadProjection(parentThreadId)).subagents.find( + (task) => task.id === child.taskId, + )?.result, + ); + const parentRun = (yield* orchestrator.getThreadProjection(parentThreadId)).runs.find( + (run) => run.id === parentRunId, + )!; + yield* eventSink.write({ + events: [ + { + id: EventId.make("event:question-answer-parent-completed"), + type: "run.updated", + threadId: parentThreadId, + runId: parentRunId, + occurredAt: now, + payload: { ...parentRun, status: "completed", completedAt: now }, + }, + ], + }); + const requestId = RuntimeRequestId.make("request:question-answer"); + const nodeId = NodeId.make("node:question-answer-request"); + yield* eventSink.write({ + commandId: CommandId.make("command:seed-question-answer"), + events: [ + { + id: EventId.make("event:question-answer-request"), + type: "runtime-request.updated", + threadId: child.childThreadId, + nodeId, + occurredAt: now, + payload: { + id: requestId, + nodeId, + providerTurnId: null, + nativeRequestRef: null, + kind: "user_input", + status: "pending", + responseCapability: { type: "message" }, + createdAt: now, + resolvedAt: null, + }, + }, + { + id: EventId.make("event:question-answer-item"), + type: "turn-item.updated", + threadId: child.childThreadId, + nodeId, + occurredAt: now, + payload: { + id: TurnItemId.make("turn-item:question-answer"), + type: "user_input_request", + threadId: child.childThreadId, + runId: child.childRunId, + nodeId, + providerThreadId: null, + providerTurnId: null, + nativeItemRef: null, + parentItemId: null, + ordinal: 0, + status: "waiting", + title: null, + startedAt: now, + completedAt: null, + updatedAt: now, + requestId, + questions: [ + { id: "color", header: "Color", question: "Which color?", options: [] }, + ], + }, + }, + ], + }); + + const executor = yield* ThreadCommandExecutor.ThreadCommandExecutor; + const parentHeld = yield* Deferred.make(); + const releaseParent = yield* Deferred.make(); + const answerWaiting = yield* Deferred.make(); + const withLock = executor.withLock; + const parentMutation = yield* withLock( + parentThreadId, + Deferred.succeed(parentHeld, undefined).pipe( + Effect.andThen(Deferred.await(releaseParent)), + ), + ).pipe(Effect.forkScoped({ startImmediately: true })); + yield* Deferred.await(parentHeld); + const observeLock: ThreadCommandExecutor.ThreadCommandExecutor["Service"]["withLock"] = ( + key, + effect, + ) => + key === parentThreadId + ? Deferred.succeed(answerWaiting, undefined).pipe(Effect.andThen(withLock(key, effect))) + : withLock(key, effect); + const spy = vi.spyOn(executor, "withLock").mockImplementation(observeLock); + yield* Effect.gen(function* () { + const answer = yield* orchestrator + .dispatch({ + type: "runtime-request.respond", + commandId: CommandId.make("command:question-answer"), + threadId: child.childThreadId, + requestId, + answers: { color: "Blue" }, + }) + .pipe(Effect.forkScoped({ startImmediately: true })); + yield* Effect.raceFirst(Deferred.await(answerWaiting), Fiber.join(answer)); + const waiting = yield* orchestrator.getThreadProjection(child.childThreadId); + assert.equal( + waiting.runtimeRequests.find((row) => row.id === requestId)?.status, + "pending", + ); + assert.isFalse((yield* executor.activeKeys).includes(child.childThreadId)); + yield* Deferred.succeed(releaseParent, undefined); + yield* Fiber.join(parentMutation); + yield* Fiber.join(answer); + }).pipe(Effect.ensuring(Effect.sync(() => spy.mockRestore()))); + + const answerRun = (yield* orchestrator.getThreadProjection(child.childThreadId)).runs.find( + (run) => run.userMessageId === MessageId.make(`async-answer:${requestId}`), + ); + assert.isDefined(answerRun?.delegatedTaskId); + const followup = (yield* orchestrator.getThreadProjection(parentThreadId)).subagents.find( + (task) => task.id === answerRun?.delegatedTaskId, + ); + assert.equal(followup?.childThreadId, child.childThreadId); + assert.isNull(followup?.result); + assert.isTrue( + (yield* orchestrator.getThreadShell(parentThreadId))?.pendingBackgroundTasks?.some( + (task) => task.kind === "subagent" && task.childThreadId === child.childThreadId, + ), + ); + }), + ); }, ); diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index dd892631b776c..e99ed65d4632e 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -62,12 +62,14 @@ import { ProviderInstanceId, ProviderInteractionMode, resolveProviderInstanceEnabled, + resolveProjectProviderInstanceEnabled, type ProviderSessionId, RunId, RuntimeMode, ThreadLinkedPullRequest, ThreadId, TurnItemId, + TrimmedNonEmptyString, } from "@t3tools/contracts"; import { modelSelectionsEqual } from "@t3tools/shared/model"; import { recordThreadOpen } from "@t3tools/shared/paletteSearch"; @@ -166,6 +168,10 @@ import { planThreadDeletion } from "./ThreadDeletion.ts"; import type * as ProviderAdapter from "@t3tools/provider-core/server/ProviderAdapter"; import * as ServerSettings from "../serverSettings.ts"; +const isMessageQuestionAnswer = Schema.is( + Schema.Union([TrimmedNonEmptyString, Schema.NonEmptyArray(TrimmedNonEmptyString)]), +); + export class OrchestratorDispatchError extends Schema.TaggedError()( "OrchestratorDispatchError", { @@ -722,7 +728,7 @@ function delegatedTaskTerminalStatus( function nextQueuedRun( projection: Pick, ): OrchestrationV2Run | undefined { - return queuedRunsInDeliveryOrder(projection)[0]; + return queuedRunsInDeliveryOrder(projection).find((run) => run.queueHeld !== true); } function followsCurrentConversation( @@ -1484,8 +1490,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio if ( projection.thread.archivedAt !== null || projection.thread.deletedAt !== null || - projection.runs.some(isBlockingRun) || - projection.runs.some((run) => run.status === "queued" && run.queueHeld === true) + projection.runs.some(isBlockingRun) ) { return; } @@ -5605,6 +5610,18 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }), ), ); + const queueIsHeld = projection.runs.some( + (candidate) => candidate.status === "queued" && candidate.queueHeld === true, + ); + // Buffered wakes need a T3 run for output the CLI already started producing. + // Stop still holds wakes that arrive before the active run settles. + const wakeSkipsHold = + !worktreeSetupUnresolved && + activeRun !== undefined && + queueIsHeld && + command.createdBy === "agent" && + command.creationSource === "provider" && + !(yield* stopReachedRun(command, command.threadId, activeRun.id)); const run: OrchestrationV2Run = { id: runId, threadId: command.threadId, @@ -5617,10 +5634,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio activeAttemptId: attemptId, status: "queued", ...(worktreeMissingRun !== undefined || - (heldRunsToRelease.length === 0 && - projection.runs.some( - (candidate) => candidate.status === "queued" && candidate.queueHeld === true, - )) + (heldRunsToRelease.length === 0 && queueIsHeld && !wakeSkipsHold) ? { queueHeld: true } : {}), queuePosition: @@ -8516,6 +8530,77 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ); } + const approvalTurnItem = context.item; + const resolvedNodeStatus = + command.decision === "decline" || command.decision === "cancel" + ? ("cancelled" as const) + : ("completed" as const); + const replies: string[] = []; + if ( + runtimeRequest.responseCapability.type === "message" && + resolvedNodeStatus !== "cancelled" + ) { + if (approvalTurnItem?.type !== "user_input_request") { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "The question for this request was not found.", + }); + } + const thread = yield* projectionStore + .getThread(command.threadId) + .pipe(mapDispatchError(command)); + const settings = yield* serverSettings.getSettings.pipe(Effect.orDie); + if ( + !resolveProjectProviderInstanceEnabled( + settings, + thread.projectId, + thread.providerInstanceId, + ) + ) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "The provider for this project is disabled.", + }); + } + const questionIds = new Set(approvalTurnItem.questions.map((question) => question.id)); + if ( + !Object.entries(answers ?? {}).every( + ([questionId, answer]) => + questionIds.has(questionId) || isMessageQuestionAnswer(answer), + ) + ) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "Answer each question before sending.", + }); + } + for (const question of approvalTurnItem.questions) { + const answer = answers?.[question.id]; + const answered = isMessageQuestionAnswer(answer); + // Clients send "" for a skipped optional question, such as an attachment-only answer. + if (!answered && question.required === false) continue; + if (!answered) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "Answer each question before sending.", + }); + } + const text = typeof answer === "string" ? answer : answer.join(", "); + replies.push(`${question.question}\n${text.trim()}`); + } + if (replies.length === 0) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "Enter an answer before sending.", + }); + } + } + const now = yield* DateTime.now; const resolvedRequest = { ...runtimeRequest, @@ -8589,10 +8674,6 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }, }); } - const resolvedNodeStatus = - command.decision === "decline" || command.decision === "cancel" - ? ("cancelled" as const) - : ("completed" as const); yield* emitEvent({ type: "runtime-request.updated", threadId: command.threadId, @@ -8628,7 +8709,6 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }); } - const approvalTurnItem = context.item; if (approvalTurnItem !== undefined) { yield* emitEvent({ type: "turn-item.updated", @@ -8667,33 +8747,6 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio } if (runtimeRequest.responseCapability.type === "message") { if (resolvedNodeStatus === "cancelled") return; - if (approvalTurnItem?.type !== "user_input_request") { - return yield* new OrchestratorDispatchError({ - commandId: command.commandId, - commandType: command.type, - cause: "The question for this request was not found.", - }); - } - const replies: string[] = []; - for (const question of approvalTurnItem.questions) { - const answer = answers?.[question.id]; - if (typeof answer !== "string" || answer.trim().length === 0) { - if (question.required === false) continue; - return yield* new OrchestratorDispatchError({ - commandId: command.commandId, - commandType: command.type, - cause: "Answer each question before sending.", - }); - } - replies.push(`${question.question}\n${answer.trim()}`); - } - if (replies.length === 0) { - return yield* new OrchestratorDispatchError({ - commandId: command.commandId, - commandType: command.type, - cause: "Enter an answer before sending.", - }); - } let dispatchMode: Extract< OrchestrationV2Command, { type: "message.dispatch" } @@ -8727,21 +8780,19 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio dispatchMode = { type: "steer_active", targetRunId: activeRun.id }; } } - return yield* dispatchMessage( - { - type: "message.dispatch", - commandId: command.commandId, - threadId: command.threadId, - messageId: MessageId.make(`async-answer:${command.requestId}`), - text: replies.join("\n\n"), - attachments: [], - createdBy: "user", - creationSource: "server", - dispatchMode, - }, - events, - effects, - ); + const answer = { + type: "message.dispatch" as const, + commandId: command.commandId, + threadId: command.threadId, + messageId: MessageId.make(`async-answer:${command.requestId}`), + text: replies.join("\n\n"), + attachments: [], + createdBy: "user" as const, + creationSource: "server" as const, + dispatchMode, + }; + yield* dispatchMessage(answer, events, effects); + return yield* recordSubagentFollowup(answer, events).pipe(mapDispatchError(answer)); } if (providerSessionId === null) return; yield* Ref.update(effects, (existing) => [ @@ -12629,7 +12680,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }); } } - const nextRun = selectedRun ?? nextQueuedRun(projection); + const nextRun = selectedRun ?? queuedRunsInDeliveryOrder(projection)[0]; if ( usageLimitHoldsQueue( projection, @@ -12968,7 +13019,11 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio commandThreadId(command), dispatchWithReceiptEffect(command), ); - if (command.type !== "message.dispatch" || !mayStartSubagentFollowup(command)) + // Answering a message-mode question can also start a follow-up. + if ( + command.type !== "runtime-request.respond" && + (command.type !== "message.dispatch" || !mayStartSubagentFollowup(command)) + ) return yield* dispatch; const parentThreadId = yield* appOwnedSubagentParentThreadId(command.threadId).pipe( Effect.mapError( diff --git a/apps/server/src/orchestration-v2/ProjectionStore.test.ts b/apps/server/src/orchestration-v2/ProjectionStore.test.ts index 98c2591962e12..328e19edb3f3f 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.test.ts @@ -186,6 +186,41 @@ const addRolledBackRecoveryCandidate = Effect.fn("addRolledBackRecoveryCandidate return threadId; }); +it.effect.each([ + ["memory", ProjectionStore.layerMemory], + ["SQL", layerTest], +] as const)("%s queue readiness skips held runs without bypassing active work", ([, layer]) => + Effect.gen(function* () { + const store = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* addRolledBackRecoveryCandidate("held-queue"); + const { runs } = yield* store.getThreadProjection(threadId); + const run = runs[0]!; + const now = yield* DateTime.now; + const held = { ...run, status: "queued" as const, queueHeld: true }; + yield* store.apply({ + id: EventId.make("held-queue:held"), + type: "run.updated", + threadId, + occurredAt: now, + payload: held, + }); + assert.isFalse(yield* store.canStartQueuedRun(threadId)); + + const wake = { ...run, id: RunId.make("held-queue:wake"), ordinal: 2 }; + for (const status of ["running", "queued", "completed"] as const) { + yield* store.apply({ + id: EventId.make(`held-queue:wake:${status}`), + type: "run.updated", + threadId, + occurredAt: now, + payload: { ...wake, status }, + }); + assert.equal(yield* store.canStartQueuedRun(threadId), status === "queued"); + } + assert.deepEqual((yield* store.getThreadProjection(threadId)).runs[0], held); + }).pipe(Effect.provide(layer)), +); + it.effect.each([ ["memory", ProjectionStore.layerMemory], ["SQL", layerTest], diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 913ab3836e0bb..0f71a69506469 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -4499,6 +4499,10 @@ export const layer: Layer.Layer = AND node.status IN ('pending', 'starting', 'running', 'waiting') AND ( (node.run_id IS NULL AND node.kind = 'root_turn') + OR node.node_id IN ( + SELECT node_id FROM orchestration_v2_projection_runtime_requests + WHERE thread_id = ${threadId} AND status = 'pending' AND kind = 'user_input' + ) OR node.run_id IN ( SELECT run_id FROM orchestration_v2_projection_runs WHERE thread_id = ${threadId} @@ -4677,6 +4681,13 @@ export const layer: Layer.Layer = AND status IN ('queued', 'preparing', 'starting', 'running', 'waiting') ) OR item.type IN ('command_execution', 'dynamic_tool', 'subagent') + OR ( + item.type = 'user_input_request' + AND json_extract(item.payload_json, '$.requestId') IN ( + SELECT runtime_request_id FROM orchestration_v2_projection_runtime_requests + WHERE thread_id = ${threadId} AND status = 'pending' AND kind = 'user_input' + ) + ) OR ( item.run_id IS NULL AND item.node_id IN ( @@ -5293,11 +5304,11 @@ export const layer: Layer.Layer = SELECT 1 AS ready FROM orchestration_v2_projection_threads AS thread WHERE thread.thread_id = ${threadId} AND thread.archived_at IS NULL AND thread.deleted_at IS NULL AND EXISTS (SELECT 1 FROM orchestration_v2_projection_runs - WHERE thread_id = ${threadId} AND status = 'queued') + WHERE thread_id = ${threadId} AND status = 'queued' + AND json_extract(payload_json, '$.queueHeld') IS NOT 1) AND NOT EXISTS (SELECT 1 FROM orchestration_v2_projection_runs WHERE thread_id = ${threadId} - AND (status IN ('preparing', 'starting', 'running', 'waiting') - OR (status = 'queued' AND json_extract(payload_json, '$.queueHeld') = 1)))`; + AND status IN ('preparing', 'starting', 'running', 'waiting'))`; return rows.length > 0; }), ) @@ -7556,14 +7567,13 @@ export const layerMemory: Layer.Layer = Layer.effect( return ( projection.thread.archivedAt === null && projection.thread.deletedAt === null && - projection.runs.some((run) => run.status === "queued") && + projection.runs.some((run) => run.status === "queued" && run.queueHeld !== true) && !projection.runs.some( (run) => run.status === "preparing" || run.status === "starting" || run.status === "running" || - run.status === "waiting" || - (run.status === "queued" && run.queueHeld === true), + run.status === "waiting", ) ); }), diff --git a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts index 910dc97bfdafd..23a2a5b99b4c1 100644 --- a/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts +++ b/apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts @@ -2,6 +2,7 @@ import { runRanAfter } from "@t3tools/shared/orchestrationV2ThreadError"; import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { CommandId, + isProviderNativeSubagentThread, type OrchestrationV2DomainEvent, type ProviderThreadId, type OrchestrationV2RestartCancelledBackgroundWork, @@ -232,16 +233,37 @@ export const make = Effect.gen(function* () { } runs.push(run); } + const pendingRequests = projection.runtimeRequests.filter( + (request) => request.status === "pending", + ); + // The lost provider callback cannot resume; persisted questions can be + // answered by a new user message in threads that accept messages. + const questions = pendingRequests.filter( + (request) => + request.kind === "user_input" && + request.responseCapability.type === "live" && + projection.turnItems.some( + (item) => + item.type === "user_input_request" && + item.requestId === request.id && + item.nodeId === request.nodeId && + item.questions.length > 0 && + item.questions.every((question) => question.question.trim().length > 0), + ) && + !isProviderNativeSubagentThread(projection.thread), + ); + const recoveredRequestIds = new Set(questions.map((request) => request.id)); const messageRequestNodeIds = new Set( - projection.runtimeRequests + pendingRequests .filter( (request) => - request.status === "pending" && request.responseCapability.type === "message", + request.responseCapability.type === "message" || recoveredRequestIds.has(request.id), ) .map((request) => request.nodeId), ); - const requests = projection.runtimeRequests.filter( - (request) => request.status === "pending" && request.responseCapability.type !== "message", + const requests = pendingRequests.filter( + (request) => + request.responseCapability.type !== "message" && !recoveredRequestIds.has(request.id), ); // Delegated task rows, items and nodes stay open: the child settles them. const delegatedTaskNodeIds = new Set([ @@ -338,6 +360,16 @@ export const make = Effect.gen(function* () { payload: { ...run, queueHeld: true }, }); } + for (const question of questions) { + events.push({ + id: yield* allocateEventId(), + type: "runtime-request.updated", + threadId: projection.thread.id, + nodeId: question.nodeId, + occurredAt: now, + payload: { ...question, responseCapability: { type: "message" } }, + }); + } for (const request of requests) { events.push({ id: yield* allocateEventId(), diff --git a/apps/server/src/orchestration-v2/ProviderSessionManager.test.ts b/apps/server/src/orchestration-v2/ProviderSessionManager.test.ts index 8d84f578660cc..726ee561fa064 100644 --- a/apps/server/src/orchestration-v2/ProviderSessionManager.test.ts +++ b/apps/server/src/orchestration-v2/ProviderSessionManager.test.ts @@ -2915,6 +2915,92 @@ it.effect("ProviderSessionManagerV2 settles a request the event pump persists du }), ); +it.effect.each(["manual_shutdown", "server_shutdown"] as const)( + "ProviderSessionManagerV2 preserves questions only for server shutdown (%s)", + (reason) => + Effect.gen(function* () { + const state = yield* Ref.make(emptyState); + const effect = Effect.gen(function* () { + const eventSink = yield* EventSink.EventSinkV2; + const idAllocator = yield* IdAllocator.IdAllocatorV2; + const manager = yield* ProviderSessionManager.ProviderSessionManagerV2; + const projectionStore = yield* ProjectionStore.ProjectionStoreV2; + const now = yield* DateTime.now; + const projectId = yield* idAllocator.allocate.project({ + fixtureName: "provider-session-manager-request-expire", + }); + const threadId = yield* idAllocator.allocate.thread({ + fixtureName: "provider-session-manager-request-expire", + projectId, + }); + const providerSessionId = yield* idAllocator.allocate.providerSession({ + providerInstanceId: modelSelection.instanceId, + threadId, + }); + const providerThread = makeProviderThread({ + idAllocator, + threadId, + providerSessionId, + now, + }); + + yield* eventSink.write({ + events: [yield* makeThreadCreatedEvent({ idAllocator, threadId, now })], + }); + const pendingRequest = yield* makePendingRuntimeRequestEvents({ + idAllocator, + threadId, + providerSessionId, + providerThread, + now, + }); + yield* eventSink.write({ + events: pendingRequest.events.map((event) => + event.type === "turn-item.updated" + ? { + ...event, + payload: { + ...event.payload, + type: "user_input_request", + questions: [ + { id: "color", header: "Color", question: "Which color?", options: [] }, + ], + }, + } + : event.type === "runtime-request.updated" + ? { ...event, payload: { ...event.payload, kind: "user_input" } } + : event, + ), + }); + yield* manager.open({ + threadId, + providerSessionId, + modelSelection, + runtimePolicy, + }); + yield* manager.release({ + providerSessionId, + reason, + }); + + const projection = yield* projectionStore.getThreadProjection(threadId); + const request = projection.runtimeRequests.at(-1); + const requestNode = projection.nodes.find((node) => node.id === request?.nodeId); + const requestTurnItem = projection.turnItems.find( + (item) => item.type === "user_input_request" && item.requestId === request?.id, + ); + + const preserved = reason === "server_shutdown"; + assert.equal(request?.status, preserved ? "pending" : "cancelled"); + assert.equal(request?.responseCapability.type, preserved ? "live" : "not_resumable"); + assert.equal(requestNode?.status, preserved ? "waiting" : "cancelled"); + assert.equal(requestTurnItem?.status, preserved ? "waiting" : "cancelled"); + }); + + yield* effect.pipe(Effect.provide(layerTest({ state, idleTimeoutMs: 1000 }))); + }), +); + it.effect("ProviderSessionManagerV2 persists session-scoped runtime requests without a run", () => Effect.gen(function* () { const state = yield* Ref.make(emptyState); diff --git a/apps/server/src/orchestration-v2/ProviderSessionManager.ts b/apps/server/src/orchestration-v2/ProviderSessionManager.ts index 53b722447d15d..d707eaa686205 100644 --- a/apps/server/src/orchestration-v2/ProviderSessionManager.ts +++ b/apps/server/src/orchestration-v2/ProviderSessionManager.ts @@ -626,7 +626,9 @@ export const layerWithOptions = ( request.status === "pending" && request.responseCapability.type === "live" && request.responseCapability.providerSessionId === providerSessionId && - DateTime.isLessThanOrEqualTo(request.createdAt, input.releasedAt), + DateTime.isLessThanOrEqualTo(request.createdAt, input.releasedAt) && + // Shutdown reconciliation decides which questions can survive the lost callback. + !(input.reason === "server_shutdown" && request.kind === "user_input"), ); for (const request of releasedRequests) { diff --git a/apps/server/src/orchestration-v2/ProviderTurnStartService.ts b/apps/server/src/orchestration-v2/ProviderTurnStartService.ts index 2e0eee3828ea7..94ba406b106f5 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnStartService.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnStartService.ts @@ -95,13 +95,14 @@ const isProviderTurnStartError = Schema.is(ProviderTurnStartError); /** * Claude refuses these before it reads the prompt: replacing a process running background work, - * launch arguments whose appended system prompt cannot be read, or MCP registration that never - * answers while the query opens. + * launch arguments whose appended system prompt or settings cannot be read, or MCP registration + * that never answers while the query opens. */ const PROMPT_REFUSAL_TAGS = [ "ClaudeBackgroundWorkBlocksQueryReplacementError", "ClaudeAppendSystemPromptFileError", "ClaudeAppendSystemPromptMissingValueError", + "ClaudeSettingsLaunchArgError", "ClaudeMcpRegistrationTimeoutError", ] as const; diff --git a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts index d283e3b40a132..4fc416023fb99 100644 --- a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts @@ -1323,6 +1323,21 @@ it.effect.each(["failed", "interrupted"] as const)( createdBy: "user", creationSource: "web", }); + yield* threads.dispatch({ + type: "message.dispatch", + commandId: CommandId.make(`command:launch:late-queued-follow-up-${ending}:wake`), + threadId: launched.threadId, + messageId: MessageId.make(`message:launch:late-queued-follow-up-${ending}:wake`), + text: "Background task completed.", + attachments: [], + dispatchMode: { type: "queue_after_active" }, + createdBy: "agent", + creationSource: "provider", + }); + const wake = (yield* threads.getThreadProjection(launched.threadId)).runs.at(-1)!; + assert.equal(wake.status, "queued"); + assert.isTrue(wake.queueHeld); + // Nor does the next one, though the run that message started is now the // latest and carries no workspace preparation. yield* threads.sendToThread({ diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 31060d2774ae3..9402123145583 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -18,6 +18,7 @@ import { TurnItemId, type ModelSelection, type OrchestrationV2Run, + type OrchestrationV2RuntimeRequest, ProjectId, type PullRequestDetail, type PullRequestComment, @@ -1072,15 +1073,288 @@ it.layer(layerTest)("OrchestrationV2LayerLive", (it) => { }), ); + const seedPendingQuestion = Effect.fnUntraced(function* ( + threadId: ThreadId, + runId: RunId | null, + responseCapability: OrchestrationV2RuntimeRequest["responseCapability"], + ) { + const eventSink = yield* EventSink.EventSinkV2; + const now = yield* DateTime.now; + const requestId = RuntimeRequestId.make(`${threadId}:request`); + const nodeId = NodeId.make(`${threadId}:node`); + const itemId = TurnItemId.make(`${threadId}:item`); + yield* eventSink.write({ + commandId: CommandId.make(`${threadId}:seed`), + events: [ + { + id: EventId.make(`${threadId}:node-event`), + type: "node.updated", + threadId, + nodeId, + occurredAt: now, + payload: { + id: nodeId, + threadId, + runId, + parentNodeId: null, + rootNodeId: nodeId, + kind: "user_input_request", + status: "waiting", + countsForRun: false, + providerThreadId: null, + providerTurnId: null, + nativeItemRef: null, + runtimeRequestId: requestId, + checkpointScopeId: null, + startedAt: now, + completedAt: null, + }, + }, + { + id: EventId.make(`${threadId}:request-event`), + type: "runtime-request.updated", + threadId, + nodeId, + occurredAt: now, + payload: { + id: requestId, + nodeId, + providerTurnId: null, + nativeRequestRef: null, + kind: "user_input", + status: "pending", + responseCapability, + createdAt: now, + resolvedAt: null, + }, + }, + { + id: EventId.make(`${threadId}:item-event`), + type: "turn-item.updated", + threadId, + nodeId, + occurredAt: now, + payload: { + id: itemId, + type: "user_input_request", + threadId, + runId, + nodeId, + providerThreadId: null, + providerTurnId: null, + nativeItemRef: null, + parentItemId: null, + ordinal: 0, + status: "waiting", + title: null, + startedAt: now, + completedAt: null, + updatedAt: now, + requestId, + questions: [ + { id: "color", header: "Color", question: "Which color?", options: [] }, + { + id: "sizes", + header: "Sizes", + question: "Which sizes?", + options: [], + multiSelect: true, + }, + { + id: "note", + header: "Note", + question: "Anything else?", + options: [], + required: false, + }, + ], + }, + }, + ], + }); + return { requestId, nodeId, itemId }; + }); + + it.effect.each([ + { trigger: "startup", scope: "run" }, + { trigger: "shutdown", scope: "session" }, + ] as const)( + "recovers only reconstructable user questions and keeps their cards pending on $trigger ($scope)", + ({ trigger, scope }) => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const eventSink = yield* EventSink.EventSinkV2; + const now = yield* DateTime.now; + const threadId = ThreadId.make(`runtime-recovered-question-${trigger}-${scope}`); + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make(`runtime-recovered-question-create-${trigger}-${scope}`), + createdBy: "user", + creationSource: "web", + threadId, + projectId: ProjectId.make(`runtime-recovered-question-project-${trigger}-${scope}`), + title: "Async question", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: process.cwd(), + }); + if (scope === "run") { + yield* orchestrator.dispatch({ + type: "message.dispatch", + commandId: CommandId.make(`${threadId}:run`), + threadId, + messageId: MessageId.make(`${threadId}:message`), + text: "Ask me a question", + attachments: [], + createdBy: "user", + creationSource: "web", + dispatchMode: { type: "start_immediately" }, + }); + } + const runId = (yield* orchestrator.getThreadProjection(threadId)).runs[0]?.id ?? null; + const { requestId, nodeId, itemId } = yield* seedPendingQuestion(threadId, runId, { + type: "live", + providerSessionId: ProviderSessionId.make(`lost-session-${trigger}-${scope}`), + }); + const recovery = yield* ProviderRuntimeRecoveryService.ProviderRuntimeRecoveryService; + const original = yield* orchestrator.getThreadProjection(threadId); + const request = original.runtimeRequests[0]!; + const questionItem = original.turnItems.find((item) => item.type === "user_input_request")!; + const nativeThreadId = ThreadId.make(`runtime-recovered-native-child-${trigger}-${scope}`); + const nativeRequestId = RuntimeRequestId.make( + `runtime-recovered-native-request-${trigger}-${scope}`, + ); + const approvalId = RuntimeRequestId.make(`runtime-recovered-approval-${trigger}-${scope}`); + const missingTextId = RuntimeRequestId.make( + `runtime-recovered-missing-text-${trigger}-${scope}`, + ); + yield* eventSink.write({ + events: [ + { + id: EventId.make(`recovery-approval-${trigger}-${scope}`), + type: "runtime-request.updated", + threadId, + occurredAt: now, + payload: { ...request, id: approvalId, kind: "command" }, + }, + { + id: EventId.make(`recovery-missing-text-${trigger}-${scope}`), + type: "runtime-request.updated", + threadId, + occurredAt: now, + payload: { ...request, id: missingTextId }, + }, + { + id: EventId.make(`recovery-native-thread-${trigger}-${scope}`), + type: "thread.created", + threadId: nativeThreadId, + occurredAt: now, + payload: { + ...original.thread, + id: nativeThreadId, + creationSource: "provider", + lineage: { + parentThreadId: threadId, + rootThreadId: threadId, + relationshipToParent: "subagent", + }, + }, + }, + { + id: EventId.make(`recovery-native-request-${trigger}-${scope}`), + type: "runtime-request.updated", + threadId: nativeThreadId, + occurredAt: now, + payload: { ...request, id: nativeRequestId }, + }, + { + id: EventId.make(`recovery-native-item-${trigger}-${scope}`), + type: "turn-item.updated", + threadId: nativeThreadId, + occurredAt: now, + payload: { + ...questionItem, + id: TurnItemId.make(`recovery-native-item-${trigger}-${scope}`), + type: "user_input_request", + threadId: nativeThreadId, + runId: null, + requestId: nativeRequestId, + questions: [ + { id: "color", header: "Color", question: "Which color?", options: [] }, + ], + }, + }, + ], + }); + yield* recovery.reconcile(trigger); + const recovered = yield* orchestrator.getThreadProjection(threadId); + if (scope === "run") { + assert.equal(recovered.runs[0]?.status, "cancelled"); + } else { + assert.isEmpty(recovered.runs); + } + assert.deepEqual( + recovered.runtimeRequests.find((row) => row.id === requestId), + { + ...request, + responseCapability: { type: "message" }, + }, + ); + assert.equal(recovered.nodes.find((row) => row.id === nodeId)?.status, "waiting"); + assert.equal(recovered.turnItems.find((row) => row.id === itemId)?.status, "waiting"); + for (const id of [approvalId, missingTextId]) { + const closed = recovered.runtimeRequests.find((row) => row.id === id); + assert.equal(closed?.status, trigger === "startup" ? "expired" : "cancelled"); + assert.equal(closed?.responseCapability.type, "not_resumable"); + } + const native = yield* orchestrator.getThreadProjection(nativeThreadId); + assert.equal( + native.runtimeRequests[0]?.status, + trigger === "startup" ? "expired" : "cancelled", + ); + assert.equal(native.runtimeRequests[0]?.responseCapability.type, "not_resumable"); + yield* recovery.reconcile("shutdown"); + yield* recovery.reconcile("startup"); + assert.deepEqual( + (yield* orchestrator.getThreadProjection(threadId)).runtimeRequests, + recovered.runtimeRequests, + ); + yield* orchestrator.dispatch({ + type: "runtime-request.respond", + commandId: CommandId.make(`recovery-answer-${trigger}-${scope}`), + threadId, + requestId, + answers: { color: "Blue", sizes: ["Small", "Large"] }, + }); + const answered = yield* orchestrator.getThreadProjection(threadId); + assert.equal( + answered.runtimeRequests.find((row) => row.id === requestId)?.status, + "resolved", + ); + assert.equal(answered.nodes.find((row) => row.id === nodeId)?.status, "completed"); + assert.equal(answered.turnItems.find((row) => row.id === itemId)?.status, "completed"); + assert.lengthOf(answered.runs, recovered.runs.length + 1); + assert.equal(answered.messages.at(-1)?.runId, answered.runs.at(-1)?.id); + assert.equal( + answered.messages.at(-1)?.text, + "Which color?\nBlue\n\nWhich sizes?\nSmall, Large", + ); + assert.deepEqual( + (yield* (yield* EffectOutbox.EffectOutboxV2).listByCommandId( + CommandId.make(`recovery-answer-${trigger}-${scope}`), + )).map((effect) => effect.request.type), + ["provider-turn.start"], + ); + }), + ); + it.effect("answers an async question after its provider exits and commits the answer once", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; const eventSink = yield* EventSink.EventSinkV2; - const now = yield* DateTime.now; const threadId = ThreadId.make("runtime-async-question"); - const requestId = RuntimeRequestId.make("runtime-async-question-request"); - const nodeId = NodeId.make("runtime-async-question-node"); - const itemId = TurnItemId.make("runtime-async-question-item"); yield* orchestrator.dispatch({ type: "thread.create", commandId: CommandId.make("runtime-async-question-create"), @@ -1095,100 +1369,40 @@ it.layer(layerTest)("OrchestrationV2LayerLive", (it) => { branch: null, worktreePath: process.cwd(), }); - yield* eventSink.write({ - commandId: CommandId.make("runtime-async-question-seed"), - events: [ - { - id: EventId.make("runtime-async-question-node-event"), - type: "node.updated", - threadId, - nodeId, - occurredAt: now, - payload: { - id: nodeId, - threadId, - runId: null, - parentNodeId: null, - rootNodeId: nodeId, - kind: "user_input_request", - status: "waiting", - countsForRun: false, - providerThreadId: null, - providerTurnId: null, - nativeItemRef: null, - runtimeRequestId: requestId, - checkpointScopeId: null, - startedAt: now, - completedAt: null, - }, - }, - { - id: EventId.make("runtime-async-question-request-event"), - type: "runtime-request.updated", - threadId, - nodeId, - occurredAt: now, - payload: { - id: requestId, - nodeId, - providerTurnId: null, - nativeRequestRef: null, - kind: "user_input", - status: "pending", - responseCapability: { type: "message" }, - createdAt: now, - resolvedAt: null, - }, - }, - { - id: EventId.make("runtime-async-question-item-event"), - type: "turn-item.updated", - threadId, - nodeId, - occurredAt: now, - payload: { - id: itemId, - type: "user_input_request", - threadId, - runId: null, - nodeId, - providerThreadId: null, - providerTurnId: null, - nativeItemRef: null, - parentItemId: null, - ordinal: 0, - status: "waiting", - title: null, - startedAt: now, - completedAt: null, - updatedAt: now, - requestId, - responseMode: "message", - questions: [{ id: "color", header: "Color", question: "Which color?", options: [] }], - }, - }, - ], + const { requestId, nodeId, itemId } = yield* seedPendingQuestion(threadId, null, { + type: "message", }); - const invalid = yield* orchestrator - .dispatch({ - type: "runtime-request.respond", - commandId: CommandId.make("runtime-async-question-blank"), - threadId, - requestId, - answers: { color: " " }, - }) - .pipe(Effect.result); - assert.equal(invalid._tag, "Failure"); - const unanswered = yield* orchestrator.getThreadProjection(threadId); - assert.equal(unanswered.runtimeRequests[0]?.status, "pending"); - assert.deepEqual(unanswered.messages, []); + const outbox = yield* EffectOutbox.EffectOutboxV2; + const beforeInvalid = yield* orchestrator.getThreadProjection(threadId); + const beforeSequence = yield* eventSink.latestSequence(); + for (const [index, answers] of [ + { color: " ", sizes: ["Small"] }, + { color: "Blue", sizes: [] }, + { color: "Blue", sizes: ["Small", " "] }, + { color: "Blue", sizes: ["Small"], extra: " " }, + ].entries()) { + const commandId = CommandId.make(`runtime-async-question-invalid:${index}`); + const invalid = yield* orchestrator + .dispatch({ + type: "runtime-request.respond", + commandId, + threadId, + requestId, + answers, + }) + .pipe(Effect.result); + assert.equal(invalid._tag, "Failure", JSON.stringify(answers)); + assert.deepEqual(yield* orchestrator.getThreadProjection(threadId), beforeInvalid); + assert.equal(yield* eventSink.latestSequence(), beforeSequence); + assert.deepEqual(yield* outbox.listByCommandId(commandId), []); + } const command = { type: "runtime-request.respond" as const, commandId: CommandId.make("runtime-async-question-answer"), threadId, requestId, - answers: { color: " Blue " }, + answers: { color: " Blue ", sizes: ["Small", "Large"], note: "" }, }; const accepted = yield* orchestrator.dispatch(command); const repeated = yield* orchestrator.dispatch(command); @@ -1205,13 +1419,21 @@ it.layer(layerTest)("OrchestrationV2LayerLive", (it) => { requestId, answers: command.answers, attachmentsByQuestionId: {}, - questionTextById: { color: "Which color?" }, + questionTextById: { + color: "Which color?", + sizes: "Which sizes?", + note: "Anything else?", + }, }); } assert.equal(answered.messages.length, 1); - assert.equal(answered.messages[0]?.text, "Which color?\nBlue"); + assert.equal(answered.messages[0]?.text, "Which color?\nBlue\n\nWhich sizes?\nSmall, Large"); assert.equal(answered.messages[0]?.role, "user"); assert.equal(answered.runs.length, 1); + assert.deepEqual( + (yield* outbox.listByCommandId(command.commandId)).map((effect) => effect.request.type), + ["provider-turn.start"], + ); const duplicate = yield* orchestrator .dispatch({ @@ -4659,6 +4881,23 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { threadId, runId: limited.id, }); + yield* eventSink.write({ + events: [ + { + id: EventId.make(`${threadId}:hold-available`), + type: "run.updated", + threadId, + runId: available.id, + occurredAt: now, + payload: { ...available, queueHeld: true }, + }, + ], + }); + yield* orchestrator.dispatch({ + type: "queue.resume", + commandId: CommandId.make(`${threadId}:resume-available`), + threadId, + }); yield* orchestrator.resumeQueuedRuns; const after = yield* orchestrator.getThreadProjection(threadId); const started = after.runs.find((run) => run.id === available.id); @@ -5041,6 +5280,80 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("starts a provider wake queued beside a held queue", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const recovery = yield* ProviderRuntimeRecoveryService.ProviderRuntimeRecoveryService; + const events = yield* EventSink.EventSinkV2; + const threadId = ThreadId.make("queue-hold-provider-wake"); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make(`${threadId}:create`), + threadId, + projectId: ProjectId.make(`${threadId}:project`), + title: "Wake beside a held queue", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: process.cwd(), + }); + const send = (index: number, text: string, source: "web" | "provider" | "server" = "web") => + orchestrator.dispatch({ + type: "message.dispatch", + createdBy: source === "web" ? "user" : "agent", + creationSource: source, + commandId: CommandId.make(`${threadId}:message:${index}`), + threadId, + messageId: MessageId.make(`${threadId}:message:${index}`), + text, + attachments: [], + modelSelection, + dispatchMode: { type: index === 0 ? "start_immediately" : "queue_after_active" }, + }); + yield* send(0, "Active"); + yield* send(1, "Queued before the restart"); + yield* recovery.reconcile("startup"); + yield* send(2, "First background task completed.", "provider"); + yield* send(3, "Background task completed.", "provider"); + yield* send(4, "Continue after the command completed.", "server"); + const queued = yield* orchestrator.getThreadProjection(threadId); + const [held, active, wake] = [queued.runs[1]!, queued.runs[2]!, queued.runs[3]!]; + assert.equal(active.status, "starting"); + assert.isTrue(held.queueHeld); + assert.equal(wake.status, "queued"); + assert.notEqual(wake.queueHeld, true); + const textContinuation = queued.runs[4]!; + assert.isTrue(textContinuation.queueHeld); + const now = yield* DateTime.now; + yield* events.write({ + events: [ + { + id: EventId.make(`${threadId}:active-done`), + type: "run.updated", + threadId, + runId: active.id, + occurredAt: now, + payload: { ...active, status: "completed", completedAt: now }, + }, + ], + }); + yield* orchestrator.resumeQueuedRuns; + const after = yield* orchestrator.getThreadProjection(threadId); + assert.equal(after.runs.find((run) => run.id === wake.id)?.status, "starting"); + assert.deepEqual( + after.runs.find((run) => run.id === held.id), + held, + ); + assert.deepEqual( + after.runs.find((run) => run.id === textContinuation.id), + textContinuation, + ); + }), + ); + it.effect("edits and removes queued runs", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; diff --git a/apps/server/src/process/agentScope.test.ts b/apps/server/src/process/agentScope.test.ts index 44e9d6dbf4c2a..94ca314ac1f33 100644 --- a/apps/server/src/process/agentScope.test.ts +++ b/apps/server/src/process/agentScope.test.ts @@ -227,6 +227,24 @@ describe("AgentScope service", () => { ), ); + it.effect("delegates every agent scope so rootless containers can manage their cgroups", () => + withFakeSystemd(({ scope }) => + Effect.gen(function* () { + const agent = yield* scope.wrap({ command: "agent", args: [], name: "t", threadId: "a" }); + const codex = yield* launchCodex(scope, "a"); + const terminal = yield* scope.wrap({ + command: "agent", + args: [], + name: "terminal", + survivesRestart: true, + }); + for (const launch of [agent, codex, terminal]) { + expect(launch.args).toContain("--property=Delegate=yes"); + } + }), + ), + ); + it.effect("stops agents a killed server left before the first launch", () => withFakeSystemd(({ scope, units, calls, stateDir }) => Effect.gen(function* () { diff --git a/apps/server/src/process/agentScope.ts b/apps/server/src/process/agentScope.ts index 1e54691161ae4..cc6d3a04121f6 100644 --- a/apps/server/src/process/agentScope.ts +++ b/apps/server/src/process/agentScope.ts @@ -83,7 +83,8 @@ function parseCgroupCounter(text: string, key: string): number | undefined { } // systemd's default 90 s stop timeout would outlast our leftover-agent wait. -const STOP_TIMEOUT_OPTION = "--property=TimeoutStopSec=10s"; +// Delegation lets rootless containers (Podman, kind) manage their own cgroups. +const SCOPE_OPTIONS = ["--property=TimeoutStopSec=10s", "--property=Delegate=yes"]; const STOPPING_POLL_INTERVAL = "100 millis"; const STOPPING_POLL_ATTEMPTS = 20; const CLEAR_FAILED_INTERVAL = "1 second"; @@ -167,8 +168,8 @@ export const make = Effect.gen(function* () { "true", ]).pipe(Effect.map((result) => result._tag === "Some" && result.value.code === 0)); // Scopes take OOMPolicy from systemd 253; older managers still get scopes. - const supportsOomPolicy = yield* probe([STOP_TIMEOUT_OPTION, "--property=OOMPolicy=stop"]); - if (!supportsOomPolicy && !(yield* probe([STOP_TIMEOUT_OPTION]))) { + const supportsOomPolicy = yield* probe([...SCOPE_OPTIONS, "--property=OOMPolicy=stop"]); + if (!supportsOomPolicy && !(yield* probe(SCOPE_OPTIONS))) { yield* Effect.logInfo("Agent scopes are off: no systemd user manager"); return undefined; } @@ -359,7 +360,7 @@ export const make = Effect.gen(function* () { // Without an owning thread nothing reads the result, so let systemd // unload the unit even when it fails. options: [ - STOP_TIMEOUT_OPTION, + ...SCOPE_OPTIONS, ...(scope.supportsOomPolicy ? [`--property=OOMPolicy=${continuing ? "continue" : "stop"}`] : []), diff --git a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts index aab0a64ed53d4..76528cf212032 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts @@ -113,3 +113,6 @@ describe("deriveProviderInstanceConfigMap", () => { expect(instances[CLAUDE_INSTANCE_ID]?.driver).toBe("claudeAgent"); }); }); + +// Coder: upstream's `seedDevReplayInstance` tests are not carried with the dev replay provider +// (#26207). diff --git a/apps/server/src/provider/ProviderInstanceRegistryHydration.ts b/apps/server/src/provider/ProviderInstanceRegistryHydration.ts index 4ea8042d02ef1..7f50652fe7fa8 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryHydration.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryHydration.ts @@ -67,6 +67,8 @@ export const deriveProviderInstanceConfigMap = ( return merged as ProviderInstanceConfigMap; }; +// Coder: upstream's dev replay provider (#26207, `seedDevReplayInstance`) is not carried, and the +// Claude runner reports no agents to port discovery (#22542), so hydration needs neither. const layerSettingsWatcher = (settingsChanges: Stream.Stream) => Layer.effectDiscard( Effect.gen(function* () { diff --git a/apps/server/src/provider/model-manifest.json b/apps/server/src/provider/model-manifest.json index 9d2f5ae7256bc..444c130c1ee11 100644 --- a/apps/server/src/provider/model-manifest.json +++ b/apps/server/src/provider/model-manifest.json @@ -1,6 +1,6 @@ { "version": 1, - "updatedAt": "2026-10-26T20:00:36Z", + "updatedAt": "2026-10-27T23:47:46Z", "compatibility": [ { "driver": "codex", diff --git a/apps/server/src/sourceControl/ServerSourceControlHost.ts b/apps/server/src/sourceControl/ServerSourceControlHost.ts index de62afef246a4..aed4011bf1ad6 100644 --- a/apps/server/src/sourceControl/ServerSourceControlHost.ts +++ b/apps/server/src/sourceControl/ServerSourceControlHost.ts @@ -29,6 +29,8 @@ export const layer = Layer.effect( fetchRemoteBranch: git.fetchRemoteBranch, fetchRemoteTrackingBranch: git.fetchRemoteTrackingBranch, setBranchUpstream: git.setBranchUpstream, + createBranch: git.createBranch, + writeConfigValue: git.writeConfigValue, switchRef: git.switchRef, }, }); diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index a03545065a852..6176c0a982ef9 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -60,6 +60,8 @@ export interface ExecuteGitInput { /** With appendTruncationMarker, line callbacks continue beyond the buffer cap for progress-only consumers. */ readonly keepLineCallbacksAfterTruncation?: boolean; readonly progress?: ExecuteGitProgress; + /** Writes .git/config, which every worktree shares and Git locks without waiting. */ + readonly writesSharedConfig?: boolean; } export interface ExecuteGitResult { @@ -281,6 +283,18 @@ export interface GitFetchWorktreeBaseCommitInput { refName: string; } +export interface GitCreateBranchInput { + readonly cwd: string; + readonly branch: string; + readonly startPoint: string; +} + +export interface GitWriteConfigValueInput { + readonly cwd: string; + readonly key: string; + readonly value: string; +} + export interface GitSetBranchUpstreamInput { cwd: string; branch: string; @@ -401,6 +415,11 @@ export class GitVcsDriver extends Context.Service< readonly setBranchUpstream: ( input: GitSetBranchUpstreamInput, ) => Effect.Effect; + /** Tracks the start point like `git branch` does. */ + readonly createBranch: (input: GitCreateBranchInput) => Effect.Effect; + readonly writeConfigValue: ( + input: GitWriteConfigValueInput, + ) => Effect.Effect; readonly removeWorktree: ( input: VcsRemoveWorktreeInput, ) => Effect.Effect; diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index ba695684f333f..02f16f816b043 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -16,6 +16,7 @@ import * as Scope from "effect/Scope"; import * as Schema from "effect/Schema"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; import { ChildProcess, ChildProcessSpawner } from "effect/process"; import { @@ -87,6 +88,41 @@ const makeFailedHandle = (stderr: string) => stderr: Stream.encodeText(Stream.make(stderr)), }); +// Releases the lock as soon as a matching git command fails on it, before the driver retries. +const makeDriverReleasingLock = ( + matches: (args: ReadonlyArray) => boolean, + lockPath: string, +) => + Effect.gen(function* () { + const failures = yield* Ref.make(0); + const fileSystem = yield* FileSystem.FileSystem; + const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + const handle = yield* delegate.spawn(command); + if (!ChildProcess.isStandardCommand(command) || !matches(command.args)) return handle; + return ChildProcessSpawner.makeHandle({ + ...handle, + exitCode: handle.exitCode.pipe( + Effect.tap((exitCode) => + exitCode === 0 + ? Effect.void + : Ref.update(failures, (count) => count + 1).pipe( + Effect.andThen(fileSystem.remove(lockPath, { force: true })), + Effect.orDie, + ), + ), + ), + }); + }), + ); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.provide(layerServerConfig), + ); + return { driver, failures }; + }); + describe("Git output collection", () => { // Git writes in arbitrary chunk sizes; one byte per chunk splits every // multibyte character across reads. @@ -1060,6 +1096,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { it.effect.each([ { action: "worktree add", detail: "git worktree add failed" }, + { action: "worktree add -b", detail: "git worktree add failed" }, { action: "switch", detail: "git checkout failed" }, ])( "reports a missing ref during $action with stable, redacted diagnostics", @@ -1074,7 +1111,9 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { const spawner = ChildProcessSpawner.make((command) => { if ( ChildProcess.isStandardCommand(command) && - (command.args.includes("worktree") || command.args[0] === "checkout") + (command.args.includes("worktree") || + command.args[0] === "branch" || + command.args[0] === "checkout") ) { locales.push(command.options.env?.LC_ALL); } @@ -1091,7 +1130,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { .createWorktree({ cwd, refName, - newRefName: "t3/new-thread", + ...(action === "worktree add -b" ? { newRefName: "t3/new-thread" } : {}), path: path.join(cwd, "worktree"), }) .pipe(Effect.flip); @@ -3043,6 +3082,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { yield* git(previous.worktree.path, ["branch", "--show-current"]), initialBranch, ); + const fromPrevious = yield* driver.createWorktree({ + cwd, + path: pathService.join(worktrees, "from-previous"), + refName: "-", + newRefName: "from-previous", + }); + assert.equal( + yield* git(fromPrevious.worktree.path, ["rev-parse", "HEAD"]), + yield* git(cwd, ["rev-parse", initialBranch]), + ); }), ); }); @@ -3141,6 +3190,85 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("creates parallel worktrees in one repository without racing on its config", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const remote = yield* makeTmpDir("git-remote-"); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "origin", initialBranch]); + const path = yield* Path.Path; + const worktrees = yield* makeTmpDir("git-worktrees-"); + + // Git has no lock timeout for .git/config, which every worktree shares: a writer that + // overlaps another fails, and a branch can then be left without its worktree. + let configWriters = 0; + let peakConfigWriters = 0; + const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; + const countingSpawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + const args = ChildProcess.isStandardCommand(command) ? command.args : []; + const writesConfig = + (args.includes("worktree") && args.includes("add") && args.includes("-b")) || + (args[0] === "branch" && + (args[1] === "-m" || args[1]?.startsWith("--set-upstream-to"))) || + (args[0] === "config" && !args[1]?.startsWith("-")); + if (!writesConfig) return yield* delegate.spawn(command); + configWriters += 1; + peakConfigWriters = Math.max(peakConfigWriters, configWriters); + const handle = yield* delegate.spawn(command); + return ChildProcessSpawner.makeHandle({ + ...handle, + exitCode: handle.exitCode.pipe( + Effect.ensuring(Effect.sync(() => (configWriters -= 1))), + ), + }); + }), + ); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, countingSpawner), + Effect.provide(layerServerConfig), + ); + const branches = Array.from({ length: 8 }, (_, index) => `feature/parallel-${index}`); + + yield* Effect.forEach( + branches, + (branch) => + driver.createWorktree({ + cwd, + path: path.join(worktrees, branch.replace("/", "-")), + refName: `origin/${initialBranch}`, + newRefName: branch, + baseRefName: `origin/${initialBranch}`, + }), + { concurrency: "unbounded" }, + ); + // Launches rename their temporary branch from inside the new worktree. + yield* Effect.forEach( + branches, + (branch) => + driver.renameBranch({ + cwd: path.join(worktrees, branch.replace("/", "-")), + oldBranch: branch, + newBranch: `${branch}-renamed`, + exactName: true, + }), + { concurrency: "unbounded" }, + ); + + assert.equal(peakConfigWriters, 1); + for (const branch of branches) { + const renamed = `${branch}-renamed`; + assert.equal(yield* git(cwd, ["config", `branch.${renamed}.remote`]), "origin"); + assert.equal( + yield* git(cwd, ["config", `branch.${renamed}.t3-merge-base-ref`]), + `refs/remotes/origin/${initialBranch}`, + ); + } + }), + ); + it.effect.each(["release/v2", "refs/heads/release/v2", "deadbeef"])( "records the branch name when creating a worktree from %s", (baseRef) => @@ -3450,7 +3578,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); - it.effect("still creates the worktree when the base-ref config is locked", () => + it.effect("still creates the worktree when the base-ref config stays locked", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); const { initialBranch } = yield* initRepoWithCommit(cwd); @@ -3460,13 +3588,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { yield* writeTextFile(cwd, ".git/config.lock", ""); const driver = yield* GitVcsDriver.GitVcsDriver; - const created = yield* driver.createWorktree({ - cwd, - path: worktreePath, - refName: initialBranch, - newRefName: "feature/locked-config", - baseRefName: initialBranch, - }); + // The base-ref write retries on the real clock before giving up. + const created = yield* TestClock.withLive( + driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/locked-config", + baseRefName: initialBranch, + }), + ); assert.deepEqual(created.worktree, { path: worktreePath, @@ -3715,6 +3846,129 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("ensureRemote returns the remote a parallel call added for the same URL", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* git(cwd, ["init"]); + // Hold the first add until both calls have listed remotes, so both decide to add. + let listings = 0; + const bothListed = yield* Deferred.make(); + const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + const args = ChildProcess.isStandardCommand(command) ? command.args : []; + if (args[0] === "remote" && args[1] === "-v") { + listings += 1; + if (listings === 2) yield* Deferred.succeed(bothListed, undefined); + } + if (args[0] === "remote" && args[1] === "add") yield* Deferred.await(bothListed); + return yield* delegate.spawn(command); + }), + ); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.provide(layerServerConfig), + ); + const input = { cwd, preferredName: "fork", url: "https://github.com/fork/t3code.git" }; + + const names = yield* Effect.all([driver.ensureRemote(input), driver.ensureRemote(input)], { + concurrency: "unbounded", + }); + + assert.deepEqual(names, ["fork", "fork"]); + }), + ); + + it.effect("retries a config write while another process holds the config lock", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const lockPath = path.join(cwd, ".git", "config.lock"); + yield* fileSystem.writeFileString(lockPath, ""); + const { driver, failures } = yield* makeDriverReleasingLock( + (args) => args[1] === "t3.test", + lockPath, + ); + + // The retry waits on the real clock; the lock is already gone by then. + yield* TestClock.withLive( + driver.writeConfigValue({ cwd, key: "t3.test", value: "written" }), + ); + + assert.equal(yield* Ref.get(failures), 1); + assert.equal(yield* git(cwd, ["config", "t3.test"]), "written"); + }), + ); + + it.effect("tracks a remote start point once another process releases the config lock", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const remote = yield* makeTmpDir("git-remote-"); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "origin", initialBranch]); + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const lockPath = path.join(cwd, ".git", "config.lock"); + yield* fileSystem.writeFileString(lockPath, ""); + const { driver, failures } = yield* makeDriverReleasingLock( + (args) => args.some((arg) => arg.startsWith("--set-upstream-to")), + lockPath, + ); + + // The upstream retry waits on the real clock; the lock is already gone by then. + yield* TestClock.withLive( + driver.createWorktree({ + cwd, + path: path.join(yield* makeTmpDir("git-worktrees-"), "tracked"), + refName: `origin/${initialBranch}`, + newRefName: "feature/tracked", + }), + ); + + assert.equal(yield* Ref.get(failures), 1); + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", "feature/tracked@{upstream}"]), + `origin/${initialBranch}`, + ); + }), + ); + + it.effect("fetches a remote branch into a tracking branch past another process's lock", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const remote = yield* makeTmpDir("git-remote-"); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "origin", `${initialBranch}:refs/heads/pr-head`]); + const lockPath = (yield* Path.Path).join(cwd, ".git", "config.lock"); + yield* (yield* FileSystem.FileSystem).writeFileString(lockPath, ""); + const { driver, failures } = yield* makeDriverReleasingLock( + (args) => args.some((arg) => arg.startsWith("--set-upstream-to")), + lockPath, + ); + + yield* TestClock.withLive( + driver.fetchRemoteBranch({ + cwd, + remoteName: "origin", + remoteBranch: "pr-head", + localBranch: "review/pr-head", + }), + ); + + assert.equal(yield* Ref.get(failures), 1); + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", "review/pr-head@{upstream}"]), + "origin/pr-head", + ); + }), + ); + // regression: #12485 explain fetch failures without exposing remote output it.effect("explains a real fetch failure for a missing local remote", () => Effect.gen(function* () { @@ -4228,6 +4482,45 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("rebases a pushed branch's pulls when branch.autoSetupRebase asks for it", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-remote-"); + yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["config", "branch.autoSetupRebase", "always"]); + yield* git(cwd, ["checkout", "-q", "-b", "feature/rebase"]); + const driver = yield* GitVcsDriver.GitVcsDriver; + + assert.deepInclude(yield* driver.pushCurrentBranch(cwd, null), { setUpstream: true }); + assert.equal(yield* driver.readConfigValue(cwd, "branch.feature/rebase.rebase"), "true"); + }), + ); + + it.effect("reports a push whose upstream could not be recorded", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-remote-"); + yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["checkout", "-q", "-b", "feature/locked"]); + yield* writeTextFile(cwd, ".git/config.lock", ""); + const driver = yield* GitVcsDriver.GitVcsDriver; + + // Recording the upstream retries on the real clock before giving up. + const pushed = yield* TestClock.withLive(driver.pushCurrentBranch(cwd, null)); + + assert.deepInclude(pushed, { status: "pushed", setUpstream: false }); + assert.include( + yield* git(cwd, ["ls-remote", "origin", "refs/heads/feature/locked"]), + yield* git(cwd, ["rev-parse", "HEAD"]), + ); + assert.equal(yield* driver.readConfigValue(cwd, "branch.feature/locked.remote"), null); + }), + ); + it.effect.each(["feature/push", "origin/feature/push"])( "publishes %s when the configured diff base already contains HEAD", (branch) => diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 68e4b18d5b806..1917abb18de3c 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -16,6 +16,7 @@ import * as PlatformError from "effect/PlatformError"; import * as Ref from "effect/Ref"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; +import * as Schedule from "effect/Schedule"; import * as Scope from "effect/Scope"; import * as Semaphore from "effect/Semaphore"; import * as Stream from "effect/Stream"; @@ -185,6 +186,7 @@ interface ExecuteGitOptions { maxOutputBytes?: number | undefined; appendTruncationMarker?: boolean | undefined; progress?: GitVcsDriver.ExecuteGitProgress | undefined; + writesSharedConfig?: boolean | undefined; } function parseBranchAb(value: string): { ahead: number; behind: number } { @@ -435,6 +437,8 @@ const GIT_FAILURE_REASON_PATTERNS: ReadonlyArray(); + const withSharedConfigLock = (cwd: string, effect: Effect.Effect) => + resolveRepositoryPaths(cwd).pipe( + Effect.orElseSucceed(() => null), + Effect.flatMap((repositoryPaths) => { + if (repositoryPaths === null) return effect; + let lock = sharedConfigLocks.get(repositoryPaths.gitCommonDir); + if (lock === undefined) { + lock = Semaphore.makeUnsafe(1); + sharedConfigLocks.set(repositoryPaths.gitCommonDir, lock); + } + return lock.withPermit(effect); + }), + ); + // Coder: Git execution has no metrics collection or export. const execute: GitVcsDriver.GitVcsDriver["Service"]["execute"] = (input) => executeRaw(input).pipe( @@ -1080,6 +1101,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "git.args_count": input.args.length, }, }), + // Outside the process permit: resolving the repository may itself need one. + (execution) => + input.writesSharedConfig ? withSharedConfigLock(input.cwd, execution) : execution, ); const executeGit = ( @@ -1104,6 +1128,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ? { appendTruncationMarker: options.appendTruncationMarker } : {}), ...(options.progress ? { progress: options.progress } : {}), + ...(options.writesSharedConfig ? { writesSharedConfig: true } : {}), }).pipe( Effect.flatMap((result) => { if (options.allowNonZeroExit || result.exitCode === 0) { @@ -1685,17 +1710,20 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* )(function* (input) { const preferredName = sanitizeRemoteName(input.preferredName); const normalizedTargetUrl = sourceControlDefinitions.repositoryKeyFromRemote(input.url); - const remoteFetchUrls = yield* runGitStdout( + const listRemoteFetchUrls = runGitStdout( "GitVcsDriver.ensureRemote.listRemoteUrls", input.cwd, ["remote", "-v"], ).pipe(Effect.map((stdout) => parseRemoteFetchUrls(stdout))); + const findMatchingRemote = (remoteFetchUrls: Map) => + Array.from(remoteFetchUrls.entries()).find( + ([, remoteUrl]) => + sourceControlDefinitions.repositoryKeyFromRemote(remoteUrl) === normalizedTargetUrl, + )?.[0]; - for (const [remoteName, remoteUrl] of remoteFetchUrls.entries()) { - if (sourceControlDefinitions.repositoryKeyFromRemote(remoteUrl) === normalizedTargetUrl) { - return remoteName; - } - } + const remoteFetchUrls = yield* listRemoteFetchUrls; + const existing = findMatchingRemote(remoteFetchUrls); + if (existing !== undefined) return existing; let remoteName = preferredName; let suffix = 1; @@ -1704,14 +1732,24 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* suffix += 1; } - yield* runGit("GitVcsDriver.ensureRemote.add", input.cwd, [ - "remote", - "add", - "--", - remoteName, - input.url, - ]); - return remoteName; + return yield* executeGitWithStableDiagnostics( + "GitVcsDriver.ensureRemote.add", + input.cwd, + ["remote", "add", "--", remoteName, input.url], + { writesSharedConfig: true }, + ).pipe( + retryWhileConfigLocked, + Effect.as(remoteName), + // A parallel launch for the same fork may have added it since the listing above. + Effect.catch((error) => + listRemoteFetchUrls.pipe( + Effect.flatMap((current) => { + const added = findMatchingRemote(current); + return added === undefined ? Effect.fail(error) : Effect.succeed(added); + }), + ), + ), + ); }); const resolveComparisonBaseRef = Effect.fn("resolveComparisonBaseRef")(function* ( @@ -2463,6 +2501,31 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return { commitSha }; }); + // `push -u` exits 0 without the upstream when another writer holds the config lock. + const recordPushedUpstream = Effect.fn("recordPushedUpstream")( + function* (cwd: string, branch: string, remoteName: string, publishBranch: string) { + // Without a prior upstream, Git ignores a merge ref with no remote, so losing the second + // write leaves none rather than a wrong one. + yield* writeConfigValue({ + cwd, + key: `branch.${branch}.merge`, + value: `refs/heads/${publishBranch}`, + }); + yield* writeConfigValue({ cwd, key: `branch.${branch}.remote`, value: remoteName }); + // Matches what `push -u` sets for a remote upstream. + const autoSetupRebase = yield* readConfigValue(cwd, "branch.autoSetupRebase"); + if (autoSetupRebase === "always" || autoSetupRebase === "remote") { + yield* writeConfigValue({ cwd, key: `branch.${branch}.rebase`, value: "true" }); + } + return true; + }, + Effect.catch((cause) => + Effect.logWarning("pushed, but could not record the branch upstream", { cause }).pipe( + Effect.as(false), + ), + ), + ); + const pushCurrentBranch: GitVcsDriver.GitVcsDriver["Service"]["pushCurrentBranch"] = Effect.fn( "pushCurrentBranch", )(function* (cwd, fallbackBranch, options) { @@ -2485,14 +2548,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushWithRequestedRemote", cwd, - ["push", "-u", "--", requestedRemoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "--", requestedRemoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { status: "pushed" as const, branch, upstreamBranch: `${requestedRemoteName}/${publishBranch}`, - setUpstream: true, + setUpstream: yield* recordPushedUpstream(cwd, branch, requestedRemoteName, publishBranch), }; } @@ -2561,14 +2624,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushWithUpstream", cwd, - ["push", "-u", "--", publishRemoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "--", publishRemoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { status: "pushed" as const, branch, upstreamBranch: `${publishRemoteName}/${publishBranch}`, - setUpstream: true, + setUpstream: yield* recordPushedUpstream(cwd, branch, publishRemoteName, publishBranch), }; } @@ -2589,30 +2652,30 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const publishBranch = yield* resolvePublishBranchName(cwd, branch); // Publishing retargets the upstream, so preserve its comparison base first. if ((yield* readBranchBase(cwd, branch)) === null) { - yield* runGit("GitVcsDriver.pushCurrentBranch.recordMergeBase", cwd, [ - "config", - branchBaseConfigKey(branch), - currentUpstream.branchName, - ]); + yield* writeConfigValue({ + cwd, + key: branchBaseConfigKey(branch), + value: currentUpstream.branchName, + }); if (!(yield* readConfigValue(cwd, `branch.${branch}.t3-merge-base-ref`))) { - yield* runGit("GitVcsDriver.pushCurrentBranch.recordComparisonRef", cwd, [ - "config", - `branch.${branch}.t3-merge-base-ref`, - `refs/remotes/${currentUpstream.upstreamRef}`, - ]); + yield* writeConfigValue({ + cwd, + key: `branch.${branch}.t3-merge-base-ref`, + value: `refs/remotes/${currentUpstream.upstreamRef}`, + }); } } yield* runGit( "GitVcsDriver.pushCurrentBranch.pushOwnBranch", cwd, - ["push", "-u", "--", remoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "--", remoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { status: "pushed" as const, branch, upstreamBranch: `${remoteName}/${publishBranch}`, - setUpstream: true, + setUpstream: yield* recordPushedUpstream(cwd, branch, remoteName, publishBranch), }; } @@ -3330,6 +3393,70 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* Effect.map((trimmed) => (trimmed.length > 0 ? trimmed : null)), ); + // Git never waits for .git/config.lock, and git run outside T3, such as an agent's, can hold it. + // Only writes that are safe to repeat retry. + const retryWhileConfigLocked = (effect: Effect.Effect) => + effect.pipe( + Effect.retry({ + while: (error) => error.reason === "config_locked", + times: 10, + schedule: Schedule.spaced("50 millis"), + }), + ); + + const writeConfigValue: GitVcsDriver.GitVcsDriver["Service"]["writeConfigValue"] = Effect.fn( + "writeConfigValue", + )(function* (input) { + yield* executeGitWithStableDiagnostics( + "GitVcsDriver.writeConfigValue", + input.cwd, + ["config", input.key, input.value], + { writesSharedConfig: true }, + ).pipe(retryWhileConfigLocked); + }); + + // `git branch` writes a remote start point's upstream itself and fails outright if the + // config is locked, so tracking is recorded afterwards by a write that retries. + const createBranchAt = Effect.fn("createBranchAt")(function* ( + operation: string, + input: GitVcsDriver.GitCreateBranchInput, + fallbackErrorDetail: string, + ) { + // `git branch` does not read a lone - as the previous checkout. + const startPoint = input.startPoint === "-" ? "@{-1}" : input.startPoint; + yield* executeGitWithStableDiagnostics( + operation, + input.cwd, + ["branch", "--no-track", "--", input.branch, startPoint], + { fallbackErrorDetail }, + ); + const startRef = yield* executeGit( + `${operation}.resolveStartPoint`, + input.cwd, + ["rev-parse", "--symbolic-full-name", startPoint], + { allowNonZeroExit: true }, + ).pipe(Effect.map((result) => (result.exitCode === 0 ? result.stdout.trim() : ""))); + if (!startRef.startsWith("refs/remotes/")) return; + yield* executeGitWithStableDiagnostics( + `${operation}.track`, + input.cwd, + ["branch", `--set-upstream-to=${startRef}`, "--", input.branch], + { writesSharedConfig: true }, + ).pipe( + retryWhileConfigLocked, + Effect.catch((cause) => + Effect.logWarning("created the branch without tracking its start point", { cause }), + ), + ); + }); + + const createBranch: GitVcsDriver.GitVcsDriver["Service"]["createBranch"] = Effect.fn( + "createBranch", + )(function* (input) { + yield* refuseOptionLikeRefName("GitVcsDriver.createBranch", input.cwd, input.startPoint); + yield* createBranchAt("GitVcsDriver.createBranch", input, "git branch create failed"); + }); + const readBranchBase: GitVcsDriver.GitVcsDriver["Service"]["readBranchBase"] = Effect.fn( "GitVcsDriver.readBranchBase", )(function* (cwd, branch) { @@ -3749,9 +3876,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } worktreePath = path.join(parentDir, repoName, sanitizedBranch); } - const args = input.newRefName - ? ["worktree", "add", "-b", input.newRefName, "--", worktreePath, input.refName] - : ["worktree", "add", "--", worktreePath, input.refName]; + if (input.newRefName) { + // Kept apart from the checkout so the shared-config lock only covers the tracking write. + yield* createBranchAt( + "GitVcsDriver.createWorktree.createBranch", + { cwd: input.cwd, branch: input.newRefName, startPoint: input.refName }, + "git worktree add failed", + ); + } + const args = ["worktree", "add", "--", worktreePath, input.newRefName ?? input.refName]; const progress = options?.progress; const onCheckoutProgress = progress?.onCheckoutProgress; @@ -3807,19 +3940,28 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* progress.onSubmodulesStarted(); } const onSubmoduleLine = progress?.onSubmoduleLine; + // Registering submodule URLs writes the shared config; the clones that follow do not. yield* runGit( - "GitVcsDriver.createWorktree.updateSubmodules", + "GitVcsDriver.createWorktree.initSubmodules", worktreePath, - submoduleMode.value === "recursive" - ? ["submodule", "update", "--init", "--recursive"] - : ["submodule", "update", "--init"], - onSubmoduleLine - ? { - env: { LC_ALL: "C" }, - progress: { onStdoutLine: onSubmoduleLine, onStderrLine: onSubmoduleLine }, - } - : {}, + ["submodule", "init"], + { writesSharedConfig: true }, ).pipe( + Effect.andThen( + runGit( + "GitVcsDriver.createWorktree.updateSubmodules", + worktreePath, + submoduleMode.value === "recursive" + ? ["submodule", "update", "--init", "--recursive"] + : ["submodule", "update", "--init"], + onSubmoduleLine + ? { + env: { LC_ALL: "C" }, + progress: { onStdoutLine: onSubmoduleLine, onStderrLine: onSubmoduleLine }, + } + : {}, + ), + ), Effect.matchEffect({ onFailure: (cause) => Effect.logWarning("worktree submodule checkout failed; submodule paths are empty", { @@ -3845,18 +3987,18 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const base = yield* resolveBranchBase(input.cwd, input.baseRefName); if (base) { yield* Effect.gen(function* () { - yield* runGit("GitVcsDriver.createWorktree.configureBaseRef", input.cwd, [ - "config", - branchBaseConfigKey(newRefName), - base.branchName, - ]); + yield* writeConfigValue({ + cwd: input.cwd, + key: branchBaseConfigKey(newRefName), + value: base.branchName, + }); if (base.baseRef.startsWith("refs/remotes/")) { // The base key names a branch on the host; this one keeps the remote it came from. - yield* runGit("GitVcsDriver.createWorktree.configureComparisonRef", input.cwd, [ - "config", - `branch.${input.newRefName}.t3-merge-base-ref`, - base.baseRef, - ]); + yield* writeConfigValue({ + cwd: input.cwd, + key: `branch.${input.newRefName}.t3-merge-base-ref`, + value: base.baseRef, + }); } }).pipe( Effect.catchTags({ @@ -4143,12 +4285,18 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const localBranchAlreadyExists = yield* branchExists(input.cwd, input.localBranch); const targetRef = `${input.remoteName}/${input.remoteBranch}`; + if (!localBranchAlreadyExists) { + return yield* createBranchAt( + "GitVcsDriver.fetchRemoteBranch.materialize", + { cwd: input.cwd, branch: input.localBranch, startPoint: targetRef }, + "git branch create failed", + ); + } yield* runGit( "GitVcsDriver.fetchRemoteBranch.materialize", input.cwd, - localBranchAlreadyExists - ? ["branch", "--force", "--", input.localBranch, targetRef] - : ["branch", "--", input.localBranch, targetRef], + ["branch", "--force", "--", input.localBranch, targetRef], + { writesSharedConfig: true }, ); }); @@ -4165,13 +4313,18 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }); const setBranchUpstream: GitVcsDriver.GitVcsDriver["Service"]["setBranchUpstream"] = (input) => - runGit("GitVcsDriver.setBranchUpstream", input.cwd, [ - "branch", - "--set-upstream-to", - `${input.remoteName}/${input.remoteBranch}`, - "--", - input.branch, - ]); + executeGitWithStableDiagnostics( + "GitVcsDriver.setBranchUpstream", + input.cwd, + [ + "branch", + "--set-upstream-to", + `${input.remoteName}/${input.remoteBranch}`, + "--", + input.branch, + ], + { writesSharedConfig: true }, + ).pipe(retryWhileConfigLocked, Effect.asVoid); /** * Unlinks links under `target` without following them and removes the @@ -4280,6 +4433,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { timeoutMs: 10_000, fallbackErrorDetail: "git branch delete failed", + // Under contention Git deletes the ref but keeps its config section. + writesSharedConfig: true, }, ); }); @@ -4301,6 +4456,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { timeoutMs: 10_000, fallbackErrorDetail: "git branch rename failed", + writesSharedConfig: true, }, ); @@ -4386,6 +4542,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { timeoutMs: 10_000, fallbackErrorDetail: "git checkout failed", + // Checking out a remote branch creates a local one that tracks it. + writesSharedConfig: !localInputExists, }, ); @@ -4403,6 +4561,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* executeGit("GitVcsDriver.createRef", input.cwd, ["branch", "--", input.refName], { timeoutMs: 10_000, fallbackErrorDetail: "git branch create failed", + // `branch.autoSetupMerge=always` records HEAD's branch as the upstream. + writesSharedConfig: true, }); if (input.switchRef) { yield* switchRef({ cwd: input.cwd, refName: input.refName }); @@ -4549,6 +4709,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* setBranchUpstream: Effect.fnUntraced(function* (input) { return yield* withListRefsInvalidation(input.cwd, setBranchUpstream(input)); }), + createBranch: Effect.fnUntraced(function* (input) { + return yield* withListRefsInvalidation(input.cwd, createBranch(input)); + }), + writeConfigValue, removeWorktree: Effect.fnUntraced(function* (input) { return yield* withListRefsInvalidation(input.cwd, removeWorktree(input)); }), diff --git a/apps/web/src/appInit.tsx b/apps/web/src/appInit.tsx index 0ddbddada5ed3..a4bfdc5150bdc 100644 --- a/apps/web/src/appInit.tsx +++ b/apps/web/src/appInit.tsx @@ -32,6 +32,7 @@ import { appAtomRegistry } from "./rpc/atomRegistry"; import { primaryServerConfigAtom } from "./state/server"; import { environmentShell } from "./state/shell"; import { installChatRouteCommands } from "./shortcuts/chatRouteCommands"; +import { startWorkspaceFocus } from "./lib/workspaceFocus"; import { installCommandPaletteCommands } from "./shortcuts/commandPaletteCommands"; import { environmentThreadShells } from "./state/threads"; import { resolveThreadRouteTarget } from "./threadRoutes"; @@ -161,6 +162,7 @@ export function startAppServices(): AppRouter { shellStateAtom: environmentShell.stateAtom, }); startMainAppLocationTracking(router); + startWorkspaceFocus(router); startBackgroundThreadAnnouncements(appAtomRegistry, router); startThreadVisitedMigration(appAtomRegistry); diff --git a/apps/web/src/commandPaletteStore.ts b/apps/web/src/commandPaletteStore.ts index f8dee5413fbf8..3fdd07383fd24 100644 --- a/apps/web/src/commandPaletteStore.ts +++ b/apps/web/src/commandPaletteStore.ts @@ -6,6 +6,7 @@ import { type CommandPaletteUiState, reduceCommandPaletteUiState, } from "./components/CommandPalette.logic"; +import { getWorkspaceFocusRequestVersion } from "./lib/workspaceFocus"; const INITIAL_STATE: CommandPaletteUiState = { open: false, @@ -16,8 +17,35 @@ const INITIAL_STATE: CommandPaletteUiState = { export const useCommandPaletteStore = create(() => INITIAL_STATE); +let returnFocus: { element: HTMLElement | null; href: string; focusRequest: number } | null = null; + export function dispatchCommandPalette(action: CommandPaletteUiAction): void { - useCommandPaletteStore.setState((state) => reduceCommandPaletteUiState(state, action), true); + const state = useCommandPaletteStore.getState(); + const next = reduceCommandPaletteUiState(state, action); + if (!state.open && next.open) { + // The workspace turns inert before the popup mounts, which blurs the opener. + returnFocus = { + element: document.activeElement instanceof HTMLElement ? document.activeElement : null, + href: window.location.href, + focusRequest: getWorkspaceFocusRequestVersion(), + }; + } + useCommandPaletteStore.setState(next, true); +} + +/** + * Where the closing palette should return focus: `false` when a command navigated or asked + * for workspace focus (the destination owns it), else the opener, or `true` for the default. + */ +export function getCommandPaletteReturnFocus(): HTMLElement | boolean { + if ( + !returnFocus || + window.location.href !== returnFocus.href || + getWorkspaceFocusRequestVersion() !== returnFocus.focusRequest + ) + return false; + const { element } = returnFocus; + return element?.isConnected && !element.matches(":disabled") ? element : true; } // The palette is only mounted on app-shell routes (not /pair, sign-in or @@ -33,7 +61,7 @@ export function isCommandPaletteMounted(): boolean { * palette cannot stay open, so the last unmount resets the store. */ export function useCommandPaletteMount(): void { - // oxlint-disable-next-line t3code/react-effect-budget -- The palette's mount gates its global shortcuts, and leaving the route while it is open calls no close handler, so only unmount can reset the store. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- The palette's mount gates its global shortcuts, and leaving the route while it is open calls no close handler, so only unmount can reset the store. useEffect(() => { mountedPalettes += 1; return () => { diff --git a/apps/web/src/components/AnimatedHeight.tsx b/apps/web/src/components/AnimatedHeight.tsx index 2c44325832057..a098851233a72 100644 --- a/apps/web/src/components/AnimatedHeight.tsx +++ b/apps/web/src/components/AnimatedHeight.tsx @@ -16,6 +16,7 @@ export function AnimatedHeight({ readonly isClipping: boolean; }>({ height: null, isClipping: false }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- fallback timer that ends clipping when the height transition never reports its end useEffect(() => { if (!heightState.isClipping) return; const timeoutId = window.setTimeout(() => { @@ -30,6 +31,7 @@ export function AnimatedHeight({ heightState.isClipping, ]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- measures the content element and observes its size before paint to animate height useLayoutEffect(() => { if (holdHeight) return; const element = contentRef.current; diff --git a/apps/web/src/components/AnimatedWidth.tsx b/apps/web/src/components/AnimatedWidth.tsx index ef152402d6dc7..dd2bfce7f0afd 100644 --- a/apps/web/src/components/AnimatedWidth.tsx +++ b/apps/web/src/components/AnimatedWidth.tsx @@ -13,6 +13,7 @@ export function AnimatedWidth({ const contentRef = useRef(null); const [width, setWidth] = useState(null); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- observes the content element's width before paint to animate width useLayoutEffect(() => { const element = contentRef.current; if (!element) return; diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx index e5a78af42ea12..1812d2f6064ca 100644 --- a/apps/web/src/components/AppSidebarLayout.tsx +++ b/apps/web/src/components/AppSidebarLayout.tsx @@ -9,6 +9,7 @@ import { } from "react"; import { useLocation, useRouter, type AnyRouter } from "@tanstack/react-router"; +import { moveWorkspaceFocus, workspaceAreaProps } from "../lib/workspaceFocus"; import { isCommandPaletteOpen } from "../commandPaletteBus"; import { releaseThreadUndoShortcut, @@ -127,6 +128,31 @@ function registerNavigationHistoryCommands(router: AnyRouter): () => void { ); } +// Capture phase, so F6 also leaves widgets that swallow keys, such as the terminal. +function registerWorkspaceFocusCommands(router: AnyRouter): () => void { + const move = (direction: 1 | -1) => (event: KeyboardEvent) => { + if (event.isComposing) return false; + // Inside a dialog the move is refused, but the key is still consumed. + event.preventDefault(); + event.stopPropagation(); + moveWorkspaceFocus(direction); + }; + const disposers = [ + registerCommands( + { "workspace.focusNext": move(1), "workspace.focusPrevious": move(-1) }, + { + capture: true, + when: (event) => !isKeybindingCaptureTarget(event), + context: () => readRoutePanelsOpen(router), + }, + ), + // Coder: no desktop browser forwards the keys as menu actions. + ]; + return () => { + for (const dispose of disposers) dispose(); + }; +} + // Escape leaves the pull requests page when no scope (a selection, a dialog, // a drag) claimed it, so it observes after every other scope. function registerPullRequestsEscapeCommand(router: AnyRouter): () => void { @@ -228,6 +254,7 @@ function registerAppShellCommands(router: AnyRouter, shellElement: Element): () const disposers = [ registerThreadUndoCommands(router, readSidebarVisible), registerNavigationHistoryCommands(router), + registerWorkspaceFocusCommands(router), registerPullRequestsEscapeCommand(router), // Coder: no desktop menu, so no "open-settings" menu action. ]; @@ -244,13 +271,12 @@ function SidebarControl() { const { toggleSidebar, isMobile, mobileSheetMounted } = useSidebar(); const isSidebarVisible = useSidebarVisibility(); const environmentIdentificationMode = useEnvironmentIdentificationMode(); - const stageBackdropVariant = useSidebarStageBackdropVariant( - environmentIdentificationMode === "artwork", - ); + const stageBackdropVariant = useSidebarStageBackdropVariant(environmentIdentificationMode); const shortcutLabel = shortcutLabelForCommand(keybindings, "sidebar.toggle", { context: { usagePageOpen }, }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- window keydown listener that toggles the sidebar on its shortcut useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { if (event.defaultPrevented) return; @@ -325,6 +351,7 @@ function SidebarControl() { } export function AppSidebarLayout({ children }: { children: ReactNode }) { + const sidebarArea = workspaceAreaProps("sidebar", 0, "Threads"); // Coder: no legacy sidebar and no desktop window bridge (fullscreen insets, menu actions). const [sidebarRouteReveal] = useState(() => ({ observedKey: null, @@ -367,6 +394,7 @@ export function AppSidebarLayout({ children }: { children: ReactNode }) { side="left" collapsible="offcanvas" data-app-sidebar="" + {...sidebarArea} role="navigation" aria-label="Threads" resizable={{ diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index ee7ec96e01295..709d39f7cd264 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -228,17 +228,7 @@ const RunContextSelector = memo(function RunContextSelector({ {activeEnvironment?.label ?? "Run on"} ); - const icon = !showEnvironmentIndicator ? ( - workspaceIcon - ) : isPanel ? ( - environmentIcon - ) : ( - // Button's SVG margins consume 4px of the gap; mx-0! restores the intended 2px. - - {environmentIcon} - {workspaceIcon} - - ); + const icon = showEnvironmentIndicator ? environmentIcon : workspaceIcon; const triggerContent = ( <> {icon} @@ -259,7 +249,11 @@ const RunContextSelector = memo(function RunContextSelector({ isPanel && THREAD_DETAILS_PANEL_LOCKED_ROW_CLASS, )} onContextMenu={handleContextMenu} - aria-label={isPanel ? "Run context" : undefined} + aria-label={ + isPanel + ? `Run context: ${[activeEnvironment?.label, workspaceLabel].filter(Boolean).join(", ")}` + : undefined + } data-composer-context-control > {triggerContent} @@ -280,7 +274,11 @@ const RunContextSelector = memo(function RunContextSelector({ } : undefined } - aria-label={isPanel ? "Run context" : undefined} + aria-label={ + isPanel + ? `Run context: ${[activeEnvironment?.label, workspaceLabel].filter(Boolean).join(", ")}` + : undefined + } data-composer-context-control data-composer-shortcut={[ showEnvironmentPicker && !envLocked ? "composer.host" : "", diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 2b580f5b00838..2aff7f4749a78 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -562,6 +562,7 @@ export function BranchToolbarBranchSelector({ // The server sets started threads' branches; defaulting here would detach the agent from its worktree. const threadStarted = serverSession !== null; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- writes a default base branch to the server thread or draft once branches load for a new worktree thread useEffect(() => { if ( threadStarted || diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index a6a4563313e0b..b4f7d3e999d2b 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -418,6 +418,7 @@ function useChatMarkdownState({ setLocalMedia(null); } // Layout cleanup blocks stale requests from reaching onImageExpand before paint. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- layout cleanup invalidates media requests still loading for the old scope before paint useLayoutEffect( () => () => { mediaRequestId.current += 1; diff --git a/apps/web/src/components/ChatView.logic.browser.test.ts b/apps/web/src/components/ChatView.logic.browser.test.ts new file mode 100644 index 0000000000000..e1c1f04a0c3a5 --- /dev/null +++ b/apps/web/src/components/ChatView.logic.browser.test.ts @@ -0,0 +1,28 @@ +import { expect, it } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; +import { shouldRefocusComposerOnWindowFocus } from "./ChatView.logic"; + +it("keeps keyboard focus on a sidebar row on window return, but not a clicked row", async () => { + const tree = document.createElement("div"); + tree.setAttribute("role", "tree"); + const row = (name: string, tabIndex: number) => { + const element = document.createElement("div"); + element.setAttribute("role", "treeitem"); + element.tabIndex = tabIndex; + element.textContent = name; + tree.append(element); + return element; + }; + const clicked = row("Clicked thread", -1); + const tabbed = row("Tabbed thread", 0); + document.body.append(tree); + using _tree = { [Symbol.dispose]: () => tree.remove() }; + + await userEvent.click(clicked); + expect(document.activeElement).toBe(clicked); + expect(shouldRefocusComposerOnWindowFocus(document.activeElement)).toBe(true); + + await userEvent.tab(); + expect(document.activeElement).toBe(tabbed); + expect(shouldRefocusComposerOnWindowFocus(document.activeElement)).toBe(false); +}); diff --git a/apps/web/src/components/ChatView.logic.ts b/apps/web/src/components/ChatView.logic.ts index 17080d0d0bdf3..53621d46d55ba 100644 --- a/apps/web/src/components/ChatView.logic.ts +++ b/apps/web/src/components/ChatView.logic.ts @@ -1085,6 +1085,10 @@ export function hasServerAcknowledgedLocalDispatch(input: { ); } +// Returning to the window should land the caret in the composer, so the reader can type right +// away. The exceptions are places where focus is deliberate: another text field, a terminal in +// the drawer or the right panel, an open dialog, menu, or popup, or a region or sidebar row +// reached from the keyboard. A plain focused button or a clicked row yields to the composer. export function shouldRefocusComposerOnWindowFocus( activeElement: | (Pick & { isContentEditable?: boolean }) @@ -1104,7 +1108,7 @@ export function shouldRefocusComposerOnWindowFocus( } return ( activeElement.closest( - '[role="dialog"], [role="alertdialog"], [role="application"], [data-slot$="-popup"], [data-terminal-owner]', + '[data-workspace-area]:focus-visible, [role="treeitem"]:focus-visible, [role="dialog"], [role="alertdialog"], [role="application"], [role="menu"], [role="listbox"], [data-slot$="-popup"], [data-terminal-owner]', ) === null ); } diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 9d64706188d50..079d23150102f 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -1,3 +1,4 @@ +import { workspaceAreaProps } from "../lib/workspaceFocus"; import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; import { queuedRunEditBlockedReason } from "@t3tools/client-runtime/queued-run-edit"; import { ScrollToEndButton, WorkspaceFileDropOverlay } from "./chat/TimelineOverlays"; @@ -131,7 +132,7 @@ import { type LegendListRef } from "@legendapp/list/react"; import { useWorkspaceMutationRefresh } from "../hooks/useWorkspaceMutationRefresh"; import { DEFAULT_INTERACTION_MODE, type ChatMessage } from "../types"; import { useTheme } from "../hooks/useTheme"; -import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; +import { copyThreadReference } from "../lib/copyThreadReference"; import { useMediaQuery } from "../hooks/useMediaQuery"; import { FileMetadataThreadProvider } from "../hooks/FileMetadataThreadProvider"; import { RIGHT_PANEL_INLINE_LAYOUT_MEDIA_QUERY } from "../rightPanelLayout"; @@ -153,7 +154,7 @@ import { DEFAULT_RESOLVED_KEYBINDINGS } from "@t3tools/shared/keybindings"; import { shortcutLabelForCommand } from "../keybindings"; import { cn } from "~/lib/utils"; import { COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS } from "~/workspaceTitlebar"; -import { stackedThreadToast, toastManager } from "./ui/toast"; +import { toastManager } from "./ui/toast"; import { projectScriptIdFromCommand } from "~/projectScripts"; import { newMessageId } from "~/lib/utils"; import { @@ -306,6 +307,7 @@ import { ChatRightPanel } from "./chat/ChatRightPanel"; import { useDraftEnvironmentRouting } from "./chat/useDraftEnvironmentRouting"; import { branchMismatchBannerItem } from "./chat/ThreadActivityBanners"; import { ThreadNow } from "./chat/ThreadNow"; +import type { ThreadStructure } from "@t3tools/client-runtime/state/shell"; import { useDelayedStatus } from "../hooks/useDelayedStatus"; import { useTimelineFollow } from "./chat/useTimelineFollow"; import { @@ -328,6 +330,7 @@ import type { CodexArtifactTemplate } from "@t3tools/shared/codexArtifactTemplat // Coder: no feedback uploads, so no messages anchor to the timeline. const EMPTY_ANCHORED_TIMELINE_MESSAGES: ReadonlyArray = []; const EMPTY_PROVIDER_SKILLS: ServerProvider["skills"] = []; +const EMPTY_TURN_ITEMS: ThreadStructure["turnItems"] = []; const EMPTY_PENDING_FILE_SURFACE_IDS: ReadonlySet = new Set(); @@ -366,6 +369,7 @@ export default function ChatView(props: ChatViewProps) { const routeThreadRef = scopeThreadRef(environmentId, threadId); const routeThreadKey = scopedThreadKey(routeThreadRef); const currentRouteThreadKeyRef = useRef(routeThreadKey); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- publishes the committed route thread key to a ref that async handlers check before applying results useLayoutEffect(() => { currentRouteThreadKeyRef.current = routeThreadKey; return () => { @@ -605,6 +609,7 @@ export default function ChatView(props: ChatViewProps) { setIsWorkspaceFileDragActive(false); } + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- window dragend listener that clears the workspace file drag state useEffect(() => { if (!isWorkspaceFileDragActive) return; const clearWorkspaceFileDrag = () => setIsWorkspaceFileDragActive(false); @@ -633,6 +638,8 @@ export default function ChatView(props: ChatViewProps) { const legendListRef = useRef(null); const getTimelineScrollableNode = () => legendListRef.current?.getScrollableNode() ?? null; const getTimelineScrollableNodeStable = useStableCallback(getTimelineScrollableNode); + const conversationArea = workspaceAreaProps("conversation", 1, "Conversation"); + const composerArea = workspaceAreaProps("composer", 2, "Message input"); const [composerOverlayElement, setComposerOverlayElement] = useState(null); const [composerTimelineInset, setComposerTimelineInset] = useState(0); const composerTimelineInsetRef = useRef(0); @@ -866,6 +873,7 @@ export default function ChatView(props: ChatViewProps) { ); const diffOpen = activeRightPanelKind === "diff"; const explicitDiffOpenRef = useRef(null); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- resets the diff panel store to branch scope when the diff opens without an explicit request useLayoutEffect(() => { const explicitThreadRef = explicitDiffOpenRef.current; explicitDiffOpenRef.current = null; @@ -977,6 +985,7 @@ export default function ChatView(props: ChatViewProps) { }; const handleFilePendingChangeStable = useStableCallback(handleFilePendingChange); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- drops file surfaces from the right panel store once bootstrap shows the thread has no workspace useEffect(() => { if (!activeThreadRef || !activeEnvironmentBootstrapComplete) return; useRightPanelStore.getState().reconcileFileSurfaces(activeThreadRef, activeProject !== null); @@ -1419,6 +1428,7 @@ export default function ChatView(props: ChatViewProps) { payload: { answers: request.answers }, })), ); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- persists the server-reported resume compaction dismissal to localStorage useEffect(() => { if (nativeResumeCompactionDismissed && !resumeCompactionPermanentlyDismissed) { setResumeCompactionPermanentlyDismissed(true); @@ -1448,6 +1458,7 @@ export default function ChatView(props: ChatViewProps) { const activeTerminalLaunchContext = terminalUiLaunchContext?.threadId === activeThreadId ? terminalUiLaunchContext : null; const liveIsGitRepo = gitStatusQuery.data?.isRepo; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- records live git repo status in the session cache read while the status query is unavailable useEffect(() => { if (gitStatusCwd !== null && liveIsGitRepo !== undefined) { rememberCheckoutIsRepo(environmentId, gitStatusCwd, liveIsGitRepo); @@ -1723,6 +1734,7 @@ export default function ChatView(props: ChatViewProps) { visiblePullRequestCount, onDiffPanelOpen, }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- consumes a pending maximize request from the right panel store once the panel can maximize useEffect(() => { if (!canMaximizeRightPanel) return; if (useRightPanelStore.getState().consumeMaximizeRequest(routeThreadRef)) { @@ -1837,6 +1849,7 @@ export default function ChatView(props: ChatViewProps) { setTimelineAnchor, }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- schedules a frame to focus the composer DOM element when a thread opens with the terminal closed useEffect(() => { if (!activeThread?.id || terminalUiState.terminalOpen) return; const frame = window.requestAnimationFrame(() => { @@ -1847,6 +1860,7 @@ export default function ChatView(props: ChatViewProps) { }; }, [activeThread?.id, focusComposerStable, terminalUiState.terminalOpen]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- window focus listener that refocuses the composer after the window regains focus useEffect(() => { if (!activeThread?.id || terminalUiState.terminalOpen || isMobileViewport) return; let frame: number | null = null; @@ -2004,6 +2018,7 @@ export default function ChatView(props: ChatViewProps) { composerRestingRef.current = resting; }; const onComposerRestingChangeStable = useStableCallback(onComposerRestingChange); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- measures the composer overlay element before paint for a new thread or composer mount useLayoutEffect(() => { if (!composerOverlayElement) return; composerTimelineInsetRef.current = 0; @@ -2018,6 +2033,7 @@ export default function ChatView(props: ChatViewProps) { publishComposerOverlayHeightStable, ]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- observes the composer overlay element's size before paint and publishes its height useLayoutEffect(() => { if (!composerOverlayElement) return; @@ -2043,37 +2059,18 @@ export default function ChatView(props: ChatViewProps) { ]); const openPanelPullRequestUrl = useOpenPanelPullRequestUrl(activeThreadRef); const activeThreadReferenceCopyTarget = - activeThreadId === null || !isServerThread + activeThreadRef === null || !isServerThread ? null : resolveThreadReferenceCopyTarget({ - threadId: activeThreadId, + threadRef: activeThreadRef, + title: activeThread?.title ?? "", openPanelPullRequestUrl, pullRequests: activeThread?.pullRequests, linkedPullRequestUrl: linkedThreadPullRequest?.url ?? null, }); const copyActiveThreadReference = () => { - const target = activeThreadReferenceCopyTarget; - if (target === null) return; - void writeTextToClipboard(target.value, target.clipboardTarget).then( - (didCopy) => { - if (!didCopy) return; - toastManager.add({ - type: "success", - title: target.successTitle, - description: target.value, - }); - }, - (error) => { - console.error(error); - toastManager.add( - stackedThreadToast({ - type: "error", - title: target.failureTitle, - description: error instanceof Error ? error.message : "An error occurred.", - }), - ); - }, - ); + if (activeThreadReferenceCopyTarget === null) return; + void copyThreadReference(activeThreadReferenceCopyTarget); }; const pullRequestPanelTarget = activeThread ? threadPullRequestPanelTarget({ @@ -2154,6 +2151,7 @@ export default function ChatView(props: ChatViewProps) { runtimeMode, isWorking, pendingBackgroundTasks, + turnItems: serverProjection?.turnItems ?? EMPTY_TURN_ITEMS, activePendingApproval, activePendingUserInput, localCheckoutBranchMismatch, @@ -2318,6 +2316,7 @@ export default function ChatView(props: ChatViewProps) { // Entering a thread with its drawer open focuses the drawer terminal; opening // it from a handler requests focus there. A closed drawer leaves focus to the // composer effect above. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- focuses the open drawer terminal when entering a thread useEffect(() => { if (activeThreadRef) return focusTerminalOnThreadEnter(activeThreadRef); }, [activeThreadRef]); @@ -2596,6 +2595,8 @@ export default function ChatView(props: ChatViewProps) { })} onEditQueuedRun={queuedRunEdit.beginEdit} onCancelEdit={queuedRunEdit.cancelEdit} + timestampFormat={timestampFormat} + onOpenThread={onOpenRelatedThreadStable} /> ) : null; @@ -2616,6 +2617,7 @@ export default function ChatView(props: ChatViewProps) { !activeEnvironmentUnavailable && !activePendingProgress && !feedbackUploading; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- switches the draft to local mode, then resends the waiting message once sending is idle useEffect(() => { if (!workLocallyResendReady || sendInFlightRef.current) { return; @@ -2697,6 +2699,7 @@ export default function ChatView(props: ChatViewProps) { const consumePendingFileDrop = useSidebarPendingFileDropStore( (state) => state.consumePendingFileDrop, ); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- hands pending sidebar file drops to the composer's imperative API once it mounts useEffect(() => { if (pendingSidebarFileDrops.length === 0) return; if ( @@ -2836,6 +2839,7 @@ export default function ChatView(props: ChatViewProps) { )} data-chat-column-maximized-away={rightPanelMaximized ? "true" : "false"} data-chat-column + inert={rightPanelMaximized} >
-
+
{ setComposerOverlayElement(overlay); attachFollowComposerOverlay(overlay); diff --git a/apps/web/src/components/CommandPalette.tsx b/apps/web/src/components/CommandPalette.tsx index f91d82e02ff8e..e9dc218403966 100644 --- a/apps/web/src/components/CommandPalette.tsx +++ b/apps/web/src/components/CommandPalette.tsx @@ -61,6 +61,7 @@ import { PaletteIcon, RotateCcwIcon, SettingsIcon, + MonitorIcon, SquarePenIcon, TextSearchIcon, } from "lucide-react"; @@ -79,7 +80,11 @@ import { useAtomValue } from "@effect/atom-react"; import { useHandleNewThread } from "../hooks/useHandleNewThread"; import { useOpenPanelPullRequestUrl } from "../hooks/useOpenPanelPullRequestUrl"; -import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; +import { copyThreadReference } from "../lib/copyThreadReference"; +import { + queueWorkspaceFocusFromPalette, + restoreWorkspaceFocusFromPalette, +} from "../lib/workspaceFocus"; import { useClientSettings } from "../hooks/useSettings"; import { useStableCallback } from "../hooks/useStableCallback"; import { filesystemEnvironment } from "../state/filesystem"; @@ -110,6 +115,7 @@ import { } from "../lib/projectPaths"; import { dispatchCommandPalette, + getCommandPaletteReturnFocus, useCommandPaletteMount, useCommandPaletteStore, } from "../commandPaletteStore"; @@ -298,8 +304,12 @@ function CommandPaletteDialog(props: { data-palette-mode={props.mode} data-testid="command-palette" finalFocus={() => { + if (restoreWorkspaceFocusFromPalette()) return false; + const returnFocus = getCommandPaletteReturnFocus(); + if (returnFocus === false) return false; const composer = composerHandleRef?.current; - if (!composer) return true; + // Behind the Settings modal the composer is inert and cannot take focus. + if (!composer || document.querySelector("[data-app-modal]")) return returnFocus; composer.focusAtEnd(); return false; }} @@ -412,33 +422,16 @@ function OpenCommandPaletteDialog(props: { referenceThreadRef === null || (pathname === "/pull-requests" && !openPanelPullRequestUrl) ? null : resolveThreadReferenceCopyTarget({ - threadId: referenceThreadRef.threadId, + threadRef: referenceThreadRef, + title: activeThread?.title ?? "", openPanelPullRequestUrl, pullRequests: activeThread?.pullRequests, linkedPullRequestUrl: activeThread?.linkedPullRequest?.url ?? activeThread?.branchPullRequest?.url ?? null, }); const copyActiveThreadReference = async () => { - const target = activeThreadReferenceCopyTarget; - if (target === null) return; - try { - const didCopy = await writeTextToClipboard(target.value, target.clipboardTarget); - if (!didCopy) return; - toastManager.add({ - type: "success", - title: target.successTitle, - description: target.value, - }); - } catch (error) { - console.error(error); - toastManager.add( - stackedThreadToast({ - type: "error", - title: target.failureTitle, - description: error instanceof Error ? error.message : "An error occurred.", - }), - ); - } + if (activeThreadReferenceCopyTarget === null) return; + await copyThreadReference(activeThreadReferenceCopyTarget); }; const threads = useThreadShells(); const keybindings = useAtomValue(primaryServerKeybindingsAtom); @@ -668,6 +661,7 @@ function OpenCommandPaletteDialog(props: { }); }; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- invalidates the browse navigation coordinator and in-flight clone lookups when the palette session unmounts useEffect( () => () => { browseNavigation.invalidate(); @@ -1262,6 +1256,7 @@ function OpenCommandPaletteDialog(props: { }); } + // Coder: no "Triage an issue"; it files T3 Code issues from a clone of upstream's repository. if (activeThreadKey !== null && supportsThreadFind) { actionItems.push({ kind: "action", @@ -1284,10 +1279,10 @@ function OpenCommandPaletteDialog(props: { searchTerms: activeThreadReferenceCopyTarget.kind === "pull-request" ? ["copy", "merge request", "mr link", "reference"] - : ["copy", "thread id", "reference"], + : ["copy", "thread", "thread id", "reference"], title: - activeThreadReferenceCopyTarget.kind === "pull-request" ? "Copy MR link" : "Copy thread ID", - description: activeThreadReferenceCopyTarget.value, + activeThreadReferenceCopyTarget.kind === "pull-request" ? "Copy MR link" : "Copy thread", + description: activeThreadReferenceCopyTarget.description, icon: , shortcutCommand: "thread.copyReference", run: copyActiveThreadReference, @@ -1469,6 +1464,21 @@ function OpenCommandPaletteDialog(props: { }); } + for (const direction of [1, -1] as const) { + const title = direction === 1 ? "Focus Next Workspace Area" : "Focus Previous Workspace Area"; + actionItems.push({ + kind: "action", + value: `action:workspace-focus:${direction}`, + title, + searchTerms: [title, "accessibility", "navigation", "focus", "F6"], + icon: , + shortcutCommand: direction === 1 ? "workspace.focusNext" : "workspace.focusPrevious", + run: async () => { + queueWorkspaceFocusFromPalette(direction); + }, + }); + } + actionItems.push({ kind: "action", value: "action:settings", diff --git a/apps/web/src/components/ComposerPromptEditorTiptap.tsx b/apps/web/src/components/ComposerPromptEditorTiptap.tsx index af5a200e264af..abc9549ee3d00 100644 --- a/apps/web/src/components/ComposerPromptEditorTiptap.tsx +++ b/apps/web/src/components/ComposerPromptEditorTiptap.tsx @@ -1074,7 +1074,7 @@ function ComposerPromptEditorTiptapInner( const editorAttributes = { class: cn( - "composer-tiptap -m-1 block max-h-52 min-h-12 overflow-y-auto p-1 whitespace-pre-wrap wrap-break-word bg-transparent leading-relaxed text-foreground focus:outline-none", + "composer-tiptap -m-1 block max-h-52 min-h-[calc(2lh+--spacing(2))] overflow-y-auto p-1 whitespace-pre-wrap wrap-break-word bg-transparent leading-relaxed text-foreground focus:outline-none", className, ), "data-testid": "composer-editor", @@ -1445,11 +1445,13 @@ function ComposerPromptEditorTiptapInner( [], ); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- pushes the disabled prop into the imperative Tiptap editor via setEditable useEffect(() => { editor?.setEditable(!disabled); }, [disabled, editor]); // Push attributes before layout measurements; Tiptap updates them later in a passive effect. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- pushes DOM attributes into the ProseMirror view before layout measurements read them useLayoutEffect(() => { if (!editor?.isInitialized) return; editor.view.setProps({ attributes: editorAttributes }); @@ -1510,6 +1512,7 @@ function ComposerPromptEditorTiptapInner( // The one place the controlled value reaches the editor. Imperative handlers compare against // the committed value, so it is recorded here even before the editor exists. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- writes the controlled value and cursor into the Tiptap document before paint useLayoutEffect(() => { latestValueRef.current = value; if (!editor) return; @@ -1594,6 +1597,7 @@ function ComposerPromptEditorTiptapInner( ]); // Reconcile loaded or switched provider skills without rebuilding citation node views. + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- replaces skill chip nodes in the Tiptap document when the provider skill list loads or changes useEffect(() => { if (!editor || literalText) return; const replacements = skillChipReplacements(editor.state.doc, (name) => @@ -1602,6 +1606,7 @@ function ComposerPromptEditorTiptapInner( if (replacements.length > 0) replaceSkillChips(editor, replacements); }, [editor, literalText, skills]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- restores ProseMirror focus and caret scroll when the keyed editor remounts, capturing focus on teardown useLayoutEffect(() => { if (!editor) return; if (restoreFocusRef.current) { diff --git a/apps/web/src/components/CustomSnoozeDialog.tsx b/apps/web/src/components/CustomSnoozeDialog.tsx index 74414dc5f1ea2..efbc5d90da6b6 100644 --- a/apps/web/src/components/CustomSnoozeDialog.tsx +++ b/apps/web/src/components/CustomSnoozeDialog.tsx @@ -56,6 +56,7 @@ function startOfLocalDay(ms: number) { export function CustomSnoozeDialogHost() { const request = useRequest((state) => state.request); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- resolves any pending snooze request promise with null when the dialog host unmounts useEffect(() => () => finish(null), []); return request ? : null; } diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index f677f074c9c50..5eaf63cb0d941 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -266,6 +266,7 @@ export default function DiffPanel({ return right.completedAt.localeCompare(left.completedAt); }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- rewrites the persisted diff store selection to the latest turn when the selected turn disappears useEffect(() => { if (!routeThreadRef || diffSelection.kind !== "turn") return; useDiffPanelStore.getState().reconcileTurnSelection( @@ -592,6 +593,7 @@ export default function DiffPanel({ // A file another view asked to reveal (a chat diff link): retried as the diff loads until it // holds the file, then not again until a new request, so it does not undo what the user did since. const externalRevealRef = useRef(null); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- reveals an externally requested file in the diff viewer, retrying as the diff DOM loads useEffect(() => { if (!selectedFilePath) { externalRevealRef.current = null; @@ -678,6 +680,7 @@ export default function DiffPanel({ render={ - ) : scopedProjectGroup ? ( - `No threads in ${scopedProjectGroup.displayName} yet` + ) : scopedProjectGroups.length > 1 ? ( + "No threads in these projects yet" + ) : scopedProjectGroups[0] ? ( + `No threads in ${scopedProjectGroups[0].displayName} yet` ) : ( "No threads yet" )} diff --git a/apps/web/src/components/SidebarStageBackdrop.tsx b/apps/web/src/components/SidebarStageBackdrop.tsx index b272c5724feb7..b5c9d34ac6670 100644 --- a/apps/web/src/components/SidebarStageBackdrop.tsx +++ b/apps/web/src/components/SidebarStageBackdrop.tsx @@ -1,4 +1,5 @@ import { useAtomValue } from "@effect/atom-react"; +import type { EnvironmentIdentificationMode } from "@t3tools/contracts"; import { useId } from "react"; import { APP_STAGE_LABEL } from "../branding"; @@ -12,9 +13,11 @@ const STAGE_BACKDROP_VIEW_BOX = "0 0 8192 96"; export function resolveSidebarStageBackdropVariant( stageLabel: string, - enabled = true, + mode: EnvironmentIdentificationMode = "artwork", ): SidebarStageBackdropVariant | null { - if (!enabled) return null; + if (mode === "nightly-artwork") return "nightly"; + if (mode === "dev-artwork") return "dev"; + if (mode !== "artwork") return null; const normalized = stageLabel.trim().toLowerCase(); if (normalized === "nightly") return "nightly"; if (normalized === "dev") return "dev"; @@ -40,8 +43,10 @@ export function useEnvironmentStageLabel(): string { }); } -export function useSidebarStageBackdropVariant(enabled = true): SidebarStageBackdropVariant | null { - return resolveSidebarStageBackdropVariant(useEnvironmentStageLabel(), enabled); +export function useSidebarStageBackdropVariant( + mode: EnvironmentIdentificationMode, +): SidebarStageBackdropVariant | null { + return resolveSidebarStageBackdropVariant(useEnvironmentStageLabel(), mode); } export function SidebarStageBackdrop({ variant }: { variant: SidebarStageBackdropVariant }) { @@ -55,7 +60,7 @@ export function SidebarStageBackdrop({ variant }: { variant: SidebarStageBackdro ); } -export function StageBackdropArt({ variant }: { variant: SidebarStageBackdropVariant }) { +function StageBackdropArt({ variant }: { variant: SidebarStageBackdropVariant }) { return variant === "nightly" ? : ; } diff --git a/apps/web/src/components/ThreadRouteView.tsx b/apps/web/src/components/ThreadRouteView.tsx index cf3c0427c57c3..d9a8e1ffd681e 100644 --- a/apps/web/src/components/ThreadRouteView.tsx +++ b/apps/web/src/components/ThreadRouteView.tsx @@ -90,6 +90,7 @@ export function ThreadRouteView({ }); const environmentHasAnyThreads = environmentThreadRefs.length > 0 || environmentHasDraftThreads; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- clears pending file drops and navigates the router home once a server thread is confirmed missing useEffect(() => { if (target.kind !== "server" || !bootstrapComplete) { return; diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index 5e87cd70b784a..aeef386331b81 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -1,3 +1,4 @@ +import { workspaceAreaProps } from "../lib/workspaceFocus"; import { useAtomMount } from "@effect/atom-react"; import { isAtomCommandInterrupted, @@ -595,12 +596,14 @@ function TerminalSessionViewport({ }, ); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- sets the terminal input's readOnly from the operate grant and consumes a pending focus request useLayoutEffect(() => { if (terminalRef.current) terminalRef.current.input.readOnly = !canOperateTerminal; if (canOperateTerminal && visibleRef.current) consumePendingTerminalFocus(threadRef, terminalId); }, [canOperateTerminal, terminalId, threadRef]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- pushes visibility into the terminal surface before paint and consumes focus requests made while hidden useLayoutEffect(() => { visibleRef.current = visible; terminalRef.current?.setVisible(visible); @@ -608,6 +611,7 @@ function TerminalSessionViewport({ if (visible) consumePendingTerminalFocus(threadRef, terminalId); }, [terminalId, threadRef, visible]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- applies font family and size changes to the live Ghostty terminal surface useEffect(() => { const current = terminalFontRef.current; if (current.family === terminalFontFamily && current.size === terminalFontSize) return; @@ -615,6 +619,7 @@ function TerminalSessionViewport({ void terminalRef.current?.setFont(terminalFontOptions(terminalFontFamily, terminalFontSize)); }, [terminalFontFamily, terminalFontSize]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- creates the Ghostty terminal in the mount node and subscribes it to the terminal session useEffect(() => { const mount = containerRef.current; if (!mount) return; @@ -652,6 +657,7 @@ function TerminalSessionViewport({ terminal.setTheme(terminalThemeFromApp(mount)); setupTerminal = terminal; terminalRef.current = terminal; + terminal.input.setAttribute("data-workspace-focus-default", ""); terminal.input.readOnly = !hasTerminalWriteAccess(); // Reapply font settings hydrated during loading, when terminalRef was null. const currentFont = terminalFontRef.current; @@ -1365,6 +1371,7 @@ export default function ThreadTerminalDrawer({ terminalLaunchLocationsById, }: ThreadTerminalDrawerProps) { const canOperateTerminal = useEnvironmentScope(threadRef.environmentId, AuthTerminalOperateScope); + const terminalArea = workspaceAreaProps("terminal", 5, "Terminal"); const isPanel = mode === "panel"; const [advancedTypography] = useLocalStorage( TYPOGRAPHY_ADVANCED_STORAGE_KEY, @@ -1622,6 +1629,7 @@ export default function ThreadTerminalDrawer({ if (heightBudgetChanged) setDrawerLayout(layout); }, ); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- measures the drawer before visible paint and observes window and chat column resizes while visible useLayoutEffect(() => { if (!visible) { return; @@ -1648,6 +1656,7 @@ export default function ThreadTerminalDrawer({ return (