Skip to content

Fix executeCode return values - #210

Open
fancydirty wants to merge 2 commits into
cloudflare:mainfrom
fancydirty:codex/preserve-executecode-output
Open

Fix executeCode return values#210
fancydirty wants to merge 2 commits into
cloudflare:mainfrom
fancydirty:codex/preserve-executecode-output

Conversation

@fancydirty

@fancydirty fancydirty commented Aug 14, 2026

Copy link
Copy Markdown

What does this change?

Part of #209.

During a live Cloudflare OS session, several executeCode steps completed with no visible output.
That symptom can have more than one cause. This PR addresses one independently reproducible case:
a code-mode module returns a value without calling console.log(), but the dynamic Worker harness
awaits and discards that value, so the completed invocation renders blank output.

The patch returns the module value from the harness, carries it through executeCodeMode(), and
appends it to existing console output. Plain objects and arrays use JSON; other RPC-supported values
use their string representation, with an explicit fallback if rendering throws. undefined keeps
the existing console-only behavior.

This PR does not claim to explain every blank or timed-out code-mode step.

Why is this obviously correct and trivially verifiable?

The patch follows the existing call path directly: the harness returns agent(...), the RPC type
accepts that value, executeCodeMode() captures it, and one small formatter appends it. The test
checks structured and non-JSON return rendering, plus a circular plain object that exercises the
explicit serialization fallback. No bindings, outbound access, authorization, or execution policy
changes.

Verification: Workshop unit tests (285/285), integration tests (2 passed, 4 existing skips),
Workshop build/typecheck, full repository tests and lint, and git diff --check all pass on Node 24.

Checklist

  • This is a small, concrete change; it is not a feature, refactor, or low-value cleanup.
  • I understand that maintainers decide whether the change is obviously correct and trivially verifiable.
  • I have read and followed the contribution guidelines.

Copilot AI lite review requested due to automatic review settings August 14, 2026 16:58
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions github-actions Bot added the kernel Changes to the Workshop kernel label Aug 14, 2026
@fancydirty

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Aug 14, 2026

Copilot AI 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.

Pull request overview

This PR fixes a code-mode execution gap in the Workshop backend by preserving a code-mode module’s return value from the dynamic Worker harness, propagating it through executeCodeMode(), and appending it to the existing console-output-based result.

Changes:

  • Update the code-mode harness to return the user module’s agent(...) result.
  • Thread the harness return value through the CodeModeEntrypoint.run() RPC typing and executeCodeMode() output formatting via a new appendCodeModeReturnValue() helper.
  • Add a unit test covering harness text and basic return-value formatting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/workshop-backend/src/overseer.ts Return the code-mode module value from the harness and append it to formatted execution logs.
packages/workshop-backend/tests/code-mode-return.test.ts Adds a unit test for the harness return and return-value rendering helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +3 to +7
it("preserves code mode return values", () => {
expect(CODE_MODE_HARNESS).toContain("return await agent");
expect(appendCodeModeReturnValue("", 1n)).toBe("1");
expect(appendCodeModeReturnValue("started", { ok: true })).toBe('started\n{"ok":true}');
});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added a circular plain-object case that makes JSON.stringify() throw and asserts the explicit fallback. I also removed the source-text assertion so the test exercises return rendering behavior directly. The mutation check fails with "[object Object]" when the fallback is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Changes to the Workshop kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants