[3mdistal]

Landing hold: Per Steve's review, do not merge this PR independently of the password-write hardening in #8093. That change remains separately reviewed; integration must include it before this work lands on main.

Clips comment listing and deletion now follow recording access settings consistently. Listing keeps password-protected recordings in the password-aware share flow, deletion respects disabled comments for every role, and author checks normalize email case. Owners retain access to saved discussion. This replaces the earlier deletion cleanup exception.

Workspace lookup failures now stay distinct from confirmed Personal scope for ordinary and delegated callers through credential resolution, session backfill, MCP and background work. Credential wrappers preserve typed availability errors and their original causes; revoked delegated membership remains a definitive refusal. MCP resolves scope before server construction, and background work does so before creating resources. The narrow first-boot exception for a missing membership table remains intact.

The existing access-error class now lives in a small module that does not load sharing database code. Its public export retains the same identity. This also repairs collection of unchanged Brain and Slides test fixtures. Includes a core patch changeset.

Engine listing retains per-provider diagnostics when a credential lookup is unavailable and reports current-selection failure separately. Runtime credential checks remain strict. The existing client model display fallback is unchanged.

Background automation attempts now retain terminal run history when creator-membership or alert-recipient lookup fails, preserving the original typed failure before credential or agent execution. The separate, pre-existing scheduler identity-preflight path can still lack a history row when none is supplied; it is outside this repair.

Stacked on #8088, after #8059. Password-aware comment mutations remain separately reviewed in #8093.

Validation:

• Failing-first Clips regressions: 6 failed, 14 controls passed; relevant suites: 251 passed across 15 files.
• Workspace, credential-wrapper and session-backfill regressions failed before their fixes. Prior core verification: 1,894 passed across 33 files; root rerun: 572 passed. Confirmed Personal, credential priority, first-boot and refusal controls pass.
• Both new automation-history regressions failed on unchanged production, then passed. Six relevant core suites pass 394 tests; the root runner/history recheck passes 125.
• Current-source imports reproduced the CI collection failures. Unchanged Brain fixtures now pass 23 tests and Slides fixtures pass 20; root reruns confirm both.
• Content parity: 185 tests across 19 files pass using current compiled core. The 597-row fixture now declares its intended Personal scope; pagination and preservation assertions are unchanged.
• Required Content MCP fixtures reproduced three setup failures before repair. Declaring their intended Personal scope explicitly restores all 28 tests across three suites; production access checks are unchanged.
• Core and Clips typechecks, formatting, and all 93 repository guards pass, with GNU grep on PATH.
• Independent Sol review found no actionable issues in the repair.

Tests use fake data. The automation regressions verify real persisted history; other focused suites isolate persistence as appropriate. No live beta check was run for this PR. The Clips credential doctor still reports 197 env-read findings outside this diff (exit 1).

[github-actions]

Here's a visual recap of what changed:

Open the full interactive recap

[builder-io-integration]

Builder reviewed your changes and found 3 potential issues 🔴

Review Details

Code Review Summary

PR #8092 hardens Clips comment authorization and changes organization lookup to distinguish a confirmed Personal scope from an unavailable membership lookup. The Clips changes centralize recording access checks for comment deletion, preserve owner cleanup behavior, normalize author-email comparisons, and prevent direct comment reads from bypassing password-protected share flows. The core change makes unknown organization scope explicit so credential resolution can fail closed rather than silently treat an unreadable membership table as Personal. The direction is sound and the added tests cover important role and lookup cases, but several call sites and the comment write path do not consistently preserve those guarantees.

Risk: High — this changes access-control behavior and credential-scope resolution.

Key findings

• 🔴 HIGH — The new generic unavailable-scope error bypasses action dispatch's existing first-boot handling for a missing org_members table, causing affected action requests to fail.
• 🟡 MEDIUM — MCP identity and background-agent callers still catch lookup failures and continue without an org, treating unavailable scope like Personal.
• 🟡 MEDIUM — The password check applies to comment reads but not mutations, allowing comment writes to password-protected public recordings without password-aware access.

The owner-access behavior, case-insensitive author check, and disabled-comments enforcement are otherwise covered by focused tests. 🧪 Browser testing: Will run after this review (PR touches user-facing comment action behavior).

[builder-io-integration]

🟡 Require password-aware access for comment writes

The new password check is limited to operation === "read"; default write checks only enableComments, and comment mutation actions (including add-comment) use that default. A caller with viewer access to a password-protected public recording can therefore mutate comments without passing through password-aware share access. Apply an equivalent verified-password requirement to writes, or explicitly define and test a different policy for password-protected shares.

Additional Info

Reported by 2 of 3 independent code-review agents; confirmed against comment-access.ts and add-comment.ts:59-60.

[3mdistal]

Addressed separately in #8093, stacked on this PR. Successful password unlocks issue a distinct, short-lived comment grant carried in request headers. Owners and unlocked viewers retain commenting; media and agent-read grants do not authorize comment writes. Failing-first regressions, 280 final tests, typecheck, repository guards, and independent review pass there.

[steve8708]

Thanks for the update. I see the password-unlock grant is stacked in #8093, but this head still leaves comment writes gated only by enableComments; add-comment uses that default check, so a viewer of a password-protected public recording can comment without unlocking it. Please include the write check here or keep #8092 from merging independently until #8093 lands.

[3mdistal]

Understood. This PR is now on hold and will not merge independently of the password-write hardening in #8093. The grant mechanism remains separately reviewed as requested; its UI review repairs and final-head live verification are still in progress. The PR body records the landing hold.

[3mdistal]

Updated in 6c5963d: credential wrappers and session backfill preserve unavailable workspace lookups as errors, and the access-error class keeps its public identity without loading sharing database code. Current-source Brain and Slides fixtures pass unchanged. The PR body records the failing-first and final checks. The separate password-write review in #8093 and this PR's landing hold remain in place.

[steve8708]

Thanks for the update. The current head still lets a membership read failure stop runBackgroundAutomation before startAutomationRun, leaving no run-history row. Please preserve the failure signal while recording the failed run; I’ll re-review once the current thread is addressed.

[3mdistal]

Pushed 69be50b with the run-history repair. Failed creator-membership or organization-admin recipient lookups now leave a scoped terminal error row and preserve the typed failure and original cause before any credential, thread, or agent execution. Both regressions failed on unchanged production; the six relevant core suites pass 394 tests, the root runner/history recheck passes 125, and core typecheck, formatting and all 93 guards pass. Independent Sol review found no actionable issues.

The Content DB failures were fixture incompatibilities exposed by strict scope lookup. Declaring the three MCP fixtures' intended Personal scope explicitly restores all 28 tests; production authorization is unchanged. CI is running on the new head.

One separate path remains: the scheduler's identity preflight can leave no history row when none was supplied. That behavior predates this PR and is outside this runner/helper fix. It is recorded as a vault follow-up and has not been changed here. The existing landing hold remains.

[builder-io-integration]

Builder reviewed your changes and found 2 potential issues 🟡

Review Details

Incremental review — high risk

This update extends the org-scope failure handling into background automation history: when creator-membership or alert-recipient lookup fails, it attempts to persist a terminal failed run before credential or agent execution. The core approach remains sound, and I verified that the prior finding about membership-query failures suppressing automation history is addressed; its thread was resolved. The Clips write-access password issue remains unchanged and open, and the PR still documents the integration hold with #8093.

Findings

• 🟡 MEDIUM — getSession() now propagates org-backfill failures, but notification and progress routes still map any rejection to null and return 401. This misclassifies an unavailable membership store as an unauthenticated user.
• 🟡 MEDIUM — If persisting the new terminal failure state itself fails, the raw error is caught as a generic history-creation failure and the automation proceeds without durable failed history.

The focused code changes preserve the distinction between authorization refusal and unavailable scope, but these downstream failure paths need consistent handling. 🧪 Browser testing: Skipped — the browser-test-planner agent is unavailable in this environment; the dev server is running.

[builder-io-integration]

🟡 Org-backfill failures become unauthenticated responses in session consumers

getSession() now rejects when event-based org backfill cannot read memberships, but consumers such as notifications and progress still use getSession(event).catch(() => null) and return 401. Preserve the distinction between a real missing session and an unavailable membership lookup so logged-in users do not receive false unauthenticated responses.

[builder-io-integration]

🟡 Failed history finalization can still let the automation execute

If this finishAutomationRun(..., { requirePersisted: true }) call fails, the raw history-write error reaches runBackgroundAutomation, whose catch continues on non-BackgroundAutomationRunError failures. The automation can then execute despite the recipient lookup failure, without a durable terminal history state; wrap or propagate finalization errors so execution stops in this case.