From de25ef5a75da64003bc84d5ba950c513053e2589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 09:36:52 +0200 Subject: [PATCH 1/3] fix(serenity): check retryOnQuota's shared deadline before every attempt, not just between retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of #2874 found that the deadline was only consulted inside the catch block, after a poll attempt had already failed — so the FIRST attempt always ran regardless of how long ensure() itself took (e.g. queued behind other same-child recoveries on withResourceLock's own up-to-10s safety valve). That undermines the shared per-request deadline's whole purpose: bounding a request where multiple stacked wrap sites (createProject + createPromptsByIds in the same generateTopics create-market call) each hit the disguised-405 lag could still compound past budget in a narrower but real case. Move the deadline check to run before every attempt, including the first, tracking the most recent metered-quota error so a pre-attempt bail still throws something meaningful. Co-Authored-By: Claude Sonnet 5 --- .../serenity/dynamic-allocation-active.js | 31 ++++++-- .../dynamic-allocation-active.test.js | 74 ++++++++++++++----- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/src/support/serenity/dynamic-allocation-active.js b/src/support/serenity/dynamic-allocation-active.js index 90605a694..72c6704a3 100644 --- a/src/support/serenity/dynamic-allocation-active.js +++ b/src/support/serenity/dynamic-allocation-active.js @@ -151,9 +151,13 @@ export function createHeadroomGuard(transport, { // per-child lock contention (round-2 SRE review) — so `ensure()` is called exactly ONCE, up // front, and the actual fix is the WAIT between retries, not a repeated top-up. // - // Bounded by whichever comes first: `maxAttempts`, or the shared `requestDeadline` above. A - // non-metered error (or a second/subsequent metered error once bounded) propagates untouched — - // this never masks a genuinely non-retryable failure, it only spans the known settle lag. + // Bounded by whichever comes first: `maxAttempts`, or the shared `requestDeadline` above — the + // deadline is checked BEFORE every attempt (including the first poll attempt, not only between + // retries), so a slow `ensure()` call (e.g. queued behind other same-child recoveries on + // `withResourceLock`'s own safety valve) can't silently blow past the shared budget before the + // budget is ever consulted. A non-metered error (or a second/subsequent metered error once + // bounded) propagates untouched — this never masks a genuinely non-retryable failure, it only + // spans the known settle lag. retryOnQuota: async (fn, { callSite = 'unknown' } = {}) => { try { return await fn(); @@ -174,8 +178,23 @@ export function createHeadroomGuard(transport, { subWorkspaceId: childId, callSite, }); await ensure({}, { includeDrafted: true }); - let attempt = 1; + let attempt = 0; + let lastError = e; for (;;) { + // Checked BEFORE every attempt, including the first (not just between retries): if + // `ensure()` itself already consumed the whole shared budget (e.g. it queued behind + // other same-child recoveries on `withResourceLock`'s own up-to-10s safety valve), don't + // spend a further `fn()` call on top of an already-blown deadline. This closes a real + // gap found on self-review: checking the deadline only inside the catch (i.e. only after + // a poll attempt had already failed) meant the FIRST attempt always ran regardless of + // how long `ensure()` took, so the shared per-request budget this deadline exists to + // enforce (see the comment above) wasn't actually a hard cap — exactly the + // stacked-call-site compounding-latency risk the deadline was built to close. + if (now() >= requestDeadline) { + recordQuotaRetryOutcome('exhausted', { attempt, callSite }); + throw lastError; + } + attempt += 1; try { // eslint-disable-next-line no-await-in-loop const result = await fn(); @@ -191,13 +210,13 @@ export function createHeadroomGuard(transport, { recordQuotaRetryOutcome('abandoned', { attempt, callSite }); throw e2; } - if (attempt >= maxAttempts || now() >= requestDeadline) { + lastError = e2; + if (attempt >= maxAttempts) { recordQuotaRetryOutcome('exhausted', { attempt, callSite }); throw e2; } // eslint-disable-next-line no-await-in-loop await sleep(backoffMs); - attempt += 1; } } } diff --git a/test/support/serenity/dynamic-allocation-active.test.js b/test/support/serenity/dynamic-allocation-active.test.js index 5eaa08057..e403a1689 100644 --- a/test/support/serenity/dynamic-allocation-active.test.js +++ b/test/support/serenity/dynamic-allocation-active.test.js @@ -351,10 +351,14 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => expect(fn).to.have.callCount(2); }); - it('ON, the shared deadline (not the attempt cap) stops retrying: exhausts after the deadline passes even with attempts still available', async () => { - // maxAttempts is generous, but `now()` is stubbed to jump straight past totalBudgetMs after the - // first poll attempt — the deadline, not the attempt count, must be what ends the loop (round-2 - // SRE review: this is the seam that bounds a stacked-call-site request, not per-call attempts). + it('ON, the shared deadline (not the attempt cap) stops retrying: exhausts after one poll attempt when the deadline passes before the next', async () => { + // maxAttempts is generous, but `now()` is stubbed to jump past totalBudgetMs right after the + // FIRST poll attempt's own deadline check — the deadline, not the attempt count, must be what + // ends the loop (round-2 SRE review: this is the seam that bounds a stacked-call-site request, + // not per-call attempts). The deadline is checked BEFORE every attempt (self-review finding): + // 1st `now()` call = guard construction; 2nd = the deadline check before poll attempt 1 (still + // within budget, so attempt 1 runs and fails); 3rd = the deadline check before poll attempt 2 + // (now past budget, so it never runs). const t = makeTransport({ child: resources(dimObj(0, 0, 0), dimObj(0, 0, 0)), master: resources(dimObj(0, 0, 100), dimObj(0, 0, 800)), @@ -362,9 +366,7 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => let callCount = 0; const now = () => { callCount += 1; - // 1st call: guard construction (t0). 2nd+ calls: inside the loop's deadline check — jump - // straight past the budget so the very first deadline check already fails. - return callCount <= 1 ? 0 : 1_000_000; + return callCount <= 2 ? 0 : 1_000_000; }; const guard = createHeadroomGuard( t, @@ -391,6 +393,46 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => expect(fn).to.have.callCount(2); }); + it('ON, the deadline is checked BEFORE the very first poll attempt, not only between retries: a budget already blown by ensure() skips the attempt entirely', async () => { + // Self-review finding: checking the deadline only inside the catch (i.e. only after a poll + // attempt already failed) meant the first attempt always ran regardless of how long `ensure()` + // itself took. Simulate `ensure()` alone having consumed the whole budget (e.g. queued behind + // other same-child recoveries) — `now()` is already past the deadline by the time the loop's + // first check runs, so `fn()` must NOT be called again at all, and the ORIGINAL triggering + // error (not a fabricated new one) propagates. + const t = makeTransport({ + child: resources(dimObj(0, 0, 0), dimObj(0, 0, 0)), + master: resources(dimObj(0, 0, 100), dimObj(0, 0, 800)), + }); + let nowCallCount = 0; + const now = () => { + nowCallCount += 1; + // 1st call: guard construction (t0=0, so requestDeadline=9000). Every call after that + // (i.e. the loop's pre-attempt check) reports as already far past the budget. + return nowCallCount <= 1 ? 0 : 1_000_000; + }; + const guard = createHeadroomGuard( + t, + { + enabled: true, + subWorkspaceId: CHILD, + parentWorkspaceId: MASTER, + retryOnQuota: fastRetry({ maxAttempts: 10, totalBudgetMs: 9000, now }), + }, + log, + ); + const initialError = quota405(); + const fn = sinon.stub().rejects(initialError); + let caught; + try { + await guard.retryOnQuota(fn); + } catch (e) { + caught = e; + } + expect(caught).to.equal(initialError); + expect(fn).to.have.been.calledOnce; // only the initial call — no poll attempt was ever made + }); + it('ON, the deadline is SHARED across two sequential retryOnQuota calls on the SAME guard, not recomputed per call (MysticatBot review)', async () => { // This is the PR's key design intent: one guard is built per inbound request, and every wrap // site sharing that guard instance (e.g. createProject -> createPromptsByIds -> @@ -429,22 +471,20 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => // guard's budget, even though `maxAttempts: 10` would otherwise allow many more poll attempts. clock = 150; - // Call site B: if the deadline were shared (correct), it's already past — B exhausts on its - // very first poll attempt despite `maxAttempts: 10`. If a regression recomputed the deadline - // fresh inside this call (bug), B would get a full new 100ms budget from clock=150 and NOT - // exhaust here. - const lastError = quota405(); - const fnB = sinon.stub(); - fnB.onFirstCall().rejects(quota405()); - fnB.onSecondCall().rejects(lastError); + // Call site B: if the deadline were shared (correct), it's already past — the deadline check + // before B's first poll attempt fails immediately, so B never even gets a poll attempt, only + // its initial call. If a regression recomputed the deadline fresh inside this call (bug), B + // would get a full new 100ms budget from clock=150 and NOT exhaust here. + const initialErrorB = quota405(); + const fnB = sinon.stub().rejects(initialErrorB); let caught; try { await guard.retryOnQuota(fnB, { callSite: 'B' }); } catch (e) { caught = e; } - expect(caught).to.equal(lastError); - expect(fnB).to.have.callCount(2); + expect(caught).to.equal(initialErrorB); + expect(fnB).to.have.been.calledOnce; }); it('ON, the recovery ensure() itself throws (e.g. org pool exhausted): that error propagates and fn is NOT retried', async () => { From 2da90f4b386bf333da2ab511c2cb96c01f5a9793 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 15:18:25 +0200 Subject: [PATCH 2/3] fix(serenity): bail before sleeping if the sleep would itself overshoot the deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Alicia Adriani's review of #2882: the trailing sleep(backoffMs) between poll attempts was unconditional, so a failed attempt landing just inside the budget could still sleep the full backoffMs and overshoot the shared deadline by up to that amount before the next loop-top check caught it — undercutting the very "hard cap" the pre-attempt check was meant to enforce. Check now() + backoffMs against the deadline before sleeping, so an attempt that can't be usefully retried within budget bails immediately instead of sleeping first and bailing anyway. Also: updated allocation-metrics.js's JSDoc for the attempt=0 case (a pre-attempt deadline bail reports no poll attempt was made), and trimmed two comments per MysticatBot's nit to state the invariant rather than its discovery story. Co-Authored-By: Claude Sonnet 5 --- src/support/serenity/allocation-metrics.js | 22 ++++--- .../serenity/dynamic-allocation-active.js | 25 ++++---- .../dynamic-allocation-active.test.js | 58 +++++++++++++++++++ 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/support/serenity/allocation-metrics.js b/src/support/serenity/allocation-metrics.js index d310ed331..ab15af291 100644 --- a/src/support/serenity/allocation-metrics.js +++ b/src/support/serenity/allocation-metrics.js @@ -58,8 +58,10 @@ * shared deadline), or `abandoned` (a genuinely non-quota error cut the cycle short before * either of the above — distinct from `exhausted` so a dashboard summing * `recovered + exhausted + abandoned` as "total recovery cycles" doesn't silently undercount, - * per Alicia Adriani's review). `Attempt` (the 1-based attempt the cycle ended on) and - * `CallSite` (a small closed set — `publishProject`, `createProject`, `createOnePrompt`, + * per Alicia Adriani's review). `Attempt` (the 1-based poll attempt the cycle ended on, or `0` + * if the shared deadline was already exhausted before any poll attempt could even be made — see + * `retryOnQuota`'s pre-attempt deadline check, api-service#2882) and `CallSite` (a small closed + * set — `publishProject`, `createProject`, `createOnePrompt`, * `createPromptsByIds`, ...) together give the attempt-to-recovery distribution per write path, * without which a rising `exhausted` rate or a shift toward later attempts is invisible until it * becomes an incident (round-2 SRE review). @@ -168,13 +170,15 @@ export function recordMeteredQuotaClassifier(matched) { /** * @param {'recovered'|'exhausted'|'abandoned'} outcome - * @param {{ attempt: number, callSite: string }} dims - `attempt` is the 1-based attempt the - * poll-retry cycle resolved (recovered), gave up on (exhausted), or was cut short on by a - * non-quota error (abandoned); `callSite` is a short closed-vocabulary label identifying which - * wrapped write/publish this recovery was for. `attempt` is a raw number, not pre-bucketed — it - * stays low-cardinality only because `retryOnQuota`'s `maxAttempts` is small (3 today); if - * `maxAttempts` is ever raised significantly, revisit whether `Attempt` should be capped/bucketed - * to hold the module's low-cardinality dimension contract (see the module doc above). + * @param {{ attempt: number, callSite: string }} dims - `attempt` is the 1-based poll attempt the + * cycle resolved (recovered), gave up on (exhausted), or was cut short on by a non-quota error + * (abandoned) — OR `0` for `exhausted` specifically when the shared deadline was already blown + * before any poll attempt could be made (api-service#2882). `callSite` is a short + * closed-vocabulary label identifying which wrapped write/publish this recovery was for. + * `attempt` is a raw number, not pre-bucketed — it stays low-cardinality only because + * `retryOnQuota`'s `maxAttempts` is small (3 today); if `maxAttempts` is ever raised + * significantly, revisit whether `Attempt` should be capped/bucketed to hold the module's + * low-cardinality dimension contract (see the module doc above). * @returns {void} */ export function recordQuotaRetryOutcome(outcome, { attempt, callSite }) { diff --git a/src/support/serenity/dynamic-allocation-active.js b/src/support/serenity/dynamic-allocation-active.js index 72c6704a3..051646c8c 100644 --- a/src/support/serenity/dynamic-allocation-active.js +++ b/src/support/serenity/dynamic-allocation-active.js @@ -181,15 +181,9 @@ export function createHeadroomGuard(transport, { let attempt = 0; let lastError = e; for (;;) { - // Checked BEFORE every attempt, including the first (not just between retries): if - // `ensure()` itself already consumed the whole shared budget (e.g. it queued behind - // other same-child recoveries on `withResourceLock`'s own up-to-10s safety valve), don't - // spend a further `fn()` call on top of an already-blown deadline. This closes a real - // gap found on self-review: checking the deadline only inside the catch (i.e. only after - // a poll attempt had already failed) meant the FIRST attempt always ran regardless of - // how long `ensure()` took, so the shared per-request budget this deadline exists to - // enforce (see the comment above) wasn't actually a hard cap — exactly the - // stacked-call-site compounding-latency risk the deadline was built to close. + // Checked BEFORE every attempt, including the first — an already-blown deadline (e.g. + // `ensure()` itself ate the whole budget queued behind another same-child recovery) must + // not spend a further `fn()` call on top of it. See the header comment above for why. if (now() >= requestDeadline) { recordQuotaRetryOutcome('exhausted', { attempt, callSite }); throw lastError; @@ -202,16 +196,17 @@ export function createHeadroomGuard(transport, { return result; } catch (e2) { if (!isMeteredQuota(e2)) { - // A non-quota error mid-recovery still ENDS this cycle (Alicia Adriani review): - // record it as `abandoned` so a dashboard built on `recovered + exhausted + - // abandoned` as "total recovery cycles" doesn't silently undercount cycles cut - // short by an unrelated failure — distinct from `exhausted` (which specifically - // means "still a metered 405 after every attempt"). + // A non-quota error mid-recovery still ENDS this cycle — `abandoned` is distinct + // from `exhausted` (still a metered 405 after every attempt) so a dashboard summing + // recovered+exhausted+abandoned as "total cycles" doesn't undercount. recordQuotaRetryOutcome('abandoned', { attempt, callSite }); throw e2; } lastError = e2; - if (attempt >= maxAttempts) { + // Bail here too, BEFORE sleeping, if the sleep would itself run past the deadline — + // an unconditional sleep could otherwise overshoot the budget by up to a full + // `backoffMs` doing nothing useful. + if (attempt >= maxAttempts || now() + backoffMs >= requestDeadline) { recordQuotaRetryOutcome('exhausted', { attempt, callSite }); throw e2; } diff --git a/test/support/serenity/dynamic-allocation-active.test.js b/test/support/serenity/dynamic-allocation-active.test.js index e403a1689..330731803 100644 --- a/test/support/serenity/dynamic-allocation-active.test.js +++ b/test/support/serenity/dynamic-allocation-active.test.js @@ -665,6 +665,64 @@ describe('dynamic-allocation-active — retryOnQuota emits QuotaRetryOutcome (My ); }); + it('ON, a sleep that WOULD overshoot the deadline is skipped entirely: bails at the current attempt instead of sleeping uselessly (Alicia Adriani review)', async () => { + // Sequence of now() calls: 1st = construction (t0=0, budget=1000 => deadline=1000). 2nd = the + // pre-attempt check before poll attempt 1 (still within budget). 3rd = the pre-sleep check + // after poll attempt 1 fails: now()=950, backoffMs=3000 => 950+3000=3950 >= 1000 — sleeping + // would badly overshoot the deadline, so the fix must bail HERE, without ever calling sleep, + // rather than sleeping the full 3000ms only to bail at the next loop-top check anyway. + const recordQuotaRetryOutcome = sinon.stub(); + const { createHeadroomGuard: mockedCreateHeadroomGuard } = await esmock( + '../../../src/support/serenity/dynamic-allocation-active.js', + { '../../../src/support/serenity/allocation-metrics.js': { recordQuotaRetryOutcome } }, + ); + const t = makeTransport({ + child: resources(dimObj(0, 0, 0), dimObj(0, 0, 0)), + master: resources(dimObj(0, 0, 100), dimObj(0, 0, 800)), + }); + let nowCallCount = 0; + const now = () => { + nowCallCount += 1; + if (nowCallCount === 1) { + return 0; // construction + } + if (nowCallCount === 2) { + return 100; // pre-attempt check before poll attempt 1 + } + return 950; // pre-sleep check after poll attempt 1 fails + }; + const sleep = sinon.stub().resolves(); + const guard = mockedCreateHeadroomGuard( + t, + { + enabled: true, + subWorkspaceId: CHILD, + parentWorkspaceId: MASTER, + retryOnQuota: { + maxAttempts: 5, backoffMs: 3000, totalBudgetMs: 1000, now, sleep, + }, + }, + log, + ); + const lastError = quota405(); + const fn = sinon.stub(); + fn.onCall(0).rejects(quota405()); + fn.onCall(1).rejects(lastError); + let caught; + try { + await guard.retryOnQuota(fn, { callSite: 'publishProject' }); + } catch (e) { + caught = e; + } + expect(caught).to.equal(lastError); + expect(fn).to.have.callCount(2); // initial call + exactly one poll attempt + expect(sleep).to.not.have.been.called; + expect(recordQuotaRetryOutcome).to.have.been.calledOnceWith( + 'exhausted', + { attempt: 1, callSite: 'publishProject' }, + ); + }); + it('sleep(backoffMs) is actually invoked between poll attempts, not skipped — catches a regression that drops the await', async () => { const recordQuotaRetryOutcome = sinon.stub(); const { createHeadroomGuard: mockedCreateHeadroomGuard } = await esmock( From bab43e3946ddcf4e06a3f7a2b9391c7888517f27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 15:57:16 +0200 Subject: [PATCH 3/3] test(serenity): address MysticatBot's second-round nits on retryOnQuota deadline fix - Assert recordQuotaRetryOutcome('exhausted', { attempt: 0, callSite: 'unknown' }) is actually emitted in the "budget blown by ensure()" test, pinning the one new metric dimension value this fix introduces. - Use the full-qualified adobe/spacecat-api-service#2882 reference per workspace convention. - Rename the deadline-vs-attempt-cap test to describe what it actually proves (deadline takes priority over maxAttempts), distinct from the pre-attempt-check test which is the real regression guard for that behavior. Co-Authored-By: Claude Sonnet 5 --- src/support/serenity/allocation-metrics.js | 6 ++--- .../dynamic-allocation-active.test.js | 26 +++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/support/serenity/allocation-metrics.js b/src/support/serenity/allocation-metrics.js index ab15af291..a4648be6b 100644 --- a/src/support/serenity/allocation-metrics.js +++ b/src/support/serenity/allocation-metrics.js @@ -60,8 +60,8 @@ * `recovered + exhausted + abandoned` as "total recovery cycles" doesn't silently undercount, * per Alicia Adriani's review). `Attempt` (the 1-based poll attempt the cycle ended on, or `0` * if the shared deadline was already exhausted before any poll attempt could even be made — see - * `retryOnQuota`'s pre-attempt deadline check, api-service#2882) and `CallSite` (a small closed - * set — `publishProject`, `createProject`, `createOnePrompt`, + * `retryOnQuota`'s pre-attempt deadline check, adobe/spacecat-api-service#2882) and `CallSite` + * (a small closed set — `publishProject`, `createProject`, `createOnePrompt`, * `createPromptsByIds`, ...) together give the attempt-to-recovery distribution per write path, * without which a rising `exhausted` rate or a shift toward later attempts is invisible until it * becomes an incident (round-2 SRE review). @@ -173,7 +173,7 @@ export function recordMeteredQuotaClassifier(matched) { * @param {{ attempt: number, callSite: string }} dims - `attempt` is the 1-based poll attempt the * cycle resolved (recovered), gave up on (exhausted), or was cut short on by a non-quota error * (abandoned) — OR `0` for `exhausted` specifically when the shared deadline was already blown - * before any poll attempt could be made (api-service#2882). `callSite` is a short + * before any poll attempt could be made (adobe/spacecat-api-service#2882). `callSite` is a short * closed-vocabulary label identifying which wrapped write/publish this recovery was for. * `attempt` is a raw number, not pre-bucketed — it stays low-cardinality only because * `retryOnQuota`'s `maxAttempts` is small (3 today); if `maxAttempts` is ever raised diff --git a/test/support/serenity/dynamic-allocation-active.test.js b/test/support/serenity/dynamic-allocation-active.test.js index 330731803..dcd058562 100644 --- a/test/support/serenity/dynamic-allocation-active.test.js +++ b/test/support/serenity/dynamic-allocation-active.test.js @@ -351,7 +351,7 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => expect(fn).to.have.callCount(2); }); - it('ON, the shared deadline (not the attempt cap) stops retrying: exhausts after one poll attempt when the deadline passes before the next', async () => { + it('ON, the deadline takes priority OVER the attempt cap: exhausts after one poll attempt even though maxAttempts allows many more', async () => { // maxAttempts is generous, but `now()` is stubbed to jump past totalBudgetMs right after the // FIRST poll attempt's own deadline check — the deadline, not the attempt count, must be what // ends the loop (round-2 SRE review: this is the seam that bounds a stacked-call-site request, @@ -394,12 +394,18 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => }); it('ON, the deadline is checked BEFORE the very first poll attempt, not only between retries: a budget already blown by ensure() skips the attempt entirely', async () => { - // Self-review finding: checking the deadline only inside the catch (i.e. only after a poll - // attempt already failed) meant the first attempt always ran regardless of how long `ensure()` - // itself took. Simulate `ensure()` alone having consumed the whole budget (e.g. queued behind - // other same-child recoveries) — `now()` is already past the deadline by the time the loop's - // first check runs, so `fn()` must NOT be called again at all, and the ORIGINAL triggering - // error (not a fabricated new one) propagates. + // Checking the deadline only inside the catch (i.e. only after a poll attempt already failed) + // meant the first attempt always ran regardless of how long `ensure()` itself took. Simulate + // `ensure()` alone having consumed the whole budget (e.g. queued behind other same-child + // recoveries) — `now()` is already past the deadline by the time the loop's first check runs, + // so `fn()` must NOT be called again at all, the ORIGINAL triggering error (not a fabricated + // new one) propagates, and the metric records the `attempt: 0` case this closes (MysticatBot + // review). + const recordQuotaRetryOutcome = sinon.stub(); + const { createHeadroomGuard: mockedCreateHeadroomGuard } = await esmock( + '../../../src/support/serenity/dynamic-allocation-active.js', + { '../../../src/support/serenity/allocation-metrics.js': { recordQuotaRetryOutcome } }, + ); const t = makeTransport({ child: resources(dimObj(0, 0, 0), dimObj(0, 0, 0)), master: resources(dimObj(0, 0, 100), dimObj(0, 0, 800)), @@ -411,7 +417,7 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => // (i.e. the loop's pre-attempt check) reports as already far past the budget. return nowCallCount <= 1 ? 0 : 1_000_000; }; - const guard = createHeadroomGuard( + const guard = mockedCreateHeadroomGuard( t, { enabled: true, @@ -431,6 +437,10 @@ describe('dynamic-allocation-active — createHeadroomGuard.retryOnQuota', () => } expect(caught).to.equal(initialError); expect(fn).to.have.been.calledOnce; // only the initial call — no poll attempt was ever made + expect(recordQuotaRetryOutcome).to.have.been.calledOnceWith( + 'exhausted', + { attempt: 0, callSite: 'unknown' }, + ); }); it('ON, the deadline is SHARED across two sequential retryOnQuota calls on the SAME guard, not recomputed per call (MysticatBot review)', async () => {