Drive worker guards from route contracts#41
Conversation
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-41.isaac-a46.workers.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR adds packages/worker-runtime and migrates API, Content, and Upload workers to a contract-driven registrar pipeline. Contracts gain per-route rateLimit metadata and additional agentView routes. The runtime provides principal types, error helpers, rate-limit bindings, and createRegistrar (idempotency → replay → rate-limit → scope → handler). Workers now mount route contracts, export mountedRouteIds and nonContractRoutePaths, and convert handlers to principal/db/guard signatures. Tests were added/updated to assert route mounting, rate-limit declarations, and scope-based authorization; docs mark the cutover steps done. Sequence Diagram(s)sequenceDiagram
participant Client
participant Registrar
participant AuthResolver
participant RateLimit
participant Handler
participant DB
Client->>Registrar: HTTP Request + Contract
Registrar->>AuthResolver: Resolve Principal
AuthResolver-->>Registrar: Principal or Error
alt Auth Failure
Registrar-->>Client: 401 Error
else Auth Success
Registrar->>Registrar: Check Idempotency Key
alt Missing Idempotency Key (required)
Registrar-->>Client: 400 invalid_idempotency_key
else Idempotency Valid or Not Required
Registrar->>DB: Check Idempotency Replay
DB-->>Registrar: Replay Result or None
alt Replay Found
Registrar-->>Client: 200 Cached Result
else No Replay
Registrar->>RateLimit: Apply Rate Limit
alt Rate Limited
Registrar-->>Client: 429 rate_limited_*
else Not Limited
Registrar->>Registrar: Validate Scopes
alt Missing Required Scope
Registrar-->>Client: 403 forbidden
else Scopes Valid
Registrar->>Handler: Execute Handler(context, principal, db, guard)
Handler->>DB: Read/Write Operations
DB-->>Handler: Results
Handler-->>Registrar: Response
Registrar-->>Client: 200 OK
end
end
end
end
end
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: 2
🤖 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 `@packages/worker-runtime/src/errors.ts`:
- Around line 66-73: The error response builder returns JSON without cache
directives; add a "cache-control": "no-store" header to the headers object so
error envelopes (4xx/5xx) are never cached. Update the Response construction in
the error envelope function (where ERROR_STATUS and REQUEST_ID_HEADER are used)
to include "cache-control": "no-store" alongside the existing content-type,
defaultHeaders, and headers; mirror the behavior used by jsonResponse to ensure
intermediaries don't cache error responses.
In `@packages/worker-runtime/src/registrar.ts`:
- Around line 159-163: The idempotency-key check in registrar.ts currently
accepts whitespace-only values; update the validation to trim the header value
and reject if the trimmed string is empty. Specifically, after retrieving
idempotencyKey from context.req.raw.headers.get("idempotency-key"), compute a
trimmed value (e.g., const key = idempotencyKey?.trim()) and change the guard to
if (!key) return { ok: false, code: "invalid_idempotency_key" }; and return {
ok: true, state: { idempotencyKey: key } } so the stored key is normalized.
🪄 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: 645897f9-aa6f-48dc-b193-09c2359e4aa6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
apps/api/package.jsonapps/api/src/index.test.tsapps/api/src/index.tsapps/api/src/local-mvp.test.tsapps/content/package.jsonapps/content/src/index.test.tsapps/content/src/index.tsapps/upload/package.jsonapps/upload/src/error-envelope.test.tsapps/upload/src/index.test.tsapps/upload/src/index.tsdocs/ops/project-status.mddocs/ops/worker-runtime-todo.mdpackages/contracts/src/mvp-contracts.test.tspackages/contracts/src/routes.tspackages/worker-runtime/package.jsonpackages/worker-runtime/src/errors.tspackages/worker-runtime/src/index.tspackages/worker-runtime/src/principal.tspackages/worker-runtime/src/rate-limit.tspackages/worker-runtime/src/registrar.test.tspackages/worker-runtime/src/registrar.tspackages/worker-runtime/tsconfig.json
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-41.isaac-a46.workers.dev |
|
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. |
1 participant
Summary
Route contracts now drive worker request enforcement instead of only documenting it. This implements ADR 0072 by moving auth, scopes, rate-limit selection, idempotency header validation, and common error envelopes into a shared worker runtime registrar.
Changes
@agent-paste/worker-runtimewith contract-backed route mounting, principal types, centralized error status mapping, rate-limit handling, idempotency handling, and focused guard tests.routeContractswithrateLimit, WorkOS callback provisioning metadata, and the two authenticated Agent View routes.Risk: HIGH
Test plan
bun run ci:checkunder Node 24.15.0CodeRabbit notes: skipped the
hono^4.12.22bump because all workspace Hono packages currently use^4.12.21; skipped exposing Zod validation details because current worker error envelopes intentionally return genericinvalid_requestresponses.Summary by CodeRabbit
New Features
Improvements
Documentation