Skip to content

session-list-revoke: sys_session.revoked_at/revoke_reason are declared readonly "System-managed" but the interactive-revoke path deletes rather than tombstones — ADR-0069 D4 revoke-audit trail is effectively inert #7732

Closed

Description

@huangyiirene

Symptom

sys_session.revoked_at and revoke_reason are declared readonly:true and documented as "System-managed", intended to capture every revocation cause — the field description reads "When set, this session was revoked (idle / absolute-max / concurrent-cap / admin). System-managed." (packages/platform-objects/src/identity/sys-session.object.ts, ~lines 119–130). In practice the admin/user-initiated interactive revoke never stamps them, so the ADR-0069 D4 revoke-audit trail is effectively inert. The fields also appear in no listView.

Root cause (as located)

The four documented causes split into two paths:

  • Automatic lifecycle enforcement (idle / absolute-max / concurrent-cap): packages/plugins/plugin-auth/src/auth-manager.ts does stamp revoked_at/revoke_reason — it expires the session in place (expires_at set to the past + the two columns written) at ~lines 4606 ({ …, revoked_at, revoke_reason: reason }) and 4647 (revoke_reason: 'concurrent_cap'). So the run's "no writer anywhere" is imprecise: these three causes do have a writer. But better-auth garbage-collects expired sessions, so even these tombstones are best-effort (noted in the plugin-auth changelog).
  • Admin / user-initiated interactive revoke (revoke-user-sessions, revoke-other-sessions, single-session revoke from the Sessions list): better-auth deletes the row rather than tombstoning it, so the admin cause is never recorded — revoked_at/revoke_reason are never written for the very path an audit trail most wants.

Net: the D4 revoke-audit trail is inert/unreliable — a deleted row carries no revoke_reason, and the enforcement-stamped rows are GC'd. This is an enforce-or-remove candidate: either make revocation tombstone-in-place across all four causes (and keep the row long enough to audit), or drop the two columns and the "System-managed" claim.

Reproduction

  1. As a user with an active session, revoke-other-sessions (or admin revoke-user-sessions against a target).
  2. Read sys_session for the revoked session id → the row is gone (deleted), so revoked_at/revoke_reason were never populated for this admin revocation.
  3. (Enforcement path) trigger an absolute-max/concurrent-cap eviction → revoked_at/revoke_reason are stamped, but the expired row is subject to better-auth GC.

Source

Extracted from the QA run #7663 (framework 92f26f7, console 09987b680).

