From 3dc824ea12879584e804c1e3e2496eda19849205 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 2 Nov 2026 20:05:28 -0600 Subject: [PATCH] fix(row 333): a failed lineage lookup forgets orphan ids the document dropped (Vikunja #783) observeOrphans kept the whole remembered set when the lookup failed, so a leaf removed and added back during the failure logged no second orphan line. It now keeps only the ids the refreshed document still holds and adds none. Quota decisions are unchanged: a failed lookup still refuses. --- packages/bus/src/limits.mjs | 11 ++++-- packages/bus/tests/p5-key-lineage.test.mjs | 42 +++++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/bus/src/limits.mjs b/packages/bus/src/limits.mjs index 593deb79a..c6ec3ba90 100644 --- a/packages/bus/src/limits.mjs +++ b/packages/bus/src/limits.mjs @@ -85,11 +85,16 @@ export function createLimits({ store, read, log, makeLoader = createLoader, keyF return ids; } // The orphaned ids now, [] when no daily key quota is set, or null when the lineages can't be - // read (the remembered set then stays as it was). + // read. Then the remembered set keeps only the ids the document still holds and gains none: an + // unreadable lookup proves no leaf orphaned, but the document does prove which leaves were removed. function observeOrphans(business, limits) { - if (!dailyKeyIds(limits).length) { orphansLogged.set(business, new Set()); return []; } + const ids = dailyKeyIds(limits); + if (!ids.length) { orphansLogged.set(business, new Set()); return []; } const lineages = lineagesFor(business); - return lineages ? noteOrphans(business, limits, lineages) : null; + if (lineages) return noteOrphans(business, limits, lineages); + const before = orphansLogged.get(business); + if (before) orphansLogged.set(business, new Set([...before].filter(id => ids.includes(id)))); + return null; } // The business's lineages, or null when they can't be read or are malformed. function lineagesFor(business) { diff --git a/packages/bus/tests/p5-key-lineage.test.mjs b/packages/bus/tests/p5-key-lineage.test.mjs index b6087b8c6..9c8ed5706 100644 --- a/packages/bus/tests/p5-key-lineage.test.mjs +++ b/packages/bus/tests/p5-key-lineage.test.mjs @@ -51,6 +51,7 @@ async function fixture(t) { const f = { root, configDir, storeRoot, storeDir, proxy, runs, stopped, logs, receipts, prefix, doc: quotaDoc({}), + lookupFails: false, read: () => f.doc, start() { f.store = new Store(storeDir); @@ -61,7 +62,7 @@ async function fixture(t) { stopRuns: async rs => { for (const r of rs) { stopped.push(r.run); runs.delete('demo:' + r.run); } } }); f.limits = createLimits({ store: f.store, read: business => f.read(business), log: e => logs.push(e), keyForSubject: (business, subject) => modelUsageKey(f.credentials.status(), runs, business, subject), - keyLineages: business => modelKeyLineages(f.credentials.status(), business) }); + keyLineages: business => { if (f.lookupFails) throw new Error('lineage lookup down'); return modelKeyLineages(f.credentials.status(), business); } }); }, stop() { f.credentials.close(); f.store.close(); }, // A bus restart: every in-memory object goes; the record, the store and the file come back from disk. @@ -366,3 +367,42 @@ test('R4 an orphaned leaf removed and added back logs twice; repeated refreshes for (let i = 0; i < 3; i++) await f.limits.refresh('demo'); assert.deepEqual(f.logs, [line, line, line]); }); + +// #783: a failed lineage lookup cannot say which present leaves are orphans, so it adds none. It can +// still see which leaves the refreshed document dropped, and forgets those, so a leaf added back logs. +test('#783 a leaf removed while the lineage lookup fails logs again when it is added back; a leaf present throughout does not', async t => { + const f = await fixture(t); await f.keys.reconcile(); await f.keys.ensure('demo/coder'); + const live = f.record().keyId, X = hash('fired-agent-key'), Z = hash('another-fired-key'); + const lineFor = id => ({ event: 'limit-key-orphaned', business: 'demo', path: ['tokens', 'keys', id] }); + f.doc = quotaDoc({ [X]: quota(100), [live]: quota(100) }); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X]); + assert.deepEqual(f.logs, [lineFor(X)]); + // The lookup fails with X still in the file: nothing is known and nothing is logged. + f.lookupFails = true; + await f.limits.refresh('demo'); + assert.equal(f.limits.orphanedKeys('demo'), null); + assert.deepEqual(f.logs, [lineFor(X)]); + // X is removed while the lookup is down; the live leaf stays, so a daily key quota still exists. + f.doc = quotaDoc({ [live]: quota(100) }); await f.limits.refresh('demo'); + // X is added back before any good observation: still nothing to say. + f.doc = quotaDoc({ [X]: quota(100), [live]: quota(100) }); await f.limits.refresh('demo'); + assert.deepEqual(f.logs, [lineFor(X)]); + // The lookup recovers: the next observation logs X once more. + f.lookupFails = false; + await f.limits.refresh('demo'); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X]); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X)]); + // A second failure with X present throughout adds no line, and a leaf added during it is not + // called an orphan until the lookup can say so; then it logs once. + f.lookupFails = true; + f.doc = quotaDoc({ [X]: quota(100), [Z]: quota(100), [live]: quota(100) }); await f.limits.refresh('demo'); + assert.equal(f.limits.orphanedKeys('demo'), null); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X)]); + f.lookupFails = false; + await f.limits.refresh('demo'); await f.limits.refresh('demo'); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X, Z]); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X), lineFor(Z)]); + // Quota decisions still refuse while the lookup is down. + f.lookupFails = true; + assert.deepEqual(await f.limits.mayLaunch('demo', SUBJECT), { allow: false, check: null, code: 'proxy-key-refused' }); +});