From 7c099fd11415fec8dc1c1e7f0044edfd1d1d2c91 Mon Sep 17 00:00:00 2001 From: idevlab Date: Mon, 28 Sep 2026 13:53:01 +0800 Subject: [PATCH 1/3] Stabilize agent orchestration and retire Plan/Goal controls --- apps/desktop/src-host/src/scene_mcp.rs | 8 +- apps/desktop/src/App.tsx | 230 +---- apps/desktop/src/bridge.ts | 118 +-- .../src/environment/EnvironmentPopover.tsx | 33 +- apps/desktop/src/i18n/strings.ts | 80 +- apps/desktop/src/providers/registry.ts | 6 +- apps/desktop/src/session/Composer.tsx | 219 +---- .../src/session/RichTranscriptPreview.tsx | 9 - apps/desktop/src/session/SceneEditor.tsx | 25 - apps/desktop/src/session/SideChatPanel.tsx | 1 - apps/desktop/src/session/TaskPlanPanel.tsx | 205 ----- apps/desktop/src/session/TrajectoryView.tsx | 3 - apps/desktop/src/session/TurnCard.tsx | 4 +- apps/desktop/src/session/composerDrafts.ts | 3 - apps/desktop/src/session/config.ts | 3 +- apps/desktop/src/session/scene.ts | 27 - apps/desktop/src/session/trajectory.ts | 18 - apps/desktop/src/session/turns.ts | 33 +- .../tests/checkoutPickerRendered.test.tsx | 3 +- apps/desktop/tests/composerDrafts.test.ts | 23 +- apps/desktop/tests/issuesDelegation.test.tsx | 2 +- apps/desktop/tests/planDocument.test.tsx | 129 --- .../tests/reasoningScaleRendered.test.tsx | 92 +- .../tests/retiredSessionFeatures.test.ts | 29 + apps/desktop/tests/scene.test.ts | 34 +- apps/desktop/tests/sceneChip.test.tsx | 12 +- .../tests/sceneStudioRendered.test.tsx | 2 +- apps/desktop/tests/sessionState.test.ts | 20 + crates/core/examples/live_demo.rs | 9 - .../1.0.0/examples/acceptance.scene.json | 3 +- .../1.0.0/examples/develop.scene.json | 5 +- .../1.0.0/examples/fix.scene.json | 3 +- .../1.0.0/examples/research.scene.json | 3 +- .../1.0.0/examples/test.scene.json | 3 +- .../agent-scenes/1.0.0/scene.schema.json | 3 +- crates/core/src/acp/client.rs | 23 +- crates/core/src/acp/handler.rs | 2 +- crates/core/src/acp/mod.rs | 13 +- crates/core/src/acp/wire.rs | 45 +- crates/core/src/codex_runtime.rs | 11 +- crates/core/src/cost.rs | 2 +- crates/core/src/engine.rs | 854 +++++++----------- crates/core/src/event.rs | 26 +- crates/core/src/lib.rs | 6 +- crates/core/src/memory.rs | 9 - crates/core/src/models.rs | 387 +++----- crates/core/src/plugins/app/plugins/engine.rs | 20 - .../src/plugins/app/plugins/scene_commands.rs | 6 - crates/core/src/plugins/app/protocol/mod.rs | 35 +- crates/core/src/plugins/app/service.rs | 6 +- crates/core/src/scene.rs | 28 +- crates/core/src/session.rs | 12 +- crates/core/src/skill.rs | 13 - crates/core/src/unix_process_group.rs | 30 + crates/core/tests/acp_process_cleanup.rs | 82 ++ crates/core/tests/engine_activity.rs | 88 ++ crates/core/tests/engine_builtin_models.rs | 50 +- crates/core/tests/engine_permission.rs | 7 +- crates/core/tests/engine_provider_switch.rs | 237 ++++- crates/core/tests/engine_store.rs | 1 - crates/core/tests/scene_conformance.rs | 5 +- crates/server/src/t3_compat.rs | 252 ++---- crates/server/tests/t3_mobile_compat.rs | 62 +- docs/reference/scenes.md | 8 +- .../intent.md | 19 + .../plan.md | 22 + .../spec.md | 29 + .../verification.md | 79 ++ .../2026-09-28-remove-plan-goal/intent.md | 19 + .../2026-09-28-remove-plan-goal/plan.md | 20 + .../2026-09-28-remove-plan-goal/spec.md | 24 + .../verification.md | 45 + 72 files changed, 1519 insertions(+), 2458 deletions(-) delete mode 100644 apps/desktop/src/session/TaskPlanPanel.tsx delete mode 100644 apps/desktop/tests/planDocument.test.tsx create mode 100644 apps/desktop/tests/retiredSessionFeatures.test.ts create mode 100644 crates/core/src/unix_process_group.rs create mode 100644 crates/core/tests/acp_process_cleanup.rs create mode 100644 docs/sdlc/changes/2026-09-28-orchestration-stability/intent.md create mode 100644 docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md create mode 100644 docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md create mode 100644 docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md create mode 100644 docs/sdlc/changes/2026-09-28-remove-plan-goal/intent.md create mode 100644 docs/sdlc/changes/2026-09-28-remove-plan-goal/plan.md create mode 100644 docs/sdlc/changes/2026-09-28-remove-plan-goal/spec.md create mode 100644 docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md diff --git a/apps/desktop/src-host/src/scene_mcp.rs b/apps/desktop/src-host/src/scene_mcp.rs index 7f683bbd..8e3e16e0 100644 --- a/apps/desktop/src-host/src/scene_mcp.rs +++ b/apps/desktop/src-host/src/scene_mcp.rs @@ -527,8 +527,8 @@ async fn dispatch_broker( .resolve(&reference) .map(codetwo_core::SceneLibrary::reference_for) }); - let (changed, pending, plan_first) = if current.as_deref() == Some(&canonical) { - (false, Vec::new(), None) + let (changed, pending) = if current.as_deref() == Some(&canonical) { + (false, Vec::new()) } else { let outcome = core .call( @@ -565,8 +565,7 @@ async fn dispatch_broker( .filter_map(Value::as_str) .map(str::to_string) .collect(); - let plan_first = outcome.get("plan_first").and_then(Value::as_bool); - (true, pending, plan_first) + (true, pending) }; let (memory_read, memory_write) = store .session_memory_policy(&request.session) @@ -580,7 +579,6 @@ async fn dispatch_broker( "title": title, "reason": reason, "pending": pending, - "planFirst": plan_first, "memoryRead": memory_read, "memoryWrite": memory_write, }), diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index 400e313e..b5815407 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -73,13 +73,11 @@ import { call, compileDoc, confirmNative, - controlGoal, addProject, DEFAULT_KEYMAP, defaultCwd, describeBlock, discardSessionWorktree, - fallbackProviders, getKeymap, getAppshot, getPromptImage, @@ -169,7 +167,6 @@ import { getSessionAutoScene, listPipelines, listScenes, - recordSceneArtifact, sceneSessionPlan, sessionPipeline, setModel as setSessionModel, @@ -199,7 +196,6 @@ import type { AppshotCapture, GitStatus, GitHubPullRequest, - GoalSnapshot, GitHubPullRequestDetail, Issue, KeymapEntry, @@ -215,7 +211,6 @@ import type { ProviderInfo, ProviderQuotaReport, PermissionMode, - PlanEntry, Sandbox, SessionActivity, SessionInfo, @@ -372,7 +367,6 @@ import { sceneCustomized, softApplyPending, MEMORY_PRESET_POLICY, - sceneCollaborationChoice, sceneEffortChoice, } from "./session/scene"; import type { SceneInfo } from "./session/scene"; @@ -415,7 +409,6 @@ import { StageTrack } from "./session/StageTrack"; // resolver matches the pair case-insensitively without it. import { deriveBurnRate } from "./session/statusline.ts"; import { TaskHandoffDialog } from "./session/TaskHandoffDialog"; -import { planChecklistMarkdown } from "./session/TaskPlanPanel"; import { TemplateDialog } from "./session/TemplateDialog"; import { activeInteractivePreview, @@ -857,7 +850,7 @@ const EMPTY_PANE_TRANSCRIPT_STATE: PaneTranscriptState = { }; export default function App() { - const [providers, setProviders] = useState(fallbackProviders); + const [providers, setProviders] = useState([]); const [providersStatus, setProvidersStatus] = useState< "loading" | "ready" | "error" >("loading"); @@ -888,10 +881,11 @@ export default function App() { // Row 2 of every rail entry. Refreshed when a turn ends rather than per streamed chunk — the // preview is a glance, and requerying the transcript table on every token would be absurd. const [previews, setPreviews] = useState>({}); - const [provider, setProvider] = useState("grok"); + const [provider, setProvider] = useState(""); const [providerSwitchingSessions, setProviderSwitchingSessions] = useState< Set >(() => new Set()); + const providerSwitchRequestsRef = useRef(new Set()); const [cwd, setCwd] = useState("."); const [mode, setMode] = useState("ask"); const [sandbox, setSandboxState] = useState("workspace_write"); @@ -906,7 +900,6 @@ export default function App() { >([]); const [worktreeOptionsLoading, setWorktreeOptionsLoading] = useState(false); const worktreeOptionsRequestRef = useRef(0); - const [planMode, setPlanMode] = useState(false); const [memoryRead, setMemoryRead] = useState("inherit"); const [memoryWrite, setMemoryWrite] = useState("inherit"); // The tiling workspace: a recursive split tree plus the id -> content map. `activeSession` is the @@ -1257,8 +1250,6 @@ export default function App() { const autoSceneRef = useRef(false); /** Sessions whose scene reasoning_effort has been applied (once options arrived). */ const sceneEffortAppliedRef = useRef(new Set()); - /** Last scene plan posture sent through each session's provider-owned collaboration option. */ - const scenePlanAppliedRef = useRef(new Map()); /** Stage binding for the next created session (advance-in-new-session handshake). */ const pendingPipelineBindRef = useRef<{ instanceId: string; @@ -1333,7 +1324,6 @@ export default function App() { const [interactionCapabilities, setInteractionCapabilities] = useState< Record >({}); - const [goals, setGoals] = useState>({}); // Provider-reported context windows are session-level state, not transcript parts. Keeping the // map keyed by id prevents a late/background provider event from repainting the active session. const [contextWindows, setContextWindows] = useState( @@ -1394,7 +1384,6 @@ export default function App() { mode, sandbox, worktreeBase, - planMode, memoryRead, memoryWrite, scene: activeSceneName, @@ -1406,7 +1395,6 @@ export default function App() { mode, sandbox, worktreeBase, - planMode, memoryRead, memoryWrite, scene: activeSceneName, @@ -1718,38 +1706,6 @@ export default function App() { const componentEnabledRef = useRef<(id: BuiltinUiComponentId) => boolean>( () => false ); - // ---- R4 plan-as-document (docs/archive/scenes-v1/frontend-implementation-plan.md Item 3) ---- - // Plan markdown waiting on the Replace/Append/Cancel decision because the composer isn't empty. - const [planDocPending, setPlanDocPending] = useState(null); - /** The edited plan IS the next prompt: it opens into this session's composer document. */ - const openPlanAsDocument = (entries: PlanEntry[]) => { - const markdown = planChecklistMarkdown(entries); - if (docEmpty) { - void insertMarkdownRef.current?.(markdown, "replace"); - setDocMode(true); - } else { - setPlanDocPending(markdown); - } - }; - const resolvePlanDocPending = (mode: "replace" | "append" | null) => { - const markdown = planDocPending; - setPlanDocPending(null); - if (markdown == null || markdown === "" || !mode) return; - void insertMarkdownRef.current?.(markdown, mode); - setDocMode(true); - }; - const pinPlanArtifact = (markdown: string) => { - if (!componentEnabledRef.current("scenes.surface")) return; - const session = activeSessionRef.current; - if (session == null || session === "") return; - void recordSceneArtifact(session, "plan", markdown).then((record) => { - if (record) toast(t("planDoc.pinned"), "success"); - else toast(t("planDoc.pinFailed"), "error"); - }); - }; - const canPinPlan = ( - scenes.find((s) => s.reference === activeSceneName)?.artifacts ?? [] - ).some((artifact) => artifact.kind === "plan"); // ---- R2 template-from-history (docs/archive/scenes-v1/frontend-implementation-plan.md Item 8) ---- // Stable so the memoized TurnCards don't re-render on every App render. const openTemplateDraft = (promptText: string) => { @@ -2122,7 +2078,6 @@ export default function App() { setMode(posture.mode); setSandboxState(posture.sandbox); setWorktreeBase(posture.worktreeBase); - setPlanMode(posture.planMode); memoryReadRef.current = posture.memoryRead; memoryWriteRef.current = posture.memoryWrite; setMemoryRead(posture.memoryRead); @@ -2396,7 +2351,6 @@ export default function App() { memoryWrite, mode, pendingAppshots, - planMode, provider, sandbox, scheduleActiveComposerDraftSave, @@ -3136,7 +3090,7 @@ export default function App() { } } } - // Connect as soon as the durable shell exists so Plan/Goal capability selectors can be + // Connect as soon as the durable shell exists so provider configuration can be // provider-authored before the next prompt instead of appearing only after it runs. void prepareSession(ev.session).catch(() => { /* empty */ @@ -3192,15 +3146,13 @@ export default function App() { } if (originFocused) setActiveSceneName(pendingScene); // Provider-owned config ids do not exist until the session reports its options, - // so scene effort and collaboration posture stay pending until that handshake. + // so scene effort stays pending until that handshake. const pending: string[] = []; if ( scene?.execution?.reasoning_effort != null && scene?.execution?.reasoning_effort !== "" ) pending.push("reasoning_effort"); - if (scene?.execution?.plan_first !== undefined) - pending.push("plan_first"); if (originFocused) setScenePendingFields(pending); } else { if (originFocused) { @@ -3363,16 +3315,13 @@ export default function App() { const { [ev.session]: _old, ...rest } = current; return rest; }); - setGoals((current) => ({ ...current, [ev.session]: null })); sceneEffortAppliedRef.current.delete(ev.session); - scenePlanAppliedRef.current.delete(ev.session); if (ev.session === activeSessionRef.current) { setProvider(nextProvider); setModels([]); setCurrentModel(nextModel); setDefaultModel(null); setConfigOptions([]); - setPlanMode(false); const scene = scenesRef.current.find( (candidate) => candidate.reference === activeSceneNameRef.current ); @@ -3381,9 +3330,6 @@ export default function App() { scene?.execution?.reasoning_effort !== "" ? ["reasoning_effort"] : []), - ...(scene?.execution?.plan_first === undefined - ? [] - : ["plan_first"]), ]); } if (canvasProviderRetrySessionRef.current === ev.session) { @@ -3446,16 +3392,11 @@ export default function App() { ...previous, [ev.session]: { steering: ev.steering, - goal: ev.goal, compact_context: ev.compact_context ?? false, }, })); return; } - if (ev.event === "goal_changed") { - setGoals((previous) => ({ ...previous, [ev.session]: ev.goal })); - return; - } if ( ev.event === "exit_criteria_met" || ev.event === "hook_suggestion" || @@ -3550,12 +3491,6 @@ export default function App() { if (ev.session !== activeSessionRef.current) return; // The agent's set is authoritative — it replaces any optimistic UI state wholesale. setConfigOptions(ev.options); - const collaboration = ev.options.find( - (option) => - option.category === "collaboration_mode" || - option.id === "collaboration_mode" - ); - if (collaboration) setPlanMode(collaboration.current === "plan"); if (model?.current != null && model?.current !== "") { setCurrentModel(model.current); // Same rule as `models`: the first report after a reset is the adapter's own pick. @@ -3588,42 +3523,6 @@ export default function App() { } } } - { - // `plan_first` is also provider-owned. A scene can request it, but the request is sent - // only after the adapter advertises the collaboration selector and its native values. - const scene = scenesRef.current.find( - (s) => s.reference === activeSceneNameRef.current - ); - const wanted = scene?.execution?.plan_first; - const applied = scenePlanAppliedRef.current.get(ev.session); - if (wanted !== undefined && applied !== wanted) { - const choice = sceneCollaborationChoice(ev.options, wanted); - if (choice) { - scenePlanAppliedRef.current.set(ev.session, wanted); - if (collaboration?.current === choice.value) { - setScenePendingFields((prev) => - prev.filter((field) => field !== "plan_first") - ); - } else { - setPlanMode(wanted); - void setConfigOption( - ev.session, - choice.configId, - choice.value - ) - .then(() => { - setScenePendingFields((prev) => - prev.filter((field) => field !== "plan_first") - ); - }) - .catch(() => { - scenePlanAppliedRef.current.delete(ev.session); - setPlanMode(collaboration?.current === "plan"); - }); - } - } - } - } return; } if (ev.event === "execution_policy_changed") { @@ -3891,7 +3790,6 @@ export default function App() { memoryWriteRef.current = event.memoryWrite; setMemoryRead(event.memoryRead); setMemoryWrite(event.memoryWrite); - if (event.planFirst !== null) setPlanMode(event.planFirst); toast( t("scene.autoSwitched", { scene: event.title, reason: event.reason }), "success" @@ -4349,12 +4247,6 @@ export default function App() { toast(t("toast.modelBusy"), "error"); return; } - if ( - option?.category === "collaboration_mode" || - configId === "collaboration_mode" - ) { - setPlanMode(value === "plan"); - } if ( (option?.category === "model" || configId === "model") && value !== currentModelRef.current @@ -6639,7 +6531,6 @@ export default function App() { mode: sessionMode(mode, sandbox), memoryRead, memoryWrite, - planFirst: planMode, provider, model: currentModel, }; @@ -6650,44 +6541,6 @@ export default function App() { onMemoryPolicyChange(preset.read, preset.write); } const pending = softApplyPending(scene, live); - if (execution?.plan_first !== undefined) { - const wanted = execution.plan_first; - const choice = sceneCollaborationChoice(configOptions, wanted); - if (session == null || session === "" || !choice) { - if (!pending.includes("plan_first")) pending.push("plan_first"); - } else { - const previousPlanMode = planMode; - scenePlanAppliedRef.current.set(session, wanted); - setPlanMode(wanted); - setConfigOptions((options) => - options.map((option) => - option.id === choice.configId - ? { ...option, current: choice.value } - : option - ) - ); - void setConfigOption(session, choice.configId, choice.value).catch( - (error: unknown) => { - scenePlanAppliedRef.current.delete(session); - setPlanMode(previousPlanMode); - setConfigOptions((options) => - options.map((option) => - option.id === choice.configId - ? { - ...option, - current: previousPlanMode ? "plan" : "default", - } - : option - ) - ); - setScenePendingFields((fields) => - fields.includes("plan_first") ? fields : [...fields, "plan_first"] - ); - toast(t("toast.configFailed", { error: String(error) }), "error"); - } - ); - } - } setActiveSceneName(reference); setScenePendingFields(pending); if (session != null && session !== "") { @@ -7592,6 +7445,8 @@ export default function App() { toast(t("toast.providerSwitchBusy"), "error"); return; } + if (providerSwitchRequestsRef.current.has(sessionId)) return; + providerSwitchRequestsRef.current.add(sessionId); setProviderSwitchingSessions((current) => new Set(current).add(sessionId)); void switchProvider(sessionId, next, nextModel) .catch((error: unknown) => { @@ -7601,6 +7456,7 @@ export default function App() { ); }) .finally(() => { + providerSwitchRequestsRef.current.delete(sessionId); setProviderSwitchingSessions((current) => { if (!current.has(sessionId)) return current; const remaining = new Set(current); @@ -7622,6 +7478,8 @@ export default function App() { nextProvider, nextModel ), + providerSwitching: + activeSession !== null && providerSwitchingSessions.has(activeSession), providerChangeDisabled: activeSession !== null && (runningSessions.has(activeSession) || @@ -7641,8 +7499,7 @@ export default function App() { worktreeOptions, worktreeOptionsLoading, onWorktreeBase: setWorktreeBase, - planMode, - onPlan: setPlanMode, + memoryRead, memoryWrite, memoryEnabled: memorySettingsEnabled, @@ -7678,7 +7535,6 @@ export default function App() { mode: sessionMode(mode, sandbox), memoryRead, memoryWrite, - planFirst: planMode, provider, model: currentModel, }); @@ -8369,10 +8225,6 @@ export default function App() { activeSession != null && activeSession !== "" ? (interactionCapabilities[activeSession] ?? null) : null; - const activeGoal = - activeSession != null && activeSession !== "" - ? (goals[activeSession] ?? null) - : null; const activeAppshotKey = activeSession ?? `draft:${(activeProject ?? cwd) || "."}`; const activeAppshots = @@ -8387,6 +8239,9 @@ export default function App() { ...sessionConfig, provider: paneProvider, hasSession: activeSession !== null, + providerSwitching: + activeSession !== null && + providerSwitchingSessions.has(activeSession), providerChangeDisabled: activeSession !== null && (running || @@ -8550,10 +8405,6 @@ export default function App() { setSettingsInitialTab("general"); setShowSettings(true); }} - turns={turns} - onOpenPlanAsDocument={openPlanAsDocument} - onPinPlanArtifact={pinPlanArtifact} - canPinPlan={scenesSurfaceEnabled && canPinPlan} preview={interactivePreview} /> @@ -8964,29 +8815,6 @@ export default function App() { activeInteractionCapabilities?.steering ?? false } - goalCapability={ - activeInteractionCapabilities?.goal ?? null - } - goal={activeGoal} - onGoal={async (action, objective) => { - const session = activeSession; - if (session == null || session === "") return; - try { - await controlGoal( - session, - action, - objective - ); - } catch (error) { - toast( - t("toast.goalFailed", { - error: String(error), - }), - "error" - ); - throw error; - } - }} onStop={() => activeSession != null && activeSession !== "" && @@ -9440,36 +9268,6 @@ export default function App() { /> )} - {planDocPending != null && planDocPending !== "" && ( - !o && resolvePlanDocPending(null)}> - - - {t("planDoc.title")} - -

- {t("planDoc.confirm")} -

- - - - - - - - )} - {skillDraft && ( !o && setSkillDraft(null)}> diff --git a/apps/desktop/src/bridge.ts b/apps/desktop/src/bridge.ts index b1ae5bb7..8161bddf 100644 --- a/apps/desktop/src/bridge.ts +++ b/apps/desktop/src/bridge.ts @@ -809,28 +809,12 @@ export interface ConfigOptionInfo { choices: ModelChoice[]; } -export interface GoalCapabilityInfo { - control_method: string; - actions: string[]; -} - export interface SessionInteractionCapabilities { steering: boolean; - goal: GoalCapabilityInfo | null; /** Set only after the live ACP session advertises its native `/compact` command. */ compact_context: boolean; } -export interface GoalSnapshot { - objective: string; - status: string; - created_at: number; - updated_at: number; - token_budget: number | null; - tokens_used: number; - time_used_seconds: number; -} - /// Neutral document shape the editor serializes into; matches core `DocBlock` serde. export type DocBlock = | { type: "text"; text: string } @@ -957,12 +941,6 @@ export type CoreEvent = outputs?: ToolOutput[]; transcript_seq?: number | null; } - | { - event: "plan"; - session: string; - entries: (PlanEntry | string)[]; - transcript_seq?: number | null; - } | { event: "permission_request"; session: string; @@ -1002,10 +980,8 @@ export type CoreEvent = event: "session_capabilities"; session: string; steering: boolean; - goal: GoalCapabilityInfo | null; compact_context?: boolean; } - | { event: "goal_changed"; session: string; goal: GoalSnapshot | null } | { event: "prompt_queued"; session: string; @@ -1112,13 +1088,6 @@ export interface PtyAttach { restore: string; } -export interface PlanEntry { - content: string; - priority?: string | null; - status?: string | null; -} - -/// Mirrors core `Part` (tagged by `kind`). export type Part = | { kind: "text"; text: string } | { kind: "prompt"; text: string; display: string } @@ -1132,7 +1101,7 @@ export type Part = agent_input?: unknown; outputs?: ToolOutput[]; } - | { kind: "plan"; entries: (PlanEntry | string)[] }; + | { kind: "plan"; entries: unknown[] }; export interface ArtifactRef { id: string; @@ -1773,72 +1742,6 @@ export async function reloadDevelopmentPlugins(): Promise ); } -const fallbackProvider = ( - id: string, - display_name: string, - needs_node: boolean -): ProviderInfo => ({ - id, - display_name, - custom: false, - available: false, - enabled: true, - needs_node, - models: [], - capabilities: [], - management: { - installed: false, - version: null, - latest_version: null, - update_available: null, - check_error: null, - install_supported: false, - upgrade_supported: false, - launch_mode: "unavailable", - }, - configuration: defaultProviderConfiguration({ id }), -}); - -const FALLBACK_PROVIDERS: ProviderInfo[] = [ - fallbackProvider("claude_code", "Claude Code", true), - fallbackProvider("codex", "Codex", true), - fallbackProvider("grok", "Grok", false), - fallbackProvider("cursor", "Cursor", false), - fallbackProvider("opencode", "OpenCode", false), - fallbackProvider("opencode2", "OpenCode 2 (Beta)", false), - fallbackProvider("pi", "Pi", true), - fallbackProvider("kimi", "Kimi", false), - fallbackProvider("zcode", "ZCode (GLM)", true), - fallbackProvider("amp", "Amp", true), - fallbackProvider("droid", "Droid", false), -]; - -/** Stable provider identity while the desktop host is still starting or temporarily unavailable. */ -export function fallbackProviders(): ProviderInfo[] { - return FALLBACK_PROVIDERS.map((provider) => ({ - ...provider, - models: [...provider.models], - capabilities: [...provider.capabilities], - management: { ...provider.management }, - configuration: { - ...provider.configuration, - args: provider.configuration.args - ? [...provider.configuration.args] - : null, - effective_args: [...provider.configuration.effective_args], - forwarded_environment: [...provider.configuration.forwarded_environment], - missing_environment: [...provider.configuration.missing_environment], - }, - })); -} - -export function providerDisplayName(providerId: string): string { - return ( - FALLBACK_PROVIDERS.find((provider) => provider.id === providerId) - ?.display_name ?? providerId - ); -} - const FALLBACK_SKILLS: SkillInfo[] = [ { id: "reviewer", @@ -1917,7 +1820,7 @@ export async function listProviders( ? await call("providers.list", { check_updates: checkUpdates, }) - : fallbackProviders(); + : []; return providers.map(normalizeProviderInfo); } @@ -2450,19 +2353,6 @@ export async function steerPrompt( return await call("engine.steer", { session, doc, request_id: requestId }); } -export async function controlGoal( - session: string, - action: "set" | "pause" | "resume" | "clear", - objective?: string -): Promise { - if (inDesktop) - await call("engine.goal", { - session, - action, - objective: objective ?? null, - }); -} - export async function listAutomations(): Promise { return inDesktop ? await call("automation.list") : []; } @@ -5365,7 +5255,6 @@ export interface SceneApplyOutcome { applied: string[]; pending: string[]; escalation: SceneEscalation | null; - plan_first: boolean | null; suppress_unpinned: boolean; pinned_skills: string[]; } @@ -5396,7 +5285,6 @@ export interface AutoSceneChanged { title: string; reason: string; pending: string[]; - planFirst: boolean | null; memoryRead: MemoryAccess; memoryWrite: MemoryAccess; } @@ -5428,7 +5316,7 @@ const FALLBACK_SCENES: SceneInfo[] = ( "Develop", "开发", "auto_edit", - "Plan-first implementation in an isolated worktree.", + "Implementation in an isolated worktree.", ], [ "test", diff --git a/apps/desktop/src/environment/EnvironmentPopover.tsx b/apps/desktop/src/environment/EnvironmentPopover.tsx index 8b51afa8..10b91c82 100644 --- a/apps/desktop/src/environment/EnvironmentPopover.tsx +++ b/apps/desktop/src/environment/EnvironmentPopover.tsx @@ -32,12 +32,10 @@ import { Spinner } from "@/components/ui/spinner"; import { cn } from "@/lib/utils"; import { getArtifact } from "../bridge"; -import type { GitStatus, PlanEntry, Project } from "../bridge"; +import type { GitStatus, Project } from "../bridge"; import { GitSyncStatus } from "../git/GitSyncStatus"; import { useT } from "../i18n"; -import { TaskPlanPanel } from "../session/TaskPlanPanel"; import type { InteractiveToolPreview } from "../session/toolActivity"; -import type { Turn } from "../session/turns"; function EnvironmentRow({ icon: Icon, @@ -204,10 +202,6 @@ export function EnvironmentPopover({ onAddProject, onOpenSourceControl, onOpenSettings, - turns, - onOpenPlanAsDocument, - onPinPlanArtifact, - canPinPlan = false, preview = null, suppressed = false, }: { @@ -221,10 +215,6 @@ export function EnvironmentPopover({ onAddProject: () => void; onOpenSourceControl: () => void; onOpenSettings: () => void; - turns: readonly Turn[]; - onOpenPlanAsDocument?: (entries: PlanEntry[]) => void; - onPinPlanArtifact?: (markdown: string) => void; - canPinPlan?: boolean; preview?: InteractiveToolPreview | null; /** Keeps the mounted session workspace from leaking this portal over another full-page surface. */ suppressed?: boolean; @@ -393,27 +383,6 @@ export function EnvironmentPopover({ disabled={!isRepo} /> - { - setOpen(false); - onOpenPlanAsDocument(entries); - } - : undefined - } - onPinPlanArtifact={ - onPinPlanArtifact - ? (markdown) => { - setOpen(false); - onPinPlanArtifact(markdown); - } - : undefined - } - canPinPlan={canPinPlan} - /> - {preview && (
diff --git a/apps/desktop/src/i18n/strings.ts b/apps/desktop/src/i18n/strings.ts index e4dd4599..1598d8a9 100644 --- a/apps/desktop/src/i18n/strings.ts +++ b/apps/desktop/src/i18n/strings.ts @@ -964,8 +964,9 @@ export const en = { "composer.noMatchingModels": "No models match this search.", "composer.noVisibleModels": "All models for this provider are hidden in Settings.", + "composer.switchingProvider": "Switching agent…", "composer.noModels": - "We don't know this provider's models, and it doesn't report any over ACP, so the model is whatever its CLI is configured to use. Set it in the CLI's own config.", + "No model choices have been reported yet. You can continue with the agent’s configured default; available choices appear when it connects.", "composer.defaultModel": "Default model", "composer.reasoning": "Reasoning", "composer.default": "Default", @@ -1063,7 +1064,6 @@ export const en = { "trajectory.kind.reasoning": "Reasoning", "trajectory.kind.tool": "Tool", "trajectory.kind.memory": "Memory", - "trajectory.kind.plan": "Plan", "trajectory.kind.error": "Error", "turn.working": "Working…", "turn.running": "running", @@ -1079,7 +1079,6 @@ export const en = { "turn.agentStatus.completed": "completed", "turn.agentStatus.failed": "failed", "turn.thinking": "thinking", - "turn.plan": "plan", "turn.memory": "memory used", "turn.memoryTokens": "about {count} memory tokens", "turn.showMore": "Show more", @@ -1103,18 +1102,6 @@ export const en = { "chart.type.bar": "Bar chart", "chart.summary": "{title}. {type} with {series} series and {points} categories.", - "goal.label": "Goal", - "goal.objective": "Goal objective", - "goal.placeholder": "What should Codex keep pursuing?", - "goal.start": "Start goal", - "goal.pause": "Pause", - "goal.resume": "Resume", - "goal.clear": "Clear", - "goal.status.active": "Active", - "goal.status.paused": "Paused", - "goal.status.blocked": "Blocked", - "goal.status.limited": "Limited", - "goal.status.complete": "Complete", // settings "settings.title": "Settings", @@ -1923,10 +1910,6 @@ export const en = { "worktree.discardConfirm": "Discard the worktree at “{path}”? Uncommitted work in it is lost. This cannot be undone.", "worktree.discardFailed": "Could not discard the worktree: {error}", - "config.planFirst": "Plan first", - "config.planFirstHint": "Propose a plan and wait before editing.", - "config.planFirstOn": "Plan first: On", - "config.planFirstOff": "Plan first: Off", "config.providerMissing": "{name}'s CLI isn't on your PATH{node}. A new session will fail until it's installed — pick a provider with a green dot instead.", "config.needsNode": " (needs Node)", @@ -1970,13 +1953,6 @@ export const en = { "dock.sendTerminal": "Send terminal output to the agent", // current task plan - "taskPlan.title": "Current tasks", - "taskPlan.list": "Current task list", - "taskPlan.progress": "{completed} of {total} completed", - "taskPlan.step": "Step {current} / {total}", - "taskPlan.status.pending": "Pending", - "taskPlan.status.inProgress": "In progress", - "taskPlan.status.completed": "Completed", // GitHub pull request panel "githubPr.title": "GitHub pull request", @@ -2141,7 +2117,6 @@ export const en = { "toast.alreadyRunning": "A turn is already running. Stop it first.", "toast.notRunning": "There is no running turn.", "toast.steerUnsupported": "This provider does not support native steering.", - "toast.goalFailed": "Could not update the goal: {error}", "toast.configFailed": "Could not update the provider mode: {error}", "toast.nothingRunning": "Nothing is running.", "toast.turnFailed": "Could not start the turn: {error}", @@ -2457,16 +2432,6 @@ export const en = { "permission.risk": "Risk", "permission.requiredEvenFullAccess": "This approval is required even in Full Access.", - "planDoc.open": "Open as document", - "planDoc.pin": "Pin as artifact", - "planDoc.title": "Open plan as document", - "planDoc.confirm": - "The composer already has content. Replace it with the plan, or append the plan below it?", - "planDoc.replace": "Replace", - "planDoc.append": "Append", - "planDoc.cancel": "Cancel", - "planDoc.pinned": "Plan pinned as a scene artifact", - "planDoc.pinFailed": "Could not pin the plan", "voice.hold": "Voice input — hold to talk, or click to toggle dictation", "voice.stop": "Stop listening", "voice.structuring": "Structuring your dictation…", @@ -2610,7 +2575,6 @@ export const en = { "sceneEditor.permissionMode": "Permission mode", "sceneEditor.memoryPreset": "Memory preset", "sceneEditor.worktreeMode": "Worktree", - "sceneEditor.planFirst": "Plan first", "sceneEditor.inherit": "Inherit", "sceneEditor.inheritDefault": "Use default", "sceneEditor.on": "On", @@ -3887,8 +3851,9 @@ export const zhCN: Record = { "composer.clearModelSearch": "清除模型搜索", "composer.noMatchingModels": "没有符合搜索条件的模型。", "composer.noVisibleModels": "此 Provider 的模型已全部在设置中隐藏。", + "composer.switchingProvider": "正在切换 agent…", "composer.noModels": - "我们没有这个供应商的内置模型列表，它也没有通过 ACP 报告可选模型，所以模型取决于它自己 CLI 的配置。请在那边设置。", + "尚未获取到可选模型。你可以使用 agent 已配置的默认模型继续；连接后会显示它提供的选项。", "composer.defaultModel": "默认模型", "composer.reasoning": "推理强度", "composer.default": "默认", @@ -3981,7 +3946,6 @@ export const zhCN: Record = { "trajectory.kind.reasoning": "思考", "trajectory.kind.tool": "工具", "trajectory.kind.memory": "记忆", - "trajectory.kind.plan": "方案", "trajectory.kind.error": "错误", "turn.working": "处理中…", "turn.running": "进行中", @@ -3997,7 +3961,6 @@ export const zhCN: Record = { "turn.agentStatus.completed": "已完成", "turn.agentStatus.failed": "失败", "turn.thinking": "思考", - "turn.plan": "方案", "turn.memory": "本轮使用的记忆", "turn.memoryTokens": "约 {count} 个记忆 token", "turn.showMore": "展开", @@ -4020,18 +3983,6 @@ export const zhCN: Record = { "chart.type.line": "折线图", "chart.type.bar": "柱状图", "chart.summary": "{title}。{type}，共 {series} 个系列、{points} 个分类。", - "goal.label": "目标", - "goal.objective": "目标内容", - "goal.placeholder": "希望 Codex 持续推进什么？", - "goal.start": "开始目标", - "goal.pause": "暂停", - "goal.resume": "继续", - "goal.clear": "清除", - "goal.status.active": "进行中", - "goal.status.paused": "已暂停", - "goal.status.blocked": "受阻", - "goal.status.limited": "已达限制", - "goal.status.complete": "已完成", "settings.title": "设置", "settings.back": "返回", @@ -4780,10 +4731,6 @@ export const zhCN: Record = { "worktree.discardConfirm": "丢弃「{path}」的 worktree？其中未提交的改动会丢失，此操作无法撤销。", "worktree.discardFailed": "无法丢弃 worktree：{error}", - "config.planFirst": "先出方案", - "config.planFirstHint": "先提出方案并等待确认，然后再改动。", - "config.planFirstOn": "先出方案：开启", - "config.planFirstOff": "先出方案：关闭", "config.providerMissing": "{name} 的 CLI 不在 PATH 上{node}。装好之前新会话会失败——先选一个带绿点的供应商。", "config.needsNode": "（需要 Node）", @@ -4821,14 +4768,6 @@ export const zhCN: Record = { "dock.tmux": "tmux", "dock.sendTerminal": "把终端输出发给智能体", - "taskPlan.title": "当前任务", - "taskPlan.list": "当前任务清单", - "taskPlan.progress": "已完成 {completed} / {total}", - "taskPlan.step": "第 {current} / {total} 步", - "taskPlan.status.pending": "待处理", - "taskPlan.status.inProgress": "进行中", - "taskPlan.status.completed": "已完成", - "githubPr.title": "GitHub 拉取请求", "githubPr.refresh": "刷新拉取请求", "githubPr.loading": "正在检查当前分支关联的拉取请求…", @@ -4978,7 +4917,6 @@ export const zhCN: Record = { "toast.alreadyRunning": "已经有一轮在运行了，先停止它。", "toast.notRunning": "当前没有正在运行的轮次。", "toast.steerUnsupported": "这个供应商不支持原生插队。", - "toast.goalFailed": "无法更新目标：{error}", "toast.configFailed": "无法更新 Provider 模式：{error}", "toast.nothingRunning": "当前没有在运行的任务。", "toast.turnFailed": "无法开始这一轮：{error}", @@ -5278,15 +5216,6 @@ export const zhCN: Record = { "permission.risk": "风险", "permission.requiredEvenFullAccess": "即使处于完全访问模式，也必须确认此操作。", - "planDoc.open": "以文档打开", - "planDoc.pin": "固定到场景", - "planDoc.title": "将计划打开为文档", - "planDoc.confirm": "编辑器中已有内容。用计划替换现有内容，还是追加到末尾？", - "planDoc.replace": "替换", - "planDoc.append": "追加", - "planDoc.cancel": "取消", - "planDoc.pinned": "计划已固定到场景", - "planDoc.pinFailed": "无法将计划固定到场景", "voice.hold": "语音输入——按住说话，或点按切换听写", "voice.stop": "停止聆听", "voice.structuring": "正在整理听写内容…", @@ -5422,7 +5351,6 @@ export const zhCN: Record = { "sceneEditor.permissionMode": "权限模式", "sceneEditor.memoryPreset": "记忆预设", "sceneEditor.worktreeMode": "Worktree", - "sceneEditor.planFirst": "先出方案", "sceneEditor.inherit": "继承", "sceneEditor.inheritDefault": "使用默认值", "sceneEditor.on": "开启", diff --git a/apps/desktop/src/providers/registry.ts b/apps/desktop/src/providers/registry.ts index da8fa08e..9f5e6a2b 100644 --- a/apps/desktop/src/providers/registry.ts +++ b/apps/desktop/src/providers/registry.ts @@ -1,7 +1,9 @@ import type { ProviderInfo } from "../bridge"; const DEFAULT_RETRY_DELAYS_MS = [0, 250, 750] as const; -const DEFAULT_ATTEMPT_TIMEOUT_MS = 7000; +// Host discovery bounds version probes at 6s and model queries at 8s. Let it settle +// before retrying, otherwise a slow CLI spawns overlapping discovery requests. +const DEFAULT_ATTEMPT_TIMEOUT_MS = 16_000; async function timeout(promise: Promise, timeoutMs: number): Promise { return await new Promise((resolve, reject) => { @@ -31,7 +33,7 @@ async function pause(delayMs: number): Promise { /** * Desktop RPC can race the native bridge during first paint. Bound every attempt and retry the - * fixed provider catalog so one lost startup request cannot leave the picker empty forever. + * host provider discovery so one lost startup request cannot leave the picker empty forever. */ export async function loadProviderRegistry( load: () => Promise, diff --git a/apps/desktop/src/session/Composer.tsx b/apps/desktop/src/session/Composer.tsx index 9b1f2065..72699c7a 100644 --- a/apps/desktop/src/session/Composer.tsx +++ b/apps/desktop/src/session/Composer.tsx @@ -35,12 +35,10 @@ import { Square, Star, Store, - Target, Ticket, TriangleAlert, X, } from "@/components/ui/icons"; -import { Input } from "@/components/ui/input"; import { Popover, PopoverContent, @@ -54,17 +52,9 @@ import { } from "@/components/ui/tooltip"; import { cn } from "@/lib/utils"; -import { fallbackProviders } from "../bridge"; -import type { - ConfigOptionInfo, - AppshotCapture, - GoalCapabilityInfo, - GoalSnapshot, - ModelChoice, -} from "../bridge"; +import type { ConfigOptionInfo, AppshotCapture, ModelChoice } from "../bridge"; import { briefOfferVisible } from "../editor/slotCard"; import { useT } from "../i18n"; -import { td } from "../i18n/dynamic"; import { ProviderIcon } from "../providers/ProviderIcon"; import { VoiceButton } from "../voice/VoiceButton"; import { memoryPresetsForProvider } from "./config"; @@ -127,12 +117,6 @@ interface ComposerProps { onSteer: () => void; onStop: () => void; steeringSupported: boolean; - goalCapability: GoalCapabilityInfo | null; - goal: GoalSnapshot | null; - onGoal: ( - action: "set" | "pause" | "resume" | "clear", - objective?: string - ) => Promise; onAttachFile: () => void; onAttachImages: (files: readonly File[]) => void | Promise; onInsertSkill: () => void; @@ -598,169 +582,6 @@ export function MemoryPicker({ config }: { config: SessionConfig }) { ); } -/** A provider-reported collaboration selector. Plan is never synthesized into prompt text. */ -export function CollaborationModePicker({ - options, - onChange, -}: { - options: ConfigOptionInfo[]; - onChange: (configId: string, value: string) => void; -}) { - const [open, setOpen] = useState(false); - const option = options.find( - (candidate) => - candidate.category === "collaboration_mode" || - candidate.id === "collaboration_mode" - ); - if (!option || option.choices.length < 2) return null; - const current = option.choices.find((choice) => choice.id === option.current); - return ( - - - - {current?.name ?? option.current} - - - } - /> - - {option.name} - {option.choices.map((choice) => ( - { - onChange(option.id, choice.id); - setOpen(false); - }} - /> - ))} - - - ); -} - -export function GoalPicker({ - capability, - goal, - onGoal, -}: { - capability: GoalCapabilityInfo | null; - goal: GoalSnapshot | null; - onGoal: ( - action: "set" | "pause" | "resume" | "clear", - objective?: string - ) => Promise; -}) { - const t = useT(); - const [open, setOpen] = useState(false); - const [objective, setObjective] = useState(""); - const [pending, setPending] = useState(false); - if (!capability) return null; - const run = async (action: "set" | "pause" | "resume" | "clear") => { - setPending(true); - try { - await onGoal(action, action === "set" ? objective.trim() : undefined); - if (action === "set") setObjective(""); - } finally { - setPending(false); - } - }; - const can = (action: string) => capability.actions.includes(action); - return ( - - - - - {goal?.objective ?? t("goal.label")} - - - - } - /> - - {goal ? ( -
-
-

- {goal.objective} -

-

- {td(t, `goal.status.${goal.status}`)} -

-
-
- {goal.status === "paused" && can("resume") ? ( - - ) : can("pause") ? ( - - ) : null} - {can("clear") ? ( - - ) : null} -
-
- ) : can("set") ? ( -
{ - event.preventDefault(); - if (objective.trim()) void run("set"); - }} - > - setObjective(event.currentTarget.value)} - placeholder={t("goal.placeholder")} - aria-label={t("goal.objective")} - /> - -
- ) : null} - - - ); -} - const WORKTREE_BASELINES = ["current", "origin_default"] as const; /** Worktree isolation is a baseline choice, not a boolean: both commit sources stay explicit. */ @@ -917,12 +738,8 @@ export function WorktreePicker({ config }: { config: SessionConfig }) { * chip picks the family, the second the effort, and together they resolve to one of the adapter's * own ids. * - * Both APIs are optional and many adapters skip both, in which case the flat list is the core's - * built-in one for that provider rather than the agent's own — same shape either way. Only a - * provider we have no list for (a custom one) falls through to the note explaining that its CLI - * config decides. Before a session exists, the main composer only shows providers with an - * advertised model list; host surfaces may keep the explicit affordance visible while metadata is - * loading. Provider-owned config options still arrive after session creation. + * Both APIs are optional. Before session creation only CLI-discovered choices are available; + * otherwise the provider owns its default and can report model/config choices after connection. */ export function ModelPicker({ models, @@ -959,11 +776,7 @@ export function ModelPicker({ const [browseProvider, setBrowseProvider] = useState(provider); const modelTriggerRef = useRef(null); const providerSwitcherEnabled = providerConfig !== undefined; - const providerRegistry = providerConfig - ? providerConfig.providers.length > 0 - ? providerConfig.providers - : fallbackProviders() - : []; + const providerRegistry = providerConfig?.providers ?? []; const providerChoices = providerConfig ? providerRegistry.filter( (candidate) => @@ -1052,7 +865,9 @@ export function ModelPicker({ let effortLabel = ""; let effortRows: PickerRow[] = []; - if (modelOpt) { + if (providerConfig?.providerSwitching === true) { + modelLabel = t("composer.switchingProvider"); + } else if (modelOpt) { // Keep the trigger anchored to the active Provider even while the popup browses another one. modelLabel = (modelOpt.choices.find((c) => c.id === modelOpt.current)?.name ?? @@ -1216,6 +1031,11 @@ export function ModelPicker({ > @@ -1263,7 +1083,11 @@ export function ModelPicker({ aria-label={displayName} aria-selected={selected} data-selected={selected ? "true" : "false"} - disabled={unavailable} + disabled={ + disabled || + unavailable || + providerConfig.providersStatus !== "ready" + } className="max-w-40 shrink-0 justify-start px-2 font-normal" onClick={() => { setModelSearch(""); @@ -1519,9 +1343,6 @@ export function Composer({ onSteer, onStop, steeringSupported, - goalCapability, - goal, - onGoal, onAttachFile, onAttachImages, onInsertSkill, @@ -1673,12 +1494,6 @@ export function Composer({ - - - { /* empty */ }} - turns={previewTurns} - onOpenPlanAsDocument={() => { - /* empty */ - }} />
diff --git a/apps/desktop/src/session/SideChatPanel.tsx b/apps/desktop/src/session/SideChatPanel.tsx index 1541ac97..b9ca172c 100644 --- a/apps/desktop/src/session/SideChatPanel.tsx +++ b/apps/desktop/src/session/SideChatPanel.tsx @@ -164,7 +164,6 @@ const TURN_EVENTS = new Set([ "agent_text", "agent_thought", "tool_call", - "plan", "turn_ended", "error", ]); diff --git a/apps/desktop/src/session/TaskPlanPanel.tsx b/apps/desktop/src/session/TaskPlanPanel.tsx deleted file mode 100644 index f6e477d9..00000000 --- a/apps/desktop/src/session/TaskPlanPanel.tsx +++ /dev/null @@ -1,205 +0,0 @@ -import { Button } from "@/components/ui/button"; -import { - CheckCircle2, - Circle, - CircleDot, - ListTodo, -} from "@/components/ui/icons"; -import { Progress } from "@/components/ui/progress"; -import { Separator } from "@/components/ui/separator"; -import { cn } from "@/lib/utils"; - -import type { PlanEntry } from "../bridge"; -import { useT } from "../i18n"; -import type { Turn } from "./turns"; - -export type TaskPlanStatus = "pending" | "in_progress" | "completed"; - -const CHECKBOX_MARKER = /^\s*(?:-\s*)?\[([ xX])\]\s*(.*)$/u; - -export function taskPlanStatus(entry: PlanEntry): TaskPlanStatus { - const status = (entry.status ?? "").trim().toLowerCase().replaceAll("-", "_"); - if ( - ["completed", "complete", "done", "succeeded", "success"].includes(status) - ) { - return "completed"; - } - if (["in_progress", "active", "running", "started"].includes(status)) { - return "in_progress"; - } - if (status) return "pending"; - return CHECKBOX_MARKER.exec(entry.content)?.[1]?.toLowerCase() === "x" - ? "completed" - : "pending"; -} - -export function taskPlanLabel(entry: PlanEntry): string { - return CHECKBOX_MARKER.exec(entry.content)?.[2] ?? entry.content; -} - -export function planChecklistMarkdown( - entries: readonly (PlanEntry | string)[] -): string { - return entries - .map((entry) => { - const normalized = typeof entry === "string" ? { content: entry } : entry; - const marked = CHECKBOX_MARKER.exec(normalized.content); - const explicitStatus = "status" in normalized ? normalized.status : null; - if (marked && explicitStatus == null) { - return `- [${marked[1] === " " ? " " : "x"}] ${marked[2]}`; - } - const checked = taskPlanStatus(normalized) === "completed" ? "x" : " "; - return `- [${checked}] ${marked?.[2] ?? normalized.content}`; - }) - .join("\n"); -} - -export function currentTaskPlan(turns: readonly Turn[]): readonly PlanEntry[] { - return turns.at(-1)?.plan ?? []; -} - -function StatusIcon({ status }: { status: TaskPlanStatus }) { - const t = useT(); - if (status === "completed") { - return ( - - ); - } - if (status === "in_progress") { - return ( - - ); - } - return ( - - ); -} - -export function TaskPlanPanel({ - turns, - onOpenPlanAsDocument, - onPinPlanArtifact, - canPinPlan = false, -}: { - turns: readonly Turn[]; - onOpenPlanAsDocument?: (entries: PlanEntry[]) => void; - onPinPlanArtifact?: (markdown: string) => void; - canPinPlan?: boolean; -}) { - const t = useT(); - const entries = currentTaskPlan(turns); - const statuses = entries.map(taskPlanStatus); - const completed = statuses.filter((status) => status === "completed").length; - const currentIndex = statuses.indexOf("in_progress"); - const currentStep = - currentIndex === -1 - ? entries.length > 0 - ? Math.min(completed + 1, entries.length) - : 0 - : currentIndex + 1; - const progress = entries.length > 0 ? (completed / entries.length) * 100 : 0; - - if (entries.length === 0) { - return null; - } - - return ( -
- -
- -

- {t("taskPlan.title")} -

-

- {t("taskPlan.step", { current: currentStep, total: entries.length })} -

-
-
- -
- -
    - {entries.map((entry, index) => { - const status = statuses[index] ?? "pending"; - return ( -
  1. - - - {taskPlanLabel(entry)} - -
    2. - ); - })} -
- - {(onOpenPlanAsDocument != null || - (canPinPlan && onPinPlanArtifact != null)) && ( -
- {onOpenPlanAsDocument && ( - - )} - {canPinPlan && onPinPlanArtifact && ( - - )} -
- )} -
- ); -} diff --git a/apps/desktop/src/session/TrajectoryView.tsx b/apps/desktop/src/session/TrajectoryView.tsx index dd28ca44..5cb60dcc 100644 --- a/apps/desktop/src/session/TrajectoryView.tsx +++ b/apps/desktop/src/session/TrajectoryView.tsx @@ -35,7 +35,6 @@ const KIND_LABEL: Record = { reasoning: "trajectory.kind.reasoning", tool: "trajectory.kind.tool", memory: "trajectory.kind.memory", - plan: "trajectory.kind.plan", error: "trajectory.kind.error", }; @@ -51,7 +50,6 @@ const KIND_TONE: Record = { reasoning: "bg-muted-foreground", tool: "bg-warning", memory: "bg-success", - plan: "bg-primary/65", error: "bg-destructive", }; @@ -62,7 +60,6 @@ const FILTER_KINDS: (TrajectoryKind | "all")[] = [ "reasoning", "tool", "memory", - "plan", "error", ]; diff --git a/apps/desktop/src/session/TurnCard.tsx b/apps/desktop/src/session/TurnCard.tsx index a997ecac..13d3f067 100644 --- a/apps/desktop/src/session/TurnCard.tsx +++ b/apps/desktop/src/session/TurnCard.tsx @@ -508,7 +508,7 @@ function requestCanvasDuplicate(canvas: CanvasHistoryMarker): void { ); } -/** A collapsible group of secondary detail (agents / thinking / plan / memory). */ +/** A collapsible group of secondary detail (agents / thinking / memory). */ function Detail({ icon: Icon, label, @@ -680,7 +680,7 @@ function AgentRoster({ agents }: { agents: readonly AgentActivity[] }) { * The prompt sits in a bubble on the right and the answer runs full width beneath it, so a long * transcript reads as a conversation instead of a stack of equally-weighted cards. Tool calls keep * their streamed position, with adjacent calls sharing one disclosure; thinking and memory - * metadata stay collapsed underneath. The current task plan lives in the right information panel. + * metadata stay collapsed underneath. */ export const TurnCard = memo( ({ diff --git a/apps/desktop/src/session/composerDrafts.ts b/apps/desktop/src/session/composerDrafts.ts index 730dcecd..4d0e17a3 100644 --- a/apps/desktop/src/session/composerDrafts.ts +++ b/apps/desktop/src/session/composerDrafts.ts @@ -29,7 +29,6 @@ export interface ComposerDraftPosture { mode: PermissionMode; sandbox: Sandbox; worktreeBase: WorktreeBaselineKind | null; - planMode: boolean; memoryRead: MemoryAccess; memoryWrite: MemoryAccess; scene: string | null; @@ -269,7 +268,6 @@ function parsePosture(value: unknown): ComposerDraftPosture | null { posture.worktreeBase === "current" || posture.worktreeBase === "origin_default" ) || - typeof posture.planMode !== "boolean" || !isMemoryAccess(posture.memoryRead) || !isMemoryAccess(posture.memoryWrite) || !nullableStringWithin(posture.scene, 512) || @@ -286,7 +284,6 @@ function parsePosture(value: unknown): ComposerDraftPosture | null { mode: posture.mode, sandbox: posture.sandbox, worktreeBase: posture.worktreeBase, - planMode: posture.planMode, memoryRead: posture.memoryRead, memoryWrite: posture.memoryWrite, scene: diff --git a/apps/desktop/src/session/config.ts b/apps/desktop/src/session/config.ts index a94288ae..9f0573cc 100644 --- a/apps/desktop/src/session/config.ts +++ b/apps/desktop/src/session/config.ts @@ -23,6 +23,7 @@ export interface SessionConfig { onProvider: (v: string) => void; /** A running turn or in-flight runtime replacement owns the provider boundary. */ providerChangeDisabled?: boolean; + providerSwitching?: boolean; /** A foreign Provider choice replaces the active runtime; null leaves its model unspecified. */ onProviderModel: (provider: string, model: string | null) => void; onReloadProviders: () => void; @@ -40,8 +41,6 @@ export interface SessionConfig { worktreeOptions: WorktreeBaselineOption[]; worktreeOptionsLoading: boolean; onWorktreeBase: (v: WorktreeBaselineKind | null) => void; - planMode: boolean; - onPlan: (v: boolean) => void; /** Component-policy gate for the memory picker and its persistence calls. */ memoryEnabled: boolean; memoryRead: MemoryAccess; diff --git a/apps/desktop/src/session/scene.ts b/apps/desktop/src/session/scene.ts index 9b8f0861..fe5571a5 100644 --- a/apps/desktop/src/session/scene.ts +++ b/apps/desktop/src/session/scene.ts @@ -62,7 +62,6 @@ export interface SceneExecution { session_mode?: SessionMode; memory_preset?: MemoryPresetId; worktree?: "off" | "current" | "origin_default"; - plan_first?: boolean; } export interface SceneSkills { @@ -200,7 +199,6 @@ export interface LivePosture { mode: SessionMode; memoryRead: MemoryAccess; memoryWrite: MemoryAccess; - planFirst: boolean; provider: string; model: string | null; } @@ -227,11 +225,6 @@ export function sceneCustomized(scene: SceneInfo, live: LivePosture): boolean { if (preset.read !== live.memoryRead || preset.write !== live.memoryWrite) return true; } - if ( - execution.plan_first !== undefined && - execution.plan_first !== live.planFirst - ) - return true; if ( execution.providers !== undefined && execution.providers.length > 0 && @@ -339,26 +332,6 @@ export function sceneEffortChoice( return choice ? { configId: option.id, value: choice.id } : null; } -/** - * Resolve a scene's plan posture through the provider-owned collaboration-mode selector. - * There is deliberately no fallback prompt/skill: without this native option the scene leaves - * `plan_first` pending instead of pretending the provider changed modes. - */ -export function sceneCollaborationChoice( - options: readonly EffortOptionLike[], - planFirst: boolean -): { configId: string; value: string } | null { - const option = options.find( - (o) => o.category === "collaboration_mode" || o.id === "collaboration_mode" - ); - if (!option) return null; - const wanted = planFirst ? "plan" : "default"; - const choice = option.choices.find( - (candidate) => candidate.id.toLowerCase() === wanted - ); - return choice ? { configId: option.id, value: choice.id } : null; -} - /** The slice of a skill listing the scene-aware `/` picker needs. */ export interface SkillLike { id: string; diff --git a/apps/desktop/src/session/trajectory.ts b/apps/desktop/src/session/trajectory.ts index 62994179..45c40983 100644 --- a/apps/desktop/src/session/trajectory.ts +++ b/apps/desktop/src/session/trajectory.ts @@ -7,7 +7,6 @@ export type TrajectoryKind = | "reasoning" | "tool" | "memory" - | "plan" | "error"; export type TrajectoryLane = "context" | "assistant" | "tool"; @@ -145,23 +144,6 @@ export function deriveTrajectory( }); } - if (turn.plan.length > 0) { - records.push({ - id: `turn:${turn.id}:plan`, - index: 0, - kind: "plan", - lane: "assistant", - turn: turnNumber, - step: 1, - title: "Plan", - summary: compact(turn.plan.map((entry) => entry.content).join(" · ")), - startAt: turn.startedAt, - endAt: turn.startedAt, - running: false, - output: turn.plan, - }); - } - const tools = new Map(turn.tools.map((tool) => [tool.id, tool])); let step = 1; let assistantSegment = 0; diff --git a/apps/desktop/src/session/turns.ts b/apps/desktop/src/session/turns.ts index f300c497..6c14d029 100644 --- a/apps/desktop/src/session/turns.ts +++ b/apps/desktop/src/session/turns.ts @@ -3,7 +3,6 @@ import type { DocBlock, MemoryReceipt, Part, - PlanEntry, ToolOutput, TranscriptEntry, } from "../bridge"; @@ -109,21 +108,6 @@ export interface ToolEntry { lastTranscriptSeq?: number; } -/** Normalize durable legacy string entries and current structured ACP plan entries once. */ -export function normalizePlanEntries( - entries: readonly (PlanEntry | string)[] -): PlanEntry[] { - return entries.map((entry) => - typeof entry === "string" - ? { content: entry, priority: null, status: null } - : { - content: entry.content, - priority: entry.priority ?? null, - status: entry.status ?? null, - } - ); -} - /** * The render-order projection of one turn. Text chunks stay as independent atoms so a tool call * can sit between two streamed answer fragments without splitting the durable assistant message. @@ -187,7 +171,6 @@ export interface Turn { thoughts: string[]; tools: ToolEntry[]; content: TurnContentEntry[]; - plan: PlanEntry[]; memory?: MemoryReceipt; error?: string; stopReason?: string; @@ -218,7 +201,6 @@ export function newTurn( thoughts: [], tools: [], content: [], - plan: [], startedAt: Date.now(), }; } @@ -461,6 +443,7 @@ export function applyEvent( ): Turn[] { // Events that don't belong to a turn. if ( + ev.event === "provider_changed" || ev.event === "session_created" || ev.event === "session_title_changed" || ev.event === "session_activity_changed" || @@ -468,7 +451,6 @@ export function applyEvent( ev.event === "models" || ev.event === "config_options" || ev.event === "session_capabilities" || - ev.event === "goal_changed" || ev.event === "permission_request" ) { return turns; @@ -674,10 +656,6 @@ export function applyEvent( ); break; } - case "plan": { - cur.plan = normalizePlanEntries(ev.entries); - break; - } case "turn_ended": { cur.stopReason = ev.stop_reason; cur.endedAt = Date.now(); @@ -711,9 +689,6 @@ export function applyEvent( case "hook_turn_started": { throw new Error('Not implemented yet: "hook_turn_started" case'); } - case "provider_changed": { - throw new Error('Not implemented yet: "provider_changed" case'); - } case "session_cost": { throw new Error('Not implemented yet: "session_cost" case'); } @@ -881,7 +856,6 @@ export function mergeLoadedTurns( liveTurn.content, liveTurn.streamBoundaryKnown ), - plan: liveTurn.plan.length > 0 ? liveTurn.plan : loadedTail.plan, error: liveTurn.error ?? loadedTail.error, stopReason: liveTurn.stopReason ?? loadedTail.stopReason, startedAt: Math.min(loadedTail.startedAt, liveTurn.startedAt), @@ -913,6 +887,7 @@ export function turnsFromTranscript( createdAt?: number, startedAt?: number ) => { + if (part.kind === "plan") return; const at = createdAt != null && createdAt > 0 ? createdAt : Date.now(); if (role === "user" && (part.kind === "text" || part.kind === "prompt")) { out.push({ @@ -969,10 +944,6 @@ export function turnsFromTranscript( cur.content = appendToolContent(cur.content, part.id, seq, at); break; } - case "plan": { - cur.plan = normalizePlanEntries(part.entries); - break; - } } cur.endedAt = Math.max(cur.endedAt ?? at, at); }; diff --git a/apps/desktop/tests/checkoutPickerRendered.test.tsx b/apps/desktop/tests/checkoutPickerRendered.test.tsx index e0e7adb9..2581ff38 100644 --- a/apps/desktop/tests/checkoutPickerRendered.test.tsx +++ b/apps/desktop/tests/checkoutPickerRendered.test.tsx @@ -61,8 +61,7 @@ function config(overrides = {}) { ], worktreeOptionsLoading: false, onWorktreeBase: () => {}, - planMode: false, - onPlan: () => {}, + memoryEnabled: true, memoryRead: "inherit", memoryWrite: "inherit", diff --git a/apps/desktop/tests/composerDrafts.test.ts b/apps/desktop/tests/composerDrafts.test.ts index af7c06de..de57a223 100644 --- a/apps/desktop/tests/composerDrafts.test.ts +++ b/apps/desktop/tests/composerDrafts.test.ts @@ -37,7 +37,7 @@ const posture: ComposerDraftPosture = { mode: "ask", sandbox: "workspace_write", worktreeBase: "current", - planMode: false, + memoryRead: "inherit", memoryWrite: "allow", scene: "review", @@ -216,3 +216,24 @@ describe("composer drafts", () => { expect(storage.getItem(COMPOSER_DRAFT_STORAGE_KEY)).toBe("last-good-copy"); }); }); + +test("legacy plan posture is discarded without losing the draft", () => { + const storage = new MemoryStorage(); + const drafts = updateComposerDraft( + new Map(), + { + scope: project, + doc, + attachments: [], + posture: { ...posture, planMode: true } as ComposerDraftPosture, + }, + { createId: () => "legacy-draft", now: 1 } + ); + saveComposerDrafts(drafts, storage); + const loaded = loadComposerDrafts(storage).drafts.get( + composerDraftScopeKey(project) + ); + expect(loaded?.doc).toEqual(doc); + expect(loaded?.posture.provider).toBe("codex"); + expect(loaded?.posture).not.toHaveProperty("planMode"); +}); diff --git a/apps/desktop/tests/issuesDelegation.test.tsx b/apps/desktop/tests/issuesDelegation.test.tsx index 90686ad0..c04bd024 100644 --- a/apps/desktop/tests/issuesDelegation.test.tsx +++ b/apps/desktop/tests/issuesDelegation.test.tsx @@ -49,7 +49,7 @@ function sceneInfo(overrides = {}) { reference: "builtin:develop", name: "develop", title: "Develop", - description: "Plan-first implementation", + description: "Implementation", icon: "🛠️", source: "builtin", keywords: [], diff --git a/apps/desktop/tests/planDocument.test.tsx b/apps/desktop/tests/planDocument.test.tsx deleted file mode 100644 index 82e288f6..00000000 --- a/apps/desktop/tests/planDocument.test.tsx +++ /dev/null @@ -1,129 +0,0 @@ -// @ts-nocheck -import { afterEach, describe, expect, test } from "bun:test"; - -import { - activateDom, - click, - dom, - flush, - mount, - restoreDom, -} from "./domTestHarness"; - -activateDom(); -const { TaskPlanPanel, planChecklistMarkdown } = - await import("../src/session/TaskPlanPanel"); -const { I18nProvider } = await import("../src/i18n"); - -afterEach(() => { - dom.document.body.replaceChildren(); - restoreDom(); -}); - -function finishedTurn(overrides = {}) { - return { - id: 1, - accepted: true, - streamBoundaryKnown: true, - prompt: "Implement the feature", - text: "Done.", - textDeltas: [], - observedTextDeltas: 0, - observedThoughtDeltas: 0, - pendingTextDeltaSkips: 0, - pendingThoughtDeltaSkips: 0, - thoughts: [], - tools: [], - plan: [ - { content: "Survey the code", status: "in_progress" }, - { content: "Write the fix", status: "completed" }, - ], - startedAt: 1, - endedAt: 2, - ...overrides, - }; -} - -function renderPanel(props = {}) { - return mount( - - - - ); -} - -// An earlier suite in the same bun run leaks a key-echo `useT` module mock, so a label renders -// as either its English translation or its raw i18n key depending on file order. Accept both. -const OPEN_LABELS = ["Open as document", "planDoc.open"]; -const PIN_LABELS = ["Pin as artifact", "planDoc.pin"]; - -function buttonByLabel(rendered, labels) { - return [...rendered.container.querySelectorAll("button")].find((el) => - labels.includes(el.textContent?.trim()) - ); -} - -describe("planChecklistMarkdown", () => { - test("converts entries to a checklist, preserving markers and structured status", () => { - expect( - planChecklistMarkdown([ - "Survey the code", - "[x] Write the fix", - "- [ ] Test it", - { content: "Ship it", status: "completed" }, - ]) - ).toBe( - "- [ ] Survey the code\n- [x] Write the fix\n- [ ] Test it\n- [x] Ship it" - ); - }); -}); - -describe("TaskPlanPanel plan-as-document", () => { - test("offers Open as document and calls the handler with the entries", async () => { - activateDom(); - const opened = []; - const rendered = renderPanel({ - onOpenPlanAsDocument: (entries) => opened.push(entries), - }); - - const open = buttonByLabel(rendered, OPEN_LABELS); - expect(open).toBeTruthy(); - // Pin is gated on a plan-declaring scene; none is active here. - expect(buttonByLabel(rendered, PIN_LABELS)).toBeFalsy(); - - click(open); - await flush(); - expect(opened).toEqual([ - [ - { content: "Survey the code", status: "in_progress" }, - { content: "Write the fix", status: "completed" }, - ], - ]); - rendered.unmount(); - }); - - test("offers Pin as artifact only when the scene declares a plan, passing checklist markdown", async () => { - activateDom(); - const pinned = []; - const rendered = renderPanel({ - onOpenPlanAsDocument: () => {}, - onPinPlanArtifact: (markdown) => pinned.push(markdown), - canPinPlan: true, - }); - - const pin = buttonByLabel(rendered, PIN_LABELS); - expect(pin).toBeTruthy(); - click(pin); - await flush(); - expect(pinned).toEqual(["- [ ] Survey the code\n- [x] Write the fix"]); - rendered.unmount(); - }); - - test("hides both affordances when no handlers are wired", async () => { - activateDom(); - const rendered = renderPanel(); - expect(buttonByLabel(rendered, OPEN_LABELS)).toBeFalsy(); - expect(buttonByLabel(rendered, PIN_LABELS)).toBeFalsy(); - rendered.unmount(); - }); -}); diff --git a/apps/desktop/tests/reasoningScaleRendered.test.tsx b/apps/desktop/tests/reasoningScaleRendered.test.tsx index 2460f7a6..f4f18f89 100644 --- a/apps/desktop/tests/reasoningScaleRendered.test.tsx +++ b/apps/desktop/tests/reasoningScaleRendered.test.tsx @@ -11,8 +11,7 @@ import { } from "./domTestHarness"; activateDom(); -const { CollaborationModePicker, GoalPicker, ModelPicker } = - await import("../src/session/Composer"); +const { ModelPicker } = await import("../src/session/Composer"); const { I18nProvider } = await import("../src/i18n"); const { hiddenModelsForProvider, setModelHidden } = await import("../src/session/modelPreferences"); @@ -433,66 +432,33 @@ describe("ModelPicker", () => { }); }); -describe("provider-native session controls", () => { - test("renders collaboration mode only from an advertised provider option", () => { - activateDom(); - const absent = mount( - {}} /> - ); - expect(absent.container.querySelector("button")).toBeNull(); - absent.unmount(); - - const rendered = mount( - {}} +test("shows pending switching state without inventing provider choices", async () => { + activateDom(); + const rendered = mount( + + {}} + configOptions={[]} + onConfigOption={() => {}} + hasSession + disabled + providerConfig={{ + providers: [], + providersStatus: "loading", + provider: "custom-agent", + providerSwitching: true, + }} /> - ); - expect( - rendered.container.querySelector( - 'button[aria-label="Collaboration mode: Plan"]' - ) - ).toBeTruthy(); - rendered.unmount(); - }); - - test("renders Goal only when the provider advertises the extension", () => { - activateDom(); - const absent = mount( - - {}} /> - - ); - expect(absent.container.querySelector("button")).toBeNull(); - absent.unmount(); - - const rendered = mount( - - {}} - /> - - ); - expect( - rendered.container.querySelector('button[aria-label="Goal"]') - ).toBeTruthy(); - rendered.unmount(); - }); + + ); + const trigger = rendered.container.querySelector('button[title="Model"]'); + expect(trigger?.disabled).toBe(true); + expect(rendered.container.querySelector('[role="status"]')?.textContent).toBe( + "Switching agent…" + ); + rendered.unmount(); }); diff --git a/apps/desktop/tests/retiredSessionFeatures.test.ts b/apps/desktop/tests/retiredSessionFeatures.test.ts new file mode 100644 index 00000000..d7950e21 --- /dev/null +++ b/apps/desktop/tests/retiredSessionFeatures.test.ts @@ -0,0 +1,29 @@ +import { expect, test } from "bun:test"; + +import type { Part } from "../src/bridge"; +import { turnsFromTranscript } from "../src/session/turns"; + +test("legacy plan-only history produces no empty turn", () => { + expect( + turnsFromTranscript([["agent", { kind: "plan", entries: ["old item"] }]]) + ).toEqual([]); +}); + +test("legacy plans do not replace or reappear in normal conversation", () => { + const entries: [string, Part][] = [ + ["user", { kind: "text", text: "Keep working" }], + [ + "agent", + { + kind: "plan", + entries: [{ content: "Retired plan", status: "pending" }], + }, + ], + ["agent", { kind: "text", text: "Current answer" }], + ]; + const turns = turnsFromTranscript(entries); + expect(turns).toHaveLength(1); + expect(turns[0].prompt).toBe("Keep working"); + expect(turns[0].text).toBe("Current answer"); + expect(turns[0]).not.toHaveProperty("plan"); +}); diff --git a/apps/desktop/tests/scene.test.ts b/apps/desktop/tests/scene.test.ts index 1fdcfac4..0c3f49e3 100644 --- a/apps/desktop/tests/scene.test.ts +++ b/apps/desktop/tests/scene.test.ts @@ -4,7 +4,6 @@ import { MEMORY_PRESET_POLICY, escalationNeeded, nextSceneInRing, - sceneCollaborationChoice, sceneCustomized, sceneTitle, softApplyPending, @@ -30,7 +29,6 @@ const live: LivePosture = { mode: "ask", memoryRead: "inherit", memoryWrite: "inherit", - planFirst: false, provider: "claude_code", model: "m1", }; @@ -42,10 +40,8 @@ describe("sceneCustomized", () => { test("only fields the scene sets participate", () => { const s = scene({ execution: { session_mode: "ask" } }); - // Memory/plan/provider all differ from nothing — the scene doesn't set them. - expect( - sceneCustomized(s, { ...live, planFirst: true, provider: "codex" }) - ).toBe(false); + // Memory/provider all differ from nothing — the scene doesn't set them. + expect(sceneCustomized(s, { ...live, provider: "codex" })).toBe(false); expect(sceneCustomized(s, { ...live, mode: "auto_edit" })).toBe(true); }); @@ -70,7 +66,6 @@ describe("softApplyPending (binding matrix)", () => { execution: { session_mode: "read_only", memory_preset: "standard", - plan_first: true, providers: ["codex"], model: "m2", reasoning_effort: "high", @@ -91,31 +86,6 @@ describe("softApplyPending (binding matrix)", () => { }); }); -describe("sceneCollaborationChoice", () => { - const options = [ - { - id: "collaboration_mode", - category: "collaboration_mode", - choices: [{ id: "default" }, { id: "plan" }], - }, - ]; - - test("maps scene plan posture onto the provider-native values", () => { - expect(sceneCollaborationChoice(options, true)).toEqual({ - configId: "collaboration_mode", - value: "plan", - }); - expect(sceneCollaborationChoice(options, false)).toEqual({ - configId: "collaboration_mode", - value: "default", - }); - }); - - test("fails closed when the provider does not advertise collaboration mode", () => { - expect(sceneCollaborationChoice([], true)).toBeNull(); - }); -}); - describe("escalationNeeded", () => { test("is asymmetric: loosening needs confirmation, tightening never does", () => { const looser = scene({ execution: { session_mode: "full_access" } }); diff --git a/apps/desktop/tests/sceneChip.test.tsx b/apps/desktop/tests/sceneChip.test.tsx index 3e1125d5..8396c44d 100644 --- a/apps/desktop/tests/sceneChip.test.tsx +++ b/apps/desktop/tests/sceneChip.test.tsx @@ -29,7 +29,7 @@ function sceneInfo(overrides = {}) { reference: "builtin:develop", name: "develop", title: "Develop", - description: "Plan-first implementation", + description: "Implementation", icon: "🛠️", source: "builtin", keywords: [], @@ -60,8 +60,7 @@ function config(overrides = {}) { worktreeOptions: [], worktreeOptionsLoading: false, onWorktreeBase: () => {}, - planMode: false, - onPlan: () => {}, + memoryEnabled: true, memoryRead: "inherit", memoryWrite: "inherit", @@ -130,7 +129,7 @@ describe("Provider/model picker", () => { rendered.unmount(); }); - test("keeps known providers selectable and offers retry when desktop detection fails", async () => { + test("offers retry without fabricated providers when desktop detection fails", async () => { activateDom(); let retries = 0; const rendered = mount( @@ -177,8 +176,7 @@ describe("Provider/model picker", () => { '[data-slot="popover-content"]' ); expect(trigger?.textContent).toContain("Default model"); - button(popup, "Grok"); - button(popup, "Codex"); + expect(popup?.querySelectorAll('[role="option"]').length).toBe(0); button(popup, "Retry").click(); expect(retries).toBe(1); } finally { @@ -236,7 +234,7 @@ describe("SceneChip", () => { ?.className ).toContain("max-h-(--available-height)"); const detail = [...dom.document.body.querySelectorAll("span")].find( - (node) => node.textContent === "Plan-first implementation" + (node) => node.textContent === "Implementation" ); expect(detail?.classList.contains("whitespace-normal")).toBe(true); expect( diff --git a/apps/desktop/tests/sceneStudioRendered.test.tsx b/apps/desktop/tests/sceneStudioRendered.test.tsx index 616b6a90..1b63e09e 100644 --- a/apps/desktop/tests/sceneStudioRendered.test.tsx +++ b/apps/desktop/tests/sceneStudioRendered.test.tsx @@ -19,7 +19,7 @@ function scene(overrides = {}) { reference: "builtin:develop", name: "develop", title: "Develop", - description: "Plan-first implementation", + description: "Implementation", icon: null, source: "builtin", plugin_id: null, diff --git a/apps/desktop/tests/sessionState.test.ts b/apps/desktop/tests/sessionState.test.ts index 056ad908..d9100331 100644 --- a/apps/desktop/tests/sessionState.test.ts +++ b/apps/desktop/tests/sessionState.test.ts @@ -917,3 +917,23 @@ describe("execution policy projection", () => { ).toBe(false); }); }); + +test("provider replacement is metadata and never creates or mutates a turn", () => { + const turns = [newTurn("Keep this draft")]; + expect( + applyEvent(turns, { + event: "provider_changed", + session: "s", + provider: "pi", + model: null, + }) + ).toBe(turns); + expect( + applyEvent([], { + event: "provider_changed", + session: "s", + provider: "pi", + model: null, + }) + ).toEqual([]); +}); diff --git a/crates/core/examples/live_demo.rs b/crates/core/examples/live_demo.rs index 16c34c2f..8b3e87c6 100644 --- a/crates/core/examples/live_demo.rs +++ b/crates/core/examples/live_demo.rs @@ -182,14 +182,6 @@ async fn main() -> Result<(), Box> { .await .ok(); } - Event::Plan { entries, .. } => println!( - " ☰ plan: {}", - entries - .iter() - .map(|entry| entry.content.as_str()) - .collect::>() - .join(", ") - ), Event::TurnEnded { stop_reason, .. } => { println!("\n■ turn ended: {stop_reason}"); break; @@ -231,7 +223,6 @@ async fn main() -> Result<(), Box> { | Event::HookTurnStarted { .. } | Event::SessionCost { .. } | Event::SessionCapabilities { .. } - | Event::GoalChanged { .. } | Event::PromptQueued { .. } | Event::SteerAccepted { .. } | Event::WorktreeDiscarded { .. } => {} diff --git a/crates/core/schemas/agent-scenes/1.0.0/examples/acceptance.scene.json b/crates/core/schemas/agent-scenes/1.0.0/examples/acceptance.scene.json index e8f4aea4..b7fecd57 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/examples/acceptance.scene.json +++ b/crates/core/schemas/agent-scenes/1.0.0/examples/acceptance.scene.json @@ -9,8 +9,7 @@ }, "execution": { "session_mode": "read_only", - "memory_preset": "standard", - "plan_first": false + "memory_preset": "standard" }, "skills": { "inline": [ diff --git a/crates/core/schemas/agent-scenes/1.0.0/examples/develop.scene.json b/crates/core/schemas/agent-scenes/1.0.0/examples/develop.scene.json index bb1647b0..93e3b7fa 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/examples/develop.scene.json +++ b/crates/core/schemas/agent-scenes/1.0.0/examples/develop.scene.json @@ -3,15 +3,14 @@ "name": "develop", "version": "1.0.0", "title": "Develop", - "description": "Plan-first implementation in an isolated worktree, producing a plan and a change summary.", + "description": "Implementation in an isolated worktree, producing a plan and a change summary.", "localizations": { "zh-CN": { "title": "开发", "description": "在隔离 worktree 中计划先行地实现，产出计划与变更摘要。" } }, "execution": { "session_mode": "auto_edit", "memory_preset": "standard", - "worktree": "current", - "plan_first": true + "worktree": "current" }, "skills": { "inline": [ diff --git a/crates/core/schemas/agent-scenes/1.0.0/examples/fix.scene.json b/crates/core/schemas/agent-scenes/1.0.0/examples/fix.scene.json index 10c24e0a..8fb3cb79 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/examples/fix.scene.json +++ b/crates/core/schemas/agent-scenes/1.0.0/examples/fix.scene.json @@ -9,8 +9,7 @@ }, "execution": { "session_mode": "auto_edit", - "memory_preset": "standard", - "plan_first": false + "memory_preset": "standard" }, "skills": { "inline": [ diff --git a/crates/core/schemas/agent-scenes/1.0.0/examples/research.scene.json b/crates/core/schemas/agent-scenes/1.0.0/examples/research.scene.json index a6c7b917..602b9165 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/examples/research.scene.json +++ b/crates/core/schemas/agent-scenes/1.0.0/examples/research.scene.json @@ -10,8 +10,7 @@ "execution": { "session_mode": "read_only", "memory_preset": "standard", - "worktree": "off", - "plan_first": false + "worktree": "off" }, "skills": { "inline": [ diff --git a/crates/core/schemas/agent-scenes/1.0.0/examples/test.scene.json b/crates/core/schemas/agent-scenes/1.0.0/examples/test.scene.json index 21c07fd4..f619eb1f 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/examples/test.scene.json +++ b/crates/core/schemas/agent-scenes/1.0.0/examples/test.scene.json @@ -9,8 +9,7 @@ }, "execution": { "session_mode": "auto_edit", - "memory_preset": "standard", - "plan_first": false + "memory_preset": "standard" }, "skills": { "inline": [ diff --git a/crates/core/schemas/agent-scenes/1.0.0/scene.schema.json b/crates/core/schemas/agent-scenes/1.0.0/scene.schema.json index 35547c38..fd7f8ea7 100644 --- a/crates/core/schemas/agent-scenes/1.0.0/scene.schema.json +++ b/crates/core/schemas/agent-scenes/1.0.0/scene.schema.json @@ -49,8 +49,7 @@ "reasoning_effort": { "type": "string" }, "session_mode": { "enum": ["read_only", "ask", "auto_edit", "full_access"] }, "memory_preset": { "enum": ["standard", "read_only", "private", "learn_only"] }, - "worktree": { "enum": ["off", "current", "origin_default"] }, - "plan_first": { "type": "boolean" } + "worktree": { "enum": ["off", "current", "origin_default"] } }, "additionalProperties": false }, diff --git a/crates/core/src/acp/client.rs b/crates/core/src/acp/client.rs index d5577bff..787877a0 100644 --- a/crates/core/src/acp/client.rs +++ b/crates/core/src/acp/client.rs @@ -24,6 +24,8 @@ pub struct AcpClient { // Wrapped in a std Mutex so `AcpClient` is `Sync` (needed to live in desktop host state / the engine's // shared session map). We only touch it to kill the child on drop. child: Option>, + #[cfg(unix)] + process_group: Option, started_at_unix_ms: i64, terminated: AtomicBool, } @@ -33,11 +35,21 @@ impl AcpClient { Self { conn, child: child.map(Mutex::new), + #[cfg(unix)] + process_group: None, started_at_unix_ms: unix_time_millis(), terminated: AtomicBool::new(false), } } + /// Only `spawn` creates an isolated process group; callers supplying arbitrary children + /// through `new` retain direct-child ownership. + #[cfg(unix)] + pub(super) fn with_process_group(mut self, process_group: Option) -> Self { + self.process_group = process_group; + self + } + /// The underlying connection, for advanced/unsupported calls. pub fn connection(&self) -> &Arc { &self.conn @@ -238,12 +250,19 @@ impl AcpClient { /// Terminate the owned provider process without waiting for it to exit. /// /// Plugin unload is synchronous, so it must never park on `Child::wait`. Closing the child is - /// enough to end its stdio connection; the reader task then rejects outstanding requests and - /// lets their turn leases take the normal provider-failure path. + /// insufficient when an adapter wrapper leaves descendants holding stdio. On Unix, terminate + /// the isolated group as well so pending requests close and turn leases can be released. + /// Windows retains direct-child cleanup; process-tree ownership requires a Job Object. pub fn terminate(&self) { if self.terminated.swap(true, Ordering::AcqRel) { return; } + #[cfg(unix)] + if let Some(group) = self.process_group { + if let Err(error) = crate::unix_process_group::kill(group) { + tracing::warn!(%error, "could not terminate ACP process group"); + } + } if let Some(child) = &self.child { if let Ok(mut child) = child.lock() { let _ = child.start_kill(); diff --git a/crates/core/src/acp/handler.rs b/crates/core/src/acp/handler.rs index 7154107b..6d7b0fd6 100644 --- a/crates/core/src/acp/handler.rs +++ b/crates/core/src/acp/handler.rs @@ -12,7 +12,7 @@ use crate::error::RpcError; #[async_trait] pub trait ClientHandler: Send + Sync + 'static { - /// A streamed `session/update` (assistant text, thoughts, tool calls, plans). + /// A streamed `session/update` (assistant text, thoughts, tool calls). async fn session_update(&self, _note: SessionNotification) {} /// The agent needs a permission decision. Default: cancel (deny) — safe by default. diff --git a/crates/core/src/acp/mod.rs b/crates/core/src/acp/mod.rs index 447bdeb6..5b2b2474 100644 --- a/crates/core/src/acp/mod.rs +++ b/crates/core/src/acp/mod.rs @@ -43,7 +43,11 @@ pub async fn spawn( } cmd.stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()); + .stderr(Stdio::piped()) + .kill_on_drop(true); + // Wrapper CLIs (for example npx) may leave descendants holding ACP stdio open. + #[cfg(unix)] + cmd.process_group(0); let mut child = cmd.spawn().map_err(|e| { // "No such file or directory (os error 2)" is inscrutable; name the missing command and @@ -61,7 +65,12 @@ pub async fn spawn( } let conn = Connection::new(stdout, stdin, handler); - Ok(AcpClient::new(conn, Some(child))) + #[cfg(unix)] + let process_group = child.id().and_then(|pid| i32::try_from(pid).ok()); + let client = AcpClient::new(conn, Some(child)); + #[cfg(unix)] + let client = client.with_process_group(process_group); + Ok(client) } /// Forward a provider's stderr to tracing (adapters log diagnostics there). diff --git a/crates/core/src/acp/wire.rs b/crates/core/src/acp/wire.rs index 2384692a..e05ce13e 100644 --- a/crates/core/src/acp/wire.rs +++ b/crates/core/src/acp/wire.rs @@ -36,7 +36,7 @@ pub struct InitializeResponse { pub agent_capabilities: Value, #[serde(rename = "authMethods", default)] pub auth_methods: Value, - /// Provider-owned optional extensions such as native steering and long-running goals. + /// Provider-owned optional extensions such as native steering. #[serde(rename = "_meta", default)] pub meta: Value, } @@ -51,16 +51,9 @@ pub struct AgentInfo { pub version: String, } -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct GoalCapabilityInfo { - pub control_method: String, - pub actions: Vec, -} - #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct InteractionCapabilities { pub steering: bool, - pub goal: Option, } /// The agent capabilities we act on, lifted out of the raw `agentCapabilities` object. Everything @@ -111,21 +104,7 @@ impl InitializeResponse { .pointer("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/steering/supported") .and_then(Value::as_bool) .unwrap_or(false); - let goal = self.meta.get("goal").and_then(|goal| { - let control_method = goal.get("controlMethod")?.as_str()?.to_string(); - let actions = goal - .get("actions")? - .as_array()? - .iter() - .filter_map(Value::as_str) - .map(str::to_string) - .collect::>(); - (!control_method.is_empty() && !actions.is_empty()).then_some(GoalCapabilityInfo { - control_method, - actions, - }) - }); - InteractionCapabilities { steering, goal } + InteractionCapabilities { steering } } } @@ -144,7 +123,7 @@ pub struct NewSessionResponse { pub session_id: String, /// The agent's selectable models, when it reports any. Marked UNSTABLE in the ACP spec and /// absent from most adapters today, so this stays optional and its absence is a normal state — - /// the engine then offers [`crate::models::builtin_models`] for the provider instead. + /// the engine then offers the installed CLI catalogue for the provider instead. #[serde(default)] pub models: Option, /// Session config options (UNSTABLE) — where current adapters report the model selector and @@ -407,9 +386,8 @@ pub enum SessionUpdate { }, ToolCall(ToolCall), ToolCallUpdate(ToolCallUpdate), - Plan { - entries: Vec, - }, + /// Retired provider feature: accept the notification but discard its payload. + Plan {}, /// The agent's config options changed (model switched, effort adjusted, …). Carries the full /// replacement set, same as `session/new` and `session/set_config_option` responses. ConfigOptionUpdate { @@ -422,9 +400,7 @@ pub enum SessionUpdate { #[serde(rename = "availableCommands", default)] available_commands: Vec, }, - /// Provider-owned session metadata. Codex uses `_meta.goal` for live goal snapshots and - /// `_meta.codex.threadStatus.type` to delimit turns started outside `session/prompt` (for - /// example a goal continuation). + /// Provider-owned session metadata, including turn status outside `session/prompt`. SessionInfoUpdate { #[serde(default, rename = "_meta")] meta: Value, @@ -480,15 +456,6 @@ pub struct ToolCallUpdate { pub meta: Option, } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct PlanEntry { - pub content: String, - #[serde(default)] - pub priority: Option, - #[serde(default)] - pub status: Option, -} - // ---- session/request_permission (agent → client request) ----------------------------------- #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/core/src/codex_runtime.rs b/crates/core/src/codex_runtime.rs index f7606149..8931943f 100644 --- a/crates/core/src/codex_runtime.rs +++ b/crates/core/src/codex_runtime.rs @@ -20,6 +20,11 @@ pub struct CodexRuntimeDiscovery { impl CodexRuntimeDiscovery { pub fn detect() -> Self { + if let Some(path) = std::env::var_os("CODEX_PATH").filter(|path| !path.is_empty()) { + return Self { + codex_path: Some(path.into()), + }; + } #[cfg(target_os = "macos")] { let app = Path::new("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/Applications/ChatGPT.app"); @@ -31,7 +36,11 @@ impl CodexRuntimeDiscovery { } } - Self::default() + // Pin ACP and catalogue discovery to the same installed CLI. Otherwise the adapter + // silently uses its own bundled version, which can reject models discovered locally. + Self { + codex_path: crate::provider::which("codex"), + } } } diff --git a/crates/core/src/cost.rs b/crates/core/src/cost.rs index fbaed623..6940dbab 100644 --- a/crates/core/src/cost.rs +++ b/crates/core/src/cost.rs @@ -40,7 +40,7 @@ const fn price( } } -/// Published per-MTok prices for the model ids [`crate::models::builtin_models`] actually offers. +/// Published per-MTok prices for recognized model ids. Discovery does not depend on this table. /// /// Deliberately incomplete: models whose vendors publish no per-token USD price (Cursor's /// `composer-1`, the Kimi K3 and GLM-5 families) are omitted rather than guessed — the designed diff --git a/crates/core/src/engine.rs b/crates/core/src/engine.rs index fcae33d3..c1847e6d 100644 --- a/crates/core/src/engine.rs +++ b/crates/core/src/engine.rs @@ -31,9 +31,9 @@ use crate::canvas::{ CanvasPromptPayload, CanvasProviderImageCapability, }; use crate::error::AcpError; -use crate::event::{ConfigOptionInfo, Event, GoalSnapshot, ModelChoice, Op}; +use crate::event::{ConfigOptionInfo, Event, ModelChoice, Op}; use crate::memory::{prompt_source, MemoryCanvasRef, MemoryCapability, MemoryTurnProvenance}; -use crate::models::{available_models, builtin_models}; +use crate::models::available_models; use crate::permission::{ Action, ExecutionPolicy, PermissionContext, PermissionContextKind, PermissionMode, PermissionPolicy, SandboxPolicy, @@ -41,9 +41,9 @@ use crate::permission::{ use crate::provider::{LaunchSpec, Provider, ProviderId, ProviderToolset}; use crate::session::{ initial_session_title, tool_status_is_in_flight, tool_status_is_terminal, - transcript_context_with_omission, MemoryAccess, Part, PlanEntry, Role, Session, - SessionActivity, SessionId, SessionRunState, SessionTitleOrigin, TranscriptCursor, - TranscriptPage, DEFAULT_TRANSCRIPT_TURNS, + transcript_context_with_omission, MemoryAccess, Part, Role, Session, SessionActivity, + SessionId, SessionRunState, SessionTitleOrigin, TranscriptCursor, TranscriptPage, + DEFAULT_TRANSCRIPT_TURNS, }; use crate::skill::{ canonical_doc_text, compile_with_appshots, compile_with_canvas, @@ -258,7 +258,8 @@ fn push_provider_switch_record( if content.trim().is_empty() { return; } - if kind == "message" + if role == "assistant" + && kind == "message" && records .last() .is_some_and(|record| record.role == role && record.kind == kind) @@ -289,21 +290,6 @@ fn provider_switch_context( (Role::Agent, Part::Text { text }) => { push_provider_switch_record(&mut records, "assistant", "message", text.clone()) } - (Role::Agent, Part::Plan { entries }) => { - let content = entries - .iter() - .map(|entry| match (&entry.status, &entry.priority) { - (Some(status), Some(priority)) => { - format!("- [{status}; {priority}] {}", entry.content) - } - (Some(status), None) => format!("- [{status}] {}", entry.content), - (None, Some(priority)) => format!("- [{priority}] {}", entry.content), - (None, None) => format!("- {}", entry.content), - }) - .collect::>() - .join("\n"); - push_provider_switch_record(&mut records, "assistant", "plan", content); - } (Role::Agent, Part::ToolCall { title, status, .. }) => push_provider_switch_record( &mut records, "tool", @@ -314,35 +300,82 @@ fn provider_switch_context( } } + // Keep the original goal and latest user constraints even when a large assistant response + // fills the recent-history budget. Indices preserve chronology and prevent duplicate anchors. + let first_user = records.iter().position(|record| record.role == "user"); + let last_user = records.iter().rposition(|record| record.role == "user"); + let fits = records.len() <= 128 + && records + .iter() + .map(|record| record.content.chars().count()) + .sum::() + <= MAX_PROVIDER_SWITCH_CONTEXT_CHARS; let mut remaining = MAX_PROVIDER_SWITCH_CONTEXT_CHARS; - let mut selected = Vec::new(); + let mut selected = std::collections::BTreeMap::new(); let mut truncated = false; - for record in records.into_iter().rev() { + for index in [first_user, last_user].into_iter().flatten() { + if selected.contains_key(&index) { + continue; + } + let record = &records[index]; let chars = record.content.chars().count(); - if chars <= remaining { - remaining -= chars; - selected.push(serde_json::json!({ - "role": record.role, - "kind": record.kind, - "content": record.content, - })); + let keep = if fits { chars } else { chars.min(4 * 1024) }; + let content = if chars > keep { + // Preserve both ends of a long user request: goals usually lead, corrections trail. + let head: String = record.content.chars().take(keep / 2).collect(); + let tail: String = record + .content + .chars() + .skip(chars - (keep - keep / 2 - 1)) + .collect(); + format!("{head}…{tail}") + } else { + record.content.clone() + }; + truncated |= chars > keep; + remaining -= keep; + selected.insert( + index, + serde_json::json!({ + "role": record.role, "kind": record.kind, "content": content, + "truncated": chars > keep, + }), + ); + } + for (index, record) in records.iter().enumerate().rev() { + if selected.contains_key(&index) { continue; } - truncated = true; - if remaining > 0 { - let keep = remaining.saturating_sub(1); - let skip = chars.saturating_sub(keep); - let content = format!("…{}", record.content.chars().skip(skip).collect::()); - selected.push(serde_json::json!({ - "role": record.role, - "kind": record.kind, - "content": content, - "truncated": true, - })); - } - break; - } - selected.reverse(); + // Also bound metadata overhead for histories with many tiny alternating records. + if remaining == 0 || selected.len() >= 128 { + truncated = true; + break; + } + let chars = record.content.chars().count(); + let keep = chars.min(remaining); + let content = if chars > keep { + format!( + "…{}", + record + .content + .chars() + .skip(chars - keep + 1) + .collect::() + ) + } else { + record.content.clone() + }; + remaining -= keep; + truncated |= chars > keep; + selected.insert( + index, + serde_json::json!({ + "role": record.role, "kind": record.kind, "content": content, + "truncated": chars > keep, + }), + ); + } + let selected: Vec<_> = selected.into_values().collect(); serde_json::json!({ "kind": "provider_switch", "sourceProvider": from.as_str(), @@ -440,7 +473,7 @@ mod provider_switch_context_tests { assert!(content_chars <= MAX_PROVIDER_SWITCH_CONTEXT_CHARS); assert_eq!(context["olderHistoryOmitted"], true); assert!(serialized.contains("OLD_TAIL")); - assert!(!serialized.contains("OLD_HEAD")); + assert!(serialized.contains("OLD_HEAD")); assert!(serialized.contains("Compile workspace: completed")); assert!(serialized.contains("Latest assistant conclusion")); assert!(serialized.contains("Latest user request")); @@ -449,6 +482,113 @@ mod provider_switch_context_tests { assert!(!serialized.contains("PRIVATE_TOOL_OUTPUT_SECRET")); } + #[test] + fn short_history_keeps_complete_user_messages_and_their_boundaries() { + let prompt = |text: String| { + ( + Role::User, + Part::Prompt { + display: text.clone(), + text, + }, + ) + }; + let first = "目标".repeat(4_000); + let transcript = vec![prompt(first.clone()), prompt("追加约束".into())]; + let context = provider_switch_context(&ProviderId::Pi, &ProviderId::Grok, &transcript); + assert_eq!(context["history"].as_array().unwrap().len(), 2); + assert_eq!(context["history"][0]["content"], first); + assert_eq!(context["history"][1]["content"], "追加约束"); + assert_eq!(context["olderHistoryOmitted"], false); + } + + #[test] + fn long_unicode_and_many_small_records_keep_goals_and_latest_constraints() { + let prompt = |text: String| { + ( + Role::User, + Part::Prompt { + display: text.clone(), + text, + }, + ) + }; + let mut transcript = vec![prompt("最初目标：保留用户数据".into())]; + for i in 0..300 { + transcript.push(prompt(format!("request {i}"))); + transcript.push(( + Role::Agent, + Part::Text { + text: format!("reply {i}"), + }, + )); + } + transcript.push(prompt("最新约束：不要发布".into())); + transcript.push(( + Role::Agent, + Part::Text { + text: "界🌏".repeat(80_000), + }, + )); + let context = provider_switch_context(&ProviderId::Pi, &ProviderId::Grok, &transcript); + let history = context["history"].as_array().unwrap(); + assert!(history.len() <= 128); + assert!( + history + .iter() + .map(|row| row["content"].as_str().unwrap().chars().count()) + .sum::() + <= MAX_PROVIDER_SWITCH_CONTEXT_CHARS + ); + let encoded = context.to_string(); + assert!(encoded.contains("最初目标")); + assert!(encoded.contains("最新约束")); + assert_eq!(context["olderHistoryOmitted"], true); + // Record overhead is bounded independently of the content budget. + let tiny = provider_switch_context(&ProviderId::Pi, &ProviderId::Grok, &transcript[..601]); + assert!(tiny["history"].as_array().unwrap().len() <= 128); + assert_eq!(tiny["olderHistoryOmitted"], true); + } + + #[tokio::test] + async fn dropping_a_switch_releases_its_starting_client_and_session_fence() { + use crate::provider::{LaunchSpec, Provider}; + use crate::skill::SkillLibrary; + let store = Arc::new(Store::open_in_memory().unwrap()); + let session = Session::new(ProviderId::Grok, std::env::temp_dir().to_string_lossy()); + store.upsert_session(&session).unwrap(); + let target = Provider { + id: ProviderId::Pi, + display_name: "Slow target".into(), + needs_node: false, + launch: LaunchSpec::new("python3", ["-c", "import time; time.sleep(30)"]), + }; + let (engine, _rx) = + super::Engine::with_store(vec![target], SkillLibrary::new(vec![]), store.clone()); + let mut switching = Box::pin(engine.switch_provider(&session.id, ProviderId::Pi, None)); + let started = async { + loop { + if !engine.state.starting_clients.lock().unwrap().is_empty() { + break; + } + tokio::task::yield_now().await; + } + }; + tokio::select! { + result = &mut switching => panic!("unexpected switch completion: {result:?}"), + result = tokio::time::timeout(std::time::Duration::from_secs(5), started) => result.unwrap(), + } + assert_eq!(engine.state.starting_clients.lock().unwrap().len(), 1); + drop(switching); + assert!(engine.state.starting_clients.lock().unwrap().is_empty()); + assert!(!engine.session_is_switching_provider(&session.id)); + assert_eq!( + store.get_session(&session.id).unwrap().unwrap().provider, + ProviderId::Grok + ); + engine.shutdown(); + } + #[tokio::test] async fn detached_runtime_callbacks_cannot_emit_or_persist_late_frames() { let store = Arc::new(Store::open_in_memory().unwrap()); @@ -458,7 +598,6 @@ mod provider_switch_context_tests { let (events, mut received) = tokio::sync::mpsc::unbounded_channel(); let handler = SessionHandler::new( session.id.clone(), - ProviderId::Grok, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::default(), @@ -542,7 +681,6 @@ where /// parts, resolves permissions, and advances the provider-neutral liveness clock. pub struct SessionHandler { session_id: SessionId, - provider: ProviderId, events: mpsc::UnboundedSender, policy: Arc>, router: PermissionRouter, @@ -559,7 +697,7 @@ pub struct SessionHandler { /// Runtime-generation fence. A provider replacement flips the old handler inactive before /// terminating its process, so late stdio frames cannot repaint or append to the new runtime. active: Arc, - /// Goal continuation can start a provider turn without a matching `session/prompt` future. + /// Provider activity can start a turn without a matching `session/prompt` future. /// Retain its activity lease until a provider-owned `session_info_update` closes the turn. external_turn: Mutex>, } @@ -567,18 +705,16 @@ pub struct SessionHandler { impl SessionHandler { pub fn new( session_id: SessionId, - provider: ProviderId, events: mpsc::UnboundedSender, policy: Arc>, router: PermissionRouter, store: Option>, ) -> Self { - Self::new_with_activity(session_id, provider, events, policy, router, store, true) + Self::new_with_activity(session_id, events, policy, router, store, true) } fn new_with_activity( session_id: SessionId, - provider: ProviderId, events: mpsc::UnboundedSender, policy: Arc>, router: PermissionRouter, @@ -587,7 +723,6 @@ impl SessionHandler { ) -> Self { Self { session_id, - provider, events, policy, router, @@ -607,13 +742,12 @@ impl SessionHandler { fn inactive( session_id: SessionId, - provider: ProviderId, events: mpsc::UnboundedSender, policy: Arc>, router: PermissionRouter, store: Option>, ) -> Self { - Self::new_with_activity(session_id, provider, events, policy, router, store, false) + Self::new_with_activity(session_id, events, policy, router, store, false) } fn activity_flag(&self) -> Arc { @@ -652,16 +786,6 @@ impl SessionHandler { } fn handle_session_info(&self, meta: Value) { - if let Some(goal) = meta.as_object().and_then(|object| object.get("goal")) { - let normalized = normalize_goal(goal); - if goal.is_null() || normalized.is_some() { - self.emit(Event::GoalChanged { - session: self.session_id.clone(), - goal: normalized, - }); - } - } - let status = meta .pointer("/codex/threadStatus/type") .and_then(Value::as_str); @@ -1026,6 +1150,9 @@ fn config_option_infos(options: &[crate::acp::wire::SessionConfigOption]) -> Vec options .iter() .filter(|o| o.option_type.as_deref().unwrap_or("select") == "select") + .filter(|o| { + o.id != "collaboration_mode" && o.category.as_deref() != Some("collaboration_mode") + }) .map(|o| ConfigOptionInfo { id: o.id.clone(), name: o.name.clone(), @@ -1044,56 +1171,6 @@ fn config_option_infos(options: &[crate::acp::wire::SessionConfigOption]) -> Vec .collect() } -/// Normalize only provider values that are protocol-valid aliases for the same real behavior. -/// glm-acp-agent 1.6 currently exposes six accepted strings for GLM-5.3, while Z.AI documents -/// three distinct service levels; presenting all six would give the slider fake precision. -fn provider_config_option_infos( - provider: &ProviderId, - options: &[crate::acp::wire::SessionConfigOption], -) -> Vec { - let mut infos = config_option_infos(options); - let is_effort = |option: &ConfigOptionInfo| { - option.category.as_deref() == Some("thought_level") - || option.id == "effort" - || option.id == "reasoning_effort" - }; - if *provider == ProviderId::Pi { - // pi-acp 0.0.33 advertises the same six strings for every model and omits Pi's supported - // `max` value. Until the adapter consumes Pi's model-specific thinking-level RPC, hiding - // that known-false menu is more truthful than presenting a selectable fake capability. - infos.retain(|option| !is_effort(option)); - return infos; - } - if *provider != ProviderId::ZCode { - return infos; - } - let is_glm_53 = infos - .iter() - .find(|option| option.category.as_deref() == Some("model") || option.id == "model") - .map(|option| { - option.current.starts_with("glm-5.3") - || option.current.starts_with("glm-5.2") - || option.current.starts_with("glm-5.1") - }) - .unwrap_or(false); - if !is_glm_53 { - return infos; - } - if let Some(effort) = infos.iter_mut().find(|option| is_effort(option)) { - effort.current = match effort.current.as_str() { - "minimal" | "light" => "low", - "medium" => "high", - "xhigh" | "ultra" => "max", - current => current, - } - .to_string(); - effort - .choices - .retain(|choice| matches!(choice.id.as_str(), "low" | "high" | "max")); - } - infos -} - /// Some agents expose a model-specific effort ladder before ACP's config-options surface. Grok's /// current ACP server puts it in `ModelInfo._meta.reasoningEfforts` and switches it through the /// legacy `session/set_mode` method. Turn that provider-owned metadata into the same frontend shape @@ -1267,26 +1344,6 @@ fn canvas_history_projection(canonical: String, compiled: Option<&CompiledPrompt out } -fn normalize_goal(value: &Value) -> Option { - if value.is_null() { - return None; - } - let objective = value.get("objective")?.as_str()?.to_string(); - let status = value.get("status")?.as_str()?.to_string(); - Some(GoalSnapshot { - objective, - status, - created_at: value.get("createdAt").and_then(Value::as_i64).unwrap_or(0), - updated_at: value.get("updatedAt").and_then(Value::as_i64).unwrap_or(0), - token_budget: value.get("tokenBudget").and_then(Value::as_u64), - tokens_used: value.get("tokensUsed").and_then(Value::as_u64).unwrap_or(0), - time_used_seconds: value - .get("timeUsedSeconds") - .and_then(Value::as_u64) - .unwrap_or(0), - }) -} - fn encode_mcp_servers( servers: &[McpServer], caps: AgentCaps, @@ -1834,22 +1891,36 @@ mod usage_update_tests { use std::sync::{Arc, Mutex}; use super::SessionHandler; - use crate::acp::wire::{PlanEntry as AcpPlanEntry, SessionNotification, SessionUpdate}; + use crate::acp::wire::{SessionNotification, SessionUpdate}; use crate::acp::ClientHandler; use crate::activity::ActivityTracker; use crate::engine::PermissionRouter; use crate::event::Event; use crate::permission::PermissionPolicy; - use crate::provider::ProviderId; use crate::session::{SessionActivity, SessionRunState}; use tokio::sync::mpsc; + #[test] + fn planning_selectors_are_not_projected_as_supported_configuration() { + let options: Vec = serde_json::from_value(serde_json::json!([ + {"id":"custom-planner","name":"Plan","category":"collaboration_mode", + "type":"select","currentValue":"plan","options":[{"value":"plan","name":"Plan"}]}, + {"id":"collaboration_mode","name":"Mode","type":"select", + "currentValue":"plan","options":[{"value":"plan","name":"Plan"}]}, + {"id":"model","name":"Model","category":"model","type":"select", + "currentValue":"account-model","options":[{"value":"account-model","name":"Model"}]} + ])).unwrap(); + let projected = super::config_option_infos(&options); + assert_eq!(projected.len(), 1); + assert_eq!(projected[0].id, "model"); + assert_eq!(projected[0].current, "account-model"); + } + #[tokio::test] - async fn plan_update_preserves_task_status() { + async fn retired_plan_updates_are_ignored() { let (events, mut received) = mpsc::unbounded_channel(); let handler = SessionHandler::new( "session-1".into(), - ProviderId::Codex, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::default(), @@ -1859,23 +1930,15 @@ mod usage_update_tests { handler .session_update(SessionNotification { session_id: "provider-session-1".into(), - update: SessionUpdate::Plan { - entries: vec![AcpPlanEntry { - content: "Implement the panel".into(), - priority: Some("high".into()), - status: Some("in_progress".into()), - }], - }, + update: serde_json::from_value(serde_json::json!({ + "sessionUpdate": "plan", + "entries": [{"content": "Old provider plan", "status": "in_progress"}] + })) + .unwrap(), }) .await; - assert!(matches!( - received.recv().await, - Some(Event::Plan { entries, .. }) - if entries[0].content == "Implement the panel" - && entries[0].priority.as_deref() == Some("high") - && entries[0].status.as_deref() == Some("in_progress") - )); + assert!(received.try_recv().is_err()); } #[tokio::test] @@ -1883,7 +1946,6 @@ mod usage_update_tests { let (events, mut received) = mpsc::unbounded_channel(); let handler = SessionHandler::new( "session-1".into(), - ProviderId::Grok, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::default(), @@ -1937,13 +1999,12 @@ mod usage_update_tests { } #[tokio::test] - async fn session_info_drives_goal_and_external_turn_lifecycle() { + async fn session_info_ignores_goal_and_tracks_external_turn_lifecycle() { let (events, mut received) = mpsc::unbounded_channel(); let tracker = ActivityTracker::new(events.clone(), None); tracker.register("session-1", SessionActivity::default()); let handler = SessionHandler::new( "session-1".into(), - ProviderId::Codex, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::with_tracker(tracker.clone()), @@ -1967,11 +2028,6 @@ mod usage_update_tests { }) .await; - assert!(matches!( - received.recv().await, - Some(Event::GoalChanged { goal: Some(goal), .. }) - if goal.objective == "Unify plugins" && goal.status == "active" - )); assert!(matches!( received.recv().await, Some(Event::SessionActivityChanged { activity, .. }) @@ -1997,10 +2053,6 @@ mod usage_update_tests { }) .await; - assert!(matches!( - received.recv().await, - Some(Event::GoalChanged { goal: None, .. }) - )); assert!(matches!( received.recv().await, Some(Event::SessionActivityChanged { activity, .. }) @@ -2026,7 +2078,6 @@ mod native_command_tests { use crate::acp::ClientHandler; use crate::event::Event; use crate::permission::PermissionPolicy; - use crate::provider::ProviderId; use crate::skill::DocBlock; #[test] @@ -2061,7 +2112,6 @@ mod native_command_tests { let (events, mut received) = mpsc::unbounded_channel(); let handler = SessionHandler::new( "session-1".into(), - ProviderId::ClaudeCode, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::default(), @@ -2192,7 +2242,6 @@ mod tool_update_persistence_tests { let (events, mut received) = mpsc::unbounded_channel(); let handler = SessionHandler::new( session.id.clone(), - ProviderId::Codex, events, Arc::new(Mutex::new(PermissionPolicy::default())), PermissionRouter::default(), @@ -2483,31 +2532,13 @@ impl ClientHandler for SessionHandler { normalized.warnings, ) } - SessionUpdate::Plan { entries } => { - let items: Vec = entries - .into_iter() - .map(|entry| PlanEntry { - content: entry.content, - priority: entry.priority, - status: entry.status, - }) - .collect(); - ( - Some(Event::Plan { - session, - entries: items.clone(), - transcript_seq: None, - }), - Some(Part::Plan { entries: items }), - Vec::new(), - ) - } + SessionUpdate::Plan {} => return, // Agent-side config change (e.g. it switched model itself): forward the new set to the // UI. Not a transcript part — configuration isn't conversation. SessionUpdate::ConfigOptionUpdate { config_options } => ( Some(Event::ConfigOptions { session, - options: provider_config_option_infos(&self.provider, &config_options), + options: config_option_infos(&config_options), }), None, Vec::new(), @@ -2525,7 +2556,6 @@ impl ClientHandler for SessionHandler { Some(Event::SessionCapabilities { session, steering: interaction.steering, - goal: interaction.goal, compact_context, }), None, @@ -2607,10 +2637,6 @@ impl ClientHandler for SessionHandler { | Event::ToolCall { transcript_seq: seq, .. - } - | Event::Plan { - transcript_seq: seq, - .. } => *seq = transcript_seq, _ => {} } @@ -2822,15 +2848,14 @@ struct SessionRuntime { cwd: String, policy: Arc>, /// The models this session can be switched to: what the agent reported at `session/new`, or - /// [`builtin_models`] for its provider when it reports none — which is most of them today, - /// since the ACP model API is UNSTABLE and widely unimplemented. + /// the installed CLI catalogue before ACP metadata arrives. Empty means undiscovered. models: Vec, /// Last complete selector set received from the agent. Most providers return a replacement /// set after each change; Grok's legacy mode method returns `{}`, so its metadata-derived /// effort option is updated here after a successful switch. config_options: Vec, - /// Whether [`SessionRuntime::models`] came from the agent rather than our built-in list. A - /// built-in choice is one the agent never advertised, so it's applied on a best-effort + /// Whether [`SessionRuntime::models`] came from ACP rather than standalone CLI discovery. + /// A CLI choice is applied on a best-effort /// `session/set_model` and reported honestly when the agent won't take it. models_reported: bool, initial_reasoning_effort: Option, @@ -2915,10 +2940,19 @@ struct EngineState { struct ProviderSwitchGuard { state: Arc, session: SessionId, + candidate: Option>, } impl Drop for ProviderSwitchGuard { fn drop(&mut self) { + if let Some(client) = &self.candidate { + self.state + .starting_clients + .lock() + .unwrap() + .retain(|candidate| !Arc::ptr_eq(candidate, client)); + client.terminate(); + } self.state .provider_switches .lock() @@ -3193,9 +3227,49 @@ impl Engine { Ok(ProviderSwitchGuard { state: self.state.clone(), session: session.to_string(), + candidate: None, }) } + /// Registered integrations with model choices actually discovered by their live runtimes. + /// An empty list is unknown, never a fabricated provider default. + pub fn provider_catalog(&self) -> Vec<(Provider, Vec)> { + let sessions = self.state.sessions.lock().unwrap(); + self.state + .providers + .iter() + .map(|provider| { + let mut models = Vec::new(); + let mut seen = HashSet::new(); + for runtime in sessions + .values() + .filter(|runtime| runtime.session.provider == provider.id) + { + for choice in &runtime.models { + if seen.insert(choice.id.clone()) { + models.push(choice.clone()); + } + } + for option in runtime.config_options.iter().filter(|option| { + option.category.as_deref() == Some("model") || option.id == "model" + }) { + for choice in &option.choices { + if seen.insert(choice.id.clone()) { + models.push(ModelChoice { + id: choice.id.clone(), + name: choice.name.clone(), + description: choice.description.clone(), + }); + } + } + } + } + models.sort_by(|left, right| left.id.cmp(&right.id)); + (provider.clone(), models) + }) + .collect() + } + pub fn session_is_switching_provider(&self, session: &str) -> bool { self.state .provider_switches @@ -3256,6 +3330,23 @@ impl Engine { self.state.handoff_fences.lock().unwrap().remove(session); } + fn clear_restored_handoff_context(&self, session: &str) -> Result<(), String> { + let pending_switch = self + .state + .sessions + .lock() + .unwrap() + .get(session) + .and_then(|runtime| runtime.handoff_context.as_ref()) + .is_some_and(|context| { + context.get("kind").and_then(Value::as_str) == Some("provider_switch") + }); + if pending_switch { + return Ok(()); + } + self.clear_handoff_context(session) + } + fn clear_handoff_context(&self, session: &str) -> Result<(), String> { let persisted = if let Some(store) = &self.state.store { store @@ -4323,7 +4414,6 @@ impl Engine { self.emit(Event::SessionCapabilities { session: session.to_string(), steering: interaction.steering, - goal: interaction.goal, compact_context, }); if !models.is_empty() { @@ -4352,7 +4442,7 @@ impl Engine { model: Option, ) -> Result { self.assert_session_active(session)?; - let _switch = self.begin_provider_switch(session)?; + let mut switch = self.begin_provider_switch(session)?; if self.session_is_busy(session) { return Err("can't switch providers while a turn is running or awaiting input".into()); } @@ -4396,7 +4486,7 @@ impl Engine { Some(store) => store .transcript(session) .map_err(|error| format!("couldn't read conversation history: {error}"))?, - None => Vec::new(), + None => return Err("switching agents requires saved conversation history".into()), }; let continuation = provider_switch_context(&original.provider, &provider, &transcript); let policy = Arc::new(Mutex::new(PermissionPolicy { @@ -4406,7 +4496,6 @@ impl Engine { })); let handler = Arc::new(SessionHandler::inactive( session.to_string(), - provider.clone(), self.state.events.clone(), policy.clone(), self.state.router.clone(), @@ -4442,11 +4531,10 @@ impl Engine { if !self.track_starting_client(&client) { return Err("engine is shutting down".into()); } + switch.candidate = Some(client.clone()); let init = match client.initialize(client_capabilities()).await { Ok(init) => init, Err(error) => { - self.untrack_starting_client(&client); - client.terminate(); return Err(format!( "couldn't initialize {}: {error}", target.display_name @@ -4460,8 +4548,6 @@ impl Engine { let mut live_sessions = self.state.sessions.lock().unwrap(); if self.state.shutting_down.load(Ordering::Acquire) { drop(live_sessions); - self.untrack_starting_client(&client); - client.terminate(); return Err("engine is shutting down".into()); } let runtime_matches = match (&expected_client, live_sessions.get(session)) { @@ -4474,8 +4560,6 @@ impl Engine { }; if !runtime_matches { drop(live_sessions); - self.untrack_starting_client(&client); - client.terminate(); return Err("the session runtime changed while its provider was switching".into()); } @@ -4490,8 +4574,6 @@ impl Engine { active.store(true, Ordering::Release); } drop(live_sessions); - self.untrack_starting_client(&client); - client.terminate(); return Err("can't switch providers while a turn is running or awaiting input".into()); } @@ -4518,8 +4600,6 @@ impl Engine { active.store(true, Ordering::Release); } drop(live_sessions); - self.untrack_starting_client(&client); - client.terminate(); return Err( "the durable provider changed while this switch was starting".into(), ); @@ -4529,8 +4609,6 @@ impl Engine { active.store(true, Ordering::Release); } drop(live_sessions); - self.untrack_starting_client(&client); - client.terminate(); return Err(format!("couldn't persist provider switch: {error}")); } } @@ -4539,6 +4617,7 @@ impl Engine { let old_runtime = live_sessions.remove(session); callback_active.store(true, Ordering::Release); self.untrack_starting_client(&client); + switch.candidate = None; live_sessions.insert( session.to_string(), SessionRuntime { @@ -4590,14 +4669,9 @@ impl Engine { session: session.to_string(), options: Vec::new(), }); - self.emit(Event::GoalChanged { - session: session.to_string(), - goal: None, - }); self.emit(Event::SessionCapabilities { session: session.to_string(), steering: interaction.steering, - goal: interaction.goal, compact_context: compact_context_supported(&native_commands), }); self.emit(Event::Models { @@ -4608,224 +4682,6 @@ impl Engine { Ok(updated) } - /// Establish the provider-side session when a command (currently goal control) needs an ACP - /// session id before any prompt has done so. - async fn ensure_acp_session(&self, session: &str) -> Result<(), String> { - self.assert_session_active(session)?; - if !self.state.sessions.lock().unwrap().contains_key(session) { - self.revive_session(session).await?; - } - let snapshot = { - let sessions = self.state.sessions.lock().unwrap(); - let runtime = sessions - .get(session) - .ok_or_else(|| "no such session".to_string())?; - ( - runtime.client.clone(), - runtime.acp_session_id.clone(), - runtime.resume_acp_session_id.clone(), - runtime.cwd.clone(), - runtime.caps, - runtime.replaying.clone(), - runtime.interaction.clone(), - runtime.provider_toolset.clone(), - runtime.session.provider.clone(), - runtime.session.model.clone(), - runtime.config_options.clone(), - runtime.initial_reasoning_effort.clone(), - compact_context_supported(&runtime.native_commands), - ) - }; - let ( - client, - existing, - resume, - cwd, - caps, - replaying, - interaction, - toolset, - provider, - pending_model, - existing_options, - pending_effort, - compact_context, - ) = snapshot; - self.emit(Event::SessionCapabilities { - session: session.to_string(), - steering: interaction.steering, - goal: interaction.goal.clone(), - compact_context, - }); - if existing.is_some() { - if !existing_options.is_empty() { - self.emit(Event::ConfigOptions { - session: session.to_string(), - options: existing_options, - }); - } - return Ok(()); - } - - let mut servers = toolset.mcp_servers.clone(); - if let Some(config) = &self.state.desktop_mcp { - attach_host_mcp_servers(&mut servers, [config.scene_server_for_session(session)]); - if provider == ProviderId::Codex - && toolset.browser_access_enabled - && config.browser_enabled - { - attach_host_mcp_servers(&mut servers, [config.browser_server_for_session(session)]); - } - } - let encoded = encode_mcp_servers(&servers, caps)?; - let mut restored_provider_context = false; - let (acp_session_id, models, mut options, mut current) = if let Some(resume_id) = resume - .as_deref() - .filter(|_| caps.resume_session || caps.load_session) - { - match restore_provider_session( - &client, - caps, - resume_id, - &cwd, - encoded.clone(), - &replaying, - ) - .await - { - Ok(response) => { - restored_provider_context = true; - let current = response - .models - .as_ref() - .map(|models| models.current_model_id.clone()) - .unwrap_or_default(); - let options = response - .config_options - .as_deref() - .map(|options| provider_config_option_infos(&provider, options)) - .unwrap_or_default(); - (resume_id.to_string(), response.models, options, current) - } - Err(_) => { - let response = client - .new_session_full(&cwd, encoded.clone()) - .await - .map_err(|error| error.to_string())?; - let current = response - .models - .as_ref() - .map(|models| models.current_model_id.clone()) - .unwrap_or_default(); - let options = response - .config_options - .as_deref() - .map(|options| provider_config_option_infos(&provider, options)) - .unwrap_or_default(); - (response.session_id, response.models, options, current) - } - } - } else { - let response = client - .new_session_full(&cwd, encoded) - .await - .map_err(|error| error.to_string())?; - let current = response - .models - .as_ref() - .map(|models| models.current_model_id.clone()) - .unwrap_or_default(); - let options = response - .config_options - .as_deref() - .map(|options| provider_config_option_infos(&provider, options)) - .unwrap_or_default(); - (response.session_id, response.models, options, current) - }; - - if let Some(model) = pending_model.as_deref().filter(|model| *model != current) { - match client.set_model(&acp_session_id, model).await { - Ok(()) => { - current = model.to_string(); - reflect_flat_model_in_options(&mut options, model); - } - Err(error) => self.emit(Event::Error { - session: Some(session.to_string()), - message: format!("{model} wasn't accepted: {error}"), - terminal: false, - request_id: None, - }), - } - } - if let Some(effort) = pending_effort.as_deref() { - if let Some(option) = options.iter().find(|option| { - option.category.as_deref() == Some("thought_level") - || matches!(option.id.as_str(), "effort" | "reasoning_effort") - }) { - options = client - .set_config_option(&acp_session_id, &option.id, effort) - .await - .map(|options| provider_config_option_infos(&provider, &options)) - .map_err(|error| error.to_string())?; - } - } - let reported = models - .as_ref() - .map(|models| { - models - .available_models - .iter() - .map(|model| ModelChoice { - id: model.model_id.clone(), - name: model.name.clone(), - description: model.description.clone(), - }) - .collect::>() - }) - .unwrap_or_default(); - { - let mut sessions = self.state.sessions.lock().unwrap(); - let runtime = sessions - .get_mut(session) - .ok_or_else(|| "session closed while connecting".to_string())?; - runtime.acp_session_id = Some(acp_session_id.clone()); - runtime.resume_acp_session_id = None; - runtime.mcp_servers = servers; - runtime.config_options = options.clone(); - runtime.initial_reasoning_effort = None; - runtime.session.acp_session_id = Some(acp_session_id); - if !current.is_empty() { - runtime.session.model = Some(current.clone()); - } - if !reported.is_empty() { - runtime.models = reported.clone(); - runtime.models_reported = true; - } - if let Some(store) = &self.state.store { - store - .upsert_session(&runtime.session) - .map_err(|error| error.to_string())?; - } - } - if !reported.is_empty() { - self.emit(Event::Models { - session: session.to_string(), - available: reported, - current, - }); - } - if !options.is_empty() { - self.emit(Event::ConfigOptions { - session: session.to_string(), - options, - }); - } - if restored_provider_context { - self.clear_handoff_context(session)?; - } - Ok(()) - } - pub async fn steer_prompt( &self, session: &str, @@ -4919,65 +4775,6 @@ impl Engine { Ok(outcome.to_string()) } - pub async fn control_goal( - &self, - session: &str, - action: &str, - objective: Option, - ) -> Result<(), String> { - self.ensure_acp_session(session).await?; - let (client, acp_session_id, goal) = { - let sessions = self.state.sessions.lock().unwrap(); - let runtime = sessions - .get(session) - .ok_or_else(|| "no such session".to_string())?; - ( - runtime.client.clone(), - runtime - .acp_session_id - .clone() - .ok_or_else(|| "ACP session is unavailable".to_string())?, - runtime.interaction.goal.clone(), - ) - }; - let goal = - goal.ok_or_else(|| format!("the provider did not advertise goal action {action}"))?; - if !goal.actions.iter().any(|candidate| candidate == action) { - return Err(format!( - "the provider did not advertise goal action {action}" - )); - } - let objective = objective - .map(|value| value.trim().to_string()) - .filter(|value| !value.is_empty()); - if action == "set" && objective.is_none() { - return Err("goal objective is required".into()); - } - let mut params = Map::from_iter([ - ("sessionId".into(), Value::String(acp_session_id)), - ("action".into(), Value::String(action.to_string())), - ]); - if let Some(objective) = objective { - params.insert("objective".into(), Value::String(objective)); - } - let response: Value = client - .connection() - .request(&goal.control_method, Value::Object(params)) - .await - .map_err(|error| error.to_string())?; - if let Some(value) = response.get("goal") { - let goal = normalize_goal(value); - if !value.is_null() && goal.is_none() { - return Err("the provider returned an invalid goal snapshot".into()); - } - self.emit(Event::GoalChanged { - session: session.to_string(), - goal, - }); - } - Ok(()) - } - fn overlay_activities(&self, mut sessions: Vec) -> Vec { for session in &mut sessions { if let Some(activity) = self.state.activity.activity(&session.id) { @@ -5044,7 +4841,6 @@ impl Engine { })); let handler = Arc::new(SessionHandler::new( id.to_string(), - sess.provider.clone(), self.state.events.clone(), policy.clone(), self.state.router.clone(), @@ -5145,7 +4941,6 @@ impl Engine { self.emit(Event::SessionCapabilities { session: id.to_string(), steering: interaction.steering, - goal: interaction.goal, compact_context: compact_context_supported(&native_commands), }); if !models.is_empty() { @@ -5519,7 +5314,6 @@ impl Engine { })); let handler = Arc::new(SessionHandler::new( sess.id.clone(), - sess.provider.clone(), self.state.events.clone(), policy.clone(), self.state.router.clone(), @@ -5780,7 +5574,6 @@ impl Engine { self.emit(Event::SessionCapabilities { session: session_id.clone(), steering: interaction.steering, - goal: interaction.goal, compact_context: compact_context_supported(&native_commands), }); for (hook, error) in hook_errors { @@ -6378,9 +6171,7 @@ impl Engine { let mut restored_options = resp .config_options .as_deref() - .map(|options| { - provider_config_option_infos(¤t_provider, options) - }) + .map(|options| config_option_infos(options)) .unwrap_or_default(); if !restored_options.iter().any(|option| { option.category.as_deref() == Some("thought_level") @@ -6431,7 +6222,7 @@ impl Engine { options, }); } - if let Err(error) = self.clear_handoff_context(&session) { + if let Err(error) = self.clear_restored_handoff_context(&session) { tracing::warn!( "clear provider-restored handoff context failed: {error}" ); @@ -6494,9 +6285,7 @@ impl Engine { let mut options = resp .config_options .as_deref() - .map(|options| { - provider_config_option_infos(¤t_provider, options) - }) + .map(|options| config_option_infos(options)) .unwrap_or_default(); if !options .iter() @@ -6601,14 +6390,10 @@ impl Engine { next }) } else { - client.set_config_option(&id, &option.id, &want).await.map( - |options| { - provider_config_option_infos( - ¤t_provider, - &options, - ) - }, - ) + client + .set_config_option(&id, &option.id, &want) + .await + .map(|options| config_option_infos(&options)) }; match changed { Ok(updated) => options = updated, @@ -6804,7 +6589,7 @@ impl Engine { } } } - if clear_handoff_after_prompt { + if clear_handoff_after_prompt && stop != StopReason::Cancelled { if let Err(error) = turn_engine.clear_handoff_context(&sess_for_task) { @@ -7041,7 +6826,7 @@ impl Engine { .cloned() { Some(provider) => available_models(&provider).await, - None => builtin_models(&stored_session.provider), + None => Vec::new(), }; (stored_session, models, false) } @@ -7146,6 +6931,20 @@ impl Engine { return Ok(()); } + // Only exposed, provider-reported choices can be changed. Retired controls + // cannot be reactivated by a stale frontend or direct command invocation. + if !previous_options.iter().any(|option| { + option.id == config_id && option.choices.iter().any(|choice| choice.id == value) + }) { + self.emit(Event::Error { + session: Some(session), + message: format!("unsupported session config choice: {config_id}"), + terminal: false, + request_id: None, + }); + return Ok(()); + } + // Grok's current ACP extension advertises effort in ModelInfo metadata and uses // the legacy mode method to change it. Keep the generic frontend contract while // sending the method this provider actually implements. @@ -7178,7 +6977,7 @@ impl Engine { match client.set_config_option(&acp_sid, &config_id, &value).await { Ok(options) => { - let options = provider_config_option_infos(&provider, &options); + let options = config_option_infos(&options); { let mut map = self.state.sessions.lock().unwrap(); if let Some(rt) = map.get_mut(&session) { @@ -7703,12 +7502,9 @@ mod cwd_tests { #[cfg(test)] mod model_option_tests { - use super::{ - provider_config_option_infos, reasoning_option_from_models, reflect_flat_model_in_options, - }; + use super::{config_option_infos, reasoning_option_from_models, reflect_flat_model_in_options}; use crate::acp::wire::{ModelInfo, SessionModelState}; use crate::event::{ConfigOptionInfo, ModelChoice}; - use crate::provider::ProviderId; fn choice(id: &str) -> ModelChoice { ModelChoice { @@ -7776,7 +7572,7 @@ mod model_option_tests { } #[test] - fn glm_53_aliases_collapse_to_three_real_service_levels() { + fn model_effort_choices_preserve_the_adapter_catalogue() { let wire = serde_json::from_value::>( serde_json::json!([ {"id":"model","name":"Model","type":"select","category":"model", @@ -7791,24 +7587,24 @@ mod model_option_tests { ) .unwrap(); - let options = provider_config_option_infos(&ProviderId::ZCode, &wire); + let options = config_option_infos(&wire); let effort = options .iter() .find(|option| option.id == "thought_level") .unwrap(); - assert_eq!(effort.current, "max"); + assert_eq!(effort.current, "xhigh"); assert_eq!( effort .choices .iter() .map(|choice| choice.id.as_str()) .collect::>(), - ["low", "high", "max"] + ["minimal", "low", "medium", "high", "xhigh", "max"] ); } #[test] - fn pi_fixed_adapter_ladder_is_not_presented_as_model_specific_truth() { + fn reported_effort_is_not_hidden_by_provider_identity() { let wire: Vec = serde_json::from_value(serde_json::json!([ { @@ -7837,9 +7633,11 @@ mod model_option_tests { ])) .unwrap(); - let options = provider_config_option_infos(&ProviderId::Pi, &wire); - assert_eq!(options.len(), 1); + let options = config_option_infos(&wire); + assert_eq!(options.len(), 2); assert_eq!(options[0].id, "model"); + assert_eq!(options[1].current, "high"); + assert_eq!(options[1].choices.len(), 6); } } diff --git a/crates/core/src/event.rs b/crates/core/src/event.rs index 8af297be..c53d8026 100644 --- a/crates/core/src/event.rs +++ b/crates/core/src/event.rs @@ -9,7 +9,7 @@ use crate::memory::MemoryReceipt; use crate::permission::ExecutionPolicy; use crate::provider::ProviderId; -use crate::session::{PlanEntry, SessionActivity, SessionId}; +use crate::session::{SessionActivity, SessionId}; use crate::skill::DocBlock; use crate::task::TaskId; use crate::worktree::ResolvedWorktreeBaseline; @@ -191,12 +191,6 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] transcript_seq: Option, }, - Plan { - session: SessionId, - entries: Vec, - #[serde(default, skip_serializing_if = "Option::is_none")] - transcript_seq: Option, - }, /// A permission decision is needed from the user. `options` are `(option_id, label)` pairs. PermissionRequest { session: SessionId, @@ -230,7 +224,7 @@ pub enum Event { cost_usd: Option, }, /// The models this session can run on: the agent's own list (reported at `session/new` and - /// echoed after a switch), or [`crate::models::builtin_models`] for its provider — emitted as + /// echoed after a switch), or the installed CLI catalogue for its provider — emitted as /// soon as the session exists — when the agent doesn't implement the (UNSTABLE) ACP model API. /// `current` is empty when nothing has been chosen yet. Models { @@ -248,15 +242,10 @@ pub enum Event { SessionCapabilities { session: SessionId, steering: bool, - goal: Option, /// True only after this live ACP session advertises the native `/compact` command. #[serde(default)] compact_context: bool, }, - GoalChanged { - session: SessionId, - goal: Option, - }, PromptQueued { session: SessionId, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -399,17 +388,6 @@ pub struct ConfigOptionInfo { pub choices: Vec, } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct GoalSnapshot { - pub objective: String, - pub status: String, - pub created_at: i64, - pub updated_at: i64, - pub token_budget: Option, - pub tokens_used: u64, - pub time_used_seconds: u64, -} - #[cfg(test)] mod tests { use super::{Event, Op}; diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 57a3ffb3..adef3385 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -8,13 +8,16 @@ //! - [`kernel`] — Cordis-style reactive plugin runtime: contexts, services, injections, commands. //! - [`acp`] — Agent Client Protocol client (JSON-RPC over stdio) used to drive provider CLIs. //! - [`provider`] — registry of provider launch specs (Claude Code / Codex / Grok). -//! - [`models`] — built-in model lists for providers that don't report their own over ACP. +//! - [`models`] — model catalogues discovered from installed provider runtimes. //! - [`session`] — session / message / part model. //! - [`skill`] — skill library + the document → prompt compiler (the product differentiator). //! - [`permission`] — ask/allow/deny engine and permission modes (incl. YOLO). //! - [`event`] — the Op/Event types exchanged with frontends. //! - [`error`] — shared error types. +#[cfg(unix)] +mod unix_process_group; + pub mod kernel; pub mod plugins; pub mod acp; @@ -119,7 +122,6 @@ pub use memory::{ MemoryCanvasRef, MemoryContext, MemoryReceipt, MemoryReceiptItem, MemoryRecord, MemorySettings, MemorySourceRef, MemoryStats, MemoryTurnAudit, MemoryTurnProvenance, }; -pub use models::builtin_models; pub use orchestrator::{ apply_orchestration_patch, ExecutionPreparation, ExecutionRequest, ExecutionStep, ExecutorAssignment, ExecutorOutcome, ExecutorPort, GraphOperation, InMemoryExecutor, diff --git a/crates/core/src/memory.rs b/crates/core/src/memory.rs index 44666981..933d2c93 100644 --- a/crates/core/src/memory.rs +++ b/crates/core/src/memory.rs @@ -1996,15 +1996,6 @@ fn agent_text_after(conn: &Connection, session_id: &str, seq: i64) -> Result(&row?)? { Part::Text { text } => answer.push_str(&text), - Part::Plan { entries } if answer.is_empty() => { - answer.push_str( - &entries - .iter() - .map(|entry| entry.content.as_str()) - .collect::>() - .join("; "), - ); - } _ => {} } } diff --git a/crates/core/src/models.rs b/crates/core/src/models.rs index 79119337..82581177 100644 --- a/crates/core/src/models.rs +++ b/crates/core/src/models.rs @@ -1,17 +1,7 @@ -//! Built-in model lists, for providers that don't report their own. -//! -//! ACP's model API (`session/new` → `models`, `session/set_model`) is marked UNSTABLE and most -//! adapters skip the reporting half entirely. That left the picker with nothing to show and the -//! user with nothing to do but go edit the CLI's config file — so we keep a short list of each -//! CLI's own model ids here and offer that instead. -//! -//! These are a fallback, never an override: whatever the agent reports at `session/new` wins, and -//! is the only list shown when it exists. The ids are the ones each CLI accepts for its own -//! `--model` (so `session/set_model` has a chance of taking them); the names are what we render. -//! A provider we have no list for still gets the "set it in the CLI's config" note. +//! Model choices discovered from the installed provider CLI or ACP session metadata. +//! Discovery failures leave the catalogue empty; the provider still owns its default. use std::process::Stdio; -use std::sync::OnceLock; use std::time::Duration; use serde::Deserialize; @@ -21,12 +11,6 @@ use tokio::process::Command; use crate::event::ModelChoice; use crate::provider::{which, Provider, ProviderId}; -static CODEX_MODELS: OnceLock> = OnceLock::new(); -static GROK_MODELS: OnceLock> = OnceLock::new(); -static CURSOR_MODELS: OnceLock> = OnceLock::new(); -static OPENCODE_MODELS: OnceLock> = OnceLock::new(); -static OPENCODE2_MODELS: OnceLock> = OnceLock::new(); - fn choice(id: &str, name: &str, description: Option<&str>) -> ModelChoice { ModelChoice { id: id.to_string(), @@ -35,17 +19,6 @@ fn choice(id: &str, name: &str, description: Option<&str>) -> ModelChoice { } } -fn codex_family(id: &str, name: &str, description: &str, efforts: &[&str]) -> Vec { - efforts - .iter() - .map(|effort| ModelChoice { - id: format!("{id}[{effort}]"), - name: format!("{name} ({})", effort_label(effort)), - description: Some(description.to_string()), - }) - .collect() -} - fn effort_label(effort: &str) -> String { let mut chars = effort.chars(); match chars.next() { @@ -54,188 +27,33 @@ fn effort_label(effort: &str) -> String { } } -/// The models we offer for `provider` when it reports none of its own. Empty for providers we -/// don't ship a list for — including every [`ProviderId::Custom`], whose models we can't know. -pub fn builtin_models(provider: &ProviderId) -> Vec { - match provider { - // Claude Code owns these aliases and resolves them to the current model in each tier. Keep - // the fallback alias-based; the adapter reports the account's exact catalogue and each - // model's exact effort ladder once the ACP session starts. - ProviderId::ClaudeCode => vec![ - choice( - "default", - "Default", - Some("Claude Code resolves the account default"), - ), - choice( - "best", - "Best available", - Some("Claude Code chooses the strongest available model"), - ), - choice("fable", "Claude Fable", Some("Latest Fable alias")), - choice("opus", "Claude Opus", Some("Latest Opus alias")), - choice( - "opus[1m]", - "Claude Opus 1M", - Some("Latest Opus alias, 1M context"), - ), - choice( - "opusplan", - "Claude Opus Plan", - Some("Opus for planning, Sonnet for execution"), - ), - choice("sonnet", "Claude Sonnet", Some("Latest Sonnet alias")), - choice( - "sonnet[1m]", - "Claude Sonnet 1M", - Some("Latest Sonnet alias, 1M context"), - ), - choice("haiku", "Claude Haiku", Some("Fastest")), - ], - // Codex is queried live by [`available_models`]. Keep the same current catalogue here so - // a temporarily unavailable app-server still leaves the pre-session picker useful. - ProviderId::Codex => [ - codex_family( - "gpt-5.6-sol", - "GPT-5.6-Sol", - "Latest frontier agentic coding model.", - &["low", "medium", "high", "xhigh", "max", "ultra"], - ), - codex_family( - "gpt-5.6-terra", - "GPT-5.6-Terra", - "Balanced agentic coding model for everyday work.", - &["low", "medium", "high", "xhigh", "max", "ultra"], - ), - codex_family( - "gpt-5.6-luna", - "GPT-5.6-Luna", - "Fast and affordable agentic coding model.", - &["low", "medium", "high", "xhigh", "max"], - ), - codex_family( - "gpt-5.5", - "GPT-5.5", - "Frontier model for complex coding, research, and real-world work.", - &["low", "medium", "high", "xhigh"], - ), - codex_family( - "gpt-5.4", - "GPT-5.4", - "Strong model for everyday coding.", - &["low", "medium", "high", "xhigh"], - ), - codex_family( - "gpt-5.4-mini", - "GPT-5.4-Mini", - "Small, fast, and cost-efficient model for simpler coding tasks.", - &["low", "medium", "high", "xhigh"], - ), - codex_family( - "gpt-5.3-codex-spark", - "GPT-5.3-Codex-Spark", - "Ultra-fast coding model.", - &["low", "medium", "high", "xhigh"], - ), - ] - .into_iter() - .flatten() - .collect(), - // These CLIs are queried live by `available_models`; the entries below are conservative - // fallbacks for a transient catalogue failure, not claims about the user's account. - ProviderId::Grok => vec![choice( - "grok-4.6", - "Grok 4.6", - Some("Current Grok CLI default"), - )], - ProviderId::Cursor => vec![choice( - "auto", - "Auto", - Some("Cursor selects a model available to this account"), - )], - // OpenCode and Pi catalogues are entirely account/configuration-owned. Inventing a global - // fallback here is worse than showing the honest “start a session / configure the CLI” - // state; these ACP endpoints report the real list once a session exists. - ProviderId::OpenCode | ProviderId::OpenCode2 | ProviderId::Pi => Vec::new(), - ProviderId::Kimi => vec![ - choice("kimi-code/k3", "Kimi K3", Some("Managed Kimi Code alias")), - choice( - "kimi-code/kimi-for-coding", - "Kimi for Coding", - Some("Managed Kimi Code alias"), - ), - choice( - "kimi-code/kimi-for-coding-highspeed", - "Kimi for Coding Highspeed", - Some("Managed Kimi Code alias"), - ), - ], - // Kept in lock-step with the glm-acp-agent package we launch. The agent replaces this with - // its own model/config-option response after session/new. - ProviderId::ZCode => vec![ - choice("glm-5.3", "GLM-5.3", Some("Default, 1M context")), - choice("glm-5-turbo", "GLM-5 Turbo", Some("Faster, 128K context")), - choice("glm-4.7", "GLM-4.7", None), - ], - // Amp routes internally across frontier models; these are its quality modes exposed as - // ACP model ids by the amp-acp adapter. - ProviderId::Amp => vec![ - choice("medium", "Medium", Some("Default — balanced quality, speed, and cost")), - choice("low", "Low", Some("Fast, low-cost mode for small tasks")), - choice("high", "High", Some("Deeper reasoning, more time and cost")), - choice("ultra", "Ultra", Some("Maximum capability, open-ended tasks")), - ], - // Droid's ACP mode reports its own model list from the user's Factory account once a - // session starts. An empty fallback is better than inventing ids that may not exist. - ProviderId::Droid => Vec::new(), - ProviderId::Custom(_) => Vec::new(), - } -} - -/// Resolve the model list shown before an ACP session exists. Prefer each installed CLI's live, -/// account-specific catalogue wherever it exposes one; static entries are fallback aliases only. +/// Query afresh so account/configuration changes and transient failures can recover. +/// Providers without a standalone catalogue report their choices through ACP session metadata. pub async fn available_models(provider: &Provider) -> Vec { let queried = match provider.id { ProviderId::Codex => { - if let Some(models) = CODEX_MODELS.get() { - return models.clone(); - } let executable = provider .launch .env .iter() .find_map(|(key, value)| (key == "CODEX_PATH").then_some(value.as_str())) .map(std::path::PathBuf::from) - .filter(|path| path.is_file()) .or_else(|| which("codex")); match executable { - Some(executable) => query_codex_models(executable).await, + Some(executable) => query_codex_models(executable, &provider.launch.env).await, None => Err(()), } - .map(|models| (models, &CODEX_MODELS)) } - ProviderId::Grok => query_cli_catalog(provider, &["models"], parse_grok_models) - .await - .map(|models| (models, &GROK_MODELS)), - ProviderId::Cursor => query_cli_catalog(provider, &["--list-models"], parse_cursor_models) - .await - .map(|models| (models, &CURSOR_MODELS)), - ProviderId::OpenCode => query_cli_catalog(provider, &["models"], parse_opencode_models) - .await - .map(|models| (models, &OPENCODE_MODELS)), - ProviderId::OpenCode2 => query_cli_catalog(provider, &["models"], parse_opencode_models) - .await - .map(|models| (models, &OPENCODE2_MODELS)), - _ => return builtin_models(&provider.id), - }; - - match queried { - Ok((models, cache)) if !models.is_empty() => { - let _ = cache.set(models.clone()); - models + ProviderId::Grok => query_cli_catalog(provider, &["models"], parse_grok_models).await, + ProviderId::Cursor => { + query_cli_catalog(provider, &["--list-models"], parse_cursor_models).await } - _ => builtin_models(&provider.id), - } + ProviderId::OpenCode | ProviderId::OpenCode2 => { + query_cli_catalog(provider, &["models"], parse_opencode_models).await + } + _ => return Vec::new(), + }; + queried.unwrap_or_default() } async fn query_cli_catalog( @@ -243,20 +61,11 @@ async fn query_cli_catalog( args: &[&str], parse: fn(&str) -> Vec, ) -> Result, ()> { - let cache = match provider.id { - ProviderId::Grok => GROK_MODELS.get(), - ProviderId::Cursor => CURSOR_MODELS.get(), - ProviderId::OpenCode => OPENCODE_MODELS.get(), - ProviderId::OpenCode2 => OPENCODE2_MODELS.get(), - _ => None, - }; - if let Some(models) = cache { - return Ok(models.clone()); - } let executable = which(&provider.launch.command).ok_or(())?; let mut command = Command::new(executable); command .args(args) + .envs(provider.launch.env.iter().cloned()) .stdin(Stdio::null()) .stderr(Stdio::null()) .kill_on_drop(true); @@ -368,9 +177,13 @@ fn parse_codex_models(result: &serde_json::Value) -> Result, () Ok(choices) } -async fn query_codex_models(executable: std::path::PathBuf) -> Result, ()> { +async fn query_codex_models( + executable: std::path::PathBuf, + env: &[(String, String)], +) -> Result, ()> { let mut child = Command::new(executable) .args(["app-server", "--stdio"]) + .envs(env.iter().cloned()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -394,6 +207,8 @@ async fn query_codex_models(executable: std::path::PathBuf) -> Result Result return Err(()), - Some(2) => return parse_codex_models(message.get("result").ok_or(())?), + Some(2) => { + let result = message.get("result").ok_or(())?; + choices.extend(parse_codex_models(result)?); + let Some(cursor) = result + .get("nextCursor") + .and_then(serde_json::Value::as_str) + .filter(|cursor| !cursor.is_empty()) + else { + return Ok(choices); + }; + if !cursors.insert(cursor.to_string()) { + return Err(()); + } + write_codex_rpc(&mut stdin, serde_json::json!({ + "method": "model/list", "id": 2, "params": { "limit": 100, "cursor": cursor } + })).await?; + } _ => {} } } @@ -421,7 +252,7 @@ async fn query_codex_models(executable: std::path::PathBuf) -> Result>(), + ["page-one", "page-two"] + ); + assert!( + query_codex_models(executable, &[("CYCLE".into(), "1".into())]) + .await + .is_err() + ); } #[test] diff --git a/crates/core/src/plugins/app/plugins/engine.rs b/crates/core/src/plugins/app/plugins/engine.rs index dad70af3..56b52606 100644 --- a/crates/core/src/plugins/app/plugins/engine.rs +++ b/crates/core/src/plugins/app/plugins/engine.rs @@ -730,26 +730,6 @@ fn register_commands( } })?; - #[derive(Deserialize)] - struct GoalArgs { - session: String, - action: String, - #[serde(default)] - objective: Option, - } - let goals = engine.clone(); - ctx.command("engine.goal", move |args| { - let engine = goals.clone(); - async move { - let args: GoalArgs = take_args(args)?; - engine - .control_goal(&args.session, &args.action, args.objective) - .await - .map_err(PluginError::new)?; - Ok(Value::Bool(true)) - } - })?; - let draining_engine = engine.clone(); let draining_bus = bus.clone(); let mut queue_events = bus.subscribe(); diff --git a/crates/core/src/plugins/app/plugins/scene_commands.rs b/crates/core/src/plugins/app/plugins/scene_commands.rs index 8e0f5352..40244e96 100644 --- a/crates/core/src/plugins/app/plugins/scene_commands.rs +++ b/crates/core/src/plugins/app/plugins/scene_commands.rs @@ -368,7 +368,6 @@ async fn apply_scene_to( applied: Vec::new(), pending: Vec::new(), escalation: Some(EscalationOut::from_core(escalation)), - plan_first: None, suppress_unpinned: false, pinned_skills: Vec::new(), }); @@ -395,9 +394,6 @@ async fn apply_scene_to( .map_err(PluginError::new)?; applied.push("memory_preset"); } - if plan.plan_first.is_some() { - applied.push("plan_first"); - } inputs .store .set_session_scene(session, Some(&scene_ref), false) @@ -409,7 +405,6 @@ async fn apply_scene_to( applied, pending: plan.pending.into_iter().map(pending_field_str).collect(), escalation: None, - plan_first: plan.plan_first, suppress_unpinned: skills.suppress_unpinned, pinned_skills: skills.pinned, }) @@ -494,7 +489,6 @@ struct SceneApplyOutcome { applied: Vec<&'static str>, pending: Vec<&'static str>, escalation: Option, - plan_first: Option, suppress_unpinned: bool, pinned_skills: Vec, } diff --git a/crates/core/src/plugins/app/protocol/mod.rs b/crates/core/src/plugins/app/protocol/mod.rs index d7b920fc..50abe0ee 100644 --- a/crates/core/src/plugins/app/protocol/mod.rs +++ b/crates/core/src/plugins/app/protocol/mod.rs @@ -35,6 +35,9 @@ //! │ (unload: process killed, registrations gone) //! ``` +#[cfg(unix)] +use crate::unix_process_group; + mod peer; mod wire; @@ -275,38 +278,6 @@ fn terminate_child_process(mut child: tokio::process::Child, label: &str) { } } -#[cfg(unix)] -mod unix_process_group { - use std::io; - - const SIGKILL: i32 = 9; - const ESRCH: i32 = 3; - - unsafe extern "C" { - fn killpg(process_group: i32, signal: i32) -> i32; - } - - pub(super) fn kill(process_group: i32) -> io::Result<()> { - signal(process_group, SIGKILL).map(|_| ()) - } - - pub(super) fn is_missing(error: &io::Error) -> bool { - error.raw_os_error() == Some(ESRCH) - } - - fn signal(process_group: i32, signal: i32) -> io::Result { - if unsafe { killpg(process_group, signal) } == 0 { - return Ok(true); - } - let error = io::Error::last_os_error(); - if is_missing(&error) { - Ok(false) - } else { - Err(error) - } - } -} - /// A plugin that lives in another process. /// /// Everything the kernel knows about its command surface comes from the Manifest. The handshake diff --git a/crates/core/src/plugins/app/service.rs b/crates/core/src/plugins/app/service.rs index c99b1ac0..0d344580 100644 --- a/crates/core/src/plugins/app/service.rs +++ b/crates/core/src/plugins/app/service.rs @@ -683,7 +683,7 @@ mod tests { "skills", PluginPolicy { components: BTreeMap::from([( - "skill:plan-first".into(), + "skill:test-writer".into(), PluginOverride::Disabled, )]), ..Default::default() @@ -708,8 +708,8 @@ mod tests { Arc::new(Paths::new(data.path())), Some(Arc::new(Mutex::new(store))), ); - assert!(service.list().iter().any(|skill| skill.id == "plan-first")); - assert!(service.library().get("plan-first").is_none()); + assert!(service.list().iter().any(|skill| skill.id == "test-writer")); + assert!(service.library().get("test-writer").is_none()); assert!(service.library().get("reviewer").is_some()); service.reload(Some(&project)); diff --git a/crates/core/src/scene.rs b/crates/core/src/scene.rs index 8c58f9d9..813939b1 100644 --- a/crates/core/src/scene.rs +++ b/crates/core/src/scene.rs @@ -106,8 +106,9 @@ pub struct SceneExecution { pub memory_preset: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub worktree: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub plan_first: Option, + /// Decode old scene files without enabling or re-exporting the retired feature. + #[serde(default, rename = "plan_first", skip_serializing)] + pub _legacy_plan_first: Option, } /// Declaration order IS the loosening order; `Ord` on it drives the escalation rule. @@ -1069,7 +1070,6 @@ pub struct SceneApplyPlan { pub scene_ref: String, pub execution: Option, pub memory: Option<(MemoryAccess, MemoryAccess)>, - pub plan_first: Option, pub pending: Vec, /// Set ⇒ NOTHING was applied; the caller re-calls with `confirm_escalation = true`. pub escalation: Option, @@ -1092,7 +1092,6 @@ pub fn plan_apply( scene_ref: scene_ref.to_string(), execution: None, memory: None, - plan_first: None, pending: Vec::new(), escalation: Some(escalation), new_session: None, @@ -1100,7 +1099,6 @@ pub fn plan_apply( } }; let memory = execution.memory_preset.map(memory_preset_policy); - let plan_first = execution.plan_first; match strength { ApplyStrength::Soft => { let mut pending = Vec::new(); @@ -1120,7 +1118,7 @@ pub fn plan_apply( scene_ref: scene_ref.to_string(), execution: applied, memory, - plan_first, + pending, escalation: None, new_session: None, @@ -1139,7 +1137,7 @@ pub fn plan_apply( scene_ref: scene_ref.to_string(), execution: applied, memory, - plan_first, + pending: Vec::new(), escalation: None, new_session: Some(SceneSessionParams { @@ -1396,6 +1394,17 @@ mod tests { use super::*; use crate::skill::SlotKind; + #[test] + fn legacy_plan_first_is_readable_but_not_reexported() { + let execution: SceneExecution = serde_json::from_value(serde_json::json!({ + "plan_first": true, "model": "account-model" + })) + .unwrap(); + let value = serde_json::to_value(execution).unwrap(); + assert!(value.get("plan_first").is_none()); + assert_eq!(value["model"], "account-model"); + } + fn scene_json(name: &str, extra: &str) -> String { format!(r#"{{"$schema":"{SCENE_SCHEMA_ID}","name":"{name}","title":"T"{extra}}}"#) } @@ -1579,13 +1588,13 @@ mod tests { fn plan_apply_soft_vs_full() { let mut scene = minimal_scene("x"); scene.execution = Some(SceneExecution { + _legacy_plan_first: None, providers: vec!["claude_code".into()], model: Some("m".into()), reasoning_effort: None, session_mode: Some(SceneSessionMode::ReadOnly), memory_preset: Some(SceneMemoryPreset::Private), worktree: Some(SceneWorktree::Current), - plan_first: Some(true), }); let current = ExecutionPolicy::default(); // ask / workspace_write @@ -1596,7 +1605,6 @@ mod tests { Some(session_mode_policy(SceneSessionMode::ReadOnly)) ); assert_eq!(soft.memory, Some((MemoryAccess::Deny, MemoryAccess::Deny))); - assert_eq!(soft.plan_first, Some(true)); assert_eq!( soft.pending, vec![ @@ -1621,7 +1629,6 @@ mod tests { scene.execution = Some(SceneExecution { session_mode: Some(SceneSessionMode::FullAccess), memory_preset: Some(SceneMemoryPreset::Private), - plan_first: Some(true), ..Default::default() }); let current = ExecutionPolicy::default(); @@ -1631,7 +1638,6 @@ mod tests { assert_eq!(escalation.to, SceneSessionMode::FullAccess); assert!(plan.execution.is_none()); assert!(plan.memory.is_none()); - assert!(plan.plan_first.is_none()); assert!(plan.new_session.is_none()); let confirmed = plan_apply(¤t, &scene, "builtin:x", ApplyStrength::Soft, true); diff --git a/crates/core/src/session.rs b/crates/core/src/session.rs index b05b12b8..a4425511 100644 --- a/crates/core/src/session.rs +++ b/crates/core/src/session.rs @@ -185,6 +185,7 @@ pub enum PendingInputKind { Elicitation, } +/// Read-only compatibility for plan rows written by older versions. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct PlanEntry { pub content: String, @@ -655,12 +656,7 @@ pub fn transcript_context_with_omission( let text = match part { Part::Text { text } => text.trim().to_string(), Part::Prompt { text, .. } => text.trim().to_string(), - Part::Plan { entries } => entries - .iter() - .map(|entry| format!("- {}", entry.content)) - .collect::>() - .join("\n"), - Part::Reasoning { .. } | Part::ToolCall { .. } => continue, + Part::Reasoning { .. } | Part::ToolCall { .. } | Part::Plan { .. } => continue, }; if text.is_empty() { continue; @@ -750,7 +746,7 @@ mod transcript_context_tests { use super::{transcript_context, Part, Role}; #[test] - fn context_omits_reasoning_and_tools_but_keeps_conversation_and_plans() { + fn context_omits_reasoning_tools_and_retired_plans() { let transcript = vec![ ( Role::User, @@ -786,7 +782,7 @@ mod transcript_context_tests { let context = transcript_context("Release", &transcript); assert!(context.contains("ship the feature")); - assert!(context.contains("- verify\n- release")); + assert!(!context.contains("- verify\n- release")); assert!(!context.contains("private chain")); assert!(!context.contains("dangerous payload")); } diff --git a/crates/core/src/skill.rs b/crates/core/src/skill.rs index d3c0712a..7c2f3d05 100644 --- a/crates/core/src/skill.rs +++ b/crates/core/src/skill.rs @@ -1159,19 +1159,6 @@ fn private_appshot_file( /// and the TUI; merged with any user skills loaded from disk. pub fn builtin_skills() -> Vec { vec![ - Skill { - id: "plan-first".into(), - name: "Plan first".into(), - description: "Propose a plan and wait for approval before editing".into(), - icon: None, - source: None, - payload: SkillPayload::Fragment { - text: "Before changing anything, produce a short numbered plan of the steps you \ - intend to take, and wait for my approval. Do not edit files or run \ - destructive commands until I approve the plan." - .into(), - }, - }, Skill { id: "reviewer".into(), name: "Code Reviewer".into(), diff --git a/crates/core/src/unix_process_group.rs b/crates/core/src/unix_process_group.rs new file mode 100644 index 00000000..61530d40 --- /dev/null +++ b/crates/core/src/unix_process_group.rs @@ -0,0 +1,30 @@ +//! Signal a process group created and owned by this host. + +use std::io; + +const SIGKILL: i32 = 9; +const ESRCH: i32 = 3; + +unsafe extern "C" { + fn killpg(process_group: i32, signal: i32) -> i32; +} + +pub(crate) fn kill(process_group: i32) -> io::Result<()> { + signal(process_group, SIGKILL).map(|_| ()) +} + +pub(crate) fn is_missing(error: &io::Error) -> bool { + error.raw_os_error() == Some(ESRCH) +} + +fn signal(process_group: i32, signal: i32) -> io::Result { + if unsafe { killpg(process_group, signal) } == 0 { + return Ok(true); + } + let error = io::Error::last_os_error(); + if is_missing(&error) { + Ok(false) + } else { + Err(error) + } +} diff --git a/crates/core/tests/acp_process_cleanup.rs b/crates/core/tests/acp_process_cleanup.rs new file mode 100644 index 00000000..1266a1c6 --- /dev/null +++ b/crates/core/tests/acp_process_cleanup.rs @@ -0,0 +1,82 @@ +//! Adapter wrappers can spawn children that retain ACP stdio after the wrapper is killed. +#![cfg(unix)] + +use std::sync::Arc; +use std::time::Duration; + +use codetwo_core::acp::{spawn, RecordingHandler}; +use codetwo_core::provider::LaunchSpec; +use serde_json::json; + +const WRAPPER: &str = r#" +import json, os, subprocess, sys +child = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)']) +for line in sys.stdin: + message = json.loads(line) + if message.get('method') == 'initialize': + print(json.dumps({'jsonrpc':'2.0', 'id':message['id'], 'result':{ + 'protocolVersion':1, 'agentInfo':{'name':str(child.pid), 'version':'test'}}}), flush=True) + if os.environ.get('EXIT_WRAPPER'): break +"#; + +fn is_running(pid: u32) -> bool { + let output = std::process::Command::new("ps") + .args(["-o", "stat=", "-p", &pid.to_string()]) + .output() + .unwrap(); + let state = String::from_utf8_lossy(&output.stdout); + !state.trim().is_empty() && !state.trim().starts_with('Z') +} + +#[tokio::test] +async fn terminate_and_drop_close_descendants_and_pending_rpc() { + // An unrelated process must survive cleanup of each separately owned ACP process group. + let mut unrelated = tokio::process::Command::new("python3") + .args(["-c", "import time; time.sleep(60)"]) + .kill_on_drop(true) + .spawn() + .unwrap(); + for (explicit, wrapper_exits) in [(true, false), (false, false), (true, true)] { + let mut launch = LaunchSpec::new("python3", ["-c", WRAPPER]); + if wrapper_exits { + launch.env.push(("EXIT_WRAPPER".into(), "1".into())); + } + let client = spawn(&launch, Arc::new(RecordingHandler::default())) + .await + .unwrap(); + let info = client + .initialize(json!({})) + .await + .unwrap() + .agent_info + .unwrap(); + let descendant = info.name.parse::().unwrap(); + assert!(is_running(descendant)); + let connection = client.connection().clone(); + if explicit { + client.terminate(); + client.terminate(); // Idempotent while the handle still exists. + } + drop(client); + let closed = tokio::time::timeout( + Duration::from_secs(2), + connection.request::<_, serde_json::Value>("test/pending", json!({})), + ) + .await; + let deadline = tokio::time::Instant::now() + Duration::from_secs(2); + while is_running(descendant) && tokio::time::Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(10)).await; + } + let leaked = is_running(descendant); + if leaked { + // Dispose the exact fixture child even when this regression fails on the old code. + let _ = std::process::Command::new("kill") + .args(["-KILL", &descendant.to_string()]) + .status(); + } + assert!(!leaked, "the adapter descendant survived teardown"); + assert!(matches!(closed, Ok(Err(_))), "pending RPC did not close"); + assert!(is_running(unrelated.id().unwrap())); + } + unrelated.kill().await.unwrap(); +} diff --git a/crates/core/tests/engine_activity.rs b/crates/core/tests/engine_activity.rs index 152ad38f..d8a8b6f9 100644 --- a/crates/core/tests/engine_activity.rs +++ b/crates/core/tests/engine_activity.rs @@ -401,6 +401,94 @@ async fn model_switch_is_rejected_while_a_turn_owns_the_session() { } } +#[tokio::test] +async fn retired_planning_config_is_rejected_while_supported_choices_still_work() { + const AGENT: &str = r#" +import json, sys +for line in sys.stdin: + message = json.loads(line) + method = message.get("method") + if method == "initialize": + result = {"protocolVersion":1} + elif method == "session/new": + result = {"sessionId":"config-session", "configOptions":[ + {"id":"collaboration_mode","name":"Mode","type":"select", + "currentValue":"default","options":[{"value":"plan","name":"Plan"}]}, + {"id":"effort","name":"Effort","type":"select", + "currentValue":"low","options":[{"value":"high","name":"High"}]} + ]} + elif method == "session/prompt": + result = {"stopReason":"end_turn"} + elif method == "session/set_config_option": + result = {"configOptions":[]} + else: + continue + print(json.dumps({"jsonrpc":"2.0","id":message["id"],"result":result}), flush=True) +"#; + let (engine, mut rx) = Engine::new(vec![provider(AGENT)], SkillLibrary::new(vec![])); + let session = create_session(&engine, &mut rx).await; + engine + .submit(prompt(&session, "config-test")) + .await + .unwrap(); + let mut saw_supported_config = false; + loop { + match next_event(&mut rx).await { + Event::ConfigOptions { options, .. } => { + assert!(options + .iter() + .all(|option| option.id != "collaboration_mode")); + saw_supported_config |= options.iter().any(|option| option.id == "effort"); + } + Event::TurnEnded { .. } => break, + Event::Error { message, .. } => panic!("unexpected prompt error: {message}"), + _ => {} + } + } + assert!(saw_supported_config); + + engine + .submit(Op::SetConfigOption { + session: session.clone(), + config_id: "collaboration_mode".into(), + value: "plan".into(), + }) + .await + .unwrap(); + loop { + match next_event(&mut rx).await { + Event::Error { + message, terminal, .. + } => { + assert!(!terminal); + assert!( + message.contains("unsupported session config choice"), + "{message}" + ); + break; + } + Event::ConfigOptions { .. } => panic!("retired config reached the provider"), + _ => {} + } + } + + engine + .submit(Op::SetConfigOption { + session, + config_id: "effort".into(), + value: "high".into(), + }) + .await + .unwrap(); + loop { + match next_event(&mut rx).await { + Event::ConfigOptions { .. } => break, + Event::Error { message, .. } => panic!("supported config rejected: {message}"), + _ => {} + } + } +} + #[tokio::test] async fn cancel_drains_permissions_before_acp_finishes_the_turn() { let (engine, mut rx) = Engine::new(vec![provider(CANCEL_AGENT)], SkillLibrary::new(vec![])); diff --git a/crates/core/tests/engine_builtin_models.rs b/crates/core/tests/engine_builtin_models.rs index 6df9ba8b..89ba1b96 100644 --- a/crates/core/tests/engine_builtin_models.rs +++ b/crates/core/tests/engine_builtin_models.rs @@ -1,12 +1,6 @@ -//! A provider that reports no models of its own still gives the user something to pick from: the -//! engine offers the provider's built-in list as soon as the session exists, before any prompt has -//! been sent — which is also the only moment an agent would ever report its own. -//! -//! The mock agent here answers `initialize` and nothing else, which is all `session/new` (the Op, -//! not the ACP call — that one is deferred to the first prompt) needs. +//! A silent provider must not create selectable model ids that were never discovered. use codetwo_core::event::Event; -use codetwo_core::models::builtin_models; use codetwo_core::provider::{LaunchSpec, Provider, ProviderId}; use codetwo_core::skill::SkillLibrary; use codetwo_core::{Engine, Op}; @@ -24,7 +18,7 @@ for line in sys.stdin: "#; #[tokio::test] -async fn a_silent_provider_still_gets_a_model_list() { +async fn a_silent_provider_does_not_invent_a_model_list() { let provider = Provider { id: ProviderId::Grok, display_name: "Mock".into(), @@ -49,7 +43,7 @@ async fn a_silent_provider_still_gets_a_model_list() { let mut created_request = None; let mut listed: Option<(Vec, String)> = None; - while let Some(ev) = rx.recv().await { + while let Ok(ev) = rx.try_recv() { match ev { Event::SessionCreated { request_id, .. } => created_request = request_id, Event::Models { @@ -64,12 +58,34 @@ async fn a_silent_provider_still_gets_a_model_list() { } assert_eq!(created_request.as_deref(), Some("desktop-request")); - let (ids, current) = listed.expect("a models event"); - let expected: Vec = builtin_models(&ProviderId::Grok) - .into_iter() - .map(|m| m.id) - .collect(); - assert_eq!(ids, expected); - // A desktop choice made before the session exists remains authoritative until ACP starts. - assert_eq!(current, "grok-code-fast-1"); + assert!( + listed.is_none(), + "silent provider invented choices: {listed:?}" + ); + engine.shutdown(); +} + +/// Probe in a subprocess so environment overrides cannot race other integration tests. +#[test] +fn codex_runtime_override_is_forwarded_to_the_adapter() { + const PROBE: &str = "CODETWO_RUNTIME_OVERRIDE_PROBE"; + if std::env::var_os(PROBE).is_some() { + let expected = std::env::var("CODEX_PATH").unwrap(); + let runtime = codetwo_core::codex_runtime::CodexRuntimeDiscovery::detect(); + assert_eq!(runtime.codex_path, Some(std::path::PathBuf::from(&expected))); + let provider = codetwo_core::provider::registry_with_codex_runtime(&runtime) + .into_iter() + .find(|provider| provider.id == ProviderId::Codex) + .unwrap(); + assert!(provider.launch.env.contains(&("CODEX_PATH".into(), expected))); + return; + } + let dir = tempfile::tempdir().unwrap(); + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "codex_runtime_override_is_forwarded_to_the_adapter"]) + .env(PROBE, "1") + .env("CODEX_PATH", dir.path().join("explicit-runtime")) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); } diff --git a/crates/core/tests/engine_permission.rs b/crates/core/tests/engine_permission.rs index ad13ca56..c796b7ac 100644 --- a/crates/core/tests/engine_permission.rs +++ b/crates/core/tests/engine_permission.rs @@ -13,7 +13,7 @@ use codetwo_core::event::Event; use codetwo_core::permission::{ PermissionContextKind, PermissionMode, PermissionPolicy, SandboxPolicy, }; -use codetwo_core::{PermissionRouter, ProviderId, SessionHandler}; +use codetwo_core::{PermissionRouter, SessionHandler}; use serde_json::{json, Value}; use tokio::io::{AsyncBufReadExt, AsyncWrite, AsyncWriteExt, BufReader}; use tokio::sync::mpsc; @@ -93,7 +93,6 @@ async fn permission_is_parked_then_answered() { let policy = Arc::new(Mutex::new(PermissionPolicy::default())); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, policy, router.clone(), @@ -176,7 +175,6 @@ async fn mcp_elicitation_still_parks_in_full_access() { })); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, policy, router.clone(), @@ -244,7 +242,6 @@ async fn internal_auto_scene_selection_does_not_ask_before_the_broker() { let policy = Arc::new(Mutex::new(PermissionPolicy::default())); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, policy, router, @@ -315,7 +312,6 @@ async fn internal_auto_scene_permission_escalation_still_asks() { let router = PermissionRouter::default(); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, Arc::new(Mutex::new(PermissionPolicy::default())), router.clone(), @@ -417,7 +413,6 @@ async fn sites_production_action_still_parks_in_full_access() { })); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, policy, router.clone(), diff --git a/crates/core/tests/engine_provider_switch.rs b/crates/core/tests/engine_provider_switch.rs index 23cf4148..2f157e4c 100644 --- a/crates/core/tests/engine_provider_switch.rs +++ b/crates/core/tests/engine_provider_switch.rs @@ -56,7 +56,7 @@ for line in sys.stdin: used_old_cursor = True send({"jsonrpc":"2.0","id":mid,"error":{"code":-32000,"message":"old cursor must not cross providers"}}) elif method == "session/new": - send({"jsonrpc":"2.0","id":mid,"result":{"sessionId":"target-session"}}) + send({"jsonrpc":"2.0","id":mid,"result":{"sessionId":"target-session","models":{"currentModelId":"account-model","availableModels":[{"modelId":"account-model","name":"Account model"}]}}}) elif method == "session/prompt": prompt = json.dumps(message["params"].get("prompt", [])) required = ["first user request", "from-provider-a", "Compile workspace: completed", "second user request"] @@ -233,6 +233,18 @@ async fn switch_keeps_the_conversation_and_sends_only_provider_neutral_history() let replies = run_turn(&engine, &mut rx, &session, "second user request", "turn-b").await; assert_eq!(replies, vec!["CONTINUATION_OK"]); + let catalogue = engine.provider_catalog(); + let (_, models) = catalogue + .iter() + .find(|(provider, _)| provider.id == ProviderId::Pi) + .unwrap(); + assert_eq!( + models + .iter() + .map(|model| model.id.as_str()) + .collect::>(), + ["account-model"] + ); assert!(store.handoff_context(&session).unwrap().is_none()); assert_eq!(store.transcript(&session).unwrap().len(), 6); engine.shutdown(); @@ -391,20 +403,182 @@ fn managed_task_session_lease_blocks_provider_identity_changes() { ); } -fn live_provider_id(value: &str) -> ProviderId { - match value { - "claude_code" => ProviderId::ClaudeCode, - "codex" => ProviderId::Codex, - "grok" => ProviderId::Grok, - "cursor" => ProviderId::Cursor, - "opencode" => ProviderId::OpenCode, - "opencode2" => ProviderId::OpenCode2, - "pi" => ProviderId::Pi, - "kimi" => ProviderId::Kimi, - "zcode" => ProviderId::ZCode, - "amp" => ProviderId::Amp, - "droid" => ProviderId::Droid, - other => panic!("unsupported live provider id: {other}"), +#[tokio::test] +async fn repeated_and_unprompted_switches_use_canonical_history() { + let store = Arc::new(Store::open_in_memory().unwrap()); + let providers = vec![ + provider(ProviderId::Grok, "Source mock", SOURCE_AGENT), + provider(ProviderId::Pi, "Target mock", TARGET_AGENT), + ]; + let (engine, mut rx) = Engine::with_store(providers, SkillLibrary::new(vec![]), store.clone()); + let session = create_session(&engine, &mut rx, ProviderId::Grok).await; + run_turn(&engine, &mut rx, &session, "first user request", "first").await; + for round in 0..4 { + engine + .switch_provider(&session, ProviderId::Pi, None) + .await + .unwrap(); + // Switch back before the target has consumed the pending continuation. + engine + .switch_provider(&session, ProviderId::Grok, None) + .await + .unwrap(); + engine + .switch_provider(&session, ProviderId::Pi, None) + .await + .unwrap(); + let context = store.handoff_context(&session).unwrap().unwrap(); + assert_eq!(context["kind"], "provider_switch"); + assert!(!context["history"].to_string().contains("sourceProvider")); + assert_eq!( + run_turn( + &engine, + &mut rx, + &session, + "second user request", + &format!("round-{round}") + ) + .await, + vec!["CONTINUATION_OK"] + ); + engine + .switch_provider(&session, ProviderId::Grok, None) + .await + .unwrap(); + } + engine.shutdown(); +} + +#[tokio::test] +async fn cancelled_first_turn_retains_context_through_restart_and_native_restore() { + const CANCEL_THEN_RESTORE: &str = r#" +import json, sys +restored = False +for line in sys.stdin: + m = json.loads(line) + method, mid = m.get("method"), m.get("id") + result = {} + if method == "initialize": + result = {"protocolVersion":1,"agentCapabilities":{"loadSession":True}} + elif method == "session/new": + result = {"sessionId":"retry-session"} + elif method == "session/load": + restored = True + elif method == "session/prompt": + if not restored: + result = {"stopReason":"cancelled"} + else: + prompt = json.dumps(m["params"]["prompt"]) + ok = "first user request" in prompt and "from-provider-a" in prompt + print(json.dumps({"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"retry-session","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"RESTORED_OK" if ok else "LOST_CONTEXT"}}}}), flush=True) + result = {"stopReason":"end_turn"} + if mid is not None: + print(json.dumps({"jsonrpc":"2.0","id":mid,"result":result}), flush=True) +"#; + let store = Arc::new(Store::open_in_memory().unwrap()); + let providers = vec![ + provider(ProviderId::Grok, "Source", SOURCE_AGENT), + provider(ProviderId::Pi, "Retry target", CANCEL_THEN_RESTORE), + ]; + let (engine, mut rx) = + Engine::with_store(providers.clone(), SkillLibrary::new(vec![]), store.clone()); + let session = create_session(&engine, &mut rx, ProviderId::Grok).await; + run_turn(&engine, &mut rx, &session, "first user request", "first").await; + engine + .switch_provider(&session, ProviderId::Pi, None) + .await + .unwrap(); + run_turn(&engine, &mut rx, &session, "cancelled request", "cancel").await; + assert!(store.handoff_context(&session).unwrap().is_some()); + engine.shutdown(); + let (revived, mut rx) = Engine::with_store(providers, SkillLibrary::new(vec![]), store.clone()); + assert_eq!( + run_turn(&revived, &mut rx, &session, "retry", "retry").await, + vec!["RESTORED_OK"] + ); + assert!(store.handoff_context(&session).unwrap().is_none()); + revived.shutdown(); +} + +#[tokio::test] +async fn unavailable_history_refuses_switch_instead_of_silently_losing_context() { + let (engine, mut rx) = Engine::new( + vec![ + provider(ProviderId::Grok, "Source", SOURCE_AGENT), + provider(ProviderId::Pi, "Target", TARGET_AGENT), + ], + SkillLibrary::new(vec![]), + ); + let session = create_session(&engine, &mut rx, ProviderId::Grok).await; + assert!(engine + .switch_provider(&session, ProviderId::Pi, None) + .await + .unwrap_err() + .contains("saved conversation history")); + assert_eq!( + run_turn(&engine, &mut rx, &session, "still usable", "first").await, + vec!["from-provider-a"] + ); + engine.shutdown(); +} + +#[tokio::test] +async fn failed_first_prompt_keeps_continuation_for_retry() { + let store = Arc::new(Store::open_in_memory().unwrap()); + let mut target = provider(ProviderId::Pi, "Fail once", TARGET_AGENT); + target.launch.args[1] = TARGET_AGENT + .replace( + "used_old_cursor = False", + "used_old_cursor = False\nfailed = False", + ) + .replace( + " prompt = json.dumps", + r#" if not failed: + failed = True + send({"jsonrpc":"2.0","id":mid,"error":{"code":-32000,"message":"temporary failure"}}) + continue + prompt = json.dumps"#, + ); + let (engine, mut rx) = Engine::with_store( + vec![provider(ProviderId::Grok, "Source", SOURCE_AGENT), target], + SkillLibrary::new(vec![]), + store.clone(), + ); + let session = create_session(&engine, &mut rx, ProviderId::Grok).await; + run_turn(&engine, &mut rx, &session, "first user request", "first").await; + engine + .switch_provider(&session, ProviderId::Pi, None) + .await + .unwrap(); + engine + .submit(prompt(&session, "second user request", "failure")) + .await + .unwrap(); + loop { + if let Event::Error { + terminal: true, + message, + .. + } = next_event(&mut rx).await + { + assert!(message.contains("temporary failure")); + break; + } + } + assert!(store.handoff_context(&session).unwrap().is_some()); + assert_eq!( + run_turn(&engine, &mut rx, &session, "second user request", "retry").await, + vec!["CONTINUATION_OK"] + ); + assert!(store.handoff_context(&session).unwrap().is_none()); + engine.shutdown(); +} + +struct LiveEngineGuard(Engine); + +impl Drop for LiveEngineGuard { + fn drop(&mut self) { + self.0.shutdown(); } } @@ -467,10 +641,24 @@ async fn run_live_turn( let token = format!("C2_{}_{}_OK", provider.as_str().to_uppercase(), index); let expected = format!("{token}_{continuity_key}"); let instruction = if index == 0 { + let context_chars = std::env::var("CODETWO_LIVE_SWITCH_CONTEXT_CHARS") + .map(|value| { + value + .parse::() + .expect("context size must be an integer") + }) + .unwrap_or(0); + assert!( + context_chars <= 256 * 1024, + "live fixture is limited to 256 Ki characters" + ); + let filler = "Synthetic reference data: the desktop app uses Rust and TypeScript. "; + let reference = filler.repeat(context_chars.div_ceil(filler.len())); + let reference = &reference[..context_chars]; format!( "Remember this continuity key for later provider switches: {continuity_key}. \ - The project is a Rust and TypeScript desktop app. Reply with exactly {expected} \ - and no other text. Do not use tools." + The following reference appendix is synthetic test data.\n{reference}\n\ + Reply with exactly {expected} and no other text. Do not use tools." ) } else { format!( @@ -577,13 +765,21 @@ fn assert_live_session_has_content(store: &Store, session: &str, continuity_key: #[tokio::test] #[ignore = "requires locally authenticated real provider CLIs"] async fn live_providers_switch_in_place_and_back() { + let providers = default_registry(); let provider_names = std::env::var("CODETWO_LIVE_SWITCH_PROVIDERS") - .unwrap_or_else(|_| "codex,grok,cursor,codex".into()); + .expect("set CODETWO_LIVE_SWITCH_PROVIDERS to the authenticated provider ids to exercise"); let sequence = provider_names .split(',') .map(str::trim) .filter(|value| !value.is_empty()) - .map(live_provider_id) + .map(|name| { + providers + .iter() + .find(|provider| provider.id.as_str() == name) + .unwrap_or_else(|| panic!("provider {name} is not registered")) + .id + .clone() + }) .collect::>(); assert!(sequence.len() >= 2, "provide at least two provider ids"); assert!( @@ -593,7 +789,8 @@ async fn live_providers_switch_in_place_and_back() { let store = Arc::new(Store::open_in_memory().unwrap()); let (engine, mut rx) = - Engine::with_store(default_registry(), SkillLibrary::new(vec![]), store.clone()); + Engine::with_store(providers, SkillLibrary::new(vec![]), store.clone()); + let _cleanup = LiveEngineGuard(engine.clone()); let session = create_live_session(&engine, &mut rx, sequence[0].clone()).await; let original_session = session.clone(); let continuity_key = format!("C2_CONTEXT_{}", uuid::Uuid::new_v4().simple()); diff --git a/crates/core/tests/engine_store.rs b/crates/core/tests/engine_store.rs index ee679c20..0e9a8514 100644 --- a/crates/core/tests/engine_store.rs +++ b/crates/core/tests/engine_store.rs @@ -91,7 +91,6 @@ async fn agent_output_is_persisted() { })); let handler = Arc::new(SessionHandler::new( "s1".into(), - ProviderId::Codex, events_tx, policy, router, diff --git a/crates/core/tests/scene_conformance.rs b/crates/core/tests/scene_conformance.rs index 358fb9d7..c5932ef3 100644 --- a/crates/core/tests/scene_conformance.rs +++ b/crates/core/tests/scene_conformance.rs @@ -64,7 +64,10 @@ fn develop_brief_is_typed() { let execution = develop.execution.as_ref().unwrap(); assert_eq!(execution.session_mode, Some(SceneSessionMode::AutoEdit)); assert_eq!(execution.worktree, Some(SceneWorktree::Current)); - assert_eq!(execution.plan_first, Some(true)); + assert!(serde_json::to_value(execution) + .unwrap() + .get("plan_first") + .is_none()); let artifact_ids: Vec<&str> = develop.artifacts.iter().map(|a| a.id.as_str()).collect(); assert_eq!(artifact_ids, vec!["plan", "change-summary"]); diff --git a/crates/server/src/t3_compat.rs b/crates/server/src/t3_compat.rs index 2fe97488..be4d3afb 100644 --- a/crates/server/src/t3_compat.rs +++ b/crates/server/src/t3_compat.rs @@ -22,8 +22,8 @@ use axum::{Json, Router}; use chrono::{DateTime, Duration, SecondsFormat, Utc}; use codetwo_core::event::ModelChoice; use codetwo_core::{ - builtin_models, default_registry, DocBlock, Engine, Event, ExecutionPolicy, Op, Part, - PendingInputKind, PermissionMode, ProviderId, Role, SandboxPolicy, Session, SessionRunState, + default_registry, DocBlock, Engine, Event, ExecutionPolicy, Op, Part, PendingInputKind, + PermissionMode, ProviderId, Role, SandboxPolicy, Session, SessionRunState, MAX_TRANSCRIPT_TURNS, }; use futures_util::{SinkExt, StreamExt}; @@ -35,8 +35,7 @@ use tokio::sync::{broadcast, Mutex as AsyncMutex}; use crate::{AuthState, WS_TICKET_TTL}; const T3_CONTRACT_VERSION: &str = "0.0.33-codetwo.1"; -const PLAN_SKILL_ID: &str = "plan-first"; -const PLAN_PROMPT_PREFIX: &str = "[skill:plan-first]\n\n"; +const LEGACY_PLAN_PROMPT_PREFIX: &str = "[skill:plan-first]\n\n"; const ACCESS_TOKEN_TTL_DAYS: i64 = 30; const TOKEN_EXCHANGE_GRANT: &str = "urn:ietf:params:oauth:grant-type:token-exchange"; const BOOTSTRAP_TOKEN_TYPE: &str = "urn:t3:params:oauth:token-type:environment-bootstrap"; @@ -71,8 +70,6 @@ struct CompatibilityMetadata { #[serde(default)] aliases: HashMap, #[serde(default)] - interaction_modes: HashMap, - #[serde(default)] command_receipts: Vec, } @@ -81,7 +78,6 @@ impl Default for CompatibilityMetadata { Self { version: COMPATIBILITY_STATE_VERSION, aliases: HashMap::new(), - interaction_modes: HashMap::new(), command_receipts: Vec::new(), } } @@ -133,11 +129,6 @@ fn validate_compatibility(metadata: &CompatibilityMetadata) -> Result<(), String if unique_core_ids.len() != metadata.aliases.len() { return Err("multiple public thread ids map to the same C2 session".into()); } - if metadata.interaction_modes.iter().any(|(thread_id, mode)| { - thread_id.trim().is_empty() || !matches!(mode.as_str(), "default" | "plan") - }) { - return Err("invalid persisted T3 interaction mode".into()); - } if metadata.command_receipts.len() > MAX_PERSISTED_COMMAND_RECEIPTS || metadata .command_receipts @@ -329,11 +320,6 @@ impl T3CompatState { .map_err(|error| error.error.to_string()) } - fn mark_updated(&self) { - let sequence = self.sequence.fetch_add(1, Ordering::SeqCst) + 1; - let _ = self.updates.send(CoreUpdate { sequence }); - } - fn persist_command_receipt(&self, command_id: &str, sequence: u64) -> Result<(), String> { if self.compatibility_load_error.is_some() { return Err(compatibility_unavailable_message()); @@ -579,36 +565,10 @@ impl T3CompatState { let mut seen = HashSet::new(); let mut values = Vec::new(); - for provider in default_registry() { + for (provider, models) in self.engine.provider_catalog() { let instance_id = provider_slug(&provider.id); seen.insert(instance_id.clone()); let installed = provider.is_available(); - let mut models = builtin_models(&provider.id); - for session in sessions - .iter() - .filter(|session| session.provider == provider.id) - { - if let Some(model) = session - .model - .as_ref() - .filter(|model| !model.trim().is_empty()) - { - if !models.iter().any(|choice| choice.id == *model) { - models.push(ModelChoice { - id: model.clone(), - name: model.clone(), - description: None, - }); - } - } - } - if models.is_empty() { - models.push(ModelChoice { - id: "default".into(), - name: "Default".into(), - description: None, - }); - } values.push(provider_value( &instance_id, &provider.display_name, @@ -618,23 +578,18 @@ impl T3CompatState { )); } - // Keep already-existing custom-provider threads decodable even though the public Engine - // API does not expose its private provider registry. + // Keep historical threads decodable after their integration is removed. A saved thread + // does not establish that its provider or model is currently available. for session in sessions { let instance_id = provider_slug(&session.provider); if !seen.insert(instance_id.clone()) { continue; } - let model = session.model.clone().unwrap_or_else(|| "default".into()); values.push(provider_value( &instance_id, &provider_display_name(&session.provider), - true, - vec![ModelChoice { - id: model.clone(), - name: model, - description: None, - }], + false, + Vec::new(), &checked_at, )); } @@ -662,62 +617,18 @@ impl T3CompatState { .unwrap_or_else(|| public_id.to_string()) } - fn interaction_mode(&self, public_id: &str) -> String { - self.compatibility - .lock() - .unwrap() - .interaction_modes - .get(public_id) - .cloned() - .unwrap_or_else(|| "default".into()) - } - - fn validate_interaction_mode(mode: &str) -> Result<&str, String> { + fn validate_interaction_mode(mode: &str) -> Result<(), String> { match mode { - "default" | "plan" => Ok(mode), + "default" => Ok(()), _ => Err(format!("unsupported T3 interaction mode: {mode}")), } } - fn set_interaction_mode(&self, public_id: &str, mode: &str) -> Result { - if self.compatibility_load_error.is_some() { - return Err(compatibility_unavailable_message()); - } - let mode = Self::validate_interaction_mode(mode)?; - let changed = { - let mut compatibility = self.compatibility.lock().unwrap(); - if compatibility - .interaction_modes - .get(public_id) - .is_some_and(|current| current == mode) - { - false - } else { - compatibility - .interaction_modes - .insert(public_id.to_string(), mode.to_string()); - true - } - }; - if changed { - self.persist_compatibility().map_err(|error| { - format!("could not persist the T3 mobile interaction mode: {error}") - })?; - } - Ok(changed) - } - - fn persist_thread_identity( - &self, - public_id: &str, - core_id: &str, - interaction_mode: &str, - ) -> Result<(), String> { + fn persist_thread_identity(&self, public_id: &str, core_id: &str) -> Result<(), String> { if self.compatibility_load_error.is_some() { return Err(compatibility_unavailable_message()); } - let interaction_mode = Self::validate_interaction_mode(interaction_mode)?; - let (previous_alias, previous_mode) = { + let previous_alias = { let mut compatibility = self.compatibility.lock().unwrap(); if compatibility .aliases @@ -728,13 +639,9 @@ impl T3CompatState { { return Err("C2 session is already mapped to another T3 thread".into()); } - let previous_alias = compatibility + compatibility .aliases - .insert(public_id.to_string(), core_id.to_string()); - let previous_mode = compatibility - .interaction_modes - .insert(public_id.to_string(), interaction_mode.to_string()); - (previous_alias, previous_mode) + .insert(public_id.to_string(), core_id.to_string()) }; if let Err(error) = self.persist_compatibility() { // Do not let a failed durable write turn into an in-memory-only alias that a retry @@ -750,16 +657,6 @@ impl T3CompatState { compatibility.aliases.remove(public_id); } } - match previous_mode { - Some(previous) => { - compatibility - .interaction_modes - .insert(public_id.to_string(), previous); - } - None => { - compatibility.interaction_modes.remove(public_id); - } - } return Err(format!( "could not persist the T3 mobile thread identity: {error}" )); @@ -806,7 +703,6 @@ impl T3CompatState { fn thread_shell(&self, session: &Session, updated_at: &str) -> Value { let public_id = self.public_thread_id(&session.id); - let interaction_mode = self.interaction_mode(&public_id); let project = project_path(session, &self.cwd); let created_at = millis_iso(session.created_at); let (latest_turn, status, active_turn_id, last_error) = @@ -819,7 +715,7 @@ impl T3CompatState { "title": nonempty(&session.title, "Untitled session"), "modelSelection": model_selection(session), "runtimeMode": runtime_mode(session.permission_mode, session.sandbox_policy), - "interactionMode": interaction_mode, + "interactionMode": "default", "branch": Value::Null, "worktreePath": session.worktree_path, "latestTurn": latest_turn, @@ -875,8 +771,8 @@ impl T3CompatState { Part::Prompt { text, .. } => { flush_assistant_message(&mut messages, &session.id, &mut assistant_message); // C2's display projection is intentionally capped at 400 characters. T3 - // mobile expects the complete authored message, and the adapter-owned planning - // skill must stay hidden from that user-visible text. + // mobile expects the complete authored message. Hide legacy adapter-owned + // planning markers in old transcripts. let text = t3_user_prompt(&text); messages.push(message_value( external_message_ids @@ -939,16 +835,7 @@ impl T3CompatState { "createdAt": at, })); } - Part::Plan { entries } => activities.push(json!({ - "id": format!("codetwo-plan-{}-{}", session.id, entry.seq), - "tone": "info", - "kind": "assistant.plan", - "summary": "Plan", - "payload": { "entries": entries }, - "turnId": Value::Null, - "sequence": entry.seq.max(0), - "createdAt": at, - })), + Part::Plan { .. } => {} } } flush_assistant_message(&mut messages, &session.id, &mut assistant_message); @@ -1036,6 +923,12 @@ impl T3CompatState { let command_type = required_string(&command, "type")?; match command_type.as_str() { "thread.turn.start" => { + Self::validate_interaction_mode( + command + .get("interactionMode") + .and_then(Value::as_str) + .unwrap_or("default"), + )?; let public_id = required_string(&command, "threadId")?; let text = command .pointer("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/message/text") @@ -1068,19 +961,7 @@ impl T3CompatState { } else { self.create_thread(command_id, &public_id, &command).await? }; - // A preceding `thread.interaction-mode.set` is authoritative. Queued turns can - // carry stale composer state, so existing turns never mutate the durable mode. - let interaction_mode = self.interaction_mode(&public_id); - let mut doc = vec![DocBlock::Text { text }]; - if interaction_mode == "plan" { - doc.insert( - 0, - DocBlock::Skill { - skill_id: PLAN_SKILL_ID.into(), - params: HashMap::new(), - }, - ); - } + let doc = vec![DocBlock::Text { text }]; self.submit_prompt_and_wait(core_id, doc, command_id.to_string(), message_id) .await?; } @@ -1128,9 +1009,7 @@ impl T3CompatState { return Err(format!("unknown thread: {public_id}")); } let mode = required_string(&command, "interactionMode")?; - if self.set_interaction_mode(&public_id, &mode)? { - self.mark_updated(); - } + Self::validate_interaction_mode(&mode)?; } "thread.meta.update" => { let thread_id = self.core_thread_id(&required_string(&command, "threadId")?); @@ -1208,7 +1087,7 @@ impl T3CompatState { .or_else(|| command.get("interactionMode")) .and_then(Value::as_str) .unwrap_or("default"); - let interaction_mode = Self::validate_interaction_mode(interaction_mode)?; + Self::validate_interaction_mode(interaction_mode)?; // NewSession and this receipt share one SQLite transaction. If the process died before // the JSON alias/model projection landed, replay repairs those projections without @@ -1231,15 +1110,7 @@ impl T3CompatState { .await?; } } - let recovered_mode = self - .compatibility - .lock() - .unwrap() - .interaction_modes - .get(public_id) - .cloned() - .unwrap_or_else(|| interaction_mode.to_string()); - self.persist_thread_identity(public_id, &core_id, &recovered_mode)?; + self.persist_thread_identity(public_id, &core_id)?; return Ok(core_id); } } @@ -1301,7 +1172,7 @@ impl T3CompatState { // first Prompt observes the model chosen in T3's create-thread bootstrap payload. self.set_model_and_verify(core_id.clone(), model).await?; } - self.persist_thread_identity(public_id, &core_id, interaction_mode)?; + self.persist_thread_identity(public_id, &core_id)?; Ok(core_id) } @@ -2158,7 +2029,7 @@ fn flush_assistant_message( fn t3_user_prompt(canonical: &str) -> String { canonical - .strip_prefix(PLAN_PROMPT_PREFIX) + .strip_prefix(LEGACY_PLAN_PROMPT_PREFIX) .unwrap_or(canonical) .to_string() } @@ -2262,11 +2133,6 @@ fn model_selection(session: &Session) -> Value { .model .clone() .filter(|model| !model.trim().is_empty()) - .or_else(|| { - builtin_models(&session.provider) - .first() - .map(|model| model.id.clone()) - }) .unwrap_or_else(|| "default".into()); json!({ "instanceId": provider_slug(&session.provider), "model": model }) } @@ -2422,6 +2288,28 @@ fn platform_arch() -> &'static str { mod tests { use super::*; + #[tokio::test] + async fn provider_projection_uses_registered_integrations_without_invented_models() { + let directory = tempfile::tempdir().unwrap(); + let auth = Arc::new(AuthState::load(Some(directory.path().join("devices.json")))); + let provider = codetwo_core::provider::Provider { + id: ProviderId::Custom("future-agent".into()), + display_name: "Future agent".into(), + launch: codetwo_core::provider::LaunchSpec::new( + "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/missing/future-agent", + [] as [&str; 0], + ), + needs_node: false, + }; + let (engine, rx) = Engine::new(vec![provider], codetwo_core::SkillLibrary::new(Vec::new())); + let state = T3CompatState::new(Arc::new(engine), crate::fanout(rx), auth).unwrap(); + let providers = state.providers(); + assert_eq!(providers.len(), 1); + assert_eq!(providers[0]["displayName"], "Future agent"); + assert_eq!(providers[0]["installed"], false); + assert_eq!(providers[0]["models"], json!([])); + } + #[test] fn provider_and_runtime_mappings_match_t3_contract_values() { assert_eq!(provider_slug(&ProviderId::ClaudeCode), "claudeAgent"); @@ -2470,7 +2358,10 @@ mod tests { fn prompt_projection_is_complete_and_hides_the_adapter_plan_marker() { let long = "x".repeat(700); assert_eq!(t3_user_prompt(&long), long); - assert_eq!(t3_user_prompt(&format!("{PLAN_PROMPT_PREFIX}{long}")), long); + assert_eq!( + t3_user_prompt(&format!("{LEGACY_PLAN_PROMPT_PREFIX}{long}")), + long + ); } #[test] @@ -2545,15 +2436,25 @@ mod tests { } #[test] - fn compatibility_metadata_rejects_invalid_versions_modes_and_aliases() { - let mut metadata = CompatibilityMetadata::default(); - metadata.version += 1; - assert!(validate_compatibility(&metadata).is_err()); + fn retired_interaction_modes_are_rejected_and_legacy_state_is_ignored() { + assert!(T3CompatState::validate_interaction_mode("default").is_ok()); + assert!(T3CompatState::validate_interaction_mode("plan").is_err()); + let metadata: CompatibilityMetadata = serde_json::from_value(json!({ + "version": 1, + "aliases": {"thread-1": "core-1"}, + "interactionModes": {"thread-1": "plan"} + })) + .unwrap(); + assert!(validate_compatibility(&metadata).is_ok()); + let saved = serde_json::to_value(metadata).unwrap(); + assert_eq!(saved["aliases"]["thread-1"], "core-1"); + assert!(saved.get("interactionModes").is_none()); + } + #[test] + fn compatibility_metadata_rejects_invalid_versions_and_aliases() { let mut metadata = CompatibilityMetadata::default(); - metadata - .interaction_modes - .insert("thread-1".into(), "future-mode".into()); + metadata.version += 1; assert!(validate_compatibility(&metadata).is_err()); let mut metadata = CompatibilityMetadata::default(); @@ -2588,9 +2489,8 @@ mod tests { for index in 0..16 { let state = state.clone(); writers.push(tokio::task::spawn_blocking(move || { - let mode = if index % 2 == 0 { "default" } else { "plan" }; state - .set_interaction_mode(&format!("thread-{index}"), mode) + .persist_thread_identity(&format!("thread-{index}"), &format!("core-{index}")) .unwrap(); })); } @@ -2604,7 +2504,7 @@ mod tests { error.is_none(), "persisted metadata did not reload: {error:?}" ); - assert_eq!(reloaded.interaction_modes.len(), 16); + assert_eq!(reloaded.aliases.len(), 16); } #[tokio::test] @@ -2616,7 +2516,7 @@ mod tests { let (engine, rx) = Engine::new(Vec::new(), codetwo_core::SkillLibrary::new(Vec::new())); let state = T3CompatState::new(Arc::new(engine), crate::fanout(rx), auth).unwrap(); - assert!(state.set_interaction_mode("thread-1", "plan").is_err()); + assert!(state.persist_thread_identity("thread-1", "core-1").is_err()); assert_eq!(std::fs::read(&path).unwrap(), b"{not valid JSON"); } } diff --git a/crates/server/tests/t3_mobile_compat.rs b/crates/server/tests/t3_mobile_compat.rs index 3a1ac9d4..79602062 100644 --- a/crates/server/tests/t3_mobile_compat.rs +++ b/crates/server/tests/t3_mobile_compat.rs @@ -33,7 +33,8 @@ for line in sys.stdin: prompt = json.dumps(message.get("params", {})) if "hold open" in prompt: time.sleep(0.5) - reply = "plan reply" if "Before changing anything" in prompt else "phone reply" + assert "Before changing anything" not in prompt + reply = "long reply" if "long request" in prompt else "phone reply" split = max(1, len(reply) // 2) for chunk in (reply[:split], reply[split:]): send({"jsonrpc":"2.0","method":"session/update","params":{ @@ -782,9 +783,9 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { .await .unwrap(); let mode_receipt = next_json(&mut socket).await; - assert_eq!(mode_receipt["exit"]["_tag"], "Success"); + assert_eq!(mode_receipt["exit"]["_tag"], "Failure"); - let (status, plan_detail) = http( + let (status, long_detail) = http( addr, "GET", &format!("/api/orchestration/threads/{thread_id}"), @@ -794,21 +795,21 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { ) .await; assert_eq!(status, 200); - assert_eq!(plan_detail["thread"]["interactionMode"], "plan"); + assert_eq!(long_detail["thread"]["interactionMode"], "default"); - let plan_text = format!("plan this exact long request: {}", "x".repeat(700)); - let plan_command = json!({ + let long_text = format!("answer this exact long request: {}", "x".repeat(700)); + let long_command = json!({ "type": "thread.turn.start", - "commandId": "mobile-turn-plan", + "commandId": "mobile-turn-long", "threadId": thread_id, "message": { - "messageId": "mobile-message-plan", + "messageId": "mobile-message-long", "role": "user", - "text": plan_text, + "text": long_text, "attachments": [], }, "runtimeMode": "approval-required", - // Deliberately stale. Only the preceding explicit mode-set may mutate mode. + // A rejected Plan request leaves ordinary turns available. "interactionMode": "default", "modelSelection": { "instanceId": "t3mock", "model": "phone-model" }, "createdAt": "2026-08-12T00:02:00.000Z", @@ -817,19 +818,19 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { .send(Message::Text( json!({ "_tag": "Request", - "id": "dispatch-plan", + "id": "dispatch-long", "tag": "orchestration.dispatchCommand", - "payload": plan_command, + "payload": long_command, "headers": [], }) .to_string(), )) .await .unwrap(); - let plan_receipt = next_json(&mut socket).await; - assert_eq!(plan_receipt["exit"]["_tag"], "Success"); + let long_receipt = next_json(&mut socket).await; + assert_eq!(long_receipt["exit"]["_tag"], "Success"); - let mut plan_snapshot = Value::Null; + let mut long_snapshot = Value::Null; for _ in 0..40 { let (_, current) = http( addr, @@ -844,22 +845,22 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { .as_array() .unwrap() .iter() - .any(|message| message["role"] == "assistant" && message["text"] == "plan reply"); - plan_snapshot = current; + .any(|message| message["role"] == "assistant" && message["text"] == "long reply"); + long_snapshot = current; if complete { break; } tokio::time::sleep(Duration::from_millis(25)).await; } - let plan_messages = plan_snapshot["thread"]["messages"].as_array().unwrap(); - assert_eq!(plan_snapshot["thread"]["interactionMode"], "plan"); - assert!(plan_messages + let long_messages = long_snapshot["thread"]["messages"].as_array().unwrap(); + assert_eq!(long_snapshot["thread"]["interactionMode"], "default"); + assert!(long_messages .iter() - .any(|message| message["role"] == "user" && message["text"] == plan_text)); - assert!(plan_messages + .any(|message| message["role"] == "user" && message["text"] == long_text)); + assert!(long_messages .iter() - .any(|message| message["role"] == "assistant" && message["text"] == "plan reply")); - assert!(!plan_messages.iter().any(|message| message["text"] + .any(|message| message["role"] == "assistant" && message["text"] == "long reply")); + assert!(!long_messages.iter().any(|message| message["text"] .as_str() .is_some_and(|text| text.contains("[skill:plan-first]")))); assert_eq!(store.list_sessions().unwrap().len(), 1); @@ -893,7 +894,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { "attachments": [], }, "runtimeMode": "approval-required", - "interactionMode": "plan", + "interactionMode": "default", "modelSelection": { "instanceId": "t3mock", "model": "phone-model" }, "createdAt": "2026-08-12T00:03:00.000Z", }); @@ -929,7 +930,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { "attachments": [], }, "runtimeMode": "approval-required", - "interactionMode": "plan", + "interactionMode": "default", "modelSelection": { "instanceId": "t3mock", "model": "phone-model" }, "createdAt": "2026-08-12T00:03:01.000Z", }, @@ -973,7 +974,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { })); if std::env::var_os("CODETWO_DUMP_T3_FIXTURES").is_some() { - eprintln!("T3_THREAD_FIXTURE={plan_snapshot}"); + eprintln!("T3_THREAD_FIXTURE={long_snapshot}"); } drop(socket); @@ -988,6 +989,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { serde_json::from_slice(&std::fs::read(&compatibility_path).unwrap()).unwrap(); compatibility["commandReceipts"] = json!([]); compatibility["aliases"] = json!({}); + compatibility["interactionModes"] = json!({thread_id: "plan"}); std::fs::write( &compatibility_path, serde_json::to_vec_pretty(&compatibility).unwrap(), @@ -995,7 +997,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { .unwrap(); // T3 mobile owns this public id and keeps it as its cache key. Reload around the same durable - // C2 store and confirm both the id and selected interaction mode survive a server restart. + // C2 store and confirm the id survives while retired persisted Plan state is ignored. let (restart_engine, restart_rx) = Engine::with_store( vec![provider], SkillLibrary::new(builtin_skills()), @@ -1066,7 +1068,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { "reloaded thread snapshot failed: {restarted_detail}" ); assert_eq!(restarted_detail["thread"]["id"], thread_id); - assert_eq!(restarted_detail["thread"]["interactionMode"], "plan"); + assert_eq!(restarted_detail["thread"]["interactionMode"], "default"); assert_eq!( restarted_detail["thread"]["messages"] .as_array() @@ -1083,7 +1085,7 @@ async fn native_mobile_creates_a_thread_and_dispatches_a_prompt() { "/api/orchestration/dispatch", Some("application/json"), Some(bearer), - &plan_command.to_string(), + &long_command.to_string(), ) .await; assert_eq!( diff --git a/docs/reference/scenes.md b/docs/reference/scenes.md index cff25ba3..fae1b999 100644 --- a/docs/reference/scenes.md +++ b/docs/reference/scenes.md @@ -13,7 +13,7 @@ and declarative hooks. A **pipeline** chains scenes into a lifecycle (the built- research → develop → test → fix → acceptance, with a test/fix loop). Scenes exist because every ingredient already shipped separately — permission modes, memory -presets, Plan First, worktree baselines, the skill library — but the user had to reassemble them +presets, worktree baselines, the skill library — but the user had to reassemble them by hand for every kind of work. A scene is the packaging object; it introduces **no new execution capability**. Like plugins, installing or activating a scene never runs a script. @@ -76,15 +76,17 @@ project/user defaults when creating a session). Values reuse the existing vocabu | `session_mode` | `read_only` \| `ask` \| `auto_edit` \| `full_access` | `apps/desktop/src/session/mode.ts` | | `memory_preset` | `standard` \| `read_only` \| `private` \| `learn_only` | composer memory presets | | `worktree` | `off` \| `current` \| `origin_default` | `crates/core/src/project.rs` | -| `plan_first` | boolean | composer Plan First toggle | | `providers` | provider ids, preference order | `crates/core/src/provider.rs` registry | | `model`, `reasoning_effort` | provider-defined strings | provider capabilities | +Legacy `execution.plan_first` is accepted only when reading older scene files. It has no +effect and is omitted when saving or exporting a scene. + **Binding matrix.** Not everything can change mid-session; the host applies a scene at two strengths and must show which one happened: - **Soft-apply** (switching scenes inside a live session): `session_mode`, `memory_preset`, - `plan_first`, skills, brief, guardrails take effect immediately. `providers`, `model`, + skills, brief, guardrails take effect immediately. `providers`, `model`, `reasoning_effort` apply from the next session; `worktree` is immutable per session by design. - **Full-apply** (scene chosen at session creation, or "new session in this scene"): everything applies. When a soft-apply leaves fields pending, the scene chip shows a partial indicator and diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/intent.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/intent.md new file mode 100644 index 00000000..4b412810 --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/intent.md @@ -0,0 +1,19 @@ +--- +id: 2026-09-28-orchestration-stability +schema: 5 +stage: intent +status: accepted +owner: codex +created: 2026-09-28 +source: user +risk: medium +approved_by: user +approved_at: 2026-09-28 +approval_source: "Current chat: improve orchestration stability, cross-agent context switching, dynamic discovery, and trustworthy UX." +--- + +# Intent: Orchestration Stability + +## Intent + +Improve same-conversation agent switching across long histories, repeated switches and failures. Remove invented model/provider catalogues and keep UI discovery and switch state honest. The user's current request authorizes bounded local implementation and checks. Preserve session identity, existing storage schema, provider privacy boundaries and user processes. No release or external delivery requested. diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md new file mode 100644 index 00000000..5131f12f --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md @@ -0,0 +1,22 @@ +--- +id: 2026-09-28-orchestration-stability +schema: 5 +stage: plan +status: accepted +owner: codex +created: 2026-09-28 +based_on: spec.md +scope: crates/core/src/acp/client.rs, crates/core/src/acp/mod.rs, crates/core/src/unix_process_group.rs, crates/core/src/plugins/app/protocol/mod.rs, crates/core/tests/acp_process_cleanup.rs, crates/core/src/codex_runtime.rs, crates/core/src/models.rs, crates/core/src/engine.rs, crates/core/src/lib.rs, crates/core/src/acp/wire.rs, crates/core/src/event.rs, crates/core/src/cost.rs, crates/core/tests/engine_builtin_models.rs, crates/core/tests/engine_provider_switch.rs, crates/core/tests/engine_store.rs, crates/core/tests/engine_permission.rs, crates/server/src/t3_compat.rs, apps/desktop/src/App.tsx, apps/desktop/src/bridge.ts, apps/desktop/src/session/Composer.tsx, apps/desktop/src/session/config.ts, apps/desktop/src/session/turns.ts, apps/desktop/src/providers/registry.ts, apps/desktop/src/i18n/strings.ts, apps/desktop/tests, docs/sdlc/changes/2026-09-28-orchestration-stability +--- + +# Plan: Orchestration Stability + +## Plan + +Codex owns the listed paths. Add regression cases before fixes; remove fabricated catalogues, improve the existing projection and continuation consumption, then reconcile picker state. Run focused Core integration/unit tests, workspace check for shared exports, desktop affected tests/types/lint/build, actual browser rendering, docs and SDLC worktree checks. No database schema, external protocol or release changes. + +Continuation verification: use the opt-in canary with locally installed, authenticated providers, leave model choice to runtime discovery/defaults, and exercise a synthetic 64 Ki-character history across repeated switches. Resolve requested provider ids through the registry; always shut down the test-owned engine even if the canary fails. Record account/adapter failures separately from successful routes. The live probe exposed different Codex binaries for catalogue discovery and ACP execution; align both with the explicit override or discovered installed CLI and retest, without changing model ids or adapter versions. Follow-up verification covers additional locally installed agents in independent in-memory sessions, first via bidirectional routes with a verified provider, then via a direct chain among available agents; unavailable accounts remain explicit gaps. Repeated real switching exposed ACP wrapper descendants surviving direct-child termination. Add a failing process/stdio regression, reuse the existing Unix process-group signalling helper at crate scope, and isolate ACP process groups so termination and drop stop their ordinary descendants without affecting unrelated processes. Recheck plugin teardown, Core regressions, and the full live directed-pair route. Windows process trees remain outside the Unix guarantee. + +Temporary resources: task-owned `.codex/run/orchestration-stability/` for preview evidence; this fresh worktree created `target/`, `apps/desktop/node_modules/` and `apps/desktop/dist/`. Remove dedicated compiled outputs after checks; retain installed dependencies until this worktree is retired. Stop the task's renderer process and remove disposable evidence at handoff. No user desktop restart. + +Rollback: revert only this change's scoped edits; there is no migration. diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md new file mode 100644 index 00000000..9c6cf66e --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md @@ -0,0 +1,29 @@ +--- +id: 2026-09-28-orchestration-stability +schema: 5 +stage: spec +status: accepted +owner: codex +created: 2026-09-28 +based_on: intent.md +--- + +# Spec: Orchestration Stability + +## Design + +Retain the existing switch transaction and callback fence. Canonical transcript remains the history source; reserve bounded initial/latest user context alongside recent neutral records. Cancelled or failed first prompts retain pending continuation, including native restore. Do not transfer reasoning or raw tool data. + +Discover models from installed CLI APIs or ACP session metadata; remove static model aliases, provider/model-name effort rewrites and permanent process caches. ACP metadata owns the actual selectable values. Codex catalogue discovery and ACP execution must share the selected executable; explicit executable overrides fail honestly rather than falling through to a different runtime. Empty discovery is honest and retryable. Renderer providers come only from the host registry. Built-in launch integrations remain registered by Core; registration is not evidence of installation or models. + +Show switching explicitly, fence duplicate local requests synchronously, and ignore provider metadata in transcript reducers. Preserve the old provider on failed switching. Unix ACP launches own isolated process groups; switching, shutdown and client drop terminate ordinary wrapper descendants and release pending RPCs. Reuse the plugin process-group signalling boundary; never signal an inherited or unrelated group. + +Targeted corrections cost less to implement, validate and maintain than replacing the engine: ownership, storage commit and callback isolation already have contracts/tests. Module-level replacement of the fabricated catalogue is warranted; an engine rewrite would add migration/runtime risks without improving these boundaries. + +## Acceptance criteria + +- [x] AC-1: Long and Unicode histories retain initial/latest user context within a bounded neutral payload, and repeated switches use canonical history without nested handoffs. +- [x] AC-2: Failed/cancelled continuation and provider startup, busy and competing switches preserve recoverable state, including retry after native restore. +- [x] AC-3: Model discovery has no static fallback catalogue or permanent cache; unknown providers/models are never fabricated by the renderer. +- [x] AC-4: Provider metadata cannot crash transcript projection; switching has visible pending state and duplicate actions are fenced. Actual renderer checks cover the affected controls. +- [x] AC-5: Relevant Rust, desktop, documentation and worktree lifecycle checks pass; cleanup and unchecked live-provider boundaries are recorded. diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md new file mode 100644 index 00000000..33802fd4 --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md @@ -0,0 +1,79 @@ +--- +id: 2026-09-28-orchestration-stability +schema: 5 +stage: verification +status: passed +owner: codex +created: 2026-09-28 +based_on: plan.md +revision: "Worktree based on 6ed3561f, including this change's scoped implementation and regressions" +verification_mode: owner +verified_by: codex +verified_at: 2026-09-28 +release_target: none +cleanup_status: complete +--- + +# Verification: Orchestration Stability + +## Verification + +- AC-1: PASS — `cargo test -p codetwo-core --lib` passed 550 tests. Context regressions cover full short requests, separate consecutive user prompts, oversized Unicode output, hundreds of small records, initial/latest user anchors, record/content bounds and private-state exclusion. `cargo test -p codetwo-core --test engine_provider_switch` passed nine cases, including four rounds of A/B/A/B switching without intermediate prompts or nested continuation. +- AC-2: PASS — `cargo test -p codetwo-core --lib --test engine_provider_switch --test engine_builtin_models --test acp_models --test engine_permission --test engine_store --test acp_process_cleanup --quiet` passed 574 tests after the live-discovered runtime and process cleanup fixes. Cases include failed startup, busy/awaiting approval, concurrent switches, managed leases, missing durable history, first-prompt failure/retry, cancelled first prompt followed by engine restart/native restore, stale callback suppression, and dropping initialization with candidate-client cleanup. The ordinary suite excludes the opt-in authenticated canary; its separate live result is recorded below. +- AC-3: PASS — The same Core suite checks fresh catalogue queries, configured environment, failure recovery, paginated model discovery and cursor-cycle rejection, no fabricated catalogue, ACP model/effort choices, and runtime catalogue projection. `cargo test -p codetwo-server --lib t3_compat --quiet` passed 12 tests, including registration of an unknown custom integration without invented models. `rg -n 'builtin_models|FALLBACK_PROVIDERS|fallbackProviders' crates apps/desktop/src` returned no matches. Price-estimation tables and test/demo data are not model-selection catalogues and remain outside this change. +- AC-4: PASS — `bun test tests/reasoningScaleRendered.test.tsx tests/sceneChip.test.tsx tests/sessionState.test.ts tests/providerRegistry.test.ts` in `apps/desktop` passed 69 tests / 218 expectations. Actual Vite-rendered production ModelPicker components were inspected through the in-app browser in light mode and dark 390px mode: disabled switching status, empty discovery with Retry, and a custom discovered model menu. Browser errors were empty. The App mutation path also has a synchronous per-session request fence; focused/background panes receive their own switching state. +- AC-5: PASS — `cargo check --workspace --all-targets --quiet`, `bun run build:renderer` (lint, TypeScript, Vite), `bun script/verify/docs.ts`, `bun script/verify/sdlc.ts --worktree`, and `git diff --check` passed. Local environment: macOS, rustc 1.95.0, Bun 1.4.2. Existing full-tree `cargo fmt --all --check` reports unrelated baseline drift; changed Rust hunks were formatted without reformatting unrelated code. Vite retains its existing large-chunk advisory. + +Verdict: verified. +Residual risk: Cursor live switching is unverified because its installed CLI returned Authentication required; no login or account configuration was changed. The additional provider matrix below separates successful routes from unavailable accounts/adapters. Full Electrobun integration, remote CI and release were not exercised. ACP metadata remains provider-owned; inaccurate adapter declarations must be fixed at their source. Long-history continuation is deliberately bounded to 48 Ki characters and 128 records, with omission markers; canonical stored history is retained. These tests do not claim lossless transfer of every historical detail or acceptance of every provider/account. + +## Authenticated provider verification + +PASS — `CODETWO_LIVE_SWITCH_PROVIDERS=codex,grok,codex,grok,codex CODETWO_LIVE_SWITCH_CONTEXT_CHARS=65536 CODETWO_LIVE_SWITCH_CWD="$PWD/.codex/run/orchestration-stability/live-cwd" cargo test -p codetwo-core --test engine_provider_switch live_providers_switch_in_place_and_back -- --ignored --nocapture` completed in 44.95 seconds. All five real turns recovered the generated continuity key, including four switches within the same durable Session. The initial synthetic history exceeds the 48 Ki-character transfer budget. Later prompts do not repeat the key. No model override was provided. Test data used an in-memory Store and a task-owned empty working directory; permission and elicitation requests are declined. + +The first probe failed before switching: the ACP adapter bundled Codex 0.148.0 while model discovery found local Codex 0.157.1, so the configured model was rejected by the older runtime. The fix pins ACP and catalogue discovery to the same explicit or discovered installed executable, without changing the adapter version or selecting a fixed model. A subprocess-isolated regression verifies explicit runtime forwarding; the repeated live route confirms the installed-runtime fallback. The canary now resolves requested ids against the registry, requires an explicit provider sequence, supports bounded synthetic long context, and shuts down its Engine even on assertion failure. + +A separate `grok,cursor,grok,cursor,grok` probe passed Grok's first long-context turn, then Cursor returned an authentication-required error. This is recorded as an unavailable live route, not a switching pass. Failed-run evidence is retained alongside the successful run; test-owned provider processes were shut down after each attempt. + +## Additional agent coverage + +The same opt-in canary was run with 65,536 synthetic context characters, runtime-owned default models, an in-memory Store and a separate temporary working directory per run. Executables were confirmed locally before running. No login, subscription or account configuration was changed. + +| Route | Result | Evidence | +| --- | --- | --- | +| Claude Code → Grok → Claude Code | PASS, three recalled keys, 48.40 s | `live-matrix-claude_code.log` | +| OpenCode → Grok → OpenCode | PASS, three recalled keys, 21.48 s | `live-matrix-opencode.log` | +| Amp → Grok → Amp | PASS, three recalled keys, 43.64 s | `live-matrix-amp.log` | +| Kimi → Grok → Kimi | BLOCKED before switching: account returned HTTP 403, subscription has no Kimi Code access | `live-matrix-kimi.log` | +| ZCode (GLM) → Grok → ZCode | BLOCKED before switching: no API key environment or stored credential file was configured; adapter 1.12.0 returned RPC -32603 Internal error, so there is no live continuation result | `live-matrix-zcode.log` | +| Pi → Grok → Pi | BLOCKED before switching: API key/OAuth authentication required | `live-matrix-pi.log` | +| Droid → Grok → Droid | BLOCKED before switching: authentication required | `live-matrix-droid.log` | +| OpenCode 2 → Grok → OpenCode 2 | Initial OpenCode 2 and Grok turns passed; return failed with provider authentication required. A sequential retry failed with the same error on the first turn, before any switch. Full route remains unverified. | `live-matrix-opencode2.log`, `live-matrix-opencode2-retry.log` | + +The direct-pair follow-up derives its candidate set from completed successful probes and constructs a route covering every directed pair once. This is a verification fixture, not a product provider/model catalogue. + +PASS after the process cleanup fix — Amp, Claude Code, Codex, Grok and OpenCode completed every one of their 20 directed switching pairs in one durable Session, with 21 successful continuity-key responses and 65,536 initial synthetic context characters. The complete run took 198.48 seconds. The route was `amp,opencode,grok,opencode,codex,opencode,claude_code,opencode,amp,grok,codex,grok,claude_code,grok,amp,codex,claude_code,codex,amp,claude_code,amp`. No model override was provided. Evidence: `live-direct-pairs.log`, `live-direct-pairs-results.json`; the pre-fix route's successful context result is retained as `live-direct-pairs-before-cleanup-fix.log`. Intermediate post-fix process snapshots showed only current/candidate adapter groups, and the final snapshot showed no remaining test-owned provider processes. + +## Provider process ownership regression + +The first 20-direction real route completed all 21 turns, but intermediate process snapshots showed orphaned npx adapter descendants persisting while the Engine continued running. The new `acp_process_cleanup` regression reproduced this on the old implementation: killing only the wrapper left a descendant holding stdio and pending RPCs open. + +ACP launches now create isolated Unix process groups and terminate the owned group on explicit shutdown or client drop. The existing plugin process-group signal helper moved to crate scope and is shared; plugin lifecycle semantics are unchanged. Arbitrary children supplied through the public client constructor retain direct-child ownership. The regression verifies explicit termination, repeated termination, drop, an already-exiting wrapper, closed pending RPCs and survival of an unrelated process. It failed before the fix and passed afterward (three lifecycle cases in one test). Evidence: `process-cleanup-before.log`, `process-cleanup-after.log`, the 574-test Core rerun and `live-process-samples.log`. + +Unix process groups cover ordinary descendants; descendants that deliberately create a new process group and Windows process-tree ownership are not claimed. Windows retains direct-child cleanup. + +## Cleanup + +Removed: Task-created `target/` (2.8 GiB at the first handoff; 2.3 GiB rebuilt and removed after each authenticated verification continuation), the empty live-test working directory, per-probe temporary working directories, one-off matrix runner scripts, `apps/desktop/dist/` (48 MiB), and temporary browser harness HTML/TSX under `apps/desktop/.codex/run/orchestration-stability/`. The temporary browser tab was closed and its viewport override reset. +Retained: `.codex/run/orchestration-stability/` contains small test logs, successful/failed authenticated canary evidence, and light/dark rendered evidence; `apps/desktop/node_modules/` (1.7 GiB) contains installed development dependencies. +Retention owner: Codex for this change and checkout. +Cleanup trigger: Review retained evidence on the next continuation and remove disposable logs/captures after this change is reviewed or closed; remove the installed dependencies when this worktree is retired. Provider-managed synthetic session histories and shared package caches are left to their normal lifecycle; no account history is deleted by this task. +Processes: Successful/failed canary engines shut down their provider processes; process inspection after the final run showed no task-owned provider remaining. Task-owned Vite launcher stopped; port 1437 is released. No task-owned mock provider, test, build or desktop process remains. User processes were not stopped. +Evidence: `du -sh target apps/desktop/node_modules apps/desktop/dist .codex/run/orchestration-stability` before removal; exact-path inspection/removal and post-removal inventory; `ps -Ao pid,command` and `lsof -nP -iTCP:1437 -sTCP:LISTEN`. Retained logs are `core-tests.log`, `desktop-tests.log`, `workspace-check.log`, `switch-regression-tests.log`, `live-provider-tests.log`, `live-grok-cursor-tests.log`, and `live-codex-grok-tests.log`, `live-matrix-*.log`, matrix/direct-pair result JSON, `live-direct-pairs*.log`, `process-cleanup-before.log`, `process-cleanup-after.log`, `live-process-samples.log`, and `live-final-processes.log`; render captures are `light.png` and `dark-narrow.png` in the evidence directory. + +## Review and release + +Approval: local implementation authorized in Intent. The user explicitly authorized PR creation and merge with "pr & merge" on 2026-09-28. Human review and remote checks are separate facts. +Rollback: Revert this scoped change; no schema migration. +Release: PR delivery and merge authorized; remote CI and merge outcome pending. No versioned release or production deployment requested. +Feedback: Rerun the Cursor route after its account is authenticated. Use the opt-in canary for future provider regressions; keep model selection owned by runtime discovery. diff --git a/docs/sdlc/changes/2026-09-28-remove-plan-goal/intent.md b/docs/sdlc/changes/2026-09-28-remove-plan-goal/intent.md new file mode 100644 index 00000000..197ceab2 --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-remove-plan-goal/intent.md @@ -0,0 +1,19 @@ +--- +id: 2026-09-28-remove-plan-goal +schema: 5 +stage: intent +status: accepted +owner: codex +created: 2026-09-28 +source: user +risk: medium +approved_by: user +approved_at: 2026-09-28 +approval_source: "Current user request: 移除 plan 和 goal 的支持" +--- + +# Intent: Remove Plan Goal + +## Intent + +Remove product-level agent Plan mode, plan checklist presentation and Goal controls through their UI, bridge and runtime paths. Preserve ordinary prompts, steering, model/effort selection, task objective text, existing historical records and the prior orchestration stability work. Repository development stage documents are unrelated to these product features. No release, credential or data deletion is requested. diff --git a/docs/sdlc/changes/2026-09-28-remove-plan-goal/plan.md b/docs/sdlc/changes/2026-09-28-remove-plan-goal/plan.md new file mode 100644 index 00000000..913fc2c0 --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-remove-plan-goal/plan.md @@ -0,0 +1,20 @@ +--- +id: 2026-09-28-remove-plan-goal +schema: 5 +stage: plan +status: accepted +owner: codex +created: 2026-09-28 +based_on: spec.md +scope: crates/core/src/skill.rs, crates/core/src/plugins/app/service.rs, crates/server/tests/t3_mobile_compat.rs, apps/desktop/src-host/src/scene_mcp.rs, crates/core/schemas/agent-scenes, docs/reference/scenes.md, apps/desktop/src/App.tsx, apps/desktop/src/bridge.ts, apps/desktop/src/session, apps/desktop/src/environment/EnvironmentPopover.tsx, apps/desktop/src/i18n/strings.ts, apps/desktop/tests, crates/core/src/acp, crates/core/src/engine.rs, crates/core/src/event.rs, crates/core/src/session.rs, crates/core/src/memory.rs, crates/core/src/store.rs, crates/core/src/scene.rs, crates/core/src/plugins/app/plugins/engine.rs, crates/core/src/plugins/app/plugins/scene_commands.rs, crates/core/examples/live_demo.rs, crates/core/tests, crates/server/src/t3_compat.rs, docs/sdlc/changes/2026-09-28-remove-plan-goal +--- + +# Plan: Remove Plan Goal + +## Plan + +Codex owns this removal in the listed files, preserving the earlier uncommitted stability changes. Remove native Goal routes first; then remove Plan presentation, selectors and scene/draft activation. Keep only read-only historical decoding where required for data compatibility. Add regressions for ignored metadata, retired command/config rejection and legacy record loading. Run affected Core/integration tests, desktop tests, renderer types/lint/build, actual browser rendering, workspace check and documentation/SDLC checks. + +Temporary resources: task-owned target/, apps/desktop/dist/, and .codex/run/remove-plan-goal/ for logs/preview. Stop preview processes and remove build outputs/scratch harness at handoff; retain small verification evidence until review. Reuse existing node_modules until worktree retirement. Do not restart a user desktop or delete provider/account data. + +Rollback: revert only this feature-removal diff; no data migration. diff --git a/docs/sdlc/changes/2026-09-28-remove-plan-goal/spec.md b/docs/sdlc/changes/2026-09-28-remove-plan-goal/spec.md new file mode 100644 index 00000000..18f32c04 --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-remove-plan-goal/spec.md @@ -0,0 +1,24 @@ +--- +id: 2026-09-28-remove-plan-goal +schema: 5 +stage: spec +status: accepted +owner: codex +created: 2026-09-28 +based_on: intent.md +--- + +# Spec: Remove Plan Goal + +## Design + +Remove native Goal discovery/control/snapshots and Plan mode selection, scene plan-first activation and plan checklist surfaces. Provider-advertised planning controls must not reappear through generic configuration. Remove the built-in Plan-first fragment and mobile compatibility mode/prompt injection as well. The mobile contract exposes only its default interaction mode and rejects retired mode requests. Incoming plan updates are ignored without breaking ordinary text/tool streams. Historical stored plan rows remain readable as read-only legacy data, without active projection or new plan writes. Existing drafts/scenes with removed keys continue loading while those keys no longer affect execution. Keep a single effective path for supported session behavior; remove unused helper code and tests rather than adding a second feature toggle. + +A scoped removal is cheaper and safer than replacing the session engine or rewriting persistence. No database migration is needed. Ordinary task objective text and scene document artifacts are not native Goal/Plan control features. + +## Acceptance criteria + +- [x] AC-1: No Plan/Goal controls, plan checklist panels or corresponding local draft state remain in the rendered workspace. +- [x] AC-2: Native Goal commands and capability/snapshot routes are removed; Plan updates and planning config cannot activate removed behavior. Text, tools, steering and model selection continue working. +- [x] AC-3: Existing stored plan history and old drafts/scenes load safely without enabling removed features. +- [x] AC-4: Relevant Rust/frontend regressions, workspace types/build, actual rendering, documentation and lifecycle checks pass; task resources are cleaned up. diff --git a/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md b/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md new file mode 100644 index 00000000..e465977a --- /dev/null +++ b/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md @@ -0,0 +1,45 @@ +--- +id: 2026-09-28-remove-plan-goal +schema: 5 +stage: verification +status: passed +owner: codex +created: 2026-09-28 +based_on: plan.md +revision: "Worktree based on 6ed3561f plus prior orchestration stability changes and this removal" +verification_mode: owner +verified_by: codex +verified_at: 2026-09-28 +release_target: none +cleanup_status: complete +--- + +# Verification: Remove Plan Goal + +## Verification + +- AC-1: PASS — Actual Vite-rendered workspace inspected through the in-app browser at 1280 × 720. Expanded session settings retain permissions/memory; the environment panel retains project/Git controls. Neither exposes Plan/Goal. Screenshot: `.codex/run/remove-plan-goal/workspace.png`. Composer, environment plan panel, scene Plan-first control, draft state, translations and plan-document actions are removed. +- AC-2: PASS — `cargo test -p codetwo-core --lib --test engine_activity --test scene_conformance --quiet` passed 552 unit, 10 activity and four scene-conformance tests. Regressions cover ignored Plan notifications/Goal metadata, removed planning selectors, rejected direct planning config, supported config changes and ordinary lifecycle behavior. The native Goal command and builtin Plan-first fragment are removed. `cargo test -p codetwo-server --lib t3_compat --quiet` passed 13 tests; `cargo test -p codetwo-server --test t3_mobile_compat --quiet` passed two HTTP/WebSocket integration tests, including rejected Plan mode, no prompt injection, long ordinary messages, busy-turn rejection and restart recovery. +- AC-3: PASS — Historical plan decoding remains read-only; plans are omitted from renderer turns, context handoff, memory answer extraction and T3 activity projection. Legacy draft and scene keys are ignored and not re-exported; old mobile interactionModes metadata no longer activates a mode. `bun test tests/retiredSessionFeatures.test.ts tests/composerDrafts.test.ts tests/scene.test.ts tests/reasoningScaleRendered.test.tsx tests/sceneChip.test.tsx tests/checkoutPickerRendered.test.tsx tests/sessionState.test.ts` passed 86 tests / 283 expectations. Scene editor tests passed three tests / 21 expectations; the final scene chip, studio and issue-delegation tests passed 22 tests / 83 expectations. Core stored-history/scene and mobile restart regressions also passed. No persisted user data was deleted. +- AC-4: PASS — `cargo check --workspace --all-targets`, `bun run build:renderer` (lint, TypeScript, Vite), final `bun run lint`, `bun script/verify/docs.ts`, `bun script/verify/sdlc.ts --worktree`, and `git diff --check` passed. Earlier in this change, the Core/library/provider-switch/model/activity/store/process suite passed 578 tests, with the opt-in real-provider canary ignored. Source inspection covered remaining active adapters, scene schema/examples and the legacy desktop host. Cleanup completed below. + +Verdict: verified. +Residual risk: UI evidence is the browser renderer, not a native desktop/account run. Its browser-only bridge reports an empty provider registry; it does not prove live provider discovery. This removal did not rerun the earlier real-provider matrix, remote CI, packaging or release acceptance. Vite retains its existing large-chunk warning. Old planning configuration is accepted only for legacy data reading; new control requests fail explicitly. + +Evidence: logs and screenshot retained under `.codex/run/remove-plan-goal/`. An unused mobile update helper exposed by compiler warnings was removed; the final workspace check is clean. Repository-wide Rust formatting has pre-existing drift; changed Rust hunks were formatted without rewriting unrelated code. + +## Cleanup + +Removed: task-owned `target/` (3.7 GiB), `apps/desktop/dist/` (48 MiB), superseded type/test logs. +Retained: compact `.codex/run/remove-plan-goal/` evidence, reused `apps/desktop/node_modules`, prior `.codex/run/orchestration-stability/` evidence and all source changes. +Retention owner: Codex for these verification artifacts; prior stability evidence retains its existing owner. +Cleanup trigger: review completion or worktree retirement; reused dependencies at worktree retirement. +Processes: own Vite preview stopped through its launcher; TCP 1437 released and browser test tab closed. Test processes exited. No user desktop or account process was stopped. +Evidence: `.codex/run/remove-plan-goal/cleanup.txt`; exact-path `du -sh` before removal, path/ownership checks, post-removal existence checks and `lsof -nP -iTCP:1437 -sTCP:LISTEN`. + +## Review and release + +Approval: local implementation authorized in Intent. The user explicitly authorized PR creation and merge with "pr & merge" on 2026-09-28. Human review and remote checks are separate facts. +Rollback: revert only this feature-removal diff; no data migration. +Release: PR delivery and merge authorized; remote CI and merge outcome pending. No versioned release or production deployment requested. +Feedback: link a concrete failure to its regression if follow-up runtime evidence changes this result. From e384ebbce59ff76b4720f38416a2b7dbeff9805c Mon Sep 17 00:00:00 2001 From: idevlab Date: Mon, 28 Sep 2026 13:57:34 +0800 Subject: [PATCH 2/3] Update remaining transcript and environment tests for retired plans --- .../tests/environmentPopoverRendered.test.tsx | 63 +------------------ apps/desktop/tests/transcript.test.ts | 13 ++-- .../verification.md | 4 ++ 3 files changed, 10 insertions(+), 70 deletions(-) diff --git a/apps/desktop/tests/environmentPopoverRendered.test.tsx b/apps/desktop/tests/environmentPopoverRendered.test.tsx index 8b509473..e58f2bb7 100644 --- a/apps/desktop/tests/environmentPopoverRendered.test.tsx +++ b/apps/desktop/tests/environmentPopoverRendered.test.tsx @@ -37,7 +37,6 @@ function renderEnvironment(onRefresh = () => {}, preview = null, props = {}) { onAddProject={() => {}} onOpenSourceControl={() => {}} onOpenSettings={() => {}} - turns={[]} preview={preview} {...props} /> @@ -118,6 +117,8 @@ describe("EnvironmentPopover layout", () => { expect(content?.textContent).toContain("Changes"); expect(content?.textContent).toContain("Local"); expect(content?.textContent).toContain("Commit or push"); + expect(content?.querySelector("[data-task-plan-panel]")).toBeNull(); + expect(content?.textContent).not.toContain("Current tasks"); expect(content?.textContent).not.toContain("Checkpoint now"); expect(content?.textContent).not.toContain("Copy path"); expect(content?.textContent).not.toContain("Tools"); @@ -191,64 +192,4 @@ describe("EnvironmentPopover layout", () => { expect(preview?.textContent).toContain("Open example.com"); view.unmount(); }); - - test("shows the current task list inside the environment popover", async () => { - activateDom(); - const turn = { - id: 1, - accepted: true, - streamBoundaryKnown: true, - prompt: "Implement task progress", - text: "", - textDeltas: [], - observedTextDeltas: 0, - observedThoughtDeltas: 0, - pendingTextDeltaSkips: 0, - pendingThoughtDeltaSkips: 0, - thoughts: [], - tools: [], - plan: [ - { content: "Inspect the event path", status: "completed" }, - { content: "Add tasks to quick info", status: "in_progress" }, - { content: "Run renderer checks", status: "pending" }, - ], - startedAt: 1, - }; - const view = renderEnvironment(() => {}, null, { turns: [turn] }); - - const trigger = view.container.querySelector( - '[aria-label="Project environment"]' - ); - await reactAct(async () => { - trigger?.dispatchEvent( - new dom.window.PointerEvent("pointerdown", { - bubbles: true, - cancelable: true, - button: 0, - pointerId: 4, - }) - ); - trigger?.dispatchEvent( - new dom.window.MouseEvent("click", { bubbles: true, cancelable: true }) - ); - }); - await flush(); - - const content = dom.document.body.querySelector( - '[data-slot="popover-content"]' - ); - const tasks = content?.querySelector("[data-task-plan-panel]"); - expect(tasks).toBeTruthy(); - expect(tasks?.textContent).toContain("Current tasks"); - expect(tasks?.textContent).toContain("Step 2 / 3"); - expect(tasks?.querySelectorAll("[data-task-plan-status]")).toHaveLength(3); - expect( - tasks?.querySelector('[data-task-plan-status="in_progress"]')?.textContent - ).toContain("Add tasks to quick info"); - expect( - view.container.querySelector('[data-dock-placement="right"]') - ).toBeNull(); - - view.unmount(); - }); }); diff --git a/apps/desktop/tests/transcript.test.ts b/apps/desktop/tests/transcript.test.ts index 6fa9d777..27813663 100644 --- a/apps/desktop/tests/transcript.test.ts +++ b/apps/desktop/tests/transcript.test.ts @@ -15,7 +15,7 @@ import { } from "../src/session/turns"; describe("persisted transcript projection", () => { - test("preserves structured plan status and accepts legacy string entries", () => { + test("ignores retired structured and string plan entries without losing the prompt", () => { const turns = turnsFromTranscript([ ["user", { kind: "prompt", text: "implement", display: "implement" }], [ @@ -34,14 +34,9 @@ describe("persisted transcript projection", () => { ], ]); - expect(turns[0].plan).toEqual([ - { content: "Inspect the workspace", priority: null, status: null }, - { - content: "Implement the panel", - priority: "high", - status: "in_progress", - }, - ]); + expect(turns).toHaveLength(1); + expect(turns[0].prompt).toBe("implement"); + expect(turns[0]).not.toHaveProperty("plan"); }); test("shows the canonical prompt while preserving the agent response", () => { diff --git a/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md b/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md index e465977a..5de89427 100644 --- a/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md +++ b/docs/sdlc/changes/2026-09-28-remove-plan-goal/verification.md @@ -28,6 +28,10 @@ Residual risk: UI evidence is the browser renderer, not a native desktop/account Evidence: logs and screenshot retained under `.codex/run/remove-plan-goal/`. An unused mobile update helper exposed by compiler warnings was removed; the final workspace check is clean. Repository-wide Rust formatting has pre-existing drift; changed Rust hunks were formatted without rewriting unrelated code. +## PR validation follow-up + +PR #242's first CI run failed two desktop tests that still expected retired Plan output (`environmentPopoverRendered` and `transcript`). Updated those expectations to assert absence of the plan panel/property while preserving ordinary environment controls and legacy prompts. No runtime behavior changed in this follow-up. Full `bun run test:ci` then passed 966 tests / 5,776 expectations; three native/profile opt-in tests were skipped by the suite. Evidence: `.codex/run/remove-plan-goal/full-desktop-tests.log`. The initial remote failure is [CI run 36383859967](https://github.com/IchenDEV/codeTwo/actions/runs/36383859967); replacement remote CI remains required before merge. + ## Cleanup Removed: task-owned `target/` (3.7 GiB), `apps/desktop/dist/` (48 MiB), superseded type/test logs. From d8ccdefd4b4b386c35a706d14f4615ed3a115f56 Mon Sep 17 00:00:00 2001 From: idevlab Date: Mon, 28 Sep 2026 14:11:20 +0800 Subject: [PATCH 3/3] Reject recycled Unix worktree identities using filesystem birth time --- crates/core/src/engine.rs | 2 +- crates/core/src/store.rs | 1 + crates/core/src/worktree.rs | 64 ++++++++++++++++++- .../plan.md | 6 +- .../spec.md | 2 + .../verification.md | 8 ++- 6 files changed, 78 insertions(+), 5 deletions(-) diff --git a/crates/core/src/engine.rs b/crates/core/src/engine.rs index c1847e6d..46d1b885 100644 --- a/crates/core/src/engine.rs +++ b/crates/core/src/engine.rs @@ -8037,7 +8037,7 @@ for line in sys.stdin: let error = validate_session_checkout(&session).await.unwrap_err(); assert!( - error.contains("path identity changed"), + error.contains("path identity changed") || error.contains("repository changed"), "unexpected error: {error}" ); diff --git a/crates/core/src/store.rs b/crates/core/src/store.rs index d71cd549..bde52989 100644 --- a/crates/core/src/store.rs +++ b/crates/core/src/store.rs @@ -4926,6 +4926,7 @@ mod tests { session.worktree_identity = Some(DirectoryIdentity::Unix { device: 42, inode: 108, + birth_time_ns: Some(123_456), }); session.worktree_common_dir = Some("/source/repo/.git".into()); session.worktree_git_dir = Some("/source/repo/.git/worktrees/isolated".into()); diff --git a/crates/core/src/worktree.rs b/crates/core/src/worktree.rs index 49ce6568..7a918d2d 100644 --- a/crates/core/src/worktree.rs +++ b/crates/core/src/worktree.rs @@ -460,6 +460,10 @@ pub enum DirectoryIdentity { Unix { device: u64, inode: u64, + /// Distinguishes recycled inode numbers when the filesystem exposes a birth time. + /// Older records and filesystems without birth times retain device/inode validation. + #[serde(default, skip_serializing_if = "Option::is_none")] + birth_time_ns: Option, }, Windows { volume_serial_number: u32, @@ -487,6 +491,11 @@ impl DirectoryIdentity { Ok(Self::Unix { device: metadata.dev(), inode: metadata.ino(), + birth_time_ns: metadata + .created() + .ok() + .and_then(|created| created.duration_since(std::time::UNIX_EPOCH).ok()) + .and_then(|elapsed| u64::try_from(elapsed.as_nanos()).ok()), }) } #[cfg(windows)] @@ -510,7 +519,25 @@ impl DirectoryIdentity { Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(false), Err(error) => return Err(error), }; - Ok(*self == actual) + Ok(match (self, &actual) { + ( + Self::Unix { + device, + inode, + birth_time_ns, + }, + Self::Unix { + device: actual_device, + inode: actual_inode, + birth_time_ns: actual_birth, + }, + ) => { + device == actual_device + && inode == actual_inode + && birth_time_ns.map_or(true, |expected| *actual_birth == Some(expected)) + } + _ => *self == actual, + }) } } @@ -1562,11 +1589,46 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn directory_identity_rejects_recycled_inode_birth_time_and_reads_legacy_receipts() { + let directory = tempfile::tempdir().unwrap(); + let identity = DirectoryIdentity::capture(directory.path()).unwrap(); + let DirectoryIdentity::Unix { + device, + inode, + birth_time_ns, + } = identity + else { + unreachable!(); + }; + // Deterministically model inode reuse: same device/inode, different creation time. + let recycled = DirectoryIdentity::Unix { + device, + inode, + birth_time_ns: Some(birth_time_ns.unwrap_or(0).saturating_add(1)), + }; + assert!(!recycled.matches_path(directory.path()).unwrap()); + let legacy: DirectoryIdentity = serde_json::from_value(serde_json::json!({ + "kind": "unix", "device": device, "inode": inode + })) + .unwrap(); + assert!(legacy.matches_path(directory.path()).unwrap()); + + // Normal content changes must not invalidate a live session's directory receipt. + std::fs::write(directory.path().join("file"), "changed").unwrap(); + assert!(identity.matches_path(directory.path()).unwrap()); + let restored: DirectoryIdentity = + serde_json::from_value(serde_json::to_value(&identity).unwrap()).unwrap(); + assert_eq!(restored, identity); + } + #[test] fn directory_identity_has_stable_wire_shape() { let unix = DirectoryIdentity::Unix { device: 7, inode: 11, + birth_time_ns: None, }; assert_eq!( serde_json::to_value(unix).unwrap(), diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md index 5131f12f..c6a4e1b1 100644 --- a/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/plan.md @@ -6,17 +6,19 @@ status: accepted owner: codex created: 2026-09-28 based_on: spec.md -scope: crates/core/src/acp/client.rs, crates/core/src/acp/mod.rs, crates/core/src/unix_process_group.rs, crates/core/src/plugins/app/protocol/mod.rs, crates/core/tests/acp_process_cleanup.rs, crates/core/src/codex_runtime.rs, crates/core/src/models.rs, crates/core/src/engine.rs, crates/core/src/lib.rs, crates/core/src/acp/wire.rs, crates/core/src/event.rs, crates/core/src/cost.rs, crates/core/tests/engine_builtin_models.rs, crates/core/tests/engine_provider_switch.rs, crates/core/tests/engine_store.rs, crates/core/tests/engine_permission.rs, crates/server/src/t3_compat.rs, apps/desktop/src/App.tsx, apps/desktop/src/bridge.ts, apps/desktop/src/session/Composer.tsx, apps/desktop/src/session/config.ts, apps/desktop/src/session/turns.ts, apps/desktop/src/providers/registry.ts, apps/desktop/src/i18n/strings.ts, apps/desktop/tests, docs/sdlc/changes/2026-09-28-orchestration-stability +scope: crates/core/src/worktree.rs, crates/core/src/store.rs, crates/core/src/acp/client.rs, crates/core/src/acp/mod.rs, crates/core/src/unix_process_group.rs, crates/core/src/plugins/app/protocol/mod.rs, crates/core/tests/acp_process_cleanup.rs, crates/core/src/codex_runtime.rs, crates/core/src/models.rs, crates/core/src/engine.rs, crates/core/src/lib.rs, crates/core/src/acp/wire.rs, crates/core/src/event.rs, crates/core/src/cost.rs, crates/core/tests/engine_builtin_models.rs, crates/core/tests/engine_provider_switch.rs, crates/core/tests/engine_store.rs, crates/core/tests/engine_permission.rs, crates/server/src/t3_compat.rs, apps/desktop/src/App.tsx, apps/desktop/src/bridge.ts, apps/desktop/src/session/Composer.tsx, apps/desktop/src/session/config.ts, apps/desktop/src/session/turns.ts, apps/desktop/src/providers/registry.ts, apps/desktop/src/i18n/strings.ts, apps/desktop/tests, docs/sdlc/changes/2026-09-28-orchestration-stability --- # Plan: Orchestration Stability ## Plan -Codex owns the listed paths. Add regression cases before fixes; remove fabricated catalogues, improve the existing projection and continuation consumption, then reconcile picker state. Run focused Core integration/unit tests, workspace check for shared exports, desktop affected tests/types/lint/build, actual browser rendering, docs and SDLC worktree checks. No database schema, external protocol or release changes. +Codex owns the listed paths. Add regression cases before fixes; remove fabricated catalogues, improve the existing projection and continuation consumption, then reconcile picker state. Run focused Core integration/unit tests, workspace check for shared exports, desktop affected tests/types/lint/build, actual browser rendering, docs and SDLC worktree checks. No database schema or release changes. Persisted Unix worktree identity gains an optional birth timestamp; old records remain readable. Continuation verification: use the opt-in canary with locally installed, authenticated providers, leave model choice to runtime discovery/defaults, and exercise a synthetic 64 Ki-character history across repeated switches. Resolve requested provider ids through the registry; always shut down the test-owned engine even if the canary fails. Record account/adapter failures separately from successful routes. The live probe exposed different Codex binaries for catalogue discovery and ACP execution; align both with the explicit override or discovered installed CLI and retest, without changing model ids or adapter versions. Follow-up verification covers additional locally installed agents in independent in-memory sessions, first via bidirectional routes with a verified provider, then via a direct chain among available agents; unavailable accounts remain explicit gaps. Repeated real switching exposed ACP wrapper descendants surviving direct-child termination. Add a failing process/stdio regression, reuse the existing Unix process-group signalling helper at crate scope, and isolate ACP process groups so termination and drop stop their ordinary descendants without affecting unrelated processes. Recheck plugin teardown, Core regressions, and the full live directed-pair route. Windows process trees remain outside the Unix guarantee. +CI follow-up: Linux reused an inode and Git admin path after worktree removal, exposing a false ownership match in the existing worktree regression. Include filesystem birth time when available in the existing identity receipt, retain legacy comparison for old receipts, and test recycled identity, content changes and serialization. Do not weaken checkout rejection or skip the regression. + Temporary resources: task-owned `.codex/run/orchestration-stability/` for preview evidence; this fresh worktree created `target/`, `apps/desktop/node_modules/` and `apps/desktop/dist/`. Remove dedicated compiled outputs after checks; retain installed dependencies until this worktree is retired. Stop the task's renderer process and remove disposable evidence at handoff. No user desktop restart. Rollback: revert only this change's scoped edits; there is no migration. diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md index 9c6cf66e..6457de89 100644 --- a/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/spec.md @@ -27,3 +27,5 @@ Targeted corrections cost less to implement, validate and maintain than replacin - [x] AC-3: Model discovery has no static fallback catalogue or permanent cache; unknown providers/models are never fabricated by the renderer. - [x] AC-4: Provider metadata cannot crash transcript projection; switching has visible pending state and duplicate actions are fenced. Actual renderer checks cover the affected controls. - [x] AC-5: Relevant Rust, desktop, documentation and worktree lifecycle checks pass; cleanup and unchecked live-provider boundaries are recorded. + +Worktree continuation must distinguish recycled Unix inode numbers using filesystem birth time when available. Legacy receipts and filesystems without birth time retain device/inode and Git provenance checks; that narrower guarantee remains explicit. diff --git a/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md b/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md index 33802fd4..09aa2701 100644 --- a/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md +++ b/docs/sdlc/changes/2026-09-28-orchestration-stability/verification.md @@ -62,9 +62,15 @@ ACP launches now create isolated Unix process groups and terminate the owned gro Unix process groups cover ordinary descendants; descendants that deliberately create a new process group and Windows process-tree ownership are not claimed. Windows retains direct-child cleanup. +## Linux CI ownership regression + +[PR #242 CI run 36384177651](https://github.com/IchenDEV/codeTwo/actions/runs/36384177651) passed frontend checks/build and Rust workspace check, then exposed two existing worktree provenance failures on Linux. One assertion required a path-identity error even when the repository-provenance guard correctly rejected the replacement. The other exposed a real false match: the filesystem reused a removed directory's inode and Git reused the admin path, so a distinct worktree passed the old device/inode receipt. + +Unix receipts now persist the filesystem birth timestamp when available and require it to match. Old receipts without that optional field retain their existing device/inode and Git provenance checks. Filesystems without birth timestamps retain that narrower guarantee. A deterministic regression models the same inode with a different birth time, confirms content changes keep the identity valid, and round-trips new and old receipts. The existing moved-worktree rejection test remains active; the unrelated-repository assertion accepts either valid rejection layer. `cargo test -p codetwo-core --lib --quiet` passed 553 tests locally. `cargo check --workspace --all-targets` also passed, recorded in `inode-reuse-workspace-check.log`; Linux remote revalidation remains required before merge. No unrelated production execution or approval policy changed. + ## Cleanup -Removed: Task-created `target/` (2.8 GiB at the first handoff; 2.3 GiB rebuilt and removed after each authenticated verification continuation), the empty live-test working directory, per-probe temporary working directories, one-off matrix runner scripts, `apps/desktop/dist/` (48 MiB), and temporary browser harness HTML/TSX under `apps/desktop/.codex/run/orchestration-stability/`. The temporary browser tab was closed and its viewport override reset. +Removed: PR follow-up `target/` rebuild (1.9 GiB), verified absent after removal. Task-created `target/` (2.8 GiB at the first handoff; 2.3 GiB rebuilt and removed after each authenticated verification continuation), the empty live-test working directory, per-probe temporary working directories, one-off matrix runner scripts, `apps/desktop/dist/` (48 MiB), and temporary browser harness HTML/TSX under `apps/desktop/.codex/run/orchestration-stability/`. The temporary browser tab was closed and its viewport override reset. Retained: `.codex/run/orchestration-stability/` contains small test logs, successful/failed authenticated canary evidence, and light/dark rendered evidence; `apps/desktop/node_modules/` (1.7 GiB) contains installed development dependencies. Retention owner: Codex for this change and checkout. Cleanup trigger: Review retained evidence on the next continuation and remove disposable logs/captures after this change is reviewed or closed; remove the installed dependencies when this worktree is retired. Provider-managed synthetic session histories and shared package caches are left to their normal lifecycle; no account history is deleted by this task.