GameCult/Heimdall
Watch
9
0
Fork
You've already forked Heimdall
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Record a failed refresh on the identity row so another process does not re-present the token #8

Merged
heimdall merged 8 commits from heimdall/failed-refresh-marker into main 2026-11-09 10:31:03 +00:00
Conversation 3 Commits 8 Files changed 8 +382 -46
heimdall commented 2026-11-08 21:41:33 +00:00
Member
Copy link

Closes joystick:follow_up:cross-process-failed-refresh-marker (finding cut-h1.s2.failed-attempt-re-presented-across-processes). Target joystick:target:r3 tightens refresh-once-per-token to cover it.

Change

  • linked_identities.refresh_failed_at (nullable TIMESTAMPTZ, ADD COLUMN IF NOT EXISTS). A refresh that fails or times out writes it inside the row-lock transaction (new CredentialDecision kind failed, which writes the marker alone).
  • A process that takes the lock inside MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS (60 s) of the marker presents nothing and throws the same ManagedCredentialRefreshFailed (502 at the route). Being turned away does not extend the backoff.
  • Rotation, revocation and re-link clear the marker (upsertLinkedIdentity sets it NULL on conflict).
  • docs/service-contract.md: the "residual risk" sentence is replaced by the mechanism.

Behaviour change: a later request inside 60 s of a failure no longer retries the provider. The "single flight" test now asserts one provider call across both pools (it asserted one per pool); the two tests that retried straight after a failure now retry after the backoff.

Tests (test first; ran red, then green, on memory, postgres one app and postgres two apps against a live Postgres 16):

  • marker recorded on server_error and hang, and a second process inside the backoff makes no token call and leaves the marker unmoved
  • backoff ends exactly at its length; success clears the marker; a failure after it marks anew
  • revocation and re-link clear it
  • schema inventory count +1

Hand mutants, each applied, suite run, reverted; all killed, restored suite green (233 passed, 6 skipped):
backoff check ignored (14 fail), < to <= (12), failure not recorded (26), turned-away caller extends backoff (12), backoff halved (12), postgres rotate/revoke keeps marker (8), postgres re-link keeps marker (4), in-memory rotate/revoke keeps marker (4), postgres marker not read back (18).

Not done / known: a provider that rotates and consumed the token before an abort is covered only by the 60 s backoff, not by a distinct named state; I took the backoff form the follow-up named. Existing deployments get the column on ensureSchema. 13 other test files fail to load in this checkout (missing cultmesh-ts/cultnet-ts/cultcache-ts builds); they do not touch this change. Not merged, not deployed.

🤖 Generated with Claude Code

Closes `joystick:follow_up:cross-process-failed-refresh-marker` (finding `cut-h1.s2.failed-attempt-re-presented-across-processes`). Target `joystick:target:r3` tightens `refresh-once-per-token` to cover it. **Change** - `linked_identities.refresh_failed_at` (nullable TIMESTAMPTZ, `ADD COLUMN IF NOT EXISTS`). A refresh that fails or times out writes it inside the row-lock transaction (new `CredentialDecision` kind `failed`, which writes the marker alone). - A process that takes the lock inside `MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS` (60 s) of the marker presents nothing and throws the same `ManagedCredentialRefreshFailed` (502 at the route). Being turned away does not extend the backoff. - Rotation, revocation and re-link clear the marker (`upsertLinkedIdentity` sets it NULL on conflict). - `docs/service-contract.md`: the "residual risk" sentence is replaced by the mechanism. **Behaviour change:** a later request inside 60 s of a failure no longer retries the provider. The "single flight" test now asserts one provider call across both pools (it asserted one per pool); the two tests that retried straight after a failure now retry after the backoff. **Tests** (test first; ran red, then green, on memory, postgres one app and postgres two apps against a live Postgres 16): - marker recorded on `server_error` and `hang`, and a second process inside the backoff makes no token call and leaves the marker unmoved - backoff ends exactly at its length; success clears the marker; a failure after it marks anew - revocation and re-link clear it - schema inventory count +1 **Hand mutants**, each applied, suite run, reverted; all killed, restored suite green (233 passed, 6 skipped): backoff check ignored (14 fail), `<` to `<=` (12), failure not recorded (26), turned-away caller extends backoff (12), backoff halved (12), postgres rotate/revoke keeps marker (8), postgres re-link keeps marker (4), in-memory rotate/revoke keeps marker (4), postgres marker not read back (18). **Not done / known:** a provider that rotates and consumed the token before an abort is covered only by the 60 s backoff, not by a distinct named state; I took the backoff form the follow-up named. Existing deployments get the column on `ensureSchema`. 13 other test files fail to load in this checkout (missing cultmesh-ts/cultnet-ts/cultcache-ts builds); they do not touch this change. Not merged, not deployed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
A failed or timed-out refresh attempt now writes refresh_failed_at under the row lock; a process
that takes the lock inside a 60 s backoff answers managed_credential_refresh_failed without calling
the provider. Rotation, revocation and re-link clear the marker. Closes the cross-process residual
that refresh-once-per-token left to a follow-up, and pins it on the two-pool rig.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
heimdall commented 2026-11-08 21:52:11 +00:00
Author
Member
Copy link

