feat(auth): multi-user — persistent sessions, password/SSO signup, fail-closed isolation - #252
Merged
Conversation
Foundation for multi-user: move browser auth to @supabase/ssr so sessions persist across refreshes in cookies shared with the server. - client.tsx: createBrowserClient (cookie-persisted, PKCE); route magic-link/OAuth/confirmation through /auth/callback; add signInWithPassword / signUpWithPassword / signInWithOAuth(google|azure). - proxy.ts (Next 16 renamed middleware -> proxy): refresh the session cookie on navigation + API calls; no-op unless an sb- cookie is present (demo mode untouched, no extra auth round-trip). - app/auth/callback/route.ts: exchangeCodeForSession for the PKCE/OAuth return. - server.ts: cookie-aware (RLS-subject) server client. - auth.ts: requireAuthenticatedUser now also accepts the @supabase/ssr cookie session; Bearer still resolves first, so all 26 routes + programmatic callers are unchanged. Additive + backward-compatible; demo/local-no-auth behaviour unchanged. Typecheck + verify:cheap + format green. The real cookie/PKCE/OAuth flow needs live Supabase + a browser (staging) to fully verify. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend AuthPanel with a Sign in / Create account toggle, a Magic link / Password method switch, and Google/Microsoft SSO buttons — wired to the useAuthSession methods added in Phase 2. Magic link stays the default. No test references the panel markup; demo mode still shows the "sign-in unavailable" state when the browser Supabase env is absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hybrid retrieval RPCs treat a null owner_filter as "all owners" (fail-open), so a missing ownerId would silently return another tenant's data (see the owner-scoping isolation audit). Add requireOwnerScope() and apply it at every retrieval RPC boundary (rag.ts x8, document-enrichment, deep-memory): a real multi-user deployment now throws on a missing ownerId instead of leaking, while demo / local-no-auth / the test runner stay permissive (single-tenant). This also closes the shared-cache question: unscoped retrieval that would write a null-owner rag_response_cache entry now throws in production, so private content cannot land in the shared bucket. Adds tests/owner-scope.test.ts and the regenerated docs/site-map.md (/auth/callback). verify:cheap (986 tests) + format green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Actionable checklist for enabling open signup + confirm-email, Email/password + Google/Microsoft providers, production SMTP, Site/Redirect URLs (incl. /auth/callback), CAPTCHA, env keys, and staging verification. Live config is applied by the user; do not enable signup until isolation hardening merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… live) Verified against the live project: every owner-scoped user-data table already has RLS + an authenticated owner-read policy (owner_id = (select auth.uid())), registry + internal tables are service-role-only, and both storage buckets are private with server-minted signed URLs. So no broad RLS migration is needed for isolation — the backstop exists and, with the fail-closed app-layer guard, isolation is enforced at both layers. Residual: rag_visual_eval_cases is RLS-off (no client grant -> safe) and untracked in schema.sql (separate drift). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cklist Retrieved read-only from the live project via the Supabase MCP so the env section and OAuth callback are copy-paste ready. Security advisors returned clean (no missing-RLS findings), reconfirming the DB backstop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restores the react-hooks/set-state-in-effect fix (dropped from history by a rebase against main) — the synchronous setError in useEffect is moved into the async initializeSession and only shown when no session was restored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BigSimmo
enabled auto-merge (squash)
July 3, 2026 15:07
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:5f05d360df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+29
to
+32
| setAll(cookiesToSet) { | ||
| for (const { name, value } of cookiesToSet) request.cookies.set(name, value); | ||
| response = NextResponse.next({ request }); | ||
| for (const { name, value, options } of cookiesToSet) response.cookies.set(name, value, options); |
There was a problem hiding this comment.
When @supabase/ssr refreshes or clears auth cookies in the proxy, it calls setAll with a second headers argument containing no-store cache headers. This handler drops that argument, so authenticated responses that also set refreshed Supabase cookies can be returned without the cache guard; behind a CDN/ISR deployment, such a response can be cached and replayed with another user's Set-Cookie session. Please accept the headers parameter and copy those headers onto the rebuilt response.
Useful? React with 👍 / 👎.
Comment on lines
+17
to
+20
| if (isDemoMode() || isLocalNoAuthMode() || process.env.NODE_ENV === "test") { | ||
| return undefined; | ||
| } | ||
| throw new Error( |
There was a problem hiding this comment.
In a real local/staging environment this throws for every missing ownerId, even for trusted global retrieval callers: npm run verify:release ends with eval:quality:release, and scripts/eval-quality.ts calls searchChunksWithTelemetry without an owner, so release/eval runs against a configured Supabase project now fail before any RPC. Either thread the existing allowGlobalSearch contract through this guard or update those scripts to provide an owner scope.
Useful? React with 👍 / 👎.
Comment on lines
+33
to
+35
| const { error } = await supabase.auth.exchangeCodeForSession(code); | ||
| if (error) { | ||
| return failure(error.message); |
There was a problem hiding this comment.
For magic-link or signup-confirmation emails opened in a different browser/profile/device than the one that requested the email, this code-only callback has no sb-...-code-verifier cookie, so exchangeCodeForSession(code) fails and the user is redirected back with an auth error. The previous hash-token flow did not require same-browser state; to keep email auth reliable, add a token-hash/verifyOtp confirmation path or make the configured email templates explicitly produce links this route can verify without the verifier cookie.
Useful? React with 👍 / 👎.
Comment on lines
+196
to
+200
| const { data, error: signUpError } = await active.auth.signUp({ | ||
| email, | ||
| password, | ||
| options: { emailRedirectTo: authCallbackRedirect() }, | ||
| }); |
There was a problem hiding this comment.
If the operator follows the new checklist and enables Supabase Auth CAPTCHA for open signup, this signup request has no CAPTCHA widget/token and sends only emailRedirectTo, so Supabase will reject account creation instead of letting new users onboard. Add the CAPTCHA frontend token to the auth options (and the same pattern for other protected auth forms) or remove the dashboard CAPTCHA instruction until the UI supplies it.
Useful? React with 👍 / 👎.
cursorBot
pushed a commit
that referenced
this pull request
Aug 5, 2026
* fix(gates): catch lint and type errors before push, not in CI Two open PRs burned full CI cycles this week on defects a single local command would have caught: #1606 on a react-hooks/set-state-in-effect lint error, #1618 on a TS2339 for `mode.devOnly` (a union member that lacks the property, where app-modes.ts already exports the correct `"devOnly" in mode` guard). Neither lint nor typecheck was in the pre-push path. Typecheck could not simply be added, because it was already unusable (outstanding-issues #210). tsconfig.json's `include` carries `.next/types/**/*.ts` and `.next/dev/types/**/*.ts` — gitignored build artifacts — so deleting a page leaves the stale generated validator importing a removed module. Reproduced rather than inferred: a planted `.next/dev/types/validator.ts` referencing a removed mockup page yields `error TS2307: Cannot find module .../mockups/deleted-mockup-route/page.js`, base config exit 2, source-only config exit 0. Full source typecheck is clean (71s cold, 8.8s warm). Red locally and green in CI is how the gate got abandoned, which is how the real type error then reached CI. - tsconfig.typecheck.json + `typecheck:source`: identical compiler options, minus the `.next` globs, with a separate tsbuildinfo so the two incremental caches cannot invalidate each other. Route-signature validation is not lost; `next build` still covers it in CI. - guard-push.mjs gains a fourth guard running eslint over the pushed files and this typecheck. Verified to reproduce both defects above with CI-identical messages. Scoped to the lint roots and to pushes that touch TS, skips loudly when node_modules is absent rather than pushing people to GUARD_PUSH_DISABLE=1, and overridable with SKIP_STATIC_GUARD=1. Also corrects a doc claim that made #1580 surprising: "mockups are exempt" was being read as blanket. Mockups are exempt from the wiring and reachability gates and nothing else — they are still typechecked, and their client chunks still count toward check:bundle-budget, which totals every built chunk rather than the initial production bundle. That the budget's scope contradicts ledger #13's "not an initial production bundle" position is a real unmade decision, now recorded as #237 rather than papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * docs(issues): capture the phone Category soft-menu fix salvaged from PR #1606#1606 is closed, but it carried the one fix nothing else in the queue provides: MobileResultFilterControl's native <select> paints a harsh system-blue highlight on phones, and #1615 keeps that native select (its change is the iOS 16px anti-zoom rule). So the fix does not survive #1615 landing. Records it as #238 with the two defects the redo must not repeat: the unresolved keyboard trap on disabled options, and the set-state-in-effect lint error that PR #1620's new pre-push guard would now catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * issues: capture #239 stale Cloud acceptance pin on PR #1617, #240 remote-container browser gate drift * Tighten guard coordinator test Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): shared source-typecheck lease and safer static pre-push Treat typecheck:source:internal as a shared read-only coordinator lease with a distinct per-worktree buildinfo file, drop the pinned in-repo cache path, and harden staticGuard: acquire a short exclusive lease (fail-open when busy), use a private eslint cache, escalate lint on eslint policy changes, fail closed when the push tip is not HEAD, cover eslint-rules, and add Vitest coverage. Align hook/docs wording with the fourth guard and point CLAUDE.md at #252. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): keep tsBuildInfoFile when run-heavy has no npm_execpath Pre-push invokes run-heavy via plain node, so the npm_execpath spawn path was skipped and the fallback dropped effectiveForwarded — undoing the per-worktree buildinfo injection. Also warn when staticGuard passes on a dirty working tree. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: refresh scripts-index for lint:changed:internal Keep docs:check-inventory green after adding the pre-push eslint wrapper script to package.json. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): address Devin findings on static pre-push guard - Treat "Database focused-test capacity is full" as coordinator busy so shared typecheck slot exhaustion fails open instead of faking a type error. - Skip source typecheck when every changed .ts path is excluded by tsconfig.typecheck.json (edge functions, archive, scratch, worktrees). - Restore check-github-shell-access.mjs (and its Role notes) in the scripts index. * chore(ledger): record PR #1620 babysit * fix(gates): emit structured heavy-run admission-busy signal Prefer exit 75 + DATABASE_HEAVY_RUN_ADMISSION_BUSY over prose matching so tsc/eslint output that quotes busy strings cannot false-pass the static guard. * fix(gates): tip-check only when static work runs; isolate typecheck cache Addresses follow-up Devin on PR #1620: - Reorder staticGuard so tip-vs-HEAD fails closed only when lint/typecheck will actually read the working tree; ignore tag refs in the tip check. - Pin a distinct tsBuildInfoFile on tsconfig.typecheck.json so direct tsc does not collide with the base config cache (run-heavy still overrides). * fix(gates): keep lint failures when typecheck admission is busy Addresses Devin on PR #1620 — a prior eslint failure must still block the push if the follow-up source typecheck cannot get a coordinator slot. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 9, 2026
check:bundle-budget totals every built chunk, mockups included, and main sits at ~+9.4% against a 10% tolerance. The study's two scratch chunks (~9.8 KiB gzip) alone took the repo to +10.1% and failed Build — the same failure PR #1580 hit, at the same number. Measured on this branch: main alone 308 chunks / 1538.9 KiB (+9.42%, passing); with the study 310 / 1548.7 KiB (+10.1%, failing); without it 309 / 1542.4 KiB (+9.67%, passing). The implementation itself adds no new route chunk — it edits existing components. Direction 02 has shipped, so the runnable route had already served its purpose. A design-scratch route that 404s in production is the wrong thing to spend the last of that headroom on, and raising the baseline would have settled the open #13/#252 question — whether scratch should count toward this budget at all — by default, in the direction of "raise the ceiling". mockups/README.md keeps the three directions and says why the route went, so the alternatives stay recoverable from history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBAt8pJVz2TxMEUJeWEdUy
cursorBot
pushed a commit
that referenced
this pull request
Aug 9, 2026
…reset Relative keyboard moves read pageRef so key-repeat advances before React re-renders. Budget retained canvases against the largest measured page cost. Reset DocumentImageList on collectionKey. Soften #252 tip-only bundle wording; tighten #294 OffscreenCanvas close criteria. Drop the PR-added legacy shadow-tight on the empty page slot to keep the DS ratchet. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit
that referenced
this pull request
Aug 9, 2026
…d reading mode, and the first canvas gate (#1772) * test(viewer): gate the PDF canvas raster in a real browser The viewer's raster surface had no browser proof: unit tests cover the raster budget, DOM tests cover gestures, and a static contract covers the lazy boundary, but none of them can see whether a clinical source page actually paints. A blank canvas still reports correct dimensions, a correct aria-label, and a resolved render promise. tests/ui-document-canvas.spec.ts reads the raster back — ink pixels on page 1, the real page count in the one toolbar readout, and a page flip whose FNV pixel signature differs from page 1's. It also attaches an advisory page-flip cost measurement (long tasks + time to paint) as the input the OffscreenCanvas decision is conditioned on. pdfjs-dist@6 calls Map.prototype.getOrInsertComputed, which ships in Chromium 151 but not in the 141 build some sandboxed containers pre-bake and pin via PLAYWRIGHT_BROWSERS_PATH. The skip guard is therefore asymmetric: without CI it skips with a reason naming the browser version; with CI set a missing engine feature FAILS, because a gate that can skip itself green on the machine that gates the merge is worse than no gate. Both directions were verified locally. Spec collection is three hand-maintained lists that must agree, so all three are updated together and tests/playwright-project-isolation.test.ts gains a fail-closed assertion for this basename — "did not run" and "ran and skipped" are indistinguishable in a log otherwise. Closes#279 in docs/outstanding-issues.md. Its two refuted remedies (bump the pinned Playwright build, pin pdfjs-dist down) were not actioned; the recorded measurements were re-derived after install and match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * perf(viewer): virtualize the PDF reader into a windowed page column The reader rasterised exactly one page into one canvas, so every page flip on a long guideline was a cold pdf.js render. That is the remaining felt slowness in the document view. The viewer now renders a column of page slots and keeps a small window of them rastered. Each slot reserves its page's box whether or not a canvas is currently in it, so disposing a far page does not move the scroll position a reader navigates by, and pages outside the window drop their backing store instead of holding it until collection. Three constraints shaped this and are resolved explicitly rather than deferred: The raster budget is now document-wide. resolveCanvasRasterPlan bounds ONE canvas against WebKit's ~2^24 ceiling and says nothing about how many exist, so N individually-legal canvases could still exhaust device memory. resolveLiveCanvasWindow caps total retained raster instead. Its useful property is the curve, not the constant: a fit-width phone page never binds against it, while a page at maximum zoom costs the whole per-canvas ceiling and collapses the window to one — render-ahead disappears exactly where retaining neighbours would be most dangerous, with no special-casing of zoom. MAX_CANVAS_PIXELS is unchanged. Render-ahead is reconciled with disableAutoFetch rather than trading it away. Those flags exist because a reader looks at one page and pdf.js would otherwise pull a whole guideline over cellular; rendering neighbours pulls exactly those bytes back. Both flags stay, and the policy is bounded on three independent axes: one page either side, deferred to requestIdleCallback so a fast flip never pays for pages it passes, and switched off entirely under Save-Data or 2g. Three resident pages, never the document. Page sync stays one-way. Intent scrolls the column, scroll position derives the displayed page, and a derived page writes the route only when it did not come from a programmatic scroll. Two real races surfaced while testing this: the route effect re-runs when pdf.js reports its page count, which is always after the reader can have scrolled, so it now acts only when the route asks for somewhere the reader is not; and the in-flight gate is armed when intent is registered rather than a frame later when the scroll executes, since intersections landing in that gap read as reader input and cancel the jump. Multi-page documents get a bounded reading pane so the column is the thing that scrolls; single-page documents keep their existing geometry exactly. The fit scale now derives from the holder's content box rather than clientWidth minus a fixed 16px, which was 16px short at sm:p-4 — invisible with one canvas, a layout shift once slots reserve boxes from the same number. Preserved: the per-run pageToCleanup isolation (Sentry 15801413), canvas zeroing on dispose, the renderZoom debounce with its interim transform, and the isLikelyExpiredUrl recovery path — which now also fires for a neighbour's range 403 while refusing to blank the reader's good page over a failed prefetch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * perf(viewer): window the document rail's figure cards The rail mapped every clinical figure and every audit figure into a DocumentImage on mount. That row is not cheap: it parses table markdown, decides whether a structured AccessibleTable can render at all, computes quality warnings and evidence tags, and mounts a SignedImage frame. A guideline with ninety indexed tables paid all of it during hydration, before the reader had opened the section, and the audit list underneath paid it again. DocumentImageList renders a window of six and grows it as a sentinel comes into view, with an explicit control to reveal the rest. Short lists — the overwhelming majority of indexed documents — render whole and get no extra chrome at all. The window is derived during render rather than synchronised in an effect, so a list that shrinks underneath an expanded reader clamps immediately instead of pointing past the end of the array for a frame. The filmstrip is left whole on purpose: it is one button per figure with no image behind it, and it is the cheap way to reach any page. One correction to the Phase 3 brief, recorded in the test rather than assumed either way. The brief says collapsed audit rows "still mint signed URLs". SignedImage already defers its fetch behind an IntersectionObserver and a closed <details> is display:none, so that claim is at least doubtful — but it is a claim about real browser layout, and jsdom does no layout, so nothing available here settles it. What is certain, and is what this commit removes, is the mounting cost, which applies whether the section is open or shut. The rail's observer also uses a 320px root margin rather than SignedImage's 640px, so the two do not both run far ahead of the viewport once the section does open. No virtualization dependency: rows have data-dependent heights, a windowed list needs no measurement to be correct, and check:bundle-budget totals every built chunk, so a library would land straight on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * perf(viewer): give deferred rail figures a lower fetch and decode priority Two levers, both about the same thing: a secondary figure rail should not contend with whatever the reader actually opened. SignedImage now sets fetchPriority explicitly — high when the caller marked the figure above-the-fold, low otherwise. next/image already emits decoding="async", which governs when a decode blocks; fetch priority governs whether the image competes for that budget at all, and it was the missing half of the pair. The document rail passes a 240px IntersectionObserver root margin instead of the shared 640px default. The wide default suits a surface whose images are the point of the page; the rail's are not, and at 640px it minted signed URLs for rows most of a viewport away, which land while the reader is looking at something else. There is no cross-surface request scheduler, so this margin differential is the ordering: surfaces on the wide default resolve first. The 100-id batch signed-URL route stays unwired, deliberately. Beyond keeping a privileged owner-scoped API route out of a component-only diff, the case for it has actually weakened: windowing the rail to six rows means a figure-heavy document no longer mounts N rows at once, which was the many-distinct-images scenario the batch was meant to serve. Recorded on #283 with the measurement that should decide it, rather than left as a standing assumption. use-signed-image-url.ts is untouched. Its identity-in-the-dedupe-key and cache-write-outside-the-shared-promise fixes were confirmed green before and after (tests/auth-signed-url-cache.dom.test.tsx, 6 passed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * feat(viewer): complete the PDF reader's keyboard reading mode The holder handled arrow keys, +/-, and 0. Phase 3 adds Page Up / Page Down, Home / End, F for fit-to-width, and R for rotate. Rotation needed a route back out. `rotation` arrives as a controlled prop with no callback, so the keyboard could reach every viewing control except that one. Rather than give the viewer its own rotation state — a second source of truth for a single toolbar button — R calls the same `handlePdfRotate` that DocumentFrame's rotate control already calls, threaded down as `onRotate`. When no handler is supplied, R stays inert rather than swallowed: the event is not preventDefault'ed, so it still reaches whatever else wants it. Modified keystrokes are now explicitly ignored. Ctrl/Cmd+0 is the browser's own zoom reset and Cmd+Left is history back on macOS; a reader that lost either to the viewer would be worse off than one with no bindings at all. The holder's aria-label names the bindings, so a screen-reader user hears them on focus instead of having to discover them. Contract documented in docs/wiring-conventions.md and covered by tests/document-viewer-keyboard.dom.test.tsx, including the two rules that are easy to regress silently: only keystrokes aimed at the holder itself are handled, and rotation goes through the frame's callback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * docs(viewer): record the Phase 3 outcome and the OffscreenCanvas decision Phase 3's table said what to build; it now says what landed, including the two items that deliberately did not. Crop -> page overlay stays out: bbox is already SELECTed in document-detail.ts but absent from DocumentDetailImage, so it is a contract change across src/lib/**document** rather than a viewer change, and it is called out as the one remaining Phase 3 capability with the shape of the work named. OffscreenCanvas is not implemented, which is the plan's own instruction rather than a shortcut — it conditions the work on "measured main-thread paint cost", and no such measurement existed. Two things changed that. Virtualization keeps the reader's page and a neighbour already rastered, so the cold-render-per-flip cost that motivated a worker raster is largely gone before any threading work starts; and the new canvas gate now attaches the number (flip-to-painted, long task count and duration, backing pixels) on every Production UI run. #290 records how to read it and what result would close the question either way. Nothing about this could be measured locally: pdfjs-dist@6 needs Map.prototype.getOrInsertComputed, which this container's Chromium 141 lacks and Node 24.13.0 lacks too, so no browser and no headless harness here can raster a page at all. Toolbar density is struck from the table — it shipped in Phase 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * test(viewer): keep the canvas gate out of the phone-chrome consumer list The new spec imported blockExternalRequests from tests/helpers/phone-scroll, which silently enrolled it in scripts/verify-phone-chrome.mjs's consumer list — tests/verify-phone-chrome.test.ts asserts that list equals the set of specs importing that helper, and went red. Enrolling it would have been wrong anyway: this is a desktop raster gate and has nothing to do with phone chrome selection, so it keeps a local copy of the request block instead, with a comment naming the coupling so the next person does not re-import it. Also records #291: tests/pr-handoff-stop.test.ts fails for any session running as root, because it injects a write failure with chmod 0o555 and root ignores directory write bits. Confirmed pre-existing on a clean origin/main worktree with no local diff, so it is not from this branch — CI runs non-root and stays green, and only container sessions ever see it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * docs(issues): record the measured bundle-budget headroom on #252 A clean production build for this branch reports 306 client chunks at 1538.4 KiB gzip against the 1406.4 KiB baseline captured 2026-08-04 — +9.4% inside a 10% tolerance, so roughly 8 KiB of gzip headroom remains. The drift is pre-existing rather than from this branch: Phase 3 adds no dependency and its code delta is small. But it means the next feature-sized PR of any kind trips check:bundle-budget whatever it touches, which turns #252's open question — whether counting mockup chunks makes that a real signal — from theoretical into the thing that decides how the next red build is read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * docs(ledger): record the Phase 3 review for PR #1772 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7 * fix(pdf-canvas-viewer): release canvas backing store on render=false and reset geometry on rotation Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com> * fix(viewer): key-repeat pageRef, live-canvas max budget, rail window reset Relative keyboard moves read pageRef so key-repeat advances before React re-renders. Budget retained canvases against the largest measured page cost. Reset DocumentImageList on collectionKey. Soften #252 tip-only bundle wording; tighten #294 OffscreenCanvas close criteria. Drop the PR-added legacy shadow-tight on the empty page slot to keep the DS ratchet. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(viewer): remount rail image list on document change via key Avoid setState-in-effect for the expanded window reset; React remounts DocumentImageList when the document-scoped key changes. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * style(tests): format document-rail image window test after prettier Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR 1772 review-and-fix at content tip 2cd72f1 Superseding heavy-scope row with decisive verify:cheap / verify:pr-local lines, merge-tree clean, and the Phase 3 review dispositions. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit
that referenced
this pull request
Aug 9, 2026
…tes, and pin the factsheet heading census (#1779) * test(factsheets): census every h1 in the factsheet detail document The hero <h1> and the portaled print sheet's <h1> are correct and mutually exclusive by construction: on screen `.factsheet-print-sheet { display: none }` removes the print subtree, and in print `html.factsheets-printing body > *:not(.factsheet-print-portal)` removes the shell that owns the hero. Neither state exposes two headings to the accessibility tree, and the printed PDF is a separate document whose section headings are already <h2>, so demoting its title would leave it with no top-level heading. The real gap was that the existing assertion was scoped to the page testid, so the document-level invariant was asserted nowhere and a stray third <h1> would not have been caught. Pin the census instead: exactly two, one per container, both carrying the title, plus a non-empty <h2> outline in the print sheet. jsdom applies no stylesheet, so a census is the right guard rather than a visibility assertion. FactsheetPrintSheet stays in factsheet-detail-page.tsx — design-system-contract-utils.mjs scopes its raw-colour exemption to the literal factsheet-print-sheet marker and fails closed if it moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * docs(testing): record the verified recipe for restoring local browser gates The remote/Cloud drift note said to delegate browser proof to CI and left the impression that local gates were unrecoverable. They are recoverable; the blocker was two separate image faults, and the second is why the obvious fix looks impossible. The baked node_modules is stale or incomplete — containers have shipped none at all, and earlier ones reported playwright 1.62.0 against a locked 1.62.1 with tailwind-merge absent entirely, which is an incomplete install rather than a version skew, so the lockfile pin was never wrong. And npm ci cannot repair it because jsdom@30.0.1 requires node ^22.22.2 || ^24.15.0 || >=26.0.0 while images have shipped v24.13.0, so the install dies on EBADENGINE under engine-strict. Installing Node 24.19.0 clears that, npm ci then exits 0 and parity reports all seven pinned packages, and `npx playwright install` supplies Chromium 1234 (images ship only 1194). Verified end to end this session, launch included. Keeps the existing Stop intact and makes it cheap to honour: install the matching revision rather than forcing a run against 1194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): split production weight from mockup scratch One number could not honestly answer two questions. totalGzipBytes summed every built client chunk, including src/app/mockups/** design scratch that 404s in production, against a ceiling named as though it were production weight. #13 held that mockup chunks are not a production bundle; this gate charged them anyway, which is how PR #1580 blocked at +10.1% for chunks no user can load. #252 recorded the contradiction and left the metric undecided. Measured on a clean build of main at af85cbc, the blur had become the whole signal: 1546.5 KiB total was +9.96% of the 1406.4 KiB baseline — 576 bytes from failing Build — while production-only was 1279.1 KiB, 9.06% BELOW that same baseline. Every byte of the apparent regression was design scratch (267.5 KiB across 76 chunks over 66 mockup routes) and production had actually shrunk. latency-audit-2026-07-28 corroborates: 1,309,274 bytes then against 1,309,772 production-only now, flat to +0.04%, so the 2026-08-04 bump to 1,440,201 had absorbed mockup growth as production growth. Raising the ceiling again would have hidden that permanently, so this splits rather than ratchets. production (10%) covers every chunk a non-mockup route reaches plus chunks no manifest claims — framework, polyfills, runtime. mockups (25%) covers chunks reachable only from /mockups/**, as a runaway detector rather than a per-mockup gate; a ceiling tight enough to fire on the next mockup would just be --update'd reflexively. A chunk shared by both counts as production because it would be built either way. Attribution reads the per-route *_client-reference-manifest.js files under .next/server/app, since Next 16 webpack emits no app-build-manifest.json, and fails closed when that tree is missing or resolves no routes so the buckets can never silently collapse. Both fail paths proven against the real build. Also captures #296: pr-handoff-stop.test.ts fails in any root container because it chmods a fixture dir to force a write failure and root ignores permission bits — pre-existing, reproduced on clean af85cbc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): fail closed on bad manifests and zero baselines Review feedback on PR #1779: treat unparseable route manifests as fatal attribution errors instead of counting them as resolved empty routes, and handle a zero mockup/production baseline without NaN percentage math. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
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 freeto 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.
Summary
Makes the app genuinely multi-user: persistent cross-refresh sessions, three sign-in methods with open signup, and fail-closed per-user isolation. All code ships here; the live Supabase Auth config (signup toggle, providers, SMTP, redirect URLs, CAPTCHA) is a user-applied checklist in
docs/multi-user-auth-setup.md— this PR does not enable anything live.Implements the approved plan (phases 2 → 3 → 4, plus the Phase 1 doc). Per-user data was already
owner_id-scoped; this closes the session-persistence, auth-method, and fail-open gaps.What changed
Persistent cookie sessions (
@supabase/ssr)src/lib/supabase/server.ts(new) — cookie-aware server client vianext/headers.src/proxy.ts(new) — Next 16 renamedmiddleware.ts→proxy.ts; refreshes the session cookie per request (no-op unless configured and ansb-cookie exists).src/app/auth/callback/route.ts(new) — PKCEexchangeCodeForSessionfor OAuth / magic-link / confirmation returns, with an open-redirect guard.client.tsx→createBrowserClient(cookie-persisted); logins now survive refresh and the API can read the session.auth.ts—requireAuthenticatedUsernow also accepts the ssr cookie session; theAuthorization: Bearerpath still resolves first, so all 26 API routes are unchanged (signature preserved).Auth methods + UI (Phase 3)
useAuthSession()gainssignInWithPassword,signUpWithPassword,signInWithOAuth("google" | "azure"); magic link stays.auth-panel.tsx— Sign in ↔ Create account toggle, Magic link / Password segmented control, Google + Microsoft SSO buttons.Fail-closed owner scoping (Phase 4)
src/lib/owner-scope.ts(new)requireOwnerScope()applied at every retrieval/cache RPC boundary (rag.ts×8,document-enrichment,deep-memory). A nullownerIdnow throws in a real deployment instead of silently returning all owners' rows; stays permissive under demo / local-no-auth /NODE_ENV=test. Also closes the shared null-owner cache question (no unscoped retrieval → no null-owner cache writes in prod).tests/owner-scope.test.ts(new) covers present / permissive / production-throw.Phase 1 config doc
docs/multi-user-auth-setup.md— the live Supabase checklist the user applies, now with the concrete project URL, publishable key, and OAuth callback URL. §7 documents that the DB owner-RLS + private storage backstop is already in place (verified against live; security advisors clean), so no broad RLS migration is required.Isolation posture
Two layers: app-layer fail-closed
owner_idscoping (this PR) and the live DB backstop already present — every owner-scoped user-data table has RLS + anauthenticatedowner-read policyowner_id = (select auth.uid()); registry/internal tables are service-role-only; both storage buckets are private (server-minted signed URLs).Verification
npm run verify:cheap— 986 tests pass (107 files), lint + typecheck + sitemap green.format:checkclean.Not in this PR (needs live config)
🤖 Generated with Claude Code