-
Notifications
You must be signed in to change notification settings - Fork 3
Introduce a new persistent execution model #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Also add docs and tests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @josh-padnick, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request fundamentally changes the execution model for Runbooks by introducing a persistent environment. Previously, each script block ran in isolation, making it challenging to build workflows that relied on state changes like setting environment variables or changing directories. With this update, Runbooks now behave more like an interactive shell, where environment modifications and working directory changes persist across subsequent script executions within the same session. This enhancement significantly improves the usability for complex, multi-step runbooks and includes support for multiple browser tabs to interact with the same shared session. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a major and well-designed feature: a persistent execution environment for runbooks. The implementation is comprehensive, covering backend session management with thread-safe logic, frontend context providers for token handling, and updated documentation. The use of bearer tokens for session authentication is a good security measure. The code is generally of high quality, with thorough test coverage for the new session logic.
My review focuses on a few key areas to further improve robustness and maintainability:
- A critical bug in environment variable filtering.
- A potential race condition in token validation.
- Opportunities to reduce code duplication in API handlers.
- More robust error handling for session updates.
Overall, this is an excellent contribution that significantly enhances the capabilities of Runbooks. The following suggestions aim to polish the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 5
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Document in docs and comments, and update code to make it explicit that only bash scripts can propagate env vars.
…rd way to write files from scripts to the files panel.
|
/gemini Can you review the most recent commit? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a robust persistent execution model, which is a significant improvement over the previous stateless execution. The implementation is well-structured, with a thread-safe SessionManager on the backend, a clean SessionProvider on the frontend, and comprehensive unit tests. The new model, which maintains environment state across script executions, is a major feature that will greatly improve the usability of Runbooks for multi-step workflows. The changes to file capture, now using a standardized $RUNBOOKS_OUTPUT environment variable, are also a welcome simplification. My feedback includes a few suggestions for improving maintainability and error handling in the backend logic. Overall, this is an excellent and well-executed pull request.
| # Save user's handler to call during exit | ||
| __RUNBOOKS_USER_EXIT_HANDLER="$handler" | ||
| fi | ||
| return 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trap override mishandles query flag as handler
Medium Severity
The trap() override function in the wrapper script incorrectly handles the -p (print) flag. When a user script calls trap -p EXIT to query the current trap handler, the override sees two arguments containing "EXIT" and treats -p as if it were a handler command. This stores "-p" as __RUNBOOKS_USER_EXIT_HANDLER, overwriting any previously registered cleanup handler. On exit, eval "-p" fails silently due to || true, and the user's actual cleanup code never executes. The fix would be to check if $1 is -p or -l and pass through to builtin trap before the EXIT interception logic.
Also squeeze in an update to an error message and cursorrules.
| __RUNBOOKS_USER_EXIT_HANDLER="$handler" | ||
| fi | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trap override drops non-EXIT signals when combined
Medium Severity
The trap override function silently drops non-EXIT signal handlers when EXIT is combined with other signals. When a user script calls trap 'cleanup' EXIT SIGINT, the function detects EXIT, saves the handler for EXIT interception, then returns at line 470 without calling builtin trap for SIGINT. This means the SIGINT (or any other combined signal) trap is completely lost. Scripts that use compound traps like trap 'cleanup' EXIT SIGINT SIGTERM for comprehensive cleanup will have only their EXIT handler honored while other signal handlers are silently ignored.
| default: | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bash wrapper executed with sh fails on non-bash systems
High Severity
The isBashInterpreter function incorrectly treats sh as bash-compatible, but the wrapped script is executed with the original detected interpreter. When a script has #!/bin/sh, the bash wrapper (containing bash-specific syntax like [[ ]] and builtin trap) is applied at line 174, but the script is executed with sh at line 197. On systems where /bin/sh is not bash (Debian/Ubuntu with dash, Alpine with busybox ash, BSD systems), the bash-specific syntax causes immediate syntax errors, completely breaking environment persistence for sh scripts. The interpreter should be overridden to bash when the wrapper is applied, or sh should not be considered bash-compatible.
Previously, Runbooks had a confusing shell execution model. Each Command or Check block inherited from the original environment, but didn't persist changes to the environment. We sort of had a concept of sandboxing and isolation, but it was not clearly specified.
Persistent Execution Environment
With this PR, Runbooks now maintains environment state across script execution, just like a persistent terminal session. For example, the following is now possible:
export AWS_REGION=us-east-1)$AWS_REGIONunset AWS_REGION)Here are some other features that are included.
Capture files by writing them to `$RUNBOOKS_OUTPUT
Previously, we had a confusing approach to capturing files generated from scripts.
Now users need only write files to
$RUNBOOKS_OUTPUT/optionally/some/dir, and those files will be automatically written to/generated/optionally/some/dir(or whatever output path the user has selected.Support for multi-line env vars
Runbooks will also support multi-line env vars like this:
Multi-tab support
Opening the same runbook in multiple browser tabs works correctly. All tabs share the same environment state, like having multiple terminal windows connected to the same shell session. Changes made in one tab are visible in all others.
Clear session lifecycle
We track env vars in a "session."
Docs
Updated docs, and added two new Runbooks showcasing the features!
Note
Implements a persistent execution model with session-scoped env/dir state and simplified file capture.
Backend (api):
SessionManager) with token-based auth and endpoints:POST /api/session,POST /api/session/join,GET/POST/DELETE /api/session/*HandleExecRequestnow validates Bearer tokens, persists env/pwd for bash scripts (wrapper +env -0), setsRUNBOOKS_OUTPUT, and copies files to generated dir viacopyFilesFromCaptureDir$RUNBOOKS_OUTPUT; adds multiline env parsing and filtering helpersFrontend (web):
SessionProvider/useSession; sendsAuthorizationheader on/api/execcaptureFilesprops fromCommandand execution hook; auto-updates file tree onfiles_capturedDocs & Examples:
$RUNBOOKS_OUTPUTdemo-runbook-execution-model,demo-runbook-capture-files-from-scriptsCI/Meta: minor workflow tweak (setup-task repo-token) and rules note (prefer OpenTofu).
Written by Cursor Bugbot for commit 8686408. This will update automatically on new commits. Configure here.