Skip to content

fix(execution): honor inline JavaScript module format - #1949

Merged
NathanFlurry merged 1 commit into
mainfrom
stack/fix-execution-honor-inline-javascript-module-format-ukvlwnnu
Sep 17, 2026
Merged

NathanFlurry merged 1 commit into
mainfrom
stack/fix-execution-honor-inline-javascript-module-format-ukvlwnnu

Conversation

@NathanFlurry

Copy link
Copy Markdown
Member

No description provided.

@railway-app

railway-app Bot commented Sep 2, 2026

Copy link
Copy Markdown

🚅 Environment agentos-pr-1949 in rivet-frontend has no services deployed.

Module-format inline JavaScript execute/evaluate was rewritten to CommonJS
and run as a script, so inline ESM such as `export const y = 1;` failed.
Run it as a real module via AGENTOS_GUEST_ENTRYPOINT_MODULE_MODE instead.

Module cells no longer share top-level lexical bindings across calls in a
context, so the context docs, example, and client format doc comments now
describe sharing state through globalThis (or format: "commonjs").

Refs #1949, #1953.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@NathanFlurry
NathanFlurry force-pushed the stack/fix-execution-honor-inline-javascript-module-format-ukvlwnnu branch from 7558810 to dd19e27 Compare September 17, 2026 02:16
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
Document that module-format inline JavaScript evaluates each call as an
independent root ES module, so retained contexts share globalThis rather
than top-level lexical bindings. Update the contexts example to use
globalThis and add matching Rust/TS doc comments. Companion to #1949.

Refs #1953

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 1 high · 🟠 1 medium

Reviewed commit dd19e27.

Comment on lines 526 to 529
lowered_process(payload.process, "node", vec![String::from("-e"), source]);
operation.retained_language = Some(RetainedExecutionLanguage::JavaScript);
operation.retained_source = Some(retained_source);
operation.retained_file_path = Some(file_path.clone());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 High · Module execution fails after exporting functions or classes

This switches inline execute() calls onto V8's module path, which always passes the module namespace through serialize_module_exports. That serializer turns every export into a plain object and structured-clones it; function and class values are not cloneable, so ordinary source such as export function run() {} executes and then finishes with failed to serialize exports/exit code 1. Because the public execute() API does not return module exports, this can also report failure after user side effects already ran. The module path needs to skip export serialization for these semantic executions (or otherwise tolerate uncloneable exports) before making module mode the default.

@@ -528,16 +521,19 @@ fn lower_operation(payload: RequestPayload) -> Result<LoweredOperation, SidecarE
let module = payload.format == Some(JavaScriptModuleFormat::Module);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · CommonJS format is ignored for the first context call

CommonJs is represented only as the absence of the module-mode env var. During the initial launch, JavascriptExecution::prepare still ORs inline_code_uses_module_mode(source) into the mode, so a request explicitly sent with format: CommonJs is promoted to ESM whenever the source contains export, a static import, or import.meta. Once the context is resident, the same format is passed as module = false directly and the same source is parsed as a script, making behavior depend on whether this is the first call. Carry an explicit format through preparation (including an explicit false/override) and only use source inference when no format was supplied.

@NathanFlurry
NathanFlurry merged commit 8463e6a into main Sep 17, 2026
9 of 10 checks passed
@NathanFlurry
NathanFlurry deleted the stack/fix-execution-honor-inline-javascript-module-format-ukvlwnnu branch September 17, 2026 02:43
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
Document that module-format inline JavaScript evaluates each call as an
independent root ES module, so retained contexts share globalThis rather
than top-level lexical bindings. Update the contexts example to use
globalThis and add matching Rust/TS doc comments. Companion to #1949.

Refs #1953
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
Re-run the same inline ES module twice in one retained context with a fixed
file_path and assert globalThis.moduleRuns is 2, so repeated module calls are
evaluated independently instead of served from a module cache (#1955).

The base test javascript_module_execution_accepts_inline_exports_in_a_context
comes from #1949; this commit adds it in its extended form so it applies to
main. When landing after #1949, keep this version of the function.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
)

Re-run the same inline ES module twice in one retained context with a fixed
file_path and assert globalThis.moduleRuns is 2, so repeated module calls are
evaluated independently instead of served from a module cache (#1955).

The base test javascript_module_execution_accepts_inline_exports_in_a_context
comes from #1949; this commit adds it in its extended form so it applies to
main. When landing after #1949, keep this version of the function.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
Add a wire-level native-sidecar test that an inline format=Module
JavaScript source using top-level await in a named context settles and
exits Succeeded/0. Ports #1956 onto main; depends on the inline module
format fix from #1949 and fails without it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NathanFlurry added a commit that referenced this pull request Sep 17, 2026
Add a wire-level native-sidecar test that an inline format=Module
JavaScript source using top-level await in a named context settles and
exits Succeeded/0. Ports #1956 onto main; depends on the inline module
format fix from #1949 and fails without it.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant