Production-grade rebuild: architecture, auth, reminders, resilience#3
Merged
Conversation
- vitest + zod; test/test:watch scripts - Capacitor config + Electron desktop scaffold (@capacitor-community/electron) - BUILD_TARGET=native static export in next.config - eslint/tsconfig ignore native output; gitignore native + local settings Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, reminders, auth - Layered lib/: time (local day keys), domain (zod schemas + selectors + validation), store (versioned migrate, StoragePort seam, actions-only mutation) - Rewire components/hooks to the new layers; computed streak (was hardcoded) - Fix systemic Radix data-attr bug: data-[orientation]/data-[state] in slider, tabs, separator, dialog, sheet - Landing at /, clickable dashboard cards, branded error/404/global-error - Input validation (limits, habit dedupe/cap, real word count) with inline errors - Honest reminders: NotificationPort + scheduler + settings UI - Supabase email/password auth, (protected) route-group gate, /login, sign-out; graceful local-only mode when unconfigured Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- ROADMAP.md: state, architecture conventions, parallelizable workstreams, sync SQL - CLAUDE.md: per-session conventions + the quality gate - docs/showcase.md: LinkedIn post draft + asset checklist - README rewrite (web/mobile/desktop run steps) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add CI workflow: typecheck, lint, test, build on push/PR to main - Add persistence.test.ts: SSR guard, read/write/remove, error resilience - Add actions.test.ts: updateTodayEntry, addHabit/removeHabit, setNotificationTime - Add TimeOfDaySchema to entry.ts, use it in setNotificationTime to reject invalid times (was a silent bug — any string was accepted) - Verification: 51 tests pass (was 39), tsc --noEmit clean
ci: add GitHub Actions CI + store tests + validate notification time
# Conflicts: # app/page.tsx # components/evening/step-journal.tsx # components/morning/step-mood.tsx
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConverts the app to an auth-gated, domain-driven architecture: Zod domain models and validation, a migrated SSR-compatible reactive store with migration and actions, notification scheduling and ReminderScheduler, Supabase auth provider and login, protected pages and routing updates, Electron/Capacitor native wiring, CI/tooling, and documentation/tests. ChangesAuthentication & Protected App Shell
State & Domain Architecture
Notifications & Scheduling
App Pages, Routing, & Component Updates
Native Platform Support (Electron & Capacitor)
Project Configuration & Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/timeline-view.tsx (1)
154-156:⚠️ Potential issue | 🟡 Minor | ⚡ Quick wincomponents/timeline-view.tsx: sort timeline entries by the day key (not
new Date(dayKey))
state.entries[*].dateis a localYYYY-MM-DDday key; sorting vianew Date(b.date)relies on JS date-only parsing semantics and bypasses thelib/time/todayday-key approach. Compare the normalized keys directly.Suggested fix
- const entries = Object.values(state.entries).sort( - (a, b) => new Date(b.date).getTime() - new Date(a.date).getTime() - ) + const entries = Object.values(state.entries).sort((a, b) => b.date.localeCompare(a.date))🤖 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 `@components/timeline-view.tsx` around lines 154 - 156, The sort currently converts the local YYYY-MM-DD day key via new Date(b.date) which relies on JS parsing and bypasses our lib/time/today semantics; update the sort of entries (state.entries → entries) to compare the normalized day-key strings directly (e.g., compare b.date and a.date lexicographically for descending order) instead of using new Date(...), so the ordering uses the canonical day-key representation.components/ui/tabs.tsx (1)
15-23:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPass
orientationtoTabsPrimitive.Root.Radix’s
Tabs.Rootuses theorientationprop to configure its internal keyboard/ARIA logic (e.g., switching arrow-key bindings for vertical tabs). Setting onlydata-orientationaffects styling/CSS selectors but won’t update the component’s interaction behavior, so keyboard accessibility can stay inconsistent with the visual orientation.Proposed fix
<TabsPrimitive.Root data-slot="tabs" - data-orientation={orientation} + orientation={orientation} className={cn( "group/tabs flex gap-2 data-[orientation=horizontal]:flex-col", className )} {...props} />🤖 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 `@components/ui/tabs.tsx` around lines 15 - 23, The TabsPrimitive.Root is only receiving a data-orientation attribute for styling, but not the actual orientation prop so Radix's keyboard/ARIA behavior won't match visual orientation; update the Tabs component to forward the orientation prop to TabsPrimitive.Root (in addition to setting data-orientation and className) so that TabsPrimitive.Root receives the orientation value from props (ensure the orientation prop passed into your component is included in the spread/explicit props sent to TabsPrimitive.Root).
🟠 Major comments (22)
electron/electron-builder.config.json-2-2 (1)
2-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace scaffold bundle identifiers before release.
Line 2 and Line 25 still use template values (
com.yourdoamnin.yourapp,your.app.category.type). This can break signing/update identity and produce misidentified artifacts in production packaging.Suggested fix
- "appId": "com.yourdoamnin.yourapp", + "appId": "app.anchor.ritual", @@ - "category": "your.app.category.type", + "category": "public.app-category.productivity",Also applies to: 25-25
🤖 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 `@electron/electron-builder.config.json` at line 2, Replace the scaffold bundle identifiers in the JSON: update the "appId" value ("com.yourdoamnin.yourapp") to your real reverse-DNS bundle identifier and replace the category value ("your.app.category.type") with the correct Mac App Store category string; locate these keys in electron-builder.config.json (the "appId" and "category" properties) and ensure they match your official app/team identifiers used for signing and updates before building/releases.electron/package.json-9-12 (1)
9-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate repository metadata required for GitHub publishing.
Line 9-12 leaves
repository.urlempty while the builder config publishes to GitHub; release publishing may fail or target the wrong project.Suggested fix
"repository": { "type": "git", - "url": "" + "url": "https://github.com/TargiX/Next.js-Tether.git" },🤖 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 `@electron/package.json` around lines 9 - 12, The package.json's repository metadata is missing the repo URL which breaks GitHub publishing; set the "repository.url" field to the repo's git URL (for example "https://github.com/OWNER/REPO.git" or "git+https://github.com/OWNER/REPO.git") so the builder can target the correct GitHub project—update the "repository" object (the existing "type" and "url" keys) to include the correct URL string.electron/live-runner.js-64-66 (1)
64-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose the previous watcher before replacing it.
At Line 64-66, setting
reloadWatcher.watcher = nullwithout calling.close()leaks active chokidar watchers and can cause duplicate restart triggers over time.Suggested fix
- reloadWatcher.watcher = null; - setupReloadWatcher(); + if (reloadWatcher.watcher) { + await reloadWatcher.watcher.close(); + reloadWatcher.watcher = null; + } + setupReloadWatcher();🤖 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 `@electron/live-runner.js` around lines 64 - 66, The reloadWatcher currently nulls out reloadWatcher.watcher without closing it, leaking chokidar watchers and causing duplicate triggers; update the code that resets the watcher used by setupReloadWatcher() to first call reloadWatcher.watcher.close() (or await its close if it returns a Promise) and handle/ignore any errors from close(), then set reloadWatcher.watcher = null and call setupReloadWatcher() so the old watcher is properly disposed before replacement.electron/live-runner.js-18-22 (1)
18-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast on build errors instead of always resolving.
At Line 18-22,
runBuild()resolves even whennpm run buildexits non-zero, so restart can proceed with invalid artifacts.Suggested fix
function runBuild() { - return new Promise((resolve, _reject) => { - let tempChild = cp.spawn(npmCmd, ['run', 'build']); - tempChild.once('exit', () => { - resolve(); - }); + return new Promise((resolve, reject) => { + const tempChild = cp.spawn(npmCmd, ['run', 'build']); + tempChild.once('error', reject); + tempChild.once('exit', (code) => { + if (code === 0) resolve(); + else reject(new Error(`Build failed with exit code ${code}`)); + }); tempChild.stdout.pipe(process.stdout); + tempChild.stderr.pipe(process.stderr); }); }🤖 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 `@electron/live-runner.js` around lines 18 - 22, The runBuild promise currently always resolves when tempChild exits; change runBuild to reject the promise when the spawned process (tempChild, using npmCmd and args ['run','build']) exits with a non-zero code or is terminated by a signal, and also listen for the 'error' event on tempChild to reject with that error; only resolve when exit code === 0. Ensure the reject includes the exit code, signal, or error object so callers can fail fast on build errors.electron/src/setup.ts-127-129 (1)
127-129:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix background color config path mismatch.
The guard checks
this.CapacitorFileConfig.backgroundColor, but the value is read fromthis.CapacitorFileConfig.electron.backgroundColor. Use the same path for both.💡 Suggested fix
- if (this.CapacitorFileConfig.backgroundColor) { - this.MainWindow.setBackgroundColor(this.CapacitorFileConfig.electron.backgroundColor); + if (this.CapacitorFileConfig.electron?.backgroundColor) { + this.MainWindow.setBackgroundColor(this.CapacitorFileConfig.electron.backgroundColor); }🤖 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 `@electron/src/setup.ts` around lines 127 - 129, The guard currently checks this.CapacitorFileConfig.backgroundColor but reads this.CapacitorFileConfig.electron.backgroundColor, causing a path mismatch; update the condition to check the same path used when setting the color (e.g., ensure this.CapacitorFileConfig.electron exists and use this.CapacitorFileConfig.electron.backgroundColor in the if statement) before calling this.MainWindow.setBackgroundColor so the property is consistently referenced.electron/src/index.ts-48-50 (1)
48-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate updater calls to packaged builds.
electron/src/index.tscallsautoUpdater.checkForUpdatesAndNotify()unconditionally (noapp.isPackagedguard), despite the comment saying it’s for packaged apps; this can cause noisy failures during dev.💡 Suggested fix
- // Check for updates if we are in a packaged app. - autoUpdater.checkForUpdatesAndNotify(); + // Check for updates only in packaged builds. + if (app.isPackaged) { + await autoUpdater.checkForUpdatesAndNotify().catch((error) => { + console.error('Auto-update check failed:', error); + }); + }🤖 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 `@electron/src/index.ts` around lines 48 - 50, The updater is being invoked unconditionally; wrap the autoUpdater.checkForUpdatesAndNotify() call in an app.isPackaged guard so it only runs in packaged builds. Locate the IIFE or module init that currently calls autoUpdater.checkForUpdatesAndNotify() and change it to check app.isPackaged (from electron.app) before invoking autoUpdater.checkForUpdatesAndNotify(), ensuring the updater is skipped during development.components/habit-icon.tsx-16-18 (1)
16-18: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winType
iconfrom the domain schema instead ofstring.Using
stringhere weakens the contract and allows invalid icon keys at compile time. Please derive this prop type from the domain Zod schema (z.infer) so UI and storage stay in sync.As per coding guidelines: "
**/*.{ts,tsx}: Zod schemas are the source of truth; derive types usingz.infer."🤖 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 `@components/habit-icon.tsx` around lines 16 - 18, The prop type for icon should be derived from the domain Zod schema rather than string; import the appropriate Zod schema (e.g. habitSchema or the schema that defines the icon field), create a type alias like IconKey = z.infer<typeof <YourSchema>>['icon'], and update the HabitIcon signature to use { icon }: { icon: IconKey } & LucideProps so the icon prop is checked against the domain schema (ensure the ICONS lookup still accepts IconKey values and adjust imports to reference the schema name you use)..github/workflows/ci.yml-13-17 (1)
13-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden workflow action supply chain and tighten token scope.
- Uses mutable action tags:
actions/checkout@v4andactions/setup-node@v4- Checkout credential persistence isn’t explicitly disabled (
persist-credentials: falsemissing)- No top-level
permissions:block to constrainGITHUB_TOKENscope🔐 Suggested patch
name: CI on: push: branches: [main] pull_request: branches: [main] +permissions: + contents: read + jobs: check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@<pinned-commit-sha> + with: + persist-credentials: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@<pinned-commit-sha> with: node-version-file: .node-version🤖 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 @.github/workflows/ci.yml around lines 13 - 17, Replace the mutable action tags and tighten token scope: pin "uses: actions/checkout@v4" and "uses: actions/setup-node@v4" to their immutable commit SHAs (replace the `@v4` refs with the exact SHA for each action), add "persist-credentials: false" to the checkout step to avoid leaving GITHUB_TOKEN in the workspace, and add a top-level "permissions:" block that limits GITHUB_TOKEN to the minimal scopes your workflow needs (for example "contents: read" plus any specific additional scopes like "id-token: write" only if required).lib/auth/credentials.ts-7-13 (1)
7-13: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove
PASSWORD_MINinto centralizedLIMITSand import it here.This file introduces a hardcoded limit outside
lib/domain/validation, which breaks the repo-wide limit centralization rule.As per coding guidelines, “Limits and magic numbers must live in `lib/domain/validation` (`LIMITS`)”.Proposed change
-import { z } from "zod" +import { z } from "zod" +import { LIMITS } from "`@/lib/domain/validation`" @@ -export const PASSWORD_MIN = 8 +export const PASSWORD_MIN = LIMITS.passwordMin🤖 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 `@lib/auth/credentials.ts` around lines 7 - 13, Replace the hardcoded PASSWORD_MIN constant with the centralized LIMITS export from lib/domain/validation: remove the local export const PASSWORD_MIN and import LIMITS (or its PASSWORD_MIN property) at the top of this file, then update PasswordSchema to use LIMITS.PASSWORD_MIN (or LIMITS.passwordMin as applicable); keep EmailSchema and PasswordSchema names unchanged so only the constant source is changed to comply with the repo-wide limit centralization rule.lib/store/state.ts-15-16 (1)
15-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate persisted notification times with
TimeOfDaySchema.Lines 15–16 and 72–73 accept any string, so invalid times survive migration and can break downstream reminder logic.
Suggested fix
-import { DayEntrySchema, type DayEntry } from "`@/lib/domain/entry`" +import { DayEntrySchema, TimeOfDaySchema, type DayEntry } from "`@/lib/domain/entry`" export const AppStateSchema = z.object({ entries: z.record(z.string(), DayEntrySchema), habits: z.array(HabitSchema), - notificationMorning: z.string(), - notificationEvening: z.string(), + notificationMorning: TimeOfDaySchema, + notificationEvening: TimeOfDaySchema, }) @@ - const morning = z.string().safeParse(obj.notificationMorning) - const evening = z.string().safeParse(obj.notificationEvening) + const morning = TimeOfDaySchema.safeParse(obj.notificationMorning) + const evening = TimeOfDaySchema.safeParse(obj.notificationEvening)As per coding guidelines,
**/*.{ts,tsx}: Zod schemas are the source of truth; derive types usingz.infer. Validate at the storage boundary only.Also applies to: 72-73
🤖 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 `@lib/store/state.ts` around lines 15 - 16, Update the Zod schema so persisted notification times are validated by TimeOfDaySchema: replace notificationMorning: z.string() and notificationEvening: z.string() with notificationMorning: TimeOfDaySchema and notificationEvening: TimeOfDaySchema in the state schema (e.g., StateSchema or whatever schema contains those keys) and ensure any exported type is derived via z.infer<typeof StateSchema>; also make sure validation happens at the storage boundary when reading/writing state so invalid strings cannot survive migration.lib/store/state.ts-27-28 (1)
27-28: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove default notification times into
LIMITSinstead of inline literals.Lines 27–28 introduce hardcoded values in state initialization; these should come from
lib/domain/validationto keep limits/defaults centralized.As per coding guidelines,
**/*.{ts,tsx}: Limits and magic numbers must live inlib/domain/validation(LIMITS).🤖 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 `@lib/store/state.ts` around lines 27 - 28, Replace the hardcoded "08:00" and "20:00" in the state initialization (properties notificationMorning and notificationEvening) with the centralized defaults from LIMITS: add an import for LIMITS from lib/domain/validation and use LIMITS.notificationMorning and LIMITS.notificationEvening (or the corresponding LIMITS keys) in place of the inline literals so limits/defaults remain centralized.lib/store/actions.ts-15-23 (1)
15-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent
patch.datefrom desynchronizing entry identity.Line 21 spreads
patchlast, soupdateTodayEntry({ date: ... })can create an entry whose internaldateno longer matches its map key.Suggested fix
export function updateTodayEntry(patch: Partial<DayEntry>): void { const key = getTodayKey() + const { date: _ignoredDate, ...safePatch } = patch setState((prev) => ({ ...prev, entries: { ...prev.entries, - [key]: { ...(prev.entries[key] ?? emptyEntry(key)), ...patch }, + [key]: { ...(prev.entries[key] ?? emptyEntry(key)), ...safePatch, date: key }, }, })) }🤖 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 `@lib/store/actions.ts` around lines 15 - 23, The updateTodayEntry function currently spreads patch last which allows patch.date to override the entry's identity; change the merge order so the entry's date is derived from the map key (use getTodayKey()) or the existing entry/emptyEntry and cannot be overwritten by patch — i.e., when constructing the new entry in updateTodayEntry ensure date is explicitly set from key (or prev.entries[key]?.date or emptyEntry(key).date) after merging and ignore patch.date so patch updates other fields only; update the object construction that uses emptyEntry(key) and patch accordingly.lib/domain/entry.ts-24-26 (1)
24-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce local day-key format in
DayEntrySchema.Line 25 currently accepts any string, so non-local keys (including UTC-derived formats) can pass storage validation and poison streak/day lookups.
Suggested fix
+export const DayKeySchema = z.string().regex(/^\d{4}-\d{2}-\d{2}$/) + export const DayEntrySchema = z.object({ /** Local calendar day, `YYYY-MM-DD`. */ - date: z.string(), + date: DayKeySchema,As per coding guidelines,
**/*.{ts,tsx}: Use local day keys vialib/time/today, never usetoISOString()(UTC bug).🤖 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 `@lib/domain/entry.ts` around lines 24 - 26, DayEntrySchema's date currently accepts any string; tighten validation by changing the date schema from z.string() to a constrained string that only accepts local day-keys (YYYY-MM-DD) — e.g., use z.string().regex(/^\d{4}-\d{2}-\d{2}$/, 'local day key') or z.string().refine(...) to enforce the pattern, and ensure code that produces keys uses the lib/time/today helper rather than toISOString(); update the schema's date field (referenced as date in DayEntrySchema / DayEntry type) accordingly.lib/notifications/schedule.ts-8-13 (1)
8-13: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winCentralize reminder time bounds in
LIMITSinstead of inline literals.
parseTimehardcodes limit values (23,59). Please source these fromlib/domain/validation(LIMITS) so limits stay single-sourced and don’t drift across modules.As per coding guidelines,
**/*.{ts,tsx}: Limits and magic numbers must live inlib/domain/validation(LIMITS).🤖 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 `@lib/notifications/schedule.ts` around lines 8 - 13, Replace the inline magic numbers in parseTime with the centralized LIMITS from lib/domain/validation: import { LIMITS } from 'lib/domain/validation' at the top of lib/notifications/schedule.ts and change the range checks that use 23 and 59 to use LIMITS.HOURS (or LIMITS.MAX_HOURS) and LIMITS.MINUTES (or LIMITS.MAX_MINUTES) respectively, keeping the regex and Number(match[1])/Number(match[2]) logic unchanged so the function still returns { hours, minutes } or null.app/(protected)/settings/page.tsx-26-27 (1)
26-27: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftReplace direct
setStateusage with store actions.Line 26 through Line 27 and Line 33 through Line 37 (plus Line 89 through Line 90) mutate component state directly. Per repo policy, these updates should flow through
lib/store/actions.As per coding guidelines:
**/*.{ts,tsx}: Mutate state only vialib/store/actions, never usesetStatedirectly in a component.Also applies to: 33-37, 88-90
🤖 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 `@app/`(protected)/settings/page.tsx around lines 26 - 27, This component currently mutates local state via setNewHabit and setHabitError; replace those direct setState calls with store actions from lib/store/actions: create or use actions such as setNewHabit (or updateHabitInput) and setHabitError in lib/store/actions, import them into this component, dispatch them wherever the code currently calls setNewHabit(...) or setHabitError(...), and read the values via the corresponding store selectors instead of useState; ensure handlers referenced around the current newHabit/setNewHabit and habitError/setHabitError usages (the input change handler, validation/submit logic, and error-clear logic) call the exported action creators rather than calling the setState functions directly.components/auth-provider.tsx-26-42 (1)
26-42: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove auth state mutations behind store actions.
Line 26 through Line 42 directly mutates component state via
useStatesetters. This breaks the TS/TSX state-mutation rule for this repo and should be routed throughlib/store/actions.As per coding guidelines:
**/*.{ts,tsx}: Mutate state only vialib/store/actions, never usesetStatedirectly in a component.🤖 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 `@components/auth-provider.tsx` around lines 26 - 42, The effect currently calls setUser and setStatus directly inside useEffect (in the supabase.auth.getSession promise and supabase.auth.onAuthStateChange callback); instead route these mutations through the store action API in lib/store/actions: import the appropriate actions (e.g., updateAuthUser and updateAuthStatus or a single updateAuthFromSession action) and replace calls to setUser(...) and setStatus(...) with calls to those actions (or create a new action that accepts the session and sets both user and status). Keep the logic that derives user from session (session?.user ?? null) and status (session ? "authed" : "anon") but invoke the store actions instead of using setUser/setStatus directly in the getSession and onAuthStateChange handlers.app/error.tsx-43-47 (1)
43-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid nested interactive elements in the “Back to app” action.
Line 43 through Line 47 wraps a button inside a link, which creates invalid nested interactive controls and can break keyboard/screen-reader behavior.
Proposed fix
- <Link href="/app"> - <Button variant="outline" className="rounded-2xl"> - Back to app - </Button> - </Link> + <Button asChild variant="outline" className="rounded-2xl"> + <Link href="/app">Back to app</Link> + </Button>🤖 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 `@app/error.tsx` around lines 43 - 47, The "Back to app" control nests a Button inside a Link (Link + Button), which creates invalid nested interactive elements; fix by making only one interactive element: either remove the Link and make Button a client-side navigation control (use useRouter().push('/app') in the error component's onClick of Button) or remove the Button and style the Link like the Button (give Link the Button's className/variant and role="button"/aria-label). Update the JSX to use only Link or only Button (reference the existing Link and Button symbols) and ensure keyboard/accessibility attributes (role/aria-label) are present if you convert Link to look like a button.app/(protected)/app/page.tsx-17-27 (1)
17-27: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove time-state mutation behind store actions/hook boundary.
useTimeInfomutates local component state viasetTime, which conflicts with the project’s actions-only mutation rule.As per coding guidelines
Mutate state only via lib/store/actions, never use setState directly in a component.🤖 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 `@app/`(protected)/app/page.tsx around lines 17 - 27, The component directly calls setTime inside useEffect (setTime, useEffect in page.tsx) which violates the rule "mutate state only via lib/store/actions"; refactor by moving the time update behind a store action or custom hook: create or use an action such as updateTime/getTime in your store (or a useTimeInfo hook that calls the store action) and replace the direct setTime call in the useEffect with a call to that action/hook to set mounted and hour; keep the SSR default state (mounted: false, hour: 12) in the component but delegate all mutations to the store action/hook so the effect calls store.updateTime({ mounted: true, hour: new Date().getHours() }) instead of setTime.app/page.tsx-66-71 (1)
66-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse one interactive element per CTA (avoid
Link/awrappingButton).Current markup risks invalid nested controls and accessibility regressions. Switch to
Button asChildfor link CTAs.♻️ Proposed fix
- <Link href="/app"> - <Button size="sm" className="rounded-xl"> + <Button asChild size="sm" className="rounded-xl"> + <Link href="/app"> Open app <ArrowRight className="size-4" data-icon="inline-end" /> - </Button> - </Link> + </Link> + </Button> ... - <Link href="/app"> - <Button size="lg" className="h-12 rounded-2xl px-7 text-base font-medium"> + <Button asChild size="lg" className="h-12 rounded-2xl px-7 text-base font-medium"> + <Link href="/app"> Try it now <ArrowRight className="size-4" data-icon="inline-end" /> - </Button> - </Link> - <a href="`#features`"> - <Button - size="lg" - variant="outline" - className="h-12 rounded-2xl px-7 text-base font-medium" - > - See how it works - </Button> - </a> + </Link> + </Button> + <Button + asChild + size="lg" + variant="outline" + className="h-12 rounded-2xl px-7 text-base font-medium" + > + <a href="`#features`">See how it works</a> + </Button> ... - <Link href="/app"> - <Button size="lg" className="mt-8 h-12 rounded-2xl px-8 text-base font-medium"> + <Button asChild size="lg" className="mt-8 h-12 rounded-2xl px-8 text-base font-medium"> + <Link href="/app"> Open Anchor <ArrowRight className="size-4" data-icon="inline-end" /> - </Button> - </Link> + </Link> + </Button>Also applies to: 122-137, 268-273
🤖 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 `@app/page.tsx` around lines 66 - 71, The current markup nests a Link around a Button (Link + Button + ArrowRight), causing invalid nested interactive controls; replace that pattern by using Button with the asChild prop and render the Link (or <a> with href="/app") as the Button's child so the resulting element is a single interactive control. Update the instances that use Link wrapping Button (the one containing Open app with ArrowRight and the other occurrences at the same patterns) to: keep the ArrowRight inside the Button, remove the outer Link, use Button asChild and pass the Link (or anchor with href) as the child element so the rendered element is a single accessible link-button. Ensure to import/use the same Link component as the child and keep Button props like size and className intact.app/login/page.tsx-18-24 (1)
18-24: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftRoute login form mutations through
lib/store/actions.This page directly mutates component state via
setMode,setEmail,setPassword,setErrors,setNotice, andsetSubmitting. Please move these transitions behind store actions/hooks to keep mutation paths consistent with the app architecture.As per coding guidelines
Mutate state only via lib/store/actions, never use setState directly in a component.Also applies to: 33-42, 127-131
🤖 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 `@app/login/page.tsx` around lines 18 - 24, The component is directly calling setMode, setEmail, setPassword, setErrors, setNotice, and setSubmitting; move these mutations into store actions in lib/store/actions and replace direct setState calls with those actions via the store hook (e.g., useStore or dispatch). Specifically, create actions like setLoginMode(mode: Mode), updateLoginEmail(email: string), updateLoginPassword(password: string), setLoginErrors(errors), setLoginNotice(notice: string | null), and setLoginSubmitting(submitting: boolean) in lib/store/actions, then import and invoke those actions from the login page instead of calling setMode/setEmail/setPassword/setErrors/setNotice/setSubmitting directly; update any event handlers and effect logic referencing Mode, the email/password state, errors, notice, or submitting to read state from the store hook and trigger the new actions. Ensure all other occurrences (lines ~33-42 and ~127-131) are refactored the same way.app/login/page.tsx-43-54 (1)
43-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnsure
submittingis always cleared on auth call failure.If
signIn/signUpthrows,setSubmitting(false)is skipped and the form can stay locked.🔧 Proposed fix
- if (mode === "signin") { - const { error } = await signIn(email.trim(), password) - if (error) setErrors({ form: error }) - else router.replace("/app") - } else { - const { error, needsConfirmation } = await signUp(email.trim(), password) - if (error) setErrors({ form: error }) - else if (needsConfirmation) setNotice("Check your email to confirm your account.") - else router.replace("/app") - } - setSubmitting(false) + try { + if (mode === "signin") { + const { error } = await signIn(email.trim(), password) + if (error) setErrors({ form: error }) + else router.replace("/app") + } else { + const { error, needsConfirmation } = await signUp(email.trim(), password) + if (error) setErrors({ form: error }) + else if (needsConfirmation) setNotice("Check your email to confirm your account.") + else router.replace("/app") + } + } catch { + setErrors({ form: "Something went wrong. Please try again." }) + } finally { + setSubmitting(false) + }🤖 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 `@app/login/page.tsx` around lines 43 - 54, The auth branch can throw and skip setSubmitting(false), leaving the form locked; wrap the signIn/signUp calls and their result handling in a try/catch/finally (or ensure setSubmitting(false) is executed in a finally block) so setSubmitting(false) always runs even on exceptions; reference the existing mode check and call sites (signIn, signUp), keep the current error handling via setErrors and setNotice, and only call router.replace("/app") on success while moving setSubmitting(false) into finally.app/not-found.tsx-19-26 (1)
19-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid nested interactive elements (
LinkwrappingButton).This structure can produce invalid nested controls and unreliable keyboard/screen-reader behavior. Render a single interactive element using
Button asChild.♿ Proposed fix
- <div className="flex gap-3"> - <Link href="/app"> - <Button className="rounded-2xl">Go to app</Button> - </Link> - <Link href="/"> - <Button variant="outline" className="rounded-2xl"> - Home - </Button> - </Link> - </div> + <div className="flex gap-3"> + <Button asChild className="rounded-2xl"> + <Link href="/app">Go to app</Link> + </Button> + <Button asChild variant="outline" className="rounded-2xl"> + <Link href="/">Home</Link> + </Button> + </div>🤖 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 `@app/not-found.tsx` around lines 19 - 26, The code nests interactive elements (Link wrapping Button) which is invalid; update the two instances so Button is the single interactive element by using Button's asChild prop and moving the anchor/HREF into the Button (e.g., replace <Link href="/app"><Button>...</Button></Link> with <Button asChild className="..."><a href="/app">Go to app</a></Button> and similarly for the "/" Home button), removing the outer Link components and preserving existing classes/variants.
🟡 Minor comments (8)
lib/time/context.ts-10-25 (1)
10-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExtract hour cutoffs to shared limits and validate
hourrange.Inline
12/18violates the limits rule, and these helpers currently accept invalid values (e.g.,-1,99) silently. Move cutoffs into shared limits and guard/clamp input at the boundary.As per coding guidelines: "
**/*.{ts,tsx}: Limits and magic numbers must live inlib/domain/validation(LIMITS)".🤖 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 `@lib/time/context.ts` around lines 10 - 25, Extract the magic cutoffs (12 and 18) into the shared LIMITS in lib/domain/validation (e.g., LIMITS.HOUR_NOON and LIMITS.HOUR_EVENING or use existing HOUR_MIN/HOUR_MAX constants), import LIMITS into lib/time/context.ts, and update getTimeContext, getGreeting, and getTimeLabel to first validate/clamp the incoming hour to the allowed range (e.g., clamp to LIMITS.HOUR_MIN..LIMITS.HOUR_MAX) before using the cutoff constants; replace inline 12/18 with the shared LIMITS constants so all three helpers use the same boundaries..claude/launch.json-5-9 (1)
5-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
npm run devfor theanchor-devlaunch profile.
anchor-devcurrently runsnpm run start, which is a production command and can fail/no-op in local dev unless a build already exists. Switching todevmakes this launch profile reliable for day-to-day use.Suggested diff
- "runtimeArgs": ["run", "start"], + "runtimeArgs": ["run", "dev"],Based on learnings: "Run the gate before calling work done ... and verify user-facing changes in the browser."
🤖 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 @.claude/launch.json around lines 5 - 9, The anchor-dev launch profile currently uses runtimeArgs ["run","start"] (production start) which can fail locally; update the anchor-dev profile to use ["run","dev"] so the launch configuration runs the development command reliably—locate the "anchor-dev" profile entry and change its runtimeArgs from "start" to "dev".README.md-72-86 (1)
72-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language identifier to the project-tree fenced block.
This currently triggers MD040 and will fail strict markdown linting.
Suggested fix
-``` +```text app/ # Next.js App Router routes page.tsx # landing app/page.tsx # dashboard @@ next.config.mjs # conditional static export when BUILD_TARGET=native</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 72 - 86, The README.md project's tree code fence is
missing a language identifier (causing MD040); update the opening fence for the
block containing the app/ ... next.config.mjs lines to include a language id
(e.g., changetotext) so the fenced block reads as a labeled code block;
leave the contents (lines with "app/", "page.tsx", "next.config.mjs", etc.)
unchanged.</details> </blockquote></details> <details> <summary>ROADMAP.md-28-42 (1)</summary><blockquote> `28-42`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Specify a language for the architecture tree code fence.** This triggers MD040 and is easy to resolve. <details> <summary>Suggested fix</summary> ```diff -``` +```text lib/ time/ local-date keys (NOT UTC), time-of-day context. Pure + tested. @@ app/ / (landing), /login, (protected)/ group = gated app routes. ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@ROADMAP.mdaround lines 28 - 42, The fenced architecture tree in ROADMAP.md
is missing a language specifier (causing MD040); update the opening code fence
fromto a fenced block with a language token such astext so the block
reads like ```text and the rest of the content remains unchanged (target the
fenced block containing "lib/ time/ domain/ store/ notifications/ supabase/
auth/ components/ app/").</details> </blockquote></details> <details> <summary>docs/showcase.md-20-24 (1)</summary><blockquote> `20-24`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Normalize blockquote spacing to satisfy MD027.** Lines in the quoted list have extra spaces after `>`. <details> <summary>Suggested fix</summary> ```diff -> • Next.js 16 + React 19 + TypeScript, with a typed domain layer (zod schemas -> as the source of truth) and unit tests on the logic. +> • Next.js 16 + React 19 + TypeScript, with a typed domain layer (zod schemas +> as the source of truth) and unit tests on the logic. > • Offline-first local store with versioned, validated persistence (and a -> storage seam so the same code runs native). +> storage seam so the same code runs native). ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/showcase.md` around lines 20 - 24, The blockquote lines in the markdown use extra spaces after the '>' marker which triggers MD027; edit the quoted list (the lines starting with '> • Next.js 16...', '> as the source of truth)', '> • Offline-first...', '> persistence (and a', '> storage seam...', '> • Supabase auth...') to remove the extra spaces so each '>' is followed by a single space and the content aligns consistently (e.g., change '> ' to '> '), ensuring all quote lines use the same single-space format. ``` </details> </blockquote></details> <details> <summary>lib/domain/entry.ts-35-35 (1)</summary><blockquote> `35-35`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Use `TimeOfDaySchema` for `tomorrowBedtime`.** Line 35 allows arbitrary strings while a canonical time schema already exists, which weakens runtime guarantees at the storage boundary. <details> <summary>Suggested fix</summary> ```diff - tomorrowBedtime: z.string().optional(), + tomorrowBedtime: TimeOfDaySchema.optional(), ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/entry.ts` at line 35, Replace the loose z.string().optional() used for the tomorrowBedtime property with the canonical TimeOfDaySchema to enforce a validated time shape: change tomorrowBedtime: z.string().optional() to tomorrowBedtime: TimeOfDaySchema.optional() and ensure TimeOfDaySchema is imported into this module (e.g., from its defining module) so the schema uses the shared time validator. ``` </details> </blockquote></details> <details> <summary>components/reminder-scheduler.tsx-25-35 (1)</summary><blockquote> `25-35`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Prevent timeout handle accumulation in long-lived sessions.** Fired timeout IDs remain in `timers`, so the collection grows indefinitely while the app stays open. Remove handles after execution (or use a `Set`) before re-arming. <details> <summary>Proposed fix</summary> ```diff - const timers: ReturnType<typeof setTimeout>[] = [] + const timers = new Set<ReturnType<typeof setTimeout>>() @@ - const timer = setTimeout(() => { + const timer = setTimeout(() => { + timers.delete(timer) notify(reminder.title, { body: reminder.body }) schedule() // re-arm for the next day }, delay) - timers.push(timer) + timers.add(timer) @@ - return () => timers.forEach(clearTimeout) + return () => timers.forEach((timer) => clearTimeout(timer)) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/reminder-scheduler.tsx` around lines 25 - 35, The timers array is accumulating fired timeout IDs; update schedule in components/reminder-scheduler.tsx so each timer is removed from the collection when it executes (or switch timers to a Set and delete the id) before re-arming: inside schedule capture the timer id returned from setTimeout, add it to timers (or timers.add), and in the timeout callback remove that id from timers (e.g., timers.splice/timers = timers.filter(id => id !== timer) or timers.delete(timer)) prior to calling schedule() again; also ensure any cleanup/unmount code clears remaining timers using the same collection. ``` </details> </blockquote></details> <details> <summary>app/login/page.tsx-56-58 (1)</summary><blockquote> `56-58`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Handle `status === "loading"` with a non-interactive state.** The form is interactive while auth is still resolving, which can cause a brief incorrect screen and early user actions before redirect settles. Based on learnings `No slop: handle empty/loading/error states; no cosmetic-only features; always read & correct AI-generated code`. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/login/page.tsx` around lines 56 - 58, The page currently returns null only for status === "authed" or "unconfigured" but leaves the login form interactive during status === "loading"; update the render logic around the status variable in page.tsx (the same block that checks status) to explicitly handle status === "loading" by rendering a non-interactive state (e.g., a disabled form or a simple spinner/placeholder) so no inputs/buttons are actionable while auth is resolving, and ensure any components like the login form or submit button are disabled when status === "loading". ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>electron/resources/electron-publisher-custom.js (1)</summary><blockquote> `6-8`: _⚡ Quick win_ **Re-scope the reliability concern: this custom publisher isn’t wired in current builds.** `electron/electron-builder.config.json` sets `publish.provider` to `"github"`, and the only matches for `electron-publisher-custom` in the `electron/` config search are inside `electron/resources/electron-publisher-custom.js`, so `upload(task)` is likely never invoked in the current release flow. If you intend to switch to a `"custom"` publisher later, make `upload()` fail fast instead of only logging `task.file`. <details> <summary>💡 Suggested fix</summary> ```diff class Publisher extends electronPublish.Publisher { async upload(task) { - console.log('electron-publisher-custom', task.file); + throw new Error(`Custom publisher upload is not implemented for ${task.file}`); } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/resources/electron-publisher-custom.js` around lines 6 - 8, The custom publisher's upload handler (async upload(task) in electron-publisher-custom.js) currently only logs task.file and should fail fast; replace the no-op logging with an immediate rejection/throw that includes a clear message mentioning the publisher is not wired in current builds and include task.file (or task.file path) in the error to aid debugging so callers immediately see this isn't supported. ``` </details> </blockquote></details> </blockquote></details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `fce80929-5879-4bcb-a7bd-a87b5d85d1cf` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1d3ce9ccf72397052bbcf3fae486bac3de1dbbff and 56b0a923fe075d9a793bc71a0b1dfd338593f911. </details> <details> <summary>⛔ Files ignored due to path filters (6)</summary> * `electron/assets/appIcon.ico` is excluded by `!**/*.ico` * `electron/assets/appIcon.png` is excluded by `!**/*.png` * `electron/assets/splash.gif` is excluded by `!**/*.gif` * `electron/assets/splash.png` is excluded by `!**/*.png` * `electron/package-lock.json` is excluded by `!**/package-lock.json` * `package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (81)</summary> * `.claude/launch.json` * `.claude/settings.json` * `.env.example` * `.github/workflows/ci.yml` * `.gitignore` * `CLAUDE.md` * `README.md` * `ROADMAP.md` * `app/(protected)/app/page.tsx` * `app/(protected)/evening/page.tsx` * `app/(protected)/layout.tsx` * `app/(protected)/morning/page.tsx` * `app/(protected)/settings/page.tsx` * `app/(protected)/timeline/page.tsx` * `app/error.tsx` * `app/global-error.tsx` * `app/layout.tsx` * `app/login/page.tsx` * `app/not-found.tsx` * `app/page.tsx` * `app/settings/page.tsx` * `capacitor.config.ts` * `components/auth-provider.tsx` * `components/evening/step-evening-mood.tsx` * `components/evening/step-journal.tsx` * `components/habit-icon.tsx` * `components/morning/step-affirmation.tsx` * `components/morning/step-intention.tsx` * `components/morning/step-mood.tsx` * `components/morning/step-sleep.tsx` * `components/reminder-scheduler.tsx` * `components/timeline-view.tsx` * `components/ui/dialog.tsx` * `components/ui/separator.tsx` * `components/ui/sheet.tsx` * `components/ui/slider.tsx` * `components/ui/tabs.tsx` * `docs/showcase.md` * `electron/.gitignore` * `electron/capacitor.config.ts` * `electron/electron-builder.config.json` * `electron/live-runner.js` * `electron/package.json` * `electron/resources/electron-publisher-custom.js` * `electron/src/index.ts` * `electron/src/preload.ts` * `electron/src/rt/electron-plugins.js` * `electron/src/rt/electron-rt.ts` * `electron/src/setup.ts` * `electron/tsconfig.json` * `eslint.config.mjs` * `hooks/use-store.ts` * `lib/auth/credentials.test.ts` * `lib/auth/credentials.ts` * `lib/domain/entry.ts` * `lib/domain/habit.ts` * `lib/domain/selectors.test.ts` * `lib/domain/selectors.ts` * `lib/domain/validation.test.ts` * `lib/domain/validation.ts` * `lib/notifications/index.ts` * `lib/notifications/port.ts` * `lib/notifications/schedule.test.ts` * `lib/notifications/schedule.ts` * `lib/store.ts` * `lib/store/actions.test.ts` * `lib/store/actions.ts` * `lib/store/index.ts` * `lib/store/persistence.test.ts` * `lib/store/persistence.ts` * `lib/store/state.test.ts` * `lib/store/state.ts` * `lib/store/store.ts` * `lib/supabase/client.ts` * `lib/time/context.ts` * `lib/time/today.test.ts` * `lib/time/today.ts` * `next.config.mjs` * `package.json` * `tsconfig.json` * `vitest.config.ts` </details> <details> <summary>💤 Files with no reviewable changes (2)</summary> * lib/store.ts * app/settings/page.tsx </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- tabs: pass orientation prop to TabsPrimitive.Root for keyboard/ARIA - timeline-view: sort entries by localeCompare instead of new Date - electron-builder: replace scaffold bundle identifiers with real values - electron/package.json: populate repository URL for GitHub publishing - live-runner: close watcher before replacing, fail fast on build errors - setup.ts: fix backgroundColor config path mismatch - index.ts: gate autoUpdater to packaged builds only
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(protected)/settings/page.tsx (1)
71-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit empty state for habits.
When
state.habitsis empty, the card body becomes blank. Add a small empty-state message so this tab always has a clear state.Proposed fix
- {state.habits.map((habit) => ( - <div key={habit.id} className="flex items-center justify-between px-4 py-3 rounded-xl border border-border"> - <span className="flex items-center gap-3 text-sm text-foreground"> - <HabitIcon icon={habit.icon} className="size-4 text-muted-foreground" /> - {habit.name} - </span> - <button - onClick={() => removeHabit(habit.id)} - className="text-muted-foreground hover:text-destructive transition-colors" - aria-label={`Remove ${habit.name}`} - > - <Trash2 className="size-4" /> - </button> - </div> - ))} + {state.habits.length === 0 ? ( + <p className="px-1 py-2 text-sm text-muted-foreground">No habits yet. Add your first one below.</p> + ) : ( + state.habits.map((habit) => ( + <div key={habit.id} className="flex items-center justify-between px-4 py-3 rounded-xl border border-border"> + <span className="flex items-center gap-3 text-sm text-foreground"> + <HabitIcon icon={habit.icon} className="size-4 text-muted-foreground" /> + {habit.name} + </span> + <button + onClick={() => removeHabit(habit.id)} + className="text-muted-foreground hover:text-destructive transition-colors" + aria-label={`Remove ${habit.name}`} + > + <Trash2 className="size-4" /> + </button> + </div> + )) + )}As per coding guidelines
**/*.{ts,tsx}: Handle all states explicitly: empty states, loading states, and error states; avoid cosmetic-only features.🤖 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 `@app/`(protected)/settings/page.tsx around lines 71 - 85, The habits list currently renders nothing when state.habits is empty; update the JSX around the state.habits mapping in page.tsx to explicitly handle the empty case by rendering a small empty-state card (matching the existing card styles used for each habit) when state.habits.length === 0; keep the existing mapping using state.habits.map(...) for non-empty lists, and ensure the empty card contains a concise message like "No habits yet" (and optional action hint), uses the same ARIA/accessibility patterns as the habit rows (e.g., role or aria-live if appropriate), and does not change removeHabit, HabitIcon, or Trash2 behavior for populated states.
🧹 Nitpick comments (2)
package.json (2)
1-65: 💤 Low valueConsider adding an
enginesfield for Node.js version consistency.The project includes a
.node-versionfile but lacks a correspondingenginesfield inpackage.json. Adding this field enforces Node.js version requirements at install time when used withengine-strict=truein.npmrc, preventing version-drift issues across CI and local development environments.This is especially valuable for native builds (Capacitor, Electron) which can be sensitive to Node.js version differences.
📦 Suggested addition
{ "name": "anchor", "version": "0.0.1", "type": "module", "private": true, + "engines": { + "node": ">=20.0.0" + }, "scripts": {🤖 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 `@package.json` around lines 1 - 65, Add a top-level "engines" field to package.json to pin the Node.js version (matching the existing .node-version) so installs and CI respect the required Node version; update package.json (near "name"/"version") to include "engines": {"node":"<version_or_range>"} using the same version string as .node-version to ensure commands like "build:native", "cap:sync", and Electron/Capacitor native flows run under the expected Node runtime.
26-31: 💤 Low valueConsider tighter version constraints for native platform dependencies.
The Capacitor packages currently use caret ranges (
^8.x.x), which permit minor version updates. Native platform APIs (iOS, local notifications, status bar) can introduce breaking behavioral changes in minor releases, potentially causing runtime issues that are difficult to debug.Consider using tilde ranges (
~8.3.4) for patch-only updates, or exact versions for maximum stability, especially for the marked iOS and notification packages.🤖 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 `@package.json` around lines 26 - 31, The package.json dependency ranges for Capacitor native packages ("`@capacitor/ios`", "`@capacitor/local-notifications`", "`@capacitor/status-bar`", "`@capacitor/app`", "`@capacitor/core`", "`@capacitor/haptics`") are using caret ranges (^8.x.x); tighten them to patch-only or exact versions to avoid unintended minor-version behavior changes—update the versions to tilde ranges (e.g., ~8.4.0) or pin exact versions (e.g., 8.4.0) for the listed package names to ensure stability.
🤖 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 `@components/reminder-scheduler.tsx`:
- Around line 43-45: The native scheduling branch currently calls
scheduleDailyReminders(reminders) fire-and-forget via void, which can cause
unhandled promise rejections if scheduling fails; update the
usesNativeScheduler() branch to explicitly handle the promise returned by
scheduleDailyReminders — either await it inside an async function with try/catch
or attach a .catch handler that logs or surfaces the error (use the existing
logger or error handler in this module) so failures from
scheduleDailyReminders(reminders) are handled safely.
In `@lib/notifications/port.ts`:
- Around line 34-60: The module must bootstrap the cached nativePermission so
getPermission() doesn't return the stale initial "default" on cold start; add an
initialization step that immediately (at module load) calls the Capacitor native
permissions check (e.g., LocalNotifications.checkPermissions() / similar) and
pass the result through rememberNativePermission to update nativePermission;
ensure the same mapping logic used in requestPermission() is applied so
nativeNotificationAdapter.getPermission() returns the real native state from the
start.
---
Outside diff comments:
In `@app/`(protected)/settings/page.tsx:
- Around line 71-85: The habits list currently renders nothing when state.habits
is empty; update the JSX around the state.habits mapping in page.tsx to
explicitly handle the empty case by rendering a small empty-state card (matching
the existing card styles used for each habit) when state.habits.length === 0;
keep the existing mapping using state.habits.map(...) for non-empty lists, and
ensure the empty card contains a concise message like "No habits yet" (and
optional action hint), uses the same ARIA/accessibility patterns as the habit
rows (e.g., role or aria-live if appropriate), and does not change removeHabit,
HabitIcon, or Trash2 behavior for populated states.
---
Nitpick comments:
In `@package.json`:
- Around line 1-65: Add a top-level "engines" field to package.json to pin the
Node.js version (matching the existing .node-version) so installs and CI respect
the required Node version; update package.json (near "name"/"version") to
include "engines": {"node":"<version_or_range>"} using the same version string
as .node-version to ensure commands like "build:native", "cap:sync", and
Electron/Capacitor native flows run under the expected Node runtime.
- Around line 26-31: The package.json dependency ranges for Capacitor native
packages ("`@capacitor/ios`", "`@capacitor/local-notifications`",
"`@capacitor/status-bar`", "`@capacitor/app`", "`@capacitor/core`",
"`@capacitor/haptics`") are using caret ranges (^8.x.x); tighten them to
patch-only or exact versions to avoid unintended minor-version behavior
changes—update the versions to tilde ranges (e.g., ~8.4.0) or pin exact versions
(e.g., 8.4.0) for the listed package names to ensure stability.
🪄 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
Run ID: 58983480-1c19-48b1-a8a4-d53717315e67
⛔ Files ignored due to path filters (5)
ios/App/App/Assets.xcassets/AppIcon.appiconset/AppIcon-512@2x.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-1.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-2.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
.gitignoreapp/(protected)/settings/page.tsxapp/apple-icon.tsxapp/manifest.tscomponents/reminder-scheduler.tsxios/.gitignoreios/App/App.xcodeproj/project.pbxprojios/App/App.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistios/App/App.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedios/App/App/AppDelegate.swiftios/App/App/Assets.xcassets/AppIcon.appiconset/Contents.jsonios/App/App/Assets.xcassets/Contents.jsonios/App/App/Assets.xcassets/Splash.imageset/Contents.jsonios/App/App/Base.lproj/LaunchScreen.storyboardios/App/App/Base.lproj/Main.storyboardios/App/App/Info.plistios/App/App/PrivacyInfo.xcprivacyios/App/CapApp-SPM/.gitignoreios/App/CapApp-SPM/Package.swiftios/App/CapApp-SPM/README.mdios/App/CapApp-SPM/Sources/CapApp-SPM/CapApp-SPM.swiftios/debug.xcconfiglib/notifications/index.tslib/notifications/port.tspackage.json
💤 Files with no reviewable changes (1)
- .gitignore
✅ Files skipped from review due to trivial changes (12)
- ios/debug.xcconfig
- ios/App/CapApp-SPM/Sources/CapApp-SPM/CapApp-SPM.swift
- app/manifest.ts
- ios/App/CapApp-SPM/README.md
- ios/App/App/Assets.xcassets/Contents.json
- ios/.gitignore
- ios/App/CapApp-SPM/.gitignore
- ios/App/App/PrivacyInfo.xcprivacy
- ios/App/App/Assets.xcassets/AppIcon.appiconset/Contents.json
- ios/App/App/Assets.xcassets/Splash.imageset/Contents.json
- ios/App/App.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
- ios/App/CapApp-SPM/Package.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/notifications/index.ts
- Set nodeIntegration to false in Electron setup (security) - Add .catch() to native scheduling call in reminder-scheduler - Bootstrap native notification permission on cold start in port.ts
- Harden Electron URL allowlisting (parse URLs, validate newURL in will-navigate) - Add DayKeySchema and use TimeOfDaySchema for notification fields - Prevent updateTodayEntry date desync from patch - Fix nested Link/Button patterns (error, not-found pages) - Wrap login auth calls in try/catch/finally so submitting is always cleared - Use Set for reminder timers to prevent accumulation All 51 tests pass, tsc clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns the prototype into a real, layered app. Built and verified in-browser; gate is green (typecheck/lint/test/build), 39 unit tests.
Highlights
lib/—time(local day keys, no UTC bug),domain(zod schemas as source of truth + selectors + validation),store(versionedmigrate,StoragePortseam for Capacitor, mutation only via actions).4).data-[orientation]/data-[state]— the legacy forms silently did nothing)./, clickable dashboard, brandederror/404/global-error.NotificationPort+ scheduler + settings UI (honest web caveat, native seam).(protected)route-group gate,/login, sign-out; graceful local-only mode without env.Follow-ups (see ROADMAP.md)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores