fix(api): expose worker deployment version#2212
Conversation
📝 WalkthroughWalkthroughThis PR adds Cloudflare Worker version metadata tracking by introducing a new ChangesWorker Version Metadata Headers
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
riderx
force-pushed
the
codex/expose-api-worker-version
branch
from
1e2fade to
05908c0
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/worker-source-header.unit.test.ts (1)
1-17: ⚡ Quick winUse concurrent test cases to match repo test policy.
Please switch these to
it.concurrent(...)(and prefervi.stubEnvfor env isolation) so this file complies with the parallel-test rule.♻️ Proposed refactor
-import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { createHono } from '../supabase/functions/_backend/utils/hono.ts' import { version } from '../supabase/functions/_backend/utils/version.ts' describe('worker source headers', () => { - const originalEnvName = process.env.ENV_NAME - - beforeEach(() => { - process.env.ENV_NAME = 'capgo_api-prod' - }) - afterEach(() => { - if (originalEnvName === undefined) - delete process.env.ENV_NAME - else - process.env.ENV_NAME = originalEnvName + vi.unstubAllEnvs() }) - it('exposes the Cloudflare Worker deployment version when metadata is bound', async () => { + it.concurrent('exposes the Cloudflare Worker deployment version when metadata is bound', async () => { + vi.stubEnv('ENV_NAME', 'capgo_api-prod') const app = createHono('api', version) app.get('/ok', c => c.json({ status: 'ok' })) @@ - it('keeps the existing source header when version metadata is not available', async () => { + it.concurrent('keeps the existing source header when version metadata is not available', async () => { + vi.stubEnv('ENV_NAME', 'capgo_api-prod') const app = createHono('api', version) app.get('/ok', c => c.json({ status: 'ok' }))As per coding guidelines,
tests/**/*.test.ts: “Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD”.Also applies to: 19-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/worker-source-header.unit.test.ts` around lines 1 - 17, Replace synchronous tests and manual env manipulation with concurrent tests and vitest env stubbing: change any it(...) calls inside the describe('worker source headers', ...) to it.concurrent(...), remove the manual beforeEach/afterEach that set process.env.ENV_NAME, and instead use vi.stubEnv({ ENV_NAME: 'capgo_api-prod' }) in a beforeEach (and let vi restore it automatically) so the test harness (functions like beforeEach, afterEach, and the ENV_NAME usage) conforms to parallel-test policy; apply the same it.concurrent and vi.stubEnv changes for the other test cases referenced in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/worker-source-header.unit.test.ts`:
- Around line 1-17: Replace synchronous tests and manual env manipulation with
concurrent tests and vitest env stubbing: change any it(...) calls inside the
describe('worker source headers', ...) to it.concurrent(...), remove the manual
beforeEach/afterEach that set process.env.ENV_NAME, and instead use vi.stubEnv({
ENV_NAME: 'capgo_api-prod' }) in a beforeEach (and let vi restore it
automatically) so the test harness (functions like beforeEach, afterEach, and
the ENV_NAME usage) conforms to parallel-test policy; apply the same
it.concurrent and vi.stubEnv changes for the other test cases referenced in this
file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83466d5b-c6e4-426c-b02a-5d68592e31be
📒 Files selected for processing (4)
cloudflare_workers/api/wrangler.jsoncsupabase/functions/_backend/utils/cloudflare.tssupabase/functions/_backend/utils/hono.tstests/worker-source-header.unit.test.ts
riderx
force-pushed
the
codex/expose-api-worker-version
branch
from
05908c0 to
b9c875b
Compare
|
|
P2: Expose the new worker-version response headers through CORS as well.
If the intent is for the dashboard/support tooling to verify stale Worker deployments from API responses, this needs to be added in |
KCDaemon
left a comment
KCDaemon
left a comment
There was a problem hiding this comment.
I rechecked the latest head (b9c875b) and the CORS exposure gap is still present. createHono() now emits X-Worker-Source, X-Worker-Version-Id, X-Worker-Version-Tag, and X-Worker-Version-Timestamp, but useCors still has no exposeHeaders, so browser clients on the dashboard/support side will not be able to read these response headers with fetch(...).headers.get(...) on cross-origin API responses.
This makes the new deployment-version metadata much less useful in the main client path: the headers are on the wire, but hidden from browser JavaScript unless Access-Control-Expose-Headers includes the worker headers. The current unit tests only make same-origin-style app.fetch() requests without an Origin header, so they do not catch this.
git diff --check origin/main...origin/pr-2212 passes locally and CI is green, but I would keep this blocked until useCors exposes the worker headers and a regression covers an Origin request asserting access-control-expose-headers contains them.
3 participants
Summary (AI generated)
X-Worker-Version-Id,X-Worker-Version-Tag, andX-Worker-Version-Timestampheaders while preservingX-Worker-Source.Motivation (AI generated)
X-Worker-Sourceonly showed the app release version, making the stale Cloudflare deployment hard to spot from responses.Business Impact (AI generated)
Test Plan (AI generated)
Summary by CodeRabbit
Release Notes
New Features
Tests