ActiveOrganizationStorage.clear() swallows a failed removeItem, so a cleared org can stay readable — asymmetry noted, reachability NOT demonstrated

Noticed while pinning the clear() path for #5703. Filing it plainly for triage to grade: I verified the code shape by reading it, but I could not identify a concrete browser state that reaches it, so this is an unhandled asymmetry rather than a measured defect. Grading it as such is the point of filing.

The shape

packages/auth/src/createAuthenticatedFetch.ts:

clear(): void {
  this._memoryValue = null;
  try {
    if (typeof localStorage !== 'undefined') {
      localStorage.removeItem(ACTIVE_ORG_STORAGE_KEY);
    }
  } catch { /* SSR / test */ }
},

If removeItem throws, the catch swallows it and the persisted key survives. get() prefers a non-null localStorage read (that ordering is the #5703 fix), so it keeps returning the org that clear() was supposed to remove. Sign-out is one of clear()'s callers, so the failure mode is "sign-out does not stick" — and it is silent, exactly like the defect on #5703.

Note this is not a regression from #5703: before that fix get() returned the localStorage read unconditionally, so the same stale value came back. The read ordering is unchanged for a non-null persisted read. What #5703 did change is that clear() nulling _memoryValue became load-bearing, which is what made this neighbouring path worth looking at.

Why reachability is not demonstrated

The state required is getItem succeeding while removeItem throws. The obvious candidates do not produce it:

Safari private browsing / quota-exhausted origin — setItem throws; getItem and removeItem work. This is the ActiveOrganizationStorage's in-memory fallback is unreachable when localStorage exists but rejects writes — the tenant header is then never stamped for the whole session #5703 state, not this one.
Storage access revoked in an iframe — every operation throws, getItem included. get() then takes its own catch and falls back to _memoryValue, which clear() nulled. That path is already safe.

That leaves wrapped/proxied localStorage (an extension, a polyfill) as the only candidate I can name, which is speculation. A reader who can name a real one should say so on this card — it would change the grade.

Possible shapes, for triage rather than as a decision

Leave it. An undemonstrated state may not be worth code.
Have clear() verify the removal and fall back to a tombstone in _memoryValue so get() refuses to answer with the stale persisted value.
Surface the failure rather than swallowing it, so a broken clear() is loud instead of silent.

Option 2 is the only one that keeps sign-out sticky, but it adds a third state to a two-state field; option 1 is defensible if reachability stays hypothetical. Not obvious enough to pick without a ruling.

Scope note

Related but distinct from #5664, which is about auth-active-organization-id not being namespaced per user and something rewriting it after removal. Completing #5664 would not close this: a namespaced key whose removeItem throws leaves exactly the same residue. Filed standalone rather than as a sub-issue for that reason.

Unassigned. Found while implementing #5703; deliberately not fixed there, since the correct shape is not pinned by existing evidence.

Triage (first touch): pm:queue + domain:ui.

Landing: packages/auth/src/createAuthenticatedFetch.ts — ActiveOrganizationStorage.clear()'s bare catch.

⚠️ Graded pm:queue despite the filer's explicit "reachability NOT demonstrated", and the reason should be stated so the grading is auditable: the remedy restores an invariant (a clear() that reports failure instead of swallowing it) and needs no product decision, which is the specific case the protocol says not to escalate and not to park. The filer was right to refuse to claim a defect they had not reproduced — that honesty is what makes the card usable — but "we could not reach it from a browser" is not a reason to leave a sign-out path silently lossy.

⛔ Acceptance must NOT require reproducing a browser state. Whoever takes this should not spend a run hunting a removeItem throw; the deliverable is that the failure stops being silent, plus a unit-level pin that a throwing localStorage no longer leaves get() returning the cleared org.

objectui three-stream split (maintainer 2026-08-21). Routed via the maintainer direct-dispatch channel, this session, verbatim: 「objectui 有 79 张卡在车道视图之外（占该仓 35%），你帮我分批处理」 · 「继续批 2 批 3」. PM session session_0124Qg8rLvpXnQDwCmpKUmaJ. Not a Routine triage fire — the triage seat may re-grade.

Generated by Claude Code

Claim: domain:ui execution seat (seat post #5560), round 22, slot 2 of 3.

Session: session_01EuPCi56cnGyykygi3z9w4m
Branch: claude/issue-5731-clear-must-not-swallow
Worktree: dedicated, off origin/main at 9850c6e4e
Domain: domain:ui · Tier: claude-opus-5
Clause-②: no — restores an invariant on an internal storage helper. No contract accept/reject behaviour moves, no published surface widens.

⛔ THE CARD AND ITS TRIAGE LINE BOTH NAME A FILE THAT NO LONGER HOLDS THE CODE. Both say packages/auth/src/createAuthenticatedFetch.ts. That was true when they were written; it is not true now. #5664 / PR #5744 moved ActiveOrganizationStorage out, landed 03:56Z today. On origin/main the clear() you are fixing lives in:

packages/auth/src/ActiveOrganizationStorage.ts

createAuthenticatedFetch.ts re-exports it. Re-read the shape before touching anything — it has changed twice in six hours (#5703's memory fallback, then #5664's per-user scoping and SessionUserScope), and clear() in particular now has a purge sibling next to it.

File surface granted (write):

packages/auth/src/ActiveOrganizationStorage.ts
packages/auth/src/__tests__/**
.changeset/*

⛔ packages/auth/src/createAuthenticatedFetch.ts is read-only — the re-export must keep resolving; if your change needs its signature to move, stop and report.

Serial constraints: ⛔ apps/console/.env.* · examples/console-starter/** · apps/console/vite.config.ts held by #5745 (slot 1); packages/app-shell/src/views/metadata-admin/** and packages/plugin-designer/** held by #4676 (slot 3).

Two existing pins are in your blast radius and both are load-bearing — activeOrgStorageFallback-5703.test.tsx and sessionUserChangePurge-5664.test.tsx. If either turns red, that is a signal, not a chore: #5703 pinned that clear() nulls _memoryValue before touching storage, and #5664 pinned that a session-user change drops the previous user's state. Neither may weaken.

⛔ Triage's acceptance bar, restated because it inverts the usual one: "Acceptance must NOT require reproducing a browser state. Whoever takes this should not spend a run hunting a removeItem throw; the deliverable is that the failure stops being silent, plus a unit-level pin that a throwing localStorage no longer leaves get() returning the cleared org."

The filer explicitly refused to claim a defect they had not reproduced, and triage graded it anyway with the reason recorded. Do not spend the run trying to earn a reproduction. A throwing localStorage double is the instrument.

Generated by Claude Code

fix(auth): stop `ActiveOrganizationStorage.clear()` swallowing a fail…

os-dev-report

{
  "issue": 5731,
  "status": "done",
  "branch": "claude/issue-5731-clear-must-not-swallow",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5764",
  "premise_still_valid": true,
  "summary": "The card's premise holds; only its FILE POINTER was stale. Both the card and the triage line name packages/auth/src/createAuthenticatedFetch.ts; #5664 / PR #5744 moved ActiveOrganizationStorage into its own module at 03:56Z the same day, and the asymmetry was re-derived on the post-#5664 code rather than taken from the card's quote. It survives the move intact: clear() nulls _memoryValue unconditionally but removed the persisted key best-effort inside a catch that discarded the failure, while get() prefers a non-null persisted read (the #5703 read order) — so the memory null is SHADOWED by a surviving persisted value and sign-out silently does not stick. Fix, in three parts. (1) removePersisted now judges by READ-BACK rather than by catching the throw: get() can only answer with what getItem hands back, so 'did the removal stick' IS 'is the key still readable'. That is wider in the right direction (a wrapped/proxied localStorage whose removeItem is a silent no-op never throws and leaves identical residue — the only candidate the filer could name, now covered) and narrower in the right direction (SSR and the fully-throwing partitioned iframe have nothing readable to resurrect, were already safe, and are not reported). (2) A key whose removal cannot be verified is quarantined in memory for the page-load: get() skips its persisted branch and answers from _memoryValue, which clear() just nulled and which a later set() refills with the value that write was meant to persist — correct in both directions with no release step of its own; the quarantine IS released as soon as a removal on that key sticks. (3) The failure is reported via console.warn, not thrown and not returned. Callers were enumerated FIRST and that decided the shape: sign-out's purgeSignedOutClientCaches, switchOrganization, deleteOrganization, leaveOrganization, and purgePreviousUserClientState via SessionUserScope.adopt all arrive after the transition they follow up on has already happened, so not one can act on a storage failure — and a boolean every caller ignores reads as handled when it is not. Rejected third shape: re-writing the key with an empty value, which relocates the fix into consumers' truthiness checks (AGENTS.md #0.1) and leaves a signed-out browser holding a live key. Per triage, NO browser state was reproduced — a localStorage double is the instrument. createAuthenticatedFetch.ts is untouched and its re-export still resolves. How the DELIBERATE swallow was told apart from the accidental one, since the next reader meets both: not 'which one catches' but 'does the swallow leave a path that still upholds the method's postcondition'. set()'s does — the memory copy holds the value and get()'s fallback is built to consult it (#5703's design, untouched, and pinned by a case here). clear()'s did not — nulling memory cannot uphold 'get() answers null' when the surviving persisted value outranks it.",
  "tests": "Union run at final commit 0610ca74b, clean tree, all exit codes captured before any pipe. vitest run packages/auth/ -> exit 0, 'Test Files 24 passed (24) / Tests 246 passed (246)'. Downstream consumers (filter direction: DEPENDENTS of @object-ui/auth = app-shell/console/2 examples; app-shell is the only code consumer, via MetadataProvider) — vitest run of MetadataProvider.firstBootOrgScope + .orgScopedCache + .crossPrincipalSeed -> exit 0, 3 files / 13 tests passed. pnpm --filter @object-ui/auth type-check -> exit 0 (runs BOTH tsc --noEmit and tsc -p tsconfig.test.json, so the new test file type-checks; note objectui spells the script type-check with a hyphen). pnpm --filter @object-ui/auth lint -> exit 0, eslint's own verdict line 'x 29 problems (0 errors, 29 warnings)', all pre-existing (AuthProvider/LoginForm/createAuthClient) and none in the changed files. Repo gates, each quoting its own printed verdict: check-control-bytes 'OK (scanned 4805 tracked text file(s))'; check-changeset-presence '2 source file(s) of 1 released package(s) changed ... declares 1 changeset(s)'; check-changeset-no-major 'No changeset declares a major bump'; check-changeset-fixed; check-lint-coverage '46/46 packages linted, 0 with outstanding errors'; check-type-check-coverage '41/41 packages compile their tests'; check-package-self-import; check-phantom-dependencies — all exit 0. NEW PIN: packages/auth/src/__tests__/activeOrgClearRemovalFailure-5731.test.tsx, 10 cases, headline assertions are OUTCOMES (get() is null; X-Tenant-ID absent from the wire), not 'did it warn'. Both load-bearing pins in the blast radius are green and unmodified — activeOrgStorageFallback-5703.test.tsx (9 cases) and sessionUserChangePurge-5664.test.tsx (12 cases). ABLATION — committed first; no dist rebuild is in the loop and here is why that is a real reading rather than a skipped step: @object-ui/auth is vitest-aliased to packages/auth/src in vitest.config.mts and the test imports ../ActiveOrganizationStorage relatively, so both legs and the consumer tests execute the edited SOURCE; no dist/ sits in the resolution path, so the stale-artifact false green cannot occur. Each mutation was confirmed ON DISK by single-line anchored counts in BOTH directions (the python mutator aborts unless its anchor hits exactly once — an editing tool's exit code is not evidence), and both legs ran under trap '... restore ...' EXIT INT TERM. LEG A, delete the get() quarantine guard: _unremovedKeys.has(key) counted 1 -> 0 -> 1 (baseline/mutated/restored); result 4 failed / 6 passed. LEG B, removePersisted stops verifying (return true): 'return readPersisted(key) === null;' 1 -> 0 -> 1 and bare 'return true;' 0 -> 1 -> 0; result 7 failed / 3 passed. Restored tree: counts back to baseline, git status clean, 10 passed. Direction observed, stated because it was NOT uniform: under A the three outcome pins die while the MECHANISM pins SURVIVE — clear() still populates the quarantine correctly, so the bookkeeping alone does nothing and it is the get() guard that converts it into the outcome; under B the mechanism pins die too, because verification is upstream of everything. THREE CASES SURVIVE BOTH LEGS AND THAT IS THEIR PURPOSE: 'stays quiet when the removal sticks', 'stays quiet when there is no storage at all', and the set()-swallow case. They assert behaviour on stores where nothing fails and on the swallow this card deliberately leaves alone; had any died, the change would have been over-reaching — reporting on a healthy store, crying wolf on SSR, or breaking #5703's memory fallback. Their survival is what shows the change is scoped to MEASURED failures. NARROWED LINT, DECLARED (pnpm lint is turbo run lint = per-package eslint .): the run above covers the whole @object-ui/auth package, a superset of the diff, measured at 46 files from --format json with the population decided by eslint's own config, not by my guess. Invariance for untouched files: eslint.config.js extends tseslint.configs.recommended, NOT the type-checked preset, and carries no projectService / parserOptions / project: — each grepped with the exit code captured before any pipe (grep_exit=1, 0 hits) against control terms in the same file that DO hit (languageOptions 1, rules 10, files: 6), so the zero is a controlled reading and not an unrun command. With no type-aware linting a diff confined to packages/auth cannot move any untouched file's verdict. The rest of the farm is CI's run. One instrument error found and corrected mid-run rather than reported as a result: the first restore-confirmation used a multi-line grep -F anti-anchor, which grep splits into independent per-line patterns, so it read as a failed restore; re-measured with single-line anchors, which is where the counts above come from.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #5763 (open, unassigned, no labels — left for triage to grade): sweepStore's try wraps the WHOLE loop, so one throwing removeItem aborts the walk and the remaining keys are never swept, while purgePreviousUserClientState returns normally and its caller believes the purge completed. Same defect class as this card (a swallowed removal failure) but a different function with different callers, on the SIGN-IN path, and it is what #5664 calls the load-bearing allowlist part — a partial sweep is a partial allowlist, with arbitrary residue since survival depends on Object.keys order. Not fixed in this PR on purpose: it needs a partial-failure test of its own and its behaviour is pinned by sessionUserChangePurge-5664.test.tsx, so keeping the two apart keeps both reviewable. Reachability is undemonstrated there for the same reasons as here, and I recorded that rather than grading it myself."
  ]
}

Generated by Claude Code

ACCEPT — landing. PR #5764.

Gates by name at 05:43Z (PR started 05:31:47Z): 19 success + 3 skipped = 22. Zero in_progress, zero failure. (Build Docs 12 s — the path-filtered skip.)

Fence: clean. 3 files, all inside the grant. Two controls, both verified by hash rather than by report:

packages/auth/src/createAuthenticatedFetch.ts — byte-identical. Its re-export still resolves, so Metadata type: 'api' actions send the Bearer token and X-Tenant-ID to absolute third-party URLs — the #2725 sameOriginOnly mitigation was never applied to this lane #5702's pushed branch is unaffected.
Both load-bearing pins UNMODIFIED — activeOrgStorageFallback-5703.test.tsx and sessionUserChangePurge-5664.test.tsx. That is the strongest available outcome: the change did not need to touch them, so a week-old guarantee and a two-hour-old one both stand untouched and green. A sibling's pin quietly rewritten to accommodate a new change is the failure mode this lane watches for; it did not happen.

The fix is better than the card asked for, in a way worth naming

The card framed this as "a catch swallows a throw". The implementation judges by read-back instead — removePersisted():171 returns readPersisted(key) === null — and that is not a stylistic choice, it changes what the fix covers in both directions:

Wider where it should be: a wrapped or proxied localStorage whose removeItem is a silent no-op never throws and leaves identical residue. Catching the throw would never have seen it. That was the only concrete candidate the filer could name, and it is now covered.
Narrower where it should be: SSR and a fully-throwing partitioned iframe have nothing readable to resurrect — already safe, and now correctly not reported. No crying wolf.

get() can only ever answer with what getItem hands back, so "did the removal stick" is "is the key still readable". Deriving the predicate from the consumer rather than from the exception is the right shape.

Zone 2b was answered by enumeration, and the enumeration decided the design

I asked for clear()'s callers to be listed before choosing between throw / boolean / report. They were: sign-out's purgeSignedOutClientCaches, switchOrganization, deleteOrganization, leaveOrganization, and purgePreviousUserClientState via SessionUserScope.adopt. Every one arrives after the transition it is following up on has already happened — so not one can act on a storage failure, and "a boolean every caller ignores reads as handled when it is not." Hence console.warn.

The third shape was considered and rejected on doctrine, not taste: re-writing the key with an empty value relocates the fix into consumers' truthiness checks (AGENTS.md #0.1) and leaves a signed-out browser holding a live key.

Zone 2c produced a principle rather than a list

I asked how the deliberate swallow in set()/writePersisted (#5703's memory-fallback design) was told apart from the accidental one in clear(). The answer is the reusable form:

not "which one catches", but "does the swallow leave a path that still upholds the method's postcondition"

set()'s does — the memory copy holds the value and get()'s fallback is built to consult it. clear()'s did not — nulling memory cannot uphold "get() answers null" when the surviving persisted value outranks it. That is the sentence the next reader needs, and it is now on the record.

The ablations, and specifically what survived

Ten new cases whose headline assertions are outcomes (get() is null; X-Tenant-ID absent from the wire), not "did it warn".

Leg A (delete the get() quarantine guard): 4 red / 6 green — the three outcome pins die while the mechanism pins survive, because clear() still populates the quarantine correctly and it is the get() guard that converts bookkeeping into outcome.
Leg B (removePersisted stops verifying): 7 red / 3 green — the mechanism pins die too, because verification is upstream of everything.

Three cases survive both legs, and that is their purpose: "stays quiet when the removal sticks", "stays quiet when there is no storage at all", and the set()-swallow case. Had any of them died, the change would have been over-reaching — reporting on a healthy store, crying wolf on SSR, or breaking #5703's memory fallback. Their survival is the evidence that the change is scoped to measured failures, and it is reported that way rather than as three passing tests.

One instrument error was found and corrected mid-run rather than shipped as a result: a multi-line grep -F anti-anchor, which grep splits into independent per-line patterns and which therefore read as a failed restore. Re-measured with single-line anchors.

Per triage's bar, no browser state was reproduced and none was owed — a localStorage double is the instrument, and the run was not spent hunting a removeItem throw.

Filed out of scope: #5763 — sweepStore's try wraps the whole loop, so one throwing removeItem aborts the walk and the remaining keys are never swept while purgePreviousUserClientState returns normally and its caller believes the purge completed. Same defect class, different function, on the sign-in path, and it is exactly the load-bearing allowlist half of #5664 — a partial sweep is a partial allowlist, with residue that depends on Object.keys order. Correctly kept out of this PR: it needs a partial-failure test of its own, and its behaviour is pinned by sessionUserChangePurge-5664.test.tsx. Reachability recorded as undemonstrated rather than graded.

Flipping ready → auto-merge (SQUASH). Landing verification against origin/main follows.

Generated by Claude Code

fix(auth): stop `ActiveOrganizationStorage.clear()` swallowing a fail…

fix(auth): stop sweepStore aborting the sign-in purge on one throwing…

Navigation Menu

Repository navigation

`ActiveOrganizationStorage.clear()` swallows a failed `removeItem`, so a cleared org can stay readable — asymmetry noted, reachability NOT demonstrated #5731

The shape

Why reachability is not demonstrated

Possible shapes, for triage rather than as a decision

Scope note

ACCEPT — landing. PR #5764.

The fix is better than the card asked for, in a way worth naming

Zone 2b was answered by enumeration, and the enumeration decided the design

Zone 2c produced a principle rather than a list

The ablations, and specifically what survived

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ActiveOrganizationStorage.clear() swallows a failed removeItem, so a cleared org can stay readable — asymmetry noted, reachability NOT demonstrated #5731

Description

The shape

Why reachability is not demonstrated

Possible shapes, for triage rather than as a decision

Scope note

Activity

os-zhuang commented on Aug 23, 2026

os-zhuang commented on Aug 23, 2026

os-zhuang commented on Aug 23, 2026

os-zhuang commented on Aug 23, 2026

ACCEPT — landing. PR #5764.

The fix is better than the card asked for, in a way worth naming

Zone 2b was answered by enumeration, and the enumeration decided the design

Zone 2c produced a principle rather than a list

The ablations, and specifically what survived

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`ActiveOrganizationStorage.clear()` swallows a failed `removeItem`, so a cleared org can stay readable — asymmetry noted, reachability NOT demonstrated #5731