Enforce storage bytes entitlement - #680
Conversation
📝 WalkthroughWalkthroughThis PR adds a ChangesStorage-bytes entitlement enforcement
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-680.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2ece0c3. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/worker/src/entitlements/service.ts (1)
278-514: 🚀 Performance & Scalability | 🔵 Trivial
getCurrentruns ~13 full-table aggregate scans on every guarded write.
readUserD1StorageBytesissues 13SUM(length(CAST(... AS BLOB)))aggregations across the user's rows, and it is invoked fromgetCurrenton every planned-user write chokepoint (storage_set/storage_sql, values/secrets/memory upserts, email storage). For users with largeemail_messages/package_runtime_*histories these blob-length scans are non-trivial and repeat per write.Consider materializing/caching per-user storage byte usage (e.g., a periodically-refreshed rollup row or short-lived cache) rather than recomputing all surfaces synchronously on each write, given these are denial-of-wallet caps rather than billing-grade accounting.
🤖 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 `@packages/worker/src/entitlements/service.ts` around lines 278 - 514, readUserD1StorageBytes currently does many full-table SUM scans and is called from getCurrent on every guarded write, causing expensive repeated aggregation work. Update the storage-usage path so getCurrent does not synchronously recompute all totals each time; instead, have readUserD1StorageBytes read from a cached or materialized per-user rollup, refreshed asynchronously or on a short TTL, and keep the existing callers like getCurrent, storage_set, and storage_sql pointing to that cheaper lookup.
🤖 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/src/email/inbound.ts`:
- Around line 324-329: Update the gate-ordering comment in inbound email
handling to match the actual rejection flow in the function that performs the
checks before delivery. The comment should now mention the new storage-bytes
entitlement gate by name, placed between the per-message size cap and the
per-day receive rate, so it stays aligned with the sequence around
assertWithinStorageBytesEntitlement, assertWithinDailyReceiveRateEntitlement,
and assertWithinStoredMessagesEntitlement.
---
Nitpick comments:
In `@packages/worker/src/entitlements/service.ts`:
- Around line 278-514: readUserD1StorageBytes currently does many full-table SUM
scans and is called from getCurrent on every guarded write, causing expensive
repeated aggregation work. Update the storage-usage path so getCurrent does not
synchronously recompute all totals each time; instead, have
readUserD1StorageBytes read from a cached or materialized per-user rollup,
refreshed asynchronously or on a short TTL, and keep the existing callers like
getCurrent, storage_set, and storage_sql pointing to that cheaper lookup.
🪄 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: c2a3c3ce-859c-4010-9ce2-69552b1e5728
📒 Files selected for processing (20)
docs/contributing/architecture/entitlements.mdpackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/email/inbound-entitlements.workers.test.tspackages/worker/src/email/inbound.tspackages/worker/src/email/outbound.tspackages/worker/src/entitlements/entitlements.node.test.tspackages/worker/src/entitlements/service.tspackages/worker/src/mcp/capabilities/integrations/integration-save.tspackages/worker/src/mcp/capabilities/meta/meta-memory-upsert.tspackages/worker/src/mcp/capabilities/storage/storage-query.tspackages/worker/src/mcp/capabilities/values/value-set.tspackages/worker/src/mcp/memory/service.tspackages/worker/src/mcp/run-kody-registry.tspackages/worker/src/mcp/secrets/service.tspackages/worker/src/mcp/values/service.tspackages/worker/src/package-registry/service.node.test.tspackages/worker/src/package-registry/service.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/storage-runner.tspackages/worker/src/storage-runner.workers.test.ts

Summary
storage_bytesD1 byte estimator for countable user-owned durable payloads (email raw/body metadata, values, encrypted secrets, memories, saved package/job/runtime metadata, and related projections).storage_querywritable calls, and package app storage RPC writes.getCurrenthandling, and current gaps for stores without reliable byte metadata (Artifacts/KV bodies/Vectorize scans).Verification
npm run test -- packages/worker/src/package-registry/service.node.test.ts packages/worker/src/entitlements/entitlements.node.test.ts packages/worker/src/storage-runner.workers.test.ts packages/worker/src/email/inbound-entitlements.workers.test.tsnpm run typechecknpm run format:checknpm run validate(format, lint, typecheck, unit tests, Playwright E2E, and MCP E2E all exited 0)System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@80e35c5· Head:a57a8edClassification: extends — this PR changes entitlement enforcement behavior for an existing plan resource and wires existing storage primitives into that guard.
Primitives touched
entitlementsstorage_bytesnow has a D1 byte counter, shared storage-byte assertion path, and byte-estimation/delta helpersdurable-storaged1-app-dbemailvaluessecretsmemoriessaved-packagesmcp-serverSystem map
Change flow
Invariants
per-user-isolation: all byte-counting SQL filters byuser_id, transitive child counts join back to user-owned parent rows, and StorageRunner estimates are read from the(userId, storageId)Durable Object namespace.storage_bytes: users without a known plan return before byte counting and remain unlimited for this resource.Summary by CodeRabbit
New Features
Bug Fixes
Documentation