test(document-viewer): shell-state coverage incl. the private-access gate (TCD-03)#1063
Conversation
…gate (TCD-03) DocumentViewer resolves a four-way shell state (loading / ready / auth-required / error) that decides whether a source document is shown at all, and it had no render coverage. The state is prop-drivable via initialDetail / initialError, so these tests pin it without a network: - auth-required: a private-access failure resolves to the "Sign in required" shell with its reason — not the generic failure shell (which would read as "broken" rather than "sign in"). - error: a generic load failure resolves to "Source unavailable" with the reason, and never to the sign-in shell. - ready: a supplied detail payload resolves to the document identity and to neither failure shell. Only useRouter and useAuthSession are external (the rest of the component's hooks are local state); the next/dynamic PDF preview is mocked defensively so a late resolve cannot pull a real canvas into jsdom. Verified: 3/3 pass; typecheck, lint, format:check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds deterministic DOM coverage for ChangesDocumentViewer shell-state coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #4345 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/document-viewer-shell.dom.test.tsx`:
- Around line 44-67: Update the detailPayload helper to be validated against the
actual DocumentDetailPayload type, replacing the `as never` assertion with an
explicit return type or a `satisfies DocumentDetailPayload` check while
preserving the existing fixture values.
- Around line 95-99: Update the “shows the ready shell…” test to assert the
supplied document identity directly: verify the rendered title and the exact
filename “clozapine-titration.pdf” from detailPayload(), rather than relying on
the broad /clozapine/i getAllByText match.
- Around line 10-15: Update the navigation mock setup around the hoisted vi.mock
factory to create the push spy via vi.hoisted, ensuring the factory references
the hoisted value instead of the module-scoped const push declaration. Keep the
existing router methods and behavior unchanged.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cd2b9a7d-2256-46fb-9029-0295b1e9b994
📒 Files selected for processing (1)
tests/document-viewer-shell.dom.test.tsx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b73d675714
ℹ️ 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".
…ironment The auth-required assertion passed on a workstation and failed in CI. The viewer gates every document request on a local-project identity probe (/api/local-project-id); when that probe reports no safe local origin it replaces the viewer error with its own "unsafe local project" message, so the private-access gate never engages and the sign-in shell never renders. Whether the probe succeeds depended on a reachable dev server, not on the behaviour under test. Stub the probe as a safe local origin, and await the shell text so the assertion holds whichever tick the state settles on. Flipping the stub to an unsafe origin reproduces the CI failure exactly, which is the evidence that this was the cause. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91d14788ce
ℹ️ 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".
In demo / local-no-auth mode every document is public, so the private-access gate is deliberately inert and the sign-in shell can never render. The CI runner exports one of those, which turned the gate assertion into a failure there while it passed on a workstation without them — reproduced locally with NEXT_PUBLIC_LOCAL_NO_AUTH=true, which fails exactly the same single test. Stub both flags off per test so the assertion is about the gate rather than the runner's mode. Verified green with LOCAL_NO_AUTH=true, with DEMO_MODE=true, and with a clean environment; flipping the mocked session to authenticated still turns it red, so the gate is genuinely asserted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51fbf4bf59
ℹ️ 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".
Fixed 1 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e97b7eacf8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d820823d
ℹ️ 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".
| authorizationHeader: {}, | ||
| registerAuthRequest: () => ({ epoch: 1, release: vi.fn() }), | ||
| isAuthEpochCurrent: () => true, | ||
| markSessionExpired: vi.fn(), |
There was a problem hiding this comment.
The fresh evidence in this head is that useAuthSession still constructs a new authorizationHeader object and auth callbacks on every render. DocumentViewer includes these values in the document-load effect dependencies, and each initialDetail/initialError load writes fresh array state before rerendering, so this new jsdom suite can keep aborting and restarting the same load path until it times out or hits update-depth/act noise instead of reliably proving the private-document shell. Hoist a single auth session/header/functions object from the mock.
AGENTS.md reference: AGENTS.md:L490-L503
Useful? React with 👍 / 👎.
1 participant
Summary
Adds shell-state coverage for
DocumentViewer(finding TCD-03). The component resolves a four-way state —loading/ready/auth-required/error— that decides whether a source document is shown at all, and it had no render coverage. Test-only, no product code changes.The state is prop-drivable via
initialDetail/initialError, so the tests pin it without any network:Despite the file's size, only
useRouteranduseAuthSessionare external dependencies (the rest of its hooks are local state), so the harness is small. Thenext/dynamicPDF preview is mocked defensively so a late resolve cannot pull a real canvas into jsdom.Note: the assertions are deliberately mutual-exclusivity checks on the resolved shell rather than presence checks on the dynamic preview — the preview never mounts synchronously in jsdom, so asserting on it would have been vacuous.
Verification
tests/document-viewer-shell.dom.test.tsx— 3/3 pass (jsdom)npm run typecheck,npm run lint,npm run format:checkcleanRisk and rollout
🤖 Generated with Claude Code
Summary by CodeRabbit