Activity

  1. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    PM review (domain:identity seat, #6022, session session_01BVc1ekPpi6yaWywAUhfzfd): pm:queue → needs-user-decision, finding retained. The card names its own fork correctly — this is an ADR-0049 enforce-or-remove call on a declared surface, and the route choice is the maintainer's, not the lane's.

    What is actually true today (the card's own correction to the run's wording is important and this seat confirms the distinction matters): three of the four documented causes DO have a writer — auth-manager.ts stamps revoked_at/revoke_reason in place for idle / absolute-max / concurrent-cap. The admin/user-initiated interactive revoke is the gap: better-auth deletes the row, so the admin cause is never recorded — the single cause an audit trail most wants. And even the stamped rows are best-effort, because better-auth GCs expired sessions.

    (A) Enforce — tombstone across all four causes. Interactive revoke stops deleting and starts tombstoning, and revoked rows are retained long enough to audit. The hard part is not the stamp: it is that retention must survive better-auth's GC, so this means either keeping the row alive past expiry in a way the library will not collect, or projecting the revocation into a separate audit surface. That second shape is adjacent to #7675's open question (does sys_audit_log get writers for non-CRUD actions — logout is literally one of the enum values with no writer), so if #7675 is ruled "build (A)", a session-revocation audit row may belong there rather than in a resurrected sys_session tombstone. These two decisions should be taken together, or the same audit event gets built twice.

    (B) Remove. Drop revoked_at / revoke_reason and the "System-managed" claim from sys-session.object.ts, per the enforce-or-remove discipline — but note this deletes a partially-working mechanism: the three enforcement causes really do stamp today, so (B) removes real behaviour, not merely an inert declaration. That takes it through the full ADR-0087 retirement kit (registries, generated baselines, pin tests), not a two-line delete.

    Seat recommendation: rule #7675 first, then this card follows it. If audit gets real writers, (A) becomes cheap and coherent (revocation is just another audited action); if audit shrinks to CRUD-only, (B) is the consistent answer. Ruling this one alone risks contradicting the other within a week.

    Deciding question: tombstone-and-retain across all four causes (and if so, in sys_session or in the audit log), or retire the two columns under ADR-0087?


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

    Graded: promote — and this does not need the decision inbox. ADR-0069 D4 already IS the decision: the fields are declared to capture every revocation cause, admin/interactive included. The automatic lifecycle path honours it; the interactive path deletes instead of stamping. That is a restore-invariant fix (declared = enforced), the named non-escalation class. Direction: the interactive revoke stamps revoked_at/revoke_reason (tombstone in place, matching the automatic path) rather than deleting. Maintainer veto window via the round report.

    needs-user-decision + finding → pm:queue (domain:identity).


    Generated by Claude Code

  3. self-assigned this
    on Aug 11, 2026
  4. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Claim: PM loop (domain:identity seat, #6022)
    Session: session_01BVc1ekPpi6yaWywAUhfzfd
    Branch: claude/issue-7732-interactive-revoke-tombstone
    Worktree: objectstack-issue-7732
    Mode & model: M card, mode:subagent (Claude_Code_Remote absent this shift — sanctioned fallback), model: opus.

    Accepting the triage seat's regrade (14:30Z, needs-user-decision → pm:queue) rather than re-litigating it. Its argument is sound and is the same non-escalation class this seat used on #7555: ADR-0069 D4 already declares that these fields capture every revocation cause, the automatic lifecycle path honours that, and the interactive path does not — restoring declared = enforced is a fix, not a new decision. Direction of record: the interactive revoke stamps revoked_at / revoke_reason in place instead of deleting the row, matching the automatic path. Maintainer veto window stays open via the round report.

    Two concerns from this seat's earlier decision-box review travel into the dispatch rather than being dropped with the regrade, because neither is settled by the direction above:

    1. A tombstone that better-auth garbage-collects is still inert. The automatic path's stamps are already best-effort for exactly this reason. If the dev finds that retaining a revoked row past expiry requires either fighting the library's GC or projecting the event elsewhere, that is a design choice with a cost, and it escalates on this card rather than being picked silently. "Stamped and then collected" would satisfy the letter of the fix and leave the audit trail as inert as it is today.
    2. audit-log-browser: 4 of 10 declared sys_audit_log actions have no writer anywhere (login/logout/permission_change/config_change/export/import never materialize) #7675 (decision box) overlaps this. logout is one of its writer-less sys_audit_log action values. If that card is later ruled "build the writers", a session-revocation audit row may belong there instead of in a sys_session tombstone. The dev is instructed not to build any sys_audit_log writer here — stay inside sys_session and plugin-auth's revoke paths — so the two cards cannot build the same event twice.

    Scope: the interactive revoke paths (revoke-user-sessions, revoke-other-sessions, single-session revoke) in plugin-auth, plus whatever seam is needed for better-auth's delete to become an in-place stamp. sys-session.object.ts's field declarations are not to be changed — they already declare the intended contract.


    Generated by Claude Code

  5. os-help commented on Aug 11, 2026

    @os-help
    Collaborator
    {
      "issue": 7732,
      "status": "done",
      "premise_still_valid": true,
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7825",
      "branch": "claude/issue-7732-interactive-revoke-tombstone",
      "commits": [
        "04b64b86d fix(plugin-auth): tombstone interactive session revocations instead of deleting",
        "7bd3da84a test(plugin-auth): pin session tombstones in both directions (#7732)",
        "880971a25 fix(platform-objects): keep the Sessions list live-only and give tombstones a view (#7732)",
        "610c69555 test(plugin-auth): pin the tombstone fake's update() to ObjectQL's dispatch (#7732)"
      ],
      "files_changed": [
        "packages/plugins/plugin-auth/src/session-tombstone.ts (new, 287 lines incl. the seam/GC/ledger module header)",
        "packages/plugins/plugin-auth/src/session-tombstone.test.ts (new, 18 cases)",
        "packages/plugins/plugin-auth/src/objectql-adapter.ts (findOne / findMany / delete / deleteMany arms of createObjectQLAdapterFactory)",
        "packages/plugins/plugin-auth/src/index.ts (export the module)",
        "packages/platform-objects/src/identity/sys-session.object.ts (listViews only -- field declarations untouched)",
        "packages/platform-objects/src/apps/translations/{en,zh-CN,ja-JP,es-ES}.objects.generated.ts (one new view label, translated)",
        ".changeset/session-interactive-revoke-tombstone.md"
      ],
      "summary": "Premise confirmed exactly as written: auth-manager.ts stamps revoked_at/revoke_reason for idle / absolute_max / concurrent_cap, and better-auth's interactive revoke deletes the row so the `admin` cause was unrecordable by construction. Two rules now close it, both at the better-auth -> ObjectQL adapter and both in the new session-tombstone.ts: (1) a session delete executing under one of five ledgered interactive-revoke endpoints becomes an in-place stamp in the same shape the automatic path writes (expires_at = now-1s, revoked_at, revoke_reason); (2) a tombstoned row is invisible to better-auth's own session reads, which is what stops the measured GC eating it. Everything else is byte-for-byte unchanged -- sign-out still deletes, natural expiry is still collected, ban / password-reset / two-factor drops are untouched, user-erasure routes still see and remove tombstones, and a write with no endpoint context degrades to a plain delete. sys-session.object.ts's field declarations were NOT changed; only its listViews, because retention would otherwise have made the Sessions grid show revoked sessions as live.",
      "seam_choice_rationale": "The adapter, same seam as #7725's PR #7796, for three measured reasons against better-auth 1.7.0-rc.2. (a) better-auth implements this exact substitution one layer up and we cannot reach it: internalAdapter.endPreservedSessions replaces the physical delete with updateMany({expiresAt: now}) via deleteManyWithHooks(..., {fn, executeMainFn: false}), but it is gated on secondaryStorage, which ObjectStack deliberately does not wire (#4772/#4785 -- it moves the session of record into the cache and makes every D4 control inert). Upstream's shape is right and unreachable; this is that shape at the layer we own. (b) databaseHooks.session.delete.before returning false was REFUSED: it aborts the delete, but getWithHooks then skips every delete.after hook, and @better-auth/oauth-provider (on by default here) registers session.delete.before/after to prepare and dispatch OIDC back-channel logout -- suppressing back-channel logout on an admin revoke trades an audit row for a security hole, and upstream's own preserve path is explicitly careful about this. (c) The route boundary was refused for adopt-membership.ts's reason: re-implementing these routes duplicates better-auth's ownership and freshness checks. The cause is read from getCurrentAuthContext().path -- api/dispatch.mjs runs every endpoint inside runWithEndpointContext(internalContext) where internalContext.path is the endpoint's declared path, so it is request-scoped, authoritative, and equally present for a programmatic auth.api.* call. No context (WebContainer AsyncLocalStorage, or a raw engine write) resolves to undefined and every rule degrades to today's plain delete -- the safe direction, since it loses the record and never keeps a session alive.",
      "gc_finding": "MEASURED in better-auth 1.7.0-rc.2 source, not assumed. There is NO scheduled sweeper of session rows anywhere in the library. The only expiry-driven collection is one line inside GET /get-session (api/routes/session.mjs): after `const session = await internalAdapter.findSession(token)`, if the row exists and `session.session.expiresAt < new Date()` it calls `internalAdapter.deleteSession(token)` 'to clean up the session'. That single line is exactly why the automatic path's stamps are best-effort today, and this repo already knew it -- session-of-record.test.ts's header says 'expiring a session in place makes better-auth's getSession clean the row up via deleteSession(token)'. Consequence for this card: a naive stamp-only fix would have been eaten the moment the revoked client next polled -- seconds for a browser session -- satisfying the letter of D4 and leaving the trail exactly as inert as before. The collector only fires on a row findSession RETURNED (`if (session) { ... deleteSession }`), so hiding tombstones from the adapter's session reads means the collector never sees one: the record survives AND the session dies harder than expiring it (findSession answers null rather than an expired row). Hiding also removes two problems a 'refuse the delete' rule would have created -- the delete.before/after hooks would re-fire on every stale-cookie poll, re-dispatching back-channel logout (the same repeat upstream guards against by restricting endPreservedSessions to live rows), and a later revoke-sessions sweep would re-date an older tombstone. Secondary measured fact: because that one collector is the only one, an ABANDONED session (cookie never presented again) is already immortal today -- so tombstone retention is not a new class of growth. It is still unbounded; see escalations.",
      "revoke_reason_values": "Two values, and the split is deliberate. `admin` for POST /admin/revoke-user-session and /admin/revoke-user-sessions; `user_revoked` for POST /revoke-session, /revoke-sessions and /revoke-other-sessions. Judged against the declared surface: revoked_at's description names the interactive cause `admin`, and revoke_reason is Field.text({maxLength: 64}) whose description gives an OPEN-ENDED list ('idle_timeout, absolute_max, concurrent_cap, ...'), so there is no closed enum or picklist to violate and no generated artifact to regenerate -- sys-session.object.ts is untouched either way. Within that freedom, one value would have been wrong rather than merely vague: the row carries no actor column, so revoke_reason is the ONLY thing that says who ended the session, and stamping `admin` on a user clicking 'sign out my other devices' names an actor class that took no action -- worse for an auditor than no record. Snake_case matches the three existing values. Deliberately NOT ledgered: /admin/ban-user and /admin/update-user{banned:true} (also admin-initiated, but a ban already records itself in far more detail on sys_user.banned/ban_reason/ban_expires, and the card scoped this to the three interactive-revoke families); /sign-out (not a revocation, no cause in the vocabulary, and #7675 owns whether logout earns a record); /admin/stop-impersonating (ends an impersonation session, not a real one).",
      "tests": "All on top of merged origin/main @098b62914, nothing behind. NEW: session-tombstone.test.ts, 18 cases, real better-auth pipeline through AuthManager.handleRequest (following session-of-record.test.ts -- Request objects in, better-auth's own cookie, better-auth's own revoke path). Every case pins BOTH directions: the row survives with its cause on it AND the cookie stops authenticating. Coverage: the three self-service revoke routes end to end; the re-date guard (a second sweep does not overwrite an earlier tombstone's revoked_at); the measured GC (five polls with the dead cookie -- all unauthenticated, none collects); the automatic path retained (idle_timeout tombstone survives three polls, and auth-manager.ts is unmodified); the converse (a merely EXPIRED, never-revoked row IS still collected -- otherwise 'nothing is ever deleted' would pass everything above); sign-out unchanged (row physically gone, no revoke_reason); sign-out on an already-tombstoned session keeps the record; live sessions never hidden; the path ledger for all five routes plus 'every other path and no path at all is a plain delete'; the erasure exemption for all three erasure paths; is_null-vs-undefined presence semantics; and a conformance case that every ledgered path is still in BETTER_AUTH_MOUNTED_SURFACE, so an upstream rename cannot make the fix silently inert. RUNS: `pnpm --filter @objectstack/plugin-auth test` -> 'Test Files 41 passed (41) / Tests 1015 passed (1015)'. `pnpm --filter @objectstack/platform-objects test` -> 'Test Files 13 passed (13) / Tests 311 passed (311)'. Typecheck both -> 'packages/platform-objects typecheck: Done / packages/plugins/plugin-auth typecheck: Done'. DOWNSTREAM CONSUMER SWEEP (prefix filter = consumers, per #6218): `pnpm --filter '...@objectstack/plugin-auth' --filter '...@objectstack/platform-objects' test` exited 0, including packages/qa/dogfood 'Test Files 91 passed | 1 skipped (92) / Tests 582 passed | 3 skipped (585)', packages/qa/http-conformance 72/72, packages/runtime 2033/2033, packages/cli 1182/1182, examples/app-showcase 189/189. GATES: check:nul-bytes OK (7139 files, no raw control bytes) plus a targeted grep self-scan of every changed file; check:i18n 'OK (9 package(s) -- all bundles in sync)'; check:i18n-coverage 'OK (12 config(s), 660 baselined untranslated string(s), none new)'; check:engine-double-contract 'OK -- 152 pinned, 133 in the DEBT ledger, 2 exempt'; eslint on all five changed source files exit 0. Build closure built first (`--filter '@objectstack/plugin-auth^...' build`) plus the 13 packages that had no dist, so no stale-artefact reds. NOTE: check:engine-double-contract went RED on the first run and caught a real hole -- the new fake's update() did not route through assertEngineUpdateDispatch, and since the whole fix IS an update, a fake looser than ObjectQLEngine.update could have greened a write the engine refuses. Fixed in 610c69555 rather than baselined; the sibling session-of-record.test.ts copy still carries its own #5480 DEBT entry, which a new double does not inherit.",
      "ablation": "Two ablations, direction predicted in writing BEFORE running, both matched exactly. (A) STAMP REMOVED -- reconcileSessionDelete always returns 'proceed with the delete'. Predicted red: the three revoke-route cases, the re-date guard, the poll-survival case, the ledger mapping; predicted green: everything about hiding, sign-out, natural expiry, and all of session-of-record.test.ts. Observed: 6 failed / 24 passed across the two files, exactly that set -- 'POST /revoke-session leaves the row behind', '/revoke-other-sessions', '/revoke-sessions', 'a second sweep does not re-date', 'the revoked client polling /get-session does not garbage-collect its own record', 'maps every interactive-revoke route to the reason it should record'. session-of-record.test.ts stayed 12/12 green, which is the positive evidence that the automatic paths are untouched. (B) HIDING REMOVED -- hideRevokedSessionRow/filterRevokedSessionRows made no-ops. Predicted red: both poll-survival cases, sign-out-after-tombstone, the hide/erasure unit case, and /revoke-session's trailing re-read (the isAuthenticated call in that test is itself the collecting poll); predicted GREEN despite the ablation: /revoke-other-sessions, /revoke-sessions and the re-date guard, because those are protected by reconcileSessionDelete's already-tombstoned branch rather than by hiding. Observed: 5 failed / 25 passed, exactly that partition -- the two halves of the fix are independently pinned and neither test set can pass on the other's behalf. Ablations were applied with an env-guarded early return inside the module and reverted with `git checkout HEAD -- <path>` (never git stash).",
      "neighbors": "#7770 (same package, ADR-0105 D7 collision guard): no contact. managed-extension-fields.test.ts and MANAGED_EXTENSION_FIELDS are untouched, and the tombstone needs NO whitelist widening -- verified rather than assumed: the write goes through the same withSystemContext-wrapped engine the adapter already uses, so it carries isSystem: true and the static-readonly UPDATE strip (#2948) does not run, which is exactly how auth-manager.ts's automatic path writes the same two readonly columns today. Only objectql-adapter.ts and index.ts are shared plugin-auth files and neither overlaps that card's surface. #7675 (decision box): no sys_audit_log writer built, no audit vocabulary touched -- the change stays inside sys_session and plugin-auth's revoke paths, and /sign-out (whose `logout` action value is one of #7675's writer-less enum entries) is explicitly pinned as unchanged by a test, so if #7675 is later ruled 'build', it can own that event without colliding with anything here. #7725/PR #7796 (adopt-membership.ts): read as instructed; its seam reasoning transferred for the route-boundary rejection but NOT for the hook rejection, which had to be re-derived here for a different reason (delete.after carries back-channel logout, where create had no equivalent). Merge queue: PR left as DRAFT, auto-merge NOT enabled, ready NOT flipped, per the dispatch.",
      "escalations": [
        "RETENTION WINDOW IS UNOWNED -- the design choice with a cost, surfaced rather than picked silently. Revoked sys_session rows are now retained INDEFINITELY: there is no TTL, no retention window and no sweeper for sys_session in this repo or in better-auth 1.7.0-rc.2 (measured above -- the library's only collector is the /get-session line, and it cannot reach a hidden tombstone). I judged this acceptable to ship because it is not a new CLASS of growth -- an abandoned session is already immortal for the same reason, so the table already accumulates dead rows at a strictly higher rate than interactive revocations occur -- and because the alternative (stamp and let it be collected) is the inert outcome the dispatch warned about. But 'keep the row long enough to audit' in the issue's own words implies a WINDOW, and choosing one is a product decision with a cost, not a fix: it needs a retention setting, a sweeper with an owner, and a view on whether an audit row about an ended session is retained on the same schedule as the audit log itself. Recommend a separate card; deliberately not invented here.",
        "IF #7675 IS RULED 'BUILD', REVISIT WHERE THIS EVENT LIVES. This PR records the revocation ON the sys_session row because that is where ADR-0069 D4 declares it lives. If session revocation later also becomes a sys_audit_log row, the two are complementary rather than duplicative (a tombstone is the ended session; an audit row is the action) -- but somebody should say so on purpose. The seam here is a single exported function, so redirecting or duplicating the event is cheap either way.",
        "SCOPE JUDGEMENT MADE, FLAGGING FOR REVIEW: I changed sys-session.object.ts's listViews (not its field declarations). Retention makes the Sessions grids list revoked sessions as if live, which would be a user-visible regression introduced by this change, so `My Sessions` and `All` now filter revoked_at is_null -- revoke_session still makes the row leave the grid exactly as when the row was deleted -- and a new `Revoked` view exposes the two columns, which also answers the issue's 'the fields appear in no listView'. If the PM considers the third view out of scope it can be dropped without touching the fix; the two filters cannot be, or the UX regresses."
      ],
      "deviations": [
        "Went beyond a bare stamp by also RETAINING the tombstone (hiding it from better-auth's session reads). Not silent scope creep: the dispatch's concern 1 said a stamp that is then collected leaves the trail as inert as today, and the measurement showed that is precisely what would have happened -- the collector fires on the revoked client's very next poll. The retention half is one rule at the same seam, ablated separately, and its cost is escalated above rather than buried.",
        "The legacy createObjectQLAdapter (deprecated, not used by AuthManager -- only by three test files) does NOT get the rule. Deliberate: it is the deprecated raw adapter and #7725 made the same choice for its create-side reconciliation. Noted so the asymmetry is on the record rather than discovered later.",
        "No end-to-end test drives the two /admin/* routes through a real admin session (they need an admin-roled caller and better-auth's admin plugin gate). Their path->reason mapping and their tombstone write are pinned at the seam instead, inside a real runWithEndpointContext scope, and the self-service routes are covered end to end -- so the shared machinery is proven against the real library and only the two routes' ADMISSION checks (better-auth's own, unmodified) are not re-proven here."
      ],
      "changeset": ".changeset/session-interactive-revoke-tombstone.md -- patch on @objectstack/plugin-auth and @objectstack/platform-objects, English, stating the user-visible change: the interactive revoke now records revoke_reason (user_revoked / admin) instead of deleting the row, revoked sessions are retained and no longer authenticate, the Sessions views are filtered live-only with a new Revoked view, and sign-out is unchanged."
    }

    Generated by Claude Code


    Generated by Claude Code

  6. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Review: ACCEPT — PR #7825 (PM session session_01BVc1ekPpi6yaWywAUhfzfd; adapter, object and module diffs read line-by-line).

    The GC measurement is the card. My dispatch flagged "a tombstone better-auth garbage-collects is still inert" as the thing to measure rather than assume, and the measurement inverted the obvious fix: there is no scheduled sweeper in better-auth 1.7.0-rc.2 — the library's only expiry collection is one line inside GET /get-session, which deletes a row findSession just returned. So a stamp-only fix would have been eaten on the revoked client's next poll, seconds later for a browser, satisfying D4's letter and leaving the trail exactly as inert as before. Hiding the tombstone from better-auth's own reads means the collector never sees one — and it ends the session harder than expiring it, because findSession answers null rather than an expired row.

    Scope of the hide verified in the diff, since that was the real risk. All four arms are inside createObjectQLAdapterFactory — better-auth's own read path only, never the generic data API. That is exactly why the list views had to move too, and it is what stops this from silently hiding rows from the platform. The revokedAtIsBorrowed handling is the kind of detail that decides whether this works: a caller projection omitting revoked_at would make every row look live, so it is borrowed for the check and deleted from the result before the caller sees it.

    Two seam rejections, both load-bearing and both re-derived rather than copied. session.delete.before → false was refused because aborting the delete skips every delete.after hook, and @better-auth/oauth-provider (on by default here) dispatches OIDC back-channel logout there — trading an audit row for a security hole. And upstream's own endPreservedSessions does precisely this substitution but is gated on secondaryStorage, which this platform deliberately does not wire (#4772/#4785); "upstream's shape is right and unreachable, so implement it at the layer we own" is the correct reading.

    The two reasons are right, and the argument for two is better than the argument for one. Stamping admin on a user signing out their own other device would not be vague, it would name an actor class that took no action — and since the row carries no actor column, revoke_reason is the only thing that says who ended the session. The field is free text with an open-ended documented list, so no enum is violated.

    Scope ruling on the third escalation — keep all three view changes. The two revoked_at is_null filters are not optional: without them this PR itself would make the Sessions grid show revoked sessions as live, a user-visible regression introduced by the fix. The new Revoked view is likewise in scope, not scope creep — the card explicitly names "the fields also appear in no listView" as part of the defect, so exposing them answers the card rather than extending it. Nothing to drop.

    Ablation is the strongest of the shift: two independent mutations, each predicted in writing first, each matching exactly, and the partition is the point — removing the stamp reds 6 (all in the stamping half), removing the hiding reds 5 (all in the retention half), with /revoke-other-sessions, /revoke-sessions and the re-date guard staying green under the second because they are protected by the reconcile guard rather than by hiding. Neither half can pass on the other's behalf. session-of-record.test.ts stayed 12/12 throughout, which is the positive evidence that the automatic paths are untouched.

    check:engine-double-contract earned its keep again — the new fake's update() did not route through assertEngineUpdateDispatch, and the whole fix is an update. Fixed rather than baselined. That is the third gate-caught double this shift.

    Both non-goals held: no sys_audit_log writer (and /sign-out is pinned unchanged by a test, so #7675 can own that event later without collision), and MANAGED_EXTENSION_FIELDS untouched — verified rather than asserted, since the tombstone write goes through the same withSystemContext engine the automatic path already uses.

    Retention escalation accepted and carded separately (see the new finding): revoked rows are now retained indefinitely, and no TTL, window or sweeper exists for sys_session here or upstream. The dev's argument that this is not a new class of growth holds — an abandoned session is already immortal by the same mechanism — but "keep the row long enough to audit" implies a window nobody owns, and inventing one inside a bug fix would have been the wrong move.

    ⚠️ Not arming. #7825 joins the lane's other five reviewed PRs parked behind the poisoned merge queue (#7793 / fix PR #7803); it will be flipped ready and armed automatically once that lands.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions