Skip to content

Session staleness 1 - #329

Merged
bourgeoa merged 11 commits into
stagingfrom
session-staleness-1
Sep 20, 2026
Merged

bourgeoa merged 11 commits into
stagingfrom
session-staleness-1

Conversation

@bourgeoa

Copy link
Copy Markdown
Contributor

What this is

Rebuilds the session/identity handling around ONE per-session identity state: a single record per session replaces the per-call-site predicates that could each disagree about who the current identity is. Tracks solid-logic 6.0.0-1.

The state (src/authSession/identityState.ts)

  • one record per session: the session's own view (isActive/webId), the cleared mark for a backing store that lost the session, and the cookie-probed identity;
  • every observation goes through it — session events, the setTokenDetails wrap (uvdsl does not announce an active→active WebID change), the refocus resync, the cookie probe result;
  • transitions are derived from previous/next snapshots (classifyTransition, identityReplaced) — no flags to keep in sync, and no event when nothing changed;
  • the record carries a version and the newest resync attempt, so an answer belonging to an identity the session has left (a restore started under Alice answering after Bob logged in) is dropped, and a caller that joins an attempt in flight is judged against the attempt's own baseline;
  • subscribeIdentity() returns a lifetime-bound handle: a probe that answers after dispose(), or while the session owns the identity, is ignored. One document listener per session, removed with the last subscriber;
  • onTransition fires once per applied transition and before any onEvent — "the store is already invalidated when you hear the event" is a guarantee, not subscription order.

Consumers

  • authSession: events (login/sessionRestore stay with checkUser, the rest derived), derived info, and the refocus resync that maps "no session to restore" to cleared.
  • SolidAuthnLogic: keeps the redirect handling, the NSS cookie probe and saveUser; currentUser() is one call to effectiveIdentity(). Deleted: fallbackWebId, cookieBackedFallback, cookieProbeGeneration, the refocus watcher, probeCookieIdentity()'s statuses, reportFallbackIdentityChange().
  • fetch bridge: sessionIdentityWebId() decides whose credentials a request would carry.
  • AuthnLogic.dispose?() added; reloadOnIdentityReplaced exported from the new module.

Store side (flagAuthorizationOnTransitions.ts)

  • subscribed to the state: every identity transition marks all recorded responses out-of-date, before any event listener hears about it, so editable() answers "unknown" instead of the previous identity's access;
  • a failed invalidation is remembered (invalidationFailed): the store still answers definitively for the previous identity, so its answers are repaired, never trusted;
  • ensureDocumentAuthorization() / loadAuthorizedDocument() repair by re-flagging and load()ing the document — a load of a fully flagged document refetches it (see below) — with the transition generation stamped around each attempt: an overtaken answer is retried under the new identity, up to three times, and stays unknown when it cannot be established (no load capability, a failed load, or an identity that keeps moving). Nothing forces a fetch by hand any more;
  • utilityLogic.followOrCreate* repair before reading, and fail closed with NotEditableError.

rdflib 2.4.1 (required)

