Repository navigation
Conversation
|
Here's a visual recap of what changed: Open the full interactive recap |
There was a problem hiding this comment.
Builder reviewed your changes — looks good ✅
Review Details
Incremental review summary
Reviewed PR #8059 at the latest head (a089bd1). The PR applies Trash and expiry access rules across recording data, media, comment actions, recording context, share SSR, and activity notifications, while preserving owner Trash previews and restoring access after untrash. This is a high-risk privacy/access-control change. The latest commit only changes how view-screen.ts extracts the first recording row from the query result; the logic remains equivalent, and the full current diff received four independent balanced reviews.
No new actionable issues were confirmed in the PR diff. One reviewer raised whether expired owners should receive activity notifications, but the existing policy and added notification-list tests explicitly filter expired recordings for owners too, so this is not a new regression. The approach otherwise consistently shares lifecycle checks and adds useful regression coverage. Landing remains coordinated with #8088 for the comments-off notification/search policy.
🧪 Browser testing: Skipped — browser-test-planner is unavailable in this environment; no independent browser run was performed.

Land with #8088 so the comments-off privacy policy reaches every reader together.
What changed
Activity email recipients use the same owner-aware Trash policy as notification reads. Owner previews remain available, and restoring a recording restores normal notification recipients. Existing password, expiry, resource-access, and notification preferences still apply.
Apply one shared Trash check to recording data, media, recording context, and notifications. Owners retain their Trash preview, and restoring a recording restores normal access. Completed bug-report context also follows the same recording lifecycle gate.
Share links now show the existing “Clip unavailable” state after the first Not found response, including missing and trashed recordings. The obsolete missing-row retry no longer holds visitors on a loading placeholder. Anonymous server data and social metadata also treat trashed recordings as missing. Initial loading, session discovery, live private access requests, password prompts, and processing polling remain intact.
Comment actions share recording lifecycle and discussion-setting checks. Replies reuse the existing comment action, and comment reactions follow the same expiry policy. Recording context omits disabled comments. Comment deletion respects Trash access while allowing authorized cleanup with comments disabled.
The clarified comments-off policy is implemented separately in #8088, stacked on this PR. That follow-up hides saved comments from viewers in notifications and search while preserving the owner's read access. This PR retains its existing comment-history behavior pending that follow-up.
Verification
The final array-bounds narrowing passes 15 screen-context tests and the unmodified compiler-strictness guard, launched through the same TypeScript binary via Node on Windows. Lifecycle behavior and the earlier technical review remain applicable.
Three activity-recipient regressions failed before the repair; thirteen controls passed. The final root rerun passes all 82 focused tests across four files. The five-file repair received a fresh Sol high/default review with no findings; typecheck, strict lint, oracle/source-test guards, formatting, LF and diff checks pass. Database/access/mail boundaries are mocked; no live email delivery is claimed.
After merging main (
a4031062), all 86 focused comment/lifecycle tests and Clips typecheck pass. Main’s expiry controls and catalog metadata are retained alongside the shared Trash/comment-setting gate. Current strict lint, the oracle guard, source-reading-test guard, formatting, and diff checks pass.Original action and route regressions reproduced before production edits; all 229 tests in thirteen relevant suites passed for those fixes.
Both unavailable-page cases reproduced with failing-first rendering tests. The loader/meta regression also failed before the metadata repair for public and private Trash. The earlier route suite passed 134 tests in thirteen files, and the earlier root rerun passed 173 tests across eight affected suites.
Root-owned E2B Chromium verification on
71fb7a9shows missing and trashed links display “Clip unavailable” after one 404 on desktop and mobile. Private access-request and password-prompt controls pass. Authentication was enabled, data was fake, and the sandbox and browser contexts were terminated afterward.The completed bug-report regressions failed first for Trash and expiry; 31 focused tests now pass, including owner and available-editor controls.
Clips typecheck and formatting pass. Independent Sol reviews found no actionable issues.
Earlier sweeps passed all 93 repository guards and dependency-setup recovery reruns, with GNU grep on PATH. The metadata repair additionally passes the SSR cache and test guards, and the prior CI strict-lint policy reported zero findings across all 31 changed TypeScript files. The oracle-change comments explicitly document why the obsolete retry-helper and source-string assertions were replaced by failing-first rendering tests; the current oracle guard passes. No guard baseline was changed.
No schema or stored-data changes.