iii-native harness: slim agent_call + Tier 2 polish - #104
Merged
Conversation
Tier 2 dispatcher per spec docs/superpowers/specs/2026-05-07-tier2-iii-pure-harness-design.md. Deleted: SchemaCache, format_to_input_schema, validate_payload, sandbox helpers (sandbox_alive, sandbox_decision, ensure_sandbox, etc.). The dispatcher now only validates the function field, calls iii.trigger, and maps errors. Skills (registered separately via the skills worker) will teach the LLM iii contracts — registry introspection, sandbox lifecycle. agent_call.rs: ~700 lines → ~250 lines. Tests: 72 → 56.
The comment in provisioning.rs referenced agent_call::ensure_sandbox, which was deleted in the Tier 2 thin-dispatcher refactor. Sandbox provisioning is now the LLM's responsibility via a skill recipe.
No longer used after Tier 2 dispatcher refactor — payload validation is gone.
Closes the recursion bypass identified in plan-eng-review D1: without this entry, the LLM can call bridge::trigger via agent_call to dispatch any function and bypass name-matched policy rules (the agent::before_ tool_call hook fires with name 'bridge::trigger', not the inner function id).
The harness copy is gone — only mcp's remains. TODO now flags the extraction as dormant until a second caller appears.
Reflects Tier 2 refactor: dispatcher no longer validates payloads, provisions sandboxes, or queries the registry. Skills (registered via the skills worker) carry iii contract knowledge. Trust-boundary section updated to require bridge::trigger in POLICY_DENIED_TOOLS.
Adds 13 green tests pinning behavior on every CI build, plus 4 #[ignore]'d red tests that document real latent bugs (substring matching false-positives, missing-bracket parser bug, handle_finalize panic on missing last_assistant) for follow-up fix PRs. turn-orchestrator/src/agent_call.rs: 7 green + 2 red - validate_function_field rejects Object/Array/Float Value shapes - decode_or_passthrough handles array, primitive, and partial-ToolResult inputs - TOOL_NAME / FUNCTION_ID constants pinned (wire contract) - Red: is_function_not_found and is_timeout misclassify user content containing the magic substrings turn-orchestrator/src/system_prompt.rs: 3 green - cwd newlines pass through verbatim - skills_index markdown passes through verbatim - 1MB override returns same length (no implicit truncation) turn-orchestrator/src/states/tools.rs: 2 green + 1 red - TOPIC_BEFORE / TOPIC_AFTER constants pinned (policy-gate wire contract) - before_tool_call payload shape pinned (subscriber contract) - Red: handle_finalize source-grep test for the .expect() panic on missing last_assistant policy-denylist/src/main.rs: 3 green + 1 red - parse_denied_tools handles empty / whitespace / json-array syntax - Red: malformed-bracket inputs (missing open or close) leak the bracket into the parsed tool name; the operator's intended denial never matches Plan: ~/.claude/plans/let-s-implement-more-tests-refactored-flask.md. After this lands: turn-orchestrator 56 → 68 lib tests, policy-denylist gains 3. CI signal strengthens; #[ignore]'d tests don't block.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ytallo
changed the title
Replace stringly-typed substring matching in is_function_not_found and
is_timeout with pattern matches on iii_sdk::IIIError variants. The old
heuristic misclassified inner errors whose payload happened to contain
the magic substring (e.g. an IIIError::Handler with a log line that
mentions "function_not_found", or "scheduled timeout in 30 days").
is_function_not_found now matches IIIError::Remote { code, .. } where
code == "function_not_found" — the canonical engine signal (iii.rs:1701).
is_timeout matches IIIError::Timeout — the SDK's structured timeout
variant (iii.rs:1155).
Updates the existing detector tests to construct IIIError variants and
flips the two #[ignore]'d red tests green.
…missing
Replace .expect("tools state requires last_assistant…") with a let-else
that warns and gracefully transitions to TearingDown. The state machine
should only enter ToolFinalize from AssistantFinished (which always
populates last_assistant), but a resume after a mid-turn crash or a
persistence corruption could land here. Panicking in that branch tears
the worker down hard instead of ending the turn cleanly; tool_results
are still persisted.
Flips the source-grep regression guard green.
Previously the parser used a strip_prefix('[').and_then(strip_suffix(']'))
chain that fell back to the raw input on a single missing bracket,
leaking the unmatched bracket into the first or last token. An operator
who set POLICY_DENIED_TOOLS=[bridge::trigger ended up denying the literal
"[bridge::trigger" — never matching the real tool name. Silent policy
bypass on the denylist worker.
Now: brackets must be paired (strip both, or strip neither). Unmatched
brackets log a warning and fall back to a bracket-tolerant trim so the
rest of the input still parses cleanly.
Flips the malformed-bracket regression test green.
Three pre-existing fmt nits flagged by CI's cargo fmt --check: - agent_call.rs:176 — collapse multi-line .get/.cloned/.unwrap_or chain - provisioning.rs:20 — collapse split let binding - tools.rs:340 — split long tc(...) call across multiple lines No behavioral change; unblocks the lint+test CI job.
CI's `cargo clippy --all-targets -- -D warnings` flagged the .map(...).unwrap_or(0) chain in tests/common/mod.rs::nonce as clippy::map_unwrap_or. Collapse to .map_or(0, ...). No behavior change.
andersonleal
approved these changes
May 8, 2026
andersonleal
added a commit
that referenced
this pull request
May 8, 2026
…_value with string-unwrap behavior The slim agent_call PR (#104) shipped a test asserting the pre-fix JSON-stringified form ("\"just a string\""). decode_or_passthrough now unwraps JSON Strings to their inner content per decode_or_passthrough_unwraps_string_value_into_text, so the assertion must read the raw "just a string". The two tests stay consistent.
andersonleal
added a commit
that referenced
this pull request
May 8, 2026
…_value with string-unwrap behavior The slim agent_call PR (#104) shipped a test asserting the pre-fix JSON-stringified form ("\"just a string\""). decode_or_passthrough now unwraps JSON Strings to their inner content per decode_or_passthrough_unwraps_string_value_into_text, so the assertion must read the raw "just a string". The two tests stay consistent.
andersonleal
added a commit
that referenced
this pull request
May 8, 2026
…orker (#103) * chore(harness/make): swap WORKERS list for consolidated shell * chore(harness/demo): swap WORKERS list for consolidated shell * feat(harness/demo): add shell-config.yaml and wire --config for shell * docs(harness): update ARCHITECTURE.md for shell worker consolidation * feat(harness): add fs module with build_inline_envelope helper (TDD'd) * fix(harness/fs): address clippy lints in build_inline_envelope Reflow module-level doc comment to fix doc_markdown line-length lint. Replace match-on-Result with .map_or_else() to satisfy clippy::map_unwrap_or. * feat(harness): add async read_inline driver for shell::fs::read * feat(harness): register harness::fs::read_inline on the bus * feat(harness/web): add FsLsResponse type for shell::fs::ls * feat(harness/web): migrate Composer ls call to shell::fs::ls * chore(harness/web): drop dead "io" error variant from AtBrowseState * feat(harness/web): migrate FilesystemPanel to shell::fs + harness::fs::read_inline * feat(harness/web): migrate palette sensitive patterns to shell::fs::* * test(harness/web): update tool_name fixtures to shell::fs::write * feat(harness/web): migrate App APPROVAL_REQUIRED + denylist comment to shell::fs::* * feat(harness): consolidate shell-bash + shell-filesystem into shell worker Replace the two old worker dependencies with the consolidated shell worker (v0.3.0) that exposes shell::* (process lifecycle) and shell::fs::* (filesystem) under one binary. Migrate every harness call-site, palette regex, denylist string, test fixture, and doc reference to the new namespace. Removed: - shell-bash/ and shell-filesystem/ crates (39 tracked files). - shell::bash::which, shell::bash::detect_clis, shell::filesystem::edit — no equivalents in v0.3.0, no harness callers. Harness Rust: - iii.worker.yaml: depend on shell ^0.3.0. - src/lib.rs: EXPECTED_WORKERS uses "shell"; new test asserts the swap; register harness::fs::read_inline; HarnessFunctionRefs gains a field. - src/fs.rs (new): build_inline_envelope (pure, TDD'd for UTF-8 happy path, truncation, binary marker) + async read_inline that wraps shell::fs::read, drains its StreamChannelRef under a 256 KiB cap, and returns the legacy {content:[{text}], details:{size, truncated, bytes_read}} envelope so the FilesystemPanel preview keeps working without channel-awareness leaking into the web. - Cargo.lock regenerated for the new iii-sdk channel usage. Build / scripts / docs: - Makefile + scripts/demo.sh: WORKERS list swap; comment fix; --config wiring for the shell worker (which refuses to boot unjailed by default). - shell-config.yaml (new, demo-only): allow_unjailed=true so the demo can browse the developer's machine; production deployments must pin fs.host_root. - ARCHITECTURE.md: bus diagram, function-id examples, worker table, expected-workers count (14→15), policy-denylist example (drops shell::filesystem::edit). Harness web (TypeScript): - types.ts: add FsLsResponse for the flat shell::fs::ls shape; widen FsEntry with optional is_dir/is_symlink; mark FsLsDetails @deprecated. - components/Composer.tsx: @-mention ls call → shell::fs::ls; drop the now-dead "io" error variant from AtBrowseState. - components/FilesystemPanel.tsx: ls → shell::fs::ls (flat); read → harness::fs::read_inline; entryKind prefers the new is_dir flag. - palette.ts / palette.test.ts: SENSITIVE_PATTERNS rename to shell::fs::*. - App.tsx: APPROVAL_REQUIRED + POLICY_DENIED_TOOLS comment updated. - reducer.test.ts / useStatus.test.ts: tool_name fixtures migrated. Out of scope: background-job functions (shell::exec_bg/kill/status/list) are available on the bus but not surfaced through harness palette, denylist, or UI. Deferred until we design for them. Verification: - cargo test --lib: 32 pass (incl. new EXPECTED_WORKERS swap test). - cargo test --test integration: 2 pass (incl. yaml↔lib.rs count match). - cargo clippy --lib -- -D warnings: clean. - pnpm tsc --noEmit (web): clean. - pnpm vitest run (web): 121 pass. * fix(harness): pass shell-config.yaml when spawning shell from Makefile The Makefile's _spawn-one rule launched every non-harness worker without flags. Shell's CLI default is `--config ./config.yaml`, so it tried to read the harness CWD's missing config.yaml, fell back to ShellConfig:: default(), and refused to start (host_root unset + allow_unjailed false is fail-closed by design). Adds an `elif shell` branch that mirrors what scripts/demo.sh:102-105 already does: pass --config workers/harness/shell-config.yaml. The two were out of sync. * fix(turn-orchestrator): rename stale shell:: ids to post-consolidation form The shell-bash + shell-filesystem consolidation (f0f7eec) renamed function ids to shell::exec / shell::fs::*, but turn-orchestrator's user-facing strings still pointed at the old names: - system_prompt.rs:8 — example `"shell::filesystem::ls"` shipped to every LLM turn via the system prompt. - agent_call.rs:37,43 — `agent_call` tool description's example id, sent to the model as part of the tool spec. Models that mimicked the example tried `shell::filesystem::ls`, hit `function_not_found`, and per the prompt's own instruction loaded the relevant skill via `skill::fetch` instead of calling shell directly. Also updates test data (states/tools.rs, run_start.rs, agent_call.rs test bodies) and renames the stale `shell::bash::exec` reference. All test assertions remain pass-through checks; renaming the strings keeps them aligned with the production examples. * fix(turn-orchestrator): unwrap JSON String values in decode_or_passthrough Functions like `skill::fetch` return a raw JSON String (the markdown body). `decode_or_passthrough` tried to deserialize that into a ToolResult struct (fails — strings aren't structs), fell through to the passthrough branch, and used `Value::to_string()` to derive the text. `serde_json::Value::Display` emits the JSON-encoded form: a string value with surrounding quotes and `\n` escape literals. The harness web's ToolResultBlock wraps `text` in a `<pre>`, so users saw the raw quoted form `"# iii://shell/fs_read\n\n..."` rendered verbatim instead of the formatted markdown body. The frontend was correct; the bug was in the Rust serializer. Match on `Value::String(s)` and use `s.clone()` directly. Non-String values (objects, arrays, numbers, bools, null) still go through `to_string()` because Display IS the right serialization there — that's how function-level error envelopes like `{ok: false, error}` round-trip. Adds regression test `decode_or_passthrough_unwraps_string_value_into_text` asserting no leading quote and no literal `\\n` in the text block. * chore: cargo fmt --all in harness, shell, turn-orchestrator Pre-existing rustfmt drift across the workers touched in this branch. Mechanical only: alphabetised `pub mod` ordering in harness/src/lib.rs; single-line wrapping vs multi-line in agent_call/run_start/states. No behaviour changes. * fix(turn-orchestrator): align decode_or_passthrough_handles_primitive_value with string-unwrap behavior The slim agent_call PR (#104) shipped a test asserting the pre-fix JSON-stringified form ("\"just a string\""). decode_or_passthrough now unwraps JSON Strings to their inner content per decode_or_passthrough_unwraps_string_value_into_text, so the assertion must read the raw "just a string". The two tests stay consistent.
5 tasks
ytallo
added a commit
that referenced
this pull request
May 8, 2026
…dization (#106) * refactor(auth-credentials): add manifest/config modules and standardize worker layout * refactor(harness): add manifest/config modules and standardize worker layout * refactor(hook-fanout): add manifest/config modules and standardize worker layout * refactor(llm-budget): add manifest/config modules and standardize worker layout * refactor(models-catalog): add manifest/config modules and standardize worker layout * refactor(policy-denylist): add manifest/config modules and standardize worker layout * refactor(provider-anthropic): add manifest/config modules and standardize worker layout * refactor(provider-openai): add manifest/config modules and standardize worker layout * refactor(provider-router): add manifest/config modules and standardize worker layout * refactor(session-inbox): add manifest/config modules and standardize worker layout * refactor(session-tree): add manifest/config modules and standardize worker layout * refactor(shell-bash): add manifest/config modules and standardize worker layout * refactor(shell-filesystem): add manifest/config modules and standardize worker layout * refactor(subagent): add manifest/config modules and standardize worker layout * refactor: rename function namespaces to match crate names - inbox::* -> session-inbox::* - hooks::publish_collect -> hook-fanout::publish_collect Updates README, registry/index.json, turn-orchestrator callsites, and whitelists per-worker config.yaml files in .gitignore. * refactor(subagent): enhance skill registration and update start command description - Introduced constants for skill ID, metadata, and sub-skills in `lib.rs`. - Updated the description in `start.rs` to remove an unnecessary argument. - Implemented skill registration with retry logic and graceful shutdown handling in `main.rs`. * fix(ci): clippy duration_suboptimal_units in retry/backoff loops Replace Duration::from_secs(180)/(60) and from_secs(3 * 60) with from_mins(3)/from_mins(1) in skill-register retry loops for auth-credentials, llm-budget, and session-tree. * fix(ci): cargo fmt subagent/src/lib.rs (SUB_SKILLS one-liner) * fix(ci): clippy duration_suboptimal_units in subagent retry loop * refactor: update trigger types and function IDs for durability - Changed trigger type from "subscribe" to "durable:subscriber" in multiple modules. - Updated function IDs to use the "iii::durable::publish" format for consistency across the codebase. * feat(harness): add approval-gate to EXPECTED_WORKERS + iii.worker.yaml Phase A item #3. Includes new test expected_workers_includes_approval_gate and the existing drift test passes. * feat(session-tree): add session-tree::list bus function with pagination + ordering * feat(session-tree): add entry_id to session-tree::messages response (BREAKING) * test(session-tree): bus-level integration tests for list + messages entry_ids * test(session-tree): serialize bus tests to avoid iii port collision * feat(session-tree): add session-tree::ensure for caller-supplied idempotent create * feat(turn-orchestrator): dual-write messages to session-tree (best-effort mirror) * test(turn-orchestrator): dual-write delta + lazy-create tests; abort mirror on parent-read failure * feat(session-tree): add session-tree::reconcile to repair state↔tree drift * feat(harness/web): add loadMessagesWithEntryIds helper with state::* fallback * feat(harness/web): rewrite reducer to entry-id-keyed Map; idempotent + replay-safe * test(harness): Phase A E2E acceptance — dual-write + fork + reconcile * feat(harness): add bridge::info, ui::subscribe/unsubscribe, fanout skeleton - New harness::fanout module: per-browser subscription registry (BrowserId -> {session_ids, all-sessions sentinel}). 4 unit tests cover per-session routing, all-sessions fallthrough, eviction, and global-only filtering. - New bridge::info function: returns the relative WS path (/iii/ws), protocol "ws", and the engine_url the harness was started with so reverse-proxy / HTTPS browser clients compose URLs from window.location while native callers still get a usable ws:// URL. - New ui::subscribe / ui::unsubscribe functions: add/remove a browser's interest in a session (or null = all sessions / non-session topics). Real subscribers (agent::events, state diffs, llm-budget, harness::status) wire in later steps. - HarnessFunctionRefs gains bridge_info, subscribe_fn, unsubscribe_fn; unregister_all updated. - register_with_iii_with_engine_url is the new entry point that takes the engine URL through; register_with_iii kept as a default-URL shim so the existing test surface stays unchanged. - iii-worker-manager is intentionally NOT added to EXPECTED_WORKERS: it is built into the iii engine itself (engine's builtin_defaults.rs), not a discrete worker crate. Comment added near EXPECTED_WORKERS to document the assumption. - New tests/bridge_info.rs integration test (engine-backed; auto-skips when no engine is reachable). * feat(harness/web): per-session workspace cwd with header field * feat(harness/web): headless useCommandMenu + menu items reducer (slash/at/history) * feat(harness/web): rewrite Composer with slash/at/history modes * feat(harness/web): bus function palette (Cmd-J) with JSON editor Cmd-J (Ctrl-J on Linux/Win) opens a global modal listing every function on the bus. Drill-in shows description + a raw JSON payload editor that posts through /bridge/trigger. Sensitive functions (policy::*, auth::set_*, shell filesystem writes) get a two-step Enter-to-confirm Send. Recent invocations (last 20) persist in localStorage so the next open pre-fills the payload. Files: - useGlobalShortcut.ts + test: cross-platform Cmd/Ctrl binder with pure matchesShortcut predicate so the matching logic is testable in node env. - palette.ts + test: isSensitive, workerFromId, filterPalette (mirrors the menuItems.ts ranking — substring beats fuzzy), loadRecent / pushRecent (dedupe by function_id, cap at 20, survives quota errors), curlForBridge. - components/FunctionPalette.tsx: list view with Functions/Recent tabs, arrow-key nav, ARIA dialog/listbox/option, focus trap (filter focused on open, prior focus restored on close), drill-in with JSON textarea, response pre, copy-as-curl, sensitive confirm flow, prefers-reduced-motion respect. - App.tsx: wires paletteOpen state + useGlobalShortcut + footer hint. - styles.css: .palette-* classes (backdrop, list, drill, sensitive chip). * feat(harness/web): migrate session reads to session-tree + loadMessagesWithEntryIds; group + subtitle Switch the rail to session-tree::list (with state::list drift fallback when total === 0), group rows by Today/Yesterday/Earlier, and render the workspace cwd as a left-truncated subtitle. Per-message kebab actions render fork + copy; fork is disabled with a tooltip when entry_id is null (drift case). - sessions.ts: pure groupByDate + truncatePath helpers (vitest covered). - export.ts: client-side exportMd/exportJson via Blob downloads of session-tree::messages and session-tree::tree responses. - MessageActions.tsx: per-message kebab; copy uses navigator.clipboard. - SessionList.tsx: session-tree::list reader + grouped sections + subtitle. - SessionView.tsx: thread MessageActions through with parallel entry_ids. - types.ts: SessionRow gains optional cwd + last_message_summary. - styles.css: .session-group(s|-h), .session-subtitle, .msg-action(s). * feat(harness/web): /repair, /fork, /export md|json + fork-from-message App.tsx swaps the message loader to loadMessagesWithEntryIds and tracks a parallel messageEntryIds array so fork buttons know which messages have real entry_ids. Stream-driven updates pad the array with null until WS migration (step B) carries entry_ids on the wire. - /repair calls session-tree::reconcile with the state::* snapshot, then reloads to surface the new entry_ids. - /fork forks from the most recent message with an entry_id. - /export md|json downloads via the new export module. - Per-message MessageActions wires onForkFromMessage which calls session-tree::fork and switches the active session to the new id. - menuItems: /repair, /fork, /export md, /export json built-ins. - Composer: dispatcher + slash-menu accept paths for the new commands. * feat(harness/web): add iii-client + useConnection over iii-browser-sdk * feat(harness): wire harness-ui-fanout subscribers (agent events + sessions changed) * feat(harness/web): replace SSE + 4s poll with WS-pushed handlers * feat(harness): fanout pushes for approvals + cost + workers with backpressure Adds three new long-lived fanout pumps and per-browser backpressure to the existing harness fanout: - spawn_approval_poll: 1s poll of approval::list_pending across all known sessions; emits ui::approval::requested / ui::approval::resolved on diff. - spawn_cost_poll: 2s poll of budget::list, sums spent_usd into a daily total + by-period breakdown, emits ui::cost::tick on change. The designed llm-budget::summary function does not exist in the worker, so the summary is synthesized client-side; this is the closest equivalent. - spawn_workers_poll: 5s poll of engine::workers::list, joined against EXPECTED_WORKERS to mark missing workers as down; emits ui::workers::changed on diff. Backpressure is enforced per-browser: - AtomicU64 in_flight counter capped at 1024; on overflow we roll back the failed push, emit a single deduped ui::session::resync, and wait. - Cost ticks gated to ≥100ms apart per browser (≤10/s) via a Mutex<Option<Instant>>. Pure helpers (should_emit_cost_tick, diff_workers, diff_approvals, summarize_budgets, extract_worker_status) are pub(crate) and unit-tested without spawning tasks. All four required Step E tests pass: - fanout_pump_coalesces_cost_ticks_to_10_per_second - fanout_pump_drops_oldest_and_emits_resync_on_overflow - fanout_approval_pump_emits_resolved_on_removal - fanout_workers_poll_diffs_correctly * feat(harness/web): live status strip + status tab Adds the live status surface backed by Step E's fanout pushes: - useStatus: subscribes once per page to ui::approval::requested, ui::approval::resolved, ui::cost::tick, ui::workers::changed and owns the rolling 200-event buffer. Returns {pendingApprovals, cost, workers, events, hydrated, clearEvents}. - StatusStrip (header): single Approvals chip with count, pulses on new approvals, grays out + shows reconnecting dot when WS drops, click jumps to the status tab. - FootStatus (app-foot): cost USD + workers N/M up chips. Compact, mono, color-tone matches state (ok/warn/muted) plus reconnect dot. - StatusTab: workers table (text status for a11y), inline 80x16 SVG cost sparkline, hydrated budget breakdown via budget::list + budget::usage, rolling events feed with filter chips (all/approval/cost/workers) + pause/resume/clear, "live updates paused" banner after >5s disconnect with auto re-hydration on reconnect. App.tsx adds the new "status" tab to the existing tab nav, the right- side header group with StatusStrip + StatusPill, and the FootStatus chips in the footer. styles.css adds chip / table / feed / sparkline styles tuned to the existing design tokens. 10 useStatus tests cover subscription wiring, dedup, snapshot replacement, malformed ignore, 200-entry cap, clear, hydrated flag, and unsubscribe on unmount. Full vitest suite: 121 pass. * chore(harness): add approval-gate to demo.sh WORKERS; commit session-tree Cargo.lock Phase A added approval-gate to EXPECTED_WORKERS + iii.worker.yaml but missed the demo.sh script. Surfaced when starting the live demo: 15 workers spawned instead of 16. session-tree Cargo.lock catches up with the serial_test + which dev-deps added in Phase A. * chore(harness/web): tsc noEmit so Vite owns the JS emit `npm run build` is `tsc && vite build`. Without noEmit, tsc emits compiled .js next to every .ts/.tsx, and Vite's resolver picks them over the .tsx sources on subsequent dev runs. End-to-end debugging surfaced this as crashes in components running pre-Phase-B compiled output. * fix(harness/web): agent_end shape, dedupe, and main layout Three bugs surfaced by end-to-end testing the live harness: - AgentEvent.agent_end.messages widened to (AgentMessage | {entry_id?, message})[]: backend (turn-orchestrator/crates/harness-types/src/agent_event.rs) emits bare AgentMessage[]. Reducer now detects bare-vs-wrapped per item and routes accordingly. Defensive guard in pushUnkeyed rejects anything missing a role so future similar mismatches degrade silently instead of crashing the tree. - Content-hash dedupe in pushUnkeyed (role:timestamp:content-length, matches pre-Phase-B approach). Without it, message_end (per-message during the turn) + agent_end (full transcript at end) double every reply. - .main switched from grid grid-template-rows: auto 1fr auto auto to flex column with .view as flex: 1. The grid template only described 4 rows but the chat tab renders 8 children, so the 1fr landed on ControlsBar and the controls strip stretched to fill the viewport with the chips visually centered in empty space. * feat: iii-native harness (agent_call dispatcher) - Add agent_call module: single LLM tool schema (AgentTool-shaped), schema cache + JSON Schema payload validation, lazy sandbox for shell::*, shared dispatch, register agent::call on the bus. - Unwrap agent_call in handle_prepare; route handle_execute through dispatch (never hold MutexGuard across await; warm cache after engine list fetch). - Provisioning: persist one-tool catalog, server-side system prompt + optional skill::fetch for iii://skills; drop eager sandbox provisioning. - Harness web: remove client prompt/skills hook; pass cwd on run::start. - Docs: TODOS.md and harness/ARCHITECTURE.md updates per plan. * feat(harness): add Makefile for local demo harness operations - Introduced a Makefile to streamline local demo harness operations, providing targets for building, starting, verifying, and managing worker processes. - Added commands for logging and web server management, enhancing usability for development and testing. - The Makefile resolves workspace paths and supports environment variable overrides for flexibility in configuration. * iii-native harness: slim agent_call + Tier 2 polish (#104) * refactor(turn-orchestrator): slim agent_call to thin pass-through Tier 2 dispatcher per spec docs/superpowers/specs/2026-05-07-tier2-iii-pure-harness-design.md. Deleted: SchemaCache, format_to_input_schema, validate_payload, sandbox helpers (sandbox_alive, sandbox_decision, ensure_sandbox, etc.). The dispatcher now only validates the function field, calls iii.trigger, and maps errors. Skills (registered separately via the skills worker) will teach the LLM iii contracts — registry introspection, sandbox lifecycle. agent_call.rs: ~700 lines → ~250 lines. Tests: 72 → 56. * fix(turn-orchestrator): drop stale ensure_sandbox comment The comment in provisioning.rs referenced agent_call::ensure_sandbox, which was deleted in the Tier 2 thin-dispatcher refactor. Sandbox provisioning is now the LLM's responsibility via a skill recipe. * chore(turn-orchestrator): drop jsonschema dep No longer used after Tier 2 dispatcher refactor — payload validation is gone. * feat(harness/scripts): demo policy-denylist denies bridge::trigger Closes the recursion bypass identified in plan-eng-review D1: without this entry, the LLM can call bridge::trigger via agent_call to dispatch any function and bypass name-matched policy rules (the agent::before_ tool_call hook fires with name 'bridge::trigger', not the inner function id). * docs(TODOS): update helper-extraction TODO post-Tier 2 The harness copy is gone — only mcp's remains. TODO now flags the extraction as dormant until a second caller appears. * docs(harness): describe agent_call as thin pass-through Reflects Tier 2 refactor: dispatcher no longer validates payloads, provisions sandboxes, or queries the registry. Skills (registered via the skills worker) carry iii contract knowledge. Trust-boundary section updated to require bridge::trigger in POLICY_DENIED_TOOLS. * test: adversarial unit tests for Tier 2 surface Adds 13 green tests pinning behavior on every CI build, plus 4 #[ignore]'d red tests that document real latent bugs (substring matching false-positives, missing-bracket parser bug, handle_finalize panic on missing last_assistant) for follow-up fix PRs. turn-orchestrator/src/agent_call.rs: 7 green + 2 red - validate_function_field rejects Object/Array/Float Value shapes - decode_or_passthrough handles array, primitive, and partial-ToolResult inputs - TOOL_NAME / FUNCTION_ID constants pinned (wire contract) - Red: is_function_not_found and is_timeout misclassify user content containing the magic substrings turn-orchestrator/src/system_prompt.rs: 3 green - cwd newlines pass through verbatim - skills_index markdown passes through verbatim - 1MB override returns same length (no implicit truncation) turn-orchestrator/src/states/tools.rs: 2 green + 1 red - TOPIC_BEFORE / TOPIC_AFTER constants pinned (policy-gate wire contract) - before_tool_call payload shape pinned (subscriber contract) - Red: handle_finalize source-grep test for the .expect() panic on missing last_assistant policy-denylist/src/main.rs: 3 green + 1 red - parse_denied_tools handles empty / whitespace / json-array syntax - Red: malformed-bracket inputs (missing open or close) leak the bracket into the parsed tool name; the operator's intended denial never matches Plan: ~/.claude/plans/let-s-implement-more-tests-refactored-flask.md. After this lands: turn-orchestrator 56 → 68 lib tests, policy-denylist gains 3. CI signal strengthens; #[ignore]'d tests don't block. * fix(turn-orchestrator): structured error matching in agent_call dispatch Replace stringly-typed substring matching in is_function_not_found and is_timeout with pattern matches on iii_sdk::IIIError variants. The old heuristic misclassified inner errors whose payload happened to contain the magic substring (e.g. an IIIError::Handler with a log line that mentions "function_not_found", or "scheduled timeout in 30 days"). is_function_not_found now matches IIIError::Remote { code, .. } where code == "function_not_found" — the canonical engine signal (iii.rs:1701). is_timeout matches IIIError::Timeout — the SDK's structured timeout variant (iii.rs:1155). Updates the existing detector tests to construct IIIError variants and flips the two #[ignore]'d red tests green. * fix(turn-orchestrator): graceful handle_finalize when last_assistant missing Replace .expect("tools state requires last_assistant…") with a let-else that warns and gracefully transitions to TearingDown. The state machine should only enter ToolFinalize from AssistantFinished (which always populates last_assistant), but a resume after a mid-turn crash or a persistence corruption could land here. Panicking in that branch tears the worker down hard instead of ending the turn cleanly; tool_results are still persisted. Flips the source-grep regression guard green. * fix(policy-denylist): reject unmatched brackets in parse_denied_tools Previously the parser used a strip_prefix('[').and_then(strip_suffix(']')) chain that fell back to the raw input on a single missing bracket, leaking the unmatched bracket into the first or last token. An operator who set POLICY_DENIED_TOOLS=[bridge::trigger ended up denying the literal "[bridge::trigger" — never matching the real tool name. Silent policy bypass on the denylist worker. Now: brackets must be paired (strip both, or strip neither). Unmatched brackets log a warning and fall back to a bracket-tolerant trim so the rest of the input still parses cleanly. Flips the malformed-bracket regression test green. * style(turn-orchestrator): apply cargo fmt Three pre-existing fmt nits flagged by CI's cargo fmt --check: - agent_call.rs:176 — collapse multi-line .get/.cloned/.unwrap_or chain - provisioning.rs:20 — collapse split let binding - tools.rs:340 — split long tc(...) call across multiple lines No behavioral change; unblocks the lint+test CI job. * fix(harness/tests): satisfy clippy::map_unwrap_or in nonce helper CI's `cargo clippy --all-targets -- -D warnings` flagged the .map(...).unwrap_or(0) chain in tests/common/mod.rs::nonce as clippy::map_unwrap_or. Collapse to .map_or(0, ...). No behavior change. * refactor: rename tool→function + browser eviction + Makefile cleanup (#105) * refactor(harness-types): rename tool-related types to function-related types Updated the ContentBlock and AgentMessage enums to replace ToolCall and ToolResult with FunctionCall and FunctionResult, respectively. Adjusted serialization/deserialization to maintain backward compatibility with legacy tool names. Refactored related tests and state management to reflect these changes, ensuring the system correctly handles function calls and results. This enhances clarity and aligns with the new function-based architecture. * refactor(harness): update Makefile and remove obsolete components Modified the Makefile to ensure the engine process writes its PID to the correct location. Removed the ToolResultBlock and ToolUseBlock components from the web interface, as they are no longer needed. This streamlines the codebase in preparation for a function-based architecture, enhancing maintainability and clarity. * feat(harness): implement browser eviction and error handling improvements Added a new function `is_function_not_found` to check for specific error types related to unregistered functions. Implemented `evict_browser` to remove all sessions associated with a browser when its handler is no longer available. Updated the event handling logic to trigger browser eviction on encountering a "function not found" error. Enhanced tests to cover the new eviction functionality and error matching, ensuring robust handling of browser states and error scenarios. * feat(harness/web): enhance function call and result display with markdown support - Added a new `Markdown` component to render markdown content in the UI, utilizing `react-markdown` and `remark-gfm` for enhanced formatting. - Updated `FunctionCallBlock` to display function names dynamically based on arguments. - Enhanced `FunctionResultBlock` to detect output format (JSON, markdown, or text) and render accordingly, with support for expandable views. - Modified `SessionView` to integrate markdown rendering for assistant messages, improving readability and presentation. - Adjusted CSS styles for better layout and spacing in message components. * style: apply cargo fmt across crates touched by tool→function rename The rename touched many lines across vendored harness-types copies and the orchestrator state machine, leaving formatting that drifted from rustfmt's preference. Pure formatting; no behavior change. * fix(ci): track harness/docs and silence excessive-bools on route() - harness/src/lib.rs include_str!s docs/iii-skill.md and docs/sandbox-skill.md, but the root .gitignore globally ignored `docs`, so the files were never tracked and CI failed compilation. Add a narrow `!harness/docs/` exception and check the two markdown files in. - steering::route takes four bools (abort + three queue checks). Each is an independent precondition checked in priority order; collapsing into a struct or bitflag obscures the call site without buying anything. Allow clippy::fn_params_excessive_bools with a comment. * fix(harness): add iii-sandbox to iii.worker.yaml dependencies Commit 1717655 added "iii-sandbox" to EXPECTED_WORKERS in lib.rs but left iii.worker.yaml untouched, so integration test expected_workers_matches_yaml_dependency_count failed (16 vs 17). Add the missing dependency line. * feat(harness): consolidate shell-bash + shell-filesystem into shell worker (#103) * chore(harness/make): swap WORKERS list for consolidated shell * chore(harness/demo): swap WORKERS list for consolidated shell * feat(harness/demo): add shell-config.yaml and wire --config for shell * docs(harness): update ARCHITECTURE.md for shell worker consolidation * feat(harness): add fs module with build_inline_envelope helper (TDD'd) * fix(harness/fs): address clippy lints in build_inline_envelope Reflow module-level doc comment to fix doc_markdown line-length lint. Replace match-on-Result with .map_or_else() to satisfy clippy::map_unwrap_or. * feat(harness): add async read_inline driver for shell::fs::read * feat(harness): register harness::fs::read_inline on the bus * feat(harness/web): add FsLsResponse type for shell::fs::ls * feat(harness/web): migrate Composer ls call to shell::fs::ls * chore(harness/web): drop dead "io" error variant from AtBrowseState * feat(harness/web): migrate FilesystemPanel to shell::fs + harness::fs::read_inline * feat(harness/web): migrate palette sensitive patterns to shell::fs::* * test(harness/web): update tool_name fixtures to shell::fs::write * feat(harness/web): migrate App APPROVAL_REQUIRED + denylist comment to shell::fs::* * feat(harness): consolidate shell-bash + shell-filesystem into shell worker Replace the two old worker dependencies with the consolidated shell worker (v0.3.0) that exposes shell::* (process lifecycle) and shell::fs::* (filesystem) under one binary. Migrate every harness call-site, palette regex, denylist string, test fixture, and doc reference to the new namespace. Removed: - shell-bash/ and shell-filesystem/ crates (39 tracked files). - shell::bash::which, shell::bash::detect_clis, shell::filesystem::edit — no equivalents in v0.3.0, no harness callers. Harness Rust: - iii.worker.yaml: depend on shell ^0.3.0. - src/lib.rs: EXPECTED_WORKERS uses "shell"; new test asserts the swap; register harness::fs::read_inline; HarnessFunctionRefs gains a field. - src/fs.rs (new): build_inline_envelope (pure, TDD'd for UTF-8 happy path, truncation, binary marker) + async read_inline that wraps shell::fs::read, drains its StreamChannelRef under a 256 KiB cap, and returns the legacy {content:[{text}], details:{size, truncated, bytes_read}} envelope so the FilesystemPanel preview keeps working without channel-awareness leaking into the web. - Cargo.lock regenerated for the new iii-sdk channel usage. Build / scripts / docs: - Makefile + scripts/demo.sh: WORKERS list swap; comment fix; --config wiring for the shell worker (which refuses to boot unjailed by default). - shell-config.yaml (new, demo-only): allow_unjailed=true so the demo can browse the developer's machine; production deployments must pin fs.host_root. - ARCHITECTURE.md: bus diagram, function-id examples, worker table, expected-workers count (14→15), policy-denylist example (drops shell::filesystem::edit). Harness web (TypeScript): - types.ts: add FsLsResponse for the flat shell::fs::ls shape; widen FsEntry with optional is_dir/is_symlink; mark FsLsDetails @deprecated. - components/Composer.tsx: @-mention ls call → shell::fs::ls; drop the now-dead "io" error variant from AtBrowseState. - components/FilesystemPanel.tsx: ls → shell::fs::ls (flat); read → harness::fs::read_inline; entryKind prefers the new is_dir flag. - palette.ts / palette.test.ts: SENSITIVE_PATTERNS rename to shell::fs::*. - App.tsx: APPROVAL_REQUIRED + POLICY_DENIED_TOOLS comment updated. - reducer.test.ts / useStatus.test.ts: tool_name fixtures migrated. Out of scope: background-job functions (shell::exec_bg/kill/status/list) are available on the bus but not surfaced through harness palette, denylist, or UI. Deferred until we design for them. Verification: - cargo test --lib: 32 pass (incl. new EXPECTED_WORKERS swap test). - cargo test --test integration: 2 pass (incl. yaml↔lib.rs count match). - cargo clippy --lib -- -D warnings: clean. - pnpm tsc --noEmit (web): clean. - pnpm vitest run (web): 121 pass. * fix(harness): pass shell-config.yaml when spawning shell from Makefile The Makefile's _spawn-one rule launched every non-harness worker without flags. Shell's CLI default is `--config ./config.yaml`, so it tried to read the harness CWD's missing config.yaml, fell back to ShellConfig:: default(), and refused to start (host_root unset + allow_unjailed false is fail-closed by design). Adds an `elif shell` branch that mirrors what scripts/demo.sh:102-105 already does: pass --config workers/harness/shell-config.yaml. The two were out of sync. * fix(turn-orchestrator): rename stale shell:: ids to post-consolidation form The shell-bash + shell-filesystem consolidation (f0f7eec) renamed function ids to shell::exec / shell::fs::*, but turn-orchestrator's user-facing strings still pointed at the old names: - system_prompt.rs:8 — example `"shell::filesystem::ls"` shipped to every LLM turn via the system prompt. - agent_call.rs:37,43 — `agent_call` tool description's example id, sent to the model as part of the tool spec. Models that mimicked the example tried `shell::filesystem::ls`, hit `function_not_found`, and per the prompt's own instruction loaded the relevant skill via `skill::fetch` instead of calling shell directly. Also updates test data (states/tools.rs, run_start.rs, agent_call.rs test bodies) and renames the stale `shell::bash::exec` reference. All test assertions remain pass-through checks; renaming the strings keeps them aligned with the production examples. * fix(turn-orchestrator): unwrap JSON String values in decode_or_passthrough Functions like `skill::fetch` return a raw JSON String (the markdown body). `decode_or_passthrough` tried to deserialize that into a ToolResult struct (fails — strings aren't structs), fell through to the passthrough branch, and used `Value::to_string()` to derive the text. `serde_json::Value::Display` emits the JSON-encoded form: a string value with surrounding quotes and `\n` escape literals. The harness web's ToolResultBlock wraps `text` in a `<pre>`, so users saw the raw quoted form `"# iii://shell/fs_read\n\n..."` rendered verbatim instead of the formatted markdown body. The frontend was correct; the bug was in the Rust serializer. Match on `Value::String(s)` and use `s.clone()` directly. Non-String values (objects, arrays, numbers, bools, null) still go through `to_string()` because Display IS the right serialization there — that's how function-level error envelopes like `{ok: false, error}` round-trip. Adds regression test `decode_or_passthrough_unwraps_string_value_into_text` asserting no leading quote and no literal `\\n` in the text block. * chore: cargo fmt --all in harness, shell, turn-orchestrator Pre-existing rustfmt drift across the workers touched in this branch. Mechanical only: alphabetised `pub mod` ordering in harness/src/lib.rs; single-line wrapping vs multi-line in agent_call/run_start/states. No behaviour changes. * fix(turn-orchestrator): align decode_or_passthrough_handles_primitive_value with string-unwrap behavior The slim agent_call PR (#104) shipped a test asserting the pre-fix JSON-stringified form ("\"just a string\""). decode_or_passthrough now unwraps JSON Strings to their inner content per decode_or_passthrough_unwraps_string_value_into_text, so the assertion must read the raw "just a string". The two tests stay consistent. * fix(provider-anthropic): merge parallel function results into single user message Anthropic rejects requests where parallel tool_use IDs from an assistant turn are answered across multiple user messages: messages.N: tool_use ids were found without tool_result blocks immediately after: <id1>, <id2>, <id3> The wire converter previously emitted one user message per FunctionResult, which broke any turn where the model issued parallel tool calls. Fix: accumulate consecutive FunctionResults and flush them as a single user message containing one tool_result block per result. Test parallel_function_results_collapse_into_one_user_message reproduces the wire shape (3 results -> 1 user message with 3 tool_result blocks). * style(provider-anthropic): cargo fmt the parallel-results regression test --------- Co-authored-by: Anderson Leal <andersonofl@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was implemented
Slims the
agent_calldispatcher inturn-orchestratorto a thin pass-through, fixes 3 latent bugs surfaced during the audit, and cleans up the surface left after.Dispatcher slim (
turn-orchestrator/src/agent_call.rs)agent_call.rsgoes from ~700 → ~250 lines. The dispatcher now does only what skills can't do for themselves:functionfield on the LLM call.iii.triggerwith the payload.function_not_found,timeout, generic) back toToolResultenvelopes the model can read.Removed:
SchemaCache+engine::functions::listround-trip and the 30s TTL cache.format_to_input_schema+validate_payload(no more in-dispatcher payload validation againstrequest_format).sandbox_alive,sandbox_decision,ensure_sandbox,sandbox_list_contains_id).Sandbox provisioning and registry introspection move to the LLM via skill recipes (registered separately by the skills worker). Sessions that never need a sandbox no longer pay the create cost.
Bug fixes (3 latent bugs found during the audit)
fix(turn-orchestrator): structured error matching inagent_calldispatch.is_function_not_foundandis_timeoutpreviously substring-matched on the error'sDisplayoutput, so any inner error whose message happened to mention "function_not_found" or "timeout" was misclassified into the wrong envelope. They now pattern-match oniii_sdk::IIIErrorvariants —Remote { code: "function_not_found", .. }andTimeout— the canonical engine signals.fix(turn-orchestrator): gracefulhandle_finalizewhenlast_assistantis missing. Replaces the.expect("tools state requires last_assistant…")panic with alet-elsethat warns and transitions toTearingDown. Tool results are still persisted; the lifecycle events tied tolast_assistantare skipped. The state machine should never reach this branch in practice, but a resume after a mid-turn crash no longer brings the worker down hard.fix(policy-denylist): reject unmatched brackets inparse_denied_tools. The old parser usedstrip_prefix('[').and_then(strip_suffix(']'))and fell back to the raw input when one bracket was missing, leaking the unmatched character into the first or last token. An operator who setPOLICY_DENIED_TOOLS=[bridge::triggerwas actually denying the literal[bridge::trigger, silently letting the real call through — a denylist bypass. Brackets must now be paired; unmatched brackets log a warning and fall back to a bracket-tolerant trim.Cleanups that fall out of the slim
chore(turn-orchestrator): dropjsonschemadep — unused after payload validation removal (Cargo.lock shrinks ~260 lines).fix(turn-orchestrator/states/provisioning.rs): rewrite stale// Sandbox provisioning moves to agent_call::ensure_sandbox…comment that referenced deleted code.feat(harness/scripts/demo.sh): extend the demo to exercisepolicy-denylistdenyingbridge::trigger. Without this denial the LLM can dispatch any function viabridge::triggerand bypass name-matched policy rules (theagent::before_tool_callhook seesbridge::trigger, not the inner function id).docs(harness/ARCHITECTURE.md): describeagent_callas a thin pass-through.docs(TODOS.md): refresh helper-extraction TODO to reflect the slimmed surface.What was tested
13 new green unit tests pinning the slimmed surface. The 4
#[ignore]'d red tests added during the audit are now green — the bugs they documented are fixed by the three commits above. Existing integration suite (incl.dual_write) continues to pass.turn-orchestrator/src/agent_call.rs— 9 greenvalidate_function_fieldrejectsValue::Object,Value::Array, andValue::Number(float)shapes.decode_or_passthroughhandles array inputs, primitive inputs, and partialToolResultshapes (passthrough preserves the original envelope).TOOL_NAMEandFUNCTION_IDconstants pinned — wire contract with the provider tool list and the iii bus name.is_timeout_recognizes_structured_timeout_variantandfunction_not_found_detector_matches_engine_error_code(rewritten to useIIIErrorvariants).is_function_not_found_ignores_substring_in_other_variantsandis_timeout_ignores_substring_in_other_variants(formerly#[ignore]'d red tests, now green):IIIError::Handler/IIIError::Runtime/IIIError::Remote { code: "invocation_failed", .. }payloads that mention the magic substrings are no longer misclassified.turn-orchestrator/src/system_prompt.rs— 3 greencwdcontaining newlines passes through verbatim (no normalization).skills_indexmarkdown passes through verbatim (no escaping or truncation).system_prompt_overridereturns the same length — no implicit truncation.turn-orchestrator/src/states/tools.rs— 3 greenTOPIC_BEFORE/TOPIC_AFTERconstants pinned — policy-gate wire contract withpolicy-denylist.before_tool_callpayload shape pinned (subscriber contract:tool_name,tool_input,session_id,turn_id).handle_finalize_does_not_expect_last_assistant(formerly#[ignore]'d, now green): source-grep regression guard ensuring the.expect("tools state requires last_assistant…")panic path stays removed.policy-denylist/src/main.rs— 4 greenparse_denied_toolshandles empty input, whitespace-only input, and JSON-array syntax (["a","b"]).parse_denied_tools_handles_malformed_unclosed_bracket(formerly#[ignore]'d, now green): unmatched-bracket inputs ([tool1,tool2ortool1,tool2]) parse to clean tool names instead of leaking the bracket character.Verification
cargo test --libclean acrossturn-orchestrator(71 lib tests) andpolicy-denylist(8 lib tests).cargo test --testsclean (77 tests total inturn-orchestratorincl.dual_writeintegration; 23 inpolicy-denylist).cargo clippy --tests -- -D warningsclean.harness/scripts/demo.sh:bridge::triggerdenial fires end-to-end.