feat(api,upload,content): add X-Request-Id and request_id to envelope#10
Merged
Conversation
isuttell
added a commit
that referenced
this pull request
May 22, 2026
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
WalkthroughThis PR introduces centralized request-id handling for Hono services by creating a reusable module in Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/upload/src/index.ts (1)
141-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard URL-decoding failures in upload path extraction.
Line 144 can throw
URIErrorfor malformed percent-encoding, which currently bubbles into a 500. Please handle decode failure explicitly so bad client paths don’t become internal errors.Proposed fix
function uploadFilePath(context: AppContext): string { const pathname = new URL(context.req.raw.url).pathname; const markerIndex = pathname.indexOf(UPLOAD_FILE_PATH_MARKER); - return markerIndex === -1 ? "" : decodeURIComponent(pathname.slice(markerIndex + UPLOAD_FILE_PATH_MARKER.length)); + if (markerIndex === -1) { + return ""; + } + const encodedPath = pathname.slice(markerIndex + UPLOAD_FILE_PATH_MARKER.length); + try { + return decodeURIComponent(encodedPath); + } catch { + return ""; + } }🤖 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 `@apps/upload/src/index.ts` around lines 141 - 145, The uploadFilePath function can throw a URIError when decodeURIComponent is fed malformed percent-encoding; wrap the decode step in a try/catch inside uploadFilePath (after locating UPLOAD_FILE_PATH_MARKER and slicing the path) and on a decode failure return an empty string (or other safe sentinel) instead of letting the error bubble up; keep the existing marker search logic (pathname.indexOf(UPLOAD_FILE_PATH_MARKER)) and only attempt decoding when markerIndex !== -1, but catch URIError from decodeURIComponent(pathname.slice(...)) and return "" to avoid a 500.
🤖 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.
Inline comments:
In `@apps/api/src/error-envelope.test.ts`:
- Around line 46-174: Add a 200-path test that exercises handleRequest for a
successful GET /v1/whoami (using authStub() and workspaceDb() or a DB.getWhoami
that returns a user) and assert that response.status is 200 and X-Request-Id
behavior matches the invariant: when sending a valid x-request-id header the
response header echoes it, and when no/invalid x-request-id is provided the
response includes a freshly minted x-request-id (assert
response.headers.get("x-request-id") exists and differs from a malformed input).
Use the existing helpers (handleRequest, authStub, workspaceDb) to set up the
request and keep assertions minimal.
In `@apps/api/src/index.ts`:
- Around line 994-1007: The OpenAPI contract is out-of-date: the runtime
errorResponse (which builds the body via buildErrorBody and returns via
jsonResponse, includes requestId from getRequestId(context) and docs from
context.env.DOCS_BASE_URL, and emits X-Request-Id header) no longer matches the
ErrorEnvelope schema and headers. Update the OpenAPI ErrorEnvelope schema to
include request_id (and docs or docs_base_url if used) under the error object,
and add X-Request-Id as a response header on error responses; ensure field names
exactly match what buildErrorBody emits and that any generated SDKs will receive
the new fields.
In `@apps/content/src/error-envelope.test.ts`:
- Around line 36-118: Add a happy-path test that requests an existing object and
asserts a 200/HEAD response includes the request id: use signContentToken to
create a valid token, stub ARTIFACTS.get to return a Response with status 200
(or handle a HEAD path), call handleRequest with an inbound "x-request-id"
header, then assert response.status is 200 and that
response.headers.get("x-request-id") matches the inbound id and that
expectEnvelope(response, ...) (or parsing the body) shows error.request_id (or
equivalent envelope field) equals the same id; place this new it(...) alongside
the other tests referencing handleRequest, signContentToken and expectEnvelope.
In `@apps/content/src/index.ts`:
- Around line 419-425: The OpenAPI schema for error responses is out of sync
with actual responses produced by errorResponse: update the ErrorEnvelope
definition in openApiDocument to include the request_id field (string) and the
optional docs field (string or object as used by buildErrorBody), so generated
clients/docs reflect that responses include request_id and sometimes docs;
locate and modify the ErrorEnvelope schema in openApiDocument and any referenced
response components so they match the shape emitted by errorResponse and
buildErrorBody.
In `@apps/upload/src/error-envelope.test.ts`:
- Around line 39-219: Add a success-path test that exercises handleRequest for a
successful POST /v1/upload-sessions (use Env with AUTH via workspaceAuth() and
DB stubbed to return workspaceSession() from createUploadSession and
finalizeUploadSession) and assert that: response.status is 200 (or expected
success code), response.headers.get("x-request-id") is present and, when you
supply a valid "x-request-id" on the Request, it is echoed back; also call
expectEnvelope(response, "<appropriate_success_type>") or parse the body and
assert body.request_id matches the response header. Use the existing helpers
handleRequest, expectEnvelope, workspaceAuth, workspaceSession and
DB.createUploadSession/finalizeUploadSession to locate where to add the new
test.
In `@docs/ops/project-status.md`:
- Line 184: Update the stale numbered references in the document to match the
new ordered range 1–7: find and replace occurrences of the phrases "items 1-8"
and "item 8" (and any similar mentions) with the correct "items 1-7" or the
appropriate single-item reference (e.g., "item 7"), and verify any other
downstream references in the file that mention the original 1–8 range are
updated for consistency.
---
Outside diff comments:
In `@apps/upload/src/index.ts`:
- Around line 141-145: The uploadFilePath function can throw a URIError when
decodeURIComponent is fed malformed percent-encoding; wrap the decode step in a
try/catch inside uploadFilePath (after locating UPLOAD_FILE_PATH_MARKER and
slicing the path) and on a decode failure return an empty string (or other safe
sentinel) instead of letting the error bubble up; keep the existing marker
search logic (pathname.indexOf(UPLOAD_FILE_PATH_MARKER)) and only attempt
decoding when markerIndex !== -1, but catch URIError from
decodeURIComponent(pathname.slice(...)) and return "" to avoid a 500.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 910129fd-a230-41d8-a33f-4238502f7485
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.gitignoreapps/api/src/error-envelope.test.tsapps/api/src/index.tsapps/api/wrangler.jsoncapps/content/package.jsonapps/content/src/error-envelope.test.tsapps/content/src/index.tsapps/content/wrangler.jsoncapps/upload/package.jsonapps/upload/src/error-envelope.test.tsapps/upload/src/index.tsapps/upload/wrangler.jsoncdocs/ops/project-status.mdpackages/auth/package.jsonpackages/auth/src/index.tspackages/auth/src/request-id.ts
This was referenced May 22, 2026
isuttell
added a commit
that referenced
this pull request
May 22, 2026
PR #2 added `permissions: administration: write` to the cleanup workflow, but `administration` is not a valid GITHUB_TOKEN scope. GitHub rejected the workflow at evaluation time, silently dropping every `pull_request.closed` event for PRs #2--#9 and accumulating eight stale `preview/pr-N` Neon branches that tripped the 10-branch free-tier cap (blocks PR #10/#11/#12 deploys with HTTP 422). Drop the invalid permission and the `deleteAnEnvironment` step that required it; rename the file so GitHub registers a fresh workflow id instead of reusing the wedged record; validate the resolved PR number is a positive integer before deleting anything. Stale Neon branches still need a one-time operator purge -- agent is forbidden from calling `neondatabase/delete-branch-action` autonomously. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
isuttell
added a commit
that referenced
this pull request
May 22, 2026
Locks in the 'every response carries request-id' contract on happy paths, not just error envelopes. Addresses CodeRabbit nitpicks on PR #10.
isuttell
force-pushed
the
worktree-agent-a14b512aa4365cfc5
branch
from
3bad1b6 to
3afb7db
Compare
Contributor
Author
|
Re: OpenAPI ErrorEnvelope staleness on |
isuttell
added a commit
that referenced
this pull request
May 22, 2026
PR #2 added `permissions: administration: write` to the cleanup workflow, but `administration` is not a valid GITHUB_TOKEN scope. GitHub rejected the workflow at evaluation time, silently dropping every `pull_request.closed` event for PRs #2--#9 and accumulating eight stale `preview/pr-N` Neon branches that tripped the 10-branch free-tier cap (blocks PR #10/#11/#12 deploys with HTTP 422). Drop the invalid permission and the `deleteAnEnvironment` step that required it; rename the file so GitHub registers a fresh workflow id instead of reusing the wedged record; validate the resolved PR number is a positive integer before deleting anything. Stale Neon branches still need a one-time operator purge -- agent is forbidden from calling `neondatabase/delete-branch-action` autonomously. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Every Worker response now carries an X-Request-Id header and every error
envelope body includes a matching request_id. Inbound headers matching
the [A-Za-z0-9_-]{8,128} pattern are echoed; otherwise a fresh UUID is
minted. Codes with a documented remediation gain an optional docs URL
when DOCS_BASE_URL is set on the Worker.
Shared resolveRequestId, buildErrorBody, docsUrlFor, and
requestIdMiddleware live in packages/auth so all three Workers stay in
sync. errorResponse now takes the Hono context. Golden tests per Worker
cover 404/401/409/4xx/429/500 envelope shape, header echo, and docs URL
behavior.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Locks in the 'every response carries request-id' contract on happy paths, not just error envelopes. Addresses CodeRabbit nitpicks on PR #10.
isuttell
force-pushed
the
worktree-agent-a14b512aa4365cfc5
branch
from
3afb7db to
1807c4e
Compare
|
agent-paste PR preview resources were cleaned up. The pr-preview-${context.issue.number} environment is left in place; remove it from the GitHub UI if desired. |
This was referenced May 26, 2026
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
X-Request-Idheader; every error envelope body includes a matchingrequest_id. InboundX-Request-Idmatching^[A-Za-z0-9_-]{8,128}$is echoed; anything else is silently replaced withcrypto.randomUUID().rate_limited_actor,rate_limited_workspace,idempotency_in_flight,invalid_idempotency_key,usage_policy_exceeded,file_size_cap_exceeded,file_count_cap_exceeded,revision_size_cap_exceeded) get anerror.docsURL whenDOCS_BASE_URLis set on the Worker; otherwise the field is omitted.resolveRequestId,buildErrorBody,docsUrlFor, andrequestIdMiddlewarelive inpackages/auth/src/request-id.tsso all three Workers stay in sync.errorResponsenow takes the Hono context. Golden tests per Worker cover 404/401/409/4xx/429/500 envelope shape, header echo, anddocsbehavior.Test plan
pnpm verifyis green (turbo: 49 tasks, 38 tests across api/upload/content/auth/cli/api-client).request_idmatchesX-Request-Idand validatedocsURL is present whenDOCS_BASE_URLis set, absent otherwise.CodeRabbit notes
Five findings, three rejected:
.agent-out/so generated CR reports never get committed.expectEnvelopetest helper intopackages/auth. Each Worker owns its own helper (~30 lines, self-contained); cross-app test utilities are not warranted yet.DOCS_BASE_URLinwrangler.jsonc. Intentional per task brief; Isaac will set real values later.@cloudflare/vitest-pool-workers. Existing in-memoryhandleRequest(request, env)pattern is the repo standard; no need to change harness for this change.Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com
Summary by CodeRabbit
New Features
Improvements
Tests