From c2a8f17a74800f9501b7bbbc9e28c5d29768ba2f Mon Sep 17 00:00:00 2001 From: deepagent-ai Date: Thu, 23 Jul 2026 21:01:43 +0800 Subject: [PATCH] Fix/audit review (#87) ### Issue for this PR Closes # ### Type of change - [ ] Bug fix - [ ] New feature - [ ] Refactor / code improvement - [ ] Documentation ### What does this PR do? Please provide a description of the issue, the changes you made to fix it, and why they work. It is expected that you understand why your changes work and if you do not understand why at least say as much so a maintainer knows how much to value the PR. **If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!** ### How did you verify your code works? ### Screenshots / recordings _If this is a UI change, please include a screenshot or recording._ ### Checklist - [ ] I have tested my changes locally - [ ] I have not included unrelated changes in this PR _If you do not follow this template your PR will be automatically rejected._ --------- 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 ++++++- packages/deepagent-code/src/provider/error.ts | 68 +-- .../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 +++- 22 files changed, 2366 insertions(+), 93 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/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, 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. + */} + +
+ +
- + ) }