feat: close the composable local runtime loop#272
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughThe PR adds a sandboxed local-preview runtime, expands ChangesComposable runtime and CLI
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (2)
.github/workflows/publish.yml (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the
npm@11pinWith Node
26.5.0here,npm@12is supported, so the note aboutnpm 12dropping Node22.14support is stale. Consider unpinning or updating the comment and version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yml at line 69, Update the Node.js toolchain configuration at node-version to align with npm@11: either use a Node version compatible with the existing npm pin or update the npm version and related comment to reflect Node 26.5.0 and npm@12 support. Remove the stale Node 22.14 compatibility note.packages/cli/src/integrations-command.test.ts (1)
19-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert both registration-health render paths.
The generic match only verifies connection-level output. Add a separate assertion for the workspace-level
Registration health:section.Proposed assertions
- assert.match(io.stdout.text, /registrationHealth/); + assert.match( + io.stdout.text, + /registrationHealth: {"registered":true,"healthy":true}/ + ); + assert.match( + io.stdout.text, + /Registration health:\n {"workspace":{"registered":true,"healthy":true}}/ + );Also applies to: 174-174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/integrations-command.test.ts` around lines 19 - 26, Update the integrations command test around the registrationHealth fixture to assert both rendering paths: retain the connection-level registration-health assertion and add a distinct assertion for the workspace-level “Registration health:” section. Ensure the workspace assertion verifies the expected rendered health values rather than relying on the generic connection match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/invoke-command.ts`:
- Around line 543-566: Update aggregateCaseRecords so state evidence from every
turn is preserved instead of copying only final.stateDiff. Compute an
initial-to-final stateDiff or retain the per-turn stateDiff values in the
aggregated RunRecord, while leaving the existing actions, trace, status, and
turn metadata aggregation unchanged.
- Around line 193-205: Update the scaffold-mode validation in the invoke command
to include the parsed --reads, --model, and --watch options in the invalid flags
list. Preserve the existing rejection for all other non-output flags and ensure
any of these options causes the existing error path to run.
- Around line 851-866: Update collectWatchedFiles to discover dependencies from
both prepared.personaPath and prepared.compiled.handlerEntry, ensuring
split-file personas and bare handlers resolved to sibling personas watch both
sides. Reuse the prepared invocation data and preserve the existing source,
case-fixture, seed, and sorted-result behavior.
- Around line 424-459: The executeLocalRun call in the normalized replay flow
must mark inputs as current whenever CLI overrides are applied, even when
normalized.sourceFidelity exists. Update the sourceFidelity construction around
inputs and opts.inputs so the effective override takes precedence over the
preserved bundle fidelity, while retaining historical or unavailable values when
no current overrides are supplied.
In `@packages/cli/src/invoke/case-file.ts`:
- Around line 103-114: Replace the Math.random-based ID generation in the
cron.tick branch with a deterministic ID derived from the case ID and turn
index. Thread or reuse those values from the surrounding case-event decoding
flow, preserving the existing ID format semantics while ensuring identical case
files produce identical events and RunRecords.
- Around line 117-158: Update the Slack handling branch around the type check so
it only accepts the supported slack.message.created event, rather than treating
every slack.* value as that event. Preserve the existing normalization for
slack.message.created and reject unsupported Slack event types using the
established validation behavior.
- Around line 229-235: Update the provider action mapping in the case-file
parser to validate action.threaded as a path-specific boolean rather than
coercing it with Boolean(). Preserve omission when threaded is undefined, and
reject any provided non-boolean value using the existing validation convention
and `${pathLabel}.providerActions[${index}].threaded` path.
- Around line 176-186: Update normalizeHttpFixtures to validate record.method as
an allowed HTTP method limited to GET or HEAD before constructing
ParsedCaseHttpFixture entries. Preserve the existing method parsing and reject
any other method so runCaseInvoke cannot authorize mutating requests through
allowedHttp.
In `@packages/deploy/src/integrations-list.ts`:
- Around line 483-487: Update the summary construction in the integrations-list
health-record flow so it returns a summary only when at least one of registered,
healthy, or reason is present. Include adapter and checkedAt only as optional
metadata after that health signal exists, and continue returning undefined when
no health signal is available.
In `@packages/persona-kit/schemas/persona.schema.json`:
- Around line 543-576: The PersonaHttpReadCapability and PersonaHttpReadRule
schemas must match parseHttpReadCapability’s fail-closed validation: disallow
unknown properties in both objects and require urlGlob to be non-blank, while
preserving the existing method enum and required fields. If persona.schema.json
is generated, update the source type/schema annotations that generate these
definitions as well.
In `@packages/persona-kit/src/types.ts`:
- Around line 429-432: Remove the open index signature from
PersonaHttpReadCapability so the type only exposes the supported enabled and
allow fields. Keep parseHttpReadCapability’s strict key validation unchanged and
ensure callers cannot rely on arbitrary extra properties.
In `@packages/runtime/src/local-preview-child.ts`:
- Around line 36-45: Update the process disconnect handler to clear each pending
fetch and model request’s associated IPC timeout before rejecting it, using the
timer references stored by pendingFetches and pendingModels. Preserve the
existing rejection and map-clearing behavior while ensuring no orphaned timers
remain after parent disconnect.
In `@packages/runtime/src/local-preview-executor.ts`:
- Around line 86-92: Update both file-write paths in the local preview executor
to calculate evidence bytes with Buffer.byteLength(contents, 'utf8') instead of
contents.length. Apply this change to the writeFile implementation and the
additional write path identified by the second recordAction block, preserving
the existing action data structure.
- Around line 41-45: Update the local preview execution flow around memory
initialization and persistence to derive the next memory ID sequence from the
seeded memory entries, preventing collisions with existing IDs such as mem_1.
Track the seeded memory boundary and generate stateDiff.memory using only
entries added during the current run, including the corresponding logic in the
other affected memory-save paths.
In `@packages/runtime/src/local-preview-hooks.ts`:
- Around line 72-77: Update the originalNet capture and corresponding
network-guard restoration logic around Socket to also include net.BoundSocket.
Ensure BoundSocket is saved, denied during preview, and restored afterward
alongside the existing Socket handling.
In `@packages/runtime/src/local-preview-redaction.ts`:
- Around line 48-53: Update redactPattern to identify the earliest occurring ':'
or '=' in each match, rather than prioritizing ':' whenever present. Use that
first separator to split the prefix and redact everything after it, preserving
REDACTED for matches without either separator.
In `@packages/runtime/src/local-preview.ts`:
- Around line 487-524: Update the local preview fetch flow around the fetch call
and response.arrayBuffer() to keep the timeout active until the response body is
fully consumed. Replace unbounded arrayBuffer buffering with streaming into a
bounded buffer that enforces the configured response size limit, aborting or
rejecting when the limit is exceeded, while preserving the existing response
encoding and timeout cleanup behavior.
- Around line 337-340: Update the model-request flow around respondToModel and
onModelRequest so pending work is genuinely cancellable: create and retain an
AbortController per request, propagate its signal through the callback and
provider adapter, and abort it when the worker closes or cleanup begins. Ensure
pending-response cleanup does not wait for an uncancellable provider timeout.
- Around line 780-787: Update the header filtering condition in the local
preview request-header loop so isCredentialLikeHeaderName(normalized) is
evaluated for every header name, not only names beginning with x-. Continue
stripping matching credential-like names or values, while preserving
STRIP_LIVE_REQUEST_HEADERS handling and appending only non-sensitive headers.
- Around line 318-326: Bound the stderr accumulation in the local preview worker
flow by retaining only a fixed-size tail of the captured output. Update the
`child.stderr` data handler and `stderr` state so continuous worker writes
cannot grow memory without limit, while preserving the most recent stderr
content for error reporting.
---
Nitpick comments:
In @.github/workflows/publish.yml:
- Line 69: Update the Node.js toolchain configuration at node-version to align
with npm@11: either use a Node version compatible with the existing npm pin or
update the npm version and related comment to reflect Node 26.5.0 and npm@12
support. Remove the stale Node 22.14 compatibility note.
In `@packages/cli/src/integrations-command.test.ts`:
- Around line 19-26: Update the integrations command test around the
registrationHealth fixture to assert both rendering paths: retain the
connection-level registration-health assertion and add a distinct assertion for
the workspace-level “Registration health:” section. Ensure the workspace
assertion verifies the expected rendered health values rather than relying on
the generic connection match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dd6a93a-b0d1-4e30-92c5-1ec35a056570
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
.github/workflows/ci.yml.github/workflows/deploy-e2e.yml.github/workflows/publish.yml.github/workflows/verify-publish.yml.workflow-artifacts/composable-runtime-closure/reviews/workforce-installed-layout-repair.md.workflow-artifacts/composable-runtime-closure/workforce-implementation.mdpackages/cli/package.jsonpackages/cli/src/cli-impl.tspackages/cli/src/integrations-command.test.tspackages/cli/src/integrations-command.tspackages/cli/src/invoke-command.test.tspackages/cli/src/invoke-command.tspackages/cli/src/invoke/case-file.tspackages/cli/src/invoke/prepare-target.tspackages/cli/src/runs-command.test.tspackages/cli/src/runs-command.tspackages/delivery/package.jsonpackages/deploy/src/index.tspackages/deploy/src/integrations-list.test.tspackages/deploy/src/integrations-list.tspackages/persona-kit/schemas/persona.schema.jsonpackages/persona-kit/src/index.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.tspackages/persona-kit/src/types.tspackages/runtime/package.jsonpackages/runtime/src/index.tspackages/runtime/src/local-preview-child.tspackages/runtime/src/local-preview-contract.tspackages/runtime/src/local-preview-executor.tspackages/runtime/src/local-preview-hooks.tspackages/runtime/src/local-preview-redaction.tspackages/runtime/src/local-preview.test.tspackages/runtime/src/local-preview.tspackages/runtime/src/run-contracts.test.tspackages/runtime/src/run-contracts.ts
| if (scaffoldType) { | ||
| const invalidScaffoldFlags = [ | ||
| sawFixture ? '--fixture' : '', | ||
| const invalid = [ | ||
| fixturePath ? '--fixture' : '', | ||
| scheduleName ? '--schedule' : '', | ||
| casePath ? '--case' : '', | ||
| workspaceId ? '--workspace' : '', | ||
| Object.keys(inputs).length > 0 ? '--input' : '', | ||
| Object.keys(seeds).length > 0 ? '--seed' : '', | ||
| personaPath ? '<persona-path>' : '' | ||
| personaPath ? '<agent>' : '' | ||
| ].filter(Boolean); | ||
| if (invalidScaffoldFlags.length > 0) { | ||
| throw new Error( | ||
| `invoke: --scaffold only accepts --output; remove ${invalidScaffoldFlags.join(', ')}` | ||
| ); | ||
| if (invalid.length > 0) { | ||
| throw new Error(`invoke: --scaffold only accepts --output; remove ${invalid.join(', ')}`); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject all non-output flags in scaffold mode.
--reads, --model, and --watch are parsed but omitted from invalid, so commands such as invoke --scaffold cron.tick --watch silently succeed despite claiming only --output is accepted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke-command.ts` around lines 193 - 205, Update the
scaffold-mode validation in the invoke command to include the parsed --reads,
--model, and --watch options in the invalid flags list. Preserve the existing
rejection for all other non-output flags and ensure any of these options causes
the existing error path to run.
| const { event, historicalState, stateFidelity, replayProvenance } = normalized; | ||
| const replaySource = stateFidelity !== undefined; | ||
| const inputs = normalized.historicalInputs ? { ...normalized.historicalInputs, ...(opts.inputs ?? {}) } : (opts.inputs ?? {}); | ||
| const result = await executeLocalRun({ | ||
| request: { | ||
| schemaVersion: 1, | ||
| agent: compiled, | ||
| event, | ||
| mode: replaySource ? 'replay' : 'preview', | ||
| inputs, | ||
| policy, | ||
| state: { | ||
| schemaVersion: 1, | ||
| kind: replaySource ? 'replay' : initialState?.files ? 'fixtures' : 'empty', | ||
| fidelity: stateFidelity ?? (initialState?.files ? 'fixture' : 'simulated') | ||
| } | ||
| }, | ||
| bundlePath, | ||
| sourcePath: compiled.handlerEntry, | ||
| inputs, | ||
| state: mergeState(state, historicalState), | ||
| ...(replayProvenance ? { replayProvenance } : {}), | ||
| sourceFidelity: normalized.sourceFidelity ?? { | ||
| state: stateFidelity ?? (initialState?.files ? 'fixture' : 'simulated'), | ||
| inputs: normalized.historicalInputs | ||
| ? Object.keys(opts.inputs ?? {}).length > 0 ? 'current' : 'historical' | ||
| : opts.inputs && Object.keys(opts.inputs).length > 0 ? 'current' : 'unavailable', | ||
| http: replaySource ? 'unavailable' : policy.reads === 'live' ? 'current' : 'unavailable', | ||
| model: replaySource | ||
| ? 'unavailable' | ||
| : policy.model === 'live' | ||
| ? 'current' | ||
| : policy.model === 'stub' | ||
| ? 'simulated' | ||
| : 'unavailable' | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Mark replay inputs as current when CLI overrides are applied.
Line 426 overlays opts.inputs onto historical inputs, but Line 446 preserves the bundle’s original sourceFidelity. The resulting RunRecord can claim inputs: "historical" while executing with current values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke-command.ts` around lines 424 - 459, The
executeLocalRun call in the normalized replay flow must mark inputs as current
whenever CLI overrides are applied, even when normalized.sourceFidelity exists.
Update the sourceFidelity construction around inputs and opts.inputs so the
effective override takes precedence over the preserved bundle fidelity, while
retaining historical or unavailable values when no current overrides are
supplied.
| function aggregateCaseRecords( | ||
| parsedCase: ParsedInvokeCase, | ||
| records: RunRecordV2[], | ||
| logs: string[] | ||
| ): RunRecordV2 { | ||
| const final = records[records.length - 1]!; | ||
| const failed = records.some((record) => record.status !== 'succeeded'); | ||
| return { | ||
| ...final, | ||
| status: failed ? 'failed' : 'succeeded', | ||
| actions: records.flatMap((record) => record.actions), | ||
| trace: records.flatMap((record) => record.trace), | ||
| stateDiff: final.stateDiff, | ||
| extensions: { | ||
| ...(typeof final.extensions === 'object' && final.extensions !== null ? final.extensions as Record<string, unknown> : {}), | ||
| caseId: parsedCase.id, | ||
| logs, | ||
| turns: records.map((record) => ({ | ||
| runId: record.runId, | ||
| status: record.status, | ||
| eventContract: record.eventContract | ||
| })) | ||
| } | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve state evidence from every case turn.
Actions and traces are aggregated across all records, but stateDiff is copied only from the final turn. Earlier file or memory changes disappear from the aggregate RunRecord; compute an initial-to-final diff or retain per-turn diffs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke-command.ts` around lines 543 - 566, Update
aggregateCaseRecords so state evidence from every turn is preserved instead of
copying only final.stateDiff. Compute an initial-to-final stateDiff or retain
the per-turn stateDiff values in the aggregated RunRecord, while leaving the
existing actions, trace, status, and turn metadata aggregation unchanged.
| async function collectWatchedFiles(opts: InvokeOptions): Promise<string[]> { | ||
| const files = await collectRelativeDependencyFiles(opts.personaPath); | ||
| if (opts.source.kind === 'fixture') { | ||
| files.add(opts.source.path); | ||
| } else if (opts.source.kind === 'case') { | ||
| files.add(opts.source.path); | ||
| const parsedCase = await parseInvokeCase(opts.source.path); | ||
| for (const fixture of parsedCase.http) files.add(fixture.file); | ||
| for (const fixture of parsedCase.model) { | ||
| if (fixture.file) files.add(fixture.file); | ||
| } | ||
| } | ||
| for (const localFile of Object.values(opts.seeds ?? {})) { | ||
| files.add(path.resolve(localFile)); | ||
| } | ||
| return [...files].sort(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Watch both sides of split-file targets.
Dependency discovery starts only at opts.personaPath. A persona using an onEvent path will not watch its handler, while invoking a bare handler resolved to a sibling persona will not watch that persona. Include both prepared.personaPath and prepared.compiled.handlerEntry.
Based on PR objectives, installed and split-file persona layouts must remain supported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke-command.ts` around lines 851 - 866, Update
collectWatchedFiles to discover dependencies from both prepared.personaPath and
prepared.compiled.handlerEntry, ensuring split-file personas and bare handlers
resolved to sibling personas watch both sides. Reuse the prepared invocation
data and preserve the existing source, case-fixture, seed, and sorted-result
behavior.
| if (type === 'cron.tick') { | ||
| assertOnlyKeys(record, pathLabel, ['type', 'schedule', 'name', 'cron']); | ||
| const name = expectString(record.schedule ?? record.name, `${pathLabel}.schedule`); | ||
| const cron = optionalString(record.cron); | ||
| return decodeEventFrame({ | ||
| id: `${name}_${Math.random().toString(16).slice(2, 10)}`, | ||
| workspace: 'ws-local', | ||
| type: 'cron.tick', | ||
| occurredAt: '2026-07-15T09:00:00.000Z', | ||
| name, | ||
| ...(cron ? { cron } : {}) | ||
| }).frame; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove randomness from generated case event IDs.
Math.random() makes identical case files produce different events and RunRecords. Derive the ID deterministically from the case ID and turn index.
Based on PR objectives, local preview execution must be deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke/case-file.ts` around lines 103 - 114, Replace the
Math.random-based ID generation in the cron.tick branch with a deterministic ID
derived from the case ID and turn index. Thread or reuse those values from the
surrounding case-event decoding flow, preserving the existing ID format
semantics while ensuring identical case files produce identical events and
RunRecords.
| function redactPattern(value: string, pattern: RegExp): string { | ||
| return value.replace(pattern, (match) => { | ||
| const separator = match.includes(':') ? ':' : match.includes('=') ? '=' : ''; | ||
| if (!separator) return REDACTED; | ||
| const [prefix] = match.split(separator); | ||
| return `${prefix}${separator}${REDACTED}`; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact from the first assignment separator.
Line 50 chooses : whenever it occurs anywhere in the match. For SECRET_KEY=prefix:suffix, this leaks prefix. Select whichever of : or = occurs first.
Proposed fix
function redactPattern(value: string, pattern: RegExp): string {
return value.replace(pattern, (match) => {
- const separator = match.includes(':') ? ':' : match.includes('=') ? '=' : '';
- if (!separator) return REDACTED;
- const [prefix] = match.split(separator);
- return `${prefix}${separator}${REDACTED}`;
+ const indexes = [match.indexOf(':'), match.indexOf('=')]
+ .filter((index) => index >= 0);
+ if (indexes.length === 0) return REDACTED;
+ const separatorIndex = Math.min(...indexes);
+ return `${match.slice(0, separatorIndex + 1)}${REDACTED}`;
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function redactPattern(value: string, pattern: RegExp): string { | |
| return value.replace(pattern, (match) => { | |
| const separator = match.includes(':') ? ':' : match.includes('=') ? '=' : ''; | |
| if (!separator) return REDACTED; | |
| const [prefix] = match.split(separator); | |
| return `${prefix}${separator}${REDACTED}`; | |
| function redactPattern(value: string, pattern: RegExp): string { | |
| return value.replace(pattern, (match) => { | |
| const indexes = [match.indexOf(':'), match.indexOf('=')] | |
| .filter((index) => index >= 0); | |
| if (indexes.length === 0) return REDACTED; | |
| const separatorIndex = Math.min(...indexes); | |
| return `${match.slice(0, separatorIndex + 1)}${REDACTED}`; | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime/src/local-preview-redaction.ts` around lines 48 - 53, Update
redactPattern to identify the earliest occurring ':' or '=' in each match,
rather than prioritizing ':' whenever present. Use that first separator to split
the prefix and redact everything after it, preserving REDACTED for matches
without either separator.
| let stderr = ''; | ||
| let result: LocalPreviewWorkerResult | undefined; | ||
| let ready = false; | ||
| const pendingResponses = new Map<Promise<void>, AbortController>(); | ||
|
|
||
| child.stderr?.setEncoding('utf8'); | ||
| child.stderr?.on('data', (chunk) => { | ||
| stderr += chunk; | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound captured worker stderr.
Untrusted worker code can continuously write to stderr, causing the parent process to retain an unbounded string and potentially exhaust memory. Keep only a fixed-size tail.
Proposed fix
+const MAX_WORKER_STDERR_CHARS = 64 * 1024;
let stderr = '';
child.stderr?.on('data', (chunk) => {
- stderr += chunk;
+ stderr = `${stderr}${chunk}`.slice(-MAX_WORKER_STDERR_CHARS);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let stderr = ''; | |
| let result: LocalPreviewWorkerResult | undefined; | |
| let ready = false; | |
| const pendingResponses = new Map<Promise<void>, AbortController>(); | |
| child.stderr?.setEncoding('utf8'); | |
| child.stderr?.on('data', (chunk) => { | |
| stderr += chunk; | |
| }); | |
| const MAX_WORKER_STDERR_CHARS = 64 * 1024; | |
| let stderr = ''; | |
| let result: LocalPreviewWorkerResult | undefined; | |
| let ready = false; | |
| const pendingResponses = new Map<Promise<void>, AbortController>(); | |
| child.stderr?.setEncoding('utf8'); | |
| child.stderr?.on('data', (chunk) => { | |
| stderr = `${stderr}${chunk}`.slice(-MAX_WORKER_STDERR_CHARS); | |
| }); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime/src/local-preview.ts` around lines 318 - 326, Bound the
stderr accumulation in the local preview worker flow by retaining only a
fixed-size tail of the captured output. Update the `child.stderr` data handler
and `stderr` state so continuous worker writes cannot grow memory without limit,
while preserving the most recent stderr content for error reporting.
| if (message?.type === 'model') { | ||
| const work = respondToModel(child, message, args.onModelRequest) | ||
| .finally(() => pendingResponses.delete(work)); | ||
| pendingResponses.set(work, new AbortController()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make pending model work genuinely cancellable.
The model request is stored with an unused AbortController. After the worker closes, Promise.allSettled can therefore delay completion until the model timeout, while timeout shutdown leaves the underlying provider request running. Propagate an abort signal through onModelRequest and the adapter, or bound cleanup without awaiting non-cancellable work.
Also applies to: 370-371
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime/src/local-preview.ts` around lines 337 - 340, Update the
model-request flow around respondToModel and onModelRequest so pending work is
genuinely cancellable: create and retain an AbortController per request,
propagate its signal through the callback and provider adapter, and abort it
when the worker closes or cleanup begins. Ensure pending-response cleanup does
not wait for an uncancellable provider timeout.
| const timeout = setTimeout(() => controller.abort(), localPreviewFetchTimeoutMs()); | ||
| let response: Response; | ||
| const { headers, strippedHeaders } = sanitizeLiveRequestHeaders(request.headers); | ||
| try { | ||
| response = await fetch(new Request(url, { | ||
| method, | ||
| headers, | ||
| ...(request.bodyBase64 ? { body: Buffer.from(request.bodyBase64, 'base64') } : {}), | ||
| redirect: 'manual', | ||
| signal: controller.signal | ||
| })); | ||
| } finally { | ||
| clearTimeout(timeout); | ||
| } | ||
|
|
||
| if (!isRedirect(response.status)) { | ||
| return { | ||
| type: 'fetch-response', | ||
| requestId: request.requestId, | ||
| ok: true, | ||
| action: { | ||
| kind: 'http.read', | ||
| status: 'previewed', | ||
| data: { | ||
| method, | ||
| url, | ||
| source: 'current', | ||
| ...(strippedHeaders.length > 0 ? { strippedHeaders } : {}) | ||
| }, | ||
| extensions: { | ||
| sourceFidelity: 'current' | ||
| } | ||
| }, | ||
| response: { | ||
| status: response.status, | ||
| headers: [...response.headers.entries()], | ||
| bodyBase64: Buffer.from(await response.arrayBuffer()).toString('base64') | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Apply the fetch timeout and size limit to the response body.
fetch() resolves after headers, and the timer is cleared before arrayBuffer() consumes the body. A slow or unbounded allowed response can therefore exceed the fetch timeout or exhaust parent memory. Keep the timeout active through body consumption and stream into a bounded buffer.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime/src/local-preview.ts` around lines 487 - 524, Update the
local preview fetch flow around the fetch call and response.arrayBuffer() to
keep the timeout active until the response body is fully consumed. Replace
unbounded arrayBuffer buffering with streaming into a bounded buffer that
enforces the configured response size limit, aborting or rejecting when the
limit is exceeded, while preserving the existing response encoding and timeout
cleanup behavior.
| if ( | ||
| STRIP_LIVE_REQUEST_HEADERS.has(normalized) || | ||
| (normalized.startsWith('x-') && (isCredentialLikeHeaderName(normalized) || isCredentialLikeValue(value))) | ||
| ) { | ||
| stripped.add(normalized); | ||
| continue; | ||
| } | ||
| headers.append(name, value); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Strip credential-like headers regardless of the x- prefix.
Headers such as Api-Key, Private-Token, or Client-Secret currently pass through because credential-name detection only runs for x-* names.
Proposed fix
if (
STRIP_LIVE_REQUEST_HEADERS.has(normalized) ||
- (normalized.startsWith('x-') && (isCredentialLikeHeaderName(normalized) || isCredentialLikeValue(value)))
+ isCredentialLikeHeaderName(normalized) ||
+ isCredentialLikeValue(value)
) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| STRIP_LIVE_REQUEST_HEADERS.has(normalized) || | |
| (normalized.startsWith('x-') && (isCredentialLikeHeaderName(normalized) || isCredentialLikeValue(value))) | |
| ) { | |
| stripped.add(normalized); | |
| continue; | |
| } | |
| headers.append(name, value); | |
| if ( | |
| STRIP_LIVE_REQUEST_HEADERS.has(normalized) || | |
| isCredentialLikeHeaderName(normalized) || | |
| isCredentialLikeValue(value) | |
| ) { | |
| stripped.add(normalized); | |
| continue; | |
| } | |
| headers.append(name, value); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime/src/local-preview.ts` around lines 780 - 787, Update the
header filtering condition in the local preview request-header loop so
isCredentialLikeHeaderName(normalized) is evaluated for every header name, not
only names beginning with x-. Continue stripping matching credential-like names
or values, while preserving STRIP_LIVE_REQUEST_HEADERS handling and appending
only non-sensitive headers.
1 participant
Summary
Closes the Workforce side of AgentWorkforce/cloud#2619's composable runtime loop:
agentworkforce invokesurface for schedule/case/read/model/watch preview flowsVerification
Publication of 4.1.23 and the downstream Cloud/Agents cutovers follow after merge.