feat(agentic_isolation): thin Python client for itmux run (Plan B Task 7)#250
Open
NeuralEmpowerment wants to merge 4 commits into
Open
feat(agentic_isolation): thin Python client for itmux run (Plan B Task 7)#250NeuralEmpowerment wants to merge 4 commits into
NeuralEmpowerment wants to merge 4 commits into
Conversation
…k 7) Add run_client.py: a thin Python client that shells out to the Rust `itmux run` binary. It owns no orchestration or business logic - all of that stays in the Rust subcommand. Responsibilities: - Mirror the wire contract (AgentRunResult, AgentRunEvent + payload variants) as strict Pydantic v2 models (frozen=True, extra="forbid") whose field names match the Rust serde names exactly. The event stream is an internally-tagged discriminated union on the `type` field, matching serde's #[serde(tag = "type", deny_unknown_fields)]. - run_agent(): spawn `itmux run --recipe --task [--image] --json true`, stream stdout event JSONL line-by-line, invoke on_event per event, and return the AgentRunResult from the terminal `result`-payload event. - Process-group cleanup (R10): child runs in its own session/group (start_new_session=True); on cancel/crash/timeout the whole group is signalled SIGTERM then SIGKILL so the Rust process and any docker exec grandchildren are reaped, never orphaned. Supersedes the hand-written Python contract from #240 (closed separately). Adds pydantic>=2 as a dependency of the previously zero-dep package to get the discriminated union + deny-unknown-fields guarantees for free. Gates: 12 new tests pass, ruff check/format clean, mypy --strict clean. Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
…ct models, result/timeout ordering Dual-review fixes for the thin itmux run client: - Process-group teardown is now kill-before-reap (codex must-fix 1+2): SIGTERM the whole group first (lets the Rust leader tear its container down gracefully; a premature SIGKILL would orphan the container, #248), poll a bounded grace for the GROUP to drain, SIGKILL any survivor, then reap the leader zombie last. No longer short-circuits when the leader has exited but a docker-exec grandchild survives, and never killpg's a pgid it has already reaped (PID-reuse safe). The stderr drain is joined only after the group is dead. - Strict Pydantic scalars (codex must-fix 3): u64-backed fields (seq, input_tokens, output_tokens) use strict + ge=0 and bools use StrictBool, so seq:-1, input_tokens:"1", output_tokens:-2 and success:"false" are rejected exactly as serde rejects them. - Result wins over a simultaneous timeout (Claude nit 1): a delivered terminal result is returned even if the watchdog fired in the same instant. - Break on the terminal ResultEvent (Claude nit 2): liveness no longer depends on EOF, so a child that emits the result then hangs (timeout None) cannot block run_agent. - Cleanup test uses a real survival check via ps state (codex nit 2), so a killed-but-unreaped zombie is not mistaken for a live process. Keeps pydantic a range with an upper major cap (>=2,<3): this is a library, so exact-pinning would break consumer resolution; the cap guards against a future pydantic 3 break. Gates: 19 tests pass, ruff check/format clean, mypy --strict clean on run_client.py + test_run_client.py. Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
…SIGKILL Codex re-review must-fix on _terminate_process_group: proc.poll() in the grace loop reaped the leader zombie the instant it exited, freeing its pid. Because pgid == leader pid, the pgid became eligible for OS recycling before the final killpg(SIGKILL), so a concurrent fork could get that pgid and eat the signal. Invariant now enforced: the leader stays UNREAPED (alive or zombie) until AFTER the final group signal, which keeps its pid/pgid reserved so every killpg provably targets our group. Grace phase does NO reaping - no poll(), no wait(), no waitpid. New ordering: SIGTERM group -> plain chunked sleep (no early exit; killpg(pgid,0) can't distinguish a lingering zombie leader from live descendants, so there is no correct non-reaping early exit) -> SIGKILL group -> reap the leader LAST. The early returncode guard still short-circuits a second teardown pass on an already-reaped (recyclable) pgid. Because there is no early exit, teardown now always sleeps the full grace. To keep that off the happy path, run_agent uses a short post-result grace (0.5s, leader already done) vs the full crash/timeout grace (5s), and the watchdog + finally teardown paths are serialized with a lock + once-flag so they can never both run a full teardown concurrently (which would reintroduce the cross-thread reap-then-signal race). Tests: result-wins-over-timeout is now deterministic (a blocking on_event on the ResultEvent lets the watchdog fire while run_agent holds the result); the grandchild survival check already uses ps state, not os.kill(pid,0). A fast-teardown fixture shrinks the grace so no-result/timeout tests stay quick. Gates: 19 tests pass (stable across repeats), ruff check/format clean, mypy --strict clean on run_client.py + test_run_client.py. Claude-Session: https://claude.ai/code/session_019TtekhJft4s2qBGRGgxToG
Contributor
Author
Dual-review gate: PASS
Final teardown ordering: killpg(SIGTERM) -> chunked grace sleep (no reaping) -> killpg(SIGKILL) -> proc.wait() reaps leader last, so the unreaped leader keeps the pgid reserved through the kill. Strict scalars (NonNegativeInt/StrictBool) reject serde-incompatible lines; result beats simultaneous timeout; loop breaks on the terminal ResultEvent. 19 tests, ruff + mypy --strict clean on both files. |
This was referenced Jul 7, 2026
…at/itmux-python-client
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 Task 7. A thin Python
run_agentthat shells out to the Rustitmux runbinary and parses itsAgentRunEventJSONL +AgentRunResult. Owns NO orchestration - the Rust binary owns the whole provision->stop lifecycle. This is the syn137-facing half of the Rust contract migration; supersedes the Python orchestrator/contract in #240 (which will be closed once this + #247 land). Stacked on #247.Surface
Builds
itmux run --recipe <r> --task <t> [--image <i>] --json true, streams stdout JSONL, fireson_eventper event, returns the result from theResult-payload event. RaisesItmuxRunError(carrying returncode/stderr) on non-zero-exit-no-result, unparseable output, missing terminal result, or timeout.Contract parity
Pydantic v2 discriminated union:
AgentRunEvent = Annotated[ToolStart | ToolEnd | TokenUsage | SessionEnd | Result, Field(discriminator="type")], each variant frozen +extra="forbid", serde field names matched exactly. This mirrors the Rust#[serde(tag="type", deny_unknown_fields)]behaviorally: discriminator picks the variant, per-variant forbid rejects stray keys.Process-group cleanup (R10)
Child spawned
start_new_session=True(leads its own group). On KeyboardInterrupt / parse failure / timeout,os.killpg(pgid, SIGTERM)-> grace ->os.killpg(pgid, SIGKILL)reaches the Rust process AND itsdocker execgrandchildren.try/except BaseExceptiontears down before re-raise;finallykills any survivor + joins a stderr-drain thread (no deadlock on a full stderr pipe); athreading.Timerenforcestimeout.Tests: 12, all pass
Model discrimination + unknown-field rejection (incl. Result line + unknown discriminator); happy-path result + per-event on_event; argv construction; the 3 error paths; a killpg-pgid monkeypatch unit test (SIGTERM + SIGKILL); a real orphan-reaping test (fake spawns a grandchild, blocks, timeout fires, grandchild confirmed dead). All fakes are on-disk stubs via
itmux_bin- no real docker/itmux.Reviewer note
pydantic>=2(+ pydantic.mypy plugin) to a previously zero-dep package - deliberate, per the task's Pydantic-v2 mandate and for serde parity. Flagging given the minimize-deps preference; the dep-free alternative is dataclasses + hand-rolled tag dispatch (loses the extra="forbid" parity).--json true(not bare--json) - the clap arg isArgAction::Setand requires a value.Gates: pytest 12/12, ruff check + format clean, mypy --strict clean (no Any). Tracks okrs-o78.