From a4e3f27e42cd8f2d3ddcea30971291839a381b56 Mon Sep 17 00:00:00 2001 From: deepagent-ai Date: Sat, 11 Jul 2026 15:32:21 +0800 Subject: [PATCH 1/2] fix(goal): immediate goal.updated feedback + mode-aware start button (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goal Loop control changes updated in-memory state but emitted no immediate goal.updated event, so the client status bar never reflected them until the next tick (a full subagent turn). stop() was worst: it cancels the driver job, so no further tick ever fires and the UI stayed stuck on "running" forever. - goal-manager: extract publishGoalEvent + publishControlPhase; cache the live ledger/stall/gaps on GoalControl (updated each tick) so control transitions publish the real budget, not zeros. - start/pause/resume/stop now emit an immediate goal.updated. - finalizeOutcome guards the driver's terminal tap on a live control so a cancelled driver's late "needs_human" outcome cannot clobber the user's explicit "stopped". The "convert plan -> goal" button gated on session_plan, which only the plan tool populates — but loop/design modes author the plan as the repo file goal+plan.md, so the button never appeared in the modes it belongs to and did appear in auto (redundant/confusing). - New GoalManager.startable() mirrors start()'s plan precedence (session_plan -> goal+plan.md -> none) without side effects, flag-gated. - New GET /deepagent/goal/startable route + handler. - Button now gates on capability x mode in {loop,design} x startable, and shows a success toast on start. i18n goal.start.success (en/zh/zht). ### 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) --- .../deepagent/goal-start-button.tsx | 43 ++++- .../components/deepagent/panel-goal.api.ts | 23 +++ packages/app/src/i18n/en.ts | 1 + packages/app/src/i18n/zh.ts | 1 + packages/app/src/i18n/zht.ts | 1 + .../instance/httpapi/groups/deepagent.ts | 11 ++ .../instance/httpapi/handlers/deepagent.ts | 4 + .../src/session/goal-manager.ts | 174 ++++++++++++++---- 8 files changed, 215 insertions(+), 43 deletions(-) diff --git a/packages/app/src/components/deepagent/goal-start-button.tsx b/packages/app/src/components/deepagent/goal-start-button.tsx index 9a33b798..89a9bb9d 100644 --- a/packages/app/src/components/deepagent/goal-start-button.tsx +++ b/packages/app/src/components/deepagent/goal-start-button.tsx @@ -3,9 +3,17 @@ import { Button } from "@deepagent-code/ui/button" import { Icon } from "@deepagent-code/ui/icon" import { useServerSync } from "@/context/server-sync" import { useSDK } from "@/context/sdk" +import { useLocal } from "@/context/local" import { useLanguage } from "@/context/language" import { showToast } from "@/utils/toast" -import { fetchCapabilities, startGoal, type PanelGoalClient } from "./panel-goal.api" +import { fetchCapabilities, fetchGoalStartable, startGoal, type PanelGoalClient } from "./panel-goal.api" + +// The collaboration modes where "convert plan → supervised goal" makes sense. loop/design are BOTH +// powered by the Goal Loop engine and are designed to have the human START the loop after the plan is +// authored (loop: agent writes goal+plan.md; design: user writes it). auto is autonomous end-to-end in +// the current turn — a supervised background goal would be a confusing, redundant second door there, so +// the button must NOT appear in auto. plan is hidden and never the client-visible current mode. +const GOAL_MODES = new Set(["loop", "design"]) /** * V3.9 §D — "convert plan → goal" starter. @@ -34,6 +42,7 @@ function errorMessage(err: unknown): string { export function GoalStartButton(props: { sessionID: string }) { const sdk = useSDK() const serverSync = useServerSync() + const local = useLocal() const language = useLanguage() const [busy, setBusy] = createSignal(false) @@ -45,14 +54,31 @@ export function GoalStartButton(props: { sessionID: string }) { ) const goalAvailable = createMemo(() => capabilities()?.goalLoop === true) - const plan = createMemo(() => (props.sessionID ? serverSync.data.session_plan[props.sessionID] : undefined)) - const hasPlan = createMemo(() => (plan()?.steps.length ?? 0) > 0) + // The current collaboration mode (auto/loop/design) — the button only applies to loop/design. Sourced + // from local.agent.current() (session-scoped mode selection), the same source the mode selector uses. + const currentMode = createMemo(() => local.agent.current()?.name) + const modeAllows = createMemo(() => GOAL_MODES.has(currentMode() ?? "")) - // A goal is "live" for this session iff the persistent session_goal pointer exists and is not in a - // terminal phase the user has dismissed — while present, GoalStatusBar owns the surface. + // A goal is "live" for this session iff the persistent session_goal pointer exists — while present, + // GoalStatusBar owns the surface. Checked FIRST so we don't probe startability for an already-running + // goal. const activeGoal = createMemo(() => (props.sessionID ? serverSync.data.session_goal[props.sessionID] : undefined)) - const show = createMemo(() => goalAvailable() && hasPlan() && !activeGoal()) + // Whether a plan actually exists to start, resolved server-side (session_plan OR repo goal+plan.md). + // Re-fetched when the session, mode-eligibility, active-goal, or the in-session plan changes — the last + // dependency makes the button appear promptly after the agent writes a plan mid-conversation. Only + // probed when the mode allows and no goal is live, to avoid needless requests. + const [startable] = createResource( + () => + props.sessionID && goalAvailable() && modeAllows() && !activeGoal() + ? ([props.sessionID, serverSync.data.session_plan[props.sessionID]?.steps.length ?? 0] as const) + : undefined, + ([sessionID]) => fetchGoalStartable(client(), sessionID), + ) + + const show = createMemo( + () => goalAvailable() && modeAllows() && !activeGoal() && startable()?.startable === true, + ) const onStart = async () => { if (busy() || !props.sessionID) return @@ -61,8 +87,11 @@ export function GoalStartButton(props: { sessionID: string }) { const snapshot = await startGoal(client(), { sessionID: props.sessionID }) if (!snapshot) { showToast({ title: language.t("goal.start.failed") }) + } else { + // The server emits an immediate goal.updated (phase=running) on start, so GoalStatusBar takes + // over this surface right away. This toast is the belt-and-suspenders confirmation for the click. + showToast({ title: language.t("goal.start.success"), variant: "success" }) } - // On success the goal.updated event drives GoalStatusBar to appear; nothing to do here. } catch (err) { showToast({ title: language.t("goal.start.failed"), description: errorMessage(err) }) } finally { diff --git a/packages/app/src/components/deepagent/panel-goal.api.ts b/packages/app/src/components/deepagent/panel-goal.api.ts index 65c1f34a..86859e74 100644 --- a/packages/app/src/components/deepagent/panel-goal.api.ts +++ b/packages/app/src/components/deepagent/panel-goal.api.ts @@ -175,3 +175,26 @@ export const goalStatus = async ( }) return response.data?.goal ?? null } + +export type GoalStartable = { startable: boolean; source: "plan" | "file" | "none" } + +/** + * Whether a goal can be started for this session right now, resolved SERVER-SIDE with the same plan + * precedence start() uses (session_plan → repo goal+plan.md → none). The button gates on this instead + * of reading session_plan directly, because loop/design modes author the plan as the repo file (never + * touching session_plan), so a client-only hasPlan() check would hide the button in exactly the modes + * where it belongs. Tolerant of an older server that lacks the route (treated as not-startable). + */ +export const fetchGoalStartable = async ( + client: PanelGoalClient, + sessionID: string, +): Promise => { + const response = await client.client.request({ + method: "GET", + url: `/deepagent/goal/startable?sessionID=${encodeURIComponent(sessionID)}`, + }) + return { + startable: response.data?.startable ?? false, + source: response.data?.source ?? "none", + } +} diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 42a8f0fc..9485d9d6 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -932,6 +932,7 @@ export const dict = { "sidebar.wiki": "Repo & Wiki", "goal.start.hint": "Plan ready — run it as a supervised goal", "goal.start.button": "Run as goal", + "goal.start.success": "Goal started — running in the background", "goal.start.failed": "Couldn't start the goal", "wiki.title": "Repo & Wiki", "wiki.description": "Read, search, and govern the four graphs. Knowledge and Memory are editable; Documents and Code are read-only.", diff --git a/packages/app/src/i18n/zh.ts b/packages/app/src/i18n/zh.ts index c5ac0d8a..bac8c91c 100644 --- a/packages/app/src/i18n/zh.ts +++ b/packages/app/src/i18n/zh.ts @@ -793,6 +793,7 @@ export const dict = { "sidebar.wiki": "仓库与百科", "goal.start.hint": "计划已就绪 — 转为受监督的长跑目标", "goal.start.button": "转为 Goal", + "goal.start.success": "目标已启动，正在后台运行", "goal.start.failed": "无法启动目标", "wiki.title": "仓库与百科", "wiki.description": "阅读、检索、治理四张图。知识与记忆可编辑；文档与代码只读。", diff --git a/packages/app/src/i18n/zht.ts b/packages/app/src/i18n/zht.ts index 10a458a1..86831e26 100644 --- a/packages/app/src/i18n/zht.ts +++ b/packages/app/src/i18n/zht.ts @@ -646,6 +646,7 @@ export const dict = { "review.scope.global": "全域", "goal.start.hint": "計劃已就緒 — 轉為受監督的長跑目標", "goal.start.button": "轉為 Goal", + "goal.start.success": "目標已啟動，正在背景執行", "goal.start.failed": "無法啟動目標", "wiki.title": "倉庫與百科", "wiki.description": "閱讀、檢索、治理四張圖。知識與記憶可編輯；文件與程式碼唯讀。", diff --git a/packages/deepagent-code/src/server/routes/instance/httpapi/groups/deepagent.ts b/packages/deepagent-code/src/server/routes/instance/httpapi/groups/deepagent.ts index c8ff77c0..891d2776 100644 --- a/packages/deepagent-code/src/server/routes/instance/httpapi/groups/deepagent.ts +++ b/packages/deepagent-code/src/server/routes/instance/httpapi/groups/deepagent.ts @@ -322,6 +322,10 @@ export const DeepAgentGoalSnapshot = Schema.Struct({ }) export const DeepAgentGoalStatusResult = Schema.Struct({ goal: Schema.NullOr(DeepAgentGoalSnapshot) }) export const DeepAgentGoalMutateResult = Schema.Struct({ ok: Schema.Boolean }) +export const DeepAgentGoalStartableResult = Schema.Struct({ + startable: Schema.Boolean, + source: Schema.Literals(["plan", "file", "none"]), +}) // ── V3.9 §B Repo & Wiki ──────────────────────────────────────────────────── // The human-facing projection of the four graphs. Read-only browse + governed knowledge edit + @@ -650,6 +654,13 @@ export const DeepAgentApi = HttpApi.make("deepagent").add( error: DeepAgentPromotionError, }), ) + .add( + HttpApiEndpoint.get("goalStartable", `${root}/goal/startable`, { + query: Schema.Struct({ ...WorkspaceRoutingQueryFields, sessionID: Schema.String }), + success: described(DeepAgentGoalStartableResult, "Whether a goal can be started + plan source"), + error: DeepAgentPromotionError, + }), + ) .add( HttpApiEndpoint.get("wikiPages", `${root}/wiki/pages`, { query: Schema.Struct({ ...WorkspaceRoutingQueryFields, type: Schema.optional(Schema.String) }), diff --git a/packages/deepagent-code/src/server/routes/instance/httpapi/handlers/deepagent.ts b/packages/deepagent-code/src/server/routes/instance/httpapi/handlers/deepagent.ts index fea83b61..015bf57c 100644 --- a/packages/deepagent-code/src/server/routes/instance/httpapi/handlers/deepagent.ts +++ b/packages/deepagent-code/src/server/routes/instance/httpapi/handlers/deepagent.ts @@ -559,6 +559,9 @@ export const deepagentHandlers = HttpApiBuilder.group(InstanceHttpApi, "deepagen const goalStatus = Effect.fn("DeepAgentHttpApi.goalStatus")(function* (ctx) { return { goal: yield* goals.status(ctx.query.sessionID) } }) + const goalStartable = Effect.fn("DeepAgentHttpApi.goalStartable")(function* (ctx) { + return yield* goals.startable(ctx.query.sessionID) + }) // ── V3.9 §B Repo & Wiki ───────────────────────────────────────────────── // Read-only projection + governed knowledge edit + full-text search. All fail-closed on the wiki @@ -679,6 +682,7 @@ export const deepagentHandlers = HttpApiBuilder.group(InstanceHttpApi, "deepagen .handle("goalResume", goalResume) .handle("goalStop", goalStop) .handle("goalStatus", goalStatus) + .handle("goalStartable", goalStartable) .handle("wikiPages", wikiPages) .handle("wikiPage", wikiPage) .handle("wikiSearch", wikiSearch) diff --git a/packages/deepagent-code/src/session/goal-manager.ts b/packages/deepagent-code/src/session/goal-manager.ts index 3dc6d645..caecb130 100644 --- a/packages/deepagent-code/src/session/goal-manager.ts +++ b/packages/deepagent-code/src/session/goal-manager.ts @@ -72,6 +72,13 @@ type GoalControl = { jobId: string paused: boolean stopped: boolean + // Last-known observable status, cached so pause/resume/stop can publish an IMMEDIATE goal.updated that + // carries the real ledger (not zeros). Updated every tick by publishStatus. Without this, a control + // transition would either wait for the next tick (pause/resume — slow) or never publish at all (stop + // cancels the job, so no further tick fires), leaving the UI status bar stuck on the prior phase. + ledger: { ticks: number; tokens: number; cost: number; wallclockMs: number } + stallCount: number + gaps: readonly string[] } export type StartGoalInput = { @@ -98,12 +105,23 @@ export type GoalSnapshot = { readonly running: boolean } +// Whether a goal can be started for a session RIGHT NOW, and where its plan would come from. The client +// gates the "convert plan → goal" affordance on this instead of guessing from session_plan alone — +// session_plan is only populated by the plan TOOL, but loop/design modes author the plan as the repo +// file `.deepagent-code/plans/goal+plan.md`, which start() also accepts. `source` lets the UI phrase +// the action correctly (existing in-session plan vs the authored repo file). +export type GoalStartable = { + readonly startable: boolean + readonly source: "plan" | "file" | "none" +} + export interface Interface { readonly start: (input: StartGoalInput) => Effect.Effect readonly pause: (sessionID: string) => Effect.Effect readonly resume: (sessionID: string) => Effect.Effect readonly stop: (sessionID: string) => Effect.Effect readonly status: (sessionID: string) => Effect.Effect + readonly startable: (sessionID: string) => Effect.Effect } export class Service extends Context.Service()("@deepagent-code/GoalManager") {} @@ -186,29 +204,86 @@ export const layer = Layer.effect( return m }) - // Publish a status → both the goal.updated event and the session-state active-goal pointer. + // Low-level publisher: emit a goal.updated event over the SSE bridge. Best-effort (ignore) so a + // publish failure never crashes the caller (start route or background driver tick). + const publishGoalEvent = ( + sessionID: string, + payload: { + goalId: string + planDocId: string + phase: string + ledger: { ticks: number; tokens: number; cost: number; wallclockMs: number } + stallCount: number + gaps: readonly string[] + }, + ) => + events + .publish(GoalEvent.Updated, { + sessionID: SessionID.make(sessionID), + goalId: payload.goalId, + planDocId: payload.planDocId, + phase: payload.phase, + ledger: payload.ledger, + stallCount: payload.stallCount, + gaps: [...payload.gaps], + }) + .pipe(Effect.ignore) + + // Publish a driver status → the goal.updated event, the session-state active-goal pointer, AND the + // cached last-known status on the control (so control transitions can publish the real ledger). const publishStatus = (sessionID: string, status: GoalStatus) => Effect.gen(function* () { const phase = status.phase as string AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, phase as never) - yield* events - .publish(GoalEvent.Updated, { - sessionID: SessionID.make(sessionID), - goalId: status.goalId, - planDocId: status.planDocId, - phase, - ledger: { - ticks: status.ledger.ticks, - tokens: status.ledger.tokens, - cost: status.ledger.cost, - wallclockMs: status.ledger.wallclockMs, - }, - stallCount: status.stallCount, - gaps: status.gaps, - }) - .pipe(Effect.ignore) + const ledger = { + ticks: status.ledger.ticks, + tokens: status.ledger.tokens, + cost: status.ledger.cost, + wallclockMs: status.ledger.wallclockMs, + } + yield* mutateControl(sessionID, (ctrl) => { + ctrl.ledger = ledger + ctrl.stallCount = status.stallCount + ctrl.gaps = status.gaps + }) + yield* publishGoalEvent(sessionID, { + goalId: status.goalId, + planDocId: status.planDocId, + phase, + ledger, + stallCount: status.stallCount, + gaps: status.gaps, + }) + }) + + // Publish an IMMEDIATE goal.updated for a control transition (pause/resume/stop). Reuses the control's + // cached ledger/stall/gaps so the UI keeps its live budget readout while only the phase changes. + const publishControlPhase = (sessionID: string, control: GoalControl, phase: string) => + publishGoalEvent(sessionID, { + goalId: control.goalId, + planDocId: control.planDocId, + phase, + ledger: control.ledger, + stallCount: control.stallCount, + gaps: control.gaps, }) + // Reflect a driver's terminal outcome into the active-goal pointer — but ONLY if the goal is still + // controlled. A user `stop()` clears the control (setControl null) and has already set the pointer to + // "stopped" and published it; the cancelled driver may still settle with its own outcome (e.g. it + // observed shouldStop and returned "needs_human") whose late tap would otherwise clobber "stopped". + // Guarding on a live control makes the explicit stop authoritative and drops the racing outcome. + const finalizeOutcome = (sessionID: string, outcome: string) => + getControl(sessionID).pipe( + Effect.flatMap((c) => + Effect.sync(() => { + if (outcome !== "continue" && c != null) { + AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, outcome as never) + } + }), + ), + ) + const start: Interface["start"] = (input) => Effect.gen(function* () { const sessionID = input.sessionID @@ -299,6 +374,20 @@ export const layer = Layer.effect( startedAt: new Date().toISOString(), }) + // Emit an IMMEDIATE goal.updated (phase=running, empty ledger) BEFORE the first tick. The first + // driver tick is a full subagent turn (tens of seconds), and onStatus only fires AFTER it — so + // without this, the client's session_goal store stays empty and the "convert plan → goal" hint + // never flips to the GoalStatusBar, leaving the user with no confirmation the goal started. This + // seeds the store the moment start returns, so the UI reflects the running goal instantly. + yield* publishGoalEvent(sessionID, { + goalId: handle.goalId, + planDocId: handle.planDocId, + phase: "running", + ledger: { ticks: 0, tokens: 0, cost: 0, wallclockMs: 0 }, + stallCount: 0, + gaps: [], + }) + // The driver ports read the per-session control flags (pause/stop) live. const ports: GoalDriverPorts = { onStatus: (status) => publishStatus(sessionID, status), @@ -312,15 +401,9 @@ export const layer = Layer.effect( title: `goal ${handle.goalId}`, metadata: { sessionID, goalId: handle.goalId }, run: GoalDriver.runToCompletion({ deps, handle, ports }).pipe( - Effect.tap((outcome) => - Effect.sync(() => { - // A terminal outcome clears the running pointer to its terminal phase; a paused exit - // leaves the pointer for a later resume. - if (outcome !== "continue") { - AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, outcome as never) - } - }), - ), + // A terminal outcome clears the running pointer to its terminal phase (unless the user already + // stopped it); a paused exit ("continue") leaves the pointer for a later resume. + Effect.tap((outcome) => finalizeOutcome(sessionID, outcome)), Effect.map((outcome) => `goal ${handle.goalId}: ${outcome}`), Effect.catchCause(() => Effect.succeed(`goal ${handle.goalId}: driver defect`)), ), @@ -332,6 +415,9 @@ export const layer = Layer.effect( jobId: job.id, paused: false, stopped: false, + ledger: { ticks: 0, tokens: 0, cost: 0, wallclockMs: 0 }, + stallCount: 0, + gaps: [], }) return { goalId: handle.goalId, planDocId: handle.planDocId, phase: "running", running: true } @@ -343,6 +429,8 @@ export const layer = Layer.effect( if (!c) return false yield* mutateControl(sessionID, (ctrl) => (ctrl.paused = true)) AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, "paused") + // Immediate goal.updated so the status bar flips to "paused" now, not after the in-flight tick. + yield* publishControlPhase(sessionID, c, "paused") return true }) @@ -388,17 +476,15 @@ export const layer = Layer.effect( title: `goal ${c.goalId} (resumed)`, metadata: { sessionID, goalId: c.goalId }, run: GoalDriver.runToCompletion({ deps, handle, ports }).pipe( - Effect.tap((outcome) => - Effect.sync(() => { - if (outcome !== "continue") - AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, outcome as never) - }), - ), + Effect.tap((outcome) => finalizeOutcome(sessionID, outcome)), Effect.map((outcome) => `goal ${c.goalId}: ${outcome}`), Effect.catchCause(() => Effect.succeed(`goal ${c.goalId}: driver defect`)), ), }) yield* mutateControl(sessionID, (ctrl) => (ctrl.jobId = job.id)) + // Immediate goal.updated so the status bar flips back to "running" now. The resumed driver's + // first tick may be tens of seconds away; without this the bar would stay stuck on "paused". + yield* publishControlPhase(sessionID, c, "running") return true }) @@ -410,6 +496,10 @@ export const layer = Layer.effect( yield* background.cancel(c.jobId).pipe(Effect.ignore) AgentGateway.DeepAgentSessionState.setActiveGoalPhase(sessionID, "stopped") yield* setControl(sessionID, null) + // Immediate goal.updated is MANDATORY here: cancelling the job means no further tick will ever + // fire onStatus, so this is the ONLY event that can move the status bar off its prior phase. + // Without it the UI is stuck showing "running" forever after the user hits stop. + yield* publishControlPhase(sessionID, c, "stopped") return true }) @@ -426,11 +516,23 @@ export const layer = Layer.effect( } }) - // Reference `flags` so an unused-var lint stays quiet; the real gate is makeGoalLoopWiring returning - // null when experimentalGoalLoop is off (checked in start/resume). - void flags + // Whether start() would find a plan to run — mirrors its plan-resolution precedence WITHOUT side + // effects (no doc materialized, no driver started). Flag-gated: a disabled goal loop is never + // startable. session_plan (loop-tool authored) wins; else the repo goal+plan.md (loop/design file); + // else none. Used by the client to gate the convert-to-goal affordance in the modes where it applies. + const startable: Interface["startable"] = (sessionID) => + Effect.gen(function* () { + if (!flags.experimentalGoalLoop) return { startable: false, source: "none" as const } + const existing = AgentGateway.DeepAgentSessionState.getPlan(sessionID) as PlanDoc | null + if (existing != null) return { startable: true, source: "plan" as const } + const session = yield* sessions.get(SessionID.make(sessionID)).pipe(Effect.orDie) + const cwd = session.directory ?? process.cwd() + const fromFile = readGoalPlanFile(cwd, sessionID) + if (fromFile?.plan != null) return { startable: true, source: "file" as const } + return { startable: false, source: "none" as const } + }) - return Service.of({ start, pause, resume, stop, status }) + return Service.of({ start, pause, resume, stop, status, startable }) }), ) From d8470d24b08237768e88efb5a2adc9ad8149a3d5 Mon Sep 17 00:00:00 2001 From: deepagent-ai Date: Sat, 11 Jul 2026 17:31:20 +0800 Subject: [PATCH 2/2] V3.9.2 (#60) ### 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/core/src/deepagent/command-intent.ts | 291 ++++++++++++++++++ packages/core/src/deepagent/hooks.ts | 22 +- .../core/src/deepagent/plan-controller.ts | 52 +++- packages/core/src/deepagent/session-state.ts | 31 +- .../test/deepagent/command-intent.test.ts | 155 ++++++++++ .../test/deepagent/plan-gate-loop.test.ts | 76 ++++- .../deepagent-code/src/plugin/openai/codex.ts | 15 +- .../deepagent-code/src/provider/provider.ts | 13 + packages/deepagent-code/src/session/tools.ts | 40 ++- packages/deepagent-code/src/settings/store.ts | 24 +- .../deepagent-code/test/lib/seed-ripgrep.ts | 56 ++++ .../deepagent-code/test/plugin/codex.test.ts | 20 ++ packages/deepagent-code/test/preload.ts | 9 + .../test/provider/provider.test.ts | 25 ++ .../deepagent-code/test/session/llm.test.ts | 138 ++++----- .../test/settings/store.test.ts | 27 ++ .../deepagent-code/test/tool/skill.test.ts | 7 + 17 files changed, 895 insertions(+), 106 deletions(-) create mode 100644 packages/core/src/deepagent/command-intent.ts create mode 100644 packages/core/test/deepagent/command-intent.test.ts create mode 100644 packages/deepagent-code/test/lib/seed-ripgrep.ts diff --git a/packages/core/src/deepagent/command-intent.ts b/packages/core/src/deepagent/command-intent.ts new file mode 100644 index 00000000..aaee1070 --- /dev/null +++ b/packages/core/src/deepagent/command-intent.ts @@ -0,0 +1,291 @@ +export * as CommandIntent from "./command-intent" + +// Command intent classification for the Plan Gate (U1/U9). The gate soft/hard-blocks MUTATING tools +// while the plan latch is stale, but a shell command that only INSPECTS the world (ls, cat, grep, +// git status, curl probes, …) must never be blocked — those are the agent's eyes, and blocking them +// would make a stale plan impossible to diagnose and repair. This module decides whether a shell +// command string is provably read-only. +// +// FAIL-SAFE CONTRACT (load-bearing): this classifier is used to RELAX the gate, so it must never +// misclassify a mutating command as read-only. Any ambiguity — an unknown command, an unparseable +// segment, a redirection, a write-capable operator, elevated privilege — resolves to `mutating`. +// It is acceptable (merely slightly conservative) to classify a read-only command as mutating; it is +// NOT acceptable to classify a mutating command as read-only. Every branch below is written so the +// default answer is `mutating`. +// +// This is a pure, lexical analyzer (no tree-sitter): core has no shell-parser dependency, and a +// lexical pass is the right tool for a fail-safe gate — it cannot "successfully parse" a hostile +// command into a benign shape. deepagent-code's shell tool keeps its own tree-sitter path for +// permission scanning; this is deliberately independent and stricter. + +export type CommandIntent = "read_only" | "mutating" + +// Prefix → arity: how many leading tokens define the "command" for a read-only match. Flags between +// the command word and its subcommand are skipped by the matcher, so `git --no-pager status` still +// matches the `git status` (arity 2) entry. Only commands that CANNOT mutate the filesystem, process +// table, network state, or environment in a persistent way belong here. When in doubt, leave it out. +const READ_ONLY_PREFIXES: ReadonlyArray = [ + // ── filesystem inspection ── + ["ls"], + ["cat"], + ["bat"], + ["head"], + ["tail"], + ["wc"], + ["file"], + ["stat"], + ["du"], + ["df"], + ["tree"], + ["realpath"], + ["readlink"], + ["basename"], + ["dirname"], + ["pwd"], + ["find"], // read-only UNLESS it carries an action flag (guarded separately below) + // ── content search ── + ["grep"], + ["egrep"], + ["fgrep"], + ["rg"], + ["ag"], + ["ripgrep"], + // ── environment / introspection (query forms only) ── + ["which"], + ["whereis"], + ["type"], + ["echo"], + ["printf"], + ["date"], + ["whoami"], + ["id"], + ["hostname"], + ["uname"], + ["env"], // read-only ONLY with no assignment/command args (guarded below) + ["printenv"], + ["locale"], + ["uptime"], + ["ps"], + ["top"], + ["htop"], + ["free"], + ["df"], + ["lsof"], + ["jobs"], + ["history"], + // ── version / help probes ── + ["node", "--version"], + ["node", "-v"], + ["python", "--version"], + ["python3", "--version"], + ["go", "version"], + ["rustc", "--version"], + ["java", "-version"], + // ── git (read-only subcommands only) ── + ["git", "status"], + ["git", "log"], + ["git", "diff"], + ["git", "show"], + ["git", "branch"], + ["git", "tag"], + ["git", "remote"], + ["git", "rev-parse"], + ["git", "rev-list"], + ["git", "describe"], + ["git", "blame"], + ["git", "ls-files"], + ["git", "ls-remote"], + ["git", "cat-file"], + ["git", "config", "--get"], + ["git", "config", "--list"], + ["git", "config", "-l"], + ["git", "shortlog"], + ["git", "reflog"], + ["git", "whatchanged"], + // ── container / orchestration (query verbs only) ── + ["docker", "ps"], + ["docker", "images"], + ["docker", "logs"], + ["docker", "inspect"], + ["docker", "version"], + ["docker", "info"], + ["kubectl", "get"], + ["kubectl", "describe"], + ["kubectl", "logs"], + ["kubectl", "version"], + // ── package-manager query verbs ── + ["npm", "ls"], + ["npm", "list"], + ["npm", "view"], + ["npm", "outdated"], + ["pip", "list"], + ["pip", "show"], + ["pip", "freeze"], + ["cargo", "tree"], + ["brew", "list"], + ["brew", "info"], + // ── network probes (read-only unless they write a file; guarded below) ── + ["curl"], // mutating if it carries -o/-O/--output/--remote-name (guarded) +] + +// After fd-duplication spans (2>&1, 1>&2, >&2) are stripped, ANY remaining `>` is a file-writing +// redirection (> truncate, >> append, <> read-write), which makes the segment mutating. Input `<` +// alone is not mutating. We check for a bare `>` on the stripped text. + +// Command words that are inherently mutating no matter their flags. Fast reject before prefix match. +const MUTATING_COMMANDS = new Set([ + "rm", + "rmdir", + "mv", + "cp", + "dd", + "mkdir", + "touch", + "ln", + "chmod", + "chown", + "chgrp", + "truncate", + "shred", + "tee", + "install", + "sed", // sed alone is read-only, but `sed -i` mutates; treat as mutating unless we prove otherwise (guarded) + "kill", + "killall", + "pkill", + "reboot", + "shutdown", + "mkfs", + "mount", + "umount", + "export", + "unset", + "set", + "source", + ".", + "eval", + "exec", + "apt", + "apt-get", + "yum", + "dnf", + "pacman", + "systemctl", + "service", + "crontab", +]) + +// Extract the shell segments that run as their own command. We split on the operators that separate +// commands: && || ; | & (background) and newlines. This is intentionally coarse: a segment that +// contains anything we cannot prove read-only makes the WHOLE command mutating. +const SEGMENT_SPLIT = /(?:&&|\|\||[;\n|&])/ + +// Lexical tokenizer that keeps quoted spans intact (mirrors the tokenizer in tool/bash.ts). +const tokenize = (segment: string): string[] => segment.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) ?? [] + +const unquote = (value: string) => value.replace(/^(['"])(.*)\1$/, "$2") + +// A token is a "flag" if it begins with `-` (short/long option). +const isFlag = (token: string) => token.startsWith("-") + +// Does the segment's leading command match a read-only prefix? Returns the matched prefix length (in +// tokens consumed), or 0 if no read-only prefix matches. Intervening flags that are NOT part of the +// prefix are skipped, so `git --no-pager status` still resolves to `git status`. A flag that IS the +// next expected prefix component (e.g. the `--version` in `node --version`, the `--get` in +// `git config --get`) is matched normally rather than skipped. +const matchReadOnlyPrefix = (tokens: string[]): number => { + const words = tokens.map(unquote) + for (const prefix of READ_ONLY_PREFIXES) { + let matched = 0 + let tokenCursor = 0 + for (; tokenCursor < words.length && matched < prefix.length; tokenCursor++) { + const token = words[tokenCursor] + if (token === prefix[matched]) { + matched++ + continue + } + // Skip a non-matching flag only when the expected component is itself NOT a flag — otherwise a + // flag-typed prefix component (--version/--get) could be silently skipped past. + if (isFlag(token) && !isFlag(prefix[matched])) continue + break + } + if (matched === prefix.length) return tokenCursor + } + return 0 +} + +// `find` is read-only unless it carries an action predicate that executes or deletes. +const FIND_MUTATING_ACTIONS = new Set(["-delete", "-exec", "-execdir", "-ok", "-okdir", "-fprint", "-fprintf"]) + +// `env` is read-only only as a bare query (`env`); `env FOO=bar cmd` runs a command with a mutated +// environment, so anything past the command word makes it mutating. +const isReadOnlySegment = (segment: string): boolean => { + const trimmed = segment.trim() + if (trimmed === "") return true // empty segment (e.g. trailing operator) is inert + + // Any output redirection in the segment → mutating. Strip fd-duplication (2>&1, 1>&2, >&2) first, + // which is not a file write, then look for a real `>`. + const withoutFdDup = trimmed.replace(/\d*>&\d*/g, " ") + if (withoutFdDup.includes(">")) return false + + const tokens = tokenize(trimmed) + if (tokens.length === 0) return false + + // Command substitution / process substitution / backticks can smuggle an arbitrary command — we + // cannot prove those read-only lexically, so fail safe. + if (/\$\(|<\(|>\(|`/.test(trimmed)) return false + + const head = unquote(tokens[0]) + + // A leading `VAR=value` assignment prefix (env inline) means a command runs with a mutated env, or + // the assignment itself persists in the shell — fail safe. + if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(head)) return false + + // Inherently mutating command word → mutating. + if (MUTATING_COMMANDS.has(head)) return false + + const prefixLen = matchReadOnlyPrefix(tokens) + if (prefixLen === 0) return false // unknown/unlisted command → fail safe to mutating + + // Command-specific guards for read-only prefixes that have mutating variants. + const words = tokens.map(unquote) + if (head === "find" && words.some((token) => FIND_MUTATING_ACTIONS.has(token))) return false + if (head === "env") { + // Only a bare `env` (optionally with -i/-u flags but no command/assignment) is read-only. + const rest = words.slice(1).filter((token) => !isFlag(token)) + if (rest.length > 0) return false + } + if (head === "curl" && /\s-[oO]\b|--output\b|--remote-name\b/.test(trimmed)) return false + + return true +} + +/** + * Classify a shell command string as read-only or mutating for the Plan Gate. + * + * The command is split into segments on shell command separators; EVERY segment must be provably + * read-only for the whole command to be read-only. Any segment that is mutating, unknown, or + * unparseable makes the entire command `mutating` (fail-safe). + */ +export const classifyCommand = (command: string): CommandIntent => { + if (typeof command !== "string" || command.trim() === "") return "mutating" + // Mask fd-duplication spans (2>&1, 1>&2, >&2) before splitting so their internal `&` is not + // mistaken for a background/`&&` command separator. Spans are captured by index and restored + // exactly per-segment; the placeholder holds no separator or `>` char so it survives the split, + // and isReadOnlySegment strips fd-dup again defensively. + const fdSpans: string[] = [] + const masked = command.replace(/\d*>&\d*/g, (m) => { + const token = " fd" + fdSpans.length + "fd " + fdSpans.push(m) + return token + }) + const restore = (segment: string) => segment.replace(/ fd(\d+)fd /g, (_, idx) => fdSpans[Number(idx)] ?? "") + const segments = masked.split(SEGMENT_SPLIT) + for (const rawSegment of segments) { + const segment = restore(rawSegment) + if (!isReadOnlySegment(segment)) return "mutating" + } + return "read_only" +} + +export const isReadOnlyCommand = (command: string): boolean => classifyCommand(command) === "read_only" diff --git a/packages/core/src/deepagent/hooks.ts b/packages/core/src/deepagent/hooks.ts index 58e3116e..f137ea4b 100644 --- a/packages/core/src/deepagent/hooks.ts +++ b/packages/core/src/deepagent/hooks.ts @@ -68,15 +68,31 @@ export const stopHookGate = (): HookHandler => (e) => { // U9 hard gate (high+): even with a fresh plan, a mutating tool must be BOUND to an active step. // hardGateMissBlocks=true (xhigh/max/ultra) -> block on a missing active step; false (high) -> warn. // The payload carries hardGate, hasActiveStep, hardGateMissBlocks. +// +// U1 anti-deadlock (production fix): the soft layer additionally downgrades a would-be block to a +// WARN (tool executes, reminder attached) in two cases, so the gate can never permanently deny a +// model its tools: +// - staleReason === "user_appended": a new user message is a SOFT signal that the plan MIGHT no +// longer match intent — nudge the model to re-align, but do not hard-block work the user is +// actively asking for. +// - graceRelease === true: the gate has already blocked this stale plan N times with no forward +// progress (runtime-driven counter). Rather than loop forever against a model that will not +// repair the plan, release with a strong reminder. This is the direct fix for the observed +// deadlock (280 consecutive blocked bash calls). export const planGate = (): HookHandler => (e) => { if (e.name !== "before_tool_use") return { decision: "continue" } if (e.payload["isMutating"] !== true) return { decision: "allow" } // U1 soft layer: stale plan. if (e.payload["planStale"] === true) { const reason = "the plan is stale (reality changed); update the plan before further edits" - return e.payload["lightweight"] === true - ? { decision: "warn", blockReason: reason } - : { decision: "block", blockReason: reason } + const graceReason = + "the plan is stale and has not been updated after repeated attempts; this action is allowed to proceed, but you MUST call the `plan` tool to resync your plan with reality now" + const userAppendedReason = + "a new user message arrived; your plan may no longer match the request — review it and update the `plan` tool if the goal changed" + if (e.payload["lightweight"] === true) return { decision: "warn", blockReason: reason } + if (e.payload["graceRelease"] === true) return { decision: "warn", blockReason: graceReason } + if (e.payload["staleReason"] === "user_appended") return { decision: "warn", blockReason: userAppendedReason } + return { decision: "block", blockReason: reason } } // U9 hard layer: per-step binding (high+ only; lightweight never reaches here with hardGate set). if (e.payload["hardGate"] === true && e.payload["hasActiveStep"] !== true) { diff --git a/packages/core/src/deepagent/plan-controller.ts b/packages/core/src/deepagent/plan-controller.ts index 94789951..fa393706 100644 --- a/packages/core/src/deepagent/plan-controller.ts +++ b/packages/core/src/deepagent/plan-controller.ts @@ -11,6 +11,7 @@ // models whose self-reporting is unreliable. import { randomUUID } from "node:crypto" import type { AgentMode } from "./mode" +import { classifyCommand } from "./command-intent" // Why the plan went stale. Each value maps 1:1 to a signal the live loop already produces, so the // latch can be flipped from runtime truth without any model cooperation (S1 U1 §plan_stale 触发源). @@ -65,6 +66,12 @@ export type PlanLatchState = { readonly latch: PlanLatch readonly stale_reason: StaleReason | null readonly replan_count: number + // Runtime-driven grace counter (U1 anti-deadlock). Incremented every time the gate BLOCKS a + // mutating tool while the plan is stale; reset to 0 whenever forward progress happens (the plan is + // updated with a real status change, or a mutating tool actually executes). Unlike replan_count — + // which only advances when the MODEL cooperates by calling the plan tool — this advances from the + // runtime alone, so the grace release can fire even against a model that never repairs the plan. + readonly consecutive_blocks: number } export const initialPlanLatch = (planId: string | null = null): PlanLatchState => ({ @@ -72,6 +79,7 @@ export const initialPlanLatch = (planId: string | null = null): PlanLatchState = latch: "fresh", stale_reason: null, replan_count: 0, + consecutive_blocks: 0, }) // Flip to stale (idempotent on reason). Pure: the caller persists. The runtime calls this from the @@ -80,19 +88,43 @@ export const markStale = (state: PlanLatchState, reason: StaleReason): PlanLatch state.latch === "stale" && state.stale_reason === reason ? state : { ...state, latch: "stale", stale_reason: reason } // Clear the latch after the plan was updated. Bumps replan_count so the escape hatch can fire if the -// model thrashes (keeps producing a plan that immediately goes stale again). +// model thrashes (keeps producing a plan that immediately goes stale again). Also resets the runtime +// grace counter, since updating the plan is genuine forward progress. export const clearStale = (state: PlanLatchState): PlanLatchState => state.latch === "fresh" ? state - : { ...state, latch: "fresh", stale_reason: null, replan_count: state.replan_count + 1 } + : { ...state, latch: "fresh", stale_reason: null, replan_count: state.replan_count + 1, consecutive_blocks: 0 } + +// U1 anti-deadlock: record that the gate just BLOCKED a mutating tool on a stale plan. Advances the +// runtime grace counter. Pure; the caller persists. +export const recordGateBlock = (state: PlanLatchState): PlanLatchState => ({ + ...state, + consecutive_blocks: state.consecutive_blocks + 1, +}) + +// U1 anti-deadlock: forward progress happened (a mutating tool executed), so the model is not stuck +// hammering a blocked gate. Reset the grace counter without touching the latch/replan bookkeeping. +export const resetGateBlocks = (state: PlanLatchState): PlanLatchState => + state.consecutive_blocks === 0 ? state : { ...state, consecutive_blocks: 0 } // Escape hatch (mirrors the existing no-progress -> needs_human pattern): after too many replans we // STOP forcing the model to update the plan and hand off to a human, instead of looping forever on a -// model that can't produce a stable plan. Default limit 3 (S1 U1). +// model that can't produce a stable plan. Default limit 3 (S1 U1). This is the MODEL-cooperative +// hatch: it only advances when the model actually re-plans. export const DEFAULT_REPLAN_LIMIT = 3 export const shouldEscapeToHuman = (state: PlanLatchState, limit: number = DEFAULT_REPLAN_LIMIT): boolean => state.replan_count > limit +// U1 anti-deadlock grace release: after the gate has blocked the SAME stale plan this many times +// without any forward progress, stop blocking and let the next mutating tool through (with a strong +// reminder injected by the caller). This is the RUNTIME-driven hatch — it fires without any model +// cooperation, so a model that never repairs the plan (e.g. one that degrades to giving the user +// manual commands instead of calling the plan tool) can never be permanently denied its tools. This +// is the direct fix for the production deadlock where 280 consecutive bash calls were blocked. +export const DEFAULT_GRACE_BLOCK_LIMIT = 3 +export const shouldGraceRelease = (state: PlanLatchState, limit: number = DEFAULT_GRACE_BLOCK_LIMIT): boolean => + state.consecutive_blocks >= limit + // general/direct take the lightweight path: the soft gate WARNS but never blocks, so ordinary // implement/debug/chat tasks are never slowed by the plan machinery (docs/38 §9). high+ get the real // soft block. @@ -105,13 +137,19 @@ export const isLightweightMode = (mode: AgentMode | string): boolean => mode === const MUTATING_TOOLS = new Set(["edit", "write", "patch", "apply_patch", "multiedit"]) const ALWAYS_ALLOWED_TOOLS = new Set(["read", "grep", "glob", "list", "ls", "search", "task", "webfetch"]) -export const isMutatingTool = (toolName: string): boolean => { +export const isMutatingTool = (toolName: string, command?: string | null): boolean => { const name = toolName.toLowerCase() if (ALWAYS_ALLOWED_TOOLS.has(name)) return false if (MUTATING_TOOLS.has(name)) return true - // bash/shell is mutating UNLESS the caller already classified it read-only (docs/38 §7.2 read-only - // default for deterministic queries). The payload may carry readOnly:true for `git status` etc. - if (name === "bash" || name === "shell") return true + // bash/shell is mutating UNLESS the command is provably read-only (docs/38 §7.2 read-only default + // for deterministic queries). A read-only shell command (ls/cat/grep/git status/curl probe/…) is + // the agent's eyes: it must NEVER be blocked by the plan gate, otherwise a stale plan could never + // be diagnosed and repaired. The classifier is fail-safe — any ambiguity resolves to mutating — + // so a missing/empty command (nothing to classify) is treated as mutating. + if (name === "bash" || name === "shell") { + if (command == null || command.trim() === "") return true + return classifyCommand(command) === "mutating" + } return false } diff --git a/packages/core/src/deepagent/session-state.ts b/packages/core/src/deepagent/session-state.ts index b8fa9c98..885733bd 100644 --- a/packages/core/src/deepagent/session-state.ts +++ b/packages/core/src/deepagent/session-state.ts @@ -21,6 +21,8 @@ import { initialPlanLatch, markStale, clearStale, + recordGateBlock, + resetGateBlocks, planStatusesChanged, type PlanLatchState, type StaleReason, @@ -289,6 +291,26 @@ export const clearPlanStale = (sessionId: string): void => { saveToDisk() } +// U1 anti-deadlock: record that the plan gate just blocked a mutating tool on a stale plan. Advances +// the runtime grace counter so shouldGraceRelease can fire without the model cooperating. +export const recordPlanGateBlock = (sessionId: string): void => { + const state = sessions.get(sessionId) + if (!state) return + state.planLatch = recordGateBlock(state.planLatch) + saveToDisk() +} + +// U1 anti-deadlock: a mutating tool actually executed (forward progress), so reset the grace counter. +// No-op when already zero to avoid churning disk writes on the hot path. +export const resetPlanGateBlocks = (sessionId: string): void => { + const state = sessions.get(sessionId) + if (!state) return + const next = resetGateBlocks(state.planLatch) + if (next === state.planLatch) return + state.planLatch = next + saveToDisk() +} + export const planLatch = (sessionId: string): PlanLatchState | undefined => sessions.get(sessionId)?.planLatch export const complete = (sessionId: string): void => { @@ -358,8 +380,13 @@ function normalizeState(state: SessionRunState): SessionRunState { maxTotalTokens: nextBudget.maxTotalTokens, maxRounds: nextBudget.maxRounds, }, - // Backfill: sessions persisted before U1 have no planLatch/plan on disk. - planLatch: state.planLatch ?? initialPlanLatch(), + // Backfill: sessions persisted before U1 have no planLatch/plan on disk. Sessions persisted + // between U1 and the anti-deadlock change have a planLatch WITHOUT consecutive_blocks — backfill + // that field to 0 so shouldGraceRelease reads a defined counter (an undefined would make + // `>= limit` false forever, silently disabling the grace release for older sessions). + planLatch: state.planLatch + ? { ...state.planLatch, consecutive_blocks: state.planLatch.consecutive_blocks ?? 0 } + : initialPlanLatch(), plan: state.plan ?? null, // Backfill: sessions persisted before U10 have no counter on disk. mutationsSinceReport: state.mutationsSinceReport ?? 0, diff --git a/packages/core/test/deepagent/command-intent.test.ts b/packages/core/test/deepagent/command-intent.test.ts new file mode 100644 index 00000000..b6d192ec --- /dev/null +++ b/packages/core/test/deepagent/command-intent.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, test } from "bun:test" +import { classifyCommand, isReadOnlyCommand } from "../../src/deepagent/command-intent" + +// The classifier is used to RELAX the plan gate, so its contract is FAIL-SAFE: it must never call a +// mutating command read-only. These tests pin down both directions — the read-only allow-list AND +// the mutating fail-safe — so a future edit that loosens the classifier trips a test. + +describe("CommandIntent.classifyCommand", () => { + describe("read-only commands (must be exempt from the plan gate)", () => { + const readOnly = [ + "ls", + "ls -la", + "ls -la /some/path", + "cat file.txt", + "cat src/index.ts", + "head -n 20 log.txt", + "tail -f log.txt", + "wc -l file.txt", + "pwd", + "which node", + "echo hello world", + "grep -rn pattern src/", + "rg TODO packages/", + "find . -name '*.ts'", + "find src -type f", + "stat file.txt", + "file binary", + "du -sh .", + "df -h", + "env", + "printenv PATH", + "date", + "whoami", + "uname -a", + "ps aux", + "node --version", + "python3 --version", + "git status", + "git log --oneline -20", + "git diff HEAD~1", + "git show HEAD", + "git branch -a", + "git rev-parse HEAD", + "git ls-files", + "git config --get user.name", + "docker ps", + "docker images", + "kubectl get pods", + "npm ls", + "pip list", + "curl -sL https://example.com/api", + "curl https://example.com", + ] + for (const cmd of readOnly) { + test(`read_only: ${cmd}`, () => { + expect(classifyCommand(cmd)).toBe("read_only") + expect(isReadOnlyCommand(cmd)).toBe(true) + }) + } + }) + + describe("chained read-only commands (every segment must be read-only)", () => { + const readOnlyChains = [ + "ls -la && pwd", + "cat a.txt | grep foo", + "git status; git log --oneline -5", + "cd src && ls", // cd is CWD-only, not a file mutation — but see mutating list below + "curl -sL https://example.com/api | head -80", + "grep -rn foo . | wc -l", + ] + for (const cmd of readOnlyChains) { + test(`read_only chain: ${cmd}`, () => { + // NOTE: `cd` is intentionally NOT in the read-only allow-list (it changes shell state), so the + // "cd src && ls" case documents fail-safe behavior below rather than here. + if (cmd.startsWith("cd ")) return + expect(classifyCommand(cmd)).toBe("read_only") + }) + } + }) + + describe("mutating commands (must stay gated)", () => { + const mutating = [ + "rm file.txt", + "rm -rf /some/dir", + "mv a.txt b.txt", + "cp a.txt b.txt", + "mkdir -p foo/bar", + "touch newfile", + "chmod +x script.sh", + "chown user:group file", + "ln -s a b", + "dd if=/dev/zero of=out", + "sed -i 's/a/b/' file.txt", + "tee out.txt", + "kill 1234", + "git commit -m msg", + "git push origin main", + "git checkout -b feature", + "git reset --hard", + "npm install", + "pip install requests", + "docker run image", + "kubectl apply -f manifest.yaml", + "sudo ls", // privilege elevation → never read-only + "apt-get update", + "systemctl restart nginx", + "export FOO=bar", + "source ~/.bashrc", + ] + for (const cmd of mutating) { + test(`mutating: ${cmd}`, () => { + expect(classifyCommand(cmd)).toBe("mutating") + }) + } + }) + + describe("fail-safe: ambiguous / dangerous shapes resolve to mutating", () => { + const failSafe = [ + "", // empty + " ", // whitespace only + "cat file.txt > out.txt", // output redirection + "echo hi >> log.txt", // append redirection + "ls && rm file.txt", // one mutating segment poisons the whole command + "cat a.txt; rm b.txt", + "grep foo . | tee out.txt", // pipe into a writer + "find . -delete", // find with a mutating action + "find . -exec rm {} \\;", // find -exec + "curl -o out.zip https://example.com/f.zip", // curl writing a file + "curl -O https://example.com/f.zip", + "FOO=bar ls", // inline env assignment prefix + "env FOO=bar node app.js", // env running a command + "$(rm file.txt)", // command substitution + "cat `whoami`", // backticks + "ls <(rm x)", // process substitution + "unknowncmd --flag", // unknown command + "eval 'rm x'", + "exec rm x", + ] + for (const cmd of failSafe) { + test(`fail-safe mutating: ${JSON.stringify(cmd)}`, () => { + expect(classifyCommand(cmd)).toBe("mutating") + }) + } + }) + + describe("fd-duplication is not a file write", () => { + test("2>&1 does not count as an output redirection", () => { + expect(classifyCommand("grep foo file.txt 2>&1")).toBe("read_only") + expect(classifyCommand("ls -la 2>&1 | head")).toBe("read_only") + }) + test("but a real > alongside fd-dup is still mutating", () => { + expect(classifyCommand("grep foo file.txt 2>&1 > out.txt")).toBe("mutating") + }) + }) +}) diff --git a/packages/core/test/deepagent/plan-gate-loop.test.ts b/packages/core/test/deepagent/plan-gate-loop.test.ts index 2aedbdea..4930a2ec 100644 --- a/packages/core/test/deepagent/plan-gate-loop.test.ts +++ b/packages/core/test/deepagent/plan-gate-loop.test.ts @@ -13,20 +13,29 @@ import { planGate, HookPolicy } from "../../src/deepagent/hooks" const gate = new HookPolicy().on("before_tool_use", planGate()) -// Reproduce the chokepoint decision exactly (session/tools.ts). -const decideAt = (sessionId: string, toolId: string, mode: string) => { +// Reproduce the chokepoint decision exactly (session/tools.ts), including the U1 anti-deadlock +// payload fields (staleReason, graceRelease) and the shell command classification. +const decideAt = (sessionId: string, toolId: string, mode: string, command?: string) => { const latch = SessionState.planLatch(sessionId) const planStale = latch?.latch === "stale" && !PlanController.shouldEscapeToHuman(latch) return gate.evaluate({ name: "before_tool_use", payload: { planStale, - isMutating: PlanController.isMutatingTool(toolId), + staleReason: latch?.stale_reason ?? null, + graceRelease: latch != null && PlanController.shouldGraceRelease(latch), + isMutating: PlanController.isMutatingTool(toolId, command), lightweight: PlanController.isLightweightMode(mode), }, }) } +// Mirror the tools.ts block/reset bookkeeping so the loop tests advance the runtime grace counter +// exactly as production does: a block records a gate block; a mutating tool that runs resets it. +const applyOutcome = (sessionId: string, decision: string) => { + if (decision === "block") SessionState.recordPlanGateBlock(sessionId) +} + describe("U1 soft-gate loop (chokepoint contract)", () => { beforeEach(() => { SessionState.configure(mkdtempSync(path.join(tmpdir(), "plan-gate-"))) @@ -75,4 +84,65 @@ describe("U1 soft-gate loop (chokepoint contract)", () => { // gate no longer blocks (escape hatch) — the macro-round stop gate routes to needs_human instead expect(decideAt("gate-s3", "edit", "high").decision).toBe("allow") }) + + // ── U1 anti-deadlock: read-only shell commands are never gated ────────────────────────────────── + test("read-only bash (ls/git status/grep) is allowed even while the plan is stale", () => { + SessionState.getOrCreate("gate-ro", "xhigh") + SessionState.markPlanStale("gate-ro", "validation_failed") + // a mutating bash command IS gated… + expect(decideAt("gate-ro", "bash", "xhigh", "rm -rf build").decision).toBe("block") + // …but read-only shell inspections pass (the agent must be able to see to repair the plan). + expect(decideAt("gate-ro", "bash", "xhigh", "ls -la").decision).toBe("allow") + expect(decideAt("gate-ro", "bash", "xhigh", "git status").decision).toBe("allow") + expect(decideAt("gate-ro", "bash", "xhigh", "grep -rn TODO src/").decision).toBe("allow") + expect(decideAt("gate-ro", "bash", "xhigh", "cat file.txt | head -20").decision).toBe("allow") + }) + + // ── U1 anti-deadlock: runtime-driven grace release (the production-deadlock fix) ──────────────── + test("grace release: after repeated blocks with no re-plan, the gate releases WITHOUT model help", () => { + SessionState.getOrCreate("gate-grace", "xhigh") + SessionState.markPlanStale("gate-grace", "validation_failed") + + // The model keeps trying a mutating bash command and NEVER calls the plan tool (the exact + // production failure mode: 280 consecutive blocked bash calls). Each block advances the runtime + // grace counter — no setPlan required. + for (let i = 0; i < PlanController.DEFAULT_GRACE_BLOCK_LIMIT; i++) { + const decision = decideAt("gate-grace", "bash", "xhigh", "make build").decision + expect(decision).toBe("block") + applyOutcome("gate-grace", decision) + } + + // The escape-to-human hatch has NOT fired (replan_count never advanced — the model didn't + // cooperate), proving the old hatch alone would have deadlocked forever here. + expect(PlanController.shouldEscapeToHuman(SessionState.planLatch("gate-grace")!)).toBe(false) + + // But the runtime grace release fires: the next mutating call is downgraded to a WARN (tool runs, + // reminder attached) instead of a hard block. The agent is never permanently denied its tools. + expect(PlanController.shouldGraceRelease(SessionState.planLatch("gate-grace")!)).toBe(true) + expect(decideAt("gate-grace", "bash", "xhigh", "make build").decision).toBe("warn") + }) + + test("grace counter resets when forward progress happens (mutating tool executes)", () => { + SessionState.getOrCreate("gate-reset", "xhigh") + SessionState.markPlanStale("gate-reset", "validation_failed") + SessionState.recordPlanGateBlock("gate-reset") + SessionState.recordPlanGateBlock("gate-reset") + expect(SessionState.planLatch("gate-reset")!.consecutive_blocks).toBe(2) + // a mutating tool actually ran → reset + SessionState.resetPlanGateBlocks("gate-reset") + expect(SessionState.planLatch("gate-reset")!.consecutive_blocks).toBe(0) + expect(PlanController.shouldGraceRelease(SessionState.planLatch("gate-reset")!)).toBe(false) + }) + + // ── U1 anti-deadlock: a new user message must not hard-block work the user is asking for ──────── + test("user_appended stale reason warns (tool runs) rather than hard-blocking, even at xhigh", () => { + SessionState.getOrCreate("gate-user", "xhigh") + SessionState.markPlanStale("gate-user", "user_appended") + // mutating edit is downgraded to warn (runs + reminder), not blocked + expect(decideAt("gate-user", "edit", "xhigh").decision).toBe("warn") + // whereas a genuine reality-change signal still hard-blocks + SessionState.clearPlanStale("gate-user") + SessionState.markPlanStale("gate-user", "validation_failed") + expect(decideAt("gate-user", "edit", "xhigh").decision).toBe("block") + }) }) diff --git a/packages/deepagent-code/src/plugin/openai/codex.ts b/packages/deepagent-code/src/plugin/openai/codex.ts index ebe7d8e2..6e305b4f 100644 --- a/packages/deepagent-code/src/plugin/openai/codex.ts +++ b/packages/deepagent-code/src/plugin/openai/codex.ts @@ -401,11 +401,18 @@ export async function CodexAuthPlugin(input: PluginInput, options: CodexAuthPlug }, auth: { provider: "openai", - async loader(getAuth) { + async loader(getAuth, provider) { const auth = await getAuth() - const websocketFetch = options.experimentalWebSockets - ? OpenAIWebSocketPool.createWebSocketFetch({ httpFetch: fetch }) - : undefined + // A custom baseURL means the user is routing OpenAI through a proxy / self-hosted gateway / + // air-gapped mirror (set via SettingsStore, the official-provider override channel). Those + // endpoints speak plain HTTP, not the ChatGPT-backend WebSocket protocol, so the WS transport + // must be skipped even when it is otherwise enabled — otherwise every request would try to + // open a ws:// connection the proxy cannot answer. + const hasCustomBaseURL = typeof provider?.options?.baseURL === "string" && provider.options.baseURL !== "" + const websocketFetch = + options.experimentalWebSockets && !hasCustomBaseURL + ? OpenAIWebSocketPool.createWebSocketFetch({ httpFetch: fetch }) + : undefined if (websocketFetch) { websocketFetches.push(websocketFetch) websocketFetchInstalled = true diff --git a/packages/deepagent-code/src/provider/provider.ts b/packages/deepagent-code/src/provider/provider.ts index 1d869bbc..a3ccc23c 100644 --- a/packages/deepagent-code/src/provider/provider.ts +++ b/packages/deepagent-code/src/provider/provider.ts @@ -1309,6 +1309,19 @@ export const layer = Layer.effect( // via `provider.` in config (credentials from options.apiKey/env). const officialProviderIDs = OFFICIAL_PROVIDER_ID_SET + // Seed the official-provider baseURL override (SettingsStore, the sanctioned channel for + // official transport) onto the CATALOG entry up front — before the plugin auth loaders and + // custom loaders run. This makes the endpoint visible to (a) the OpenAI codex auth loader, so + // it can skip the WebSocket transport when a proxy/gateway baseURL is set (a proxy speaks + // HTTP, not the ChatGPT-backend WS protocol), and (b) resolveSDK, which reads + // `options.baseURL`. The timeout/retry transport keys are merged later (they don't affect the + // loader path); only baseURL needs to be early. + for (const [id, transport] of Object.entries(officialTransport)) { + const entry = database[id] + if (!entry || !transport?.baseURL) continue + entry.options = { ...entry.options, baseURL: transport.baseURL } + } + const providers: Record = {} as Record const languages = new Map() const modelLoaders: { diff --git a/packages/deepagent-code/src/session/tools.ts b/packages/deepagent-code/src/session/tools.ts index 14176cc1..64119b6f 100644 --- a/packages/deepagent-code/src/session/tools.ts +++ b/packages/deepagent-code/src/session/tools.ts @@ -152,11 +152,26 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { // plan step. high warns + auto-replans; xhigh/max/ultra hard-block. const hardGate = !lightweight && AgentGateway.DeepAgentPlanController.hardGateEnabled(agentMode) const plan = AgentGateway.DeepAgentSessionState.getPlan(ctx.sessionID) + // Read-only shell commands (ls/cat/grep/git status/curl probe/…) are the agent's eyes and + // must never be gated — pass the command string so the classifier can exempt them. Any + // ambiguity is classified mutating (fail-safe). Non-shell tools ignore the command arg. + const command = + (item.id === "bash" || item.id === "shell") && + typeof (args as { command?: unknown } | undefined)?.command === "string" + ? ((args as { command: string }).command) + : null + const isMutating = AgentGateway.DeepAgentPlanController.isMutatingTool(item.id, command) + // U1 anti-deadlock: runtime-driven grace release. When the gate has already blocked this + // stale plan too many times with no forward progress, release (with a strong reminder) + // rather than deadlock a model that will not repair the plan. + const graceRelease = latch != null && AgentGateway.DeepAgentPlanController.shouldGraceRelease(latch) const gateDecision = PlanHook.evaluate({ name: "before_tool_use", payload: { planStale, - isMutating: AgentGateway.DeepAgentPlanController.isMutatingTool(item.id), + staleReason: latch?.stale_reason ?? null, + graceRelease, + isMutating, lightweight, hardGate, hasActiveStep: AgentGateway.DeepAgentPlanController.hasActiveStep(plan), @@ -164,21 +179,36 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { }, }) if (gateDecision.decision === "block") { + // U1 anti-deadlock: count this block on the runtime counter so shouldGraceRelease can + // eventually fire even if the model never calls the plan tool. + AgentGateway.DeepAgentSessionState.recordPlanGateBlock(ctx.sessionID) const reason = latch?.stale_reason != null ? `The plan is stale (${latch.stale_reason}). ${gateDecision.blockReason}. Call the \`plan\` tool to update your plan, then retry this edit.` : `${gateDecision.blockReason}. Call the \`plan\` tool first.` return { title: "Plan update required", output: reason, metadata: {} } } + // A gate WARN (grace release, user_appended, or a high-mode hard-gate binding miss) lets + // the tool run but must surface the reminder to the model. Suppressed for lightweight + // modes (general/direct), which are intentionally never slowed by the plan machinery. + const gateWarnReason = + gateDecision.decision === "warn" && !lightweight ? gateDecision.blockReason : undefined const result = yield* item.execute(args, ctx) // U10: count a successful mutating tool call toward the progress-nudge budget. Only - // mutating tools (edit/write/patch/shell) count; the counter resets when the model next - // changes a plan step's status. No-op when there is no plan. - if (AgentGateway.DeepAgentPlanController.isMutatingTool(item.id)) { + // mutating tools (edit/write/patch/shell-mutation) count; the counter resets when the model + // next changes a plan step's status. No-op when there is no plan. + if (isMutating) { AgentGateway.DeepAgentSessionState.recordMutation(ctx.sessionID) + // U1 anti-deadlock: a mutating tool actually executed → forward progress happened, so + // reset the consecutive-block counter (block cadence restarts from zero). + AgentGateway.DeepAgentSessionState.resetPlanGateBlocks(ctx.sessionID) } + const withReminder = + gateWarnReason && typeof result.output === "string" + ? { ...result, output: `⚠️ Plan gate: ${gateWarnReason}.\n\n${result.output}` } + : result const output = { - ...result, + ...withReminder, attachments: result.attachments?.map((attachment) => ({ ...attachment, id: PartID.ascending(), diff --git a/packages/deepagent-code/src/settings/store.ts b/packages/deepagent-code/src/settings/store.ts index 79e9aa7e..f972c121 100644 --- a/packages/deepagent-code/src/settings/store.ts +++ b/packages/deepagent-code/src/settings/store.ts @@ -43,8 +43,14 @@ export namespace SettingsStore { } /** Transport tuning for a single official provider. Mirrors the transport keys the provider - * loader strips from `options` and applies as fetch-level abort controllers. */ + * loader strips from `options` and applies as fetch-level abort controllers, PLUS an optional + * `baseURL` endpoint override. Official providers deliberately ignore `config.provider.`, so a + * corporate proxy / self-hosted gateway / air-gapped mirror in front of an official provider can + * ONLY be pointed to here (the connect dialog's advanced section) — never via the config file. + * When `baseURL` is set, the OpenAI codex WebSocket transport is skipped (a proxy speaks HTTP, not + * the ChatGPT-backend WS protocol); see plugin/openai/codex.ts. */ export interface TransportSettings { + baseURL?: string headerTimeout?: number | false chunkTimeout?: number timeout?: number | false @@ -112,9 +118,25 @@ export namespace SettingsStore { return Object.keys(out).length > 0 ? out : undefined } + // Accept only a well-formed absolute http(s) endpoint. A malformed or non-http value is dropped + // (not persisted) so a hand-edited settings file can never route an official provider somewhere + // unexpected — the provider then falls back to its fixed catalog endpoint. + const httpUrl = (v: unknown): string | undefined => { + const s = str(v) + if (!s) return undefined + try { + const u = new URL(s) + return u.protocol === "http:" || u.protocol === "https:" ? s : undefined + } catch { + return undefined + } + } + function normalizeTransport(input: unknown): TransportSettings | undefined { if (!isRecord(input)) return undefined const out: TransportSettings = {} + const bu = httpUrl(input.baseURL) + if (bu !== undefined) out.baseURL = bu const ht = timeout(input.headerTimeout) if (ht !== undefined) out.headerTimeout = ht const ct = posInt(input.chunkTimeout) diff --git a/packages/deepagent-code/test/lib/seed-ripgrep.ts b/packages/deepagent-code/test/lib/seed-ripgrep.ts new file mode 100644 index 00000000..45bd335a --- /dev/null +++ b/packages/deepagent-code/test/lib/seed-ripgrep.ts @@ -0,0 +1,56 @@ +import os from "os" +import path from "path" +import fs from "fs/promises" + +// Seed the ripgrep binary into a test home's cache so search-backed tools (grep/glob/shell/skill, +// and the tool-registry construction that eagerly resolves `rg`) never trigger the ~50s GitHub +// download that otherwise blows past every per-test timeout and makes those tests appear to hang. +// +// The tool resolves `rg` from `/.deepagent/code/cache/bin` (see core global-path.ts: +// resolveDataPath → DEEPAGENT_CODE_TEST_HOME/.deepagent/code, then Global.Path.bin = /cache/bin). +// Any test that repoints DEEPAGENT_CODE_TEST_HOME to a fresh dir must re-seed, or its rg resolution +// falls through to the network download. This helper is idempotent and best-effort: if no local `rg` +// is available (clean CI image with no ripgrep anywhere), it does nothing and the tool keeps its own +// download fallback — behavior is unchanged, just not pre-seeded. + +let cachedSource: string | null | undefined + +const rgName = () => (process.platform === "win32" ? "rg.exe" : "rg") + +const isFile = (p: string) => + fs + .stat(p) + .then((s) => s.isFile()) + .catch(() => false) + +// Locate a reusable `rg` binary: first a real one on PATH (shell-function shims are not files and are +// skipped), then the developer's real-home cache where a prior non-test run already downloaded it. +const findLocalRg = async (): Promise => { + if (cachedSource !== undefined) return cachedSource + const name = rgName() + for (const base of (process.env.PATH ?? "").split(path.delimiter)) { + if (!base) continue + const candidate = path.join(base, name) + if (await isFile(candidate)) return (cachedSource = candidate) + } + const realHomeRg = path.join(os.homedir(), ".deepagent", "code", "cache", "bin", name) + if (await isFile(realHomeRg)) return (cachedSource = realHomeRg) + return (cachedSource = null) +} + +/** + * Copy a local `rg` into `/.deepagent/code/cache/bin`. Idempotent (no-op if already seeded), + * best-effort (no throw). Call from preload for the default test home, and from any test that + * repoints DEEPAGENT_CODE_TEST_HOME to a fresh directory. + */ +export const seedRipgrep = async (homeDir: string): Promise => { + const name = rgName() + const binDir = path.join(homeDir, ".deepagent", "code", "cache", "bin") + const target = path.join(binDir, name) + if (await isFile(target)) return + const source = await findLocalRg() + if (!source) return + await fs.mkdir(binDir, { recursive: true }).catch(() => undefined) + await fs.copyFile(source, target).catch(() => undefined) + if (process.platform !== "win32") await fs.chmod(target, 0o755).catch(() => undefined) +} diff --git a/packages/deepagent-code/test/plugin/codex.test.ts b/packages/deepagent-code/test/plugin/codex.test.ts index f7097539..1c85bc00 100644 --- a/packages/deepagent-code/test/plugin/codex.test.ts +++ b/packages/deepagent-code/test/plugin/codex.test.ts @@ -140,6 +140,26 @@ describe("plugin.codex", () => { await enabled.dispose?.() }) + test("skips the websocket transport when a custom baseURL is set (proxy/gateway routing)", async () => { + // Even with WS enabled, a custom baseURL means OpenAI is routed through a proxy / self-hosted + // gateway that speaks plain HTTP — the ChatGPT-backend WS protocol would not work there, so the + // loader must fall back to HTTP and not install a ws fetch. + const enabled = await CodexAuthPlugin({} as never, { experimentalWebSockets: true }) + + const withBaseURL = await enabled.auth!.loader!( + async () => ({ type: "api", key: "sk-test" }) as never, + { options: { baseURL: "https://proxy.corp.internal/openai/v1" } } as never, + ) + const withoutBaseURL = await enabled.auth!.loader!( + async () => ({ type: "api", key: "sk-test" }) as never, + { options: {} } as never, + ) + + expect(withBaseURL.fetch).toBeUndefined() + expect(withoutBaseURL.fetch).toBeFunction() + await enabled.dispose?.() + }) + test("deduplicates concurrent Codex token refreshes", async () => { let auth = { type: "oauth" as const, diff --git a/packages/deepagent-code/test/preload.ts b/packages/deepagent-code/test/preload.ts index 6c1177a2..4e714c09 100644 --- a/packages/deepagent-code/test/preload.ts +++ b/packages/deepagent-code/test/preload.ts @@ -68,6 +68,15 @@ const testHome = path.join(dir, "home") await fs.mkdir(testHome, { recursive: true }) process.env["DEEPAGENT_CODE_TEST_HOME"] = testHome +// Seed the ripgrep binary into the isolated test cache. The grep/glob/shell/skill tools (and the +// tool-registry construction that eagerly resolves `rg`) look it up under +// `/.deepagent/code/cache/bin` and, when absent, download it from GitHub releases — a ~50s +// transfer that blows past every per-test timeout and makes search-backed tests appear to hang. +// Seeding it up front makes those tests deterministic and offline. Tests that repoint +// DEEPAGENT_CODE_TEST_HOME to a fresh dir re-seed via the same helper. +const { seedRipgrep } = await import("./lib/seed-ripgrep") +await seedRipgrep(testHome) + // Set test managed config directory to isolate tests from system managed settings const testManagedConfigDir = path.join(dir, "managed") process.env["DEEPAGENT_CODE_TEST_MANAGED_CONFIG_DIR"] = testManagedConfigDir diff --git a/packages/deepagent-code/test/provider/provider.test.ts b/packages/deepagent-code/test/provider/provider.test.ts index 9a7356bf..fa946b3c 100644 --- a/packages/deepagent-code/test/provider/provider.test.ts +++ b/packages/deepagent-code/test/provider/provider.test.ts @@ -11,6 +11,7 @@ import { disposeAllInstances, provideInstanceEffect, tmpdirScoped, TestInstance import { markPluginDependenciesReady } from "../fixture/plugin" import { Auth } from "@/auth" import { Config } from "@/config/config" +import { SettingsStore } from "@/settings/store" import { Env } from "../../src/env" import { Plugin } from "../../src/plugin/index" import { Provider } from "@/provider/provider" @@ -60,6 +61,10 @@ afterEach(async () => { else process.env[key] = value } originalEnv.clear() + // Reset any SettingsStore official-transport override written by a test so it can't leak into the + // next (the store is backed by a shared global file + in-memory cache). + await SettingsStore.update({ providers: { openai: {}, anthropic: {} } }).catch(() => {}) + SettingsStore.invalidate() await disposeAllInstances() }) @@ -211,6 +216,26 @@ it.instance( }, ) +it.instance( + "official provider baseURL override comes from SettingsStore (the sanctioned channel)", + Effect.gen(function* () { + // Config baseURL is ignored for official providers (proven above). The ONE supported way to point + // an official provider at a proxy / self-hosted gateway / air-gapped mirror is SettingsStore + // (the connect dialog's advanced section). Written before the provider state materializes, it must + // land on the resolved provider's options so resolveSDK routes there. + yield* Effect.promise(() => + SettingsStore.update({ providers: { openai: { baseURL: "https://proxy.corp.internal/openai/v1" } } }), + ) + SettingsStore.invalidate() + yield* connect(ProviderV2.ID.openai, "test-openai-key") + const providers = yield* list + const provider = providers[ProviderV2.ID.openai] + expect(provider).toBeDefined() + expect(provider.source).toBe("api") + expect(provider.options.baseURL).toBe("https://proxy.corp.internal/openai/v1") + }), +) + it.instance( "non-official catalog id is a third-party provider from config", Effect.gen(function* () { diff --git a/packages/deepagent-code/test/session/llm.test.ts b/packages/deepagent-code/test/session/llm.test.ts index 86cc8c87..a66779b5 100644 --- a/packages/deepagent-code/test/session/llm.test.ts +++ b/packages/deepagent-code/test/session/llm.test.ts @@ -14,6 +14,7 @@ import { Auth } from "@/auth" import { Config } from "@/config/config" import { Provider } from "@/provider/provider" import { ProviderTransform } from "@/provider/transform" +import { SettingsStore } from "@/settings/store" import { ModelsDev } from "@deepagent-code/core/models-dev" import { Plugin } from "@/plugin" @@ -30,29 +31,29 @@ import { ModelV2 } from "@deepagent-code/core/model" type ConfigModel = NonNullable[string]["models"]>[string] -const openAIConfig = (model: ModelsDev.Provider["models"][string], baseURL: string): Partial => { - const { experimental: _experimental, ...configModel } = model - return { - enabled_providers: ["openai"], - provider: { - openai: { - name: "OpenAI", - env: ["OPENAI_API_KEY"], - npm: "@ai-sdk/openai", - api: "https://api.openai.com/v1", - models: { - [model.id]: JSON.parse(JSON.stringify(configModel)) as ConfigModel, - }, - options: { - apiKey: "test-openai-key", - baseURL, - }, - }, - }, - } -} -const it = testEffect(Layer.mergeAll(LLM.defaultLayer, Provider.defaultLayer)) +const it = testEffect(Layer.mergeAll(LLM.defaultLayer, Provider.defaultLayer, Auth.defaultLayer)) + +// Register an OFFICIAL provider (openai/anthropic/google/…) for a test and resolve one of its models +// pointed at the mock server, using the SAME channels a real deployment uses. Post-"官方 provider +// 收窄+第三方隔离" (6065b22), official providers no longer read `config.provider.`: they register +// ONLY from the auth key store, and their endpoint can be overridden ONLY via SettingsStore (the +// connect dialog's advanced section — e.g. a corporate proxy / self-hosted gateway). So we: +// 1. seed the auth key store → the provider registers, +// 2. write the SettingsStore baseURL override → resolveSDK routes to the mock (and the OpenAI codex +// WS transport is skipped because a custom baseURL is present). +// Both writes happen before the first `getModel`, i.e. before the provider InstanceState +// materializes, so they are picked up. SettingsStore is global-file backed and cached, so invalidate +// to be safe against a prior test's read. Cleanup (auth/settings reset) is handled by the fresh +// per-test instance home (withTmpdirInstance) + the afterEach below. +const resolveOfficialModelAt = (providerID: string, modelID: string, baseURL: string) => + Effect.gen(function* () { + const auth = yield* Auth.Service + yield* auth.set(providerID, { type: "api", key: `test-${providerID}-key` }) + SettingsStore.invalidate() + yield* Effect.promise(() => SettingsStore.update({ providers: { [providerID]: { baseURL } } })) + return yield* Provider.use.getModel(ProviderV2.ID.make(providerID), ModelV2.ID.make(modelID)) + }) // LLM.stream returns a Stream, not an Effect, so we can't use the serviceUse proxy. const drain = (input: LLM.StreamInput) => LLM.Service.use((svc) => svc.stream(input).pipe(Stream.runDrain)) @@ -663,6 +664,9 @@ beforeAll(() => { beforeEach(() => { state.queue.length = 0 + // SettingsStore is backed by a shared global file + in-memory cache; drop the cache so a baseURL + // override written by resolveOfficialModelAt in one test never leaks into the next. + SettingsStore.invalidate() }) afterAll(() => { @@ -1014,7 +1018,7 @@ describe("session.llm.stream", () => { ] const request = waitRequest("/responses", createEventResponse(responseChunks, true)) - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, `${state.server!.url.origin}/v1`) const sessionID = SessionID.make("session-test-2") const agent = { name: "test", @@ -1054,7 +1058,7 @@ describe("session.llm.stream", () => { const maxTokens = body.max_output_tokens as number | undefined expect(maxTokens).toBe(undefined) // match codex cli behavior }), - { config: () => openAIConfig(loadFixture("openai", "gpt-5.2").model, `${state.server!.url.origin}/v1`) }, + { config: () => ({ enabled_providers: ["openai"] }) }, ) it.instance( @@ -1119,7 +1123,7 @@ describe("session.llm.stream", () => { }), ) - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, `${state.server!.url.origin}/v1`) const sessionID = SessionID.make("session-test-native-flag-off") const agent = { name: "test", @@ -1159,7 +1163,7 @@ describe("session.llm.stream", () => { expect(capture.url.pathname.endsWith("/responses")).toBe(true) expect(capture.body.model).toBe(resolved.api.id) }), - { config: () => openAIConfig(loadFixture("openai", "gpt-5.2").model, `${state.server!.url.origin}/v1`) }, + { config: () => ({ enabled_providers: ["openai"] }) }, ) it.instance( @@ -1189,7 +1193,7 @@ describe("session.llm.stream", () => { ] const request = waitRequest("/responses", createEventResponse(chunks, true)) - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, `${state.server!.url.origin}/v1`) const sessionID = SessionID.make("session-test-native") const agent = { name: "test", @@ -1226,7 +1230,7 @@ describe("session.llm.stream", () => { expect(JSON.stringify(capture.body.input)).toContain("You are a helpful assistant.") expect(capture.body.input).toContainEqual({ role: "user", content: [{ type: "input_text", text: "Hello" }] }) }), - { config: () => openAIConfig(loadFixture("openai", "gpt-5.2").model, `${state.server!.url.origin}/v1`) }, + { config: () => ({ enabled_providers: ["openai"] }) }, ) it.instance( @@ -1273,7 +1277,7 @@ describe("session.llm.stream", () => { }), ) - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, "https://injected-openai.test/v1") const sessionID = SessionID.make("session-test-native-injected-tool") const agent = { name: "test", @@ -1325,7 +1329,7 @@ describe("session.llm.stream", () => { ]) expect(executed).toEqual({ args: { query: "weather" }, toolCallId: "call-injected-tool" }) }), - { config: () => openAIConfig(loadFixture("openai", "gpt-5.2").model, "https://injected-openai.test/v1") }, + { config: () => ({ enabled_providers: ["openai"] }) }, ) it.instance( @@ -1361,7 +1365,7 @@ describe("session.llm.stream", () => { const request = waitRequest("/responses", createEventResponse(chunks, true)) let executed: unknown - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, `${state.server!.url.origin}/v1`) const sessionID = SessionID.make("session-test-native-tool") const agent = { name: "test", @@ -1414,22 +1418,7 @@ describe("session.llm.stream", () => { expect(executed).toEqual({ args: { query: "weather" }, toolCallId: "call-native-tool" }) }), { - config: () => { - const model = loadFixture("openai", "gpt-5.2").model - return { - enabled_providers: ["openai"], - provider: { - openai: { - name: "OpenAI", - env: ["OPENAI_API_KEY"], - npm: "@ai-sdk/openai", - api: "https://api.openai.com/v1", - models: { [model.id]: JSON.parse(JSON.stringify(model)) as ConfigModel }, - options: { apiKey: "test-openai-key", baseURL: `${state.server!.url.origin}/v1` }, - }, - }, - } - }, + config: () => ({ enabled_providers: ["openai"] }), }, ) @@ -1487,7 +1476,7 @@ describe("session.llm.stream", () => { ), ).toString("base64")}` - const resolved = yield* Provider.use.getModel(ProviderV2.ID.openai, ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt("openai", model.id, `${state.server!.url.origin}/v1`) const sessionID = SessionID.make("session-test-data-url") const agent = { name: "test", @@ -1526,7 +1515,7 @@ describe("session.llm.stream", () => { const capture = yield* Effect.promise(() => request) expect(capture.url.pathname.endsWith("/responses")).toBe(true) }), - { config: () => openAIConfig(loadFixture("openai", "gpt-5.2").model, `${state.server!.url.origin}/v1`) }, + { config: () => ({ enabled_providers: ["openai"] }) }, ) const minimaxFixture = { providerID: "minimax", modelID: "MiniMax-M2.5" } @@ -1671,7 +1660,11 @@ describe("session.llm.stream", () => { ] const request = waitRequest("/messages", createEventResponse(chunks)) - const resolved = yield* Provider.use.getModel(ProviderV2.ID.make("anthropic"), ModelV2.ID.make(model.id)) + const resolved = yield* resolveOfficialModelAt( + "anthropic", + model.id, + `${state.server!.url.origin}/v1`, + ) const sessionID = SessionID.make("session-test-anthropic-tools") const agent = { name: "test", @@ -1686,6 +1679,11 @@ describe("session.llm.stream", () => { time: { created: Date.now() }, agent: agent.name, model: { providerID: ProviderV2.ID.make("anthropic"), modelID: resolved.id, variant: "max" }, + // Pin the plain (non-DeepAgent) mode: this test asserts the exact provider request payload + // (message ordering, tool_use/tool_result adjacency), so the DeepAgent orchestrator's + // volatile round-context message must not be injected. The gateway is active by default in + // tests (agentMode undefined ≠ "general"), so pin general to exercise the raw provider path. + metadata: { deepagent: { agent_mode_override: "general" } }, } satisfies SessionV1.User const input = [ @@ -1834,24 +1832,7 @@ describe("session.llm.stream", () => { ], }) }), - { - config: () => { - const model = loadFixture("anthropic", "claude-opus-4-6").model - return { - enabled_providers: ["anthropic"], - provider: { - anthropic: { - name: "Anthropic", - env: ["ANTHROPIC_API_KEY"], - npm: "@ai-sdk/anthropic", - api: "https://api.anthropic.com/v1", - models: { [model.id]: configModel(model) as ConfigModel }, - options: { apiKey: "test-anthropic-key", baseURL: `${state.server!.url.origin}/v1` }, - }, - }, - } - }, - }, + { config: () => ({ enabled_providers: ["anthropic"] }) }, ) const geminiFixture = { providerID: "google", modelID: "gemini-2.5-flash" } @@ -1870,9 +1851,10 @@ describe("session.llm.stream", () => { ] const request = waitRequest(pathSuffix, createEventResponse(chunks)) - const resolved = yield* Provider.use.getModel( - ProviderV2.ID.make(geminiFixture.providerID), - ModelV2.ID.make(model.id), + const resolved = yield* resolveOfficialModelAt( + geminiFixture.providerID, + model.id, + `${state.server!.url.origin}/v1beta`, ) const sessionID = SessionID.make("session-test-4") const agent = { @@ -1891,6 +1873,9 @@ describe("session.llm.stream", () => { time: { created: Date.now() }, agent: agent.name, model: { providerID: ProviderV2.ID.make(geminiFixture.providerID), modelID: resolved.id }, + // Plain provider path (see the anthropic test above): this asserts the exact Gemini request + // body, so the DeepAgent round-context message must not be injected. + metadata: { deepagent: { agent_mode_override: "general" } }, } satisfies SessionV1.User yield* drain({ @@ -1918,15 +1903,6 @@ describe("session.llm.stream", () => { expect(config?.topP).toBe(0.8) expect(config?.maxOutputTokens).toBe(ProviderTransform.maxOutputTokens(resolved)) }), - { - config: () => ({ - enabled_providers: [geminiFixture.providerID], - provider: { - [geminiFixture.providerID]: { - options: { apiKey: "test-google-key", baseURL: `${state.server!.url.origin}/v1beta` }, - }, - }, - }), - }, + { config: () => ({ enabled_providers: [geminiFixture.providerID] }) }, ) }) diff --git a/packages/deepagent-code/test/settings/store.test.ts b/packages/deepagent-code/test/settings/store.test.ts index 0118080f..6822df43 100644 --- a/packages/deepagent-code/test/settings/store.test.ts +++ b/packages/deepagent-code/test/settings/store.test.ts @@ -157,6 +157,33 @@ describe("SettingsStore", () => { expect(cleared.settings.providers).toBeUndefined() }) + test("baseURL override for an official provider round-trips (proxy/gateway routing)", async () => { + const result = await SettingsStore.update({ + providers: { openai: { baseURL: "https://proxy.corp.internal/openai/v1", maxRetries: 5 } }, + }) + expect(result.settings.providers).toEqual({ + openai: { baseURL: "https://proxy.corp.internal/openai/v1", maxRetries: 5 }, + }) + }) + + test("http(s) baseURL accepted; malformed / non-http endpoints dropped", async () => { + const ok = await SettingsStore.update({ providers: { openai: { baseURL: "http://localhost:8080/v1" } } }) + expect(ok.settings.providers).toEqual({ openai: { baseURL: "http://localhost:8080/v1" } }) + + await SettingsStore.update({ providers: { openai: { baseURL: undefined as never } } }) + SettingsStore.invalidate() + // file:// (non-http), a bare non-URL string, and empty string are all rejected so a hand-edited + // settings file can never route an official provider somewhere unexpected. + const bad = await SettingsStore.update({ + providers: { + openai: { baseURL: "file:///etc/passwd" as never }, + anthropic: { baseURL: "not a url" as never }, + google: { baseURL: "" as never }, + }, + }) + expect(bad.settings.providers).toBeUndefined() + }) + test("ignores unknown/garbage on read", async () => { await fs.writeFile( settingsFile(), diff --git a/packages/deepagent-code/test/tool/skill.test.ts b/packages/deepagent-code/test/tool/skill.test.ts index bba2f3f8..c121d303 100644 --- a/packages/deepagent-code/test/tool/skill.test.ts +++ b/packages/deepagent-code/test/tool/skill.test.ts @@ -11,6 +11,7 @@ import { ToolRegistry } from "@/tool/registry" import { disposeAllInstances, TestInstance } from "../fixture/fixture" import { SessionID, MessageID } from "../../src/session/schema" import { testEffect } from "../lib/effect" +import { seedRipgrep } from "../lib/seed-ripgrep" const baseCtx: Omit = { sessionID: SessionID.make("ses_test"), @@ -58,6 +59,9 @@ Use this skill. process.env.DEEPAGENT_CODE_TEST_HOME = home }), ) + // Re-seed rg into the repointed test home so tool-registry construction (which resolves rg) + // does not fall through to the ~50s GitHub download and time out. + yield* Effect.promise(() => seedRipgrep(dir)) const registry = yield* ToolRegistry.Service const agent = { name: "build", mode: "primary" as const, permission: [], options: {} } @@ -104,6 +108,9 @@ Use this skill. process.env.DEEPAGENT_CODE_TEST_HOME = home }), ) + // Re-seed rg into the repointed test home so tool-registry construction (which resolves rg) + // does not fall through to the ~50s GitHub download and time out. + yield* Effect.promise(() => seedRipgrep(dir)) const registry = yield* ToolRegistry.Service const agent = { name: "build", mode: "primary" as const, permission: [], options: {} }