Recover credential-expiry deliveries instead of dropping them (CL-7209)

[TheGreatAxios]

Summary

CL-7209: notify delivery had no recovery path when a durable write outlived its paired step. This addresses the credential-expiry sweep half of that (apps/hub/src/credential-expiry-sweep.ts).

• tickCredentialExpirySweep used to call claimExpiry (active→expired) before mailing the reconnect nudge. A throw from the mail step, or zero active recipients, left the credential permanently expired with no notification ever sent and no later tick reconsidering it — findDueCredentialExpiries only looks at active rows.
• Reordered to mail first, claim after, extracted into mailThenClaimExpiry. A credential-expired event dedupes on credentialId alone (not per-tick — see packages/notify/src/render.ts's notificationExternalId and the existing deliver.test.ts coverage), so retrying the mail on a later tick is always a harmless no-op if it already went out, which is what makes claiming after mail race-safe.
• Zero recipients or a thrown mail error now leaves the credential active instead of claiming it — the sweep's own periodic due-scan is the retry mechanism, no new queue/table. This folds in the "no recipients" claim-then-drop shape the same reorder already fixes for the throw case.
• Each candidate's mail attempt is isolated in its own try/catch reported through reportError (@corbits/error-sink), so one candidate's failure no longer aborts the rest of the tick's due/refreshable candidates (previously the whole loop would abort on the first throw).
• Same fix applied to the MCP-refresh-failure branch (identical claim-then-mail shape).
• Added a doc comment + filed CL-7238 for the other half of the ticket (deliverNotification's mail-write-then-dispatch-enqueue gap in packages/notify) — see "Deferred" below.

Bounded, per review feedback: leaving a credential active forever when nobody can ever be notified traded the original notification-gap bug for a worse state-correctness one — the record would assert active (usable) for a credential that's genuinely dead at the provider, indefinitely, with anything selecting active credentials trusting a row that will never work again. mailThenClaimExpiry now bounds the grace period by how long the credential has been due (dueSince, its stored expiresAt):

• Zero recipients: MAX_UNOWNED_CREDENTIAL_AGE_MS = 7 days — a slow-moving tenant-provisioning gap (new tenant, departed user being replaced) deserves real-world time to resolve before writing off the notification as unreachable.
• Mail keeps throwing: MAX_UNMAILED_CREDENTIAL_AGE_MS = 24 hours — a mail outage is a transient, systemic condition most likely to resolve within a day if it's going to resolve at all.

Past its budget, the sweep claims the expiry anyway (so the DB stops asserting something false) and reports the abandonment through reportError with the credential id and how long it went unnotified, so a person can find and reconnect it even though its owner was never told. Applies to both the HF-style due loop and the MCP-refresh-failure branch (RefreshableMcpCredential now carries its own expiresAt, already read off the row for the cutoff filter but not previously projected into the type).

Deferred to CL-7238 (not in this PR)

deliverNotification's mail-write-then-dispatch-enqueue gap (packages/notify/src/deliver.ts) is a separate, harder problem: closing it for real needs a seam @corbits/mailbox doesn't expose today (an in-transaction hook, or a way to read back an existing mail row's id by key) — an upstream change, not something this repo can fix cleanly. Filed CL-7238 with the concrete ask. Confirmed while investigating: in the live apps/hub composition, the credential-expiry sweep is deliverNotification's only wired caller, and its dispatch store is in-memory (not durable) with no sink registered — so this gap has no active production impact today.

Process note

Critique hit the concurrent-subagent cap (another set of lanes was saturating it) both times it was attempted and was not retried; I self-reviewed each diff against the same checklist instead (posted as PR review comments), including re-running the new bound tests against the pre-bound commit to confirm they fail red before the fix and pass green after.

Test plan

• WORKBENCH_CHECK_CONCURRENCY=2 WORKBENCH_CHECK_SINCE=origin/main bun run typecheck
• WORKBENCH_CHECK_CONCURRENCY=2 WORKBENCH_CHECK_SINCE=origin/main bun run test (all affected packages green; the one unrelated failure in scripts/run-all.test.ts is a pre-existing collision between the ambient WORKBENCH_CHECK_CONCURRENCY env var and a test asserting default concurrency — reproduces identically with this branch's changes stashed)
• WORKBENCH_CHECK_CONCURRENCY=2 bun run lint (0 errors; 8 pre-existing warnings in untouched files)
• bun run check:structural

[TheGreatAxios]

self-review · substituting for Critique (concurrent-subagent cap saturated by other lanes, did not retry)

apps/hub/src/credential-expiry-sweep.ts — mail-then-claim reorder applied to both branches (main due loop line ~400, MCP-refresh-failure branch line ~432), no leftover claim-before-mail path. claimExpiry's conditional UPDATE is unchanged, so the replica race is still guarded; a deduped mail (no throw) still falls through to claim, only a thrown error skips it. reportError call shape (operation, tenantId, extra.credentialId) matches packages/error-sink/src/context.ts's ErrorContext schema.

apps/hub/test/credential-expiry-sweep.test.ts — flipped the "no recipients" test to assert not-claimed/not-mailed; added throw-then-retry-succeeds, dedupe-still-claims, and one-candidate-failure-does-not-abort-the-rest for both the HF-style and MCP-refresh-failure branches. All fail against the pre-fix code (verified before implementing) and pass after.

packages/notify/src/deliver.ts — doc-only change, references CL-7238 (filed) for the deferred mail/dispatch atomicity gap; not fixed in this PR per architecture review (real fix needs an upstream @corbits/mailbox change).

No blocking findings.

[TheGreatAxios]

self-review · substituting for Critique (concurrent-subagent cap saturated again, did not retry)

Bound how long an unnotified credential expiry stays active (HEAD) — verified:

• ageMs = now - dueSince computed and compared correctly in both branches of mailThenClaimExpiry; happy-path (mail succeeds) is unaffected by the bound and claims immediately regardless of age.
• Zero-recipients and mail-throws paths each fall through to claimExpiry only once their respective bound is crossed, never before, never double-claiming (claimExpiry's existing conditional UPDATE is unchanged).
• reportError(new Error(...), {...}) for the zero-recipients abandonment (no real thrown error to report there) has direct precedent in this repo: packages/chat/src/chat-orchestrator.ts:556.
• RefreshableMcpCredential.expiresAt: Date plumbing typechecks; the due filter's type predicate correctly narrows Date | null → Date.
• Confirmed all 4 new tests fail against the immediately-prior commit (checked out HEAD~1's implementation with HEAD's tests, all 4 new assertions failed as expected) and pass again after restoring HEAD.

No blocking findings.