Head is now 9da4b12bc27d011cd0a1405d38e10da6cfc5f17e (pushed without force). Fixes for the two Mediums and the skew Low from Libby's read at a0bd983.

  1. Stamp at failure time. The marker carried the attempt's start; it now carries the time the failure was known. Test: a clock that advances 9 s during the provider call; the marker reads t0+9 s and a second process at t0+60 s is still held off (red before).
  2. Durable after the provider answered. A throw after the answer (custody encrypt) now returns the failed decision instead of rolling back, so the marker commits. A rotation Postgres refuses to write leaves the marker in its own statement after the rollback (rotate decisions only). Tests force both: an encrypt that throws on the new access token, and a trigger that refuses the rotation UPDATE on live Postgres, one pool and two (red before). The other process makes no token call.
  3. Skew (Low). A stamp ahead of the reader's clock by the backoff or more is treated as skew, so the wait is bounded at twice the backoff; boundaries pinned both sides.

Hand mutants, each applied and reverted: failure stamped at attempt start, post-call throw rethrown, no marker after a refused rotation write, future stamp always holds, skew boundary inclusive, future stamps ignored; all caught. One survivor, triaged equivalent: writing the marker after a failed revoke write too (decision.kind !== "keep" instead of "rotate"). A revoke consumes no token, so the extra marker only adds a harmless backoff; I kept rotate-only because that is the case the marker protects.

Not covered: process death between the provider's answer and the commit. The transaction dies with the process and nothing records the attempt, so the next caller still replays the token. Closing it needs a write-ahead record of the attempt on a connection that does not wait on the row lock (a separate table), which also changes what concurrent callers see while an attempt is in flight. I did not take that without your say.

