Skip to content

expose ctx.version to workers with the enable_version_api compat flag#6283

Merged
sohpeach merged 2 commits intomainfrom
swallace/ctx_version
Mar 10, 2026
Merged

expose ctx.version to workers with the enable_version_api compat flag#6283
sohpeach merged 2 commits intomainfrom
swallace/ctx_version

Conversation

@sohpeach
Copy link
Member

@sohpeach sohpeach commented Mar 9, 2026

When a worker with the experimental enable_version_api compat flag has a versionInfo passed while its entrypoint is being fetched, ctx.version will be populated.

ctx.version is currently optional even when the enable_version_api compat flag is enabled because we have some cases where it might not be populated (dynamic workers, the ctx object created for the nonclass_entrypoint_reuses_ctx_across_invocations compat flag, and the single enable_version_api test in server-test.c++).

Similar to #6111, there's not much testing here. We haven't exposed a way to hit all these codepaths directly from workerd so we're relying on tests from the internal codebase right now. This can change once we expose VersionInfo to WorkerdDebugPort.

@sohpeach sohpeach requested review from a team as code owners March 9, 2026 19:50
Copy link
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR threads Worker::VersionInfo through the entrypoint/handler/ExecutionContext pipeline and exposes it as ctx.version behind the experimental enable_version_api compat flag.

Findings (highest severity first):

  1. Missing test coverage: No test exercises ctx.version being populated with real data. The existing server-test (loopback binding calls accept version property) passes kj::none for versionInfo at the server level and tests the loopback binding, not ctx.version content. Consider adding a test that provides a VersionInfo with real id/cohort/key values and asserts they appear correctly on ctx.version.
  2. Mutable version object: toJs() returns a plain unfrozen JS object — user code can mutate ctx.version.id etc. Consider freezing it.
  3. JSG_TS_OVERRIDE combinatorial explosion: 4 branches with duplicated version type. The TODO acknowledges this but duplicating the 4-field version type is fragile.
  4. Pre-existing: exports not traced in visitForGc: Not introduced by this PR but you're modifying this class — exports is a JsRef that should be visited.

This review was generated by an AI assistant and may contain inaccuracies.

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 9, 2026

Review posted successfully on PR #6283. Here's a summary of the findings:

  1. Missing test coverage — No test exercises ctx.version being populated with real data. The existing server-test tests the loopback binding, not the actual ctx.version content.
  2. Mutable version objecttoJs() returns an unfrozen plain JS object, so user code can mutate version metadata.
  3. JSG_TS_OVERRIDE combinatorial explosion — 4 branches with duplicated version type definition, acknowledged by the TODO.
  4. Pre-existing exports not traced in visitForGc — Suggested a one-line fix since the author is already modifying the method.

github run

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@sohpeach sohpeach marked this pull request as draft March 10, 2026 11:59
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 10, 2026

Merging this PR will not alter performance

✅ 70 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing swallace/ctx_version (6cf8ef5) with main (a0cfac4)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sohpeach sohpeach force-pushed the swallace/ctx_version branch from 86fe0f7 to 8d63505 Compare March 10, 2026 19:02
@sohpeach sohpeach marked this pull request as ready for review March 10, 2026 19:27
@sohpeach sohpeach force-pushed the swallace/ctx_version branch from 8d63505 to fe04923 Compare March 10, 2026 19:51
@sohpeach sohpeach force-pushed the swallace/ctx_version branch from fe04923 to 6cf8ef5 Compare March 10, 2026 20:50
@sohpeach sohpeach merged commit 60b9cd6 into main Mar 10, 2026
33 of 34 checks passed
@sohpeach sohpeach deleted the swallace/ctx_version branch March 10, 2026 21:30
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.

2 participants