From 28d0b76f134b72ce4efbe72769cf0306a0a697e3 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sun, 1 Nov 2026 02:53:17 -0600 Subject: [PATCH] fix(events): core.lease keeps a module's retry: false on read/apply refusals core.lease rebuilt both failure envelopes as { ok: false, error }, so a module's retry: false was dropped and the dispatcher retried a permanent refusal (e.g. module-rust's events-disabled) three times. Both returns now pass retry: false through and leave retry undefined otherwise, so the dispatcher's default still applies. Three tests in eventModuleContract cover read, apply (reservation still released) and the default. Closes #206 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- server/src/config/coreEventActions.js | 10 ++- server/test/eventModuleContract.test.js | 86 +++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/server/src/config/coreEventActions.js b/server/src/config/coreEventActions.js index dd477d1..7dc98af 100644 --- a/server/src/config/coreEventActions.js +++ b/server/src/config/coreEventActions.js @@ -442,8 +442,12 @@ const ACTIONS = [ const baseline = await lease.read({ target }) if (!baseline || baseline.ok !== true) { + // A module that says `retry: false` has said a retry cannot help, so that + // verdict is passed through; anything else stays undefined so the + // dispatcher's own default (retry) still applies. return { ok: false, + retry: baseline && baseline.retry === false ? false : undefined, error: baseline && baseline.error ? String(baseline.error) : `could not read the current value of ${lease.label}`, @@ -494,7 +498,11 @@ const ACTIONS = [ } if (!applied || applied.ok !== true) { await resourcesDb.markReverted(reserved.id) - return { ok: false, error: applied && applied.error ? String(applied.error) : `${lease.label} refused the new value` } + return { + ok: false, + retry: applied && applied.retry === false ? false : undefined, + error: applied && applied.error ? String(applied.error) : `${lease.label} refused the new value`, + } } await resourcesDb.confirm(reserved.id) diff --git a/server/test/eventModuleContract.test.js b/server/test/eventModuleContract.test.js index 45c13a2..c347bc9 100644 --- a/server/test/eventModuleContract.test.js +++ b/server/test/eventModuleContract.test.js @@ -813,6 +813,92 @@ test('a dry run of core.lease checks everything and takes nothing', async () => assert.equal(bad.outcome, 'terminal') }) +// ── A lease's refusal keeps its `retry: false` (website#206) ──────────────── +// +// `core.lease` rebuilt both failure returns as `{ ok: false, error }`, so a module +// that said "a retry cannot help" (a shard that refuses the value outright, a +// target that does not exist) was dispatched as `retry: true` and the run paused +// and re-asked on a loop. The module's `retry: false` now passes through, and +// anything else is left undefined so the dispatcher's own default still applies. +// +// The reservation lands in MariaDB, which these tests do not have, so the two +// calls `core.lease` makes on it are swapped for recorders and put back after. + +const REFUSING_LEASES = `module.exports = (ctx, api) => { + const lease = (id, read, apply) => ({ + id, label: 'Gain rate', type: 'float', min: 0.5, max: 5, maxDurationMs: 3600000, + read, apply, async restore() { return { ok: true } }, + }) + const fine = async () => ({ ok: true, value: 1 }) + const accepts = async () => ({ ok: true }) + api.registerEventLeases([ + lease('demo.read.final', async () => ({ ok: false, retry: false, error: 'no such spawner' }), accepts), + lease('demo.read.soft', async () => ({ ok: false, error: 'shard is restarting' }), accepts), + lease('demo.apply.final', fine, async () => ({ ok: false, retry: false, error: 'value refused' })), + lease('demo.apply.soft', fine, async () => ({ ok: false, error: 'shard is restarting' })), + ]) +}` + +/** Run `fn` with the lease reservation calls recorded rather than sent to a database. */ +async function withStubbedReservations(fn) { + // eslint-disable-next-line global-require + const resourcesDb = require('../src/model/events/eventRunResources.db') + const original = { reserve: resourcesDb.reserve, markReverted: resourcesDb.markReverted } + const calls = { reserved: 0, reverted: [] } + resourcesDb.reserve = async () => { calls.reserved += 1; return { ok: true, id: 77 } } + resourcesDb.markReverted = async (id) => { calls.reverted.push(id) } + try { + return await fn(calls) + } finally { + Object.assign(resourcesDb, original) + } +} + +const leaseStep = (lease) => step('core.lease', { lease, value: '3', minutes: 10 }) + +test('a lease read refusal that says retry: false is terminal, not retried', async () => { + assertRegistered(loadModule('demo', REFUSING_LEASES)) + registries.registerCore() + + await withStubbedReservations(async (calls) => { + const result = await dispatch.dispatchStep(leaseStep('demo.read.final'), { run: RUN }) + assert.equal(result.outcome, 'terminal') + assert.match(result.error, /no such spawner/) + // Nothing was reserved, so there is nothing to give back. + assert.equal(calls.reserved, 0) + }) +}) + +test('a lease apply refusal that says retry: false is terminal, and the reservation is still released', async () => { + assertRegistered(loadModule('demo', REFUSING_LEASES)) + registries.registerCore() + + await withStubbedReservations(async (calls) => { + const result = await dispatch.dispatchStep(leaseStep('demo.apply.final'), { run: RUN }) + assert.equal(result.outcome, 'terminal') + assert.match(result.error, /value refused/) + assert.equal(calls.reserved, 1) + assert.deepEqual(calls.reverted, [77]) + }) +}) + +test('a lease refusal that does not say retry: false is still retried', async () => { + assertRegistered(loadModule('demo', REFUSING_LEASES)) + registries.registerCore() + + await withStubbedReservations(async (calls) => { + const read = await dispatch.dispatchStep(leaseStep('demo.read.soft'), { run: RUN }) + assert.equal(read.outcome, 'retry') + assert.match(read.error, /shard is restarting/) + + const apply = await dispatch.dispatchStep(leaseStep('demo.apply.soft'), { run: RUN }) + assert.equal(apply.outcome, 'retry') + assert.match(apply.error, /shard is restarting/) + // The apply refusal released its reservation exactly as before. + assert.deepEqual(calls.reverted, [77]) + }) +}) + // ── Targeted leases and searchable sources (Phase 12b) ────────────────────── test('a targeted lease is a family of values, and the target rides in the ref', async () => {