Conversation
|
📚 Documentation Preview Ready! Your documentation changes have been deployed to: This preview will be automatically updated when you push new commits. Preview deployment started at Wed, 14 Jan 2026 22:19:52 GMT |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd148a8a13
ℹ️ 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".
libs/nox-py/src/step_context.rs
Outdated
| /// Gracefully terminate all s10-managed recipes (external processes). | ||
| /// | ||
| /// This signals all processes managed by s10 (registered via `world.recipe()`) to shut down | ||
| /// gracefully. The processes receive SIGTERM and have approximately 2 seconds to clean up | ||
| /// before being force-killed. |
There was a problem hiding this comment.
Align stop_recipes behavior with its SIGTERM+grace claim
The new stop_recipes API is documented here as sending SIGTERM and allowing ~2 seconds for cleanup, but the implementation only cancels the s10 CancelToken. For common recipe types (e.g., ProcessRecipe/CargoRecipe), cancellation immediately calls child.kill() with no grace period, so external processes will be force‑killed rather than shut down cleanly. Callers who rely on this method to flush state or perform cleanup will still see abrupt termination. Either update the docs/comments to reflect the actual behavior or add a SIGTERM+timeout path in the s10 cancellation logic.
Useful? React with 👍 / 👎.
|
📚 Documentation Preview Ready! Your documentation changes have been deployed to: This preview will be automatically updated when you push new commits. Preview deployment started at Wed, 14 Jan 2026 22:44:15 GMT |
shanecelis
left a comment
There was a problem hiding this comment.
Looks good to me, but I don't have a ready means of exercising it, so I'm relying on compilation and author's runtime validation.
It can be useful to decide to proactively kill s10 managed recipes at some step in the simulation. This creates that capability.
Note
Introduce graceful external-process termination from simulation callbacks.
ctx.stop_recipes()toStepContext(passesCancelToken) to signal s10-managed recipes to stopWorldBuilder.runspawns s10 withrun_recipe_with_tokenand injects the cancel token into pre/postStepContextrun_recipe_with_tokenand keep Ctrl+C handling;run_recipewraps itProcessArgs.run: on cancel, send SIGTERM (Unix), wait ~2s, then force-kill; kill immediately on non-Unixstop_recipes()in Python API and update SITL example (MAX_TICKS,max_ticks,time.sleep(0.5))Written by Cursor Bugbot for commit 8c4177c. This will update automatically on new commits. Configure here.