Repository navigation
Retain native shell timeout outcomes and offload preparation - #58
Conversation
Add a regression test asserting that a timed-out execution keeps its TimedOut status once cleanup is retried and acknowledged, rather than being masked by the earlier CleanupFailed state. The remote fixture hosts gain a wait_for_timeout flag so the test can hold cleanup pending until the timeout fires. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The DockerFaultHost fixture no longer blocks on a pending future when wait_for_timeout is set, since the flag was never exercised by the stream tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Stream executions that hit their timeout were reported as backend failures because the timeout branch never recorded why the run ended. A flag is now set when the timeout elapses and consulted when the journal is finished, so callers see ExecutionFailure::TimedOut instead of a generic backend error. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document that a pending timeout outcome survives until a later cleanup retry succeeds, making the retry semantics of the native shell execution spec explicit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Shell scratch preparation now runs on a blocking thread instead of the sole async worker, so filesystem work no longer stalls the runtime. Added a test asserting preparation happens on a different thread. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Filesystem preparation now runs via spawn_blocking so slow storage cannot stall the module runtime, with worker failures mapped to a backend error. The spec was updated to document that preparation and recursive cleanup happen on blocking workers. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ecution The execution support check now runs inside prepare_execution instead of after the resource lookup, so unsupported providers are rejected before any collector or execution state is set up. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
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: fa3f11deb3
ℹ️ 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".
| let failure = if self.timed_out.load(std::sync::atomic::Ordering::SeqCst) { | ||
| ExecutionFailure::TimedOut | ||
| } else { | ||
| ExecutionFailure::BackendFailed |
There was a problem hiding this comment.
When cancellation is requested before the shell deadline but the runner ignores it until the timeout and the first drain fails, the task records CleanupFailed; after a later drain succeeds, this branch reports TimedOut solely because the deadline also elapsed. That conflicts with the normal completion path above, where cancel_requested() takes precedence over result.is_none() and produces Cancelled. Retain the intended terminal outcome at timeout/cleanup failure so retrying cleanup does not change a cancelled execution into a timeout.
Useful? React with 👍 / 👎.
b511bb7
into
tinybox-shell-selection-7391
Tiny Sweeper review
|
Native shell timeouts could become BackendFailed when kill/reap initially failed and cleanup succeeded on a later retry. Retain the timeout outcome until cleanup is acknowledged. Native shell preparation now runs on a blocking worker so slow filesystem operations do not stall the module's sole async worker.
These address the two review findings posted on #53 while another actor merged it at e95d96a. The fixes were pushed afterward and therefore require this follow-up. Stacked on #52's branch; preserve all commits.
Public API changes: None.
Validation: