Skip to content

Handle batched code mode tail events - #211

Closed
fancydirty wants to merge 2 commits into
cloudflare:mainfrom
fancydirty:codex/handle-codemode-tail-batches
Closed

Handle batched code mode tail events#211
fancydirty wants to merge 2 commits into
cloudflare:mainfrom
fancydirty:codex/handle-codemode-tail-batches

Conversation

@fancydirty

Copy link
Copy Markdown

What does this change?

Part of #209.

When a code-mode Tail batch consists of exactly one verify() and one run(), keeps the run trace
before applying the existing exactly-one-event validation. This avoids discarding the valid run
output while all other unexpected batches still fail closed.

Why is this obviously correct and trivially verifiable?

The special case requires exactly two events and both expected RPC method names. The existing
singleton validation and single-verify early return remain unchanged for every other case. The
regression test asserts one delivery for [verify, run] and no delivery for malformed batches.
No bindings, authorization, or execution policy changes.

Verification: focused RED/GREEN reproduction, 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

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 updates the Workshop backend’s code-mode Tail loopback handling to tolerate a specific, known-valid batched trace delivery pattern (verify() and run() arriving together), ensuring the run() trace is preserved while malformed batches still fail closed.

Changes:

  • Special-case [verify, run] 2-event trace batches by keeping only the run trace before applying the existing singleton validation.
  • Add a regression test covering the valid [verify, run] batch and several malformed batch shapes.

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 Adds a targeted special-case to preserve run() traces when batched with verify() while retaining strict validation for other batches.
packages/workshop-backend/tests/code-mode-tail.test.ts Adds a regression test ensuring [verify, run] delivers exactly one run trace and malformed batches deliver none.

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

Comment on lines +7238 to +7242
const methods = events.map(event => event.event && "rpcMethod" in event.event
? event.event.rpcMethod : undefined);
if (events.length === 2 && methods.includes("verify") && methods.includes("run")) {
events = [events[methods.indexOf("run")]];
}

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.

Replaced map/includes/indexOf with an explicit two-element check. It derives the two rpcMethod values directly and selects the run item for the verify/run pair while leaving every other batch on the existing fail-closed path.

@kentonv

kentonv commented Aug 14, 2026

Copy link
Copy Markdown
Member

Have you actually observed a problem here in practice?

As far as I know, the "verify" and "run" events will never be batched in the way that this PR seems to be trying to handle, and I've never observed this happening myself nor can I find any evidence in our logs.

@fancydirty

Copy link
Copy Markdown
Author

No — I did not capture a production [verify(), run()] batch. I inferred the possibility from the array-shaped Tail API and reproduced it only by injecting a synthetic batch in a unit test. That proves how the code would behave under that input, not that the runtime emits it. Since there is no supporting runtime observation in my evidence or in your logs, the premise of this PR is not established. I have corrected #209 to remove the batching claim and am closing this PR. The independently reproducible return-value behavior remains scoped separately in #210.

@fancydirty fancydirty closed this Aug 15, 2026
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.

3 participants