feat(harness): consolidate shell-bash + shell-filesystem into shell worker - #103
Merged
andersonleal merged 21 commits intoMay 8, 2026
Merged
Conversation
|
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
approved these changes
May 7, 2026
sergiofilhowz
requested changes
May 7, 2026
sergiofilhowz
left a comment
sergiofilhowz
left a comment
Contributor
There was a problem hiding this comment.
why are we deleting these two workers?
andersonleal
force-pushed
the
feat/harness-shell-consolidation
branch
from
May 7, 2026 21:16
823b0cf to
f0f7eec
Compare
sergiofilhowz
approved these changes
May 7, 2026
andersonleal
force-pushed
the
feat/harness-shell-consolidation
branch
2 times, most recently
from
May 8, 2026 15:03
f5e59a1 to
e5a5b87
Compare
ytallo
approved these changes
May 8, 2026
sergiofilhowz
approved these changes
May 8, 2026
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.
…orker
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.
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.
…n 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.
…rough
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.
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.
…_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
force-pushed
the
feat/harness-shell-consolidation
branch
from
May 8, 2026 16:31
c5b2608 to
a8d5e88
Compare
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
3 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.
Summary
Replace the two old worker dependencies (
shell-bash,shell-filesystem) with the consolidatedshellworker (v0.3.0) that exposesshell::*(process lifecycle) andshell::fs::*(filesystem) under one binary. Migrates every harness call-site, palette regex, denylist string, test fixture, and doc reference to the new namespace.55 files changed, +577 / −7917. Net deletions because the two old worker crates are removed.
What changes
shell-bash/andshell-filesystem/crates (39 files). Also dropsshell::bash::which,shell::bash::detect_clis,shell::filesystem::edit— no equivalents in v0.3.0, no harness callers.iii.worker.yamldeps swap,Makefile+scripts/demo.shWORKERSlist swap,--configwiring for the shell worker (which refuses to boot unjailed by default), new demo-onlyharness/shell-config.yamlwithallow_unjailed: true,ARCHITECTURE.mdtext + diagram + worker-count update.EXPECTED_WORKERSuses\"shell\"; new test asserts the swap. Newharness/src/fs.rswith:build_inline_envelope(pure helper, TDD'd: UTF-8 happy path, truncation, binary marker fallback).harness::fs::read_inlineasync driver — wrapsshell::fs::read, drains itsStreamChannelRefserver-side under a 256 KiB cap, returns the legacy{content:[{text}], details:{size, truncated, bytes_read}}envelope so the FilesystemPanel preview keeps working without channel-awareness leaking into the web bridge.FsLsResponsefor the flatshell::fs::lsshape;FsEntrywidened with optionalis_dir/is_symlink; FilesystemPanelreadswitches toharness::fs::read_inline;palette.tsSENSITIVE_PATTERNS,App.tsxAPPROVAL_REQUIRED, and test fixtures all migrated toshell::fs::*.shell::exec_bg/kill/status/list) are exposed by the new worker but not surfaced through the harness palette/denylist/UI. Deferred until we design for them.Why
One worker, one binary, one config — eliminates the dual
shell-bash/shell-filesystemboundary that didn't carry its weight (no separate enforcement surface, no separate lifecycle). The new worker also exposes background-job lifecycle that we can adopt in a follow-up without re-plumbing.Test plan
Risks
Notes for review
This branch is one squashed commit (`f0f7eec`). Original 19-commit history (Tasks 1–17 of the implementation plan + 2 cleanup commits + Cargo.lock regen) is preserved in local reflog if you'd prefer to review the granular sequence.