From 269ae00738c131fe71a3560aec944397e954c8b7 Mon Sep 17 00:00:00 2001 From: deepagent-ai Date: Thu, 23 Jul 2026 13:45:46 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix(provider):=20align=20API=20error=20mess?= =?UTF-8?q?age=20format=20with=20Codex=20=E2=80=94=20include=20status=20co?= =?UTF-8?q?de,=20provider=20detail,=20url=20and=20request=20id?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/deepagent-code/src/provider/error.ts | 68 +++++++++++-------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/packages/deepagent-code/src/provider/error.ts b/packages/deepagent-code/src/provider/error.ts index c64f4fd5..d17a8815 100644 --- a/packages/deepagent-code/src/provider/error.ts +++ b/packages/deepagent-code/src/provider/error.ts @@ -27,47 +27,60 @@ function isOpenAiErrorRetryable(e: APICallError) { return status === 404 || e.isRetryable } +const RESPONSE_BODY_MAX_BYTES = 1000 + +function truncateBody(text: string, maxBytes: number) { + if (text.length <= maxBytes) return text + let cut = maxBytes + while (cut > 0 && (text.charCodeAt(cut) & 0xfc00) === 0xdc00) cut-- + return text.slice(0, cut) + "..." +} + +function displayBody(e: APICallError) { + if (!e.responseBody) return undefined + const trimmed = e.responseBody.trim() + if (!trimmed) return undefined + try { + const parsed = JSON.parse(trimmed) + const errMsg = parsed?.error?.message + if (typeof errMsg === "string" && errMsg.trim()) return errMsg.trim() + } catch {} + return truncateBody(trimmed, RESPONSE_BODY_MAX_BYTES) +} + // Providers not reliably handled in this function: // - z.ai: can accept overflow silently (needs token-count/context-window checks) function message(providerID: ProviderV2.ID, e: APICallError) { return iife(() => { - const msg = e.message - if (msg === "") { - if (e.responseBody) return e.responseBody - if (e.statusCode) { - const err = STATUS_CODES[e.statusCode] - if (err) return err - } - return "Unknown error" - } - - if (!e.responseBody || (e.statusCode && msg !== STATUS_CODES[e.statusCode])) { - return msg - } - - try { - const body = JSON.parse(e.responseBody) - // try to extract common error message fields - const errMsg = body.message || body.error || body.error?.message - if (errMsg && typeof errMsg === "string") { - return `${msg}: ${errMsg}` - } - } catch {} + const statusText = e.statusCode ? STATUS_CODES[e.statusCode] : undefined // If responseBody is HTML (e.g. from a gateway or proxy error page), // provide a human-readable message instead of dumping raw markup - if (/^\s*` to re-authenticate." } if (e.statusCode === 403) { return "Forbidden: request was blocked by a gateway or proxy. You may not have permission to access this resource — check your account and provider settings." } - return msg } - return `${msg}: ${e.responseBody}` - }).trim() + const body = displayBody(e) + const detail = body ?? e.message ?? statusText ?? "Unknown error" + + let result: string + if (e.statusCode) { + result = statusText ? `unexpected status ${e.statusCode} ${statusText}: ${detail}` : `unexpected status ${e.statusCode}: ${detail}` + } else { + result = detail + } + + if (e.url) result += `, url: ${e.url}` + const requestId = e.responseHeaders?.["x-request-id"] + if (requestId) result += `, request id: ${requestId}` + + return result.trim() + }) } function json(input: unknown) { @@ -165,7 +178,8 @@ export type ParsedAPICallError = export function parseAPICallError(input: { providerID: ProviderV2.ID; error: APICallError }): ParsedAPICallError { const m = message(input.providerID, input.error) const body = json(input.error.responseBody) - if (isContextOverflow(m) || input.error.statusCode === 413 || body?.error?.code === "context_length_exceeded") { + const rawDetail = input.error.message || input.error.responseBody || "" + if (isContextOverflow(rawDetail) || input.error.statusCode === 413 || body?.error?.code === "context_length_exceeded") { return { type: "context_overflow", message: m, From aca62551ffddf6f6b88c605f9a72f9e8722c1a2a Mon Sep 17 00:00:00 2001 From: deepagent-ai Date: Thu, 23 Jul 2026 19:42:34 +0800 Subject: [PATCH 2/2] fix(4.0.4_r4): F1-F6 stability fixes + startup UX overhaul MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: Activity-level tool-call loop detection (cross-message) - Add ToolSequenceTracker: period 1-4 detection, canonical JSON fingerprint - Share one tracker per runLoop (activity), detect ABABAB across messages - 21 new tests covering all acceptance criteria F3: Tool parameter JSON parsing safety (AI SDK v6) - Fix repair callback to use failed.toolCall.input (not .args) - Classify errors: unknown_tool / invalid_json / schema_mismatch - Never repair write/edit content; explicit fail with bounded error message - 19 new tests F4: Right-click copy session ID (verify existing impl) - Confirmed: clipboard?.writeText(props.session.id), en/zh i18n complete F5: Subagent hard depth limit + unified child admission - MAX_SUBAGENT_DEPTH=3 process-level constant, cannot be widened by config - resolveSessionDepth: metadata + parentID chain, cycle detection, fail closed - admitChildOrFail: unified boundary for normal/takeover/background/resume - Fix canTask/canTodo: evaluate() replaces rule-presence check - 29 new tests F6: Session-scoped terminal cache + LRU eviction - Replace directoryTerminalCache with ScopedKey{scope+dir+session} map - Per-tab draft UUID key, atomic draft→session migration - LRU max 5 non-current entries per workspace, eviction calls pty.remove - invalidateScopeSnapshots on runtimeId change - clearWorkspaceTerminals covers mounted + cached entries - 12 new tests F2: Startup telemetry baseline + health poll fix - 7 structured telemetry events (sidecar_spawn, layer_build, health_wait, renderer.initialization, provider.state_init, pty_create, websocket_ready) - Remove fixed 100ms pre-sleep before first health poll; try immediately Startup UX overhaul: - Splash overlay stays until deepagent-code:app-ready fires (5s failsafe) - app-ready gate: server.ready() + layoutReady() + pageReady() + autoselecting resolved + firstSessionLoadDone - App renders in background behind overlay (no circular dependency) - sidebar.opened defaults to false; transition suppressed until layoutReady() - terminal panel transition suppressed until layoutReady() - SidebarPanel shows SessionSkeleton instead of blank during loading - Bottom + side terminal: gate pty.create on runtimeId set (prevents immediate 503 on cold start with persisted opened=true state) - pty.create: 10s overall deadline prevents permanent 'starting...' hang Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/app/src/context/layout.tsx | 2 +- packages/app/src/context/terminal.test.ts | 201 ++++++++- packages/app/src/context/terminal.tsx | 293 ++++++++++++- packages/app/src/i18n/en.ts | 1 + packages/app/src/i18n/zh.ts | 1 + packages/app/src/pages/layout.tsx | 54 ++- .../app/src/pages/layout/sidebar-items.tsx | 7 + .../src/pages/session/side-panel-terminal.tsx | 6 +- .../app/src/pages/session/terminal-panel.tsx | 9 +- .../__tests__/subagent-permissions.test.ts | 385 +++++++++++++++++ .../src/agent/subagent-permissions.ts | 149 ++++++- .../deepagent-code/src/provider/provider.ts | 9 +- packages/deepagent-code/src/server/server.ts | 14 + packages/deepagent-code/src/session/llm.ts | 125 +++++- .../deepagent-code/src/session/processor.ts | 215 +++++++++- packages/deepagent-code/src/session/prompt.ts | 6 + packages/deepagent-code/src/tool/task.ts | 81 +++- .../test/session/llm-f3-tool-safety.test.ts | 338 +++++++++++++++ .../session/tool-sequence-tracker.test.ts | 393 ++++++++++++++++++ packages/desktop/src/main/server.ts | 29 +- packages/desktop/src/renderer/index.tsx | 73 +++- 21 files changed, 2325 insertions(+), 66 deletions(-) create mode 100644 packages/deepagent-code/src/agent/__tests__/subagent-permissions.test.ts create mode 100644 packages/deepagent-code/test/session/llm-f3-tool-safety.test.ts create mode 100644 packages/deepagent-code/test/session/tool-sequence-tracker.test.ts diff --git a/packages/app/src/context/layout.tsx b/packages/app/src/context/layout.tsx index 9c296052..c67c2fc4 100644 --- a/packages/app/src/context/layout.tsx +++ b/packages/app/src/context/layout.tsx @@ -328,7 +328,7 @@ export const { use: useLayout, provider: LayoutProvider } = createSimpleContext( { ...target, migrate }, createStore({ sidebar: { - opened: true, + opened: false, width: DEFAULT_SIDEBAR_WIDTH, workspaces: {} as Record, workspacesDefault: false, diff --git a/packages/app/src/context/terminal.test.ts b/packages/app/src/context/terminal.test.ts index dbaabd9f..838aed90 100644 --- a/packages/app/src/context/terminal.test.ts +++ b/packages/app/src/context/terminal.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, mock, test } from "bun:test" +import { beforeAll, beforeEach, describe, expect, mock, test } from "bun:test" import { createRoot } from "solid-js" import { ServerScope } from "@/utils/server-scope" @@ -381,3 +381,202 @@ describe("runtime terminal controller", () => { } }) }) + +// ── F6 session-scoped terminal cache (LRU, draft migration, scope, clear) ───── + +describe("F6: session-scoped terminal cache helpers", () => { + // These are resolved from the same module instance already loaded by beforeAll above. + let sessionEntryKey: (typeof import("./terminal"))["TerminalTesting"]["sessionEntryKey"] + let evictLruEntries: (typeof import("./terminal"))["TerminalTesting"]["evictLruEntries"] + let invalidateScopeSnapshots: (typeof import("./terminal"))["TerminalTesting"]["invalidateScopeSnapshots"] + let sessionTerminalCache: (typeof import("./terminal"))["TerminalTesting"]["sessionTerminalCache"] + let workspaceDiscardFn: (typeof import("./terminal"))["TerminalTesting"]["workspaceDiscardFn"] + let clearSessionCache: (typeof import("./terminal"))["TerminalTesting"]["clearSessionCache"] + let clearWorkspaceTerminals: typeof import("./terminal")["clearWorkspaceTerminals"] + type Scope = typeof import("@/utils/server-scope").ServerScope.local + + beforeAll(async () => { + const mod = await import("./terminal") + sessionEntryKey = mod.TerminalTesting.sessionEntryKey + evictLruEntries = mod.TerminalTesting.evictLruEntries + invalidateScopeSnapshots = mod.TerminalTesting.invalidateScopeSnapshots + sessionTerminalCache = mod.TerminalTesting.sessionTerminalCache + workspaceDiscardFn = mod.TerminalTesting.workspaceDiscardFn + clearSessionCache = mod.TerminalTesting.clearSessionCache + clearWorkspaceTerminals = mod.clearWorkspaceTerminals + }) + + function makeSnapshot(ptyId: string) { + return { + ptys: [{ id: "local-" + ptyId, ptyId, title: "Terminal 1", titleNumber: 1 }], + root: { kind: "leaf" as const, id: "pane-1", activeId: "local-" + ptyId, ptys: ["local-" + ptyId] }, + focusedPaneId: "pane-1", + } + } + + function makeEntry(ptyId: string, lruTick = 1) { + return { bottom: makeSnapshot(ptyId + "-bot"), side: makeSnapshot(ptyId + "-side"), lruTick } + } + + const local = "local" as Scope + const remote = "ssh:host" as Scope + + describe("sessionEntryKey", () => { + test("produces different keys for different session IDs in the same workspace", () => { + const k1 = sessionEntryKey(local, "/repo", "sess-A") + const k2 = sessionEntryKey(local, "/repo", "sess-B") + expect(k1).not.toBe(k2) + }) + + test("produces different keys for the same session in different scopes", () => { + const kLocal = sessionEntryKey(local, "/repo", "sess-A") + const kRemote = sessionEntryKey(remote, "/repo", "sess-A") + expect(kLocal).not.toBe(kRemote) + }) + + test("produces different keys for the same session in different directories", () => { + const kA = sessionEntryKey(local, "/repo-a", "sess-A") + const kB = sessionEntryKey(local, "/repo-b", "sess-A") + expect(kA).not.toBe(kB) + }) + }) + + describe("evictLruEntries", () => { + const dir = "/evict-test" + + beforeEach(() => { + clearSessionCache() + }) + + test("does not evict when non-current count equals the cap (5)", () => { + for (let i = 1; i <= 5; i++) { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-" + i), makeEntry("p" + i, i)) + } + const currentKey = sessionEntryKey(local, dir, "sess-current") + sessionTerminalCache.set(currentKey, makeEntry("p-current", 100)) + + const discarded: string[] = [] + evictLruEntries(local, dir, currentKey, (id) => discarded.push(id)) + expect(discarded).toHaveLength(0) + expect(sessionTerminalCache.size).toBe(6) + }) + + test("evicts oldest non-current entries when count exceeds cap", () => { + for (let i = 1; i <= 6; i++) { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-" + i), makeEntry("p" + i, i)) + } + const currentKey = sessionEntryKey(local, dir, "sess-current") + sessionTerminalCache.set(currentKey, makeEntry("p-current", 100)) + + const discarded: string[] = [] + evictLruEntries(local, dir, currentKey, (id) => discarded.push(id)) + + // Oldest non-current (sess-1, lruTick=1): 2 PTY IDs discarded + expect(discarded).toHaveLength(2) + expect(discarded).toContain("p1-bot") + expect(discarded).toContain("p1-side") + // 6 entries remain: sess-2..6 + current + expect(sessionTerminalCache.size).toBe(6) + expect(sessionTerminalCache.has(sessionEntryKey(local, dir, "sess-1"))).toBe(false) + }) + + test("never evicts the current key even when it has the oldest lruTick", () => { + const currentKey = sessionEntryKey(local, dir, "sess-current") + sessionTerminalCache.set(currentKey, makeEntry("p-current", 0)) + for (let i = 1; i <= 6; i++) { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-" + i), makeEntry("p" + i, i + 1)) + } + + const discarded: string[] = [] + evictLruEntries(local, dir, currentKey, (id) => discarded.push(id)) + + expect(sessionTerminalCache.has(currentKey)).toBe(true) + // Oldest non-current (sess-1, lruTick=2) evicted + expect(sessionTerminalCache.has(sessionEntryKey(local, dir, "sess-1"))).toBe(false) + }) + + test("only evicts entries for the given workspace, not other workspaces", () => { + const otherDir = "/other-workspace" + const otherKey = sessionEntryKey(local, otherDir, "sess-other") + sessionTerminalCache.set(otherKey, makeEntry("other", 1)) + for (let i = 1; i <= 6; i++) { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-" + i), makeEntry("p" + i, i + 1)) + } + const currentKey = sessionEntryKey(local, dir, "sess-current") + + evictLruEntries(local, dir, currentKey, () => undefined) + expect(sessionTerminalCache.has(otherKey)).toBe(true) + }) + }) + + describe("invalidateScopeSnapshots", () => { + beforeEach(() => { + clearSessionCache() + }) + + test("deletes all entries for the given scope", () => { + sessionTerminalCache.set(sessionEntryKey(local, "/repo", "sess-A"), makeEntry("a", 1)) + sessionTerminalCache.set(sessionEntryKey(local, "/repo", "sess-B"), makeEntry("b", 2)) + sessionTerminalCache.set(sessionEntryKey(remote, "/repo", "sess-C"), makeEntry("c", 3)) + + invalidateScopeSnapshots(local) + + expect(sessionTerminalCache.has(sessionEntryKey(local, "/repo", "sess-A"))).toBe(false) + expect(sessionTerminalCache.has(sessionEntryKey(local, "/repo", "sess-B"))).toBe(false) + // Remote scope entry must survive + expect(sessionTerminalCache.has(sessionEntryKey(remote, "/repo", "sess-C"))).toBe(true) + }) + + test("is a no-op when no entries match the scope", () => { + sessionTerminalCache.set(sessionEntryKey(remote, "/repo", "sess-A"), makeEntry("a", 1)) + invalidateScopeSnapshots(local) + expect(sessionTerminalCache.size).toBe(1) + }) + }) + + describe("clearWorkspaceTerminals — cached entries", () => { + const dir = "/clear-test" + + beforeEach(() => { + clearSessionCache() + }) + + test("clears all cached entries for the matching workspace and leaves others intact", () => { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-A"), makeEntry("a", 1)) + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-B"), makeEntry("b", 2)) + // A different workspace — must not be cleared + sessionTerminalCache.set(sessionEntryKey(local, "/other", "sess-A"), makeEntry("x", 1)) + + clearWorkspaceTerminals(dir, undefined, undefined, local) + + expect(sessionTerminalCache.has(sessionEntryKey(local, dir, "sess-A"))).toBe(false) + expect(sessionTerminalCache.has(sessionEntryKey(local, dir, "sess-B"))).toBe(false) + expect(sessionTerminalCache.has(sessionEntryKey(local, "/other", "sess-A"))).toBe(true) + }) + + test("calls registered discardFn for PTY IDs of cleared cached entries", () => { + // workspaceScopeKey = ScopedKey.from(scope, dir) = scope + NUL + dir + // The separator is the null byte used by ScopedKey.from: + const wsKey = "local\0" + dir + const discarded: string[] = [] + workspaceDiscardFn.set(wsKey, (id) => discarded.push(id)) + + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-A"), makeEntry("ptya", 1)) + + clearWorkspaceTerminals(dir, undefined, undefined, local) + + expect(discarded).toContain("ptya-bot") + expect(discarded).toContain("ptya-side") + expect(sessionTerminalCache.size).toBe(0) + + workspaceDiscardFn.delete(wsKey) + }) + + test("does not throw when no discardFn is registered for the workspace", () => { + sessionTerminalCache.set(sessionEntryKey(local, dir, "sess-A"), makeEntry("a", 1)) + // No workspaceDiscardFn registered — should silently skip pty.remove calls + expect(() => clearWorkspaceTerminals(dir, undefined, undefined, local)).not.toThrow() + expect(sessionTerminalCache.has(sessionEntryKey(local, dir, "sess-A"))).toBe(false) + }) + }) +}) diff --git a/packages/app/src/context/terminal.tsx b/packages/app/src/context/terminal.tsx index eecaf9a0..b67d8b82 100644 --- a/packages/app/src/context/terminal.tsx +++ b/packages/app/src/context/terminal.tsx @@ -1,6 +1,7 @@ import { createStore } from "solid-js/store" import { createSimpleContext } from "@deepagent-code/ui/context" -import { batch, createContext, createMemo, createSignal, onCleanup, onMount, useContext, type JSX, type ParentProps } from "solid-js" +import { batch, createContext, createEffect, createMemo, createSignal, onCleanup, onMount, useContext, type JSX, type ParentProps } from "solid-js" +import { useParams } from "@solidjs/router" import { useSDK } from "./sdk" import { usePlatform, type Platform } from "./platform" import { useServer } from "./server" @@ -316,12 +317,98 @@ type TerminalSession = ReturnType const sessions = new Set<{ dir: string; scope: ServerScopeValue; value: TerminalSession }>() /** Per-directory PTY snapshot preserved across project switches so PTYs survive navigation. */ -interface TerminalPtySnapshot { +export interface TerminalPtySnapshot { ptys: Array> root: PaneNode focusedPaneId: string } -const directoryTerminalCache = new Map() + +/** Entry for a single session's bottom + side PTY snapshots, with LRU metadata. */ +interface SessionTerminalEntry { + bottom: TerminalPtySnapshot | null + side: TerminalPtySnapshot | null + /** Monotonic counter used for LRU ordering — higher = more recently accessed. */ + lruTick: number +} + +/** + * Session-keyed terminal snapshot cache. + * Key: ScopedKey.from(scope, dir, sessionKey) — scope + directory + session ID (or draft UUID). + */ +const sessionTerminalCache = new Map() + +/** + * Per-workspace discard helper registered by the mounted TerminalProvider. + * Key: ScopedKey.from(scope, dir) — just scope + directory. + * Used by clearWorkspaceTerminals to remove cached PTYs when no provider is mounted. + */ +const workspaceDiscardFn = new Map void>() + +/** Max non-current session entries kept per {scope, directory} workspace. */ +const MAX_CACHED_SESSION_ENTRIES = 5 + +let lruClock = 0 +function nextLruTick(): number { + return ++lruClock +} + +/** Composite key for the whole workspace (scope + dir, no session). */ +function workspaceScopeKey(scope: ServerScopeValue, dir: string): string { + return ScopedKey.from(scope, dir) +} + +/** Composite key for a specific session entry (scope + dir + sessionKey). */ +function sessionEntryKey(scope: ServerScopeValue, dir: string, sessionKey: string): string { + return ScopedKey.from(scope, dir, sessionKey) +} + +/** Returns all cache entries whose key belongs to the given workspace. */ +function getWorkspaceCacheEntries(scope: ServerScopeValue, dir: string): Array<[string, SessionTerminalEntry]> { + const prefix = ScopedKey.prefix(scope, dir) + const result: Array<[string, SessionTerminalEntry]> = [] + for (const [k, v] of sessionTerminalCache.entries()) { + if (k.startsWith(prefix)) result.push([k, v]) + } + return result +} + +/** + * Evict the least-recently-used non-current entries for a workspace so that at + * most MAX_CACHED_SESSION_ENTRIES non-current entries remain. + * Calls discardFn for each PTY ID of evicted entries. + */ +function evictLruEntries( + scope: ServerScopeValue, + dir: string, + currentKey: string, + discardFn: (ptyId: string) => void, +): void { + const entries = getWorkspaceCacheEntries(scope, dir) + const nonCurrent = entries.filter(([k]) => k !== currentKey) + if (nonCurrent.length <= MAX_CACHED_SESSION_ENTRIES) return + // Sort ascending: oldest (smallest lruTick) first + nonCurrent.sort((a, b) => a[1].lruTick - b[1].lruTick) + const toEvict = nonCurrent.slice(0, nonCurrent.length - MAX_CACHED_SESSION_ENTRIES) + for (const [key, entry] of toEvict) { + const ptyIds = [ + ...(entry.bottom?.ptys.map((p) => p.ptyId) ?? []), + ...(entry.side?.ptys.map((p) => p.ptyId) ?? []), + ] + for (const ptyId of ptyIds) discardFn(ptyId) + sessionTerminalCache.delete(key) + } +} + +/** + * Invalidate all cached snapshots for the given server scope. + * Called when the server restarts (runtimeId changes) — old PTY IDs are invalid. + */ +function invalidateScopeSnapshots(scope: ServerScopeValue): void { + const prefix = scope + "" + for (const key of sessionTerminalCache.keys()) { + if (key.startsWith(prefix)) sessionTerminalCache.delete(key) + } +} export function clearWorkspaceTerminals( dir: string, @@ -329,9 +416,27 @@ export function clearWorkspaceTerminals( platform?: Platform, scope: ServerScopeValue = ServerScope.local, ) { + // 1. Clear mounted sessions (calls pty.remove for their live PTYs). for (const entry of sessions) { if (entry.dir === dir && entry.scope === scope) entry.value.clear() } + + // 2. Clear cached session entries for this workspace. + // Use the registered discard fn if a provider is currently mounted; if none + // is mounted the server-side workspace teardown handles PTY cleanup. + const wsKey = workspaceScopeKey(scope, dir) + const discardFn = workspaceDiscardFn.get(wsKey) + for (const [key, entry] of getWorkspaceCacheEntries(scope, dir)) { + const ptyIds = [ + ...(entry.bottom?.ptys.map((p) => p.ptyId) ?? []), + ...(entry.side?.ptys.map((p) => p.ptyId) ?? []), + ] + if (discardFn) { + for (const ptyId of ptyIds) discardFn(ptyId) + } + sessionTerminalCache.delete(key) + } + if (platform) removeTerminalPersistence(dir, sessionIDs, platform, scope) } @@ -341,6 +446,12 @@ function createWorkspaceTerminalSession( ) { const rootId = uuid() const [store, setStore] = createStore({ all: [] as LocalPTY[] }) + // terminal.pty_create / terminal.websocket_ready telemetry: + // Records the timestamp (performance.now()) when PTY create returns, keyed by server ptyId. + // Consumed by setStatus when status transitions to "ready". + const ptyCreateTimestamps = new Map() + // Distinguishes cold (first PTY in this session) from hot (subsequent) for telemetry. + let ptyCreatedCount = 0 const [root, setRootSignal] = createSignal({ kind: "leaf", id: rootId, @@ -444,10 +555,38 @@ function createWorkspaceTerminalSession( touchPending() if (!runtime.id()) void runtime.ensure().catch(() => undefined) const epoch = generation + // terminal.pty_create — start: about to call pty.create on the server + const cold = ptyCreatedCount === 0 + const ptyCreateT0 = performance.now() try { - const result = await withServerAbortRetry(() => sdk.client.pty.create({ title: defaultTitle(number) })) + // Race pty.create against a 10-second deadline. Without this, a hung + // server connection keeps pendingCreates non-empty forever and the UI + // shows "正在启动终端..." indefinitely. + const PTY_CREATE_TIMEOUT_MS = 10_000 + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => { + const err = new Error("Terminal creation timed out") + ;(err as unknown as { cause: object }).cause = { status: 408, message: "pty.create timeout" } + reject(err) + }, PTY_CREATE_TIMEOUT_MS), + ) + const result = await Promise.race([ + withServerAbortRetry(() => sdk.client.pty.create({ title: defaultTitle(number) })), + timeoutPromise, + ]) const ptyId = result.data?.id if (!ptyId) throw new Error("Terminal creation returned no PTY id") + // terminal.pty_create — end: server returned a PTY id + const ptyCreateDurationMs = Math.round(performance.now() - ptyCreateT0) + ptyCreatedCount++ + console.info("[startup] telemetry", { + event: "terminal.pty_create", + durationMs: ptyCreateDurationMs, + ptyId, + cold, + }) + // Record the end of pty.create so setStatus can measure terminal.websocket_ready. + ptyCreateTimestamps.set(ptyId, performance.now()) if (epoch !== generation) return false if (!canPlace()) { await discard(ptyId) @@ -674,6 +813,18 @@ function createWorkspaceTerminalSession( setStatus(id: string, ptyId: string, status: TerminalStatus, error?: TerminalFailure) { const index = store.all.findIndex((pty) => pty.id === id && pty.ptyId === ptyId) if (index === -1) return + if (status === "ready") { + // terminal.websocket_ready — end: WebSocket attached and terminal is ready + const wsT0 = ptyCreateTimestamps.get(ptyId) + if (wsT0 !== undefined) { + console.info("[startup] telemetry", { + event: "terminal.websocket_ready", + durationMs: Math.round(performance.now() - wsT0), + ptyId, + }) + ptyCreateTimestamps.delete(ptyId) + } + } setStore("all", index, { status, error: status === "ready" ? undefined : error, ...(status === "ready" ? { restored: false } : {}) }) }, update(input: Partial & { id: string }) { @@ -766,11 +917,25 @@ function createWorkspaceTerminalSession( } } -/** @internal */ export const TerminalTesting = { createWorkspaceTerminalSession } +/** @internal */ export const TerminalTesting = { + createWorkspaceTerminalSession, + // Cache inspection helpers for unit tests + sessionTerminalCache, + sessionEntryKey, + evictLruEntries, + invalidateScopeSnapshots, + workspaceDiscardFn, + clearSessionCache() { + sessionTerminalCache.clear() + workspaceDiscardFn.clear() + }, +} export type TerminalHostID = "bottom" | "side" // Dual-host provider — creates independent bottom and side sessions sharing one PTY service. +// Session-scoped: the active bottom/side pair is selected by the current session key (URL +// params.id, or a stable per-tab draft UUID when no session has been created yet). const { use: useTerminalDual, provider: TerminalProvider } = createSimpleContext({ name: "TerminalDual", gate: false, @@ -778,12 +943,26 @@ const { use: useTerminalDual, provider: TerminalProvider } = createSimpleContext const sdk = useSDK() const server = useServer() const platform = usePlatform() + const params = useParams() const scope = server.scope() + + // Stable draft key for this provider-mount (per browser tab / per SessionRoute mount). + // Used as the session key when the route has no session ID yet (/session without :id). + const draftKey = uuid() + + // Reactive effective session key: real session ID when available, draft UUID otherwise. + const effectiveSessionKey = createMemo(() => params.id ?? draftKey) + let runtimeId: string | undefined let runtimeRequest: Promise | undefined let bottomSession: TerminalSession | undefined let sideSession: TerminalSession | undefined + // Shared PTY discard helper — used for LRU eviction and workspace clear. + const discardPty = (ptyId: string): void => { + void sdk.client.pty.remove({ ptyID: ptyId }, { throwOnError: false }) + } + const ensureRuntime = (): Promise => { if (runtimeRequest) return runtimeRequest runtimeRequest = sdk.client.global @@ -797,6 +976,8 @@ const { use: useTerminalDual, provider: TerminalProvider } = createSimpleContext if (runtimeId === next) return console.info("[terminal] server runtime changed", { previousRuntimeId: runtimeId, runtimeId: next }) runtimeId = next + // Invalidate all cached snapshots for this scope — old PTY IDs are dead. + invalidateScopeSnapshots(scope) bottomSession?.resetRuntime() sideSession?.resetRuntime() }) @@ -813,22 +994,88 @@ const { use: useTerminalDual, provider: TerminalProvider } = createSimpleContext bottomSession = createWorkspaceTerminalSession(sdk, runtime) sideSession = createWorkspaceTerminalSession(sdk, runtime) - // Restore previously-saved PTYs when returning to a directory. - // The cached snapshot was written by onCleanup — PTYs stayed alive on the - // server and just need to be re-registered in local reactive state. - const cached = directoryTerminalCache.get(sdk.directory) - if (cached) { - directoryTerminalCache.delete(sdk.directory) - if (cached.bottom) bottomSession.restore(cached.bottom) - if (cached.side) sideSession.restore(cached.side) + // Restore the snapshot for the initial session key, if one exists in the cache. + // This handles returning to a directory whose session terminals were saved on navigation. + const initKey = sessionEntryKey(scope, sdk.directory, effectiveSessionKey()) + const initEntry = sessionTerminalCache.get(initKey) + if (initEntry) { + sessionTerminalCache.set(initKey, { ...initEntry, lruTick: nextLruTick() }) + if (initEntry.bottom) bottomSession.restore(initEntry.bottom) + if (initEntry.side) sideSession.restore(initEntry.side) } + // Register sessions in the global set so clearWorkspaceTerminals can reach them. const bottomReg = { dir: sdk.directory, scope, value: bottomSession } const sideReg = { dir: sdk.directory, scope, value: sideSession } sessions.add(bottomReg) sessions.add(sideReg) + + // Register the PTY discard helper so clearWorkspaceTerminals can evict cached PTYs. + const wsKey = workspaceScopeKey(scope, sdk.directory) + workspaceDiscardFn.set(wsKey, discardPty) + removeTerminalPersistence(sdk.directory, undefined, platform, scope) + // Track the previous session key so we can save/restore on switches. + // Initialised to the current key so the first effect run is a no-op. + let prevSessionKey = effectiveSessionKey() + + // Reactive session-switch: when the URL session key changes (session A → B, or + // draft → real), save the current bottom/side state to the cache and restore + // (or start fresh) for the new session key. + createEffect(() => { + const nextKey = effectiveSessionKey() + const prevKey = prevSessionKey + if (nextKey === prevKey) return + + const prevEntryKey = sessionEntryKey(scope, sdk.directory, prevKey) + const nextEntryKey = sessionEntryKey(scope, sdk.directory, nextKey) + + // Snapshot the current sessions before switching away. + const bottomSnap = bottomSession!.snapshot() + const sideSnap = sideSession!.snapshot() + + // Draft → real-session migration: when the previous key was our draft UUID and + // the target slot is empty, move the terminal state to the real session key so + // the user keeps the same shell after the first message creates the session. + const isDraftMigration = prevKey === draftKey && !sessionTerminalCache.has(nextEntryKey) + if (isDraftMigration) { + sessionTerminalCache.set(nextEntryKey, { + bottom: bottomSnap, + side: sideSnap, + lruTick: nextLruTick(), + }) + sessionTerminalCache.delete(prevEntryKey) + } else { + // Normal session switch: persist current state under the previous key. + if (bottomSnap || sideSnap) { + sessionTerminalCache.set(prevEntryKey, { + bottom: bottomSnap, + side: sideSnap, + lruTick: nextLruTick(), + }) + } + } + + prevSessionKey = nextKey + + // Reset live sessions to empty state before restoring (or starting fresh). + bottomSession!.resetRuntime() + sideSession!.resetRuntime() + + // Restore from cache if an entry exists for the target session. + const nextEntry = sessionTerminalCache.get(nextEntryKey) + if (nextEntry) { + // Touch LRU. + sessionTerminalCache.set(nextEntryKey, { ...nextEntry, lruTick: nextLruTick() }) + if (nextEntry.bottom) bottomSession!.restore(nextEntry.bottom) + if (nextEntry.side) sideSession!.restore(nextEntry.side) + } + + // Evict least-recently-used entries beyond the per-workspace cap. + evictLruEntries(scope, sdk.directory, nextEntryKey, discardPty) + }) + onMount(() => { void ensureRuntime() const timer = setInterval(() => { @@ -836,15 +1083,27 @@ const { use: useTerminalDual, provider: TerminalProvider } = createSimpleContext }, RUNTIME_POLL_MS) onCleanup(() => clearInterval(timer)) }) + onCleanup(() => { - // Save PTY state so terminals survive project navigation. Do NOT call - // clear() here — that would DELETE PTYs on the server. The PTYs keep - // running and are reconnected when the user returns to this directory. + // Save PTY state for the current session so terminals survive navigation. + // Do NOT call clear() here — that would DELETE PTYs on the server. PTYs keep + // running and are reconnected when the user returns to this session. + const currentKey = sessionEntryKey(scope, sdk.directory, effectiveSessionKey()) const bottomSnap = bottomSession?.snapshot() ?? null const sideSnap = sideSession?.snapshot() ?? null - directoryTerminalCache.set(sdk.directory, { bottom: bottomSnap, side: sideSnap }) + if (bottomSnap || sideSnap) { + sessionTerminalCache.set(currentKey, { + bottom: bottomSnap, + side: sideSnap, + lruTick: nextLruTick(), + }) + } else { + // Remove a stale empty entry if nothing is alive. + sessionTerminalCache.delete(currentKey) + } sessions.delete(bottomReg) sessions.delete(sideReg) + workspaceDiscardFn.delete(wsKey) }) return { bottom: bottomSession, side: sideSession } diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index fafc77d1..50b17750 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -1023,6 +1023,7 @@ export const dict = { "common.rename": "Rename", "common.reset": "Reset", "common.archive": "Archive", + "common.copyId": "Copy ID", "common.delete": "Delete", "common.close": "Close", "common.edit": "Edit", diff --git a/packages/app/src/i18n/zh.ts b/packages/app/src/i18n/zh.ts index caac0927..aa481a7f 100644 --- a/packages/app/src/i18n/zh.ts +++ b/packages/app/src/i18n/zh.ts @@ -896,6 +896,7 @@ export const dict = { "common.rename": "重命名", "common.reset": "重置", "common.archive": "归档", + "common.copyId": "复制 ID", "common.delete": "删除", "common.close": "关闭", "common.edit": "编辑", diff --git a/packages/app/src/pages/layout.tsx b/packages/app/src/pages/layout.tsx index 9cd70c53..a4c2ed93 100644 --- a/packages/app/src/pages/layout.tsx +++ b/packages/app/src/pages/layout.tsx @@ -3,6 +3,7 @@ import { createEffect, createMemo, createResource, + createSignal, For, on, onCleanup, @@ -94,6 +95,7 @@ import { } from "./layout/sidebar-workspace" import { ProjectDragOverlay, SortableProject, type ProjectSidebarContext } from "./layout/sidebar-project" import { SidebarContent } from "./layout/sidebar-shell" +import { SessionSkeleton } from "./layout/sidebar-items" export default function Layout(props: ParentProps) { const serverSDK = useServerSDK() @@ -1923,6 +1925,10 @@ export default function Layout(props: ParentProps) { const loadedSessionDirs = new Set() + // F2-splash: signal that the first session-list fetch has completed so the + // renderer knows the sidebar is populated and the splash can be dismissed. + const [firstSessionLoadDone, setFirstSessionLoadDone] = createSignal(false) + createEffect( on( visibleSessionDirs, @@ -1933,20 +1939,48 @@ export default function Layout(props: ParentProps) { } const next = new Set(dirs) + const loads: Promise[] = [] for (const directory of next) { if (loadedSessionDirs.has(directory)) continue - void serverSync.project.loadSessions(directory) + loads.push(serverSync.project.loadSessions(directory)) } loadedSessionDirs.clear() for (const directory of next) { loadedSessionDirs.add(directory) } + + if (loads.length > 0) { + void Promise.all(loads).then(() => setFirstSessionLoadDone(true)) + } else { + // All visible directories were already loaded on a prior run. + setFirstSessionLoadDone(true) + } }, { defer: true }, ), ) + // Fire `deepagent-code:app-ready` exactly once when every gate is satisfied: + // 1. server persist loaded (projects list populated — was missing, caused early fire) + // 2. layout persist loaded (project list available) + // 3. page persist loaded (last-session info available) + // 4. autoselecting resolved (router has navigated to the last project) + // 5. first sessions fetch done — OR no projects exist (nothing to wait for) + // The renderer's splash stays up until this event fires (5 s failsafe there). + let appReadyFired = false + createEffect(() => { + if (appReadyFired) return + if (!server.ready()) return // server persist must be loaded (populates projects list) + if (!layoutReady()) return + if (!pageReady()) return + if (autoselecting.loading) return + const projects = layout.projects.list() + if (projects.length > 0 && !firstSessionLoadDone()) return + appReadyFired = true + window.dispatchEvent(new Event("deepagent-code:app-ready")) + }) + function handleDragStart(event: unknown) { const id = getDraggableId(event) if (!id) return @@ -2191,7 +2225,18 @@ export default function Layout(props: ParentProps) { + +
+ +
+
+ } + >
@@ -2571,8 +2616,11 @@ export default function Layout(props: ParentProps) { "absolute inset-0": true, "xl:inset-y-0 xl:right-0 xl:left-[var(--main-left)]": true, "z-20": true, + // Suppress the left-slide transition until the layout persist store + // has loaded so the initial open→close (or closed→open) snap from + // async storage doesn't animate visibly. "transition-[left] duration-200 ease-[cubic-bezier(0.22,1,0.36,1)] will-change-[left] motion-reduce:transition-none": - !state.sizing, + !state.sizing && layoutReady(), }} style={{ "--main-left": layout.sidebar.opened() ? `${side()}px` : "4rem", diff --git a/packages/app/src/pages/layout/sidebar-items.tsx b/packages/app/src/pages/layout/sidebar-items.tsx index 827c8c13..cb4a19c1 100644 --- a/packages/app/src/pages/layout/sidebar-items.tsx +++ b/packages/app/src/pages/layout/sidebar-items.tsx @@ -413,6 +413,13 @@ export const SessionItem = (props: SessionItemProps): JSX.Element => { void props.archiveSession(props.session)}> {language.t("common.archive")} + { + void navigator.clipboard?.writeText(props.session.id) + }} + > + {language.t("common.copyId")} + {language.t("common.delete")} diff --git a/packages/app/src/pages/session/side-panel-terminal.tsx b/packages/app/src/pages/session/side-panel-terminal.tsx index 2f3bb23d..a3ec6434 100644 --- a/packages/app/src/pages/session/side-panel-terminal.tsx +++ b/packages/app/src/pages/session/side-panel-terminal.tsx @@ -18,8 +18,12 @@ function SidePanelTerminalContent(props: { onClose: () => void }) { const terminal = useTerminal() // Lifecycle: auto-create first PTY when side panel opens; close panel when empty. + // Guard `active` on runtimeId being set — this confirms the server instance is + // ready to accept pty.create calls. Without this guard the effect fires on the + // very first reactive pass while the server is still booting, producing an + // immediate non-retryable 503 failure and a "operation failed" error. useTerminalLifecycle({ - active: () => true, + active: () => terminal.runtimeId() !== undefined || terminal.all().length > 0, close: props.onClose, rootEl: () => document.querySelector('[data-terminal-host="side"]') ?? undefined, }) diff --git a/packages/app/src/pages/session/terminal-panel.tsx b/packages/app/src/pages/session/terminal-panel.tsx index fe9fa8cc..8bc19028 100644 --- a/packages/app/src/pages/session/terminal-panel.tsx +++ b/packages/app/src/pages/session/terminal-panel.tsx @@ -52,8 +52,10 @@ function TerminalPanelContent(props: Props) { ) // Lifecycle owner for the bottom host — auto-creates, auto-focuses, closes panel when empty. + // Gate `active` on runtimeId being set to prevent pty.create firing before the server + // instance is ready (cold-start with persisted opened=true produces immediate 503 failures). useTerminalLifecycle({ - active: terminalVisible, + active: () => terminalVisible() && (terminal.runtimeId() !== undefined || terminal.all().length > 0), close: () => panel().toggle("terminal"), rootEl: () => document.querySelector('[data-terminal-host="bottom"]') ?? root, }) @@ -91,8 +93,11 @@ function TerminalPanelContent(props: Props) { inert={!visible()} class="relative w-full shrink-0 overflow-hidden bg-background-stronger" classList={{ + // Suppress height animation until layout persisted state has loaded. + // Without this guard the panel snaps from 0→pane() height on cold start + // when the persisted `opened:true` arrives after the in-memory default. "transition-[height] duration-200 ease-[cubic-bezier(0.22,1,0.36,1)] will-change-[height] motion-reduce:transition-none": - !size.active(), + !size.active() && layout.ready(), }} style={{ height: visible() ? `${pane()}px` : "0px" }} > diff --git a/packages/deepagent-code/src/agent/__tests__/subagent-permissions.test.ts b/packages/deepagent-code/src/agent/__tests__/subagent-permissions.test.ts new file mode 100644 index 00000000..17ccca17 --- /dev/null +++ b/packages/deepagent-code/src/agent/__tests__/subagent-permissions.test.ts @@ -0,0 +1,385 @@ +/** + * F5: subagent depth limit + unified admission tests + * + * Acceptance criteria from 4.0.4_r4.md §F5: + * 1. Default researcher, reviewer, explore still cannot delegate task. + * 2. Explicitly allowed agent can delegate within depth limit. + * 3. Unauthorised target is still denied. + * 4. depth=3 cannot delegate regardless of config. + * 5. admitChildOrFail covers normal, background, takeover and resume paths consistently. + * 6. Plan Mode parent deny, Goal Loop plan capability and worktree rules do not regress. + */ + +import { describe, it, expect } from "bun:test" +import { Effect } from "effect" +import { + MAX_SUBAGENT_DEPTH, + SUBAGENT_DEPTH_META_KEY, + canDelegateTask, + admitChildOrFail, + resolveSessionDepth, + deriveSubagentSessionPermission, + subagentIsWriteType, + PLAN_WRITE_OWN_GOAL, +} from "../subagent-permissions" +import type { Agent } from "../agent" +import { PermissionV1 } from "@deepagent-code/core/v1/permission" +import { SessionID } from "../../session/schema" +import type { Session } from "../../session/session" + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeAgent(name: string, rules: PermissionV1.Rule[]): Agent.Info { + return { + name, + mode: "all", + permission: rules, + options: {}, + native: false, + } +} + +function makeRule( + permission: string, + action: "allow" | "deny" | "ask", + pattern = "*", +): PermissionV1.Rule { + return { permission, action, pattern } +} + +// Default "star deny" that researcher/reviewer/explore/goal-worker have. +const STAR_DENY: PermissionV1.Rule[] = [makeRule("*", "deny")] +// Explicit task allow rule (custom agents). +const TASK_ALLOW: PermissionV1.Rule[] = [makeRule("task", "allow")] + +// --------------------------------------------------------------------------- +// A. MAX_SUBAGENT_DEPTH constant +// --------------------------------------------------------------------------- + +describe("MAX_SUBAGENT_DEPTH", () => { + it("is 3", () => { + expect(MAX_SUBAGENT_DEPTH).toBe(3) + }) +}) + +// --------------------------------------------------------------------------- +// B. canDelegateTask — permission-only check (no depth) +// --------------------------------------------------------------------------- + +describe("canDelegateTask", () => { + it("returns false when caller agent has '*: deny' (researcher/reviewer/explore default)", () => { + expect(canDelegateTask(STAR_DENY, [], "researcher")).toBe(false) + expect(canDelegateTask(STAR_DENY, [], "explore")).toBe(false) + expect(canDelegateTask(STAR_DENY, [], "reviewer")).toBe(false) + expect(canDelegateTask(STAR_DENY, [], "goal-worker")).toBe(false) + }) + + it("returns false when caller has no task rule at all (unmatched → ask, not allow)", () => { + expect(canDelegateTask([], [], "researcher")).toBe(false) + }) + + it("returns true when caller agent has explicit task allow", () => { + expect(canDelegateTask(TASK_ALLOW, [], "researcher")).toBe(true) + }) + + it("session permission overrides agent permission (last-match-wins)", () => { + // Agent says deny, session adds allow → allow wins. + const sessionAllow: PermissionV1.Rule[] = [makeRule("task", "allow")] + expect(canDelegateTask(STAR_DENY, sessionAllow, "researcher")).toBe(true) + + // Agent says allow, session adds deny → deny wins. + const sessionDeny: PermissionV1.Rule[] = [makeRule("task", "deny")] + expect(canDelegateTask(TASK_ALLOW, sessionDeny, "researcher")).toBe(false) + }) + + it("pattern matching: allow for specific type only", () => { + const allowExplore: PermissionV1.Rule[] = [makeRule("task", "allow", "explore")] + expect(canDelegateTask(allowExplore, [], "explore")).toBe(true) + expect(canDelegateTask(allowExplore, [], "researcher")).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// C. admitChildOrFail — depth + permission gate +// --------------------------------------------------------------------------- + +describe("admitChildOrFail", () => { + it("AC4: depth >= MAX_SUBAGENT_DEPTH → error regardless of permission", () => { + const result = admitChildOrFail({ + callerDepth: MAX_SUBAGENT_DEPTH, + callerAgentPermission: TASK_ALLOW, + callerSessionPermission: [], + targetAgentType: "researcher", + }) + expect("error" in result).toBe(true) + expect((result as { error: string }).error).toMatch(/depth limit/) + }) + + it("depth 3 with explicit allow still blocked (hard ceiling)", () => { + const result = admitChildOrFail({ + callerDepth: 3, + callerAgentPermission: TASK_ALLOW, + callerSessionPermission: [], + targetAgentType: "general", + }) + expect("error" in result).toBe(true) + }) + + it("depth 0, no permission → error for unauthorised target", () => { + const result = admitChildOrFail({ + callerDepth: 0, + callerAgentPermission: STAR_DENY, + callerSessionPermission: [], + targetAgentType: "researcher", + }) + expect("error" in result).toBe(true) + expect((result as { error: string }).error).toMatch(/permission/) + }) + + it("AC2: depth 0, explicit allow → admitted with childDepth=1", () => { + const result = admitChildOrFail({ + callerDepth: 0, + callerAgentPermission: TASK_ALLOW, + callerSessionPermission: [], + targetAgentType: "researcher", + }) + expect("childDepth" in result).toBe(true) + expect((result as { childDepth: number }).childDepth).toBe(1) + }) + + it("depth 2, explicit allow → admitted with childDepth=3 (equals MAX, still ok)", () => { + const result = admitChildOrFail({ + callerDepth: 2, + callerAgentPermission: TASK_ALLOW, + callerSessionPermission: [], + targetAgentType: "explore", + }) + expect("childDepth" in result).toBe(true) + expect((result as { childDepth: number }).childDepth).toBe(3) + }) + + it("depth 4 (corrupted/injected) → error even with allow", () => { + const result = admitChildOrFail({ + callerDepth: 4, + callerAgentPermission: TASK_ALLOW, + callerSessionPermission: [], + targetAgentType: "researcher", + }) + expect("error" in result).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// D. resolveSessionDepth — metadata path, chain path, conflict resolution +// --------------------------------------------------------------------------- + +describe("resolveSessionDepth", () => { + // SessionID requires "ses" prefix — use ses_ prefixed keys throughout. + const S = { + root: "ses_root0000000000", + child: "ses_child000000000", + grand: "ses_grand000000000", + a: "ses_aaaaaaaaaaaaaaaa", + b: "ses_bbbbbbbbbbbbbbbb", + none: "ses_nonexistent0000", + } as const + + function makeSessionsService( + sessions: Record }>, + ): Session.Interface { + return { + get: (id: SessionID) => + Effect.gen(function* () { + const s = sessions[id] + if (!s) return yield* Effect.fail({ message: `not found: ${id}` } as never) + return { + id, + parentID: s.parentID as SessionID | undefined, + metadata: s.metadata, + } as Session.Info + }), + } as unknown as Session.Interface + } + + it("root session (no parent, no metadata) → depth 0", async () => { + const svc = makeSessionsService({ [S.root]: {} }) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.root))) + expect(depth).toBe(0) + }) + + it("reads depth from metadata when present", async () => { + const svc = makeSessionsService({ + [S.root]: {}, + [S.child]: { + parentID: S.root, + metadata: { deepagent: { [SUBAGENT_DEPTH_META_KEY]: 1 } }, + }, + }) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.child))) + expect(depth).toBe(1) + }) + + it("walks parentID chain for historical sessions without metadata", async () => { + const svc = makeSessionsService({ + [S.root]: {}, + [S.child]: { parentID: S.root }, + [S.grand]: { parentID: S.child }, + }) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.grand))) + expect(depth).toBe(2) + }) + + it("prefers stricter value when metadata conflicts with chain", async () => { + // metadata says depth=1, chain says depth=2 → use 2 (stricter) + const svc = makeSessionsService({ + [S.root]: {}, + [S.a]: { parentID: S.root }, + [S.b]: { parentID: S.a, metadata: { deepagent: { [SUBAGENT_DEPTH_META_KEY]: 1 } } }, + }) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.b))) + expect(depth).toBe(2) + }) + + it("fails closed at MAX_SUBAGENT_DEPTH on cycle detection", async () => { + const svc = makeSessionsService({ + [S.a]: { parentID: S.b }, + [S.b]: { parentID: S.a }, // cycle: a→b→a + }) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.a))) + expect(depth).toBe(MAX_SUBAGENT_DEPTH) + }) + + it("fails closed at MAX_SUBAGENT_DEPTH on unknown session", async () => { + const svc = makeSessionsService({}) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(S.none))) + expect(depth).toBe(MAX_SUBAGENT_DEPTH) + }) + + it("fails closed on super-long chain (over limit)", async () => { + // Build a chain of length MAX_SUBAGENT_DEPTH + 10, using ses_ prefixed ids. + const sessions: Record = {} + const chainLen = MAX_SUBAGENT_DEPTH + 10 + const ids = Array.from({ length: chainLen + 1 }, (_, i) => `ses_chain${String(i).padStart(10, "0")}`) + for (let i = 0; i <= chainLen; i++) { + sessions[ids[i]!] = i > 0 ? { parentID: ids[i - 1] } : {} + } + const svc = makeSessionsService(sessions) + const depth = await Effect.runPromise(resolveSessionDepth(svc, SessionID.make(ids[chainLen]!))) + expect(depth).toBe(MAX_SUBAGENT_DEPTH) + }) +}) + +// --------------------------------------------------------------------------- +// E. deriveSubagentSessionPermission — regression: canTask/canTodo use evaluate +// --------------------------------------------------------------------------- + +describe("deriveSubagentSessionPermission — canTask / canTodo", () => { + it("AC1: researcher (star deny) gets task:deny in derived session permission", () => { + const researcher = makeAgent("researcher", STAR_DENY) + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: researcher, + }) + const taskRule = derived.find((r) => r.permission === "task") + expect(taskRule?.action).toBe("deny") + }) + + it("AC1: reviewer (star deny) gets task:deny in derived session permission", () => { + const reviewer = makeAgent("reviewer", STAR_DENY) + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: reviewer, + }) + const taskRule = derived.find((r) => r.permission === "task") + expect(taskRule?.action).toBe("deny") + }) + + it("agent with explicit task:allow does NOT get an extra task:deny appended", () => { + const custom = makeAgent("custom", [makeRule("task", "allow")]) + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: custom, + }) + // Must NOT append task:deny when agent already has an explicit allow. + const taskDenies = derived.filter((r) => r.permission === "task" && r.action === "deny") + expect(taskDenies).toHaveLength(0) + }) + + it("agent with no task rule (ask by default) gets task:deny appended", () => { + const noRules = makeAgent("general", []) + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: noRules, + }) + const taskDenies = derived.filter((r) => r.permission === "task" && r.action === "deny") + expect(taskDenies.length).toBeGreaterThan(0) + }) + + it("AC6: Goal Loop plan capability grant is preserved", () => { + const worker = makeAgent("goal-worker", [ + ...STAR_DENY, + // capability declared in registry + ]) + worker.capabilities = [PLAN_WRITE_OWN_GOAL] + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: worker, + allowPlanWriteCapability: true, + }) + const planAllow = derived.find((r) => r.permission === "plan" && r.action === "allow") + expect(planAllow).toBeTruthy() + }) + + it("AC6: Goal Loop plan capability NOT granted when allowPlanWriteCapability is false", () => { + const worker = makeAgent("goal-worker", STAR_DENY) + worker.capabilities = [PLAN_WRITE_OWN_GOAL] + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: undefined, + subagent: worker, + allowPlanWriteCapability: false, + }) + const planAllow = derived.find((r) => r.permission === "plan" && r.action === "allow") + expect(planAllow).toBeUndefined() + }) + + it("AC6: Plan Mode parent deny propagates to child session", () => { + const editDenyAgent = makeAgent("parent-plan-mode", [makeRule("edit", "deny")]) + const child = makeAgent("child", []) + const derived = deriveSubagentSessionPermission({ + parentSessionPermission: [], + parentAgent: editDenyAgent, + subagent: child, + }) + const editDeny = derived.find((r) => r.permission === "edit" && r.action === "deny") + expect(editDeny).toBeTruthy() + }) +}) + +// --------------------------------------------------------------------------- +// F. subagentIsWriteType regression +// --------------------------------------------------------------------------- + +describe("subagentIsWriteType", () => { + it("agent with bash:allow is write-type", () => { + const a = makeAgent("a", [makeRule("bash", "allow")]) + expect(subagentIsWriteType(a)).toBe(true) + }) + + it("agent with all edit-class denied is read-only", () => { + const a = makeAgent("a", STAR_DENY) + expect(subagentIsWriteType(a)).toBe(false) + }) + + it("agent with no rules (ask default) is write-type (ask ≠ deny)", () => { + const a = makeAgent("a", []) + expect(subagentIsWriteType(a)).toBe(true) + }) +}) diff --git a/packages/deepagent-code/src/agent/subagent-permissions.ts b/packages/deepagent-code/src/agent/subagent-permissions.ts index 0d6488b7..d24e330d 100644 --- a/packages/deepagent-code/src/agent/subagent-permissions.ts +++ b/packages/deepagent-code/src/agent/subagent-permissions.ts @@ -2,6 +2,10 @@ import { PermissionV1 } from "@deepagent-code/core/v1/permission" import { evaluate as evaluatePermission } from "../permission" import type { Permission } from "../permission" import type { Agent } from "./agent" +import { Effect } from "effect" +import { Log } from "@deepagent-code/core/util/log" +import type { Session } from "../session/session" +import { SessionID } from "../session/schema" // I33-3: the edit-class tool permissions that mark a subagent as WRITE-type. A subagent that can run // ANY of these (effective action !== "deny") mutates the working tree and therefore defaults to git @@ -10,6 +14,17 @@ import type { Agent } from "./agent" // mutating-tool set the plan gate uses (session/tools.ts: write/edit/patch/shell). const EDIT_CLASS_PERMISSIONS = ["edit", "write", "patch", "bash"] as const +// F5: hard process-level subagent depth limit. Root session = depth 0; its first child = depth 1. +// A session at MAX_SUBAGENT_DEPTH cannot spawn any further children — all task calls from that depth +// are denied regardless of config. This constant MUST NOT be raised via runtime config. +export const MAX_SUBAGENT_DEPTH = 3 + +// F5: metadata key path for the normalized depth value written at session-create time. +// Access via: session.metadata?.deepagent?.subagentDepth +export const SUBAGENT_DEPTH_META_KEY = "subagentDepth" as const + +const log = Log.create({ service: "subagent-permissions" }) + /** * I33-3: is this subagent WRITE-type (defaults to worktree isolation) or READ-ONLY (defaults to * shared parent dir)? Evaluates the subagent's own permission ruleset for each edit-class tool via the @@ -114,8 +129,13 @@ export function deriveSubagentSessionPermission(input: { */ allowPlanWriteCapability?: boolean }): PermissionV1.Ruleset { - const canTask = input.subagent.permission.some((rule) => rule.permission === "task") - const canTodo = input.subagent.permission.some((rule) => rule.permission === "todowrite") + // F5: use evaluate (not presence check) so "ask" does NOT count as delegation capability. + // A subagent that has only "ask"-level task permission cannot further delegate; only an explicit + // "allow" rule in the subagent's own permission set permits it. This preserves the default deny + // for researcher/reviewer/explore/goal-worker (all have `"*": "deny"` which covers "task") and + // keeps custom agents with explicit `task: allow` rules working. + const canTask = evaluatePermission("task", "*", input.subagent.permission).action === "allow" + const canTodo = evaluatePermission("todowrite", "*", input.subagent.permission).action === "allow" // V3.9 §E: controlled relaxation — a Goal Loop worker gets plan-write ONLY when it BOTH declares the // capability in its Registry entry AND the call site opted in (the flag-gated goal-loop wiring). Both // conditions are required, so ordinary subagents — and any caller with the flag off — never get it. @@ -138,3 +158,128 @@ export function deriveSubagentSessionPermission(input: { ...(canTask ? [] : [{ permission: "task" as const, pattern: "*" as const, action: "deny" as const }]), ] } + +/** + * F5: resolve the effective subagent nesting depth for a session. + * + * Resolution order (fail-closed throughout): + * 1. Read `metadata.deepagent.subagentDepth` — present on all sessions created after F5 ships. + * 2. Fall back to walking the `parentID` chain for historical sessions that pre-date the field. + * 3. When both are available, use the STRICTER (larger) value and log a diagnostic. + * 4. Cycle detection, missing-parent termination, and over-long chain guard all fail closed at + * MAX_SUBAGENT_DEPTH so a corrupted chain cannot lower the effective depth below the real one. + * + * The function never fails — any error path returns MAX_SUBAGENT_DEPTH (fail closed). + */ +export function resolveSessionDepth( + sessions: Session.Interface, + sessionID: SessionID, +): Effect.Effect { + return Effect.gen(function* () { + const session = yield* sessions.get(sessionID).pipe(Effect.orElseSucceed(() => undefined)) + if (!session) { + log.warn("resolveSessionDepth: session not found, failing closed", { sessionID }) + return MAX_SUBAGENT_DEPTH + } + + // 1. Try normalised metadata depth written at create time. + const deepagentMeta = session.metadata?.["deepagent"] as Record | undefined + const rawMeta = deepagentMeta?.[SUBAGENT_DEPTH_META_KEY] + const metaDepth = + typeof rawMeta === "number" && Number.isFinite(rawMeta) && rawMeta >= 0 + ? Math.floor(rawMeta) + : undefined + + // 2. Walk parentID chain for historical sessions / cross-validation. + const visited = new Set([sessionID]) + let cursor: string | undefined = session.parentID + let chainDepth = 0 + const chainLimit = MAX_SUBAGENT_DEPTH + 4 // generous bound against corrupted chains + + for (let i = 0; i < chainLimit && cursor !== undefined; i++) { + if (visited.has(cursor)) { + log.warn("resolveSessionDepth: cycle in parentID chain, failing closed", { sessionID, cycleAt: cursor }) + return MAX_SUBAGENT_DEPTH + } + visited.add(cursor) + const parentSID = SessionID.make(cursor) + const parent = yield* sessions.get(parentSID).pipe(Effect.orElseSucceed(() => undefined)) + if (!parent) break // chain terminates at an unknown root — depth is the hops counted so far + chainDepth++ + cursor = parent.parentID + } + // If the loop exhausted chainLimit the chain is suspiciously long — fail closed. + if (cursor !== undefined) { + log.warn("resolveSessionDepth: parentID chain exceeds limit, failing closed", { sessionID }) + return MAX_SUBAGENT_DEPTH + } + + // 3. Reconcile metadata vs chain. + if (metaDepth === undefined) return chainDepth + if (metaDepth !== chainDepth) { + log.warn("resolveSessionDepth: metadata/chain depth conflict — using stricter value", { + sessionID, + metaDepth, + chainDepth, + }) + return Math.max(metaDepth, chainDepth) + } + return metaDepth + }) +} + +/** + * F5: check whether the calling session's effective permission allows it to delegate a task + * to the given target agent type. + * + * Uses the existing Permission.evaluate function (last-match-wins) over the MERGED caller + * ruleset (agent permission first, session permission second so session overrides take effect). + * "ask" is NOT a delegation capability — only an explicit "allow" rule grants it. + * + * Pure + synchronous; no I/O. Call resolveSessionDepth separately and combine with this. + */ +export function canDelegateTask( + callerAgentPermission: PermissionV1.Ruleset, + callerSessionPermission: PermissionV1.Ruleset, + targetAgentType: string, +): boolean { + // Session permission is passed second so it is the "last" ruleset and wins over agent permission. + const result = evaluatePermission("task", targetAgentType, callerAgentPermission, callerSessionPermission) + return result.action === "allow" +} + +/** + * F5: unified child-admission check for ALL task-creation paths (normal, takeover, background). + * + * Returns the depth the NEW child session should carry, or fails with a descriptive error string. + * Does NOT create the session or write metadata — the caller must do both. + * + * Checks (in order): + * 1. Hard depth ceiling: parent depth >= MAX_SUBAGENT_DEPTH → always deny. + * 2. Caller delegation permission: effective permission for "task" + targetAgentType must be "allow". + */ +export function admitChildOrFail(input: { + callerDepth: number + callerAgentPermission: PermissionV1.Ruleset + callerSessionPermission: PermissionV1.Ruleset + targetAgentType: string +}): { childDepth: number } | { error: string } { + const childDepth = input.callerDepth + 1 + if (input.callerDepth >= MAX_SUBAGENT_DEPTH) { + return { + error: + `Subagent depth limit reached (max ${MAX_SUBAGENT_DEPTH}). ` + + `This session is at depth ${input.callerDepth} and cannot spawn further subagents. ` + + `Restructure the task to reduce nesting.`, + } + } + if (!canDelegateTask(input.callerAgentPermission, input.callerSessionPermission, input.targetAgentType)) { + return { + error: + `This agent does not have permission to delegate to "${input.targetAgentType}". ` + + `The effective "task" permission evaluates to deny or ask for this agent type. ` + + `Grant explicit task permission in the agent configuration to enable delegation.`, + } + } + return { childDepth } +} diff --git a/packages/deepagent-code/src/provider/provider.ts b/packages/deepagent-code/src/provider/provider.ts index a92c94a9..4049c528 100644 --- a/packages/deepagent-code/src/provider/provider.ts +++ b/packages/deepagent-code/src/provider/provider.ts @@ -48,6 +48,10 @@ import { } from "./compatibility" const log = Log.create({ service: "provider" }) + +// Tracks whether InstanceState.make has been called at least once in this process. +// First call is cold start (no cached state); subsequent calls (per-directory re-init) are hot. +let providerStateInitialized = false const OPENAI_HEADER_TIMEOUT_DEFAULT = 10_000 const THIRD_PARTY_PROVIDER_CONFLICT_MESSAGE = "Provider id conflicts with an official provider. Rename this third-party provider in your config." @@ -1300,7 +1304,10 @@ export const layer = Layer.effect( const state = yield* InstanceState.make(() => Effect.gen(function* () { - using _ = log.time("state") + // provider.state_init — structured telemetry: first call per process is cold start. + const isFirstProviderInit = !providerStateInitialized + providerStateInitialized = true + using _ = log.time("provider.state_init", { cold: isFirstProviderInit }) const bridge = yield* EffectBridge.make() const cfg = yield* config.get() // Official providers ignore config.provider. entirely; their transport tuning diff --git a/packages/deepagent-code/src/server/server.ts b/packages/deepagent-code/src/server/server.ts index c1b5ac70..0fe481d4 100644 --- a/packages/deepagent-code/src/server/server.ts +++ b/packages/deepagent-code/src/server/server.ts @@ -19,6 +19,10 @@ globalThis.AI_SDK_LOG_WARNINGS = false const log = Log.create({ service: "server" }) +// Tracks whether this process has already completed a successful Server.listen. +// First call is cold start; subsequent calls are hot restarts. +let serverHasListened = false + export type Listener = { hostname: string port: number @@ -84,7 +88,17 @@ export async function listen(opts: ListenOptions): Promise { const listenEffect: (opts: ListenOptions) => Effect.Effect = Effect.fn("Server.listen")( function* (opts: ListenOptions) { + const cold = !serverHasListened + const layerBuildT0 = yield* Effect.sync(() => Date.now()) const state = yield* startWithPortFallback(opts) + yield* Effect.sync(() => { + log.info("startup", { + event: "server.layer_build", + durationMs: Date.now() - layerBuildT0, + cold, + }) + serverHasListened = true + }) const address = yield* tcpAddress(state) const listenerUrl = makeURL(opts.hostname, address.port) url = listenerUrl diff --git a/packages/deepagent-code/src/session/llm.ts b/packages/deepagent-code/src/session/llm.ts index 85fcfbc2..87d9f0ac 100644 --- a/packages/deepagent-code/src/session/llm.ts +++ b/packages/deepagent-code/src/session/llm.ts @@ -6,7 +6,7 @@ import { Log } from "@deepagent-code/core/util/log" import { Global } from "@deepagent-code/core/global" import { Context, Effect, Layer } from "effect" import * as Stream from "effect/Stream" -import { streamText, wrapLanguageModel, type ModelMessage, type Tool, APICallError } from "ai" +import { streamText, wrapLanguageModel, type ModelMessage, type Tool, APICallError, NoSuchToolError, InvalidToolInputError } from "ai" import { type LLMEvent } from "@deepagent-code/llm" import { AgentGateway } from "@deepagent-code/core/agent-gateway" import { LLMClient, RequestExecutor, WebSocketExecutor } from "@deepagent-code/llm/route" @@ -41,6 +41,23 @@ const deepagentModelAuthProviderID = (model: Provider.Model) => { return typeof value === "string" && value.length > 0 ? value : undefined } +// Detect whether the resolved request options enable extended thinking / reasoning. +// Providers reject `tool_choice: required/object` while thinking is active (e.g. +// "The tool_choice parameter does not support being set to required or object in +// thinking mode"), which broke structured-output subagent calls. When thinking is +// on, the caller downgrades toolChoice to auto and relies on the schema-aware +// system prompt (buildStructuredOutputSystemPrompt) to elicit the structured tool +// call instead of forcing it. +const thinkingActive = (options: Record | undefined): boolean => { + if (!options || typeof options !== "object") return false + const effort = options.reasoningEffort ?? options.reasoning?.effort + if (typeof effort === "string" && effort !== "none") return true + const thinking = options.thinking + if (thinking && typeof thinking === "object" && thinking.type !== "disabled") return true + if (options.thinkingConfig && typeof options.thinkingConfig === "object") return true + return false +} + export type StreamInput = { user: SessionV1.User sessionID: string @@ -152,6 +169,11 @@ const live: Layer.Layer< }, }) + // Structured-output callers force toolChoice:"required", which providers reject + // while thinking is active. Downgrade to auto in that case so the call still + // succeeds; the schema-aware system prompt keeps the model on the structured path. + const effectiveToolChoice = thinkingActive(prepared.params.options) ? undefined : input.toolChoice + // Wire up toolExecutor for DWS workflow models so that tool calls // from the workflow service are executed via deepagent-code's tool system // and results sent back over the WebSocket. @@ -164,12 +186,38 @@ const live: Layer.Layer< workflowModel.sessionID = input.sessionID workflowModel.systemPrompt = prepared.system.join("\n") workflowModel.toolExecutor = async (toolName, argsJson, _requestID) => { + // (1) Unknown tool — classify before attempting parse or execute. const t = prepared.tools[toolName] if (!t || !t.execute) { - return { result: "", error: `Unknown tool: ${toolName}` } + l.warn("workflow tool call: unknown tool", { tool: toolName, errorType: "unknown_tool" }) + return { + result: "", + error: `[unknown_tool] Tool "${toolName}" is not available. Resend the request using a valid tool name.`, + } + } + + // (2) JSON parse — separate from execution so we can classify invalid_json. + // Do NOT attempt to repair or fill in missing brackets/quotes. + let parsedArgs: unknown + try { + parsedArgs = JSON.parse(argsJson) + } catch (parseErr: any) { + const inputPreview = argsJson.length > 200 ? argsJson.slice(0, 200) + "…[truncated]" : argsJson + l.warn("workflow tool call: invalid JSON", { + tool: toolName, + errorType: "invalid_json", + errorMessage: (parseErr?.message ?? "parse error").slice(0, 200), + inputPreview, + }) + return { + result: "", + error: `[invalid_json] Arguments for tool "${toolName}" are not valid JSON (${(parseErr?.message ?? "parse error").slice(0, 200)}). Resend the request with complete, valid JSON arguments.`, + } } + + // (3) Execute — classify schema_mismatch vs other runtime errors. try { - const result = await t.execute!(JSON.parse(argsJson), { + const result = await t.execute!(parsedArgs, { toolCallId: _requestID, messages: input.messages, abortSignal: input.abort, @@ -181,7 +229,22 @@ const live: Layer.Layer< title: typeof result === "object" ? result?.title : undefined, } } catch (e: any) { - return { result: "", error: e.message ?? String(e) } + // Effect Schema parse errors expose ._tag === "ParseError"; Zod errors expose .issues. + const isSchemaError = + Array.isArray(e?.issues) || e?._tag === "ParseError" || e?.cause?._tag === "ParseError" + const errorType = isSchemaError ? "schema_mismatch" : "execution_error" + l.warn("workflow tool call: execution error", { + tool: toolName, + errorType, + errorMessage: (e?.message ?? String(e)).slice(0, 200), + }) + if (isSchemaError) { + return { + result: "", + error: `[schema_mismatch] Arguments for tool "${toolName}" do not match the expected schema. Resend the request with correctly structured arguments.`, + } + } + return { result: "", error: (e?.message ?? String(e)).slice(0, 500) } } } @@ -271,7 +334,7 @@ const live: Layer.Layer< llmClient, messages: prepared.messages, tools: prepared.tools, - toolChoice: input.toolChoice, + toolChoice: effectiveToolChoice, temperature: prepared.params.temperature, topP: prepared.params.topP, topK: prepared.params.topK, @@ -343,9 +406,10 @@ const live: Layer.Layer< }) }, async experimental_repairToolCall(failed) { + // (a) Tool name case fix only — keep failed.toolCall.input exactly as-is. const lower = failed.toolCall.toolName.toLowerCase() if (lower !== failed.toolCall.toolName && prepared.tools[lower]) { - l.info("repairing tool call", { + l.info("tool call repair: name case fix", { tool: failed.toolCall.toolName, repaired: lower, }) @@ -354,14 +418,49 @@ const live: Layer.Layer< toolName: lower, } } - return { - ...failed.toolCall, - input: JSON.stringify({ + + // Log bounded diagnostics — do NOT echo the full input (may contain file content). + const rawInput: string = failed.toolCall.input + const inputPreview = rawInput.length > 200 ? rawInput.slice(0, 200) + "…[truncated]" : rawInput + + // (b/c) Classify by error type and return null. + // Returning null lets AI SDK propagate the original error as a tool result, + // giving the model actionable feedback to resend with correct parameters. + // We never attempt to repair JSON syntax (no bracket filling, no quote fixing, + // no control-char stripping) — write/edit content must stay intact. + if (NoSuchToolError.isInstance(failed.error)) { + l.warn("tool call repair skipped: unknown tool", { tool: failed.toolCall.toolName, - error: failed.error.message, - }), - toolName: "invalid", + errorType: "unknown_tool", + errorMessage: failed.error.message.slice(0, 300), + }) + return null } + + if (InvalidToolInputError.isInstance(failed.error)) { + // SyntaxError cause → invalid JSON; otherwise → schema mismatch. + const isSyntaxError = failed.error.cause instanceof SyntaxError + const errorType = isSyntaxError ? "invalid_json" : "schema_mismatch" + l.warn("tool call repair skipped", { + tool: failed.toolCall.toolName, + errorType, + errorMessage: failed.error.message.slice(0, 300), + inputPreview, + }) + return null + } + + // Unrecognized error type — return null rather than guessing. + // Cast to unknown: TypeScript narrows failed.error to never after the two + // isInstance guards above (the union is exhausted), but we keep this block + // as a runtime safety net in case AI SDK adds new error subtypes. + const unknownErr = failed.error as unknown + l.warn("tool call repair skipped: unrecognized error", { + tool: failed.toolCall.toolName, + errorMessage: (unknownErr instanceof Error ? unknownErr.message : String(unknownErr)).slice(0, 300), + inputPreview, + }) + return null }, temperature: prepared.params.temperature, topP: prepared.params.topP, @@ -369,7 +468,7 @@ const live: Layer.Layer< providerOptions: ProviderTransform.providerOptions(input.model, prepared.params.options), activeTools: Object.keys(prepared.tools).filter((x) => x !== "invalid"), tools: prepared.tools, - toolChoice: input.toolChoice, + toolChoice: effectiveToolChoice, maxOutputTokens: prepared.params.maxOutputTokens, abortSignal: input.abort, headers: prepared.headers, diff --git a/packages/deepagent-code/src/session/processor.ts b/packages/deepagent-code/src/session/processor.ts index b2e9965e..3aca0ed8 100644 --- a/packages/deepagent-code/src/session/processor.ts +++ b/packages/deepagent-code/src/session/processor.ts @@ -35,8 +35,140 @@ import { toolFileSourceFromUri, Usage, type LLMEvent } from "@deepagent-code/llm import { ToolOutput } from "@deepagent-code/core/tool-output" const DOOM_LOOP_THRESHOLD = 3 +const DOOM_LOOP_SEQUENCE_WINDOW = 12 +const DOOM_LOOP_MIN_REPEATS = 3 +const DOOM_LOOP_MAX_PERIOD = 4 const log = Log.create({ service: "session.processor" }) +// --------------------------------------------------------------------------- +// F1: Activity-level tool-call sequence tracker +// --------------------------------------------------------------------------- + +/** + * Produce a canonical JSON string for any value. Object keys are sorted + * recursively so that `{"b":1,"a":2}` and `{"a":2,"b":1}` produce the same + * fingerprint. Array order is preserved. `undefined` is serialised as + * `null` to match JSON semantics. + */ +function canonicalJson(value: unknown): string { + if (value === null || value === undefined) return "null" + if (typeof value !== "object") return JSON.stringify(value) ?? "null" + if (Array.isArray(value)) return "[" + value.map(canonicalJson).join(",") + "]" + const obj = value as Record + const pairs = Object.keys(obj) + .sort() + .map((k) => JSON.stringify(k) + ":" + canonicalJson(obj[k])) + return "{" + pairs.join(",") + "}" +} + +/** Build the stable fingerprint for one tool invocation. */ +function toolFingerprint(toolName: string, input: unknown): string { + return toolName + ":" + canonicalJson(input) +} + +/** + * Activity-level sequence tracker. One instance is created per durable + * user activity and shared across all processor instances (provider steps) + * within that activity. + * + * Lifecycle guarantees (enforced by the caller in prompt.ts): + * - Created fresh at the start of every runLoop call. + * - NOT process-global; NOT persisted across the whole session. + * - Steer events that are merged into the current activity do NOT reset it. + */ +export class ToolSequenceTracker { + private readonly calls: { fingerprint: string; done: boolean }[] = [] + private readonly callIdToIndex = new Map() + private readonly triggeredSequences = new Set() + + /** Record a newly started (running) tool call. */ + push(callId: string, fingerprint: string): void { + this.calls.push({ fingerprint, done: false }) + this.callIdToIndex.set(callId, this.calls.length - 1) + if (this.calls.length > DOOM_LOOP_SEQUENCE_WINDOW) { + this.calls.shift() + // Adjust stored indices after the shift (oldest entry was removed). + for (const [id, idx] of this.callIdToIndex) { + const next = idx - 1 + if (next < 0) this.callIdToIndex.delete(id) + else this.callIdToIndex.set(id, next) + } + } + } + + /** + * Mark a call as done (completed or failed). Must be called from + * settleToolCall so that the "prior calls must be done" invariant holds + * before the next tool starts. + */ + markDone(callId: string): void { + const idx = this.callIdToIndex.get(callId) + if (idx !== undefined && idx >= 0 && idx < this.calls.length) { + this.calls[idx].done = true + } + this.callIdToIndex.delete(callId) + } + + /** + * Detect a repeating sequence in the current window. + * + * Rules: + * - The last call may be running (done = false) — it is the current call. + * - All prior calls in the detection window must have left the pending + * state (done = true). + * - Period 1–4; at least DOOM_LOOP_MIN_REPEATS complete repetitions. + * + * Returns period/count/sequenceKey on detection, null otherwise. + */ + detect(): { period: number; count: number; sequenceKey: string } | null { + if (this.calls.length === 0) return null + const fps = this.calls.map((c) => c.fingerprint) + + for (let period = 1; period <= DOOM_LOOP_MAX_PERIOD; period++) { + const needed = period * DOOM_LOOP_MIN_REPEATS + if (fps.length < needed) continue + + const windowCalls = this.calls.slice(-needed) + const windowFps = fps.slice(-needed) + + // All calls except the last (which may still be running) must be done. + let priorAllDone = true + for (let i = 0; i < windowCalls.length - 1; i++) { + if (!windowCalls[i].done) { + priorAllDone = false + break + } + } + if (!priorAllDone) continue + + const pattern = windowFps.slice(0, period) + let matches = true + for (let i = period; i < needed; i++) { + if (windowFps[i] !== pattern[i % period]) { + matches = false + break + } + } + if (matches) { + // Use NUL as separator — fingerprints contain ":" and tool output + // JSON but never raw NUL bytes. + return { period, count: DOOM_LOOP_MIN_REPEATS, sequenceKey: pattern.join("\x00") } + } + } + return null + } + + /** True if this exact sequence has already raised a permission request. */ + hasTriggered(sequenceKey: string): boolean { + return this.triggeredSequences.has(sequenceKey) + } + + /** Record that a permission request was raised for this sequence. */ + setTriggered(sequenceKey: string): void { + this.triggeredSequences.add(sequenceKey) + } +} + // PR-2: N-gram sliding-window degeneration detector for reasoning streams. // Detects repetitive/stuck output before it grows unbounded; configurable via // RuntimeFlags.degenerationDetectorMode ("off" | "shadow" | "enforce"). @@ -144,6 +276,13 @@ type Input = { assistantMessage: SessionV1.Assistant sessionID: SessionID model: Provider.Model + /** + * Shared sequence tracker for the current durable user activity. + * Created once in prompt.ts runLoop and passed to every processor.create + * call within the same activity so cross-message loops are detectable. + * Absent only in legacy callers that have not been updated yet. + */ + sequenceTracker?: ToolSequenceTracker } export interface Interface { @@ -204,6 +343,7 @@ export const layer = Layer.effect( assistantMessage: input.assistantMessage, sessionID: input.sessionID, model: input.model, + sequenceTracker: input.sequenceTracker, toolcalls: {}, shouldBreak: false, snapshot: initialSnapshot, @@ -226,6 +366,9 @@ export const layer = Layer.effect( }) const settleToolCall = Effect.fn("SessionProcessor.settleToolCall")(function* (toolCallID: string) { + // Notify the activity-level tracker that this call has finished so it + // satisfies the "prior calls must be done" precondition for detection. + ctx.sequenceTracker?.markDone(toolCallID) const done = ctx.toolcalls[toolCallID]?.done delete ctx.toolcalls[toolCallID] if (done) yield* Deferred.succeed(done, undefined).pipe(Effect.ignore) @@ -638,23 +781,64 @@ export const layer = Layer.effect( : value.providerMetadata, })) + // --------------------------------------------------------------- + // F1: Activity-level cross-message loop detection (primary path) + // --------------------------------------------------------------- + if (ctx.sequenceTracker) { + const fp = toolFingerprint(value.name, input) + ctx.sequenceTracker.push(value.id, fp) + const detected = ctx.sequenceTracker.detect() + if (detected && !ctx.sequenceTracker.hasTriggered(detected.sequenceKey)) { + const agent = yield* agents.get(ctx.assistantMessage.agent) + yield* permission.ask({ + permission: "doom_loop", + patterns: [value.name], + sessionID: ctx.assistantMessage.sessionID, + metadata: { + tool: value.name, + input, + period: detected.period, + count: detected.count, + }, + always: [value.name], + ruleset: agent.permission, + }) + ctx.sequenceTracker.setTriggered(detected.sequenceKey) + } + // Tracker handles all detection for this call; skip legacy path. + return + } + + // --------------------------------------------------------------- + // Legacy per-message detection (fallback when no tracker present) + // --------------------------------------------------------------- const parts = yield* MessageV2.parts(ctx.assistantMessage.id).pipe( Effect.provideService(Database.Service, database), ) const recentParts = parts.slice(-DOOM_LOOP_THRESHOLD) - if ( - recentParts.length !== DOOM_LOOP_THRESHOLD || - !recentParts.every( + const singleRepeat = + recentParts.length === DOOM_LOOP_THRESHOLD && + recentParts.every( (part) => part.type === "tool" && part.tool === value.name && part.state.status !== "pending" && JSON.stringify(part.state.input) === JSON.stringify(input), ) - ) { - return - } + + const sequenceRepeat = + !singleRepeat && + detectRepeatingSequence( + parts + .filter( + (part): part is SessionV1.ToolPart => + part.type === "tool" && part.state.status !== "pending", + ) + .map((part) => `${part.tool}:${JSON.stringify(part.state.input)}`), + ) + + if (!singleRepeat && !sequenceRepeat) return const agent = yield* agents.get(ctx.assistantMessage.agent) yield* permission.ask({ @@ -1185,4 +1369,23 @@ export const defaultLayer = Layer.suspend(() => ), ) +function detectRepeatingSequence(fingerprints: string[]): boolean { + const tail = fingerprints.slice(-DOOM_LOOP_SEQUENCE_WINDOW) + for (let period = 2; period <= DOOM_LOOP_MAX_PERIOD; period++) { + const needed = period * DOOM_LOOP_MIN_REPEATS + if (tail.length < needed) continue + const window = tail.slice(-needed) + const pattern = window.slice(0, period) + let matches = true + for (let i = period; i < needed; i++) { + if (window[i] !== pattern[i % period]) { + matches = false + break + } + } + if (matches) return true + } + return false +} + export * as SessionProcessor from "./processor" diff --git a/packages/deepagent-code/src/session/prompt.ts b/packages/deepagent-code/src/session/prompt.ts index 9b7f201e..6d36fb83 100644 --- a/packages/deepagent-code/src/session/prompt.ts +++ b/packages/deepagent-code/src/session/prompt.ts @@ -2171,6 +2171,11 @@ export const layer = Layer.effect( // default-safe: a construction defect (fs error) yields a no-op writer, never a turn crash. const logWriter = yield* ConversationLogWriter.make(sessionID) + // F1: one tracker per durable user activity; shared by every provider step (processor + // instance) created in this runLoop call so cross-message ABABAB/ABCABC/... patterns + // are detectable. Reset implicitly on the next runLoop invocation (new variable). + const toolSequenceTracker = new SessionProcessor.ToolSequenceTracker() + // V3.8 Phase 3 (v3.8.1 §B.3): fire ONE lightweight code-index pass for this workspace, the real // trigger that finally puts code_symbol nodes on the graph (indexFiles had zero prod callers). // Gated to once-per-session-per-process (indexedSessions) so re-prompts don't re-walk; forked @@ -2421,6 +2426,7 @@ export const layer = Layer.effect( assistantMessage: msg, sessionID, model, + sequenceTracker: toolSequenceTracker, }) .pipe(Effect.onInterrupt(() => finalizeInterruptedAssistant)) diff --git a/packages/deepagent-code/src/tool/task.ts b/packages/deepagent-code/src/tool/task.ts index 17e018e5..5269ae6d 100644 --- a/packages/deepagent-code/src/tool/task.ts +++ b/packages/deepagent-code/src/tool/task.ts @@ -8,7 +8,16 @@ import { SessionID, MessageID } from "../session/schema" import { Identifier } from "@/id/id" import { MessageV2 } from "../session/message-v2" import { Agent } from "../agent/agent" -import { deriveSubagentSessionPermission, filterPrimaryToolsForSubagent, subagentIsWriteType } from "../agent/subagent-permissions" +import { + deriveSubagentSessionPermission, + filterPrimaryToolsForSubagent, + subagentIsWriteType, + resolveSessionDepth, + admitChildOrFail, + MAX_SUBAGENT_DEPTH, + SUBAGENT_DEPTH_META_KEY, +} from "../agent/subagent-permissions" +import { evaluate as evaluatePermission } from "../permission" import type { SessionPrompt } from "../session/prompt" import { Config } from "@/config/config" import { Cause, Effect, Exit, Option, Schema, Scope } from "effect" @@ -235,6 +244,54 @@ export const TaskTool = Tool.define( ? yield* agent.get(parent.agent).pipe(Effect.catchCause(() => Effect.succeed(undefined))) : undefined + // F5: unified child admission — resolves depth once and validates for ALL creation paths + // (takeover and default). Resume (task_id present) takes the validation-only branch; a fresh + // spawn takes the full admission-check branch. Both share `childDepth` for metadata writes. + const parentDepth = yield* resolveSessionDepth(sessions, ctx.sessionID) + + if (session !== undefined) { + // Resume validation: the target session must be a direct child of THIS session with the + // expected agent type and a valid depth. Guards against cross-tree resume or injected IDs. + if (session.parentID !== ctx.sessionID) { + return yield* Effect.fail( + new Error( + `Cannot resume task "${params.task_id}": it is not a direct child of the current session. ` + + `Use a task_id returned by a task you launched in this session.`, + ), + ) + } + if (session.agent && session.agent !== params.subagent_type) { + return yield* Effect.fail( + new Error( + `Cannot resume task "${params.task_id}": its agent type is "${session.agent}" ` + + `but this call requests "${params.subagent_type}". Omit task_id to start a fresh subagent.`, + ), + ) + } + const resumedDepth = yield* resolveSessionDepth(sessions, session.id) + if (resumedDepth > MAX_SUBAGENT_DEPTH) { + return yield* Effect.fail( + new Error( + `Cannot resume task "${params.task_id}": resolved depth ${resumedDepth} exceeds ` + + `the hard limit (MAX_SUBAGENT_DEPTH=${MAX_SUBAGENT_DEPTH}).`, + ), + ) + } + } else { + // New session: full admission gate — depth ceiling then delegation permission. + const admission = admitChildOrFail({ + callerDepth: parentDepth, + callerAgentPermission: parentAgent?.permission ?? [], + callerSessionPermission: parent.permission ?? [], + targetAgentType: params.subagent_type, + }) + if ("error" in admission) { + return yield* Effect.fail(new Error(admission.error)) + } + } + // childDepth is used by BOTH the takeover path and the default path when writing metadata. + const childDepth = parentDepth + 1 + // v4.0.4 块1 (1a+1b): timeout + takeover — enabled ONLY when DEEPAGENT_CODE_SUBAGENT_TIMEOUT_MS // is set; when it is not, execution falls through to the default path below, which stays // byte-identical to the pre-flag behavior. timeout and takeover are an inseparable unit: a bare @@ -292,6 +349,11 @@ export const TaskTool = Tool.define( title: params.description + ` (@${next.name} subagent)`, agent: next.name, ...(worktreeInfo ? { directory: worktreeInfo.directory } : {}), + // F5: write normalised depth into metadata so future resolveSessionDepth calls for this + // session return the correct value without needing to walk the full parentID chain. + metadata: { + deepagent: { [SUBAGENT_DEPTH_META_KEY]: childDepth }, + }, permission: [ ...deriveSubagentSessionPermission({ parentSessionPermission: parent.permission ?? [], @@ -364,12 +426,14 @@ export const TaskTool = Tool.define( } : {}), tools: { - ...(next.permission.some((rule) => rule.permission === "todowrite") ? {} : { todowrite: false }), + // F5: use evaluatePermission (not presence check) — "ask" is not sufficient, only + // an explicit "allow" rule on the subagent's own permission makes these tools visible. + ...(evaluatePermission("todowrite", "*", next.permission).action === "allow" ? {} : { todowrite: false }), // task_status inspects the PARENT session's dispatched-subagent list, so it is a // task-management capability: gate it on the same `task` permission. A subagent that // cannot dispatch tasks has no sibling list to inspect, and (like `task`) must not be // able to reach into task orchestration unless explicitly granted. - ...(next.permission.some((rule) => rule.permission === id) ? {} : { task: false, task_status: false }), + ...(evaluatePermission(id, "*", next.permission).action === "allow" ? {} : { task: false, task_status: false }), ...Object.fromEntries((cfg.experimental?.primary_tools ?? []).map((item) => [item, false])), }, parts: promptParts, @@ -759,6 +823,11 @@ export const TaskTool = Tool.define( title: params.description + ` (@${next.name} subagent)`, agent: next.name, ...(worktreeInfo ? { directory: worktreeInfo.directory } : {}), + // F5: write normalised depth into metadata so future resolveSessionDepth calls for this + // session return the correct value without needing to walk the full parentID chain. + metadata: { + deepagent: { [SUBAGENT_DEPTH_META_KEY]: childDepth }, + }, permission: [ ...deriveSubagentSessionPermission({ parentSessionPermission: parent.permission ?? [], @@ -880,9 +949,11 @@ export const TaskTool = Tool.define( ? { format: new SessionV1.OutputFormatJsonSchema({ type: "json_schema", schema: resolvedOutputSchema }) } : {}), tools: { - ...(next.permission.some((rule) => rule.permission === "todowrite") ? {} : { todowrite: false }), + // F5: use evaluatePermission (not presence check) — "ask" is not sufficient, only + // an explicit "allow" rule on the subagent's own permission makes these tools visible. + ...(evaluatePermission("todowrite", "*", next.permission).action === "allow" ? {} : { todowrite: false }), // task_status is gated on the same `task` permission (see the takeover-path block above). - ...(next.permission.some((rule) => rule.permission === id) ? {} : { task: false, task_status: false }), + ...(evaluatePermission(id, "*", next.permission).action === "allow" ? {} : { task: false, task_status: false }), ...Object.fromEntries((cfg.experimental?.primary_tools ?? []).map((item) => [item, false])), }, parts: promptParts, diff --git a/packages/deepagent-code/test/session/llm-f3-tool-safety.test.ts b/packages/deepagent-code/test/session/llm-f3-tool-safety.test.ts new file mode 100644 index 00000000..7ce0263a --- /dev/null +++ b/packages/deepagent-code/test/session/llm-f3-tool-safety.test.ts @@ -0,0 +1,338 @@ +/** + * F3: Tool parameter JSON parsing safety tests + * + * Acceptance criteria (from docs/4.0.4_r4.md §F3): + * 1. Valid JSON passes unchanged. + * 2. Tool name case fix still works. + * 3. Invalid JSON produces bounded, non-leaking tool error with resubmit hint. + * 4. Schema mismatch not misclassified as JSON syntax error. + * 5. Truncated write content is NOT executed or guessed. + * 6. Same bad call eventually hits F1 loop protection. + * + * These tests exercise the two F3-changed paths in llm.ts: + * A. AI SDK experimental_repairToolCall callback — via error type discrimination helpers. + * B. Workflow toolExecutor — via isolated helper that mirrors the closure logic. + */ + +import { describe, test, expect } from "bun:test" +import { InvalidToolInputError, NoSuchToolError } from "ai" + +// --------------------------------------------------------------------------- +// A. Error type discrimination helpers (mirrors repair callback logic) +// --------------------------------------------------------------------------- + +/** + * Mirrors the repair callback classification: given a repair error, return + * "case_fix" | "unknown_tool" | "invalid_json" | "schema_mismatch" | "unknown". + */ +function classifyRepairError( + toolName: string, + loweredName: string, + toolExists: boolean, + error: NoSuchToolError | InvalidToolInputError, +): "case_fix" | "unknown_tool" | "invalid_json" | "schema_mismatch" | "unknown" { + if (loweredName !== toolName && toolExists) return "case_fix" + if (NoSuchToolError.isInstance(error)) return "unknown_tool" + if (InvalidToolInputError.isInstance(error)) { + return error.cause instanceof SyntaxError ? "invalid_json" : "schema_mismatch" + } + return "unknown" +} + +/** + * Mirrors the bounded preview logic used in the repair callback. + * Must never exceed 200 chars in the truncated preview. + */ +function boundedPreview(rawInput: string, limit = 200): string { + return rawInput.length > limit ? rawInput.slice(0, limit) + "…[truncated]" : rawInput +} + +describe("F3.A repair callback — error type discrimination", () => { + test("case fix: lowercase matches existing tool → case_fix", () => { + const error = new NoSuchToolError({ toolName: "Bash" }) + const result = classifyRepairError("Bash", "bash", /* toolExists */ true, error) + expect(result).toBe("case_fix") + }) + + test("unknown tool: NoSuchToolError, no lowercase match → unknown_tool", () => { + const error = new NoSuchToolError({ toolName: "nonexistent" }) + const result = classifyRepairError("nonexistent", "nonexistent", /* toolExists */ false, error) + expect(result).toBe("unknown_tool") + }) + + test("invalid JSON: InvalidToolInputError with SyntaxError cause → invalid_json", () => { + let syntaxErr: SyntaxError + try { + JSON.parse("{bad json}") + } catch (e) { + syntaxErr = e as SyntaxError + } + const error = new InvalidToolInputError({ + toolName: "write", + toolInput: "{bad json}", + cause: syntaxErr!, + }) + const result = classifyRepairError("write", "write", /* toolExists */ true, error) + expect(result).toBe("invalid_json") + }) + + test("schema mismatch: InvalidToolInputError with non-SyntaxError cause → schema_mismatch", () => { + // Schema validation errors are not SyntaxErrors (e.g. Zod validation failures). + const zodLikeError = new TypeError("Expected string, received number") + const error = new InvalidToolInputError({ + toolName: "bash", + toolInput: '{"command":123}', + cause: zodLikeError, + }) + const result = classifyRepairError("bash", "bash", /* toolExists */ true, error) + expect(result).toBe("schema_mismatch") + }) + + test("schema mismatch is NOT classified as invalid_json", () => { + // This is the key invariant: a schema mismatch (valid JSON, wrong shape) + // must not be reported as a JSON syntax error to the model. + const zodError = Object.assign(new Error("Expected string"), { issues: [{ code: "invalid_type" }] }) + const error = new InvalidToolInputError({ + toolName: "read", + toolInput: '{"filePath":42}', + cause: zodError, + }) + const result = classifyRepairError("read", "read", /* toolExists */ true, error) + expect(result).toBe("schema_mismatch") + expect(result).not.toBe("invalid_json") + }) +}) + +describe("F3.A repair callback — input preview safety", () => { + test("input within limit passes through unchanged", () => { + const short = '{"command":"ls -la"}' + expect(boundedPreview(short)).toBe(short) + }) + + test("input exceeding 200 chars is truncated with ellipsis suffix", () => { + const long = "x".repeat(300) + const preview = boundedPreview(long) + expect(preview.length).toBeLessThanOrEqual(213) // 200 + length of "…[truncated]" + expect(preview.endsWith("…[truncated]")).toBe(true) + }) + + test("preview never echoes full file content for a large write body", () => { + // Simulates a write tool call with a large content field. + const largeContent = "function foo() {\n" + " const x = 1;\n".repeat(200) + "}" + const fakeInput = JSON.stringify({ filePath: "src/index.ts", content: largeContent }) + const preview = boundedPreview(fakeInput) + // The preview must be bounded regardless of content size. + expect(preview.length).toBeLessThanOrEqual(213) + expect(preview.endsWith("…[truncated]")).toBe(true) + }) + + test("error message is bounded to 300 chars", () => { + const longMessage = "Parse error at position 0: " + "x".repeat(400) + const bounded = longMessage.slice(0, 300) + expect(bounded.length).toBe(300) + // Confirms the slice(0, 300) cap used in the repair callback is enforced. + expect(bounded).not.toContain("x".repeat(350)) + }) +}) + +// --------------------------------------------------------------------------- +// B. Workflow toolExecutor — isolated helper tests +// --------------------------------------------------------------------------- + +/** + * Isolated version of the workflow toolExecutor logic from llm.ts. + * Mirrors the actual implementation to allow direct unit testing without + * spinning up the full Effect layer. + */ +async function workflowToolExecutor( + toolName: string, + argsJson: string, + requestID: string, + tools: Record Promise }>, +): Promise<{ result: string; error?: string; metadata?: unknown; title?: unknown }> { + // (1) Unknown tool + const t = tools[toolName] + if (!t || !t.execute) { + return { + result: "", + error: `[unknown_tool] Tool "${toolName}" is not available. Resend the request using a valid tool name.`, + } + } + + // (2) JSON parse + let parsedArgs: unknown + try { + parsedArgs = JSON.parse(argsJson) + } catch (parseErr: any) { + const inputPreview = argsJson.length > 200 ? argsJson.slice(0, 200) + "…[truncated]" : argsJson + return { + result: "", + error: `[invalid_json] Arguments for tool "${toolName}" are not valid JSON (${(parseErr?.message ?? "parse error").slice(0, 200)}). Resend the request with complete, valid JSON arguments.`, + } + } + + // (3) Execute + try { + const result = await t.execute!(parsedArgs, { toolCallId: requestID, messages: [], abortSignal: undefined }) + const output = typeof result === "string" ? result : ((result as any)?.output ?? JSON.stringify(result)) + return { + result: output, + metadata: typeof result === "object" ? (result as any)?.metadata : undefined, + title: typeof result === "object" ? (result as any)?.title : undefined, + } + } catch (e: any) { + const isSchemaError = + Array.isArray(e?.issues) || e?._tag === "ParseError" || e?.cause?._tag === "ParseError" + if (isSchemaError) { + return { + result: "", + error: `[schema_mismatch] Arguments for tool "${toolName}" do not match the expected schema. Resend the request with correctly structured arguments.`, + } + } + return { result: "", error: (e?.message ?? String(e)).slice(0, 500) } + } +} + +describe("F3.B workflow toolExecutor — error classification", () => { + const echoTool = { + execute: async (args: unknown) => ({ output: JSON.stringify(args) }), + } + + // AC1: valid JSON passes through unchanged + test("AC1: valid JSON is parsed and passed to execute without modification", async () => { + const result = await workflowToolExecutor( + "echo", + '{"message":"hello world"}', + "call-1", + { echo: echoTool }, + ) + expect(result.error).toBeUndefined() + expect(result.result).toBe('{"message":"hello world"}') + }) + + // AC3: invalid JSON → bounded error with resubmit hint + test("AC3: invalid JSON produces bounded error with [invalid_json] prefix", async () => { + const result = await workflowToolExecutor( + "echo", + '{"message": unquoted}', + "call-2", + { echo: echoTool }, + ) + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[invalid_json\]/) + expect(result.error).toContain("echo") + expect(result.error).toContain("Resend") + }) + + // AC3: error message is bounded even for very long parse error messages + test("AC3: invalid JSON error message does not echo full large content", async () => { + const largeContent = '{"content":"' + "a".repeat(1000) + '"truncated here...' + const result = await workflowToolExecutor("echo", largeContent, "call-3", { echo: echoTool }) + expect(result.error).toBeDefined() + expect(result.error!.length).toBeLessThan(600) // bounded; not 1000+ chars + expect(result.error).toMatch(/^\[invalid_json\]/) + }) + + // AC4: schema mismatch not misclassified as JSON syntax error + test("AC4: schema mismatch produces [schema_mismatch], not [invalid_json]", async () => { + const schemaTool = { + execute: async (_args: unknown) => { + // Simulate Effect Schema ParseError + const err = Object.assign(new Error("Expected string"), { _tag: "ParseError" as const }) + throw err + }, + } + const result = await workflowToolExecutor( + "schema_tool", + '{"value":42}', + "call-4", + { schema_tool: schemaTool }, + ) + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[schema_mismatch\]/) + expect(result.error).not.toMatch(/invalid_json/) + expect(result.error).toContain("Resend") + }) + + // AC4: Zod-style schema error (has .issues array) + test("AC4: Zod schema error (issues array) classified as schema_mismatch", async () => { + const zodTool = { + execute: async (_args: unknown) => { + const err = Object.assign(new Error("Validation failed"), { + issues: [{ code: "invalid_type", expected: "string", received: "number" }], + }) + throw err + }, + } + const result = await workflowToolExecutor("zod_tool", '{"x":1}', "call-5", { zod_tool: zodTool }) + expect(result.error).toMatch(/^\[schema_mismatch\]/) + }) + + // AC5: truncated write content is not executed + test("AC5: truncated JSON (missing closing brace) is not executed", async () => { + const writeTool = { + execute: async (args: unknown) => ({ output: `wrote ${(args as any).filePath}` }), + } + // Truncated write call — missing closing } + const truncated = '{"filePath":"src/main.ts","content":"function foo() {\\n return 1;' + const result = await workflowToolExecutor("write", truncated, "call-6", { write: writeTool }) + // Must be a parse error, not a successful write + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[invalid_json\]/) + // The execute function must NOT have been called + expect(result.result).toBe("") + }) + + // Unknown tool classification + test("unknown tool produces [unknown_tool] error with resubmit hint", async () => { + const result = await workflowToolExecutor("nonexistent", '{"x":1}', "call-7", {}) + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[unknown_tool\]/) + expect(result.error).toContain("nonexistent") + expect(result.error).toContain("Resend") + }) + + // Runtime execution error (not schema, not JSON parse) stays as-is but bounded + test("runtime execution error produces bounded error message", async () => { + const failingTool = { + execute: async () => { + throw new Error("disk full: " + "x".repeat(600)) + }, + } + const result = await workflowToolExecutor("failing", '{"x":1}', "call-8", { failing: failingTool }) + expect(result.error).toBeDefined() + // Not a schema or JSON error + expect(result.error).not.toMatch(/^\[(invalid_json|schema_mismatch|unknown_tool)\]/) + // Bounded to 500 chars + expect(result.error!.length).toBeLessThanOrEqual(500) + }) +}) + +describe("F3 constraints — no regex repair, no content guessing", () => { + test("unescaped quote in content is not silently fixed — parse error is returned", async () => { + // A write call where the content has an unescaped quote (common model error). + // The correct behaviour is to return an error, not to fix the quote. + const writeTool = { + execute: async (args: unknown) => ({ output: `ok: ${(args as any).content?.slice(0, 20)}` }), + } + const badInput = '{"filePath":"a.ts","content":"let x = "hello""}' + const result = await workflowToolExecutor("write", badInput, "call-9", { write: writeTool }) + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[invalid_json\]/) + // execute must NOT have been called with guessed/repaired content + expect(result.result).toBe("") + }) + + test("control characters in content are not stripped — parse error is returned", async () => { + // Content with a raw newline inside a JSON string (not \\n escaped) + const writeTool = { + execute: async (args: unknown) => ({ output: "ok" }), + } + const badInput = '{"filePath":"b.ts","content":"line1\nline2"}' + const result = await workflowToolExecutor("write", badInput, "call-10", { write: writeTool }) + // JSON.parse rejects this — it must NOT be silently fixed + expect(result.error).toBeDefined() + expect(result.error).toMatch(/^\[invalid_json\]/) + expect(result.result).toBe("") + }) +}) diff --git a/packages/deepagent-code/test/session/tool-sequence-tracker.test.ts b/packages/deepagent-code/test/session/tool-sequence-tracker.test.ts new file mode 100644 index 00000000..eb3d1768 --- /dev/null +++ b/packages/deepagent-code/test/session/tool-sequence-tracker.test.ts @@ -0,0 +1,393 @@ +/** + * Tests for F1: activity-level tool-call sequence tracker. + * + * Coverage targets from 4.0.4_r4.md §F1 acceptance criteria: + * 1. Single-message ABABAB triggers (period-2, ≥3 repetitions within one processor). + * 2. Cross-6-message ABABAB triggers (same sequence spread across 6 assistant messages). + * 3. ABCABCABC triggers (period-3). + * 4. ABCDABCDABCD triggers (period-4). + * 5. AAA triggers (period-1 — original gate preserved). + * 6. Incomplete cycles do NOT trigger (ABAB = only 2 repetitions). + * 7. Continuously changing arguments do NOT trigger. + * 8. Two distinct user activities do NOT share state (each gets a fresh tracker). + * 9. Duplicate permission requests are suppressed for the same sequence. + * 10. Object key order does not affect the fingerprint (canonical JSON). + */ + +import { describe, expect, test } from "bun:test" +import { ToolSequenceTracker } from "@/session/processor" + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** Simulate a complete (done) tool call on a tracker. */ +function push(tracker: ToolSequenceTracker, id: string, tool: string, input: unknown = {}): void { + tracker.push(id, fingerprint(tool, input)) + tracker.markDone(id) +} + +/** Push without marking done (simulates the running/current call). */ +function pushRunning(tracker: ToolSequenceTracker, id: string, tool: string, input: unknown = {}): void { + tracker.push(id, fingerprint(tool, input)) +} + +/** Replicate the fingerprint logic from processor.ts for test assertions. */ +function fingerprint(tool: string, input: unknown): string { + return tool + ":" + canonicalJson(input) +} + +function canonicalJson(value: unknown): string { + if (value === null || value === undefined) return "null" + if (typeof value !== "object") return JSON.stringify(value) ?? "null" + if (Array.isArray(value)) return "[" + (value as unknown[]).map(canonicalJson).join(",") + "]" + const obj = value as Record + const pairs = Object.keys(obj) + .sort() + .map((k) => JSON.stringify(k) + ":" + canonicalJson(obj[k])) + return "{" + pairs.join(",") + "}" +} + +// --------------------------------------------------------------------------- +// Acceptance criterion 5: period-1 (AAA) +// --------------------------------------------------------------------------- + +describe("period-1 detection (AAA)", () => { + test("AAA triggers on the 3rd call", () => { + const t = new ToolSequenceTracker() + push(t, "1", "bash", { cmd: "ls" }) + push(t, "2", "bash", { cmd: "ls" }) + pushRunning(t, "3", "bash", { cmd: "ls" }) + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(1) + expect(result?.count).toBe(3) + }) + + test("AA (only 2) does NOT trigger", () => { + const t = new ToolSequenceTracker() + push(t, "1", "bash", { cmd: "ls" }) + pushRunning(t, "2", "bash", { cmd: "ls" }) + expect(t.detect()).toBeNull() + }) + + test("different inputs do NOT trigger period-1", () => { + const t = new ToolSequenceTracker() + push(t, "1", "bash", { cmd: "ls" }) + push(t, "2", "bash", { cmd: "pwd" }) + pushRunning(t, "3", "bash", { cmd: "ls" }) + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 1 & 2: period-2 (ABABAB), single and cross-message +// --------------------------------------------------------------------------- + +describe("period-2 detection (ABABAB)", () => { + test("ABABAB in a single sequence triggers (criterion 1)", () => { + const t = new ToolSequenceTracker() + push(t, "1", "read", { path: "/a" }) + push(t, "2", "bash", { cmd: "x" }) + push(t, "3", "read", { path: "/a" }) + push(t, "4", "bash", { cmd: "x" }) + push(t, "5", "read", { path: "/a" }) + pushRunning(t, "6", "bash", { cmd: "x" }) + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(2) + }) + + test("ABAB (4 calls, only 2 repetitions) does NOT trigger (criterion 6)", () => { + const t = new ToolSequenceTracker() + push(t, "1", "read", { path: "/a" }) + push(t, "2", "bash", { cmd: "x" }) + push(t, "3", "read", { path: "/a" }) + pushRunning(t, "4", "bash", { cmd: "x" }) + expect(t.detect()).toBeNull() + }) + + test("cross-6-message ABABAB triggers (criterion 2)", () => { + // Six separate tracker.push calls — one per assistant-message as in prod. + const t = new ToolSequenceTracker() + // Message 1 → call A + push(t, "m1c1", "read", { path: "/a" }) + // Message 2 → call B + push(t, "m2c1", "bash", { cmd: "x" }) + // Message 3 → call A + push(t, "m3c1", "read", { path: "/a" }) + // Message 4 → call B + push(t, "m4c1", "bash", { cmd: "x" }) + // Message 5 → call A + push(t, "m5c1", "read", { path: "/a" }) + // Message 6 → call B (current, running) + pushRunning(t, "m6c1", "bash", { cmd: "x" }) + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(2) + expect(result?.count).toBe(3) + }) + + test("AB (different tools) plus a third different tool does not trigger period-2", () => { + const t = new ToolSequenceTracker() + push(t, "1", "read", { path: "/a" }) + push(t, "2", "bash", { cmd: "x" }) + pushRunning(t, "3", "write", { path: "/c" }) + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 3: period-3 (ABCABCABC) +// --------------------------------------------------------------------------- + +describe("period-3 detection (ABCABCABC)", () => { + test("ABCABCABC triggers", () => { + const t = new ToolSequenceTracker() + push(t, "1", "read", { path: "/a" }) + push(t, "2", "bash", { cmd: "x" }) + push(t, "3", "write", { path: "/c" }) + push(t, "4", "read", { path: "/a" }) + push(t, "5", "bash", { cmd: "x" }) + push(t, "6", "write", { path: "/c" }) + push(t, "7", "read", { path: "/a" }) + push(t, "8", "bash", { cmd: "x" }) + pushRunning(t, "9", "write", { path: "/c" }) + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(3) + }) + + test("ABCABC (6 calls, only 2 reps) does NOT trigger", () => { + const t = new ToolSequenceTracker() + push(t, "1", "read", { path: "/a" }) + push(t, "2", "bash", { cmd: "x" }) + push(t, "3", "write", { path: "/c" }) + push(t, "4", "read", { path: "/a" }) + push(t, "5", "bash", { cmd: "x" }) + pushRunning(t, "6", "write", { path: "/c" }) + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 4: period-4 (ABCDABCDABCD) +// --------------------------------------------------------------------------- + +describe("period-4 detection (ABCDABCDABCD)", () => { + test("ABCDABCDABCD triggers", () => { + const t = new ToolSequenceTracker() + const calls = [ + ["read", { path: "/a" }], + ["bash", { cmd: "x" }], + ["write", { path: "/c" }], + ["list", { dir: "/d" }], + ] as const + let id = 0 + for (let rep = 0; rep < 3; rep++) { + for (let i = 0; i < 4; i++) { + id++ + const [tool, input] = calls[i] + const isLast = rep === 2 && i === 3 + if (isLast) pushRunning(t, String(id), tool, input) + else push(t, String(id), tool, input) + } + } + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(4) + }) + + test("ABCDABCD (8 calls, 2 reps) does NOT trigger", () => { + const t = new ToolSequenceTracker() + const calls = [ + ["read", { path: "/a" }], + ["bash", { cmd: "x" }], + ["write", { path: "/c" }], + ["list", { dir: "/d" }], + ] as const + let id = 0 + for (let rep = 0; rep < 2; rep++) { + for (let i = 0; i < 4; i++) { + id++ + const [tool, input] = calls[i] + const isLast = rep === 1 && i === 3 + if (isLast) pushRunning(t, String(id), tool, input) + else push(t, String(id), tool, input) + } + } + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 7: continuously changing arguments +// --------------------------------------------------------------------------- + +describe("continuously changing arguments do NOT trigger", () => { + test("same tool with monotonically changing input does not trigger", () => { + const t = new ToolSequenceTracker() + for (let i = 1; i <= 12; i++) { + const id = String(i) + if (i < 12) push(t, id, "bash", { cmd: `step-${i}` }) + else pushRunning(t, id, "bash", { cmd: `step-${i}` }) + } + expect(t.detect()).toBeNull() + }) + + test("alternating tools with always-different arguments do not trigger", () => { + const t = new ToolSequenceTracker() + for (let i = 1; i <= 12; i++) { + const tool = i % 2 === 0 ? "bash" : "read" + const id = String(i) + if (i < 12) push(t, id, tool, { step: i }) + else pushRunning(t, id, tool, { step: i }) + } + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 8: two activities use fresh trackers +// --------------------------------------------------------------------------- + +describe("activity isolation (fresh tracker per activity)", () => { + test("sequence split across two tracker instances does not trigger", () => { + // Simulate activity 1: calls A, B, A + const t1 = new ToolSequenceTracker() + push(t1, "1", "read", { path: "/a" }) + push(t1, "2", "bash", { cmd: "x" }) + pushRunning(t1, "3", "read", { path: "/a" }) + // activity 1 never reaches ABABAB threshold + + // Simulate activity 2: starts fresh — picks up B, A, B + const t2 = new ToolSequenceTracker() + push(t2, "4", "bash", { cmd: "x" }) + push(t2, "5", "read", { path: "/a" }) + pushRunning(t2, "6", "bash", { cmd: "x" }) + // t2 only has 3 calls; no 6-call window for period-2 + expect(t1.detect()).toBeNull() + expect(t2.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 9: duplicate permission suppression +// --------------------------------------------------------------------------- + +describe("duplicate permission suppression (hasTriggered / setTriggered)", () => { + test("same sequence only triggers once", () => { + const t = new ToolSequenceTracker() + push(t, "1", "bash", { cmd: "ls" }) + push(t, "2", "bash", { cmd: "ls" }) + pushRunning(t, "3", "bash", { cmd: "ls" }) + + const first = t.detect()! + expect(first).not.toBeNull() + expect(t.hasTriggered(first.sequenceKey)).toBe(false) + t.setTriggered(first.sequenceKey) + expect(t.hasTriggered(first.sequenceKey)).toBe(true) + + // Simulate the tool result arriving: settleToolCall → markDone("3") + t.markDone("3") + + // Next activity steps produce the same pattern again + push(t, "4", "bash", { cmd: "ls" }) + pushRunning(t, "5", "bash", { cmd: "ls" }) + const second = t.detect()! + expect(second).not.toBeNull() + // Sequence key is the same → already suppressed + expect(t.hasTriggered(second.sequenceKey)).toBe(true) + }) + + test("different sequences have independent triggered state", () => { + const t = new ToolSequenceTracker() + push(t, "1", "bash", { cmd: "ls" }) + push(t, "2", "bash", { cmd: "ls" }) + pushRunning(t, "3", "bash", { cmd: "ls" }) + + const r = t.detect()! + t.setTriggered(r.sequenceKey) + + // A completely different sequence key should NOT be marked as triggered + expect(t.hasTriggered("other\x00sequence")).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// Acceptance criterion 10: canonical JSON — key order independence +// --------------------------------------------------------------------------- + +describe("canonical JSON fingerprint (key order independence)", () => { + test("objects with the same keys/values in different order produce the same fingerprint", () => { + const t = new ToolSequenceTracker() + + // Two calls with logically identical input but different key order + const inputA = { b: 2, a: 1, c: { z: 26, m: 13 } } + const inputB = { a: 1, c: { m: 13, z: 26 }, b: 2 } + + push(t, "1", "bash", inputA) + push(t, "2", "bash", inputA) + pushRunning(t, "3", "bash", inputB) // same canonical form as inputA + + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(1) // treated as the same call repeated + }) + + test("arrays preserve order in canonical JSON", () => { + const t = new ToolSequenceTracker() + // Different array orders must NOT match + push(t, "1", "bash", { args: [1, 2, 3] }) + push(t, "2", "bash", { args: [1, 2, 3] }) + pushRunning(t, "3", "bash", { args: [3, 2, 1] }) + // The third call has a different fingerprint → no period-1 match + expect(t.detect()).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// "Prior calls must be done" invariant +// --------------------------------------------------------------------------- + +describe("done invariant", () => { + test("calls not yet marked done do NOT contribute to detection window", () => { + const t = new ToolSequenceTracker() + // Push 3 calls but don't mark any as done — all are still "running" + t.push("1", fingerprint("bash", { cmd: "ls" })) + t.push("2", fingerprint("bash", { cmd: "ls" })) + t.push("3", fingerprint("bash", { cmd: "ls" })) + // Only the LAST may be running; the first two are not done → should not trigger + expect(t.detect()).toBeNull() + }) + + test("once prior calls are marked done the loop is detected", () => { + const t = new ToolSequenceTracker() + t.push("1", fingerprint("bash", { cmd: "ls" })) + t.markDone("1") + t.push("2", fingerprint("bash", { cmd: "ls" })) + t.markDone("2") + t.push("3", fingerprint("bash", { cmd: "ls" })) // current, not done + expect(t.detect()).not.toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Window size enforcement (keeps only last 12) +// --------------------------------------------------------------------------- + +describe("sliding window (max 12 calls)", () => { + test("13th call evicts the 1st; pattern that relied on 1st call is gone", () => { + const t = new ToolSequenceTracker() + // Push 12 unique calls then one that would form AAA only if 1st were present + for (let i = 1; i <= 12; i++) push(t, String(i), "unique-" + i, {}) + // Now push something that forms a period-1 with only its peers: + push(t, "13", "bash", { cmd: "ls" }) + push(t, "14", "bash", { cmd: "ls" }) + pushRunning(t, "15", "bash", { cmd: "ls" }) + // There are only 3 bash calls in the window — should detect AAA + const result = t.detect() + expect(result).not.toBeNull() + expect(result?.period).toBe(1) + }) +}) diff --git a/packages/desktop/src/main/server.ts b/packages/desktop/src/main/server.ts index c8ab3664..1e55f014 100644 --- a/packages/desktop/src/main/server.ts +++ b/packages/desktop/src/main/server.ts @@ -7,6 +7,10 @@ import { getUserShell, loadShellEnv } from "./shell-env" import { getStore } from "./store" import { DEFAULT_SERVER_URL_KEY } from "./store-keys" +// Counts how many times a local sidecar has been spawned in this process. +// spawnIndex === 1 means cold start; > 1 means a hot restart. +let sidecarSpawnCount = 0 + export type HealthCheck = { wait: Promise } type SidecarMessage = @@ -58,7 +62,13 @@ export async function spawnLocalServer( password: string, options: SpawnLocalServerOptions, ) { + const spawnIndex = ++sidecarSpawnCount + const cold = spawnIndex === 1 + const logger = getLogger() + const sidecar = join(dirname(fileURLToPath(import.meta.url)), "sidecar.js") + // desktop.sidecar_spawn — start: utility process fork + const sidecarSpawnT0 = Date.now() const child = utilityProcess.fork(sidecar, [], { cwd: process.cwd(), env: createSidecarEnv(), @@ -108,6 +118,13 @@ export async function spawnLocalServer( if (done) return done = true cleanup() + // desktop.sidecar_spawn — end: sidecar ready message received + const sidecarSpawnDuration = Date.now() - sidecarSpawnT0 + logger?.scope("startup").info("telemetry", { + event: "desktop.sidecar_spawn", + durationMs: sidecarSpawnDuration, + cold, + }) resolve() return } @@ -142,6 +159,8 @@ export async function spawnLocalServer( const wait = (async () => { const url = `http://${hostname}:${port}` let healthy = false + // desktop.health_wait — start: ready message received, now polling for API readiness + const healthWaitT0 = Date.now() const gone = exit.promise.then((code) => { if (healthy) return throw new Error(`Sidecar exited before health check passed with code ${code}`) @@ -149,11 +168,19 @@ export async function spawnLocalServer( const ready = async () => { while (true) { - await new Promise((resolve) => setTimeout(resolve, 100)) + // Try immediately first; sleep only after a failed attempt. + // This removes the previous unconditional 100ms pre-sleep before the first request. if (await checkHealth(url, password)) { healthy = true + // desktop.health_wait — end: health first success + logger?.scope("startup").info("telemetry", { + event: "desktop.health_wait", + durationMs: Date.now() - healthWaitT0, + cold, + }) return } + await new Promise((resolve) => setTimeout(resolve, 100)) } } diff --git a/packages/desktop/src/renderer/index.tsx b/packages/desktop/src/renderer/index.tsx index f231bf08..8ad8d93f 100644 --- a/packages/desktop/src/renderer/index.tsx +++ b/packages/desktop/src/renderer/index.tsx @@ -29,6 +29,12 @@ import "./styles.css" import { Splash } from "@deepagent-code/ui/logo" import { useTheme } from "@deepagent-code/ui/theme/context" +// renderer.initialization — start: renderer module begins executing. +// Captured at module level so it includes all synchronous setup before render(). +const rendererStartTime = performance.now() +/** Set to true the first time the renderer reaches ready() to avoid duplicate logs. */ +let rendererReadyLogged = false + const root = document.getElementById("root") if (import.meta.env.DEV && !(root instanceof HTMLElement)) { throw new Error(t("error.dev.rootNotFound")) @@ -329,15 +335,42 @@ render(() => { function App() { const wslServers = useWslServers() - const splash = ( -
- -
- ) + + // App-ready gate: the app layer fires "deepagent-code:app-ready" once layout + // persist + routing + first sessions fetch are all done. A 5-second failsafe + // prevents a permanent hang. We use an OVERLAY approach: the app renders + // immediately (so Layout can mount and do its async work), and the splash sits + // on top until appReady fires. This avoids the circular dependency where the + // app can never fire the event because it can't render until the event fires. + const [appReady, setAppReady] = createSignal(false) + onMount(() => { + const fallback = window.setTimeout(() => setAppReady(true), 5_000) + window.addEventListener( + "deepagent-code:app-ready", + () => { + clearTimeout(fallback) + setAppReady(true) + }, + { once: true }, + ) + onCleanup(() => clearTimeout(fallback)) + }) const ready = createMemo( () => !defaultServer.loading && !sidecar.loading && !windowCount.loading && !locale.loading, ) + + // renderer.initialization — end: basic resources resolved. + createEffect(() => { + if (!ready() || rendererReadyLogged) return + rendererReadyLogged = true + const durationMs = Math.round(performance.now() - rendererStartTime) + console.info("[startup] telemetry", { + event: "renderer.initialization", + durationMs, + cold: true, + }) + }) const servers = createMemo(() => { const data = initializationData(sidecar) const list: ServerConnection.Any[] = [] @@ -361,15 +394,29 @@ render(() => { ) return ( - - - {(key) => ( - - - - )} + <> + {/* App renders immediately so Layout can mount and do async startup work. */} + + + {(key) => ( + + + + )} + + + {/* + * Splash overlay: sits on top until both server resources are ready AND + * the app layer has signalled session-UI readiness (deepagent-code:app-ready). + * The 5-second failsafe in the onMount above ensures this never hangs + * permanently even if the async chain encounters an error. + */} + +
+ +
-
+ ) }