feat(auth): provision signup with listee libs#4
Conversation
WalkthroughAdds two local Bun-linked packages, enables skipLibCheck in TypeScript, and extends the auth service with token decoding helpers, a cached AccountProvisioner, and an account provisioning step during signup completion; also removes some exported type aliases. Changes
Sequence DiagramsequenceDiagram
participant User
participant SignupFlow
participant AuthService
participant ProvisionerLib
User->>SignupFlow: complete signup (fragment)
SignupFlow->>AuthService: completeSignupFromFragment(result)
AuthService->>AuthService: parse fragment & extract tokens
AuthService->>AuthService: decodeSupabaseToken(accessToken)
AuthService->>ProvisionerLib: getAccountProvisioner() (cached)
AuthService->>ProvisionerLib: provision(userId, tokenPayload, email)
ProvisionerLib-->>AuthService: provisioning result
AuthService->>AuthService: store refresh token
AuthService-->>SignupFlow: return signup completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
package.json (1)
17-18: Confirm portability of "link:" deps; consider workspace:/file: instead."link:" is tool-specific and may not install under npm/pnpm without Bun. If this CLI is used outside your monorepo or by contributors without Bun, prefer:
- workspaces:
"@listee/auth": "workspace:*"/"@listee/types": "workspace:*", or- explicit local paths:
"file:../auth".If Bun-only is intentional, ignore. Otherwise, adjust for wider compatibility.
tsconfig.json (1)
16-16: Avoid global skipLibCheck; it masks real type errors.This disables checks for all .d.ts (including your linked libs), weakening safety and IDE feedback.
- Prefer moving it to a build-only config (tsconfig.build.json) and keep strict checks in the editor.
- Or fix the offending library types; if only default libs are noisy, use
skipDefaultLibCheckinstead.Minimal change if you keep single config:
- "skipLibCheck": true, + "skipLibCheck": false,src/services/auth-service.ts (2)
18-25: Singleton is fine; add a test reset or DI hook.
- Expose a small internal reset (for tests) or allow injecting a provisioner to improve testability.
- Confirm
createAccountProvisioner()is synchronous; if it ever becomes async, guard against concurrent initialization.Example (optional):
// Only in test builds: export const __resetProvisionerForTests = () => { cachedAccountProvisioner = null; };
453-453: Be explicit about provisioning failure behavior.Provisioning runs before storing the refresh token. If provisioning fails, the user may need to re-trigger confirmation. If that’s intended (atomic signup), fine; otherwise consider storing the token first and rolling back on failure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
package.json(1 hunks)src/services/auth-service.ts(5 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/services/**/*.ts: Place Supabase-facing logic under src/services/
Read SUPABASE_URL and SUPABASE_ANON_KEY from environment variables or .env; never hardcode credentials
Default Keytar service name to "listee-cli"; allow overrides via env vars or CLI flags
Files:
src/services/auth-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict practices: forbid any, forbid as assertions, no implicit any; implement type guards for narrowing
Use two-space indentation and LF line endings
Use camelCase for identifiers
Keep comments purposeful; add brief context only for non-trivial flows
Maintain ASCII in files unless the file already uses Unicode
Files:
src/services/auth-service.ts
src/**/[a-z0-9-]*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case filenames for modules under src/
Files:
src/services/auth-service.ts
🧬 Code graph analysis (1)
src/services/auth-service.ts (1)
src/types/auth.ts (1)
SignupRedirect(37-40)
🔇 Additional comments (4)
src/services/auth-service.ts (4)
2-6: Imports look good; type-only used correctly.Verify that the linked packages resolve under your chosen package manager on CI and for contributors.
224-231: LGTM.Subject extraction with sanity checks is correct.
233-235: Consolidation through decodeSupabaseToken is good.Single decode path improves consistency.
357-361: LGTM.Path construction with
encodeURIComponentis correct and safe.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/services/auth-service.ts (5)
2-6: Private packages and skipLibCheck: confirm install/runtime plan.Using
@listee/authand@listee/typesvia local links is fine for dev, but will break for consumers unless these are resolvable in their env. Also, relying onskipLibCheckcan hide real type issues bubbling from these packages. Please confirm distribution strategy (publish, vend, or feature‑gate), and consider minimal local.d.tsshims for only the types you consume to avoid broadskipLibCheck.
18-25: Provisioner singleton: add a small test hook/DI seam.The cache is good. For tests, consider a tiny setter to inject/reset the provisioner and avoid cross‑test bleed.
Example (outside this hunk):
export const setAccountProvisionerForTests = (prov: AccountProvisioner | null): void => { cachedAccountProvisioner = prov; };
242-249: Clarify expired-token vs malformed-token error.Currently any failure in
isSupabaseTokenPayloadyields “payload structure is invalid,” which also covers expiry. Emit a distinct message for expired tokens to guide users.const decodeSupabaseToken = (token: string): SupabaseToken => { - const payload = decodeJwtPayload(token); - if (!isSupabaseTokenPayload(payload)) { - throw new Error("Access token payload structure is invalid."); - } - return payload; + const payload = decodeJwtPayload(token); + // Offer a clearer path when only expiry is the problem + if (isRecord(payload) && isNumber((payload as any).exp)) { + const now = Math.floor(Date.now() / 1000); + if ((payload as any).exp <= now) { + throw new Error("Access token has expired. Please restart signup with a fresh link."); + } + } + if (!isSupabaseTokenPayload(payload)) { + throw new Error("Access token payload structure is invalid."); + } + return payload; };
476-483: Ordering: provision before storing refresh token—verify intent.This avoids a “logged‑in but unprovisioned” state, which is reasonable for CLI UX. Confirm this trade‑off is desired (users must re-trigger signup if provisioning fails). Alternative: store first and rollback on failure to enable retry with the stored session.
485-505: Avoidconsole.errorin service; propagate structured error instead.Elsewhere the pattern is to throw enriched Errors without direct logging. Consider removing the side‑effect log and include account context in the thrown message.
try { await provisioner.provision({ userId, token: tokenPayload, email: result.account, }); } catch (error) { - const message = toErrorMessage(error); - console.error( - `Account provisioning failed for ${result.account}: ${message}`, - ); - throw new Error(`Account provisioning failed: ${message}`); + throw new Error( + `Account provisioning failed for ${result.account}: ${toErrorMessage(error)}` + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/auth-service.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/services/**/*.ts: Place Supabase-facing logic under src/services/
Read SUPABASE_URL and SUPABASE_ANON_KEY from environment variables or .env; never hardcode credentials
Default Keytar service name to "listee-cli"; allow overrides via env vars or CLI flags
Files:
src/services/auth-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict practices: forbid any, forbid as assertions, no implicit any; implement type guards for narrowing
Use two-space indentation and LF line endings
Use camelCase for identifiers
Keep comments purposeful; add brief context only for non-trivial flows
Maintain ASCII in files unless the file already uses Unicode
Files:
src/services/auth-service.ts
src/**/[a-z0-9-]*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case filenames for modules under src/
Files:
src/services/auth-service.ts
🧬 Code graph analysis (1)
src/services/auth-service.ts (1)
src/types/auth.ts (1)
SignupRedirect(37-40)
🔇 Additional comments (4)
src/services/auth-service.ts (4)
211-241: Stricter token payload guard — LGTM.Validates
sub,iat,exp, and rejects expired tokens.
251-258: Subject extraction — LGTM.Clear checks and precise error.
260-268: Email extraction via decoded payload — LGTM.Consistent with the stricter decode path.
383-388: Signup endpoint path construction — LGTM.Correct handling of optional
redirect_towith encoding.
1 participant
Summary
Summary by CodeRabbit
New Features
Refactor