Repository navigation
Execute native shell requests through the compiled module - #53
Conversation
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document the design and rollout plan for running shell commands natively, covering the intended behaviour and the steps needed to implement it. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a StartShellExec method and ShellExecRequest type to the host vocabulary, bumping the contract to 1.11, and expose a TimedOut execution failure. The limited and jailed hosts gain a run_command_observed entry point so a prepared command can be collected under the same bounded output, timeout and cleanup ownership as argv execution. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…/src/lib_tests.rs,crates/tinybo Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The result of the awaited stream call is an Option<Result>, so the error and empty cases must be matched as Some(Err(_)) | None rather than Err(_). The previous arms were swapped, causing the wrong failure handling to run. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…aths Add tests for native shell execution covering idempotent replay of the original request, rejection of a changed request, scratch retention until output release, reservation preservation after an invalid timeout budget, and child cleanup acknowledgement on timeout. Existing stream fixtures gain the new shell field so they keep compiling. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the native shell collector selection out of the spawn path into a dedicated helper so the shell and jail branches share one entry point. Add a test covering startup hook clearing, approved env overrides, and binary-safe pipes, and make the grants manifest e2e exercise a quoted Windows executable path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…yhumansai/tinybox into tinybox-native-shell-7391
The inline check that rejects resources without an owned native collector is now a shared `require_owned_collector` helper so the same validation can be reused by other call sites. Behaviour is unchanged. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Wire up a cfg(test) module that loads the native shell tests from mod_tests.rs, so the module's behaviour can be covered by unit tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extract the process and resource id checks into a shared helper so the same validation can be reused, and bump the capability contract version snapshot to 1.11 to match. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a README section describing the StartShellExec operation introduced in contract 1.11, covering environment snapshots, scratch and output lifetime, and the refusal behaviour of jail and remote providers. Link the new native-shell-execution spec from the plans and specs indexes, and split the grants manifest e2e test into discovery and native shell helpers. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
ShellContext::prepare now delegates to a new prepare_with_backend helper that accepts a JailBackend, so the backend is resolved once at the call site instead of inside the jail setup path. This makes the jail preparation testable with a fixture backend, which the new test uses to verify that the owned scratch directory is added and that unsupported constraints are refused. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extracted ShellContext and ShellHost into a dedicated types module and re-exported ShellContext so existing call sites keep working. This keeps the module's implementation focused and gives the shared types a clearer home. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the StartShellExec method to the README's method table so the documented surface matches the implemented interface. The test's unix-only gate is dropped, letting the shell timeout cleanup test run on all platforms. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…yhumansai/tinybox into tinybox-native-shell-7391
Add a unix-only test asserting that a shell command exceeding the output limit fails with OutputLimit and can still be released and closed cleanly. The existing timeout test now picks a platform-appropriate long-running payload so it works on Windows as well. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The plan and spec now record that native shell execution is implemented upstream, with review and cross-platform CI still pending. The previous wording still described implementation as in progress. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…yhumansai/tinybox into tinybox-native-shell-7391
|
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 configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: a3a26543bb
ℹ️ 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".
…tion Add a test asserting that when scratch directory removal fails, the output stream is retained and both release_output and close return errors so the caller can retry. Once the obstruction is cleared, close succeeds and the stream is dropped. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
… the bus Add an integration test that drives SelectShell and StartShellExec through the bus codec and checks the returned invocation matches the host selection. It also verifies the process exits cleanly with the expected output and that the sandbox scratch directory is left empty after release. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Scratch removal now happens during drain and keeps the TempDir alive when deletion fails, so a later close can retry instead of losing the path. Cleanup acknowledgement also waits on the collector, and the README documents the retained-context behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…rnal The cleanup acknowledgement check now reads through the stream's journal field, matching where the flag is actually stored. The scratch deletion test was extended to cover both the release and close retry paths. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ssertion The assertion message in the native cancel and close test now interpolates the mode and process status, so a failure reports which mode was exercised and what the descendant's state actually was instead of a bare message. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…d nondirectory collectors Add a test asserting that start_shell_exec refuses a shell request with UNSUPPORTED_OPERATION when the resource is not a native host or has no workspace root, so the pre-admission checks stay enforced. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b577f602c
ℹ️ 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".
Add a test asserting that a cancelled native shell keeps its sandbox scratch directory until output is released, and that release clears it. Also extend the timeout test with the same scratch-count check before release. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Scratch directory deletion now happens on a blocking thread instead of the async worker, and stream teardown disposes the shell scratch so cleanup failures surface as backend errors. The scratch state is shared via Arc so the blocking closure can report whether removal succeeded. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Scratch removal now happens inside spawn_blocking so the async runtime is not blocked by filesystem deletion, and the scratch handle and failure flag are cloned into the task instead of borrowed. The README was updated to describe that output release and resource close remove scratch on a blocking worker, while timeout and cancellation drain the process but retain scratch as long as its journal is retained. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The blocking cleanup closure now locks the scratch handle captured before the task was spawned instead of reaching back through self, which is not available inside the move closure. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a2d6deff7
ℹ️ 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".
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e3b9c635e
ℹ️ 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".
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The test compared grant paths against non-canonicalized temp dir paths, which could fail on systems where the temp directory is a symlink. Both assertions now canonicalize the expected paths before checking membership. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2ebf59c59
ℹ️ 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".
52fc1f9
into
tinybox-shell-selection-7391
Tiny Sweeper review
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybox-bus/src/lib.rs, crates/tinybox-bus/src/lib_tests.rs, crates/tinybox-host/src/limited/jailed.rs, crates/tinybox-host/src/limited/mod.rs, crates/tinybox-module/src/tinybus_module/mod.rs, crates/tinybox-module/src/tinybus_module/mod_tests.rs, crates/tinybox-module/src/tinybus_module/native_shell/mod.rs, crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs and 9 more.
$0.0068 · 1,289,883 in / 111,257 out · 196,864 cached (15%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 735 embedded
critique: $0.0043 · 408,312 in / 79,060 out · 97,408 cached (24%) · deepseek-v4.1-flash
security: $0.0021 · 660,582 in / 27,889 out · 87,552 cached (13%) · deepseek-v4.1-flash,
tests: $0.0001 · 127,392 in / 774 out · 7,040 cached (6%) · deepseek-v4.1-flash
description: $0.0001 · 63,630 in / 535 out · 4,864 cached (8%) · deepseek-v4.1-flash
tinysweeper
Bot
added
the
priority: p2
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybox-bus/src/lib.rs, crates/tinybox-bus/src/shell_execution.rs, crates/tinybox-host/src/limited/jailed.rs, crates/tinybox-module/Cargo.toml, crates/tinybox-module/src/tinybus_module/README.md, crates/tinybox-module/src/tinybus_module/native_shell/README.md, crates/tinybox-module/src/tinybus_module/native_shell/mod.rs, crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs and 9 more.
$0.0181 · 1,394,866 in / 159,914 out · 261,760 cached (19%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 735 embedded
critique: $0.0099 · 486,232 in / 96,677 out · 137,472 cached (28%) · deepseek-v4.1-flash,
security: $0.0077 · 692,903 in / 58,284 out · 86,528 cached (12%) · deepseek-v4.1-flash
tests: $0.0001 · 123,674 in / 1,082 out · 7,168 cached (6%) · deepseek-v4.1-flash
description: $0.0000 · 61,645 in / 486 out · 30,592 cached (50%) · deepseek-v4.1-flash
Add contract 1.14
StartNativeShellExec(ShellExecRequest) -> ProcessReffor native execution with owned scratch, timeout and optional jail inside the compiled TinyBus module. Preserve the existing two-argumentStartShellExec(SpawnRequest, SelectShellRequest)operation and its wire compatibility.The operation clears ambient environment, snapshots only execution/bootstrap names, applies approved overrides then removes startup hooks/options, owns private scratch, captures binary pipes under the shared output budget, and retains existing reservations/replay/cancellation/close barriers. A positive runtime budget publishes
TimedOutonly after supervised cleanup. Explicit native jails validate approved grants and add owned scratch; other providers refuse this local-only operation. Caller authorization of the state directory and private Windows parent ACLs remains required.Stacked on canonical PR #52 (which depends on #51). Retarget to main after those prerequisites land, preserving all commits. This is the upstream native-execution slice of tinyhumansai/openhuman#7391; it does not update an OpenHuman gitlink or claim a released module is available. Docker lifecycle and the OpenHuman host adapter remain separate work.
Validation:
The compiled-artifact admission regression first failed with
StartShellExec: UnknownMethod, then passed after implementation. It builds the dynamic artifact with--locked --offlineand exercises production admission and execution through TinyBus.Scratch deletion regression failed before the fix. It now verifies failed ReleaseOutput and Close retain ownership, then succeed after filesystem repair. A directory-to-file replacement forces the failure deterministically without depending on platform ACLs or the test user's privileges.
cargo fmt --all -- --check,cargo clippy --all-targets --all-features -- -D warnings,cargo build --all-targets --all-features, andcargo test --all-features: all pass..github/scripts/check-file-coverage.sh 90 coverage.json: passes every source file. Module dispatch: 94.86%; stream lifecycle: 93.00%; native shell preparation/cleanup: 91.85%.Descendant cleanup passed 100 focused repetitions; its assertions remain intact and failures now include the cleanup mode and process status.
Added bus codec coverage for shell selection/execution and admission refusals for nonlocal or nondirectory collectors.
Recursive deletion runs on a blocking worker; a current-thread runtime regression first failed with synchronous deletion and now passes. Timeout and Cancel retain scratch through journal release, verified by regressions.
All hosted checks passed on merge 9e3b9c6, including Rust coverage and Windows native supervision. Hosted validation of the latest compatibility fix is pending.
Cleanup tracks the allocated scratch pathname. Trusted passthrough workloads that deliberately move scratch elsewhere take those files outside managed cleanup; arbitrary rename tracking was declined in review because missing paths cannot distinguish deletion from relocation.
The original two-argument StartShellExec bus-codec regression reproduced BadArguments before the compatibility fix and passes afterward. Both shell APIs have distinct additive method names; the original admission and retry-identity tests remain covered.