Repository navigation
perf(server): idle agent sessions free their memory after 2 minutes #35711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,7 +366,7 @@ const CLAUDE_STOP_CONFIRM_TIMEOUT_MS = 10_000; | |
| const CLAUDE_STDERR_TAIL_LENGTH = 2_048; | ||
| // The process group is killed on exit, so its stderr closes promptly. | ||
| const CLAUDE_STDERR_DRAIN_TIMEOUT = Duration.seconds(1); | ||
| // How often the CLI's process tree is recorded when no agent scope will stop what it leaves. | ||
| // How often the working CLI's process tree is recorded when no agent scope will stop what it leaves. | ||
| const CLAUDE_DESCENDANT_POLL = "1 second"; | ||
| // Claude Code's refusal of bypassPermissions as root, which it explains only on stderr. | ||
| const CLAUDE_ROOT_REFUSAL = | ||
|
|
@@ -1286,14 +1286,36 @@ export const layerQueryRunner: Layer.Layer< | |
| for (const row of claudeDescendants(rows, roots)) tracked.set(identity(row), row); | ||
| return running; | ||
| }); | ||
| const stop = refresh(false).pipe( | ||
| const refreshRunning = refresh(true).pipe(Effect.asVoid); | ||
| let exited = false; | ||
| let polling: Fiber.Fiber<unknown> | undefined; | ||
| // Jobs start only while the CLI works, so polling runs from launch or a prompt until a result. | ||
| const poll = () => { | ||
| if (exited || polling !== undefined) return; | ||
| polling = runFork( | ||
| refreshRunning.pipe(Effect.repeat(Schedule.spaced(CLAUDE_DESCENDANT_POLL))), | ||
| ); | ||
| }; | ||
| const pause = Effect.suspend(() => { | ||
| const fiber = polling; | ||
| polling = undefined; | ||
| return fiber === undefined ? Effect.void : Fiber.interrupt(fiber); | ||
| }); | ||
| const settle = pause.pipe( | ||
| Effect.andThen(Effect.suspend(() => (exited ? Effect.void : refreshRunning))), | ||
| ); | ||
| const stop = Effect.suspend(() => { | ||
| exited = true; | ||
| return pause; | ||
| }).pipe( | ||
| Effect.andThen(refresh(false)), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Serialize
If the settle read completes after the stop read has finished its kill pass, a job found in that settle read is recorded in Run the process-table read through one shared, single-permit gate so that every
|
||
| Effect.map((running) => { | ||
| for (const row of tracked.values()) { | ||
| if (running.has(identity(row))) killClaudeDescendant(row); | ||
| } | ||
| }), | ||
| ); | ||
| return { refresh: refresh(true).pipe(Effect.asVoid), stop }; | ||
| return { refresh: refreshRunning, poll, settle, stop }; | ||
| }; | ||
|
|
||
| return ClaudeAgentSdkQueryRunner.of({ | ||
|
|
@@ -1359,7 +1381,7 @@ export const layerQueryRunner: Layer.Layer< | |
| if (answered) return; | ||
| stderrTail = (stderrTail + text).slice(-CLAUDE_STDERR_TAIL_LENGTH); | ||
| }; | ||
| let refreshDescendants: Effect.Effect<void> = Effect.void; | ||
| let descendants: ReturnType<typeof trackDescendants> | undefined; | ||
| const spawnProcess = | ||
| options.spawnClaudeCodeProcess ?? | ||
| ((spawnOptions: ClaudeSpawnOptions) => { | ||
|
|
@@ -1375,13 +1397,9 @@ export const layerQueryRunner: Layer.Layer< | |
| ); | ||
| if (unit === undefined && child.pid !== undefined) { | ||
| const tracker = trackDescendants(child.pid); | ||
| const polling = runFork( | ||
| tracker.refresh.pipe(Effect.repeat(Schedule.spaced(CLAUDE_DESCENDANT_POLL))), | ||
| ); | ||
| child.once("exit", () => | ||
| runFork(Fiber.interrupt(polling).pipe(Effect.andThen(tracker.stop))), | ||
| ); | ||
| refreshDescendants = tracker.refresh; | ||
| tracker.poll(); | ||
| child.once("exit", () => runFork(tracker.stop)); | ||
| descendants = tracker; | ||
| } | ||
| return child; | ||
| } | ||
|
|
@@ -1542,6 +1560,16 @@ export const layerQueryRunner: Layer.Layer< | |
| answered = true; | ||
| }), | ||
| ), | ||
| // The result waits for the turn's last tree read, so a session closed on it keeps that job. | ||
| Stream.tap((message) => | ||
| descendants === undefined | ||
| ? Effect.void | ||
| : message.type === "result" | ||
| ? descendants.settle | ||
| : message.type === "assistant" | ||
| ? Effect.sync(descendants.poll) | ||
| : Effect.void, | ||
| ), | ||
| Stream.tap((message) => | ||
| logProtocolEvent({ | ||
| direction: "incoming", | ||
|
|
@@ -1553,7 +1581,12 @@ export const layerQueryRunner: Layer.Layer< | |
| offer: (message, onQueued) => | ||
| Effect.uninterruptibleMask((restore) => | ||
| restore(Queue.offer(promptQueue, message)).pipe( | ||
| Effect.tap(() => Effect.sync(() => onQueued?.())), | ||
| Effect.tap(() => | ||
| Effect.sync(() => { | ||
| descendants?.poll(); | ||
| onQueued?.(); | ||
| }), | ||
| ), | ||
| ), | ||
| ).pipe( | ||
| Effect.asVoid, | ||
|
|
@@ -1691,7 +1724,9 @@ export const layerQueryRunner: Layer.Layer< | |
| ), | ||
| ), | ||
| // A job started since the last poll is recorded before the CLI is told to exit. | ||
| close: Effect.suspend(() => (processExited ? Effect.void : refreshDescendants)).pipe( | ||
| close: Effect.suspend(() => | ||
| processExited || descendants === undefined ? Effect.void : descendants.refresh, | ||
| ).pipe( | ||
| Effect.andThen(Queue.shutdown(promptQueue)), | ||
| Effect.andThen(closeClaudeQuery(queryRuntime)), | ||
| Effect.tap(() => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,16 @@ import { makeProviderFailure } from "@t3tools/provider-core/server/failure"; | |
| import * as ProviderAdapterRegistry from "./ProviderAdapterRegistry.ts"; | ||
| import * as ProjectionStore from "./ProjectionStore.ts"; | ||
|
|
||
| const DEFAULT_IDLE_TIMEOUT_MS = 30 * 60 * 1000; | ||
| const DEFAULT_IDLE_TIMEOUT_MS = 2 * 60 * 1000; | ||
| // These resume natively only when the external ACP agent supports it; otherwise a fresh session | ||
| // gets a summary of the thread, so they keep the longer timeout. | ||
| const UNVERIFIED_RESUME_IDLE_TIMEOUT_MS = 30 * 60 * 1000; | ||
| const UNVERIFIED_RESUME_DRIVERS: ReadonlySet<string> = new Set([ | ||
| "acpRegistry", | ||
| "grok", | ||
| "kiro", | ||
| "fx", | ||
| ]); | ||
| const DEFAULT_MAX_IDLE_PIN_MS = 4 * 60 * 60 * 1000; | ||
| const RELEASE_SCOPE_CLOSE_TIMEOUT_MS = 30 * 1000; | ||
| /** Past adapters' own 10 s stop fallbacks, so they settle first when the provider answers at all. */ | ||
|
|
@@ -503,7 +512,14 @@ export const layerWithOptions = ( | |
| readonly providerSessionId: ProviderSessionId; | ||
| readonly threadId: ThreadId; | ||
| }) => `${input.providerSessionId}\u0000${input.threadId}`; | ||
| const idleTimeoutMs = Math.max(1, options.idleTimeoutMs ?? DEFAULT_IDLE_TIMEOUT_MS); | ||
| const idleTimeoutFor = (runtime: ProviderAdapter.ProviderAdapterV2SessionRuntime) => | ||
| Math.max( | ||
| 1, | ||
| options.idleTimeoutMs ?? | ||
| (UNVERIFIED_RESUME_DRIVERS.has(runtime.driver) | ||
| ? UNVERIFIED_RESUME_IDLE_TIMEOUT_MS | ||
| : DEFAULT_IDLE_TIMEOUT_MS), | ||
|
Comment on lines
+519
to
+521
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Skip idle release when the runtime cannot resume after restart.
Apply the same guard that Suggested fix
|
||
| ); | ||
| const maxIdlePinMs = Math.max(0, options.maxIdlePinMs ?? DEFAULT_MAX_IDLE_PIN_MS); | ||
| interface PreparedMcpCredential { | ||
| readonly mcpCredentialId: string | undefined; | ||
|
|
@@ -1390,7 +1406,7 @@ export const layerWithOptions = ( | |
| pinnedForMs: now - pinnedSinceMs, | ||
| }); | ||
| // Recheck on this fiber; scheduleIdleReleaseInternal would interrupt it and self-deadlock. | ||
| yield* Effect.sleep(Duration.millis(idleTimeoutMs)); | ||
| yield* Effect.sleep(Duration.millis(idleTimeoutFor(probedRuntime))); | ||
| return yield* releaseIfStillIdle(input); | ||
| } | ||
| yield* Effect.logWarning("orchestration-v2.driver-session.idle-release-pin-expired", { | ||
|
|
@@ -1459,7 +1475,7 @@ export const layerWithOptions = ( | |
|
|
||
| yield* cancelIdleFiber(entry.idleFiber); | ||
| const generation = entry.idleGeneration + 1; | ||
| const idleFiber = yield* Effect.sleep(Duration.millis(idleTimeoutMs)).pipe( | ||
| const idleFiber = yield* Effect.sleep(Duration.millis(idleTimeoutFor(entry.runtime))).pipe( | ||
| Effect.andThen(releaseIfStillIdle({ providerSessionId, generation })), | ||
| Effect.forkIn(layerScope), | ||
| ); | ||
|
Comment on lines
1475
to
1481
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Count pending Pi prompts in the background-work probe. The Pi Suggested fix
|
||
|
|
@@ -1812,7 +1828,7 @@ export const layerWithOptions = ( | |
| if (tracked === undefined) return; | ||
| const [threadId, pending] = tracked; | ||
| const generation = pending.generation + 1; | ||
| const fiber = yield* Effect.sleep(Duration.millis(idleTimeoutMs)).pipe( | ||
| const fiber = yield* Effect.sleep(Duration.millis(idleTimeoutFor(entry.runtime))).pipe( | ||
| Effect.andThen(unloadIdleThread({ providerSessionId, threadId, generation })), | ||
| Effect.forkIn(layerScope), | ||
| ); | ||
|
|
@@ -1890,10 +1906,12 @@ export const layerWithOptions = ( | |
| : yield* entry.runtime | ||
| .hasPendingBackgroundWorkForThread(pending.providerThread) | ||
| .pipe(Effect.catchCause(() => Effect.succeed(false))); | ||
| if (hasPendingWork) return "deferred" as const; | ||
| if (hasPendingWork) return { retryAfterMs: idleTimeoutFor(entry.runtime) }; | ||
| const child = yield* delegatedChildArchive(input.threadId); | ||
| // Unloading now would miss the archive once the parent takes the result. | ||
| if (child === "awaiting_delivery") return "deferred" as const; | ||
| if (child === "awaiting_delivery") { | ||
| return { retryAfterMs: idleTimeoutFor(entry.runtime) }; | ||
| } | ||
| const archive = child === "ready"; | ||
| if (input.archiveOnly === true && !archive) return "skipped" as const; | ||
| const unloading = yield* Ref.modify(sessions, (current) => { | ||
|
|
@@ -1958,8 +1976,8 @@ export const layerWithOptions = ( | |
| }), | ||
| ); | ||
| // Wait outside the lock so the next attach is not blocked. | ||
| if (outcome === "deferred" && input.archiveOnly !== true) { | ||
| yield* Effect.sleep(Duration.millis(idleTimeoutMs)); | ||
| if (typeof outcome === "object" && input.archiveOnly !== true) { | ||
| yield* Effect.sleep(Duration.millis(outcome.retryAfterMs)); | ||
| return yield* unloadIdleThread(input); | ||
| } | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep tracking descendants while background jobs can create children.
If a recorded background job starts a detached child after
settle, then exits before the CLI closes, the detached child becomes unrecorded and unreachable. Polling has stopped. Neither the close read norrefresh(false)can rediscover the child through its terminated parent. The CLI group kill also cannot reach the detached child.This change therefore allows background processes to survive session cleanup. Keep tracking while recorded background jobs remain, or use an ownership mechanism that also covers children created after the turn.