fix: actually invalidate the session on OAuth logout

[heskew]

clearOAuthSession gated on typeof session.delete === 'function', but the real Harper request.session is a shallow copy of the hdb_session record exposing only .update (a persisting put) — never .delete (confirmed against harper@5.1.9, security/auth.ts:110). So the delete branch never ran, and the in-memory fallback (session.user = null) was never persisted: after an explicit logout — or when a token is expired/revoked and clearOAuthSession runs — the stored hdb_session record kept authenticating until cookie TTL.

Fix: invalidate by persisting session.update({ user: null, oauth: null, oauthUser: null }), mirroring Harper's own logout() (security/auth.ts:381). The next request resolves session.user to no user. Falls back to an in-memory clear only when no .update exists (non-session transports).

Tests updated across handlers / sessionValidator / withOAuthValidation — several had encoded the inverted delete-based model (asserting session.delete(id) and in-memory clears that never actually persisted). Full suite: 1157 unit + 15 integration, 0 fail.

[github-actions]

1 blocker found.

1. In-memory session state inconsistency in production path

File: src/lib/handlers.ts:546
What: The updated clearOAuthSession persists an invalidation via session.update({ user: null }) in the production path but does NOT clear the in-memory session fields (user, oauth, oauthUser) for the remainder of the current request.
Why it matters: As noted in the new documentation in src/lib/withOAuthValidation.ts, this causes the onValidationError callback and any requireAuth: false resources to observe a stale "authenticated" session state for the remainder of the current request, even though the session has been invalidated in the database. This violates the goal of "actually invalidate the session" and creates a regression in consistency compared to the fallback path (and the previous version of the code which always cleared in-memory state). The inconsistency is even explicitly pinned by a new test case in test/lib/withOAuthValidation.test.js:1186.
Suggested fix: Move the in-memory field clearing (setting session.user = null and deleting session.oauth/oauthUser) outside the else block so it runs in both the production and fallback paths.

Suggestions (non-blocking)

• src/lib/withOAuthValidation.ts:117 — Use clearOAuthSession for persistence and consistency. clearStaleOAuth currently only clears fields in memory. Consider importing clearOAuthSession from ./handlers.ts and updating this helper to call it. This ensures that "stale provider" paths (e.g. when a provider is removed from config) also benefit from the new database-level invalidation, preventing the malformed session from being reloaded on the next request.
• src/lib/withOAuthValidation.ts:117 — Nullify session.user in clearStaleOAuth. To ensure requireAuth: false resources observe a truly anonymous state when validation fails, request.session.user (the Harper identity) should also be nullified in the current request. (Note: using clearOAuthSession as suggested above would resolve this automatically).

[gemini-code-assist]

Code Review

This pull request updates the clearOAuthSession function to use session.update instead of session.delete to invalidate sessions, aligning with the actual API of Harper's hdb_session record which only exposes .update. This ensures that a null-user record is persisted to invalidate the session, rather than attempting a non-existent delete method. Corresponding unit tests have been updated to mock and assert the new .update behavior and verify that session properties are correctly set to null upon invalidation. There are no review comments, and I have no additional feedback to provide.

[claude]

1 blocker found (inline): clearOAuthSession's production path persists the invalidation but never clears request.session's in-memory user/oauth/oauthUser, so a requireAuth:false resource (or the index.ts provider-gone middleware passthrough) still observes a live-looking authenticated session on the request that just invalidated it -- pinned by this PR's own test at withOAuthValidation.test.js:1140-1190. This corroborates an independently-opened, unresolved gemini thread on the same line. One non-blocking suggestion posted inline re: a logout-vs-refresh race.

[github-actions]

🔴 Blocker — In-memory session state inconsistency in production path

The production path calls update() but doesn't clear the in-memory session fields. This causes requireAuth: false resources to see stale identity/tokens for the remainder of the current request. Move the in-memory clearing (session.user = null, delete session.oauth/oauthUser) outside the else block.

[claude]

Blocker — production path leaves request.session looking fully authenticated for the rest of the request

What: When session.id && typeof session.update === 'function' (the normal case for any real cookie-loaded session), clearOAuthSession persists { user: null } to the DB but never touches the in-memory session.user / session.oauth / session.oauthUser. The else fallback clears them; this branch doesn't. Pre-fix, every call fell into that same in-memory-clearing branch (since .delete never existed), so this is a new gap, not a pre-existing one.

Why it matters: Both mid-request callers of clearOAuthSession let the request continue afterward: validateAndRefreshSession's expired/revoked-token paths feed withOAuthValidation's requireAuth:false passthrough, and index.ts's "provider not found" middleware branch calls clearOAuthSession then next(request). Downstream code — a requireAuth:false resource, or any app code reading request.session — now observes fully populated, live-looking oauth data on the very request that just invalidated the session. This PR's own test pins it: test/lib/withOAuthValidation.test.js:1140-1190 shows a requireAuth:false resource running (status: 200) with calls[0].oauthAfterValidate !== undefined right after clearOAuthSession ran — the resource believes the user still has a valid access token while the DB record backing it was just nulled. That's the opposite of "actually invalidate the session" for the request that detects the problem.

This matches an independently-opened, unresolved thread from the gemini reviewer on this same line (this same commit) — two independent traces landing on the same gap.

Suggested fix: Clear the in-memory fields in the if branch too (after the persisting update call), same as the else branch. If the intent is still for onValidationError to see stale data for audit logging (per the JSDoc above), mirror the existing clearStaleOAuth() pattern already used elsewhere in withOAuthValidation.ts — invoke the callback first, then clear — rather than never clearing on this path.

[claude]

Suggestion (non-blocking): This session.update({ user: null }) and validateAndRefreshSession's session.update(session) (sessionValidator.ts:92,169, unmodified by this PR — a full pre-refresh snapshot write-back) are both full-replace writes to the same hdb_session row with no version/CAS check. If a concurrent request's refresh write was read before this logout persisted and commits after it, it silently overwrites { user: null } with the stale, still-authenticated snapshot — resurrecting a session the user just logged out of. Realistic with multiple tabs/devices or a request racing a proactive 80%-lifetime refresh. Not fixable in this diff alone (no CAS primitive available here), but worth re-validating session.user isn't already cleared immediately before a refresh writes back, or tracking as a follow-up — the author's own commit message already flags this as deferred, so a tracking issue would make it visible outside commit history.