Skip to content

Sign-in, Terms and server checks fail closed; declaration at first open (S2) - #3838

Merged
BigSimmo merged 13 commits into
mainfrom
claude/project-thread-lte75r
Oct 26, 2026
Merged

BigSimmo merged 13 commits into
mainfrom
claude/project-thread-lte75r

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Requested by Josh · project thread

Summary

Stream S2 of the 26 Oct master plan: checks that happened only in the browser, or that opened when something was missing, now run on the server and fail closed. It also builds the health professional declaration at first open (D26-07), which Josh chose on 26 Oct and which is recorded in docs/decisions/2026-10-26-health-professional-declaration-at-first-open.md, and turns R-05 on by default after his yes to question 5.

  • N-06. clinicalPageSignInRequired returned "no sign-in needed" when the server Supabase client was missing. It now shows the sign-in card in that case. It also shows it for a blocked account, and for a signed-in account that has not made the declaration. Demo mode and local no-auth development are unchanged.
  • N-03. Questionnaire scoring, therapy ranking and the patient details panel run in the browser, so the browser gate is their only gate. It used to open them whenever the browser had no Supabase config. A production build now keeps them closed unless it is a deliberate demo deploy (NEXT_PUBLIC_DEMO_MODE=true).
  • D26-40. Answer already checks the declaration on the server (Check the health professional declaration on the server before AI answers; reword three tool descriptions #3796). The signed link to a shared document's original file now makes the same check, read fresh from the account. A user's own uploads are exempt. The Terms prompt now counts only the account record, not this device's copy, so a save that failed keeps asking instead of leaving the user refused on the server with no prompt. The save error now says it will ask again.
  • D26-07 (built, Josh chose "Build first open"). The existing Terms prompt is the first-open step. Every clinical tool (calculators, therapy ranking, catalogues, library, snapshot pages) now waits on the same account record. After "Not now" a tool shows a card, "Accept the Terms to use this", whose button reopens the prompt. Accepting unlocks every tool in place. The prompt's own wording is unchanged.
  • R-05 (on by default, Josh's yes to Q5). New server setting THIRD_PARTY_ORIGINALS_ADMIN_ONLY, default true. The original file of any document the caller does not own goes only to administrators, checked on a live account lookup that refuses on failure. Cited excerpts, page images and registry pages stay open. Setting it to false in Railway reopens originals. Its decision entry docs/decisions/2026-10-26-third-party-pdfs-admin-only.md (merged from Reconcile 25 ledger inbox requests #3842) now points at src/lib/env.ts.
  • PD-03. The sign-in and sign-up form now links the privacy notice ("How we handle your data is in the privacy notice.").
  • PP-04. Any save, change or delete (anything but GET, HEAD, OPTIONS) arriving on the proxy's claims now rechecks the account live, as administrator routes already did. Live lookups refuse an account that is currently banned.
  • Calculator layering fix. Found while taking screenshots: "Review the Terms" inside a calculator opened the prompt behind it, because the calculator sheet sat in the page and painted over the overlay root. The calculator sheet now portals into the modal layer like every other Sheet. The old test file comment already named this as "the larger correct fix".

New wording, approved by Josh on 26 Oct (decision card; recorded in docs/decisions/2026-10-26-sign-in-and-declaration-card-wording-approved.md):

  • Sign-in form: "How we handle your data is in the privacy notice."
  • Terms save error: "Your acceptance could not be saved to your account, so we will ask again next time you open PsychSift."
  • Tool card: "Accept the Terms to use this" / "PsychSift's clinical tools are for health professionals. Accept the Terms once and every tool opens." / button "Review the Terms".

Not changed, and why:

  • /login still lands on Favourites. That redirect was a deliberate choice (audit L3, next.config.ts). A different landing page is a design change for S5.
  • Scoring code ships to every browser (N-03 residual), and /therapy-compass-data/*.json is a public static file. Someone using developer tools could run scoring signed out. The instruments and bands are public, so moving scoring to the server is a large change for little gain. That judgement is for Josh and counsel.
  • Page images (/api/images/*/signed-url, figure crops, cover pages) do not get the declaration or admin check. They appear inside answers and search results.
  • Reads keep the proxy fast path (PP-04 covers writes). A blocked account can still read its own data until the token expires, up to an hour. Closing that adds a network round trip to every read, which belongs with the speed work (W4).
  • Non-AI clinical APIs (/api/medications, differentials, search) check sign-in only, not the declaration. The pages that use them are behind the declaration card.
  • Document searches still fire for an undeclared account in use-dashboard-search-execution.ts; the results are hidden behind the card. That file belongs to S5.
  • Another device. If the declaration was made on another device, this device's stored session may not show it until the token refreshes, so the prompt can appear once more. Accepting again is harmless.

RAG impact: no retrieval behaviour change. No protected search file is touched.

Verification

  • Checks run:
    • npm run verify:cheap: lock parity, Codex hooks, diff integrity, lint and typecheck passed. Unit suite Tests 4 failed | 33533 passed | 19 skipped. All four failures were the repo-awareness tests refusing a decision file I created while the run was going ("Untracked Markdown documents would be omitted"). After committing it, tests/repo-awareness-gate.test.ts and tests/repo-awareness-generator.test.ts: Tests 58 passed (58).
    • After merging the latest main (no conflicts): typecheck passed, eslint on every changed file clean, the 15 changed and neighbouring test files Tests 176 passed (176).
    • All 22 calculator and sheet suites: Tests 219 passed (219).
    • Synthetic-session browser check on a local dev server (fake Supabase URL, every auth call intercepted, nothing sent to Supabase): first open shows the prompt, "Not now" then PHQ-9 shows the card, "Review the Terms" reopens the prompt above the calculator, accepting unlocks the questions. Screenshots are in the project thread.

New tests prove each check fails closed:

  • the original-file route returns an error with no link when the server client cannot be built
  • the original-file route refuses a signed-in account with no declaration before any link is signed
  • the admin-only helper refuses (503) when either account lookup fails, and is on by default
  • a save with valid proxy claims but a signed-out session gets 401
  • a banned account is refused on both the proxy path and a bearer token
  • clinical snapshot pages show the sign-in card for no server client, a banned account, and a missing or old declaration
  • the browser gate returns "declaration-required" for a signed-in account without the current Terms version
  • the Terms prompt stays up when only this device holds the acceptance, and reopens on request
  • the calculator sheet renders in the modal overlay layer

Risk and rollout

  • Risk: every signed-in account without the current declaration will see the Terms prompt on first open after deploy, and tools stay locked until they accept. Originals of shared documents become administrator-only. Saves make one extra auth lookup each.
  • Rollback: revert this PR. R-05 alone can be undone by setting THIRD_PARTY_ORIGINALS_ADMIN_ONLY=false in Railway.
  • Provider or production effects: none from this PR. Nothing was run against Supabase, OpenAI or Railway.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains PsychSift Production (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Evidence / not applicable / unresolved:

  • This tightens access to health-professional-only tools (docs/decisions/2026-10-20-clinical-decision-tools-need-sign-in.md). The N-03 residual above needs Josh's and counsel's view. No clinical content changed. Synthetic ids and accounts only in tests and screenshots.

Ready for review: Josh approved the wording on 26 Oct. Up to date with main.

🤖 Generated with Claude Code

https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp

- N-06: server-rendered clinical pages show the sign-in card when the server
  client is missing, instead of opening the record.
- N-03: the browser gate for scoring, therapy ranking and patient details
  fails closed in a production build with no Supabase config, unless it is
  a deliberate demo deploy.
- D26-40: the signed link to a shared document's original checks the health
  professional declaration on the account, as Answer does.
- R-05: THIRD_PARTY_ORIGINALS_ADMIN_ONLY limits shared originals to
  administrators. Off by default.
- PD-03: the sign-up form links the privacy notice beside the Terms.
- PP-04: saves recheck the live account instead of trusting the proxy's
  locally verified claims, and banned accounts are refused.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bo66x2f17KZ9k3KNquiG4i
…he account record (S2)

The server-rendered medicine and differential pages now treat a banned
account as signed out (PP-04). The signed-in Terms prompt no longer hides
on this device's copy alone, because Answer and original documents check
the account record on the server (D26-40).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
…owner decision 26 Oct)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
…laration (D26-07)

The owner chose to move the declaration from the first Ask to first open,
before any clinical tool. A signed-in account without the declaration on its
account record now sees a card in place of each clinical tool, whose button
reopens the Terms prompt. The server-rendered medicine and differential
pages check the account record too and refresh once it is saved.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
…opens above it (D26-07)

"Review the Terms" on a locked calculator reopened the Terms prompt behind the
calculator: the sheet sat in the page on the same modal rung and painted over
the overlay root. It now portals into OverlayPortal's modal layer like every
Sheet, and its stack and focus effects wait for the portalled panel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iRNGzm9De17uFBsQkHDGp
@supabase

supabase Bot commented Oct 26, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Next included review available in 56 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 90 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 1ad67db and 2e95de9.

Files selected for processing: 36
  • .env.example
  • data/repo-awareness-snapshot.json
  • docs/decisions/2026-10-26-health-professional-declaration-at-first-open.md
  • docs/decisions/2026-10-26-sign-in-and-declaration-card-wording-approved.md
  • docs/decisions/2026-10-26-third-party-pdfs-admin-only.md
  • src/app/api/documents/[id]/signed-url/route.ts
  • src/components/ClinicalDashboard.tsx
  • src/components/calculators/calculator-sheet.tsx
  • src/components/calculators/calculator-ui.tsx
  • src/components/calculators/use-signed-out-session-reset.ts
  • src/components/clinical-dashboard/email-auth-form.tsx
  • src/components/front-door/terms-acceptance-prompt.tsx
  • src/components/mode-kit/clinical-catalogue-gate.tsx
  • src/components/mode-kit/clinical-page-sign-in-notice.tsx
  • src/components/mode-kit/clinical-sign-in-notice.tsx
  • src/components/mode-kit/use-clinical-tool-access.ts
  • src/components/therapy-compass/screens/recommend-screen.tsx
  • src/lib/clinical-access.ts
  • src/lib/clinical-page-access.ts
  • src/lib/env.ts
  • src/lib/front-door-content.ts
  • src/lib/original-document-access.ts
  • src/lib/supabase/auth.ts
  • src/lib/terms-acceptance.ts
  • tests/auth-live-recheck.test.ts
  • tests/calculator-sheet-close.dom.test.tsx
  • tests/calculators-clinical-safety.dom.test.tsx
  • tests/canonical-publication-consumers.test.ts
  • tests/challenger-m5-stress.dom.test.tsx
  • tests/clinical-catalogue-sign-in.dom.test.tsx
  • tests/clinical-sign-in-gates.dom.test.tsx
  • tests/clinical-snapshot-pages-sign-in.dom.test.tsx
  • tests/document-viewer-shell.dom.test.tsx
  • tests/email-auth-form.dom.test.tsx
  • tests/front-door.dom.test.tsx
  • tests/original-document-access.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@BigSimmo BigSimmo self-assigned this Oct 26, 2026
@gitar-bot

gitar-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Gitar is working

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

Gitar

@BigSimmo
BigSimmo marked this pull request as ready for review October 26, 2026 15:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T15:26:08.728811Z 2e95de9 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

Copy link
Copy Markdown
Owner Author

Migration replay failed on 2e95de9 before any migration ran. Docker image pulls were rate limited ("toomanyrequests: Data limit exceeded" on the Supabase postgrest, kong, imgproxy and vector images). The diff doesn't touch supabase/, so this failure isn't from this PR, and no fix is needed in code. I'll re-run the failed job once when the rest of the workflow finishes, because the run is still in progress and GitHub refuses a re-run until then.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Lighthouse budget failed on 2e95de9. Only one cell went over: desktop /on-call LCP was 1257 ms against a 1027 ms baseline (+22.4%, tolerance +20%), with 2 of 3 samples over. This PR changes no On Call, layout or proxy file, and the page is measured signed out, so neither the Terms prompt nor the declaration card renders there. Every other route and metric stayed inside budget. The On Call changes that reached main today (#3832, #3834) are the likelier cause, and this branch picked them up in the merge. I'll re-run the failed jobs once, together with Migration replay, after the run finishes. If /on-call breaches again I'll compare against main before touching anything.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Review record
Branch: claude/project-thread-lte75r (PR #3838)
Head: 2e95de9
Scope: Run PR sweep
Outcome: no conflict (git merge-tree against origin/main is clean; branch is 32 commits behind main, which is not a reason to sync) and no unresolved review threads (0). Required CI was red earlier on Migration replay (Docker image rate limit) and Lighthouse budget (/on-call LCP); both were re-run and now pass, and the PR required aggregate is success on this head. Nothing to fix, no push made.
Checks: read-only review of hosted CI jobs on run 40348844768 (attempt 2): all required jobs success, advisory jobs skipped. No local gates run, no provider-backed checks run. Not verified by me: the clinical and privacy substance of the auth/Terms/declaration changes, which are left for Josh's merge decision.


Generated by Claude Code

@BigSimmo
BigSimmo merged commit 4705d27 into main Oct 26, 2026
55 of 59 checks passed
@BigSimmo
BigSimmo deleted the claude/project-thread-lte75r branch October 26, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants