Add aio runtime sandbox exec command by riddhi2910 · Pull Request #430 · adobe/aio-cli-plugin-runtime

riddhi2910 · 2026-06-11T21:27:23Z

Description

Adds a new aio runtime sandbox exec command. it is an non-interactive counterpart to aio runtime sandbox run. It creates a sandbox, runs one or more commands, then tears the sandbox down.

Commands can be provided as:

a one-shot quoted argument: aio runtime sandbox exec "node --version"
a newline-separated list via --file: aio runtime sandbox exec --file commands.txt (-f)
both the one-shot command runs first, then the file's commands in order
It supports the same sandbox flags as run (--name, --port, --egress, --max-lifetime) plus two exec-specific flags: --command-timeout (per-command, default 30s) and --fail-fast (stop on the first non-zero exit; otherwise all commands run and the process exits with the last non-zero code). run now emits a soft hint pointing users to exec when they pass a command or piped input.

Related Issue

https://jira.corp.adobe.com/browse/ACNA-4665

Motivation and Context

run is only for interactive use. It does not support "command" or commands from stdin. We also found that using file redirection could cause issues.

exec solves this problem. It reads all commands from stdin before running them, so exec -f commands.txt works 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 keeping run for interactive use.

How Has This Been Tested?

npm lint
npm test
manually tried different exec commands by linking the local version to the aio cli
e.g

aio runtime sandbox exec "echo hello world"                    
 ›   Warning: @adobe/aio-cli update available from 11.0.2 to 11.1.2.
 ›   Run npm install -g @adobe/aio-cli to update.
 ›   Warning: @adobe/aio-cli-plugin-api-mesh update available from 5.6.1 to 5.7.0
 ›   Warning: Run aio plugins:install @adobe/aio-cli-plugin-api-mesh to update to 
 ›   the latest

Creating sandbox...
[aio-lib-sandbox] alpha — APIs may change without notice
Created: sb-c8453286e1c54a57
Network policy: default-deny (DNS + NATS only)

$ echo hello world
hello world
[exit: 0]
Sandbox destroyed.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

github-actions

🤖 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 · 2026-06-11T21:28:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

MichaelGoberling

Looks great! Just a few suggestions

MichaelGoberling · 2026-06-12T13:33:31Z

+  shellQuote
+} = require('../../../sandbox-helpers')
+
+const COMMAND_TIMEOUT_MS = 30000


Suggestion: DEFAULT_COMMAND_TIMEOUT_MS so its clear

MichaelGoberling · 2026-06-12T13:47:14Z


    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.')


I don't think "-- command" works with run, its solely for interactive sessions, so we can probably remove specifying this here

MichaelGoberling · 2026-06-12T13:50:41Z

+    process.exitCode = 2
+  }
+
+  _logPolicy (policy) {


Run and exec both use these identically I believe, would be good to have in the same place

MichaelGoberling · 2026-06-12T13:53:17Z

+    })
+  }
+
+  async _logPreviewUrls (sandbox, ports) {


Could add to a shared lib and re-use here as well I believe

github-actions

🤖 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

Superseded by new review

github-actions

🤖 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

Superseded by new review

github-actions

🤖 PR Reviewer

The diff introduces a new sandbox exec command with good structure, comprehensive tests, and clean extraction of shared helpers (logPolicy, logPreviewUrls, buildCommandList) into sandbox-helpers.js. The code is well-documented and test coverage is thorough. One minor issue: the copyright year says 2026 in both new files, but that's cosmetic. There's one real concern in _readStdin — it attaches listeners directly to process.stdin without removing them on success, which can cause listener leaks if run() is called multiple times (e.g. in tests).

✅ LGTM! This PR looks good to merge.

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

github-actions · 2026-06-16T22:17:54Z

+      }
+    }
+  }
+
+  _readStdin () {
+    return new Promise((resolve, reject) => {
+      const chunks = []
+      process.stdin.on('data', chunk => chunks.push(chunk))


The _readStdin method attaches listeners to process.stdin but only removes them in the error path (via the reject callback) — on success there's no cleanup. In a long-lived process or repeated test runs this can accumulate listeners. Use once instead of on, or explicitly remove listeners in the end handler.

Suggested change

}

}

}

