Route token grant picker and mutations through the shard-aware service layer - #198
Conversation
…e layer The /settings/tokens page read Documents and Collections straight off the default Y.Doc, so anything created via the service layer since #113/#120 (its own real shard) was silently missing from the grant picker. It also defined its own CURRENT_USER and did Space validation/persistence/audit inline in the route. Adds src/lib/services/tokens.ts (createToken/revokeToken: validate -> mutate -> audit, in one place) and points the route's load/actions at it and at the shard-aware listDocuments/listCollections via locals.requestContext.caller, matching the pattern already used by +layout.server.ts and space/[spaceId]/+page.server.ts. Closes #188 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (6)
Limit details: You’ve used all 2 included reviews currently available. Your 55 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughThe token settings flow now uses service-layer token operations and request callers. Token creation validates Space grants before persistence and records audits. The route loads Documents and Collections through shard-aware services, with regression coverage for sharded content. ChangesToken settings service flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Token settings now use shard-aware resource listing and centralized token operations while retaining validation, auditing, and one-time token display behavior. No current merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant Request
participant SettingsRoute
participant TokenService
participant Persistence
participant AuditLog
Request->>SettingsRoute: Submit token action
SettingsRoute->>TokenService: Create or revoke token with caller
TokenService->>Persistence: Validate and persist token state
Persistence-->>TokenService: Return token result
TokenService->>AuditLog: Record one audit event
TokenService-->>SettingsRoute: Return result or validation error
SettingsRoute-->>Request: Return action response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation The changes are limited to the token settings route, token service, related tests, service exports, and documentation required by issue Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Usage-based review receipt
Note This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing. Comment |
Summary
Closes #188 — the
/settings/tokensgrant picker read Documents/Collections straight off the defaultY.Doc, so anything created via the service layer since #113/#120 (its own real shard) was silently invisible there even though the sidebar and MCP listing surfaces saw it fine. The route also owned token mutation, Space validation, actor construction, and audit calls directly, outside the service manifest/request-context conventions the rest of the app follows.src/lib/services/tokens.ts—createToken/revokeToken, each a single function owning validate → mutate → audit, perservice-layer.md.createTokenvalidates every submittedallowedSpaceIdsentry against the workspace's real Spaces (UnknownSpaceError), same behavior as before, just moved out of the route.spaces.ts'screateSpacewas the existing precedent for this shape;tokens.tsfollows it (and, likespaces.ts, isn't registered inservices/manifest.ts's MCP/UI wiring table — no MCP tool exposes token minting, so that table's parity check doesn't apply).src/routes/settings/tokens/+page.server.tsnow loads Documents/Collections vialistDocuments/listCollections($lib/services, shard-aware) withlocals.requestContext.caller, matching+layout.server.ts/space/[spaceId]/+page.server.ts's existing pattern — no more route-localCURRENT_USERliteral.create_token/revoke_tokenaudit action names.docs/specifications/service-layer.md§3's module layout now liststokens.ts(previously only anticipated in prose) alongsidespaces.ts, with a note on why neither needs MCP manifest registration.Test plan
npm run check— typecheck cleannpm run lint— prettier + eslint cleannpm run test— 1010 tests pass, including new coverage:src/lib/services/tokens.test.ts(create/revoke audit-exactly-once, human vs. token-caller attribution, unknown-Space rejection with no partial token persisted) and a new regression test insrc/routes/settings/tokens/page.server.test.tsthat creates a Document and Collection through the service layer (therefore in their own real shards) and verifies both are listed byload().npm run test:e2e:tier-a— 16 tests pass, unaffected.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes