From 370fda14ff0332f9465ca881cc5db52b7edd33f5 Mon Sep 17 00:00:00 2001 From: jiwangyihao Date: Tue, 12 May 2026 21:08:44 +0800 Subject: [PATCH] =?UTF-8?q?fix(js-eval):=20=E4=BD=BF=E7=94=A8=E7=A7=81?= =?UTF-8?q?=E6=9C=89=E6=9C=80=E7=BB=88=E8=A1=A8=E8=BE=BE=E5=BC=8F=E6=A7=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/eval/js/shared/rewrite-imports.ts | 2 +- .../src/eval/js/shared/runtime.ts | 25 +++++--- .../test/core/js-executor.test.ts | 58 +++++++++++++++---- 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/src/eval/js/shared/rewrite-imports.ts b/packages/coding-agent/src/eval/js/shared/rewrite-imports.ts index b6bc00607..6cab0397b 100644 --- a/packages/coding-agent/src/eval/js/shared/rewrite-imports.ts +++ b/packages/coding-agent/src/eval/js/shared/rewrite-imports.ts @@ -187,7 +187,7 @@ function returnFinalExpression(code: string): { source: string; returned: boolea const suffix = code.slice(expression.end); const semicolonMatch = statement.match(/;\s*$/); const trimmedStatement = semicolonMatch ? statement.slice(0, semicolonMatch.index) : statement; - return { source: `${prefix}globalThis.__omp_final_expr__ = (${trimmedStatement});${suffix}`, returned: true }; + return { source: `${prefix}__omp_set_final_expr__((${trimmedStatement}));${suffix}`, returned: true }; } /** diff --git a/packages/coding-agent/src/eval/js/shared/runtime.ts b/packages/coding-agent/src/eval/js/shared/runtime.ts index 250073724..e3e49cf5a 100644 --- a/packages/coding-agent/src/eval/js/shared/runtime.ts +++ b/packages/coding-agent/src/eval/js/shared/runtime.ts @@ -48,6 +48,8 @@ export class JsRuntime { readonly sessionId: string; #env: Map; #getHooks: () => RuntimeHooks | null; + #finalExpressionSet = false; + #finalExpressionValue: unknown; constructor(opts: RuntimeOptions) { this.#cwd = opts.initialCwd; @@ -82,16 +84,21 @@ export class JsRuntime { } async run(code: string, filename?: string): Promise { - Reflect.deleteProperty(globalThis, "__omp_final_expr__"); + this.#finalExpressionSet = false; + this.#finalExpressionValue = undefined; const wrapped = wrapCode(code); const value = indirectEval(wrapped.source, filename); - const awaited = await awaitMaybePromise(value); - if (wrapped.finalExpressionReturned && Object.hasOwn(globalThis, "__omp_final_expr__")) { - const finalValue = (globalThis as { __omp_final_expr__?: unknown }).__omp_final_expr__; - Reflect.deleteProperty(globalThis, "__omp_final_expr__"); - return await awaitMaybePromise(finalValue); + if (wrapped.finalExpressionReturned) { + const awaited = await awaitMaybePromise(value); + if (this.#finalExpressionSet) { + const finalValue = this.#finalExpressionValue; + this.#finalExpressionSet = false; + this.#finalExpressionValue = undefined; + return await awaitMaybePromise(finalValue); + } + return awaited; } - return awaited; + return await awaitMaybePromise(value); } displayValue(value: unknown): void { @@ -133,6 +140,10 @@ export class JsRuntime { this.#getHooks()?.onText(text.endsWith("\n") ? text : `${text}\n`); }, __omp_display__: (value: unknown) => this.displayValue(value), + __omp_set_final_expr__: (value: unknown) => { + this.#finalExpressionSet = true; + this.#finalExpressionValue = value; + }, webcrypto: crypto, // `process` is intentionally not overridden — user code gets the host worker's real // `process` object. Subsetting it caused segfaults in workers that share state with diff --git a/packages/coding-agent/test/core/js-executor.test.ts b/packages/coding-agent/test/core/js-executor.test.ts index 7e680a4de..9bff02368 100644 --- a/packages/coding-agent/test/core/js-executor.test.ts +++ b/packages/coding-agent/test/core/js-executor.test.ts @@ -94,20 +94,22 @@ describe("executeJs", () => { expect(persisted.output.trim()).toBe("41"); }); - it("ignores inherited final expression markers", async () => { - try { - await executeJs("Object.prototype.__omp_final_expr__ = 'poisoned';", { sessionId, session, sessionFile }); + it("does not expose the final expression marker as a global property", async () => { + const result = await executeJs("const localOnly = 7; localOnly;", { sessionId, session, sessionFile }); + expect(result.exitCode).toBe(0); + expect(result.output.trim()).toBe("7"); - const result = await executeJs("const localOnly = 7;", { sessionId, session, sessionFile }); - expect(result.exitCode).toBe(0); - expect(result.output.trim()).toBe(""); + const marker = await executeJs("return Object.hasOwn(globalThis, '__omp_final_expr__');", { + sessionId, + session, + sessionFile, + }); + expect(marker.exitCode).toBe(0); + expect(marker.output.trim()).toBe("false"); - const persisted = await executeJs("return localOnly;", { sessionId, session, sessionFile }); - expect(persisted.exitCode).toBe(0); - expect(persisted.output.trim()).toBe("7"); - } finally { - await executeJs("delete Object.prototype.__omp_final_expr__;", { sessionId, session, sessionFile }); - } + const persisted = await executeJs("return localOnly;", { sessionId, session, sessionFile }); + expect(persisted.exitCode).toBe(0); + expect(persisted.output.trim()).toBe("7"); }); it("ignores user-assigned final expression markers without a rewritten final expression", async () => { @@ -121,6 +123,38 @@ describe("executeJs", () => { expect(result.output.trim()).toBe("actual"); }); + it("captures promise-valued final expression before promise callbacks can mutate the marker", async () => { + const result = await executeJs( + "const pending = Promise.resolve(1).then(value => { globalThis.__omp_final_expr__ = 999; return value; }); pending;", + { sessionId, session, sessionFile }, + ); + + expect(result.exitCode).toBe(0); + expect(result.output.trim()).toBe("1"); + }); + + it("awaits rewritten thenable final expressions once", async () => { + const result = await executeJs( + [ + "globalThis.thenCalls = 0;", + "const thenable = {", + " then(resolve) {", + " globalThis.thenCalls++;", + " resolve('done');", + " },", + "};", + "thenable;", + ].join("\n"), + { sessionId, session, sessionFile }, + ); + expect(result.exitCode).toBe(0); + expect(result.output.trim()).toBe("done"); + + const calls = await executeJs("return globalThis.thenCalls;", { sessionId, session, sessionFile }); + expect(calls.exitCode).toBe(0); + expect(calls.output.trim()).toBe("1"); + }); + it("exposes the worker's real process object", async () => { const result = await executeJs( [