chore(#107): code-health umbrella — all frontend strands (a11y/perf/quality/architecture/validation/keyboard/i18n) - #172
Conversation
…ing parity, orphan cores, motion, form a11y
…, API-key expiry parity, error states, profile a11y, RNGH swipe
…reakdown parity, status-text tokens, goals/cal a11y
…r codes, dead code, @next/env, offline copy, emoji/checklist keyboard
…line keys in both locales
…atus-text + fg-on-bad tokens, control names, focus rings
…ightText→shared, dead exports, drill error, focus rings, path constants, register roots
…omments, transform shimmer, modal back, split sub-hook tests
…le roles, status-tint ceiling family deferral
…, strict contrast improvement within deferred family)
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
One hard-rule violation to fix before merge.
Finding: apps/web/lib/auth-proxy.ts:50-52 — logAuthRouteFailure introduces a console.error in production code, violating CLAUDE.md rule 4 ("don't log when no project logger exists"). The old empty-catch blocks silently swallowed errors (a rule 8 violation); the right fix is to remove the log rather than add one. Each catch already returns a structured 500 with the requestId, which is sufficient for client-side observability. Remove logAuthRouteFailure and its four call-sites in the auth route handlers, reverting each catch (error) binding to a bare catch.
Everything else in this 401-file sweep is clean: i18n parity is 1628=1628, web/mobile parity is maintained throughout, types are sound in production code (any in test files is explicitly allowed by the ESLint config), no dead exports, no useless comments introduced, and the new test coverage is substantive.
…ective/preferences/profile) for SonarCloud new-code gate
There was a problem hiding this comment.
Prior finding still unresolved. logAuthRouteFailure (apps/web/lib/auth-proxy.ts:50-52) introduces a console.error in production code, violating CLAUDE.md rule 4. The two new commits (b94ff06 test coverage, b04bece test type fix) are clean. Fix: delete logAuthRouteFailure and its call-sites in the auth route handlers — each catch already returns a structured 500 with the requestId, which is sufficient for observability without a console log.
There was a problem hiding this comment.
Prior finding still unresolved. logAuthRouteFailure (apps/web/lib/auth-proxy.ts:50-52) introduces a console.error in production code, violating CLAUDE.md rule 4. The two new commits (b94ff06 test coverage, b04bece test type fix) are clean. Fix: delete logAuthRouteFailure and its call-sites in the auth route handlers — each catch already returns a structured 500 with the requestId, which is sufficient for observability without a console log.
There was a problem hiding this comment.
Prior finding still unresolved: logAuthRouteFailure at apps/web/lib/auth-proxy.ts:50-52 introduces console.error in production code, violating CLAUDE.md rule 4. The two new commits (b94ff06 test coverage, b04bece test type fix) are otherwise clean. Fix: delete logAuthRouteFailure and its four call-sites in the auth route handlers.
logAuthRouteFailure wrapped a console.error, which rule 4 prohibits (no console in production code; no project logger exists). Each auth route catch already returns a structured 500 with requestId, so the log carried nothing the response doesn't. Remove the helper, its four call-sites, and the three console.error spy assertions (which tested an implementation detail, not behavior). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
Solid code-health umbrella. Zero rule violations across all 410 files reviewed. Highlights: the auth store logout now goes through apiClient (dropping the old direct fetch with a hardcoded base-URL fallback — two rule-5 violations fixed in one); all empty catch blocks properly surface errors to state rather than swallowing them; any usage is confined to test helpers; no console statements in production code; i18n parity held at 1628 keys each locale. The function splits, accessibility token migration, overlay-back stack, and cross-platform validation parity are all clean and complete. Ready to merge.
There was a problem hiding this comment.
Solid code-health umbrella. Zero rule violations across all 410 files reviewed. Highlights: the auth store logout now goes through apiClient (dropping the old direct fetch with a hardcoded base-URL fallback); all empty catch blocks properly surface errors to state rather than swallowing them; any usage is confined to test helpers; no console statements in production code; i18n parity held at 1628 keys each locale. The function splits, accessibility token migration, overlay-back stack, and cross-platform validation parity are all clean and complete. Ready to merge.
There was a problem hiding this comment.
Solid code-health umbrella. Zero rule violations across 410 files. The auth store logout now uses apiClient (dropping a direct fetch with hardcoded base-URL fallback). All catch blocks surface errors to state. No any in production, no console statements, i18n parity held at 1628 keys per locale. Function splits, a11y token migration, overlay-back stack, and cross-platform validation parity all clean. Ready to merge.
…#174) * fix(mobile): lazy Supabase client to stop grey-screen crash on launch #172 removed the public Supabase URL/key fallbacks in lib/supabase.ts and added a top-level `throw new Error('Supabase config missing')`. The build sets no EXPO_PUBLIC_SUPABASE_* vars, so the module threw at import-time; expo-router then failed to load the route importing it (via lib/google-auth.ts) and crashed with "Cannot read property 'ErrorBoundary' of undefined" — a grey crash-loop on every launch, regardless of auth state. Mirror apps/web/lib/supabase.ts: create the client lazily via getSupabaseClient() so a missing/optional config can never crash the app shell at module-eval. Restore the public project URL + sb_publishable key as in-code fallbacks (a publishable key + EXPO_PUBLIC_ vars ship in the client bundle anyway — not secrets). Google OAuth call sites in google-auth.ts use the getter. Verified on device: login screen renders, no Supabase/ErrorBoundary errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mobile): update google-auth mock for getSupabaseClient The lazy-client refactor renamed the supabase export; the google-auth test mock still returned `supabase`, so the suite failed with "No getSupabaseClient export is defined on the @/lib/supabase mock". Mock getSupabaseClient() instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bmit gate (#175) * fix(ui): UX/QA batch across web + mobile (26 items) + create-habit submit gate Cross-platform UX/QA pass plus shared error localization. Highlights: - Chat: voice-recording race fix; language picker moved to the chat header - Calendar: tappable centered month nav, day-detail modal, centered muted legend, removed the "x/30 dias" summary - Errors: backend errors render localized via ERROR_CODE_TO_KEY; login surfaces errors through a toast only (no reference code / inline yellow text) - Retrospective: stats-dashboard + narrative UI with an empty-period state - Upgrade: exact hero copy + Free/Pro two-column comparison - Frequency: swipe carousel; streak-freeze celebration auto-dismisses; bad = red / overdue = yellow log dots on both platforms - Preferences option legends relocated into the picker modal; AI-facts paginator inline with an icon select - Support: removed name/email fields; advanced devs: deduped MCP tab - Widget: fallback drawables + first-add render fix - pt-BR naturalization across the locale (parity preserved at 1665/1665) - fix(mobile): restore the title-presence submit gate on the create/edit habit modals. #172 reverted #145: with a buffered title input, setValue without shouldValidate re-broadcasts the cached onSubmit-mode isValid, so the button stayed disabled with a title filled. Gate on useWatch('title') instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(i18n): remove dead orbitMcp.claudeDesktop key The desktop MCP tab was dropped (MCP_CONFIG_TABS no longer includes 'desktop'), so this key has no remaining callers. Flagged by the PR review bot. Removed from both locales; en/pt-BR parity preserved at 1664/1664. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(harness): three memory-derived guardrail gates Convert three operational memories into deterministic dual-target gates (pure _lib rule -> Claude Code hook + .opencode plugin + test-hooks), so the knowledge is enforced structurally instead of living only in recallable memory. - mobile supabase-lazy (checkMobileSupabaseLazy, rules-source.mjs): blocks a module-scope throw or top-level createClient() in apps/mobile/**/supabase.ts; eager module-eval crashes to a grey screen at launch (#172/#174). - EF-migration IF-NOT-EXISTS (checkEfMigrationRawIndex, rules-source.mjs): blocks a raw migrationBuilder.Sql CREATE INDEX / DROP INDEX lacking IF [NOT] EXISTS in orbit-api Migrations; EF runs migrations at startup on Render and a duplicate raw index throws Postgres 42P07, failing the deploy. Leaves CreateIndex() alone. - worktree junction guard (checkGitWorktreeRemove, rules-git.mjs): blocks the forced form of git worktree remove; on Windows it follows a junction and deletes the link TARGET. Strips heredoc bodies so a message naming the flag is not a false positive. Wired into the existing git-guardrails PreToolUse. Gate: node .claude/hooks/test-hooks.mjs is green (existing + new assertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(harness): close false-negative gaps in the three guardrail gates Addresses the #556 review — each finding was a verified false negative in a gate whose purpose is preventing a specific prior incident from recurring. - supabase-lazy (High): the `throw` check anchored to column 0, missing an indented / `if (!x) throw` guard-clause form; the createClient regex rejected a typed const `export const supabase: SupabaseClient = createClient(...)` — the exact style the real apps/mobile/lib/supabase.ts already uses. Now a string/comment-aware bracket-depth scan flags any module-scope throw, and the regex allows an optional type annotation. The lazy `() => createClient` arrow still passes (a new test pins that no false positive was introduced). - ef-migration-idempotency (High): IF [NOT] EXISTS was tested once against the whole Sql() blob, so one idempotent statement masked a sibling raw index in a batched call. Now each `;`-separated statement is checked independently. - worktree-junction (Medium x2): force detection is now segment-scoped like checkGitCommand, so `git worktree remove path && npm test -- --force` no longer false-blocks; and the block message inlines the SAFE cleanup order instead of pointing at a CLAUDE.md section that does not exist. Gate: node .claude/hooks/test-hooks.mjs green (existing + 6 new regression assertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What
Consolidated code-health PR for the #107 umbrella — all frontend strands in one PR (web + mobile + shared), driven by a recursive audit → fix → re-sweep loop until every audit domain reports zero non-deferred findings.
Closes #107
Paired backend PR: thomasluizon/orbit-api#205 — squash-merge it first so the additive
errorCodeAPI contract is live before this consumer side.Highlights
packages/sharednow enforceslocal/no-comments=error(284 narration comments autostripped); rule-5 documented in the three workspace CLAUDE.md files; 24 stale CLAUDE.md references fixed.titleFilledmobile gate is gone.useChatComposer(both platforms),ChatPage,CalendarSyncScreen, the profile/ai-settings/preferences/retrospective/login/support/streak/achievements pages.-text/fg-3/fg-on-badtokens;accessibilityRoleadded to every mobile interactive element; hit targets ≥44; overlay focus restore; control names; focus rings; reduced-motion gating.useOverlayEscapeweb /useOverlayBackmobile) so ESC and Android-back resolve the top-most layer LIFO across date-picker/emoji/description-viewer/tour/level-up/push-prompt;ErrorBoundaryon mobile root + web(chat); dnd KeyboardSensor.tag-selection-core,dismiss-guard-core) wired into both platforms;highlightTextlifted to shared.getErrorMessage/auth error mapping now resolve the backenderrorCodefirst (paired with the API change).underscoreDoS pinned out,@next/envremoved.Validation
npm run lint+npm run type-check+npm testall green (981 shared / 548 mobile / 1523 web). i18n parity 1628=1628.Deferrals
Each over-the-bar item carries a written justification (function-size orchestration roots/adapters/non-functions; a11y token-ceiling residuals where WCAG 1.4.1 is met and 1.4.3 needs a DESIGN.md token decision; a few pre-existing test-quality nits). Full register summarized on issue #107.
Not run (per request)
Lighthouse / runtime perf were not run (no chrome-devtools). Worth an eyeball in the running app: the Today/Calendar RNGH swipe and the transform-based loading shimmer.
🤖 Generated with Claude Code