feat: support for agent sandboxes

[MichaelGoberling]

npm install github:adobe/aio-lib-runtime#agent-sandboxes

const RuntimeAPI = require('@adobe/aio-lib-runtime/src/RuntimeAPI')

async function main () {
  const runtime = await new RuntimeAPI().init({
    apihost: "https://va6.next-adobeioruntime.net/",
    namespace: "test_namespace",
    api_key: "test_key"
  })

  const sandbox = await runtime.compute.sandbox.create({
    name: 'my-sandbox',
    size: 'MEDIUM',
    type: 'cpu:nodejs',
    workspace: 'workspace',
    maxLifetime: 3600,
    envs: {
      API_KEY: 'your-api-key'
    }
  })

  console.log('sandbox ready:', sandbox.id)

  const status = await runtime.compute.sandbox.getStatus(sandbox.id)
  console.log('status:', status)

  console.log('executing command...')
  const result = await sandbox.exec('ls -al', { timeout: 10000 })
  console.log('stdout:', result.stdout.trim())
  console.log('exit code:', result.exitCode)

  // write a file using the file API instead of echo-redirects
  console.log('writing file via writeFile...')
  const script = `console.log('hello from sandbox script', process.version)\n`
  const writeResult = await sandbox.writeFile('hello.js', script)
  console.log('writeFile result:', writeResult)

  // read it back to confirm
  console.log('reading file back via readFile...')
  const content = await sandbox.readFile('hello.js')
  console.log('readFile content:', content.trim())

  // list the directory
  console.log('listing /workspace via listFiles...')
  const entries = await sandbox.listFiles('.')
  console.log('listFiles entries:', entries)

  // run the script
  console.log('running the script...')
  const result4 = await sandbox.exec('node hello.js', { timeout: 10000 })
  console.log('stdout:', result4.stdout.trim())
  console.log('stderr:', result4.stderr.trim())
  console.log('exit code:', result4.exitCode)

  await sandbox.destroy()
  console.log('sandbox destroyed')
}

main().catch(err => { console.error(err); process.exit(1) })

[github-actions]

🤖 PR Reviewer

The implementation is well-structured with good test coverage, proper error typing, and clean separation between SandboxAPI and Sandbox classes. A few notable concerns include credential exposure via logging, inconsistent property name (timeout vs timeoutMs) between types.d.ts and actual implementation, and repeated https.Agent instantiation in two places that could be refactored into a shared helper.

📝 6 suggestion(s) - Please review inline comments below.

---

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

⚠️ Inline comments could not be attached (lines not in diff). See summary above.

[github-actions]

🤖 PR Reviewer

This PR adds a well-structured Sandbox/ComputeAPI feature with WebSocket-based command execution, file management, and network policy support. The code is generally clean with good test coverage, but there are several issues: a missing connect() method implementation in SandboxAPI, an inconsistency between the TypeScript declaration and implementation, a potential resource leak in the https require pattern, and a security concern around API key handling in headers.

📝 7 suggestion(s) - Please review inline comments below.

---

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

[github-actions]

The SandboxAPI class is missing a connect() method but it is declared in types.d.ts (line 232: connect(options: SandboxConnectOptions): Promise<Sandbox>). Without this method, users cannot reconnect to an existing sandbox. The _buildSandbox helper already does the heavy lifting, so this should be straightforward to add.

Suggested change

	async connect (options = {}) {
	const payload = await this._request(
	'get sandbox status',
	'GET',
	`${this._getSandboxPath()}/${options.sandboxId}`
	)
	payload.token = options.token || payload.token
	return await this._buildSandbox(payload)
	}

[github-actions]

Requiring node:https inside a hot path (_request is called for every API operation) is inefficient — Node caches modules but the conditional expression still runs on every call. Move the require to the top of the file.

Suggested change

	const https = require('node:https')
	const https = require('node:https')

[github-actions]

Same pattern: require('node:https') is called inside getStatus() which may be called repeatedly. Move it to the top of the file alongside other requires.