_readStdin () {

return new Promise((resolve, reject) => {

const chunks = []

process.stdin.on('data', chunk => chunks.push(chunk))

_readStdin () {

return new Promise((resolve, reject) => {

const chunks = []

const onData = chunk => chunks.push(chunk)

const onEnd = () => {

cleanup()

resolve(Buffer.concat(chunks).toString())

}

const onError = err => {

cleanup()

reject(err)

}

const cleanup = () => {

process.stdin.removeListener('data', onData)

process.stdin.removeListener('end', onEnd)

process.stdin.removeListener('error', onError)

}

process.stdin.on('data', onData)

process.stdin.on('end', onEnd)

process.stdin.on('error', onError)

})

}

Superseded by new review

github-actions

🤖 PR Reviewer

The previous suggestion about _readStdin listener cleanup has been addressed — the new implementation properly calls teardown() in both the resolve and reject paths. The new exec.js command is well-structured with good error handling, proper cleanup in finally, and comprehensive tests. No significant issues remain.

✅ LGTM! This PR looks good to merge.

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

…PowerShell

github-actions

🤖 PR Reviewer

The diff introduces a new sandbox exec command with solid structure: clean separation of concerns via sandbox-helpers.js, proper stdin handling with listener cleanup, reasonable error handling, and good test coverage. A few minor issues worth noting: the copyright year says 2026, the stdinPromise.catch(() => {}) suppression is a bit fragile, and there's a subtle issue with the no-command error path when stdin is TTY where _readStdin is not called but the code still awaits the pre-created promise which resolved to empty string — that's actually fine. Overall the code is well-written and ready to merge.

✅ LGTM! This PR looks good to merge.

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

github-actions · 2026-06-17T01:25:28Z

+    // parse, and attaching listeners afterwards would miss them.
+    const stdinPromise = process.stdin.isTTY === true ? Promise.resolve('') : this._readStdin()
+    stdinPromise.catch(() => {}) // parse may reject first; avoid an unhandled rejection
+


The .catch(() => {}) silently swallows any stdin read error when the parse path resolves first. If stdin errors and the command proceeds (e.g. due to a one-shot arg), the error is silently dropped. Consider logging it or only suppressing the specific unhandled-rejection scenario more precisely. This is a minor concern since in practice a stdin error here would be unusual.

Suggested change

stdinPromise.catch((_err) => {}) // prevent unhandled rejection if parse resolves first; error surfaced via await below

Superseded by new review

github-actions

🤖 PR Reviewer

The diff introduces a new sandbox exec command with solid test coverage, properly refactors shared helpers into sandbox-helpers.js, and updates the README documentation. The code is well-structured with good error handling and the stdin race-condition fix is thoughtful. One previous suggestion about the silent .catch(() => {}) remains, though the user accepted the copyright year concern.

✅ LGTM! This PR looks good to merge.

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

github-actions · 2026-06-17T01:35:23Z

+    const stdinPromise = process.stdin.isTTY === true ? Promise.resolve('') : this._readStdin()
+    // Avoid an unhandled rejection if parse rejects before we await this; a real
+    // stdin read error still surfaces at the `await stdinPromise` below.
+    stdinPromise.catch(() => {})


[Re-raised] The .catch(() => {}) silently swallows any stdin read error when the parse path resolves first. If stdin errors and the command proceeds, the error is silently dropped. Consider a comment clarifying intent or logging the suppressed error.

Suggested change

stdinPromise.catch(() => {})

stdinPromise.catch((_err) => {}) // suppress unhandled-rejection only; real errors surface at `await stdinPromise` below

Superseded by new review

github-actions

🤖 PR Reviewer

The new exec command is well-structured, thoroughly tested, and the refactoring of shared helpers (logPolicy, logPreviewUrls, buildCommandList) into sandbox-helpers.js is clean. The previous suggestion about .catch(() => {}) is no longer applicable since stdin piping was replaced with a --file flag approach. No significant issues found.

✅ LGTM! This PR looks good to merge.

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

Superseded by new review

Add aio runtime sandbox exec command

9cbae64

github-actions Bot previously approved these changes Jun 11, 2026

View reviewed changes

Comment thread src/commands/runtime/sandbox/exec.js

Comment thread test/commands/runtime/sandbox/exec.test.js

MichaelGoberling approved these changes Jun 12, 2026

View reviewed changes

Fix constant naming and duplicated logging

995b0f2

github-actions Bot previously requested changes Jun 12, 2026

View reviewed changes

Comment thread test/commands/runtime/sandbox/exec.test.js

Fix sandbox exec non-interactively

a5aeebe

github-actions Bot previously approved these changes Jun 12, 2026

View reviewed changes

Replace end of options delimiter to quoted string argument

4d1d377

github-actions Bot previously approved these changes Jun 16, 2026

View reviewed changes

Add suggested changes

d88b4f4

github-actions Bot previously approved these changes Jun 16, 2026

View reviewed changes

read stdin before flag parse to avoid missing piped input on Windows …

ffefa23

…PowerShell

github-actions Bot previously approved these changes Jun 17, 2026

View reviewed changes

clarify stdinPromise.catch comment that errors still surface via await

bb65b0a

github-actions Bot previously approved these changes Jun 17, 2026

View reviewed changes

accept command as quoted arg + --file list; drop -- and stdin

4218008

github-actions Bot approved these changes Jun 17, 2026

View reviewed changes

riddhi2910 requested a review from MichaelGoberling June 17, 2026 16:16

MichaelGoberling approved these changes Jun 17, 2026

View reviewed changes

riddhi2910 merged commit 850c823 into master Jun 17, 2026
11 checks passed

-      }
-    }
-  }
-  _readStdin () {
-    return new Promise((resolve, reject) => {
-      const chunks = []
-      process.stdin.on('data', chunk => chunks.push(chunk))
+  _readStdin () {
+    return new Promise((resolve, reject) => {
+      const chunks = []
+      const onData = chunk => chunks.push(chunk)
+      const onEnd = () => {
+        cleanup()
+        resolve(Buffer.concat(chunks).toString())
+      }
+      const onError = err => {
+        cleanup()
+        reject(err)
+      }
+      const cleanup = () => {
+        process.stdin.removeListener('data', onData)
+        process.stdin.removeListener('end', onEnd)
+        process.stdin.removeListener('error', onError)
+      }
+      process.stdin.on('data', onData)
+      process.stdin.on('end', onEnd)
+      process.stdin.on('error', onError)
+    })
+  }


	stdinPromise.catch((_err) => {}) // prevent unhandled rejection if parse resolves first; error surfaced via await below

	stdinPromise.catch(() => {})
	stdinPromise.catch((_err) => {}) // suppress unhandled-rejection only; real errors surface at `await stdinPromise` below

Uh oh!

Conversation

riddhi2910 commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 11, 2026

Codecov Report

Uh oh!

MichaelGoberling left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelGoberling Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelGoberling Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelGoberling Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelGoberling Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

github-actions Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 PR Reviewer

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

riddhi2910 commented Jun 11, 2026 •

edited

Loading