Head is now `9da4b12bc27d011cd0a1405d38e10da6cfc5f17e` (pushed without force). Fixes for the two Mediums and the skew Low from Libby's read at a0bd983. 1. **Stamp at failure time.** The marker carried the attempt's start; it now carries the time the failure was known. Test: a clock that advances 9 s during the provider call; the marker reads t0+9 s and a second process at t0+60 s is still held off (red before). 2. **Durable after the provider answered.** A throw after the answer (custody `encrypt`) now returns the `failed` decision instead of rolling back, so the marker commits. A rotation Postgres refuses to write leaves the marker in its own statement after the rollback (rotate decisions only). Tests force both: an encrypt that throws on the new access token, and a trigger that refuses the rotation UPDATE on live Postgres, one pool and two (red before). The other process makes no token call. 3. **Skew (Low).** A stamp ahead of the reader's clock by the backoff or more is treated as skew, so the wait is bounded at twice the backoff; boundaries pinned both sides. Hand mutants, each applied and reverted: failure stamped at attempt start, post-call throw rethrown, no marker after a refused rotation write, future stamp always holds, skew boundary inclusive, future stamps ignored; all caught. One survivor, triaged equivalent: writing the marker after a failed *revoke* write too (`decision.kind !== "keep"` instead of `"rotate"`). A revoke consumes no token, so the extra marker only adds a harmless backoff; I kept rotate-only because that is the case the marker protects. **Not covered:** process death between the provider's answer and the commit. The transaction dies with the process and nothing records the attempt, so the next caller still replays the token. Closing it needs a write-ahead record of the attempt on a connection that does not wait on the row lock (a separate table), which also changes what concurrent callers see while an attempt is in flight. I did not take that without your say. 
The marker carried the time the attempt began, so a slow failure ate the backoff; it now carries the
time the provider's answer (or failure) was known. A throw after the provider answered (custody
encrypt) records the marker instead of rolling it back, and a rotation Postgres refuses to write
leaves the marker in a transaction of its own. A stamp from a writer's clock ahead of the reader's by
the backoff or more is treated as skew, not a fresh failure.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
heimdall commented 2026-11-08 21:53:13 +00:00
Author
Member
Copy link

Head is now 1118f235b02b937fadb7569f40cbba61651c4353 (docs only, no force): service-contract.md names the process-death residual and cites joystick:follow_up:write-ahead-refresh-attempt-record.

Head is now `1118f235b02b937fadb7569f40cbba61651c4353` (docs only, no force): service-contract.md names the process-death residual and cites joystick:follow_up:write-ahead-refresh-attempt-record.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
heimdall commented 2026-11-08 21:57:51 +00:00
Author
Member
Copy link

Head is now 9c7d980165c867bde3ae499b6312498ae707feb1 (no force). The fallback marker now covers revoke as well as rotate, so a rotating provider that answered without a refresh token and whose revoke write fails leaves the marker. Live-Postgres test (Joystick, both no-token modes, one pool and two) failed first; the rotate-only mutant fails it, so the earlier "equivalent" judgement is withdrawn. Docs: the window opens when the request is sent, cites the steward ruling and the write-ahead follow-up, periods fixed. Not covered: a failing marker UPDATE on a failed decision (the database error propagates, nothing further to write).

Head is now `9c7d980165c867bde3ae499b6312498ae707feb1` (no force). The fallback marker now covers revoke as well as rotate, so a rotating provider that answered without a refresh token and whose revoke write fails leaves the marker. Live-Postgres test (Joystick, both no-token modes, one pool and two) failed first; the rotate-only mutant fails it, so the earlier "equivalent" judgement is withdrawn. Docs: the window opens when the request is sent, cites the steward ruling and the write-ahead follow-up, periods fixed. Not covered: a failing marker UPDATE on a failed decision (the database error propagates, nothing further to write).
A rotating provider that answered without a new refresh token has consumed the old one; the revoke
that follows is rolled back if its UPDATE fails, and no marker remained. The fallback marker now
covers revoke as well as rotate. The residual-risk paragraph says the window opens when the request
is sent, cites the steward ruling, and ends its sentences.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
When the rotation or revocation the provider's answer called for cannot be
committed, the fallback write now clears refresh_token_encrypted as well as
stamping the marker, so the end of the 60 s backoff cannot replay the consumed
token. Tests (live Postgres, trigger-refused revoke) assert no replayable token
and no second provider call. Docs: the marker is a best-effort write on a
second connection after rollback, the skew cap, the blanking write's residual.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A provider that keeps its refresh token valid (Twitch) keeps it when the
rotation cannot be committed: the marker is left, the token stays, and the
next caller after the backoff presents it. Blanking stays for a revoke and
for a provider that rotates (the rotate decision now carries
consumedRefreshToken). Live-Postgres test for Twitch written first and seen
failing; the contract states the rule inline.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
GameCult/Heimdall!8
No description provided.