HT-9: platform provider interfaces - #4
Conversation
Locks the charter §4 'Vercel-first, not Vercel-only' seams before any engine code can bypass them. Interfaces only — no adapters, no impls: - queue.ts: QueueProvider (at-least-once enqueue + dedupeKey), push- delivered QueueMessage, explicit ack/retry/deadLetter result union - scheduler.ts: recurring cron (declaration surface) + durable scheduled-actions (the scheduled_actions-table + cron-tick pattern) - blob.ts: BlobStore, signed-URL-only attachment access, per-tenant keys - inbound-email.ts: NormalizedInboundEmail (provider-agnostic) + InboundEmailProvider webhook verify/parse — no IMAP polling - index.ts: the single import surface for engine modules - src/providers/README.md, tsconfig.json (strict, src-scoped, noEmit) Proof-pass fixes: dropped an unused BlobStore import; widened verifySignature to Promise<boolean> since the first adapter (Gmail push) verifies an OIDC JWT that may fetch signing certs. Typechecks clean (tsc --noEmit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughIntroduces provider interfaces for queues, scheduling, blob storage, and inbound email, plus a barrel export, architectural documentation, and strict TypeScript compiler configuration. ChangesProvider Interface Seams
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 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 `@src/providers/blob.ts`:
- Around line 41-48: Update the BlobProvider getSignedUrl contract and every
implementing adapter to validate that expiresInSeconds is finite, strictly
positive, and no greater than a documented maximum TTL; reject invalid values
before provider-specific URL generation, and use the same bound consistently
across all adapters.
- Around line 29-33: Update the blob provider’s put method to keep contentLength
consistent with the materialized data: derive it from data.byteLength or
validate and reject any provided value that differs, ensuring stored metadata
matches the object bytes and NormalizedInboundAttachment.size.
In `@src/providers/queue.ts`:
- Around line 52-68: Update the QueueMessage.id documentation to define it as
the stable logical-message identifier reused across all redeliveries, rather
than unique to a delivery attempt. Clarify in QueueMessage that attempts
represents the delivery-attempt count and should be used for per-attempt
identity, preserving the existing deduplication contract.
In `@src/providers/scheduler.ts`:
- Around line 75-79: Update scheduleAction and its implementation to accept an
optional stable deduplication key or caller-supplied action ID, matching
EnqueueOptions.dedupeKey semantics. Enforce uniqueness during persistence and
return the existing scheduled action ID when the key or ID already exists,
rather than creating a duplicate, while preserving current behavior for calls
without a key.
- Around line 55-73: The scheduler contract documents durable, at-least-once
delivery without exposing the lifecycle needed to implement it. Update the
scheduler interface and its provider implementation around the scheduling method
to define atomic claiming/lease renewal, completion acknowledgement, and failure
rescheduling or retry operations for due actions; ensure claims prevent
concurrent cron ticks from dispatching the same action and preserve
at-least-once semantics. Alternatively, remove these guarantees from the
provider documentation and clearly assign the lifecycle and delivery guarantees
to the adapter-owned contract.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b986e5e-8730-4102-832d-48f80b3ae1f7
📒 Files selected for processing (7)
src/providers/README.mdsrc/providers/blob.tssrc/providers/inbound-email.tssrc/providers/index.tssrc/providers/queue.tssrc/providers/scheduler.tstsconfig.json
| put( | ||
| key: string, | ||
| data: Uint8Array, | ||
| opts: { contentType: string; contentLength?: number }, | ||
| ): Promise<void>; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep contentLength consistent with the bytes.
Because data is fully materialized, data.byteLength is authoritative. An independent optional length can make stored metadata disagree with the actual object and with NormalizedInboundAttachment.size; reject mismatches or derive the value internally.
🤖 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 `@src/providers/blob.ts` around lines 29 - 33, Update the blob provider’s put
method to keep contentLength consistent with the materialized data: derive it
from data.byteLength or validate and reject any provided value that differs,
ensuring stored metadata matches the object bytes and
NormalizedInboundAttachment.size.
| /** | ||
| * Mint a time-limited, signed URL for reading the object at `key`. The | ||
| * URL expires after `expiresInSeconds` and must not be usable | ||
| * afterward. This is the only way callers outside the engine (e.g. a | ||
| * browser rendering an attachment) ever read blob contents — attachments | ||
| * are served via signed URLs, never a public path. | ||
| */ | ||
| getSignedUrl(key: string, expiresInSeconds: number): Promise<string>; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bound and validate signed URL lifetimes.
expiresInSeconds: number permits NaN, negative values, zero, and unbounded values. Provider-specific handling could produce invalid or excessively long-lived URLs, contradicting the contract that blobs are never publicly readable. Require a finite positive TTL and enforce a documented maximum in every adapter.
🤖 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 `@src/providers/blob.ts` around lines 41 - 48, Update the BlobProvider
getSignedUrl contract and every implementing adapter to validate that
expiresInSeconds is finite, strictly positive, and no greater than a documented
maximum TTL; reject invalid values before provider-specific URL generation, and
use the same bound consistently across all adapters.
| export interface QueueMessage<T> { | ||
| /** Provider-assigned unique id for this delivery attempt's message. */ | ||
| id: string; | ||
|
|
||
| /** The topic/queue name this message was enqueued on. */ | ||
| topic: string; | ||
|
|
||
| /** The payload as originally enqueued. */ | ||
| payload: T; | ||
|
|
||
| /** | ||
| * How many times delivery of this message has been attempted, starting | ||
| * at `1` for the first delivery. Handlers can use this to implement | ||
| * their own retry-count-aware logic (e.g. escalate to dead-letter after | ||
| * N attempts) independent of what the platform's own retry policy does. | ||
| */ | ||
| attempts: number; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make QueueMessage.id stable across redeliveries.
The module contract requires deduplication using the same QueueMessage.id, but this field is described as unique for “this delivery attempt.” If an adapter generates a new ID per retry, handlers cannot reliably prevent duplicate side effects. Define id as the stable logical-message ID reused on redelivery; use attempts for per-attempt identity.
🤖 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 `@src/providers/queue.ts` around lines 52 - 68, Update the QueueMessage.id
documentation to define it as the stable logical-message identifier reused
across all redeliveries, rather than unique to a delivery attempt. Clarify in
QueueMessage that attempts represents the delivery-attempt count and should be
used for per-attempt identity, preserving the existing deduplication contract.
| /** | ||
| * Schedule `action` to run at or after `runAt`, carrying `payload`. | ||
| * Returns the id of the scheduled action so it can later be cancelled. | ||
| * | ||
| * This is the durable-delayed-action pattern the charter names | ||
| * alongside cron (a `scheduled_actions` table, polled by a cron tick the | ||
| * engine owns): the action must survive redeploys, cold starts, and | ||
| * process restarts, because there is no in-memory timer or long-lived | ||
| * process holding it. Delivery is via poll, not push: a recurring cron | ||
| * tick (registered separately, via `registerCron` or equivalent | ||
| * platform config) queries for actions due at-or-before "now" and | ||
| * dispatches each to its `handlerRef`. `runAt` is therefore a lower | ||
| * bound on execution time, not a precise deadline — actual delivery | ||
| * latency is bounded by the polling cron's own interval. | ||
| * | ||
| * Like `QueueProvider`, this is an at-least-once contract: a poll tick | ||
| * that dispatches an action but crashes/times out before marking it | ||
| * delivered may cause the next tick to dispatch it again. Handlers MUST | ||
| * be idempotent for the same action id. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define the durable-action claim and completion lifecycle.
The documentation promises poll-based, at-least-once delivery backed by scheduled_actions, but the interface exposes no operation to atomically claim/lease due actions, acknowledge completion, or reschedule failures. Without that lifecycle, overlapping cron ticks can dispatch the same row concurrently, or adapter code must bypass this provider seam. Add explicit claim/lease and completion/retry operations, or move these delivery guarantees into a clearly adapter-owned contract.
🤖 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 `@src/providers/scheduler.ts` around lines 55 - 73, The scheduler contract
documents durable, at-least-once delivery without exposing the lifecycle needed
to implement it. Update the scheduler interface and its provider implementation
around the scheduling method to define atomic claiming/lease renewal, completion
acknowledgement, and failure rescheduling or retry operations for due actions;
ensure claims prevent concurrent cron ticks from dispatching the same action and
preserve at-least-once semantics. Alternatively, remove these guarantees from
the provider documentation and clearly assign the lifecycle and delivery
guarantees to the adapter-owned contract.
| scheduleAction<T>( | ||
| runAt: Date, | ||
| action: HandlerRef, | ||
| payload: T, | ||
| ): Promise<string>; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add idempotency to scheduleAction.
If persistence succeeds but the caller times out before receiving the returned ID, retrying scheduleAction can create a second action. Idempotency by action ID does not help because the duplicate receives a different ID. Accept a stable deduplication key or caller-supplied action ID with uniqueness and return-existing semantics, mirroring EnqueueOptions.dedupeKey.
🤖 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 `@src/providers/scheduler.ts` around lines 75 - 79, Update scheduleAction and
its implementation to accept an optional stable deduplication key or
caller-supplied action ID, matching EnqueueOptions.dedupeKey semantics. Enforce
uniqueness during persistence and return the existing scheduled action ID when
the key or ID already exists, rather than creating a duplicate, while preserving
current behavior for calls without a key.
…101) All four counsel-gate documents were drafted 2026-07-19 (Codex-reviewed, fix-round applied): module commercial license (legal/, PR #100), terms of sale / privacy policy / managed-hosting terms (marketplace repo legal/, PR #4 merged). The gate is now sign-off + adoption/publication, not drafting — the table said 'Counsel-drafted' as if unwritten. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Defines the four platform seams the charter commits to (§4 'Vercel-first, not Vercel-only') — queue, scheduler/durable-work, blob storage, inbound email — as TypeScript contracts only. No adapters, no implementations. This is the architecture-locking artifact: it exists so engine code has interfaces to depend on before any code can call a platform SDK directly.
Design points worth a look on review:
Typechecks clean via
tsc --noEmit. Note:typescriptis not a devDependency yet — wire the typecheck script when HT-6/7/9 merge and the root package.json consolidates (kept off this branch to avoid a merge conflict).Jira: https://resonantiq.atlassian.net/browse/HT-9
🤖 Generated with Claude Code
https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Summary by CodeRabbit
New Features
Documentation
Chores