Refactor/remove event sourcing and user state infrastructure#339
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 58 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/auth.ts">
<violation number="1" location="src/lib/auth.ts:28">
P0: Hardcoded auth secret fallback is a security risk in production. If both `BETTER_AUTH_SECRET` and `AUTH_SECRET` env vars are unset (e.g., misconfiguration during deploy), the app silently uses a publicly visible secret, enabling session forgery. Since this repo is open source, the fallback value is known to everyone.
Restrict the fallback to development only, and throw at startup if no secret is configured in production.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR removes the entire event-sourcing infrastructure (event store, reducer, projector, realtime hooks) and all per-user state persistence (flashcard Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/cleanup items with no correctness impact. All six findings are P2: dead parameter, no-op stubs without comments, a stale developer note, and a defensive migration suggestion. No logic errors, data loss paths, or broken contracts were found. The core refactor (Zero mutators, server auth guards, direct DB loader, local-only flashcard/quiz state) is coherent and correct. drizzle/0003_drop_workspace_event_tables.sql (add CASCADE for safety), src/lib/zero/mutators.ts (remove dead userId parameter). Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import type { Item } from "@/lib/workspace-state/types"; | ||
|
|
||
| export function sanitizeWorkspaceItemForPersistence(item: Item): Item { | ||
| return item; | ||
| } | ||
|
|
||
| export function sanitizeWorkspaceItemChanges( | ||
| changes: Partial<Item>, | ||
| ): Partial<Item> { | ||
| return changes; | ||
| } |
There was a problem hiding this comment.
Both exported functions are pure identity pass-throughs with no comment explaining whether this is intentional (user-state fields no longer exist in the type, so nothing to strip), or whether real sanitization logic is deferred. A brief comment would prevent future contributors from accidentally skipping a needed implementation.
| import type { Item } from "@/lib/workspace-state/types"; | |
| export function sanitizeWorkspaceItemForPersistence(item: Item): Item { | |
| return item; | |
| } | |
| export function sanitizeWorkspaceItemChanges( | |
| changes: Partial<Item>, | |
| ): Partial<Item> { | |
| return changes; | |
| } | |
| import type { Item } from "@/lib/workspace-state/types"; | |
| // User-state fields (currentIndex, quiz session, YouTube progress) were | |
| // removed from Item in this refactor, so no sanitization is currently needed. | |
| // These stubs are kept as extension points for future server-side field filtering. | |
| export function sanitizeWorkspaceItemForPersistence(item: Item): Item { | |
| return item; | |
| } | |
| export function sanitizeWorkspaceItemChanges( | |
| changes: Partial<Item>, | |
| ): Partial<Item> { | |
| return changes; | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export async function loadWorkspaceState( | ||
| workspaceId: string, | ||
| _options?: { userId?: string | null }, | ||
| ): Promise<Item[]> { | ||
| return db.transaction((tx) => readWorkspaceState(tx, workspaceId)); | ||
| } |
There was a problem hiding this comment.
_options parameter is dead API surface
_options (including userId) was used by the old state loader to fetch per-user projections. Now that user state is gone it is truly dead. Keeping it preserves a confusing call-site signature across all callers (e.g., mutation-helpers.ts, API routes). Consider removing it or replacing with a comment explaining the deliberate signature omission.
| export async function loadWorkspaceState( | |
| workspaceId: string, | |
| _options?: { userId?: string | null }, | |
| ): Promise<Item[]> { | |
| return db.transaction((tx) => readWorkspaceState(tx, workspaceId)); | |
| } | |
| export async function loadWorkspaceState( | |
| workspaceId: string, | |
| ): Promise<Item[]> { | |
| return db.transaction((tx) => readWorkspaceState(tx, workspaceId)); | |
| } |
| -- Drop user state table (user state persistence removed entirely) | ||
| DROP TABLE IF EXISTS workspace_item_user_state; | ||
|
|
||
| -- Drop projection state table (no longer needed - Zero syncs directly) | ||
| DROP TABLE IF EXISTS workspace_item_projection_state; | ||
|
|
||
| -- Drop events table (no longer needed - no event sourcing) | ||
| DROP TABLE IF EXISTS workspace_events; |
There was a problem hiding this comment.
DROP TABLE IF EXISTS without CASCADE may fail if cross-table FKs exist
In PostgreSQL, DROP TABLE (even with IF EXISTS) will still raise an error if another table has a foreign-key constraint pointing at the table being dropped. If workspace_item_projection_state or workspace_events cross-reference workspace_item_user_state, dropping workspace_item_user_state first would abort the migration. Adding CASCADE makes the drops unconditionally safe regardless of FK dependency order.
| -- Drop user state table (user state persistence removed entirely) | |
| DROP TABLE IF EXISTS workspace_item_user_state; | |
| -- Drop projection state table (no longer needed - Zero syncs directly) | |
| DROP TABLE IF EXISTS workspace_item_projection_state; | |
| -- Drop events table (no longer needed - no event sourcing) | |
| DROP TABLE IF EXISTS workspace_events; | |
| -- Drop user state table (user state persistence removed entirely) | |
| DROP TABLE IF EXISTS workspace_item_user_state CASCADE; | |
| -- Drop projection state table (no longer needed - Zero syncs directly) | |
| DROP TABLE IF EXISTS workspace_item_projection_state CASCADE; | |
| -- Drop events table (no longer needed - no event sourcing) | |
| DROP TABLE IF EXISTS workspace_events CASCADE; |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
3 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/app/api/zero/query/route.ts">
<violation number="1" location="src/app/api/zero/query/route.ts:9">
P1: Missing auth guard allows unauthenticated query execution on `/api/zero/query`.</violation>
</file>
<file name="src/lib/zero/client.ts">
<violation number="1" location="src/lib/zero/client.ts:9">
P1: Avoid defaulting public API URLs to localhost in client code; this breaks production clients when the env var is missing.</violation>
</file>
<file name="src/lib/zero/mutators.ts">
<violation number="1" location="src/lib/zero/mutators.ts:60">
P2: The custom JSON validator is too permissive and accepts non-plain objects (e.g. Date/Map), allowing invalid JSON-like payloads into mutators.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (typeof value === "object") { | ||
| return Object.values(value as Record<string, unknown>).every(isJsonValue); | ||
| } |
There was a problem hiding this comment.
P2: The custom JSON validator is too permissive and accepts non-plain objects (e.g. Date/Map), allowing invalid JSON-like payloads into mutators.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/zero/mutators.ts, line 60:
<comment>The custom JSON validator is too permissive and accepts non-plain objects (e.g. Date/Map), allowing invalid JSON-like payloads into mutators.</comment>
<file context>
@@ -41,21 +38,43 @@ type JsonValue =
+ return value.every(isJsonValue);
+ }
+
+ if (typeof value === "object") {
+ return Object.values(value as Record<string, unknown>).every(isJsonValue);
+ }
</file context>
| if (typeof value === "object") { | |
| return Object.values(value as Record<string, unknown>).every(isJsonValue); | |
| } | |
| if ( | |
| typeof value === "object" && | |
| value !== null && | |
| (Object.getPrototypeOf(value) === Object.prototype || | |
| Object.getPrototypeOf(value) === null) | |
| ) { | |
| return Object.values(value as Record<string, unknown>).every(isJsonValue); | |
| } |
There was a problem hiding this comment.
2 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/auth.ts">
<violation number="1" location="src/lib/auth.ts:107">
P1: The hard-coded fallback cookie domain can break auth sessions on production deployments outside `thinkex.app`.</violation>
</file>
<file name=".env.example">
<violation number="1" location=".env.example:51">
P1: Do not hard-code the production cookie domain in `.env.example`; use a placeholder so self-hosted deployments don't inherit an invalid cookie domain and break auth cookies.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| enabled: process.env.NODE_ENV === "production", | ||
| domain: process.env.ZERO_COOKIE_DOMAIN ?? ".thinkex.app", |
There was a problem hiding this comment.
P1: The hard-coded fallback cookie domain can break auth sessions on production deployments outside thinkex.app.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/auth.ts, line 107:
<comment>The hard-coded fallback cookie domain can break auth sessions on production deployments outside `thinkex.app`.</comment>
<file context>
@@ -95,6 +103,10 @@ export const auth = betterAuth({
// Force secure cookies (HTTPS only) - critical for preventing session hijacking
useSecureCookies: process.env.NODE_ENV === "production",
+ crossSubDomainCookies: {
+ enabled: process.env.NODE_ENV === "production",
+ domain: process.env.ZERO_COOKIE_DOMAIN ?? ".thinkex.app",
+ },
</file context>
| enabled: process.env.NODE_ENV === "production", | |
| domain: process.env.ZERO_COOKIE_DOMAIN ?? ".thinkex.app", | |
| enabled: process.env.NODE_ENV === "production" && !!process.env.ZERO_COOKIE_DOMAIN, | |
| domain: process.env.ZERO_COOKIE_DOMAIN, |
| # Zero Sync | ||
| NEXT_PUBLIC_ZERO_SERVER=http://localhost:4848 | ||
| ZERO_AUTH_SECRET=your-shared-secret-for-jwt-signing | ||
| ZERO_COOKIE_DOMAIN=.thinkex.app |
There was a problem hiding this comment.
P1: Do not hard-code the production cookie domain in .env.example; use a placeholder so self-hosted deployments don't inherit an invalid cookie domain and break auth cookies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .env.example, line 51:
<comment>Do not hard-code the production cookie domain in `.env.example`; use a placeholder so self-hosted deployments don't inherit an invalid cookie domain and break auth cookies.</comment>
<file context>
@@ -48,4 +48,4 @@ MISTRAL_API_KEY=your-mistral-api-key
# Zero Sync
NEXT_PUBLIC_ZERO_SERVER=http://localhost:4848
-ZERO_AUTH_SECRET=your-shared-secret-for-jwt-signing
+ZERO_COOKIE_DOMAIN=.thinkex.app
</file context>
| ZERO_COOKIE_DOMAIN=.thinkex.app | |
| ZERO_COOKIE_DOMAIN=.your-domain.com |
* Enforce authentication and workspace read access checks on Zero query endpoint * Fix Zero query workspace context checks Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
9e05a36
into
capy/migrate-writes-zero-mutators
* Replace event-sourcing writes with Zero mutators * Fix audio duration persistence * Make workflow persistence atomic * Refactor: remove event sourcing and user state infrastructure (#339) * Delete event-sourcing infrastructure and remove per-user state persistence * Remove auth secret fallback * Preserve legacy workspace tables * Remove unused workspace table migration * Fix Zero createdAt column type * Migrate Zero integration to 1.x query/mutator architecture * Refactor Zero auth to cookie-based + dedup server mutators * Fix Zero query auth: fail-closed + rename hasWorkspaceReadAccess (#385) * Enforce authentication and workspace read access checks on Zero query endpoint * Fix Zero query workspace context checks Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* Wire ZeroProvider and replace event-sourced reads with Zero useQuery * Fix ZeroProvider unauthenticated render * Fix ZeroProvider token initialization * Refactor: workspace writes to Zero mutators (#336) * Replace event-sourcing writes with Zero mutators * Fix audio duration persistence * Make workflow persistence atomic * Refactor: remove event sourcing and user state infrastructure (#339) * Delete event-sourcing infrastructure and remove per-user state persistence * Remove auth secret fallback * Preserve legacy workspace tables * Remove unused workspace table migration * Fix Zero createdAt column type * Migrate Zero integration to 1.x query/mutator architecture * Refactor Zero auth to cookie-based + dedup server mutators * Fix Zero query auth: fail-closed + rename hasWorkspaceReadAccess (#385) * Enforce authentication and workspace read access checks on Zero query endpoint * Fix Zero query workspace context checks Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
1 participant
This PR removes the event-sourcing layer (event store, reducer, projector, realtime event hooks) and drops all user state infrastructure. Reads and writes now go directly through the Zero-synced workspace items tables. All per-user state fields (flashcard currentIndex, quiz session, YouTube progress/playbackRate) are removed from schemas and persistence.
Event Sourcing & User State Removal
Types and Schema Cleanup
Component and Hook Updates
Database Migration