Suggested change

	const opPromise = new Promise((resolve, reject) => {
	const https = require('node:https')

[github-actions]

The buildAuthorizationHeader function base64-encodes the raw API key string. If the key is already in uuid:password format this is correct for Basic auth, but the function name and JSDoc say 'uuid:key format' without validating or warning. More importantly, the API key is held in memory in plain string form and passed through multiple layers. Consider at minimum documenting that callers must not log the return value, and add a guard for empty/undefined keys to avoid silently sending an empty Authorization header.

Suggested change

	return `Basic ${Buffer.from(apiKey).toString('base64')}`
	function buildAuthorizationHeader (apiKey) {
	if (!apiKey) {
	throw new Error('apiKey is required to build an Authorization header')
	}
	return `Basic ${Buffer.from(apiKey).toString('base64')}`
	}

[github-actions]

The SandboxExecOptions type declares timeoutMs but the implementation in Sandbox.js uses timeout (line 163: options.timeout). This mismatch means TypeScript users passing timeoutMs will silently get no timeout behaviour.

Suggested change

	declare type SandboxExecOptions = {
	onOutput?: (...params: any[]) => any;
	timeout?: number;
	stdin?: string | Buffer;
	};

[github-actions]

The connect() method adds a permanent message listener via socket.on('message', onMessage) for the auth handshake, but also registers the general socket.on('message', message => this._handleMessage(message)) listener at line 78. During the auth phase both listeners fire for every message. The cleanup() function removes the auth-phase onMessage listener correctly, but if the WebSocket emits messages before auth.ok, _handleMessage will also be called with them (and safely no-op). This is acceptable, but a comment explaining the dual-listener approach would improve clarity. More critically, if connect() is called again after a close (new socket), the old permanent _handleMessage listener from line 78 is on the old socket which is replaced — this is fine. No code change needed, but add a comment.

Suggested change

	cleanup()
	// Two message listeners are registered: the permanent _handleMessage for
	// all post-auth frames, and a one-shot onMessage solely to detect auth.ok.
	// Both are harmless during the handshake since _handleMessage ignores auth frames.
	socket.on('message', message => this._handleMessage(message))

[github-actions]

When exec() successfully sends the exec.run frame but then writeStdin or closeStdin throws (e.g. due to a race where the socket closes between frames), the catch block rejects the promise, but the exec.run frame has already been sent to the server. The server may still send back exec.output/exec.exit frames for this execId after the client has rejected and deleted it from _pendingExecs. Those frames will be silently dropped by _handleMessage, which is acceptable, but worth a comment.

Suggested change

	this._ensureOpen()
	// If writeStdin/closeStdin fails after exec.run was sent, we reject
	// locally. The server may still emit output/exit frames for this execId
	// which will be silently ignored by _handleMessage.
	this._rejectPendingExec(execId, new codes.ERROR_SANDBOX_WEBSOCKET({

Superseded by new review

[github-actions]

🤖 PR Reviewer

The diff introduces a well-structured Sandbox/SandboxAPI/ComputeAPI subsystem with good test coverage. Several issues from the previous review round remain unaddressed (require inside hot path, missing apiKey guard, timeoutMs/timeout mismatch in types.d.ts), and a few new issues are present including the connect method on Sandbox registering two permanent message listeners if called after reconnect, and the copyright year '2026' across new files.

📝 9 suggestion(s) (5 new, 4 re-raised)

---

💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

[github-actions]

[Re-raised] SandboxExecOptions declares timeoutMs but the implementation in Sandbox.js reads options.timeout. TypeScript users passing timeoutMs will silently get no timeout behaviour.

Suggested change

	LARGE: SandboxSize;
	declare type SandboxExecOptions = {
	onOutput?: (...params: any[]) => any;
	timeout?: number;
	stdin?: string | Buffer;
	};

[github-actions]

[Re-raised] buildAuthorizationHeader silently sends an empty/broken Authorization header when apiKey is undefined or empty. Add a guard.

Suggested change

	}
	function buildAuthorizationHeader (apiKey) {
	if (!apiKey) {
	throw new Error('apiKey is required to build an Authorization header')
	}
	return `Basic ${Buffer.from(apiKey).toString('base64')}`
	}

[github-actions]

[Re-raised] require('node:https') is called inside _request, which is on the hot path for every API operation. Move it to the top of the file.

Suggested change

	_buildAuthorizationHeader () {
	const https = require('node:https')

[github-actions]

[Re-raised] Same pattern: require('node:https') inside getStatus() which may be called repeatedly. Move it to the top of the file.

Suggested change

	* @param {string|Buffer} content file contents
	const https = require('node:https')

[github-actions]

Every call to connect() after a socket close creates a new WebSocket and registers a new permanent socket.on('message', message => this._handleMessage(message)) listener on it. This is fine for the new socket, but if connect() is called multiple times (reconnect scenario), each new socket gets its own listener which is correct. However, there is a subtle bug: the onMessage auth-phase listener added at line 107 (socket.on('message', onMessage)) is never removed via socket.off if the socket emits open then auth.ok — cleanup() is called which does socket.off('message', onMessage) correctly. This path is safe. No code change needed here, but the dual-listener comment suggested in the previous review is still missing. Add it for clarity.

Suggested change

	socket.on('close', (code, reason) => this._handleClose(code, reason))
	// Two message listeners are registered: the permanent _handleMessage for
	// all post-auth frames, and a one-shot onMessage solely to detect auth.ok.
	// Both are harmless during the handshake since _handleMessage ignores auth frames.
	socket.on('message', message => this._handleMessage(message))

[github-actions]

When exec() successfully sends exec.run but writeStdin or closeStdin throws, the promise is rejected via _rejectPendingExec, but the server may still emit exec.output/exec.exit frames for this execId. These are silently dropped, which is acceptable, but a comment explaining this is still missing (re-raised from previous review).

Suggested change

	kill (execId, signal = 'SIGTERM') {
	// If writeStdin/closeStdin fails after exec.run was sent, we reject
	// locally. The server may still emit output/exit frames for this execId
	// which will be silently ignored by _handleMessage.
	this._rejectPendingExec(execId, new codes.ERROR_SANDBOX_WEBSOCKET({

[github-actions]

Copyright year reads '2026' which is in the future. Same issue exists in Sandbox.js and SandboxAPI.js.

Suggested change

	Copyright 2026 Adobe. All rights reserved.
	Copyright 2024 Adobe. All rights reserved.

[github-actions]

The TypeScript class is named ComputeSandbox but the actual JS class exported from SandboxAPI.js is SandboxAPI. The type name mismatch means generated .d.ts consumers cannot use it correctly. Rename to match the implementation, or update the JS export.

Suggested change

	/**
	declare class SandboxAPI {

[github-actions]

In destroy(), require('node:https') is also called inside the method body (same hot-path pattern as getStatus). Move to top of file.

Suggested change

	requestOptions.agent = new https.Agent({ rejectUnauthorized: false })
	const https = require('node:https')

Superseded by new review