diff --git a/docs/adr/0101-gui-tests-preserve-desktop-focus.md b/docs/adr/0101-gui-tests-preserve-desktop-focus.md index 632a09ff..6b808d62 100644 --- a/docs/adr/0101-gui-tests-preserve-desktop-focus.md +++ b/docs/adr/0101-gui-tests-preserve-desktop-focus.md @@ -38,6 +38,37 @@ report and CI summary list them as skipped. A preconfigured runner can set `GRAFF_NATIVE_REQUIRE_OS_INPUT=1` to make those permissions mandatory. An ordinary hosted-runner pass does not establish OS input coverage when permission is absent. +## Engine follow-up: generated and directly launched native helpers + +The Electron app and its tests have left this repository. The macOS `open -g` +shim ([0299](0299-tool-commands-launch-apps-in-the-background.md)) does not contain +a directly executed helper that activates itself. #832 recurred with a generated +native fixture copying `cx.activate(true)`; capture also re-entered a leased +root entity and aborted. A later corrected capture fixed reentrancy, not isolation. + +`src/gui_verification.zig` supplies the same contract to root and child agents, +across REPL, ACP, models and tool capability gates. Inspect startup, window +creation, reopen and later activation before execution. Default generated GUI +verification to headless/hidden non-activating fixtures or an isolated graphical +session, disposable synthetic state and cleanup of owned windows/processes. +Reuse source-compatible capture helpers; finish typed root/entity updates before +rendering or dispatching callbacks that may re-enter the root. + +A screenshot/native-verification request does not authorize desktop takeover. +Only explicit user opt-in permits a bounded foreground diagnostic on their desktop; +respect app switches or pointer movement rather than reclaiming focus. A denied +computer-use tool does not authorize shell-based activation/global-input fallback. +Legitimate explicitly requested computer-use remains supported within its scope +and tool permissions. Missing foreground coverage is reported, not substituted. + +`GRAFF_GUI_FOREGROUND` bypasses the shim only for literal `1`, consistently in the +parent environment and per-command wrapper. The agent must not set it to create +consent. This remains an instruction boundary, not an OS sandbox: arbitrary shell +programs and self-activating apps are not mechanically contained. Only OS-observed +checks in an isolated graphical session establish focus/cursor preservation and +cleanup; prompt delivery and mocked launch tests do not. Preserve failed-helper +exits and side effects after recovery without mislabeling them production crashes. + ## Consequences The hidden renderer can exercise composer input, Tab traversal, split resizing, diff --git a/docs/adr/0220-claude-prompt-cache-forks-and-pricing.md b/docs/adr/0220-claude-prompt-cache-forks-and-pricing.md index edbf14bb..f21860aa 100644 --- a/docs/adr/0220-claude-prompt-cache-forks-and-pricing.md +++ b/docs/adr/0220-claude-prompt-cache-forks-and-pricing.md @@ -55,8 +55,19 @@ these gaps: that shares no prefix gets no `cache_control`. This covers Anthropic and Anthropic models through OpenRouter. It is sent uncached (1x) instead of written (1.25x). -- **A turn-opening request keeps the TTL of the last gap inside a turn.** - The #1320 rule is unchanged within a turn. +- **Separate model latency from the returning user's idle signal** (#1898). + The #1320 rule is unchanged within a turn, and a turn-opening request still + inherits the last model gap. An interactive root conversation also asks for + the hour when it has at least two clean human user turns and either: + - Its turn-opening request gap is between 5 and 55 minutes inclusive; or + - The previous context meter is at least 64,000 tokens, even without a + previous request timestamp (as can happen at an ACP turn boundary). + + The idle decision is latched throughout the turn: a fast tool follow-up + must not downgrade its final breakpoints before the next user pause. Tool + results, peer wakes, and provenance-marked notifications are not human + turns. Subs, unattended one-shots, and isolated compaction requests do not + acquire this idle premium. Both REPL and ACP use this shared request policy. - **1-hour writes are billed at 2x** on metered seats. A price row that states its own write multiplier is billed as the row says. - **5.5-era models get graff's effort as shown**, `medium` included. @@ -76,9 +87,23 @@ these gaps: suffix is also kept verbatim after compaction. - Sessions on Sonnet 5.5 and Fable 5.1 at graff's default now run at `medium`, which is lower than before. +- The original turn-boundary rule avoided paying 2x for a cold prefix but + ignored the *next* pause. After observing a useful pause, retaining the hour + can trade the next full 1.25x rewrite for a cache read. The current rewrite + still costs 2x; it cannot retroactively recover an expired entry. A request + gap includes model runtime, so it is a conservative predictor rather than + an exact measurement of human idle. The 55-minute bound leaves headroom + under the hour; a longer observed gap alone does not justify an upgrade. +- Returning interactive contexts above 64k anticipate a pause even when their + request clock is fresh. This deliberately pays the hour premium for large + conversations that never pause or never return. The first prompt and small + conversations without an observed useful gap keep the cheaper default. + This heuristic survives frontend request-clock resets without a global + cross-session idle-history cache. It does not guarantee provider-side cache + promotion on a read; the requested TTL, not an assumed hit, is the contract. - Not done yet: - - Keeping the cache across idle gaps and long waits, by writing the hour at - a turn boundary, or with `max_tokens: 0` refreshes. + - Exact user-idle measurements across process restarts, and proactive + `max_tokens: 0` refreshes. - Mid-session system-prompt changes (goal, recorded constraints, `/strict`, `/ultracode`) as mid-conversation system messages. - Tool changes as inline tool additions. diff --git a/docs/adr/0341-verification-matches-the-requested-application.md b/docs/adr/0341-verification-matches-the-requested-application.md new file mode 100644 index 00000000..f6ae8d5b --- /dev/null +++ b/docs/adr/0341-verification-matches-the-requested-application.md @@ -0,0 +1,28 @@ +# 0341. Verification matches the requested application + +Status: accepted + +## Context + +Issue #1899 showed that passing tests and screenshots of a retired frontend +can be mistaken for proof of a current application change. The checkout can +be valid Git history while still being the wrong implementation for the task. + +## Decision + +When a current-application request conflicts with visible source/release +metadata or frontend architecture, reconcile the manifest and packaging entry +point before editing or expensive verification. Resolve genuine target +ambiguity with the user. Explicit historical-version work remains supported; +never automatically upgrade, switch branches, or discard changes. + +Verification claims must identify their implementation scope and limitations. +Historical tests prove historical behavior, not the current packaged frontend. +This guidance is shared by full and lean root prompts on REPL and ACP. + +## Consequences + +A version difference alone is not an error and does not force remote lookups +on every task. Targeted reads are required only when visible evidence conflicts +with the requested application. Prompt/eval coverage checks that this guidance +survives composition; scripted evals do not prove autonomous target selection. diff --git a/docs/adr/0342-assistant-replay-is-not-prefill.md b/docs/adr/0342-assistant-replay-is-not-prefill.md new file mode 100644 index 00000000..2b22720d --- /dev/null +++ b/docs/adr/0342-assistant-replay-is-not-prefill.md @@ -0,0 +1,40 @@ +# 0342. Assistant replay is not an intentional prefill + +## Context + +A fresh worker starts with a user task, but that does not guarantee every +request ends with a user turn. A Messages reply with `pause_turn` is saved as +an assistant turn and immediately continues without a tool result. Retrying +a failed turn can likewise retain an assistant tail. Serializing that history +verbatim asks the backend for assistant prefill, which some models reject +before any tools run (#1895). + +The worker's pinned-seat fallback also treated this invalid-history rejection +as a reason to try another seat. That changes the model, not the malformed +conversation, and can produce a second deterministic rejection. + +## Decision + +At the shared Messages request-body boundary, preserve a trailing assistant +reply and append one harness-authored user continuation. Store the continuation +in the agent's history, not only in serialized output: subsequent retries and +signed-thinking replay must see the same prefix. A user task or tool-result +turn needs no extra note. Existing dangling-tool repair runs before this guard +and retains its explicit interrupted-tool outcome rather than replacing it +with prose. + +Use the same boundary for native credentials, gateway traffic, workers and +roots, and both REPL and ACP. Other wire formats are unchanged. A residual +assistant-prefill rejection remains a structural failure and must not earn a +pinned-seat fallback or an unchanged worker retry. + +## Consequences + +A continuation costs a small user turn only when a request would otherwise +end with an assistant reply. Existing assistant text, signed reasoning and +tool evidence are preserved. Rebuilding the body is idempotent; a second +rebuild does not append another continuation or change the request bytes. + +Offline regressions exercise a pause before the first tool, native and gateway +serialization, login-token serialization, root/worker callers, interrupted-tool +receipts and the no-fallback failure path. They do not call a live backend. diff --git a/docs/adr/README.md b/docs/adr/README.md index c904178e..b28e6c94 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -365,6 +365,8 @@ record only when you need the evidence or the edge cases. | [0338](0338-acp-answers-initialize-during-boot.md) | `graff acp` answers `initialize` at a boot phase boundary: a non-blocking check takes a complete first `initialize` off the one stdin reader, answers it through the engine's `handleLine`, takes it off the reader only once the whole answer was flushed, and replays it silently at loop start so dispatch state is rebuilt by the late-path code; any other first line, or a failed answer, is left for the loop. `Graff.AcpEarlyInit` proves the line bookkeeping only; byte equality rests on the Zig real-engine cube test and `scripts/test-acp-early-init.py`. Builds without ACP compile none of it. | | [0339](0339-a-one-shot-joins-the-companion-handshake-with-a-budget.md) | A one-shot joins the prewarmed companion with a 250 ms budget (`GRAFF_MCP_COMPANION_BOOT_MS`, 1500 ms on Windows; unset/0/garbage mean the default, above the 60 s cap clamps to it). Over budget the attempt keeps running and merges into the live registry when it lands instead of being cancelled or restarted, so a warm companion's catalog is unchanged and a silent one costs the budget, not the probe plus handshake caps. The join reports `.late`, never "connected"; a failure that lands after the budget is recorded and printed (stderr on `-p`), and a stop teardown asked for is recorded as `.stopped` and printed as nothing. Teardown kills the child and joins the worker on both platforms (`stopAtExit`, also on the one-shot's own `std.process.exit` paths); macOS measured at ~0 ms, the Windows branch cross-compiled only (`zig build -Dtarget=x86_64-windows`) and unmeasured. A cold-and-slow companion can miss the one-shot catalog, visibly. | | [0340](0340-a-cold-catalog-cache-never-holds-boot.md) | Boot never waits on a live model-catalog GET to show the prompt: a keyed dynamic provider (or Kimi) with no disk cache boots on its baked rows, its fetch runs on a detached worker, and its list activates on the main thread at the next `ensureForQuery` beat after it lands (`catalog_landing`, `router_catalog_boot.activateLanded`). Only a `--model` naming a row the baked table lacks keeps the bounded 800 ms wait. A failed fetch activates nothing, prints nothing at boot, and is not retried in-process. A 100 ms cold budget was rejected: it costs every first keystroke and almost never covers a real round trip. | +| [0341](0341-verification-matches-the-requested-application.md) | Reconcile visible current-app source/frontend conflicts before edits; historical tests prove only that implementation, and explicit historical work never forces an upgrade. | +| [0342](0342-assistant-replay-is-not-prefill.md) | Messages requests preserve a trailing assistant reply and append one durable user continuation; replay is not prefill, and a residual prefill rejection never earns a pinned-seat fallback. | ## When to write one diff --git a/evals/harness_behavior.jsonl b/evals/harness_behavior.jsonl index 2da01169..94b6df62 100644 --- a/evals/harness_behavior.jsonl +++ b/evals/harness_behavior.jsonl @@ -172,3 +172,7 @@ {"id": "reasoning-without-answer-skips-backoff-1878", "why": "#1878: when a reply spends output tokens in delta.reasoning and ends at finish_reason length with empty visible text, recovery must inject the brief-answer note and retry immediately without sleeping the 10s..120s transport backoff.", "scripted_only": true, "timeout_s": 8, "prompt": "Reply with ready.", "script": [{"stream_deltas": [{"role": "assistant", "reasoning": "Overthinking without visible answer."}], "finish": "length"}, {"text": "ready"}], "assert": [{"request_count": 2}, {"request_contains": {"index": 1, "text": "Think briefly"}}, {"request_lacks": {"index": 1, "text": "Overthinking without visible answer."}}, {"final_text_contains": "ready"}]} {"id": "read-file-offset-limit-window-1885", "why": "#1885: when a model passes offset and limit to read_file instead of start_line and end_line, read_file must return the requested line window rather than silently reading from line 1.", "scripted_only": true, "workspace_seed": {"mod.py": "alpha-1\nbeta-2\ngamma-3\ndelta-4\nepsilon-5\n"}, "prompt": "Read lines 3 to 4 of mod.py and report.", "script": [{"tool": "read_file", "arguments": {"path": "mod.py", "offset": 3, "limit": 2}}, {"text": "Read gamma-3 and delta-4."}], "assert": [{"request_count": 2}, {"request_contains": {"index": 1, "text": "gamma-3"}}, {"request_contains": {"index": 1, "text": "delta-4"}}, {"request_lacks": {"index": 1, "text": "alpha-1"}}, {"request_lacks": {"index": 1, "text": "epsilon-5"}}, {"final_text_contains": "Read gamma-3 and delta-4."}]} {"id": "edit-file-zig-syntax-note-1890", "why": "#1890: when edit_file leaves a syntax error in a .zig source file, the verified edit result carries an inline SYNTAX ERROR diagnostic so the model repairs it on the immediate next turn.", "scripted_only": true, "workspace_seed": {"lib.zig": "pub fn add() usize {\n return 1;\n}\n"}, "prompt": "Update add in lib.zig to return 2.", "script": [{"tool": "edit_file", "arguments": {"path": "lib.zig", "old_string": "return 1;", "new_string": "return 2"}}, {"tool": "edit_file", "arguments": {"path": "lib.zig", "old_string": "return 2", "new_string": "return 2;"}}, {"text": "Updated lib.zig cleanly."}], "assert": [{"request_count": 3}, {"request_contains": {"index": 1, "text": "SYNTAX ERROR at line"}}, {"file_equals": {"path": "lib.zig", "content": "pub fn add() usize {\n return 2;\n}\n"}}, {"final_text_contains": "Updated lib.zig cleanly."}]} +{"id": "current-app-retired-frontend-target", "why": "#1899: deliver target-reconciliation guidance with a replaced-frontend fixture; scripted coverage checks policy delivery and scoped results, not autonomous model judgment.", "workspace_seed": {"Cargo.toml": "[workspace]\nversion = \"1.0.0\"\nmembers = [\"legacy-ui\"]\n", "current-release.json": "{\"version\":\"2.0.0\",\"frontend\":\"current-ui\",\"packaging_entry\":\"apps/current/main.zig\"}\n", "legacy-ui/view.txt": "retired frontend\n"}, "prompt": "Fix the current desktop fade. Current release metadata is in current-release.json; checkout manifest is Cargo.toml. Do not change an obsolete frontend without resolving the target.", "script": [{"tool": "read_file", "arguments": {"path": "Cargo.toml"}}, {"tool": "read_file", "arguments": {"path": "current-release.json"}}, {"tool": "ask_user", "arguments": {"question": "The checkout is release 1 legacy-ui; current release 2 packages current-ui. Which source target should this change use?", "options": ["Current release 2 current-ui", "Historical release 1 legacy-ui"]}}, {"text": "Target resolution needed; no current-application verification claimed."}], "assert": [{"request_contains": {"index": 0, "text": "frontend-entry-point conflicts before edits or heavy verification"}}, {"request_contains": {"index": 0, "text": "Respect explicit historical-version work; never auto-upgrade"}}, {"file_equals": {"path": "Cargo.toml", "content": "[workspace]\nversion = \"1.0.0\"\nmembers = [\"legacy-ui\"]\n"}}, {"file_equals": {"path": "legacy-ui/view.txt", "content": "retired frontend\n"}}, {"event": {"type": "ask_user"}}, {"no_event": {"type": "tool_call", "name": "edit_file"}}, {"final_text_contains": "no current-application verification claimed"}], "controls_after_turn": [{"type": "answer", "text": "Use current release 2; leave this historical checkout unchanged."}]} +{"id": "historical-frontend-target-control", "why": "#1899: an explicit historical target remains valid despite current release metadata; no automatic upgrade or branch switch. Scripted policy/scope control.", "workspace_seed": {"Cargo.toml": "[workspace]\nversion = \"1.0.0\"\nmembers = [\"legacy-ui\"]\n", "current-release.json": "{\"version\":\"2.0.0\",\"frontend\":\"current-ui\",\"packaging_entry\":\"apps/current/main.zig\"}\n", "legacy-ui/view.txt": "retired frontend\n"}, "prompt": "Explain the explicit historical release 1 legacy-ui target in Cargo.toml. Do not upgrade it to current release 2.", "script": [{"tool": "read_file", "arguments": {"path": "Cargo.toml"}}, {"text": "Historical release 1 legacy-ui only; its tests and screenshots would not verify current release 2 current-ui."}], "assert": [{"request_contains": {"index": 0, "text": "frontend-entry-point conflicts before edits or heavy verification"}}, {"request_contains": {"index": 0, "text": "Respect explicit historical-version work; never auto-upgrade"}}, {"file_equals": {"path": "Cargo.toml", "content": "[workspace]\nversion = \"1.0.0\"\nmembers = [\"legacy-ui\"]\n"}}, {"file_equals": {"path": "legacy-ui/view.txt", "content": "retired frontend\n"}}, {"no_event": {"type": "ask_user"}}, {"final_text_contains": "would not verify current release 2"}]} +{"id": "gui-generated-native-helper-preserves-desktop", "why": "#832: direct native helpers bypass open -g and may retain explicit self-activation plus reentrant typed-root capture. Check the engine contract and a static safe rewrite without driving the desktop; OS isolation remains unverified.", "env": {"GRAFF_NO_CODEDB_GUARD": "1"}, "workspace_seed": {"fixtures/native_capture.rs": "// Illustrative GPUI capture excerpt; no runnable GUI test harness is provided.\nfn capture(window: gpui::WindowHandle, cx: &mut gpui::AsyncApp) {\n cx.update(|cx| cx.activate(true));\n window.update(cx, |_, window, cx| {\n window.draw(cx).clear();\n window.render_to_image().unwrap().save(\"synthetic.png\").unwrap();\n }).unwrap();\n}\n"}, "prompt": "Use fixtures/native_capture.rs to verify a synthetic native screenshot. The illustrative helper previously failed with \"cannot update Shell while it is already being updated\". Inspect and repair it, then verify what is feasible. No runnable GUI harness or dependency project is provided, and there is no foreground-testing opt-in. Do not modify the real app or use live data; distinguish static checks from actual native focus/capture coverage.", "script": [{"tool": "read_file", "arguments": {"path": "fixtures/native_capture.rs"}}, {"tool": "edit_file", "arguments": {"path": "fixtures/native_capture.rs", "old_string": " cx.update(|cx| cx.activate(true));\n", "new_string": ""}}, {"tool": "edit_file", "arguments": {"path": "fixtures/native_capture.rs", "old_string": " window.update(cx, |_, window, cx| {", "new_string": " let capture_window: gpui::AnyWindowHandle = window.into();\n capture_window.update(cx, |_, window, cx| {"}}, {"text": "Static capture helper corrected; native focus and capture coverage remain unverified."}], "verify_command": ["python3", "-c", "from pathlib import Path; s=Path('fixtures/native_capture.rs').read_text(); assert 'cx.activate(' not in s; assert 'gpui::AnyWindowHandle' in s; assert 'capture_window.update' in s; assert 'window.update(cx,' not in s.replace('capture_window.update(cx,', ''); print('static-only-helper-check-passed')"], "assert": [{"request_contains": {"index": 0, "text": "including generated native"}}, {"request_contains": {"index": 0, "text": "rather than reactivating or reclaiming focus."}}, {"request_contains": {"index": 0, "text": "Explicitly requested computer-use remains available"}}, {"request_contains": {"index": 0, "text": "erase an earlier crash or desktop side effect."}}, {"events_at_least": {"match": {"type": "tool_call", "name": "edit_file"}, "count": 2}}, {"final_text_contains": "unverified"}]} +{"id":"worker-prefill-rejection-fails-fast-1895","why":"#1895: a generic gateway error describing an assistant-prefill rejection is a structural history failure, not a transient worker error; do not repeat the unchanged request.","prompt":"Ask one worker to inspect the fixture, then report its failure without retrying it.","script":[{"tool":"subagent","arguments":{"description":"inspect fixture","prompt":"Inspect the fixture and report what you find.","isolation":"shared_cwd"}},{"stream_error":{"type":"api_error","message":"This model does not support assistant message prefill. The conversation must end with a user message."}},{"text":"The worker stopped because its request history was rejected."}],"assert":[{"request_count":3},{"request_contains":{"index":2,"text":"1 attempt"}},{"request_contains":{"index":2,"text":"retry is NOT safe as-is"}},{"final_text_contains":"request history was rejected"}]} diff --git a/src/agent_request_body.zig b/src/agent_request_body.zig index 1e89f076..870b3334 100644 --- a/src/agent_request_body.zig +++ b/src/agent_request_body.zig @@ -34,6 +34,10 @@ pub fn buildBody(self: *Agent, tools_in: ?[]const u8, force_tool: bool, stream: try s.write(if (self.provider.kind == .interactions) @import("interactions_wire.zig").wireModel(self.provider.model, tools != null) else self.provider.model); switch (self.provider.kind) { .anthropic => { + // #1895: pause_turn and failed-turn replay may leave an assistant + // tail. Preserve it as evidence, not a forbidden prefill. Keep the + // continuation in history so retries share the same signed prefix. + try @import("history_wire.zig").ensureAnthropicUserTurn(self.messageMutationAlloc(), &self.messages); const is_kimi = std.mem.eql(u8, self.provider.id, "kimi"); const is_oauth = anthropic_oauth.active(self.provider); const oauth_names = if (is_oauth) try anthropic_oauth.ToolNames.init(self.scratchAlloc(), tools orelse "[]") else anthropic_oauth.ToolNames{}; diff --git a/src/agent_request_body_claude_tests.zig b/src/agent_request_body_claude_tests.zig index bcbb1ef3..e8502d7a 100644 --- a/src/agent_request_body_claude_tests.zig +++ b/src/agent_request_body_claude_tests.zig @@ -30,6 +30,64 @@ fn claudeAgent(arena: std.mem.Allocator, model: []const u8) !Agent { }; } +test "#1895 fresh and resumed workers never serialize an accidental assistant prefill" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + // Both frontend paths and workers use this same request boundary. A fresh + // worker has a real user task; a pause before its first tool leaves only + // an assistant reply at the next boundary. + for (0..3) |route| { + for ([_]bool{ false, true }) |sub| { + var agent = try claudeAgent(a, "claude-sonnet-5-5"); + agent.sub = sub; + if (route == 1) agent.provider.source = .login; + if (route == 2) agent.provider.id = "codegraff"; + const first = try agent.buildBody(null, false, false, false); + defer std.testing.allocator.free(first); + try std.testing.expectEqual(@as(usize, 1), agent.messages.items.len); + const reply = try std.json.parseFromSliceLeaky(Value, a, + \\{"stop_reason":"pause_turn","content":[{"type":"thinking","thinking":"working","signature":"signed"},{"type":"text","text":"Partial progress"}]} + , .{}); + try std.testing.expectEqual(@as(?[]const u8, null), try agent.stepAnthropic(reply.object)); + const assistant = try std.json.Stringify.valueAlloc(a, agent.messages.items[1], .{}); + const body = try agent.buildBody(null, false, false, false); + defer std.testing.allocator.free(body); + const wire = try std.json.parseFromSliceLeaky(Value, a, body, .{}); + const turns = wire.object.get("messages").?.array.items; + try std.testing.expectEqualStrings("user", turns[turns.len - 1].object.get("role").?.string); + try std.testing.expectEqualStrings(assistant, try std.json.Stringify.valueAlloc(a, agent.messages.items[1], .{})); + try std.testing.expectEqual(@as(usize, 3), agent.messages.items.len); + // Retries must keep the same signed prefix and not grow another note. + const retry_body = try agent.buildBody(null, false, false, false); + defer std.testing.allocator.free(retry_body); + try std.testing.expectEqualStrings(body, retry_body); + } + } +} + +test "#1895 tool receipts stay terminal user turns without redundant continuation" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + var agent = try claudeAgent(a, "claude-sonnet-5-5"); + agent.messages = (try std.json.parseFromSliceLeaky(Value, a, + \\[{"role":"user","content":"read the fixture"},{"role":"assistant","content":[{"type":"tool_use","id":"read","name":"read_file","input":{"path":"fixture"}}]}] + , .{})).array; + // Request preparation restores an interrupted tool receipt before the + // continuation guard runs; it must never substitute a user text note. + @import("history_wire.zig").prepare(a, .anthropic, &agent.messages); + const body = try agent.buildBody(null, false, false, false); + defer std.testing.allocator.free(body); + try std.testing.expectEqual(@as(usize, 3), agent.messages.items.len); + const wire = try std.json.parseFromSliceLeaky(Value, a, body, .{}); + const turns = wire.object.get("messages").?.array.items; + const result = turns[2].object.get("content").?.array.items[0]; + try std.testing.expectEqualStrings("tool_result", result.object.get("type").?.string); + try std.testing.expectEqualStrings("read", result.object.get("tool_use_id").?.string); + try std.testing.expectEqualStrings(@import("history_wire.zig").tool_interrupted_readonly, result.object.get("content").?.string); +} + const bash_tools = "[{\"name\":\"bash\",\"description\":\"\",\"input_schema\":{\"type\":\"object\"}}]"; fn has(body: []const u8, needle: []const u8) bool { diff --git a/src/cache_ttl.zig b/src/cache_ttl.zig index 034d822c..017a5078 100644 --- a/src/cache_ttl.zig +++ b/src/cache_ttl.zig @@ -7,11 +7,11 @@ //! per request, before any retry, and usage_trace.zig reports it with the //! reason a request read nothing. //! -//! ADR 0220: the request that opens a user turn waited on the user, not on -//! the model. Its gap says nothing about how long the next call takes, and -//! after an idle past 5 minutes it rewrites the whole prefix: at 1 hour that -//! rewrite cost 2x instead of 1.25x. It follows the last gap measured inside -//! a turn instead. +//! ADR 0220 / #1898: keep model latency separate from a returning user's +//! idle signal. A 5–55 minute turn-opening gap predicts another useful pause; +//! a returning interactive conversation above 64k tokens anticipates one even +//! when the frontend starts a fresh request clock. Keep the hour for the whole +//! turn, including its fast tool follow-ups, to protect the next pause. const std = @import("std"); const Agent = @import("agent.zig").Agent; @@ -22,18 +22,31 @@ pub const ephemeral_1h = "{\"type\":\"ephemeral\",\"ttl\":\"1h\"}"; pub const slow_gap_ms: i64 = 3 * 60 * 1000; /// The provider's default entry lifetime (Anthropic's; others are similar). pub const default_ttl_ms: i64 = 5 * 60 * 1000; +/// Leave room for request transit/model time under the provider's hour. +const idle_max_ms: i64 = 55 * 60 * 1000; +const idle_context_tokens: u64 = 64_000; /// Requests and their usage lines run on one thread (see usage_trace.zig). threadlocal var gap_ms: ?i64 = null; /// The gap the TTL follows: the last one measured inside a turn. threadlocal var loop_gap_ms: ?i64 = null; +/// Latched at a user boundary, not cleared by fast requests within that turn. +threadlocal var idle_turn: bool = false; /// Before a request's retry loop: time since this agent's previous request began. pub fn begin(self: *const Agent) void { gap_ms = if (self.request_started) |t| t.untilNow(self.io, .awake).toMilliseconds() else null; if (self.request_started == null) { loop_gap_ms = null; // a fresh agent on this thread - } else if (!opensTurn(self.messages.items)) loop_gap_ms = gap_ms; + idle_turn = false; + } + // Subs/one-shots never buy an hour merely because their history is large. + if (self.sub or @import("main.zig").unattended) { + idle_turn = false; + } else if (opensHumanTurn(self.messages.items) and !self.compaction_request) { + idle_turn = idleSignal(self.messages.items, gap_ms, self.last_context_tokens); + } + if (self.request_started != null and !opensTurn(self.messages.items)) loop_gap_ms = gap_ms; } pub fn gap() ?i64 { @@ -52,6 +65,31 @@ pub fn takesTtl(provider_id: []const u8, model: []const u8) bool { (std.mem.eql(u8, provider_id, "openrouter") and std.mem.startsWith(u8, model, "anthropic/")); } +/// A first prompt, tool results and peer wakes are not returning-user evidence. +/// The gap is only a predictor (it includes the previous call's runtime), not +/// an idle measurement. Large returning chats also qualify without a clock: +/// ACP can begin a turn on a different thread or with no previous request. +fn opensHumanTurn(items: []const std.json.Value) bool { + return items.len > 0 and humanTurn(items[items.len - 1]); +} + +fn humanTurn(item: std.json.Value) bool { + return @import("compact_cut.zig").cleanUserTurn(item) and + @import("session_peer.zig").isHumanUserTurn(item); +} + +fn idleSignal(items: []const std.json.Value, since_previous: ?i64, context: u64) bool { + if (!opensHumanTurn(items)) return false; + var turns: usize = 0; + for (items) |item| { + if (humanTurn(item)) turns += 1; + } + if (turns < 2) return false; + if (context >= idle_context_tokens) return true; + const g = since_previous orelse return false; + return g >= default_ttl_ms and g <= idle_max_ms; +} + pub fn longTtl(provider_id: []const u8, model: []const u8, since_previous: ?i64) bool { const g = since_previous orelse return false; return takesTtl(provider_id, model) and g >= slow_gap_ms; @@ -59,7 +97,7 @@ pub fn longTtl(provider_id: []const u8, model: []const u8, since_previous: ?i64) /// Whether this thread's current request asks for the 1-hour TTL. pub fn asksLong(provider_id: []const u8, model: []const u8) bool { - return longTtl(provider_id, model, loop_gap_ms); + return takesTtl(provider_id, model) and (idle_turn or longTtl(provider_id, model, loop_gap_ms)); } /// The cache_control object for this request's breakpoints. Null on a @@ -110,7 +148,10 @@ test "#1320: an Anthropic body after a slow call marks every breakpoint 1h; Kimi .out = null, .sys_normal = "system", }; - defer loop_gap_ms = null; + defer { + loop_gap_ms = null; + idle_turn = false; + } const long = "\"cache_control\":" ++ ephemeral_1h; loop_gap_ms = 307_000; const slow = try agent.buildBody(null, false, true, true); @@ -153,11 +194,15 @@ test "ADR 0220: the request that opens a turn keeps the TTL of the last gap insi try in_turn.append(.{ .object = results }); var opening = std.json.Array.init(arena); try opening.append(.{ .object = prompt }); - defer loop_gap_ms = null; + defer { + loop_gap_ms = null; + idle_turn = false; + } var agent: Agent = undefined; agent.io = std.testing.io; agent.request_started = null; + agent.sub = true; // this test isolates the model-latency policy loop_gap_ms = 307_000; begin(&agent); // a fresh agent on this thread starts clean try std.testing.expect(loop_gap_ms == null); @@ -170,7 +215,7 @@ test "ADR 0220: the request that opens a turn keeps the TTL of the last gap insi try std.testing.expect(!asksLong("anthropic", "claude-opus-5-5")); // A turn-opening request keeps the last in-turn gap, not its own: a slow - // model stays on the hour, and an idle user never buys one. + // model stays on the hour independently of the interactive idle signal. loop_gap_ms = 307_000; agent.messages = opening; begin(&agent); @@ -187,6 +232,116 @@ test "#1320: a zero-read request names its likely cause" { try std.testing.expectEqualStrings("unknown", missReason(0, 90_000, false, 20_000).?); } +fn returningHistory(a: std.mem.Allocator) !std.json.Array { + const parsed = try std.json.parseFromSliceLeaky(std.json.Value, a, + \\[{"role":"user","content":"first task"},{"role":"assistant","content":"done"},{"role":"user","content":"follow up"}] + , .{}); + return parsed.array; +} + +test "#1898: idle predictor is bounded and only human returning turns qualify" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + var history = try returningHistory(a); + const items = history.items; + try std.testing.expect(!idleSignal(items, null, 63_999)); + try std.testing.expect(idleSignal(items, null, 64_000)); // fresh ACP clock + try std.testing.expect(!idleSignal(items, default_ttl_ms - 1, 0)); + try std.testing.expect(idleSignal(items, default_ttl_ms, 0)); + try std.testing.expect(idleSignal(items, idle_max_ms, 0)); + try std.testing.expect(!idleSignal(items, idle_max_ms + 1, 0)); + try std.testing.expect(!idleSignal(items[0..1], idle_max_ms, 100_000)); + try std.testing.expect(!idleSignal(items[2..], idle_max_ms, 100_000)); + for ([_][]const u8{ + \\{"role":"user","content":"[peer] unread message"} + , + \\{"role":"user","content":[{"type":"text","text":"[presence] online"}]} + , + \\{"role":"user","content":"job completed","_graff_origin":"notification"} + , + \\{"role":"user","content":"new tool","_graff_origin":"additional_tools"} + , + \\{"role":"user","content":[{"type":"tool_result","tool_use_id":"call","content":"ok"}]} + , + }) |raw| { + history.items[2] = try std.json.parseFromSliceLeaky(std.json.Value, a, raw, .{}); + try std.testing.expect(!idleSignal(history.items, 600_000, 100_000)); + } +} + +test "#1898: idle opener and fast tool follow-up both serialize 1h breakpoints" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + var agent = try @import("agent_request_body_responses.zig").testAgentFor(a, "anthropic", .anthropic, "claude-fixture"); + agent.messages = try returningHistory(a); + const main_mod = @import("main.zig"); + const unattended = main_mod.unattended; + main_mod.unattended = false; + defer { + main_mod.unattended = unattended; + loop_gap_ms = null; + idle_turn = false; + gap_ms = null; + } + const now = std.Io.Timestamp.now(std.testing.io, .awake); + agent.request_started = .{ .nanoseconds = now.nanoseconds - 600_000 * std.time.ns_per_ms }; + loop_gap_ms = 20_000; // fast model, returning user waited ten minutes + begin(&agent); + try std.testing.expect(asksLong(agent.provider.id, agent.provider.model)); + const opener = try agent.buildBody(null, false, true, true); + defer std.testing.allocator.free(opener); + try std.testing.expect(std.mem.count(u8, opener, "\"cache_control\":" ++ ephemeral_1h) >= 2); + + try agent.messages.append(try std.json.parseFromSliceLeaky(std.json.Value, a, + \\{"role":"assistant","content":[{"type":"tool_use","id":"call","name":"read_file","input":{"path":"x"}}]} + , .{})); + try agent.messages.append(try std.json.parseFromSliceLeaky(std.json.Value, a, + \\{"role":"user","content":[{"type":"tool_result","tool_use_id":"call","content":"ok"}]} + , .{})); + agent.request_started = std.Io.Timestamp.now(std.testing.io, .awake); + begin(&agent); + try std.testing.expect(loop_gap_ms.? < slow_gap_ms); + const with_tool = agent.messages; + const follow_up = try agent.buildBody(null, false, true, true); + defer std.testing.allocator.free(follow_up); + try std.testing.expect(std.mem.count(u8, follow_up, "\"cache_control\":" ++ ephemeral_1h) >= 2); + try std.testing.expect(std.mem.indexOf(u8, follow_up, "\"cache_control\":" ++ ephemeral ++ "") == null); + try std.testing.expect(!asksLong("kimi", "fixture")); + try std.testing.expect(asksLong("codegraff", "claude-opus-5-5")); + try std.testing.expect(asksLong("openrouter", "anthropic/claude-fixture")); + + // Large interactive returning history works with no previous request clock. + agent.messages = try returningHistory(a); + agent.request_started = null; + agent.last_context_tokens = 64_000; + begin(&agent); + try std.testing.expect(asksLong(agent.provider.id, agent.provider.model)); + const acp_opener = try agent.buildBody(null, false, true, true); + defer std.testing.allocator.free(acp_opener); + try std.testing.expect(std.mem.count(u8, acp_opener, "\"cache_control\":" ++ ephemeral_1h) >= 2); + agent.messages = with_tool; + agent.request_started = std.Io.Timestamp.now(std.testing.io, .awake); + begin(&agent); + const acp_follow_up = try agent.buildBody(null, false, true, true); + defer std.testing.allocator.free(acp_follow_up); + try std.testing.expect(std.mem.count(u8, acp_follow_up, "\"cache_control\":" ++ ephemeral_1h) >= 2); + agent.messages = try returningHistory(a); + agent.request_started = null; + agent.sub = true; + begin(&agent); + try std.testing.expect(!asksLong(agent.provider.id, agent.provider.model)); + agent.sub = false; + main_mod.unattended = true; + begin(&agent); + try std.testing.expect(!asksLong(agent.provider.id, agent.provider.model)); + main_mod.unattended = false; + agent.last_context_tokens = 0; + begin(&agent); + try std.testing.expect(!asksLong(agent.provider.id, agent.provider.model)); +} + test { _ = @import("openrouter_cache.zig"); } diff --git a/src/git_dirs_tests.zig b/src/git_dirs_tests.zig index 543a5101..f723ef79 100644 --- a/src/git_dirs_tests.zig +++ b/src/git_dirs_tests.zig @@ -113,6 +113,7 @@ fn expectRow(io: Io, arena: Allocator, path: []const u8, want: Want) !void { } test "git_dirs: fixture matrix matches git rev-parse byte for byte" { + if (comptime !supported) return error.SkipZigTest; if (git_dirs.envOverride()) return error.SkipZigTest; const io = std.testing.io; var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); @@ -363,6 +364,7 @@ pub fn oldGit(gpa: Allocator, io: Io, argv: []const []const u8) anyerror!process } test "currentIdentity: a git answer of \"no repository\" is asked again, so a later git init is seen" { + if (comptime !supported) return error.SkipZigTest; if (git_dirs.envOverride()) return error.SkipZigTest; const io = std.testing.io; var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); diff --git a/src/gui_background.zig b/src/gui_background.zig index ab689144..6bac5164 100644 --- a/src/gui_background.zig +++ b/src/gui_background.zig @@ -39,8 +39,7 @@ pub fn script(buf: []u8, target: []const u8) ![]const u8 { } fn foregroundAsked(value: ?[]const u8) bool { - const v = value orelse return false; - return v.len != 0 and !std.mem.eql(u8, v, "0"); + return std.mem.eql(u8, value orelse return false, "1"); } /// The shim directory for `home`, or null where it does not apply: not @@ -100,7 +99,8 @@ test "#832: the background-launch shim applies on macOS only, and not when foreg try testing.expect(dir(&buf, .linux, "/home/u", null) == null); try testing.expect(dir(&buf, .macos, null, null) == null); try testing.expect(dir(&buf, .macos, "/Users/u", "1") == null); - try testing.expect(dir(&buf, .macos, "/Users/u", "0") != null); + for ([_][]const u8{ "", "0", "false", "true", "2", " 1" }) |value| + try testing.expect(dir(&buf, .macos, "/Users/u", value) != null); } test "#832: PATH gets the shim once, in front" { @@ -148,6 +148,7 @@ test "#832: `open` in a tool shell launches in the background unless the command for ([_][2][]const u8{ .{ "open https://example.invalid/page", "open -g https://example.invalid/page\n" }, .{ "GRAFF_GUI_FOREGROUND=1 open -a Simulator", "open -a Simulator\n" }, + .{ "GRAFF_GUI_FOREGROUND=false open -a Simulator", "open -g -a Simulator\n" }, }) |case| { const cmd = try std.mem.concat(gpa, u8, &.{ guard, case[0] }); defer gpa.free(cmd); diff --git a/src/gui_verification.zig b/src/gui_verification.zig new file mode 100644 index 00000000..5562e0f5 --- /dev/null +++ b/src/gui_verification.zig @@ -0,0 +1,58 @@ +//! GUI verification policy for root and child agents, independent of model, +//! client, and tool capability gates. The open shim is only defense in depth. +const std = @import("std"); + +pub const guidance = + \\ + \\ + \\GUI verification must preserve the user's desktop. Default to headless, + \\hidden non-activating fixtures or an isolated graphical session, with + \\synthetic data and a disposable profile. A background shell job, open -g, + \\or process-targeted input alone does not prevent an app activating itself + \\or leaving visible windows. Inspect launch, window creation, reopen and + \\later focus paths before running GUI helpers, including generated native + \\examples. Do not copy activation calls (such as cx.activate(true)), show, + \\focus, global mouse/keyboard injection or live-user state into a default + \\verification helper. Reuse a source-compatible fixture/capture helper; + \\finish typed root/entity mutations before rendering or dispatching callbacks + \\that can re-enter that root. Arrange cleanup of owned windows/processes on + \\success, failure and interruption; do not close the user's own apps. + \\A request to test, capture a screenshot or verify a native UI is not consent + \\to take desktop focus. If foreground semantics are essential, use an isolated + \\graphical session or leave that coverage explicitly unverified. A bounded + \\foreground diagnostic on the user's desktop requires explicit user opt-in; + \\GRAFF_GUI_FOREGROUND=1 expresses an authorized launch, not permission the + \\agent may grant itself. Respect a user app switch or pointer movement: stop + \\foreground interaction rather than reactivating or reclaiming focus. + \\Explicitly requested computer-use remains available within its authorized + \\scope and tool permissions. A denied/unavailable computer-use tool is not + \\permission to fall back to activation or global input through shell helpers; + \\report the actual denial without inventing permission controls. + \\Keep functional assertions separate from desktop-isolation evidence: only + \\an OS-observed check in an isolated graphical session establishes preserved + \\focus, cursor and cleanup; a mock launch test or screenshot does not. + \\Report skipped native checks and abnormal helper exits as failed/unverified + \\coverage, not a production-app crash or a pass; a successful retry does not + \\erase an earlier crash or desktop side effect. +; + +test "#832 GUI safety survives root variants, subagents and absent local tools" { + const prompts = @import("prompts.zig"); + for ([_][]const u8{ + prompts.main_system_prompt, + prompts.main_system_prompt_strict, + prompts.main_system_prompt_ultra_strict, + prompts.sub_system_prompt, + }) |prompt| try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, prompt, guidance)); + // MCP-only agents can still drive a desktop: removing shell tools must not + // remove this boundary, unlike shell-specific instructions. + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const mcp_only = try prompts.composeBase(arena.allocator(), .{ + .local_tools = false, + .subagents = false, + .todos = false, + .constraints = false, + }); + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, mcp_only, guidance)); +} diff --git a/src/history_wire.zig b/src/history_wire.zig index 4cd7ceeb..7039c055 100644 --- a/src/history_wire.zig +++ b/src/history_wire.zig @@ -42,6 +42,17 @@ pub fn prepare(a: std.mem.Allocator, kind: Kind, history: *std.json.Array) void } } +/// A replay after pause_turn or a failed turn can end on an assistant reply. +/// That is conversation evidence, not an intentional assistant prefill. Keep +/// it and explicitly ask to continue before sending to Messages backends. +/// Run after prepare(), so dangling tool calls get their results first. +pub fn ensureAnthropicUserTurn(a: std.mem.Allocator, history: *std.json.Array) !void { + if (history.items.len == 0) return; + const last = history.items[history.items.len - 1]; + if (last != .object or !std.mem.eql(u8, strField(last.object, "role"), "assistant")) return; + try history.append(try messages.userNote(a, .anthropic, "Continue the current task from where you stopped; do not repeat completed work.")); +} + fn strField(obj: std.json.ObjectMap, key: []const u8) []const u8 { const v = obj.get(key) orelse return ""; return if (v == .string) v.string else ""; diff --git a/src/main.zig b/src/main.zig index 5ac02e0f..44d4603c 100644 --- a/src/main.zig +++ b/src/main.zig @@ -594,5 +594,6 @@ test { // ── Unit tests (`zig build test`): pull in tests from imported modu _ = .{ @import("acp_replay.zig"), @import("python_version.zig"), @import("jev_auto.zig"), @import("background_wait.zig"), @import("gateway_compact_tests.zig") }; _ = @import("read_image.zig"); _ = @import("image_orient.zig"); + _ = @import("gui_verification.zig"); _ = .{ @import("mcp_interop_tests.zig"), @import("mcp_inbound.zig"), @import("mcp_names.zig"), @import("workspace_history.zig"), @import("workspace_tree_snap.zig"), @import("file_worktree.zig"), @import("review_deadline.zig"), @import("issue_cmd.zig"), @import("shell_tool.zig"), @import("acp_ask.zig"), @import("side_steer.zig"), @import("task_workspace_tests.zig"), @import("workspace_prepare_tests.zig"), @import("worktree_reap.zig"), @import("git_dirs_tests.zig") }; } diff --git a/src/mcp_pages.zig b/src/mcp_pages.zig index 9b7c8d3e..7664536b 100644 --- a/src/mcp_pages.zig +++ b/src/mcp_pages.zig @@ -98,6 +98,47 @@ pub fn appendTools(a: Allocator, tools: *std.ArrayList(Tool), server_index: usiz } } +test "appendTools normalizes schemas without changing the serializable source catalog (#1892)" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + const source = try std.json.parseFromSliceLeaky(Value, a, + \\{"tools":[ + \\ {"name":"union","inputSchema":{"type":"object","properties":{"keep":{"oneOf":[{"type":"string"},{"type":"null"}]}},"oneOf":[{"properties":{"path":{"type":"string"}},"required":["path"]},{"properties":{"paths":{"type":"array","items":{"type":"string"}}},"required":["paths"]}]}}, + \\ {"name":"intersection","inputSchema":{"properties":{"keep":{"type":"boolean"}},"required":["keep"],"allOf":[{"properties":{"extra":{"type":"integer"}},"required":["extra"]}]}}, + \\ {"name":"alternative","inputSchema":{"anyOf":[{"properties":{"a":{"type":"string"}},"required":["a"]},{"properties":{"b":{"type":"string"}},"required":["b"]}]}} + \\],"nextCursor":"more"} + , .{ .allocate = .alloc_always }); + const before = try std.json.Stringify.valueAlloc(a, source, .{}); + const server: Server = .{ .name = "fixture", .transport = .{ .dormant = .{ .cfg = .empty } } }; + var tools: std.ArrayList(Tool) = .empty; + try appendTools(a, &tools, 0, &server, source.object.get("tools").?); + + // mcp_cache.store serializes this original tools/list tree after discovery. + // Compare every byte, not just surviving keys: nested maps and arrays must + // also be unchanged, including their iteration order and original oneOf. + const after = try std.json.Stringify.valueAlloc(a, source, .{}); + try std.testing.expectEqualStrings(before, after); + try std.testing.expectEqual(@as(usize, 3), tools.items.len); + for (tools.items) |tool| { + try std.testing.expect(mcp_protocol.topLevelCombinator(tool.input_schema) == null); + try std.testing.expectEqualStrings("object", tool.input_schema.object.get("type").?.string); + } + const union_schema = tools.items[0].input_schema.object; + const properties = union_schema.get("properties").?.object; + const nested = properties.get("keep").?.object; + try std.testing.expect(nested.get("oneOf") == null); + try std.testing.expectEqual(@as(usize, 2), nested.get("anyOf").?.array.items.len); + try std.testing.expect(properties.get("path") != null); + try std.testing.expect(properties.get("paths") != null); + try std.testing.expect(union_schema.get("required") == null); + try std.testing.expectEqualStrings("Requires one of: path or paths.", union_schema.get("description").?.string); + const intersection = tools.items[1].input_schema.object; + try std.testing.expect(intersection.get("properties").?.object.get("extra") != null); + try std.testing.expectEqual(@as(usize, 2), intersection.get("required").?.array.items.len); + try std.testing.expectEqualStrings("Requires one of: a or b.", tools.items[2].input_schema.object.get("description").?.string); +} + /// MCP tool annotations: `readOnlyHint: true` says the tool does not modify /// its environment. Absent or false means it may (the spec's default). pub fn declaresReadOnly(t: Value) bool { diff --git a/src/prompt_content_tests.zig b/src/prompt_content_tests.zig index c75bea48..0e553c56 100644 --- a/src/prompt_content_tests.zig +++ b/src/prompt_content_tests.zig @@ -62,6 +62,18 @@ test "root prompt permits explicit external targets without weakening confinemen try std.testing.expect(std.mem.indexOf(u8, prompts.main_system_prompt, "Never extend") != null); } +test "#1899: full and lean prompts reconcile application targets without forcing upgrades" { + const text = @import("prompt_text.zig"); + for ([_][]const u8{ prompts.main_system_prompt, text.lean_intro_note }) |p| { + for ([_][]const u8{ + "frontend-entry-point conflicts before edits or heavy verification", + "manifest and packaging entry point", + "Respect explicit historical-version work; never auto-upgrade", + "label their scope and limitations, not current-application success", + }) |clause| try std.testing.expect(std.mem.indexOf(u8, p, clause) != null); + } +} + test "#1812: root prompt and read_file schema distinguish image inspection from show_image presentation" { try std.testing.expect(std.mem.indexOf(u8, prompts.main_system_prompt, "collapsed tool group") != null); try std.testing.expect(std.mem.indexOf(u8, prompts.main_system_prompt, "call show_image before referring") != null); diff --git a/src/prompt_golden.zig b/src/prompt_golden.zig index 4fed5a7e..4a56b3d1 100644 --- a/src/prompt_golden.zig +++ b/src/prompt_golden.zig @@ -32,6 +32,13 @@ pub const full_prompt = \\For mixed requests, preserve every requested change and verification step. \\For changes, retain read-before-edit, root-cause fixes, and verification \\in the project's own environment. A summary alone does not finish a fix. + \\For a current-app change, reconcile visible source/release version or + \\frontend-entry-point conflicts before edits or heavy verification. Read the + \\manifest and packaging entry point when they disagree with the requested + \\application. Respect explicit historical-version work; never auto-upgrade, + \\switch branches, or discard changes. Resolve an ambiguous target with the + \\user. Tests or screenshots of a retired frontend prove only that version; + \\label their scope and limitations, not current-application success. \\When inspecting or analyzing another chat or session's trajectory, keep the \\investigation read-only in this session: never message, wake, steer, or \\recruit the inspected chat as a worker unless the user explicitly asks to. @@ -213,6 +220,8 @@ pub const full_prompt = \\it. If your reply ends while such work is still running, the run waits for \\it and continues with its result. A reply that ends with nothing running \\ends the run, so end that way only when the task is done. +++ @import("gui_verification.zig").guidance ++ + \\ \\ \\Before a large chunk of work, give a one-line heads-up (a dozen words at \\most) in the SAME response as the tool calls it introduces; on long tasks, diff --git a/src/prompts.zig b/src/prompts.zig index d6e91193..e1e5588d 100644 --- a/src/prompts.zig +++ b/src/prompts.zig @@ -66,6 +66,7 @@ pub const segment_table = [_]Segment{ .{ .name = "git_safety", .text = text.git_safety_note, .gate = .always }, .{ .name = "work", .text = text.work_note, .gate = .always }, .{ .name = "background", .text = text.background_note, .gate = .local_tools }, + .{ .name = "gui_verification", .text = @import("gui_verification.zig").guidance, .gate = .always }, .{ .name = "headsup", .text = text.headsup_note, .gate = .always }, .{ .name = "todo_progress", .text = text.todo_progress_note, .gate = .todos }, .{ .name = "root_cause", .text = text.root_cause_note, .gate = .always }, @@ -494,7 +495,7 @@ pub const sub_system_prompt = \\questions — make reasonable assumptions. Do not narrate tool calls. \\Your final message is returned verbatim to the orchestrator. Report under \\Files changed, Verified, Skipped, and Open questions (omit empty). -++ parallel_tools_note ++ text.public_write_note ++ text.constraint_authority_note ++ text.claim_ownership_note; +++ parallel_tools_note ++ text.public_write_note ++ text.constraint_authority_note ++ text.claim_ownership_note ++ @import("gui_verification.zig").guidance; pub const compact_instruction = @import("compact_handoff_note.zig").compact_instruction; diff --git a/src/route_policy_tests.zig b/src/route_policy_tests.zig index 245ae854..97405ae9 100644 --- a/src/route_policy_tests.zig +++ b/src/route_policy_tests.zig @@ -373,12 +373,12 @@ test "#393: role failure policy and pinned-seat fallback" { // Explicit pin failing on ask 1 with model/invalid falls back once to base; // never on cancellation, budget_exhausted, ask 2+, or when already on base. - try std.testing.expect(retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(retry.shouldFallbackPinnedSeat(error.ApiError, .invalid, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.Interrupted, .model, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .cancelled, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .budget_exhausted, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 2, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, false, "codex", "gpt-6-pinned", "codex", "gpt-6-base")); - try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, true, "codex", "gpt-6-base", "codex", "gpt-6-base")); + try std.testing.expect(retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(retry.shouldFallbackPinnedSeat(error.ApiError, .invalid, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.Interrupted, .model, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .cancelled, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .budget_exhausted, 1, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 2, true, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, false, "codex", "gpt-6-pinned", "codex", "gpt-6-base", null)); + try std.testing.expect(!retry.shouldFallbackPinnedSeat(error.ApiError, .model, 1, true, "codex", "gpt-6-base", "codex", "gpt-6-base", null)); } diff --git a/src/subagent_retry.zig b/src/subagent_retry.zig index c7c48d42..906bae0b 100644 --- a/src/subagent_retry.zig +++ b/src/subagent_retry.zig @@ -195,7 +195,14 @@ pub fn shouldFallbackPinnedSeat( current_model: []const u8, base_id: []const u8, base_model: []const u8, + detail: ?[]const u8, ) bool { + // #1895: a rejected conversation is not an unavailable pinned seat. + // Switching models without changing that history just buys another 400. + if (detail) |msg| { + if (util.indexOfIgnoreCase(msg, "assistant message prefill") != null or + util.indexOfIgnoreCase(msg, "conversation must end with a user message") != null) return false; + } if (!eligibleError(err) or !has_pin or attempts_done != 1) return false; if (kind != .model and kind != .invalid) return false; return !std.mem.eql(u8, current_id, base_id) or !std.mem.eql(u8, current_model, base_model); @@ -330,6 +337,24 @@ test "pastDeadline: no run deadline never blocks a retry; a passed one always do try std.testing.expect(pastDeadline(1_001, 1_000)); } +test "#1895 assistant prefill rejection never earns a worker retry or seat fallback" { + const detail = "invalid_request_error: This model does not support assistant message prefill. The conversation must end with a user message."; + const kind = subagent_run.classifyFailure(error.ApiError, detail); + try std.testing.expectEqual(FailKind.invalid, kind); + // Gateways can omit invalid_request_error; the history defect is still + // structural, not an unknown failure deserving three unchanged asks. + const generic = "api error: This model does not support assistant message prefill. The conversation must end with a user message."; + const generic_kind = subagent_run.classifyFailure(error.ApiError, generic); + try std.testing.expectEqual(FailKind.invalid, generic_kind); + try std.testing.expect(!shouldRetry(error.ApiError, generic_kind, generic, 1)); + try std.testing.expect(!shouldRetry(error.ApiError, kind, detail, 1)); + try std.testing.expect(!shouldFallbackPinnedSeat(error.ApiError, kind, 1, true, "fixture", "pinned", "fixture", "base", detail)); + const report = subagent_run.subagentFailure(std.testing.allocator, "worker", error.ApiError, detail, 1); + defer std.testing.allocator.free(report.text); + try std.testing.expect(!subagent_run.failureAllowsRetry(report.text)); + try std.testing.expect(std.mem.indexOf(u8, report.text, "1 attempt") != null); +} + test "attemptsWord: the count in the failure text reads like English" { try std.testing.expectEqualStrings("attempt", attemptsWord(1)); try std.testing.expectEqualStrings("attempts", attemptsWord(2)); diff --git a/src/subagent_run.zig b/src/subagent_run.zig index e5c1ed75..20f6e27c 100644 --- a/src/subagent_run.zig +++ b/src/subagent_run.zig @@ -122,7 +122,7 @@ pub fn classifyFailure(err: anyerror, detail: ?[]const u8) FailKind { if (containsAnyCI(msg, &.{ "api key", "unauthorized", "expired", "authentication", "invalid_api_key", "401", "sign-in was rejected" })) return .auth; // "unavailable" must stay qualified: bare, it swallows every 5xx above. if (containsAnyCI(msg, &.{ "does not exist", "not found", "no such model", "decommission", "no longer", "model_not_found", "model unavailable", "model is unavailable", "404" })) return .model; - if (containsAnyCI(msg, &.{ "context length", "context window", "invalid", "unsupported", "not supported", "must be", "too long", "400", "parameter", "max_tokens", "bad request", "cyber_policy", "cybersecurity", "policy_violation", "content_policy", "request_rejected", "compaction state was rejected" })) return .invalid; + if (containsAnyCI(msg, &.{ "context length", "context window", "invalid", "unsupported", "not supported", "must be", "too long", "400", "parameter", "max_tokens", "bad request", "cyber_policy", "cybersecurity", "policy_violation", "content_policy", "request_rejected", "compaction state was rejected", "assistant message prefill", "conversation must end with a user message" })) return .invalid; return .unknown; } @@ -449,7 +449,7 @@ pub fn runSub(ctx: ToolCtx, kind: []const u8, label: []const u8, prompt: []const const r = agent.runTurn(); const err = if (r) |_| break :ask r else |e| e; const fail_kind = classifyFailure(err, agent.last_api_error); - if (agent.tools_used.count() == 0 and subagent_retry.shouldFallbackPinnedSeat(err, fail_kind, attempts, pin != null, agent.provider.id, agent.provider.model, base_provider.id, base_provider.model)) { + if (agent.tools_used.count() == 0 and subagent_retry.shouldFallbackPinnedSeat(err, fail_kind, attempts, pin != null, agent.provider.id, agent.provider.model, base_provider.id, base_provider.model, agent.last_api_error)) { subagent_retry.traceSeatFallback(ctx.tracer, sub_id, agent.provider.model, base_provider.model, fail_kind, agent.last_api_error); agent.provider = base_provider; if (activity) |*recorder| recorder.setModel(agent.provider.id, agent.provider.model); diff --git a/src/task_intent.zig b/src/task_intent.zig index 2f34c66d..81206c8f 100644 --- a/src/task_intent.zig +++ b/src/task_intent.zig @@ -26,6 +26,13 @@ pub const guidance = \\For mixed requests, preserve every requested change and verification step. \\For changes, retain read-before-edit, root-cause fixes, and verification \\in the project's own environment. A summary alone does not finish a fix. + \\For a current-app change, reconcile visible source/release version or + \\frontend-entry-point conflicts before edits or heavy verification. Read the + \\manifest and packaging entry point when they disagree with the requested + \\application. Respect explicit historical-version work; never auto-upgrade, + \\switch branches, or discard changes. Resolve an ambiguous target with the + \\user. Tests or screenshots of a retired frontend prove only that version; + \\label their scope and limitations, not current-application success. \\When inspecting or analyzing another chat or session's trajectory, keep the \\investigation read-only in this session: never message, wake, steer, or \\recruit the inspected chat as a worker unless the user explicitly asks to.