load() refetches a document whose recorded answers are all flagged (linkeddata/rdflib.js#871) — exactly the state an identity transition leaves behind — and checkEditable() heals the same way. The store side repairs by a plain load(), so 2.4.1 is a requirement, not an optimisation: consumers still pinned at 2.4.0 (solid-ui, solid-panes, mashlib) must move to 2.4.1 with this change. On 2.4.0 the flagged document was answered from the cache, and a repair could not re-establish an answer.

Tests

  • identityState.test.ts (15): predicates, login/logout, single replacement, A→B, token-update change, cross-tab clear once, joined resync, dropped stale answer, cookie adopt/replace/clear, release semantics, refocus fan-out, invalidate-before-notify;
  • solidAuthLogic.test.ts (19): currentUser, checkUser paths + probe, refocus/dispose, info;
  • flagAuthorizationOnTransitions.test.ts (12), rdflibEditableFlagContract.test.ts (3);
  • suite 131 passed / 2 skipped, both typechecks, eslint, npm run prepublishOnly exit 0.

E2E (mashlib dev server, linked packages, real pod)

login → edit → PATCH ✅; logout → logout + identityReplaced, currentUser()undefined ✅; the headline case measured: a document owned by the identity went 'SPARQL' → (logout) undefined → (anonymous re-read) false ✅.

Not testable locally: the live cross-tab switch — no worker on localhost (no push channel), and the shared IndexedDB plus token rotation means tabs overwrite each other. The deployed worker path is what the uvdsl issue is about. The NSS cookie fallback needs a *.localhost pod origin.

Notes

  • The cookie fallback and the setTokenDetails wrap are TEMPORARY: uvdsl announces only isActive changes — both are isolated so they can be deleted when the library reports identity changes (issue drafted upstream).

The identity used to be assembled per call site from session fields, a cookie
fallback and a cleared mark, each with its own predicate; every new async path
(a slow restore, a late probe, a refocus) could reintroduce the same class of
bug in a different place.

The state now lives once per session in identityState.ts:

- sessionIsActive() is the single activity rule (an explicit false wins over a
  cached WebID);
- every observation (session event, token update, refocus resync, cookie
  probe result) is applied to one record, so a change is detected and reported
  once, whichever path noticed it;
- the record carries a version and the newest resync attempt, so an answer
  that belongs to an identity the session has since left — a restore started
  under Alice answering after Bob logged in — is dropped, and a caller that
  joins an attempt in flight is judged against the attempt's own baseline;
- transitions are derived from the previous/next snapshots
  (classifyTransition/identityReplaced), not from flags, so an unchanged
  identity costs no event.

The cookie fallback and the setTokenDetails watch are TEMPORARY: they exist
because uvdsl announces only isActive changes — not a WebID change that keeps
the session active, and not a cookie identity change at all. Both are isolated
here so they become a small deletion when the library reports identity changes
itself.

Nothing consumes the state yet: the next commit moves authSession,
SolidAuthnLogic and the fetch bridge onto it.

Tests: test/identityState.test.ts (15) — activity/predicate table, login and
logout (one replacement, no second one after the WebID is cleared), A->B while
active, an A->B that uvdsl never announces around setTokenDetails, cross-tab
clear reported once, joined resync, dropped stale answer, cookie identity
adopt/replace/clear, release semantics, refocus fan-out.
authSession, SolidAuthnLogic and the fetch bridge now use the per-session
identity state (identityState.ts) instead of assembling the identity per call
site:

- authSession subscribes once (events + the refocus resync) and publishes the
  derived `info` shape. 'logout' / 'sessionChange' / 'identityReplaced' come
  from the state; 'login' / 'sessionRestore' stay with checkUser, the only
  place that knows which path activated the session.
- SolidAuthnLogic keeps only what is its own: the redirect handling, the NSS
  cookie probe and saveUser. currentUser() asks the state for the effective
  identity, and the probe result goes through its subscription — a probe that
  answers after dispose(), or while the session took ownership, can no longer
  resurrect an identity. fallbackWebId, cookieBackedFallback,
  cookieProbeGeneration, the refocus watcher, probeCookieIdentity() and
  reportFallbackIdentityChange() are gone: one record replaces them.
- solidLogicSingleton decides the fetch credentials from the state, so a
  session that reports itself inactive (or was reported cleared) cannot send
  the previous identity's credentials.
- AuthnLogic gains the optional dispose() the state's lifetime needs; the
  legacy event vocabulary is exported and includes the identity events.

Tests: the fetch-bridge test fakes the session identity instead of the derived
`info` shape, whose assignment is ignored on purpose. Full suite green.
Replaces the smoke tests with the behaviours the identity state has to keep
for SolidAuthnLogic:

- currentUser(): the active session's WebID; logged out for an explicit
  isActive:false or info.isLoggedIn:false with a cached WebID; the legacy
  WebID-only shape accepted.
- checkUser(): sessionRestore/login announced once, from the path that
  activated the session; "No session to restore." is treated as logged out
  while a failure that nevertheless left the session active is rethrown; the
  NSS cookie probe recovers the WebID on *.localhost and reports it as a
  session change, is skipped when the session already has one, and a probe
  that answers after the session took the identity is ignored.
- refocus/dispose: the cookie identity is re-probed and its loss reported; a
  disposed instance stops probing (an in-flight probe result is dropped),
  while a second instance using the same session keeps the watcher.
- authSession.info: derived, explicit false reported, assignment ignored.

The tests subscribe a forwarder like authSession does in the app, so the
emitted list means the same thing as the legacy events consumers see.
A session transition changes whose credentials a request would carry, but
editable() reads responses that are not keyed by identity: a document fetched
anonymously (before a restore completed) or under a previous WebID keeps
answering for the old identity.

flagAuthorizationOnSessionTransitions() subscribes to the identity state — one
call per applied transition, whichever path noticed it — and marks every
recorded response out-of-date, so editable() answers "unknown" instead of the
previous identity's access. A failed invalidation is remembered as
refreshRequired, so the decision points repair instead of trusting a store
that could not be invalidated.

refreshDocumentAuthorization() / ensureDocumentAuthorization() /
loadAuthorizedDocument() force-refresh (force: true, clearPreviousData: true)
with the store's transition generation stamped around each attempt: an answer
overtaken by a transition is retried under the new identity and stays
"unknown" when it cannot be established — never the previous identity's
answer. loadAuthorizedDocument also generation-checks the load, because a
response begun before a transition can be recorded after it.

utilityLogic's followOrCreate* repair before reading (and fail closed with
NotEditableError), and solidLogic wires the invalidation where the session is
created.

identityState reports an applied transition through onTransition (once),
while onEvent still carries the individual legacy events: a transition that
carries two events must not make a consumer that only invalidates do its work
twice.

On rdflib 2.4.0 a plain load cannot repair a flagged document (the literal /
NamedNode mismatch, linkeddata/rdflib.js#427); from 2.4.1 load() matches the
literal, so checkEditable() heals too — the force path remains the
deterministic repair on both.

Tests: 12 for the store side (flag on change only, every transition path,
release, failed invalidation, definitive answers, repair, overtaken load,
failure modes).
rdflib 2.4.1 carries linkeddata/rdflib.js#871: load() matches the recorded
request by its URI literal and refetches a document whose recorded answers are
all flagged, so a plain load()/checkEditable() heals a flagged document again.
Our repair path (refreshDocumentAuthorization/ensureDocumentAuthorization/
loadAuthorizedDocument) is unchanged and stays the deterministic repair — it
also covers consumers still on rdflib 2.4.0.

Contract tests against the real UpdateManager/Fetcher:
- definitive -> unknown when flagged -> definitive after a fresh response;
- load() heals a flagged, already-loaded document on 2.4.1;
- refreshDocumentAuthorization() repairs on any version.

package.json / package-lock.json: rdflib ^2.4.0 -> ^2.4.1, lock edited to the
single dependency entry instead of regenerating it.
deliver() interleaved a subscriber's own events with its transition callback,
so which ran first depended on subscription order: in the app authSession
subscribes before the store, and a legacy 'logout' listener could therefore
read the store before it had been invalidated (measured in the e2e: a
synchronous read inside the listener still saw the previous identity's write
capability).

Every subscriber's onTransition now runs before any subscriber's onEvent, so
"the store is already invalidated when you hear the event" is a guarantee
rather than an accident.

Test: subscribers in the app's order observe ['transition', 'event', 'event'].
…efresh

rdflib 2.4.1 refetches a document whose recorded answers are all flagged
(linkeddata/rdflib.js#871) — exactly the state an identity transition leaves
behind — so the store side no longer forces a fetch by hand:

- forceRefresh() and refreshDocumentAuthorization() are gone; repairDocument()
  marks the responses out-of-date (including one recorded after the transition,
  which the transition's own flag could not have marked) and loads, retrying
  under a new identity and staying unknown if it keeps moving;
- a store that cannot invalidate is repaired through the same path, and
  ensureDocumentAuthorization() fails closed when the repair cannot be
  established — a store that cannot invalidate must not have its answers
  trusted;
- the module and its tests state the requirement plainly: rdflib >= 2.4.1.

Tests: the store-side suite models the flag/load contract instead of a refresh
callback; the contract test asserts that a decision point repairs a flagged
document through a load.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The new loadAuthorizedDocument() can silently skip loading when fetcher.load is missing (contradicting its contract), and SolidLogic currently drops the invalidation subscription unsubscribe, risking leaks when instances are replaced.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Reworks session/identity handling to centralize identity transitions into a single per-session identity state, ensuring consumers (auth logic, fetch bridge, rdflib authorization caching) observe consistent identity changes and invalidate/repair cached authorization data correctly across login/logout, cross-tab changes, and NSS cookie fallback.

Changes:

  • Introduces identityState as the single source of truth for session identity snapshots, transitions, and subscriptions (including refocus resync + NSS cookie fallback).
  • Adds store-side invalidation/repair logic (flagAuthorizationOnTransitions) to prevent rdflib editable() from answering using stale authorization metadata after identity changes.
  • Updates logic + tests to use the new identity state and bumps rdflib to ^2.4.1 to rely on the flagged-document refetch behavior.
File Description
test/​solidAuthLogic.test.ts Rebuilds SolidAuthnLogic tests to use identity state, cookie probe behavior, refocus, and disposal semantics.
test/​rdflibEditableFlagContract.test.ts Adds contract tests verifying rdflib flagging → unknown → refetch behavior required by repairs.
test/​logic.test.ts Updates fetch-bridge tests to fake identity via session fields (not info) under new derived-info behavior.
test/​identityState.test.ts Adds comprehensive unit tests for identityState predicates, transitions, refocus resync, and cookie identity behavior.
test/​flagAuthorizationOnTransitions.test.ts Adds tests for store invalidation on transitions and repair behavior under overtaken loads/invalidation failures.
src/​util/​utilityLogic.ts Uses loadAuthorizedDocument() to avoid consuming stale/flagged authorization metadata before reads/writes.
src/​types.ts Extends AuthnLogic with optional dispose() for releasing listeners/subscriptions.
src/​logic/​solidLogicSingleton.ts Updates fetch bridge to avoid using a retained WebID when session explicitly reports inactive.
src/​logic/​solidLogic.ts Wires store invalidation to identity transitions at SolidLogic creation time.
src/​index.ts Re-exports reloadOnIdentityReplaced from the new identity state module.
src/​authSession/​identityState.ts Adds centralized per-session identity record, transition classification, subscriptions, refocus resync, and cookie identity application.
src/​authSession/​flagAuthorizationOnTransitions.ts Adds store invalidation/repair helpers keyed off identity transitions and rdflib metadata flagging behavior.
src/​authSession/​events.ts Expands legacy event vocabulary to include sessionChange and identityReplaced emitted via identity state.
src/​authSession/​authSession.ts Subscribes authSession to identity state transitions and defines derived/assignment-ignored legacy info.
src/​authn/​SolidAuthnLogic.ts Refactors SolidAuthnLogic to read identity via identityState, adds dispose, and routes NSS cookie fallback via identity state.
package.json Bumps rdflib dev dependency to ^2.4.1.
package-lock.json Locks rdflib to 2.4.1 with updated resolved/integrity metadata.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/authSession/flagAuthorizationOnTransitions.ts
Comment thread src/logic/solidLogic.ts
loadAuthorizedDocument() answers whether a document can be consumed; on a
store without fetcher.load it fell through to ensureDocumentAuthorization()
and could answer "yes" for a document it never loaded. It now fails like a
plain load() would.

createSolidLogic() keeps the unsubscribe that
flagAuthorizationOnSessionTransitions() returns and calls it from
authn.dispose(), so replacing a SolidLogic instance no longer leaves the old
store subscribed to the session (and retained by it).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Identified correctness issues in identityState around token-update transition detection on rejection and documented/newest resync selection not matching implementation.

Review effort: Lite
Findings: None

Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Select the newest resync action among subscribers

src/​authSession/​identityState.ts:451

The comment above says the module keeps the newest resync action, but resyncActionOf() returns the first subscriber with resync. If multiple subscribers provide resync, an older one will keep winning, which can make refocus resync behavior depend on subscription order.

Medium severity Record identity changes even when setTokenDetails rejects

src/​authSession/​identityState.ts:480

watchTokenUpdates() only calls note(record) when setTokenDetails() resolves. If setTokenDetails() mutates isActive/webId and then rejects, the identity transition will be missed (no note() call), which contradicts the intent of catching A→B changes around token updates.

watchTokenUpdates() only compared the identity when setTokenDetails()
resolved, so an update that applied the new identity and then rejected (a
failed persistence, say) left the transition unseen — the one thing the
wrapper exists to catch. The comparison now runs on success, on rejection
and on a synchronous throw; the failure itself is rethrown untouched.

resyncActionOf() promised the newest resync action but returned the first
subscriber that had one, i.e. the oldest. It keeps the last match instead,
so a second provider cannot make refocus behaviour depend on subscription
order.
@bourgeoa

Copy link
Copy Markdown
Contributor Author

"Select the newest resync action among subscribers":

Fixed in dbfcf10: resyncActionOf() now keeps the last match instead of returning the first — subscriber iteration is insertion order, so that is the newest provider, matching the doc comment. Test added: "takes the newest resync action when several subscribers provide one" (fails with the old first-match behaviour).

"Record identity changes even when setTokenDetails rejects":

Fixed in dbfcf10: the identity comparison around setTokenDetails() now runs on resolution, on rejection, and on a synchronous throw — a token update that applies the new identity and then fails no longer loses the transition. The original error is rethrown untouched. Test added: "reports an identity a rejected token update still applied".

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

It introduces a large, cross-cutting redesign of identity/session and authorization repair flows, which warrants final human review despite strong test coverage.

Review effort: Lite
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Low severity Use String(error) for consistent error logging

src/​authSession/​flagAuthorizationOnTransitions.ts:61

This log message interpolates the caught value directly; for non-Error throws it can degrade to [object Object]. Using String(error) matches the more robust formatting already used elsewhere in this file (e.g., the reload warning) and produces consistent output.

Low severity Unsubscribe visibility listener after each test

test/​identityState.test.ts:104

subscribeIdentity() attaches a visibilitychange listener per session; without unsubscribing, this test leaves the subscription (and document listener) alive for the rest of the file, which can leak memory and make later tests flaky when they dispatch visibilitychange. Please capture the returned subscription and unsubscribe (or add an afterEach that cleans up all subscriptions) throughout this test file.

… tests

invalidate() interpolated the caught value directly, which reads
"[object Object]" for a non-Error throw; String(error) matches the reload
warning in the same module.

The identityState tests now subscribe through a helper that keeps every
handle and releases it in afterEach: a first subscription attaches a
visibilitychange listener to the document, and leaving those behind lets a
later test's refocus run resyncs (and cookie probes) from a finished one.
@bourgeoa

Copy link
Copy Markdown
Contributor Author

"Use String(error) for consistent error logging":

Fixed in bb5575b — the invalidation warning now formats the caught value with String(error), consistent with the reload warning in the same module, so a non-Error throw no longer logs as [object Object].

"Unsubscribe visibility listener after each test":

Fixed in bb5575b — the file now subscribes through a helper that remembers every handle, with a top-level afterEach releasing them all (the two tests that release manually are unaffected; unsubscribe() is idempotent). All 17 call sites converted.

@bourgeoa bourgeoa added the bug Something isn't working label Sep 20, 2026
@bourgeoa bourgeoa self-assigned this Sep 20, 2026
@bourgeoa bourgeoa moved this to Ready in SolidOS NLNet UI Sep 20, 2026
@bourgeoa
bourgeoa merged commit df2e3e6 into staging Sep 20, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to Done in SolidOS NLNet UI Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants