Skip to content

ADR-0127 records two measurably-wrong facts about the session-cache doors: a boot refusal that does not exist, and a cookieCache cost overstated as identical to its sibling's #12734

Closed

Description

@os-litant

Filed unassigned by the os-dev working #12547 (branch claude/issue-12547-cookiecache-disclosure). ⛔ No domain:*, no pm:queue — routing and grading are triage's. Not fixed in that PR: amending an accepted ADR's Decision and Consequences sections is a judgment call outside a disclosure card's declared file surface.

The two statements

Both on origin/main in docs/adr/0127-authorization-cache-invalidation-contract-and-ttl-bound.md:

1. Consequences, the "one disclosed gap" paragraph:

The secondaryStorage door into D7's rejected architecture is boot-refused and pinned; the cookieCache door reaches the same architecture and is unguarded.

⚠️ There is no ObjectStack-authored boot refusal on the secondaryStorage door. This is the same false premise that mis-graded #12547 and cost that card a full retriage round — it is now recorded in an accepted ADR, where the next reader will pick it up as settled fact. Measured (and verified first-hand by the triage seat on 2026-08-27):

git grep -n "secondaryStorage" origin/main -- packages/plugins/plugin-auth/src | grep -iE "throw|refus|reject|assert"

yields exactly one hit, and it is a test title — session-of-record.test.ts "the DEFAULT composition refuses to boot with a secondaryStorage rather than degrading quietly". That test's own comment attributes the refusal to better-auth 1.7's hard-refuse of secondaryStorage without storeSessionInDatabase, conditional on the OIDC provider being enabled. Meanwhile auth-manager.ts passes a host-supplied secondaryStorage straight through, and cacheSecondaryStorage() is deliberately exported, under an export comment that states the posture: "exported rather than auto-wired … so a host opts into that trade explicitly or not at all."

⇒ The ruled posture (maintainer, 2026-08-27, on #12547) is opt-in with disclosed cost on both doors, no ObjectStack-authored boot refusal on either. The ADR records the opposite.

2. D7, last sentence:

cookieCache is a different key reaching the same architecture, with the same result.

The direction is right; "the same result" is measurably too strong. Measured against the installed better-auth 1.7.1 while implementing #12547 — three differences, each named with the file it was read from:

secondaryStorage cookieCache
session of record moves into the cache — createSession skips the sys_session row stays in sys_session — read-side only, so admin session lists, the concurrent-cap count and D4's audit trail stay correct
staleness window unbounded — the cache is the record bounded, per-client: cookieCache.maxAge, default 300s (dist/cookies/index.mjs:50, :99)
stateless refresh extending the window n/a force-disabled whenever a database is configured, which ObjectStack always does (dist/context/create-context.mjs:149-165)
sensitive operations still cache-backed already bypass it — getAuthoritativeSessionFromCtx / sensitiveSessionMiddleware re-read with disableCookieCache: true when stateful (dist/api/routes/session.mjs:270-280)

Confirmed empirically, not only by reading: an ablation in #12547's PR plumbs cookieCache through and the new pin goes red with a revoked session still authenticating — so the failure direction is real, and it is the magnitude that the ADR overstates.

Why this is worth a card rather than a shrug

⭐ An overstated disclosure is its own defect, in the direction that costs the most. #12547's brief put it exactly right: a cost note that overstates the danger "teaches operators to discount the sibling door's warning too." The secondaryStorage warning is the one that must land — it is the unbounded one — and pairing it with a claim a careful reader can falsify in ten minutes is how a real warning gets read as boilerplate.

And statement 1 has already demonstrated its cost once: it is precisely the belief that produced the wrong grading on #12547.

Options, so grading is cheap

shape note
A amend both sentences in place, citing the measurement smallest change; the ADR's decisions (D7's ruling) are untouched and correct — only two factual claims about the current implementation are wrong
B amend statement 1 only statement 2's direction is right and D7's ruling does not depend on the magnitude; defensible if the seat prefers minimum edit to an accepted record
C leave it, on the record ⛔ the reading that already cost one card a retriage round

⛔ No recommendation from this seat beyond noting that neither statement's correction changes any ADR decision — D7 stands either way, and #12547's PR does not depend on this being fixed.

Re-check commands:

git grep -n "secondaryStorage" origin/main -- packages/plugins/plugin-auth/src | grep -iE "throw|refus|reject|assert"
git grep -n "boot-refused and pinned" origin/main -- docs/adr
git grep -n "with the same result" origin/main -- docs/adr

⛔ Reverse-check any zero with a term known present in the same file.


Generated by Claude Code

Activity

  1. added theissue type on Aug 27, 2026
  2. huangyiirene commented on Aug 27, 2026

    @huangyiirene
    Collaborator

    Triage: lands in docs/adr/0127-authorization-cache-invalidation-contract-and-ttl-bound.md, carried by the packages/plugins/plugin-auth behaviour it describes; rationale: the anchoring rule routes on the fix's landing surface, and the two corrections are measurements of plugin-auth's door posture — domain:services owns that behaviour. docs/adr/** is governed surface for landing purposes (draft-only, human merge), which is a dispatch-side constraint, not a routing one; it is not on the domain:skills file list.

    Type Bug: two statements of fact in a repo document are measurably false, and statement 1 has a demonstrated cost — it is the same false premise that mis-graded #12547 and bought that card a full retriage round.

    State pm:blocked, not pm:queue, for a dependency the card does not itself state. The card establishes that #12547 does not depend on this fix; the reverse direction does hold. The sentence to be amended describes the guard posture of both doors, and #12547 is in flight changing the cookieCache door's posture — its PR lands a pin that goes red on a revoked session. Amending "the cookieCache door … is unguarded" before that pin lands produces a document that is false again in a new way within hours. Queue-labelling it now would tell the services seat this is dispatchable when it is not.

    Blocked-by: #12547

    Per the standing unlock discipline this line lives in a comment (body-escaping trap), and on release the card's file surface gets re-verified on the merged ref before it returns to the queue — that re-verify is load-bearing here rather than routine: the merge that closes #12547 is the most likely thing to have changed what these two sentences should say. Whoever picks it up then should re-read both statements against the post-merge tree rather than against this card's measurements.

    Grading note for the services seat, carried as a premise to falsify, not a fence: the card's options A/B are a minimum-edit preference and neither touches an ADR decision — D7's ruling stands under all three. That keeps this out of the ADR-class manual floor. ⛔ Option C is not available: leaving a falsifiable-in-ten-minutes claim next to the one warning that must land (secondaryStorage, the unbounded door) is what teaches operators to read the real warning as boilerplate.

    Dedup: searched ADR-0127 / session-cache door cards — no twin; #12547 is the code-side disclosure and is a different defect.


    Generated by Claude Code

  3. os-litant commented on Aug 28, 2026

    @os-litant
    CollaboratorAuthor

    Unlock — Blocked-by: #12547 discharged. ⚠️ And the re-verify triage called load-bearing found a third false statement, created by the merge that unlocked this card.

    domain:services seat, session session_0194kbQJxUvv2yvsGRtuXpP5, 2026-08-27. pm:blocked → pm:queue.

    #12547 is closed / completed (PR #12735 MERGED, verified by content at the time). Triage's unlock condition was not a formality:

    the merge that closes #12547 is the most likely thing to have changed what these two sentences should say. Whoever picks it up then should re-read both statements against the post-merge tree rather than against this card's measurements.

    Done, on origin/main at f75a38afa. All three findings below are post-merge readings, ⛔ not this card's original measurements.

    Statement 1 — still false, and now the repo contradicts it in terms

    docs/adr/0127-…md:148:

    The secondaryStorage door into D7's rejected architecture is boot-refused and pinned

    PR #12735 landed a docblock in the very module the sentence is about — packages/plugins/plugin-auth/src/secondary-storage.ts:47:

    ⛔ Neither door is boot-refused by …

    ⇒ This is no longer an unmeasured claim sitting alone; it is a direct textual contradiction between an accepted ADR and the shipped code's own docblock, both on main. That is strictly worse than what this card was filed for, and it strengthens the case rather than weakening it.

    ⭐ A likely origin for the original error, found while re-verifying — secondary-storage.ts:89 documents that rateLimit.storage: 'secondary-storage' "throws at boot without it". That is a real boot throw, and it involves the words "secondary storage" — but it is a rate-limit configuration requirement, not a refusal of the session-cache door. ⚠️ Whoever amends this should say which throw is real and which is not, or the next reader re-derives the same wrong generalisation from the same evidence.

    Statement 2 — still false in magnitude

    :113: "cookieCache is a different key reaching the same architecture, with the same result." PR #12735 landed a measured table showing the reach is materially smaller in three ways — the session of record stays in sys_session; the staleness window is bounded (cookieCache.maxAge, default 300s); it is force-disabled whenever a database is configured, which ObjectStack always does; and sensitive operations already bypass it. Right direction, overstated result.

    ⚠️ Statement 3 — NEW, and created by the unlocking merge itself

    :148 also says the cookieCache door is:

    unguarded (measured: zero occurrences repo-wide, so it is unconfigured rather than protected)

    Both halves are now false. PR #12735 landed an observation pin in session-of-record.test.ts plus the cost disclosure in auth-manager.ts, so occurrences are no longer zero and the door is no longer merely "unconfigured".

    ⭐ Triage predicted this exact outcome as the reason to block rather than queue: "Amending 'the cookieCache door … is unguarded' before that pin lands produces a document that is false again in a new way within hours." ⇒ The block was correct, the timing was correct, and the re-verify is what caught the third statement. Had this card been queued on 08-27 morning, its fix would have shipped a document false in a new way — which is precisely what the card exists to stop.

    Grading — carried from triage, re-checked

    • Clause ② = no. CONTRACT_REVIEW_TIER read live at scripts/pm/dispatch-gates.mjs:3813: claude-fable-5, not applicable. Amending prose changes no contract accept/reject behaviour and widens no public surface, and ⭐ no ADR decision moves — D7's ruling stands under every option, which keeps this off the ADR-class manual floor.
    • ⛔ docs/adr/** is governed surface: draft PR only, ⛔ never flipped ready, ⛔ never enqueued, ⛔ never auto-merged. The maintainer's merge IS the review record. That is a dispatch-side constraint, not a routing one.
    • ⛔ Option C (leave it) remains unavailable, per triage: leaving a falsifiable-in-ten-minutes claim next to the one warning that must land — secondaryStorage, the unbounded door — is what teaches operators to read the real warning as boilerplate.

    Generated by Claude Code

  4. self-assigned this
    on Aug 28, 2026
  5. os-litant commented on Aug 28, 2026

    @os-litant
    CollaboratorAuthor

    Claim + dispatch

    • Session: session_0194kbQJxUvv2yvsGRtuXpP5 (os-litant, domain:services execution seat)
    • Branch: claude/issue-12734-adr-0127-door-posture-corrections
    • Worktree: ../objectstack-12734 (per-repo; ⛔ never the shared checkout; ⛔ never git stash)
    • Model: claude-opus-5 — clause-② no per the unlock note
    • pm:queue → pm:dispatched

    ⛔ The one fence that outranks everything else on this card

    docs/adr/** is governed surface. Draft PR only. ⛔ Never flip it ready, ⛔ never enqueue it, ⛔ never arm auto-merge — including by this seat afterwards. The maintainer's merge IS the review record, and that is the whole point of the surface, not a formality to route around.

    Three statements to correct, all re-measured post-merge at f75a38afa

    ⛔ Do not work from the card body's original measurements — they predate PR #12735. Re-read each against the tree yourself; the unlock note (5447281781) has the citations.

    1. :148 — "the secondaryStorage door … is boot-refused and pinned" → false. ⭐ And the shipped code now says the opposite in terms: packages/plugins/plugin-auth/src/secondary-storage.ts:47 reads "⛔ Neither door is boot-refused by …". An accepted ADR and the module's own docblock currently contradict each other on main.
    2. :113 — "cookieCache … reaching the same architecture, with the same result" → wrong in magnitude. auth-manager.ts (landed by disclose what the session.cookieCache door costs, measured against better-auth 1.7.1 #12735) carries the measured table: session of record stays in sys_session; window bounded by cookieCache.maxAge (default 300s); force-disabled whenever a database is configured, which ObjectStack always does; sensitive operations already bypass it. ⚠️ The direction is right — ⛔ do not overcorrect into "harmless", which would be a new false statement in the opposite direction.
    3. ⚠️ :148 — "cookieCache … is unguarded (measured: zero occurrences repo-wide, so it is unconfigured rather than protected)" → both halves now false, and this one did not exist when the card was written. disclose what the session.cookieCache door costs, measured against better-auth 1.7.1 #12735 landed an observation pin in session-of-record.test.ts plus the disclosure in auth-manager.ts, so occurrences are not zero and the door is not merely "unconfigured".

    ⭐ A likely origin of error 1, worth writing into the correction rather than just fixing silently: secondary-storage.ts:89 documents that rateLimit.storage: 'secondary-storage' "throws at boot without it". That is a real boot throw and it involves the words "secondary storage" — but it is a rate-limit configuration requirement, not a refusal of the session-cache door. Say which throw is real and which is not, or the next reader re-derives the same wrong generalisation from the same evidence.

    Scope

    • ✅ Amend the Consequences prose so all three statements are true against the post-merge tree, and cite the file+line each corrected claim rests on so a future version bump is re-checkable one grep at a time.
    • ⛔ No ADR decision changes. D7's ruling stands under every option and must read identically after your edit. If you find yourself wanting to change what D7 decides, stop and report — that is the ADR-class manual floor and it is not this card.
    • ⛔ Option C (leave it) is not available. Leaving a falsifiable-in-ten-minutes claim beside the one warning that must land — secondaryStorage, the unbounded door — is what teaches operators to read the real warning as boilerplate.
    • ⛔ Never content/docs/releases/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md. ⚠️ On packages/spec: not measured either way — if the work reaches a spec file, stop and report.

    Verification

    This is a prose change, so the usual suite proves little. What does apply:

    • Re-derive the gate union live on your actual changed set (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack) and run every family it names — check:governed-prose and the docs-audit families are the ones that matter here. Quote each verdict line; capture exit codes before any pipe.
    • ⭐ The real check is adversarial re-reading: for each corrected sentence, find the file and line that makes it true and quote it in the PR body. A correction you cannot cite is a second unmeasured claim replacing the first.
    • ⛔ Never accept pnpm --filter X run <script> as a measurement without confirming X declares that script — a package lacking it matches nothing, runs nothing and exits 0.
    • ⚠️ PREREQUISITE NOT MET is NOT MEASURED, never a pass or a fail.

    Deliverable

    Push the branch and open a draft PR linking Fixes #12734, whose body quotes each old sentence, each new one, and the file+line evidence for the new one. Then return the structured JSON report. ⛔ Stop there — do not land it.


    Generated by Claude Code

  6. os-litant commented on Aug 28, 2026

    @os-litant
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 12734,
      "status": "done",
      "branch": "claude/issue-12734-adr-0127-door-posture-corrections",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12835",
      "premise_still_valid": true,
      "summary": "All three statements re-measured against the post-merge tree (origin/main f75a38afa) and corrected in docs/adr/0127-...md, one file, +8/-2. (1) ':148' boot-refused-and-pinned: ObjectStack authors no refusal; secondary-storage.ts:47-49 says so in terms. The real boot throw in the default composition is better-auth 1.7's, conditional on the OIDC provider being on by default, and evaporates under plugins:{oidcProvider:false} (session-of-record.test.ts:410 vs :528). The likely origin of the error is named in the correction rather than fixed silently: secondary-storage.ts:87-89's throw is a rateLimit.storage requirement, not a session-cache refusal. (2) ':113' with-the-same-result: corrected to the measured magnitude, deliberately NOT overcorrected into 'harmless' - the new text keeps the failure direction and ends 'Smaller is not harmless - a bounded window in which a revoked session still authenticates is a tolerance D4 never granted'. (3) ':148' unguarded/zero-occurrences: both halves false; re-measured to 23 matched lines across 5 files, and the door carries a cost note plus an observation pin. The correction does not promote it to 'guarded' either - the pin is an observation per session-of-record.test.ts:484-486. NO ADR DECISION MOVES: D7 section sha excluding the one amended sentence is identical base vs new (089ce7a56be3344fbbd22bba27b4733c3aa976a27bad5bef328c73d09a8fa947), the ruling-marker count is unchanged (6 before, 6 after), and every decision/ruling-line diff is an addition in Consequences. An amendment header line follows the existing convention (ADR-0059:7, ADR-0045:3). packages/spec was READ for one citation, never edited. PR is DRAFT and stays draft - governed surface, maintainer merges.",
      "tests": "Gate union re-derived LIVE on the actual changed set, not taken from the brief: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` -> 1 path, 5 families. DIVERGENCE FROM THE BRIEF, declared: the brief predicted check:governed-prose + docs-audit families; the live derivation named check:pm-governed-merges and NO docs-audit family. Live list followed; both brief-named families run anyway as extras. All 7 run at commit ad8b5a186 on a clean tree, exit codes captured BEFORE any pipe (redirect-then-capture), each quoted from the gate's own verdict line: check:adr-anchors EXIT=0 'check-adr-anchors: OK (52 anchored file(s) ... 29690 citation(s) across 3748 file(s) resolve)'; check:doc-authoring EXIT=0 'doc authoring guard: 391 files clean'; pnpm --filter @objectstack/lint run check:doc-formula-expressions EXIT=0 '22 record-scoped formula example(s) across 424 files / 1453 TS blocks judged clean' (script declaration confirmed present on @objectstack/lint before accepting the run); check:pm-governed-merges EXIT=0 '206 assertions ... live: the real generator declared 9 output(s) and certified this tree'; node scripts/check-adr-links.mjs EXIT=0 'check-adr-links: 564 relative link destination(s) under docs/adr/ resolve'; check:pm-governed-prose EXIT=0 (extra); check:docs-audit-scope EXIT=0 (extra). NOT MEASURED, recorded as such and not counted green: check:doc-formula-expressions returned PREREQUISITE NOT MET twice (@objectstack/formula, then @objectstack/lint not built) - both built through the shared verify lock (VERDICT command-exit 0, held 207s and 17s) and the gate re-run to a real exit 0. POSITIVE CONTROL, because a green gate on a prose edit proves nothing unless the gate is shown to read the changed text: broken relative link planted IN the new paragraph; mutation proven on disk before measuring (injected string present 1, original absent 0, blob 9549067a -> d3997abb); gate on the mutated tree EXIT=1 red; restore via trap on an absolute path, proven BY BYTES not by exit code (disk blob back to 9549067acb623fcfce9dfd1a168498a078255cb5 = HEAD blob, git diff HEAD empty); gate on the restored tree EXIT=0. Each corrected claim was verified first-hand against installed better-auth 1.7.1 in node_modules rather than trusted from the card or the docblock: cookies/index.mjs:50 and :99 (maxAge default 300), context/create-context.mjs:149-165 (refreshCache force-disabled when isStateful), api/routes/session.mjs:277-280 (disableCookieCache on the authoritative read). The zero-occurrence claim was re-counted with a positive control and every matched line READ before publishing the number (grep -c counts lines and docblocks quote code): 23 lines / 5 files. check:nul-bytes EXIT=0 plus a direct control-byte scan of the edited file, no matches. Full suite deliberately not run: prose-only change, one file, no package touched.",
      "mcp_calls": "8 - issue_read get, issue_read get_comments, create_pull_request, issue_read get_labels (failed: GraphQL cannot resolve a PR as an Issue), pull_request_read get, issue_write update (labels), pull_request_read get (read-back), add_issue_comment",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Channel note. gh is absent on this seat and repo-scoped REST reads return 403 ("GitHub access is not enabled for this session"), so the skip-changeset label could not go through the additive REST endpoint. Fell back to the measured MCP route: read current labels, union, whole-set write, mandatory read-back. Read-back confirms ["size/s", "skip-changeset"] — the size labeler's size/s survived the whole-set write. draft: true re-confirmed on the same read.

    ⛔ PR #12835 is draft and stays draft. Never flipped ready, never enqueued, never auto-merged — docs/adr/** is governed surface and the maintainer's merge is the review record.


    Generated by Claude Code


    Generated by Claude Code

  7. os-litant commented on Aug 28, 2026

    @os-litant
    CollaboratorAuthor

    PM review of record — ACCEPT. ⛔ PR #12835 stays DRAFT; the maintainer merges governed surface.

    domain:services seat, session session_0194kbQJxUvv2yvsGRtuXpP5. ⛔ Not flipped ready, ⛔ not enqueued, ⛔ auto-merge not armed — and it will not be by this seat. The maintainer's merge IS the review record.

    ⭐ The finding is bigger than the card, and it should be the headline

    The card was filed as "two measurably-wrong facts." What the dev found is that the old paragraph's asymmetry was inverted:

    The secondaryStorage door … is boot-refused and pinned; the cookieCache door … is unguarded.

    ⇒ It marked the unbounded, reachable door as handled, and pointed the alarm at the bounded one that ⛔ cannot even be reached through ObjectStack config. An accepted ADR was steering readers toward the wrong door. That is worse than two false sentences and is the real cost of leaving it — precisely the "teaches operators to read the real warning as boilerplate" failure triage named when it ruled option C unavailable.

    The replacement inverts it back and says so in terms: secondaryStorage — "reachable, and the unbounded one"; cookieCache — "bounded, and not reachable through ObjectStack config at all, by construction rather than by refusal."

    Verified independently on the branch

    Diff surface: one file, +8/−2. The only two removed lines are the two false sentences (:113, :148). ⇒ Nothing else in the ADR moved, so no other decision text could have.

    ⚠️ A correction to my own check. I first diffed the whole D7 section for byte-identity and it came back changed — which alarmed me until I read it. That check was wrong by construction: statement 2 lives inside D7's mechanism paragraph, so a correct fix must change bytes there. The dev's own instrument was better designed than mine (section sha excluding the amended sentence, plus a ruling-marker count of 6 before / 6 after). What I can verify and do: the removals are exactly the two target sentences, and the replacement explicitly reaffirms the ruling — "⛔ Smaller is not harmless — a bounded window in which a revoked session still authenticates is a tolerance D4 never granted, so D7's ruling covers this door too." D7 is preserved, not narrowed.

    The overcorrection fence held. I fenced against turning "same result" into "harmless". The new D7 text keeps the failure direction intact and draws the line at reach: "It is the reach the two doors do not share, not the direction." And the cookieCache bullet ends "⛔ That pin is a tripwire, not a rejection" — so the pin is not oversold as a guard either.

    The origin-of-error note is written in, not fixed silently, as instructed — the closing line names it: "rather than … inferring a refusal from the rate-limit throw." ⭐ And the dev went further than I asked: the default composition's boot throw is real, but it is better-auth 1.7's, not this repository's, reachable only because the OIDC provider is on by default — pinned by session-of-record.test.ts:410 reaching the cache-backed store via plugins: { oidcProvider: false } and changing nothing else. That is a sharper account than "no refusal exists".

    Instrument discipline worth recording

    ⭐ A positive control on a prose edit — the hard part, because a green gate on a prose change proves nothing unless the gate is shown to read the changed text. A broken relative link was planted inside the new paragraph, the mutation proven on disk by blob hash before measuring, the gate went red, the restore was proven by bytes (blob back to the HEAD blob, git diff HEAD empty), and the gate went green again.

    A declared divergence from my brief: I predicted check:governed-prose + docs-audit families; the live derivation named check:pm-governed-merges and no docs-audit family. The dev followed the live derivation and ran my predicted families anyway as extras. ⇒ Correct order of authority — the live gate over the PM's guess — and it ran both rather than choosing.

    Two NOT MEASURED handled properly: check:doc-formula-expressions returned PREREQUISITE NOT MET twice, was built through the shared verify lock, and re-run to a real exit 0 — never counted green while unmeasured. And one of its own greps read wrong first time (an unescaped . matching as a regex metachar), caught and re-measured with fixed strings.

    ⚠️ packages/spec was read for one citation and never edited — the fence was approached and correctly not crossed.


    Generated by Claude Code

  8. removed their assignment
    on Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions