Skip to content

Commit c6f4542

Browse files
authored
fix: re-arm the approver inbox with exponential backoff (R1.5) (#97)
The worker inbound path got a proper re-arming backoff (2s→60s, doubling, re-arms on any failure). The approver inbox — added later as a deliberately parallel pool — never did, and kept the original defect verbatim: onClose: () => { approverPool.delete(key); if (approverIdentities.has(vtaDid)) { setTimeout(() => void getApproverWarmSession(vtaDid).catch(() => undefined), 2000); } } One fixed 2s retry, scheduled only from `onClose`, with its failure swallowed. If that retry failed, no session ever opened — so `onClose` could not fire again and nothing re-armed. A mediator outage longer than ~2s left the approver's inbox permanently dead until the operator happened to re-run the biometric unlock. That inbox is the one that receives `task-consent/request`. A dead listener there means gated actions silently never get their human check — exactly the failure R1.5 exists to prevent, and a security control rather than a convenience (R7.2). Adds `ReconnectScheduler` to core: retry forever, cap the delay not the attempt count, re-arm on every failure including first-connect (where no open means no close), never stack timers per key, reset to base on success, and an optional `shouldRetry` gate so an operator lock beats a retry already in flight. Timers are injectable so the loop is testable without real waiting. The worker path deliberately keeps its own copy for now. That pool is documented as isolated so it "can never disturb the working worker inbound path" — adopting the shared scheduler there is a change to that path, not to this one, and belongs in its own PR. Verified: 9 new tests covering re-arm, doubling+cap, 60s-outage recovery, reset-on-success, timer non-stacking, lock-beats-retry, cancel-during-flight and per-key independence. Confirmed they FAIL (4/9) against a mutant with the re-arm removed, so they detect the original bug rather than merely passing. Full suite 180/180, clean build and lint, MV3 single-bundle guard holds. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
1 parent 45b9386 commit c6f4542

4 files changed

Lines changed: 416 additions & 3 deletions

File tree

packages/core/src/inbound/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ export * from "./confirm.js";
22
export * from "./task-consent.js";
33
export * from "./effect-format.js";
44
export * from "./dedup.js";
5+
export * from "./reconnect.js";
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* Re-arming exponential backoff for inbound mediator sessions (R1.5).
3+
*
4+
* A listener that stops retrying is a listener that silently misses every
5+
* consent request that arrives afterwards — the human check for a gated action
6+
* never happens and nothing reports that it didn't (R7.2). So the contract
7+
* here is deliberately narrow and deliberately relentless:
8+
*
9+
* - retry forever; cap the DELAY, never the attempt count
10+
* - re-arm on EVERY failure, including the first-connect case where no
11+
* session ever opened (an `onClose`-driven retry cannot cover that — no
12+
* open means no close)
13+
* - never let two timers stack for the same key
14+
* - reset to the base delay on success, so a recovered session doesn't carry
15+
* a grown delay into its next outage
16+
*
17+
* `attempt` must not throw: it reports success as a boolean. A throw escaping
18+
* the timer callback would kill the loop, which is precisely how the previous
19+
* fire-and-forget `.catch(() => undefined)` retry gave up for good.
20+
*
21+
* Timer functions are injectable so the loop is testable without real waiting.
22+
*/
23+
24+
export interface ReconnectTimers {
25+
setTimeout: (fn: () => void, ms: number) => ReturnType<typeof setTimeout>;
26+
clearTimeout: (handle: ReturnType<typeof setTimeout>) => void;
27+
}
28+
29+
export interface ReconnectOptions {
30+
/** First retry delay, and the value reset to on success. */
31+
baseMs: number;
32+
/** Ceiling for the doubling delay. Bounds the blind window after recovery. */
33+
maxMs: number;
34+
/** Bring the session up. Must resolve `true` on success, `false` on ANY
35+
* failure. Must not throw. */
36+
attempt: (key: string) => Promise<boolean>;
37+
/** Optional gate consulted before each attempt and again before re-arming.
38+
* Returning false abandons the loop and clears state — used for "the
39+
* operator locked this identity", which must beat a retry in flight. */
40+
shouldRetry?: (key: string) => boolean;
41+
timers?: ReconnectTimers;
42+
}
43+
44+
interface Entry {
45+
delayMs: number;
46+
timer: ReturnType<typeof setTimeout> | undefined;
47+
}
48+
49+
export class ReconnectScheduler {
50+
readonly #opts: ReconnectOptions;
51+
readonly #timers: ReconnectTimers;
52+
readonly #entries = new Map<string, Entry>();
53+
54+
constructor(opts: ReconnectOptions) {
55+
this.#opts = opts;
56+
this.#timers = opts.timers ?? {
57+
setTimeout: (fn, ms) => setTimeout(fn, ms),
58+
clearTimeout: (h) => clearTimeout(h),
59+
};
60+
}
61+
62+
/** Queue a retry for `key`, or do nothing if one is already queued. */
63+
schedule(key: string): void {
64+
if (this.#opts.shouldRetry && !this.#opts.shouldRetry(key)) {
65+
this.clear(key);
66+
return;
67+
}
68+
const entry = this.#entries.get(key) ?? { delayMs: this.#opts.baseMs, timer: undefined };
69+
if (entry.timer) return; // already queued — never stack timers for one key
70+
entry.timer = this.#timers.setTimeout(() => {
71+
entry.timer = undefined;
72+
void this.#opts.attempt(key).then((ok) => {
73+
if (ok) {
74+
this.clear(key);
75+
return;
76+
}
77+
// A lock (or equivalent) during the attempt wins over the retry.
78+
if (this.#opts.shouldRetry && !this.#opts.shouldRetry(key)) {
79+
this.clear(key);
80+
return;
81+
}
82+
// Don't resurrect a backoff cancelled while this attempt was in flight
83+
// — `clear()` dropped the entry, and re-arming would revive a loop the
84+
// caller explicitly ended.
85+
if (this.#entries.get(key) !== entry) return;
86+
entry.delayMs = Math.min(entry.delayMs * 2, this.#opts.maxMs);
87+
this.schedule(key);
88+
});
89+
}, entry.delayMs);
90+
this.#entries.set(key, entry);
91+
}
92+
93+
/** Cancel any pending retry for `key` and reset its delay to the base. */
94+
clear(key: string): void {
95+
const entry = this.#entries.get(key);
96+
if (entry?.timer) this.#timers.clearTimeout(entry.timer);
97+
this.#entries.delete(key);
98+
}
99+
100+
/** Cancel every pending retry. */
101+
clearAll(): void {
102+
for (const key of [...this.#entries.keys()]) this.clear(key);
103+
}
104+
105+
/** The delay the NEXT retry for `key` would use. Test/telemetry surface. */
106+
pendingDelayMs(key: string): number | undefined {
107+
return this.#entries.get(key)?.delayMs;
108+
}
109+
110+
/** Whether a retry is currently queued for `key`. */
111+
isArmed(key: string): boolean {
112+
return this.#entries.get(key)?.timer !== undefined;
113+
}
114+
}
Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
import { test } from "node:test";
2+
import assert from "node:assert/strict";
3+
4+
import { ReconnectScheduler } from "../dist/inbound/reconnect.js";
5+
6+
/** A controllable clock. Real timers would make these tests slow and flaky;
7+
* worse, the behaviour under test IS the timing, so it has to be observable
8+
* rather than waited out. */
9+
function fakeTimers() {
10+
let now = 0;
11+
let seq = 0;
12+
const queued = new Map(); // handle -> { at, fn }
13+
return {
14+
timers: {
15+
setTimeout: (fn, ms) => {
16+
const handle = ++seq;
17+
queued.set(handle, { at: now + ms, fn });
18+
return handle;
19+
},
20+
clearTimeout: (handle) => queued.delete(handle),
21+
},
22+
/** Advance the clock, firing due callbacks in time order. */
23+
async advance(ms) {
24+
const target = now + ms;
25+
for (;;) {
26+
let next = null;
27+
for (const [handle, t] of queued) {
28+
if (t.at <= target && (next === null || t.at < queued.get(next).at)) next = handle;
29+
}
30+
if (next === null) break;
31+
const { at, fn } = queued.get(next);
32+
queued.delete(next);
33+
now = at;
34+
fn();
35+
// Let the promise chain inside the callback settle before the next tick.
36+
await new Promise((r) => setImmediate(r));
37+
}
38+
now = target;
39+
},
40+
pending: () => queued.size,
41+
};
42+
}
43+
44+
test("a failing attempt re-arms instead of giving up after one retry", async () => {
45+
// The original bug: ONE 2s retry, and if it failed nothing ever tried again
46+
// because no session opened, so onClose could not fire a second time.
47+
const clock = fakeTimers();
48+
let attempts = 0;
49+
const s = new ReconnectScheduler({
50+
baseMs: 2_000,
51+
maxMs: 60_000,
52+
attempt: async () => {
53+
attempts++;
54+
return false;
55+
},
56+
timers: clock.timers,
57+
});
58+
59+
s.schedule("vta");
60+
await clock.advance(2_000);
61+
assert.equal(attempts, 1);
62+
// The whole point: still armed after the first failure.
63+
assert.equal(s.isArmed("vta"), true, "must re-arm after a failed attempt");
64+
65+
await clock.advance(4_000);
66+
assert.equal(attempts, 2);
67+
await clock.advance(8_000);
68+
assert.equal(attempts, 3);
69+
});
70+
71+
test("the delay doubles and then caps, so retries never stop", async () => {
72+
const clock = fakeTimers();
73+
const s = new ReconnectScheduler({
74+
baseMs: 2_000,
75+
maxMs: 16_000,
76+
attempt: async () => false,
77+
timers: clock.timers,
78+
});
79+
80+
s.schedule("vta");
81+
const seen = [];
82+
for (let i = 0; i < 8; i++) {
83+
// Advance by exactly the queued delay so each pass fires one retry —
84+
// a coarser jump would run several doublings inside one advance.
85+
const delay = s.pendingDelayMs("vta");
86+
seen.push(delay);
87+
await clock.advance(delay);
88+
}
89+
assert.deepEqual(seen, [2_000, 4_000, 8_000, 16_000, 16_000, 16_000, 16_000, 16_000]);
90+
assert.equal(s.isArmed("vta"), true, "a capped backoff is still a live backoff");
91+
});
92+
93+
test("a mediator down for 60s recovers on its own once it returns", async () => {
94+
const clock = fakeTimers();
95+
let mediatorUp = false;
96+
let connects = 0;
97+
const s = new ReconnectScheduler({
98+
baseMs: 2_000,
99+
maxMs: 60_000,
100+
attempt: async () => {
101+
if (!mediatorUp) return false;
102+
connects++;
103+
return true;
104+
},
105+
timers: clock.timers,
106+
});
107+
108+
s.schedule("vta");
109+
await clock.advance(60_000);
110+
assert.equal(connects, 0, "still down, so no connect");
111+
assert.equal(s.isArmed("vta"), true, "but still trying");
112+
113+
mediatorUp = true;
114+
await clock.advance(64_000);
115+
assert.equal(connects, 1, "recovers without a worker reboot");
116+
assert.equal(s.isArmed("vta"), false, "and stops retrying once connected");
117+
});
118+
119+
test("success resets the delay, so the next outage starts from the base", async () => {
120+
const clock = fakeTimers();
121+
let up = false;
122+
const s = new ReconnectScheduler({
123+
baseMs: 2_000,
124+
maxMs: 60_000,
125+
attempt: async () => up,
126+
timers: clock.timers,
127+
});
128+
129+
s.schedule("vta");
130+
await clock.advance(14_000); // fail a few times, growing the delay
131+
up = true;
132+
await clock.advance(60_000); // connect
133+
assert.equal(s.isArmed("vta"), false);
134+
135+
up = false;
136+
s.schedule("vta"); // next outage
137+
assert.equal(s.pendingDelayMs("vta"), 2_000, "must not inherit the grown delay");
138+
});
139+
140+
test("timers never stack for one key", async () => {
141+
const clock = fakeTimers();
142+
let attempts = 0;
143+
const s = new ReconnectScheduler({
144+
baseMs: 2_000,
145+
maxMs: 60_000,
146+
attempt: async () => {
147+
attempts++;
148+
return false;
149+
},
150+
timers: clock.timers,
151+
});
152+
153+
s.schedule("vta");
154+
s.schedule("vta");
155+
s.schedule("vta");
156+
assert.equal(clock.pending(), 1, "three schedules, one timer");
157+
await clock.advance(2_000);
158+
assert.equal(attempts, 1);
159+
});
160+
161+
test("shouldRetry=false abandons the loop — a lock beats a retry in flight", async () => {
162+
// The operator locking the approver must win: resurrecting the session would
163+
// defeat an explicit security act.
164+
const clock = fakeTimers();
165+
let unlocked = true;
166+
let attempts = 0;
167+
const s = new ReconnectScheduler({
168+
baseMs: 2_000,
169+
maxMs: 60_000,
170+
attempt: async () => {
171+
attempts++;
172+
unlocked = false; // locked during the attempt
173+
return false;
174+
},
175+
shouldRetry: () => unlocked,
176+
timers: clock.timers,
177+
});
178+
179+
s.schedule("vta");
180+
await clock.advance(2_000);
181+
assert.equal(attempts, 1);
182+
assert.equal(s.isArmed("vta"), false, "must not re-arm after a lock");
183+
184+
await clock.advance(60_000);
185+
assert.equal(attempts, 1, "and must never attempt again");
186+
});
187+
188+
test("scheduling while already locked does not arm anything", async () => {
189+
const clock = fakeTimers();
190+
let attempts = 0;
191+
const s = new ReconnectScheduler({
192+
baseMs: 2_000,
193+
maxMs: 60_000,
194+
attempt: async () => {
195+
attempts++;
196+
return false;
197+
},
198+
shouldRetry: () => false,
199+
timers: clock.timers,
200+
});
201+
202+
s.schedule("vta");
203+
assert.equal(clock.pending(), 0);
204+
await clock.advance(60_000);
205+
assert.equal(attempts, 0);
206+
});
207+
208+
test("clear() cancels a queued retry and a retry in flight cannot revive it", async () => {
209+
const clock = fakeTimers();
210+
let attempts = 0;
211+
let release;
212+
const gate = new Promise((r) => (release = r));
213+
const s = new ReconnectScheduler({
214+
baseMs: 2_000,
215+
maxMs: 60_000,
216+
attempt: async () => {
217+
attempts++;
218+
await gate; // still in flight when clear() lands
219+
return false;
220+
},
221+
timers: clock.timers,
222+
});
223+
224+
s.schedule("vta");
225+
await clock.advance(2_000);
226+
assert.equal(attempts, 1);
227+
s.clear("vta"); // cancelled mid-attempt
228+
release();
229+
await new Promise((r) => setImmediate(r));
230+
assert.equal(s.isArmed("vta"), false, "a cancelled backoff must stay cancelled");
231+
232+
await clock.advance(60_000);
233+
assert.equal(attempts, 1);
234+
});
235+
236+
test("independent keys back off independently", async () => {
237+
const clock = fakeTimers();
238+
const attempts = { a: 0, b: 0 };
239+
const s = new ReconnectScheduler({
240+
baseMs: 2_000,
241+
maxMs: 60_000,
242+
attempt: async (key) => {
243+
attempts[key]++;
244+
return key === "b"; // b connects, a keeps failing
245+
},
246+
timers: clock.timers,
247+
});
248+
249+
s.schedule("a");
250+
s.schedule("b");
251+
await clock.advance(2_000);
252+
assert.deepEqual(attempts, { a: 1, b: 1 });
253+
assert.equal(s.isArmed("a"), true, "a is still retrying");
254+
assert.equal(s.isArmed("b"), false, "b connected and stopped");
255+
});

0 commit comments

Comments
 (0)