Repository navigation
feat(admin): add UserMeter parity verification - #1128
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # packages/worker/src/email/inbound.ts # packages/worker/src/email/outbound.ts # packages/worker/worker-configuration.d.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md # packages/worker/src/account/export.node.test.ts # packages/worker/src/account/export.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughChangesUserMeter–D1 parity capability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant MCP
participant ParityLoader
participant D1
participant UserMeter
Admin->>MCP: submit stable_user_id
MCP->>ParityLoader: loadUserMeterParityReport
ParityLoader->>D1: read account and entitlement state
ParityLoader->>UserMeter: read meter inventories
D1-->>ParityLoader: D1 parity inputs
UserMeter-->>ParityLoader: UserMeter parity inputs
ParityLoader-->>MCP: validated parity report
MCP-->>Admin: report and audit success
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
@coderabbitai review |
|
🔎 Preview deployed: https://kody-pr-1128.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/worker/src/admin/user-meter-parity.ts (1)
367-376: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBound the D1 inventory query with the same cap.
The meter walk stops at
userMeterParityInventoryMaxRows, but this query returns all D1 rows. For an account above the cap,d1Onlycounts every unpaged meter row as a D1-only mismatch. Thetruncatedflag keepsparityfalse, so no false pass occurs, but the reported categories mislead the operator.Apply
LIMIT userMeterParityInventoryMaxRows + 1and mark the compare truncated when the extra row appears.🤖 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/admin/user-meter-parity.ts` around lines 367 - 376, The D1 inventory query in the user-meter parity flow must use the same row cap as the meter walk. Update the query near d1RowsResult to request userMeterParityInventoryMaxRows + 1 rows, then detect the extra row, set the comparison’s truncated flag, and limit the rows used for d1Only classification to the configured cap.
🤖 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/admin/user-meter-parity.ts`:
- Around line 250-270: Extract the duplicated paging logic from the user-meter
parity flow into a shared helper that accepts the page-fetch function, preserves
maxRows and truncation behavior, and stops when nextStartAfter is missing or
does not advance beyond the previous cursor. Update the loop at
packages/worker/src/admin/user-meter-parity.ts:250-270 to use the helper, and
replace the repeated listWriteLeases loop at
packages/worker/src/admin/user-meter-parity.ts:286-306 with the same helper;
both sites require direct changes.
---
Nitpick comments:
In `@packages/worker/src/admin/user-meter-parity.ts`:
- Around line 367-376: The D1 inventory query in the user-meter parity flow must
use the same row cap as the meter walk. Update the query near d1RowsResult to
request userMeterParityInventoryMaxRows + 1 rows, then detect the extra row, set
the comparison’s truncated flag, and limit the rows used for d1Only
classification to the configured cap.
🪄 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: e40289dc-e06c-49cd-a7f9-1a7173c5295b
📒 Files selected for processing (7)
docs/contributing/architecture/entitlements.mdpackages/worker/src/admin/user-meter-parity.node.test.tspackages/worker/src/admin/user-meter-parity.tspackages/worker/src/mcp/capabilities/admin/admin-user-meter-parity.node.test.tspackages/worker/src/mcp/capabilities/admin/admin-user-meter-parity.tspackages/worker/src/mcp/capabilities/admin/domain.tspackages/worker/src/mcp/capabilities/registry.node.test.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/admin/user-meter-parity.node.test.ts (1)
265-379: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd a cross-user isolation test.
Seed a second
stableUserIdwith distinct daily counters, storage, package services, deletion state, and leases. Load the report for each user. Assert that each report contains only that user's state.The current single-user tests cannot detect a user-scoping error in a D1 query or Durable Object lookup. As per coding guidelines, "
**/*: Every signed-in user must have a fully isolated personal assistant, including separate packages, jobs, secrets, values, memories, remote connectors, email inboxes, and durable storage."🤖 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/admin/user-meter-parity.node.test.ts` around lines 265 - 379, Extend the test around loadAdminUserMeterParityReport with a second stableUserId and distinct daily, storage, package-service, deletion, and lease data in both D1 and the meter environment. Load reports for both users and assert each report’s daily, storage, packageServices, and deletion fields contain only its own seeded state, including distinct counts, values, and lease tokens, while preserving the existing secret-safety assertion.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@packages/worker/src/admin/user-meter-parity.node.test.ts`:
- Around line 265-379: Extend the test around loadAdminUserMeterParityReport
with a second stableUserId and distinct daily, storage, package-service,
deletion, and lease data in both D1 and the meter environment. Load reports for
both users and assert each report’s daily, storage, packageServices, and
deletion fields contain only its own seeded state, including distinct counts,
values, and lease tokens, while preserving the existing secret-safety assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ca8e38e-17f4-4da4-af9d-0e5ab9289c69
📒 Files selected for processing (2)
packages/worker/src/admin/user-meter-parity.node.test.tspackages/worker/src/admin/user-meter-parity.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/admin/user-meter-parity.ts
Summary
Admin-only read-only
admin_user_meter_paritycompares one stable user's D1 and UserMeter state for daily counters, storage bytes, package services, deletion tombstones, and lease mirrors. Output is sanitized and bounded; cold/stalled/truncated reports fail closed.System recap — composes parity verification surfaces (low risk)
Mode: recap · Base:
main@f0b00c4a· Head:a6180fc3No authority/state mutation; admin read model only.
Conductor report
doOnly=0,legacyWithoutD1=0).daily.mirrorRetired=true.env.USER_METER; flipping would create split authority. Earliest safe time is after that handoff deploy plus one complete reconciliation rotation with zero parity mismatches.doOnly=0), but production still had 166 D1-only rows: holder categories includedunspecified,mcp, andweb, not exclusively known email leases. Remove mirrors only after old-isolate drain plus zero unexplained non-email D1-only rows and successful mark/list/repair smoke tests.Summary by CodeRabbit
New Features
admin_user_meter_paritycapability for read-only comparison of account entitlement data.Documentation