feat(itmux): the run contract - AgentRunSpec -> AgentRunResult over itmux (Rust)#247
Open
NeuralEmpowerment wants to merge 9 commits into
Open
feat(itmux): the run contract - AgentRunSpec -> AgentRunResult over itmux (Rust)#247NeuralEmpowerment wants to merge 9 commits into
NeuralEmpowerment wants to merge 9 commits into
Conversation
Adds the Plan B Task 1 language-neutral JSON contract for the future `itmux run` subcommand: AgentRunSpec (request), AgentRunResult (final response), and the AgentRunEvent JSONL envelope streamed on stdout during a run. - recipe is a directory PathBuf only in v1, no inline variant (R4). - AgentRunEvent flattens run_id/seq/ts with a `type`-tagged payload enum (tool_start/tool_end/token_usage/session_end) per R6. - All structs use deny_unknown_fields. AgentRunEvent itself cannot carry deny_unknown_fields directly (serde forbids combining it with #[serde(flatten)]); unknown-field rejection is preserved instead via deny_unknown_fields on each AgentRunEventPayload variant, proven by tests/contract_json.rs::agent_run_event_rejects_unknown_top_level_field. - Added schemars (justified: derives JSON Schema directly from the serde structs, keeping the schema and the Rust types from drifting apart; no unsafe code, small dep footprint) and an `emit_contract_schema` example that regenerates docs/contract/*.schema.json from the structs. Tests: tests/contract_json.rs (20 cases) covering round-trip serialization for every struct, deny_unknown_fields rejection, and the AgentRunEvent envelope for all four payload variants including a 4-line JSONL stream ending in session_end. Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
A live standalone eval found that bare `claude` drops its first
keystrokes after launch: the task prompt sent immediately after the TUI
starts never lands (captured pane shows an empty prompt). Codex tolerates
the timing via its launch dance; claude has no settle-step. Pane
stability ("ready") is not the same as the input box being input-ready.
Make submit input-readiness a PER-HARNESS adapter concern (harness-neutral,
plan R8), never a special-case in generic code:
- `needs_submit_verification(agent)` - claude opts in; codex/gemini submit
one-shot (unchanged, byte-for-byte).
- `input_reflects_submission(agent, pane, text)` - per-harness predicate
that checks the typed prompt landed in the input line (whitespace-
normalised fragment match, survives TUI wrapping).
- `run_submit_protocol(...)` - the generic, harness-NEUTRAL runner: for a
verifying harness it types -> waits -> captures -> retries (bounded,
clearing the input with C-u before each re-type so a partial land never
duplicates) -> submits; for a one-shot harness it types once and submits.
It calls only the per-harness functions, no harness-name branches, so it
is fully unit-testable with injected fakes.
A future harness (pyagent/opencode) plugs in by answering the two
per-harness functions, with zero caller changes.
Tests (tests/submit_readiness.rs, auth-independent - no docker/token):
claude retries once then lands and submits exactly once; claude exhausts
bounded retries yet still submits (never wedged); codex sends once with no
capture/sleep (one-shot path still works); plus the per-harness predicate
and verification-flag units.
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
…ping
Plan B Task 2: consume the settled APSS recipe crate
`apss-v1-0005-agent-recipe` and map a recipe directory onto itmux start
args + submit text.
- New `src/run/recipe_loader.rs`: `load_execution_plan(&AgentRunSpec)`
calls the APSS `load_recipe_dir`, resolves `default_agent`, and builds a
`RecipeExecutionPlan { recipe_name, agent, claude_plugin_dirs,
submit_text, subagents }`.
- agent: AgentKind::Claude -> Agent::Claude, Codex -> Agent::Codex.
- claude_plugin_dirs: the default agent's `skills` resolved to plugin-dir
paths in listed order (R3): `<recipe>/skills/<ref>/` if it exists, else
the ref verbatim.
- submit_text: `build_submit_text(resolved_system(agent, SYSTEM.md), task)`
- the resolved system prompt (append/replace handled by the APSS crate)
prepended to the task as a preamble, since interactive harnesses have no
separate system channel.
- R5: subagents are validated-only. `start_agents()` returns ONLY the
default agent; declared subagents are surfaced as metadata but never
launched. Test asserts the codex subagent is present-but-not-executed.
- R9: vendored `tests/fixtures/recipe-pr-reviewer/` verbatim from the APSS
crate's `examples/valid/pr-reviewer/` at rev 4819b38 (identical to that
rev, verified by git diff), used by the loader tests.
Dependency (R2): the git-pinned dep is fetchable but does NOT build at rev
4819b38 - the APSS repo workspace includes template skeletons whose package
name is the literal `{{slug}}`, which cargo rejects when resolving the
git-source workspace. Shipped a `path` dep to the local EXP-0005 worktree
crate (its explicit workspace `members` list excludes the templates) with a
`TODO(#245)` to switch back to git-pinned once EXP-0005 lands on APSS main.
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
…nt JSON
Plan B Task 3: the itmux run orchestrator, the centerpiece.
- src/run/orchestrator.rs (harness-NEUTRAL): the R7 state machine
Provisioning -> Launching -> Awaiting -> Capturing -> Terminalizing ->
Done, driven over a generic RunExecutor trait so the whole thing is
unit-tested with a fake (no docker/token).
- SINGLE terminalization path: one terminalize() chooses exactly one
terminal reason (precedence hard_cancel > adapter_error > timeout >
graceful_cancel > success), tears the workspace down EXACTLY ONCE
(the handle is consumed there and nowhere else), and emits EXACTLY ONE
session_end. A teardown failure is logged to stderr + attached to the
session log and never masks the primary outcome.
- No orphan by construction: the handle is created at Provisioning and,
once it exists, every exit path flows through the single teardown.
- Emits the R6 event envelope (run_id, monotonic seq from 0, RFC3339 ts,
type-tagged payload): tool_start/tool_end around each phase + terminal
session_end. limits.timeout_s enforced on await.
- CancelToken (graceful/hard) checked at each boundary.
- detect_outcome hook on the trait (Task 5 / #246 fills harness-aware
detection; placeholder derives success from liveness for now).
- src/run/workspace_executor.rs: the real RunExecutor over Workspace, the
public run(), a dependency-free RFC3339 UTC clock, run-id + credential
materialisation. Kept OUT of orchestrator.rs so the R8 guard holds.
- main.rs: itmux run --recipe <dir> --task <str> [--image] [--json]
[--result-file]. Event JSONL -> stdout; final AgentRunResult as a
type:"result" line or --result-file; stderr = human logs only (R6).
Tests (all without docker/token, via a fake executor):
- happy path: phases in order, correct seq, exactly one terminal session_end.
- start-failure-after-partial-startup: torn down once, NO orphan (the exact
Python bug).
- provision-failure: nothing to tear down (transactional).
- cancel-during-await: graceful, final capture collected, success=false.
- timeout-during-cancel: timeout wins over graceful (precedence).
- hard-cancel-while-launch-in-flight: bails, torn down once, NO orphan.
- teardown-failure-does-not-mask-outcome: success preserved, warning in log.
- exactly-one-session_end across all 10 scenarios (no duplicate).
- R8 guard: orchestrator.rs contains no claude/codex/gemini literals.
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
…pane liveness
Plan B Task 5 (Gap 2): a live eval found that deriving run success from
AwaitResult.ready (pane liveness) is wrong - a stable pane only means the
TUI settled, not that the task succeeded, so an idle/errored run read as
success=true.
- src/adapter.rs: new per-harness detect_outcome(agent, pane) ->
RunOutcomeSignal { success, reason }, where each harness declares its own
hard-error markers (harness-neutral, alongside readiness + submit):
- claude: API Error, authentication_error, 401, Please run /login,
Invalid API key.
- codex: CONSERVATIVE auth-focused set (not logged in, Not authenticated,
codex login, 401 Unauthorized, Unauthorized, Invalid API key). A healthy
codex launch prints benign "MCP client ... error" warnings (see
runs/smoke-codex.txt), so a bare error/failed marker would false-positive
- the set deliberately avoids those.
- gemini: no markers yet (not a Task 5 target); readiness floor only.
A hard-error marker -> failure; otherwise a pane that settled to a ready
state with no markers -> success. Documented KNOWN LIMIT: true
task-completion detection is impossible on an interactive TUI (no
structured completion signal); this is a floor that catches hard errors +
treats a clean ready pane as success. Completion sentinel = Plan 3.
- workspace_executor.rs: detect_outcome now calls the adapter (was the
liveness placeholder, #246), mapping RunOutcomeSignal -> AgentRunOutcome.
- orchestrator.rs unchanged and still harness-neutral (calls the trait
hook); R8 guard still passes.
Tests:
- outcome_detection.rs (11): per-harness claude + codex - clean ready pane
-> success; API Error / 401 / login-required (claude) and not-logged-in /
Unauthorized (codex) -> failure; benign codex MCP warning -> NOT a
failure; marker-collision guard; gemini has no markers.
- orchestrator.rs: new test asserting an "errored" pane yields success=false
through the orchestrator (success now flows from detect_outcome, not
liveness).
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
Plan B Task 6: the CLI signal wiring for `itmux run`. The orchestrator already had the CancelToken + graceful/hard state machine (Task 3); this connects OS signals to it. - src/run/orchestrator.rs: new CancelEscalator + SignalKind (pure, unit-testable): first Interrupt -> graceful, second Interrupt or any Terminate -> hard. Never downgrades a hard cancel. - src/main.rs: install_signal_watcher wires SIGINT/SIGTERM into the CancelToken via signal-hook's async-signal-safe self-pipe Signals iterator, drained on a NORMAL watcher thread (no work in handler context, no unsafe). First Ctrl-C -> graceful (collect partial result + terminal session_end); second Ctrl-C or SIGTERM -> hard (immediate teardown). The watcher is closed + joined after the run. On any signal path the orchestrator's single terminalization still tears down exactly once, so Ctrl-C never orphans a container. - Cargo.toml: add signal-hook (unix-only target dep). Justified: the crate forbids `unsafe` (`unsafe_code = "forbid"`) and std has no safe API to install a SIGINT/SIGTERM handler; signal-hook encapsulates the FFI behind a safe, async-signal-safe self-pipe iterator. Minimal footprint (signal-hook + registry + libc), unix-only matching this docker/tmux host-side driver. Non-unix builds compile with the watcher #[cfg]'d out. Tests: - tests/cancellation.rs: escalation transitions without raising OS signals - first interrupt graceful, second hard, terminate straight to hard, hard never downgraded, three interrupts stay hard. - tests/orchestrator.rs: a run receiving graceful-then-hard (two interrupts via the real CancelEscalator during await) terminalizes exactly once with the hard reason and tears down once (no orphan). R8 guard still passes. Real OS-signal delivery is covered by the live run (Task 8). Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
The APSS workspace now excludes the template skeletons whose package name
is the literal `{{slug}}` (fix(workspace): exclude template skeletons, APSS
PR #89), so cargo's git-source workspace resolution no longer chokes on
them. Swap the machine-local path dep for the git-pinned dep at rev
ce786bc (tip of APSS PR #89, feat/recipe-directory-standard, which carries
the exclude fix).
cargo build + test + clippy + fmt all pass with the git dep. Repoint the
rev to an APSS main sha once #89 merges.
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
Fix 1 - precedence on Err paths (orchestrator.rs). The provision/launch/
submit/await/capture Err branches broke AdapterError without re-checking
cancel state, so adapter_error could beat a pending hard_cancel. Introduce
a single `arbitrate(candidate, cancel)` helper that routes every terminal
reason (all 5 Err breaks AND the final success/timeout/graceful choice)
through TerminalReason::rank(), so the code and the documented precedence
table agree by construction. Test: hard-cancel pending + executor Err ->
HardCancel, one teardown, one session_end.
Fix 4 - stdout is pure AgentRunEvent JSONL (contract.rs, main.rs). The old
trailing {"type":"result"} line lacked the run_id/seq/ts envelope. Add an
AgentRunEventPayload::Result variant (+ AgentRunEvent::result ctor) so the
final result is delivered as a real AgentRunEvent; main.rs emits it with a
continuing seq. With --result-file, the result goes to the file and stdout
stays pure events. Test: the result line round-trips as an AgentRunEvent.
Fix 5 - anchored error markers (adapter.rs). Bare 401/Unauthorized/Invalid
API key scanned the whole pane, so agent prose ("returns 401 on bad auth")
false-triggered. Split markers into distinctive BANNER phrases (matched
anywhere) and AMBIGUOUS tokens (401, Invalid API key) that count ONLY on a
line that also reads as an error banner (contains "error"). Tests: prose
mentioning 401 / Invalid API key -> success; real auth banner -> hard error
(claude + codex).
Fix 2 - hard-cancel orphan window (#248). Workspace::start/await_completion
are blocking and don't observe the CancelToken; a hard cancel takes effect
only when they return (then the handle is torn down - no orphan for
catchable signals), but a SIGKILL / wedged call in that window can orphan a
container. Route taken: (a) TODO(#248) at workspace_executor provision +
await documenting the blocking-cancel + SIGKILL gap; (b) a best-effort,
PRECISE reap - the executor pins a deterministic unique container name up
front (small additive StartOptions.container_name override, ~8 LOC, default
None preserves existing behavior) and a new RunExecutor::teardown_orphans
(default no-op) that terminalize calls on the hard-cancel-with-no-handle
path to `docker rm -f` that exact name (never a prefix, so concurrent
same-recipe runs are untouched). Full cancellable start is #248. Test:
hard-cancel-before-provision reaps exactly once; happy path never reaps.
Fix 3 - skill host-path guard (#249). recipe_loader resolved bundled skills
to host paths and passed them to `claude --plugin-dir` in-container where
they do not exist. resolve_skill_plugin_dirs now returns Result and fails
fast with RecipeMapError::BundledSkillStagingUnsupported (cites #249) when a
ref resolves to a bundled host dir; container-relative refs pass through.
Tests: bundled -> error; container-relative -> ok. Added a recipe-plain
fixture (no bundled skills) for the success-path mapping tests; the vendored
pr-reviewer fixture (bundled) now drives the rejection test.
Nit - Cargo.toml comment corrected: the APSS exclude fix makes git-source
resolution SUCCEED, but cargo still PRINTS the non-fatal {{slug}}
diagnostics while scanning; the comment now says printed-but-non-fatal.
Regenerated docs/contract/agent-run-event.schema.json for the new variant.
Gates green: cargo test (147), clippy -D warnings, fmt --check. R8
harness-neutrality guard still passes.
Refs #248, #249.
Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
Contributor
Author
Dual-review gate: PASS
Fixes landed: (1) terminal-reason precedence routed through Follow-ups tracked: #248 (cancellable-start, Task 8 live), #249 (skill staging, Task 8 live). |
This was referenced Jul 7, 2026
Open
…> caef354) #89 (recipe-directory standard) merged to APSS main. Move the pin off the pre-merge branch rev (ce786bc, orphaned once that branch is deleted) to the merge commit caef354, which carries the full standard incl. the Copilot fixes. itmux builds + all tests pass against the merged recipe crate. Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
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
1 participant
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.
Plan B: moves the
AgentRunSpec -> AgentRunResultcontract + orchestrator INTO Rustitmuxas anitmux runsubcommand (per ADR-038, #244). Language-neutral JSON contract; syn137 becomes a thin client (Task 7, follow-up). Supersedes the Python contract #240. Stacked on the Plan 1a substrate (#243).Plan reviewed by codex (APPROVE, 0 gaps) before implementation; 10 pre-impl findings folded in.
What's in it (stacked commits)
AgentRunSpec/AgentRunResult/AgentRunEvent(R6 JSONL envelope: run_id, monotonic seq, RFC3339 ts, type-tagged payload) + a schemars-derived JSON schema (no drift).apss-v1-0005-agent-recipecrate (GitHub App template should not require static installation_id #89) so a recipe that validates is a recipe that runs; maps the recipe's default agent -> start args + submit text; subagents validated-only in v1.itmux runorchestrator (R7 state machine) - single terminalization path, stop-exactly-once teardown (handle moved+consumed once), terminal-reason precedence (hard_cancel > adapter_error > timeout > graceful_cancel > success), exactly onesession_end. The 5 Python concurrency defects are impossible by construction - covered by no-orphan / no-duplicate-session_end / precedence / teardown-failure tests.detect_outcome(error markers), replacing the pane-liveness heuristic; orchestrator stays harness-neutral (R8 source-scan guard).itmux run --recipe <dir> --task <str> [--image] [--json] [--result-file]- events JSONL on stdout, final result as atype:"result"line, stderr = human logs.~135 tests (all fake-executor, no docker/token needed); clippy -D warnings, fmt clean. git-pinned APSS dep (resolves #245).
Known follow-ups (tracked)
itmux runon real Docker (validates cred materialization + codex error-marker set + live send-race/cancel timing).detect_outcomeis a hard-error floor (interactive TUIs have no structured completion signal) - completion sentinel is Plan 3.Workspace::startis monolithic; the provision/launch orphan-boundary is fully exercised by the fake executor, real path leans onstart's transactional cleanup.Tracks okrs-o78 / okrs-51p.7.