Add aio runtime sandbox exec command#430
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 PR Reviewer
The new sandbox exec command is well-structured, clearly documented, and thoroughly tested. The code follows established patterns from the existing run command, handles error cases gracefully, and the test suite provides good coverage including edge cases. One minor issue: the copyright year says 2026 (future-dated) in both new files, and there's an unused import (shellQuote is imported in exec.js but only re-exported for testing, not used directly in the command logic — this is fine). Overall the implementation is solid.
✅ LGTM! This PR looks good to merge.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
MichaelGoberling
left a comment
MichaelGoberling
left a comment
There was a problem hiding this comment.
Looks great! Just a few suggestions
| shellQuote | ||
| } = require('../../../sandbox-helpers') | ||
|
|
||
| const COMMAND_TIMEOUT_MS = 30000 |
There was a problem hiding this comment.
Suggestion: DEFAULT_COMMAND_TIMEOUT_MS so its clear
|
|
||
| if (commandArgs.length > 0) { | ||
| this._failUsage('This command only supports interactive use. Omit "-- <command>" and type commands when prompted.') | ||
| this._failUsage('This command only supports interactive use. Omit "-- <command>" and type commands when prompted, or use "aio runtime sandbox exec" for one-shot or scripted commands.') |
There was a problem hiding this comment.
I don't think "-- command" works with run, its solely for interactive sessions, so we can probably remove specifying this here
| process.exitCode = 2 | ||
| } | ||
|
|
||
| _logPolicy (policy) { |
There was a problem hiding this comment.
Run and exec both use these identically I believe, would be good to have in the same place
| }) | ||
| } | ||
|
|
||
| async _logPreviewUrls (sandbox, ports) { |
There was a problem hiding this comment.
Could add to a shared lib and re-use here as well I believe
There was a problem hiding this comment.
🤖 PR Reviewer
The diff is well-structured: it refactors shared logging helpers into sandbox-helpers.js, adds a clean non-interactive exec command with good test coverage, and updates error messages to reference the new command. One minor issue remains in the test file where the description assertion checks for 'sandbox run' instead of 'sandbox exec'. The copyright year concern was acknowledged as intentional by the author.
📝 1 suggestion(s) - Please review inline comments below.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
There was a problem hiding this comment.
🤖 PR Reviewer
The code is well-structured, with good separation of concerns (shared helpers extracted to sandbox-helpers.js), solid test coverage, and clear error handling. The previously raised description test issue has been fixed — the test now correctly matches /non-interactively/. No significant issues remain.
✅ LGTM! This PR looks good to merge.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
2 participants
Description
Adds a new aio runtime sandbox exec command. it is the non-interactive counterpart to aio runtime sandbox run. It creates a sandbox, runs one or more commands, then tears the sandbox down.
Related Issue
https://jira.corp.adobe.com/browse/ACNA-4665
Motivation and Context
runis only for interactive use. It does not support-- <command>or commands from stdin. We also found that using file redirection could cause issues.execsolves this problem. It reads all commands from stdin before running them, soexec < commands.txtworks reliably. This gives us a simple way to run one-time commands or a list of commands in scripts, CI pipelines, or automation tools, while keepingrunfor interactive use.How Has This Been Tested?
aio clie.g
Screenshots (if appropriate):
Types of changes
Checklist: