feat(dispatch): add owned sandbox runner adapter - #44
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a concrete, JustAi-owned AgentRunner implementation over the existing Bubblewrap local executor, producing structured run evidence (actions/results/transcript) with explicit bounded observation truncation, while keeping “task done” dependent on both execution and verification.
Changes:
- Introduces
JustAiSandboxRunnerplus Chunk-based local execution plumbing (_chunk_for_task,_perform_chunk_action,_verify_chunk, transcript writer). - Preserves “honest completion” semantics in
_execute_single_localby mapping runner evidence to Task status without inferring completion. - Expands/adjusts tests to validate policy gating, evidence/action recording, and bounded tail preservation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
justai/agent_dispatch.py |
Adds owned sandbox runner adapter, structured evidence recording, and Chunk-based execution/verification helpers. |
tests/test_local_execution.py |
Adds owned-runner behavior tests (policy gating, evidence budgets, transcript/tail behavior) and transcript cleanup. |
tests/test_executor_routing.py |
Updates routing tests to the new action/verification result shapes (_run_local_command_result, _verify_chunk). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| run = _LocalRunResult( | ||
| chunk=chunk, | ||
| success=success, | ||
| steps_used=1 if execution.action is not None else 0, | ||
| final_diff="", | ||
| transcript_path="", | ||
| failure_class=failure_class, | ||
| latency_seconds=time.monotonic() - started_at, | ||
| actions=actions, | ||
| results=results, | ||
| execution_outcome=execution.outcome, | ||
| execution_detail=_bounded_observation_value( | ||
| execution.detail, observation_limit, tail=True | ||
| ), | ||
| verification_passed=verification.passed, | ||
| verification_detail=_bounded_observation_value( | ||
| verification.detail, observation_limit, tail=True | ||
| ), | ||
| ) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 770824dc13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _is_catastrophic(command): | ||
| return "blocked", f"refused catastrophic command: {command[:120]}" | ||
| return _ActionExecution( | ||
| "blocked", f"refused catastrophic command: {command[:120]}" | ||
| ) | ||
| action_record = ActionRecord.create("bash", {"command": command}) |
There was a problem hiding this comment.
When the model emits a catastrophic command such as rm -rf /, this early return occurs before the ActionRecord is created. The resulting safety-violation run therefore omits the blocked attempt from actions and results—and may show only the subsequently executed verifier—so typed audit evidence cannot identify or correlate the action that caused the violation. Create the action record before applying the guard and attach a failed result for blocked commands.
Useful? React with 👍 / 👎.
| # Retain the failed verifier's raw tail until the runner renders the | ||
| # Chunk-bounded observation. A head slice here would hide the terminal | ||
| # diagnostic and make the later tail policy misleading. | ||
| return _Verification(False, f"exit {result.returncode}: {result.stderr}", "fail", True) |
There was a problem hiding this comment.
When a failed verifier emits more stderr than the observation budget, the later tail truncation removes the exit <code>: prefix, leaving only the stderr suffix and a generic fail status. Consequently, the returned result and transcript lose the verifier's numeric exit code precisely for noisy failures; append the exit code after stderr, as the action executor does, so structured-tail truncation preserves it.
Useful? React with 👍 / 👎.
770824d to
07b58d3
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Independent review from Cowork — PASS on design, CHANGES-NEEDED before merge (rebase + one conflict to reconcile). The adapter itself reads well: it builds on the current boundary work, keeps Blocking: the branch is 7 commits behind
Suggested reconciliation (the conflict is only in Then re-run: current |
07b58d3 to
caefe5f
Compare
|
Re-review of the rebased head — PASS. Clean, green, and mergeable; merge when you're ready. Head Both regressions I proved against the old head are gone, and you reconciled the semantics rather than silencing the tests:
Full suite at your head: 621 passed + 14 subtests (demo-build is 612 + 14; your 9 new tests account for the delta). Design points I'd keep as-is: Your source atom, so I'm not merging it. No further review needed from me unless the head moves again. |
Summary
Verification
PYTHONDONTWRITEBYTECODE=1 pytest -q -p no:cacheproviderBoundary
Source-only candidate. No runtime installation, listener, model service, deployment, or readiness claim is included; productive execution remains separately gated.