fix(browser): serialize undefined run results as null

[rishabhraj36]

Description

Related issue:

Type of Change

• 🐛 Bug fix
• ✨ New feature
• 🌐 New site adapter
• 📝 Documentation
• ♻️ Refactor
• 🔧 CI / build / tooling

Checklist

• I ran the checks relevant to this PR
• I updated tests or docs if needed
• I included output or screenshots when useful

Adapter Notes

• Updated generated or lean docs when command discoverability changed
• Used positional args for the command's primary subject unless a named flag is clearly better
• Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

[github-actions]

🟢 No documentation gap found — medium confidence

The automated review found no documentation gap in the supplied changes.

This review is advisory and does not block merging.

[beubax]

Maintainer review: changes requested

The one-line behavior change is sensible, but this PR needs a regression test and a minimal problem statement before it is safe to merge.

Why the behavior is valid

new AsyncFunction(source)() returns JavaScript undefined when a browser program performs side effects but has no explicit return. JSON.stringify(undefined) itself returns undefined, so the current runner reports BROWSER_RUN_SERIALIZATION_ERROR even though the program executed successfully.

Representing that top-level “no value” as JSON null is a reasonable stable-envelope convention. The proposed expression preserves explicit null and leaves genuinely unserializable values—such as BigInt—on the existing error path.

Why a test is required

This branch changes the public result contract for both local and hosted browser runs:

• local Cloak calls runBrowserProgram directly;
• Webcmd Cloud imports runBrowserProgram from the pinned @agentrhq/webcmd/browser/run export.

A later cleanup could easily revert undefined to the existing serialization error while all current tests remain green, because no test invokes a program without a return value. The existing serialization test covers BigInt, not the normal no-return case.

Please add a focused test in src/browser/run/runner.test.ts equivalent to:

it('serializes a program with no explicit return as null', async () => {
  const output = await run(`
    await page.locator('button').click();
  `);

  expect(output.result).toBeNull();
});

Keep or explicitly assert the complementary invariant:

await expect(run('return 1n;')).rejects.toMatchObject({
  code: 'BROWSER_RUN_SERIALIZATION_ERROR',
});

A CLI/envelope assertion would also be useful if there is already a cheap browser-run fixture, but one runner-level regression test is the minimum needed.

PR context

The description currently contains only the repository template, has no reproduction/output, and links no issue. Please document:

• the failing no-return source;
• the current BROWSER_RUN_SERIALIZATION_ERROR;
• the intended result: null output;
• whether this was observed locally, through Cloud, or both.

Deployment implication

Once merged and released, local users receive the behavior in the new CLI package. Cloud receives it only after its exact @agentrhq/webcmd pin is bumped and deployed. The test matters for maintaining parity across those two consumers.

With that small test and description update, I would consider the implementation appropriate and minimal.

[rishabhraj36]

Maintainer review: changes requested

The one-line behavior change is sensible, but this PR needs a regression test and a minimal problem statement before it is safe to merge.

Why the behavior is valid

new AsyncFunction(source)() returns JavaScript undefined when a browser program performs side effects but has no explicit return. JSON.stringify(undefined) itself returns undefined, so the current runner reports BROWSER_RUN_SERIALIZATION_ERROR even though the program executed successfully.

Representing that top-level “no value” as JSON null is a reasonable stable-envelope convention. The proposed expression preserves explicit null and leaves genuinely unserializable values—such as BigInt—on the existing error path.

Why a test is required

This branch changes the public result contract for both local and hosted browser runs:

• local Cloak calls runBrowserProgram directly;
• Webcmd Cloud imports runBrowserProgram from the pinned @agentrhq/webcmd/browser/run export.

A later cleanup could easily revert undefined to the existing serialization error while all current tests remain green, because no test invokes a program without a return value. The existing serialization test covers BigInt, not the normal no-return case.

Please add a focused test in src/browser/run/runner.test.ts equivalent to:

it('serializes a program with no explicit return as null', async () => {
  const output = await run(`
    await page.locator('button').click();
  `);

  expect(output.result).toBeNull();
});

Keep or explicitly assert the complementary invariant:

await expect(run('return 1n;')).rejects.toMatchObject({
  code: 'BROWSER_RUN_SERIALIZATION_ERROR',
});

A CLI/envelope assertion would also be useful if there is already a cheap browser-run fixture, but one runner-level regression test is the minimum needed.

PR context

The description currently contains only the repository template, has no reproduction/output, and links no issue. Please document:

• the failing no-return source;
• the current BROWSER_RUN_SERIALIZATION_ERROR;
• the intended result: null output;
• whether this was observed locally, through Cloud, or both.

Deployment implication

Once merged and released, local users receive the behavior in the new CLI package. Cloud receives it only after its exact @agentrhq/webcmd pin is bumped and deployed. The test matters for maintaining parity across those two consumers.

With that small test and description update, I would consider the implementation appropriate and minimal.

Added the runner-level regression test for programs without an explicit return. It asserts result: null, while the existing BigInt assertion continues to cover BROWSER_RUN_SERIALIZATION_ERROR.