diff --git a/agents/honker/work/queue-194/BUILD.md b/agents/honker/work/queue-194/BUILD.md index 2cde30412..6e3568701 100644 --- a/agents/honker/work/queue-194/BUILD.md +++ b/agents/honker/work/queue-194/BUILD.md @@ -725,3 +725,33 @@ tokens are gone, but neither refusal says so. Only `credential-revoke-failed` ha re-checks under its lock); or run `beforeWrite` inside `writer.write` after the `:188` check; and in either case give `revision-conflict` and `outcome-unknown` text that says tokens may already be revoked when a revoke ran. + +### sage-1634: N1 fixed in b3 (2026-10-21T01:20Z) + +Sage ruled N1 a blocker for 194: it is the row's own code in files the row already changes. Option 1 is in. +The section above stays as written; this entry supersedes its "no b3 change". + +- `business/src/instance-settings.mjs` `commit()`: before `beforeWrite`, compare the requested revision with + `writer.read(business).revision` and refuse `revision-conflict` on a difference, without calling + `beforeWrite`. It runs for every caller (add, set, veto, hire, fire, delete). Every caller already read the + file with `writer.read` (it validates), so a file that went invalid in between refuses `BusinessError`, + also before any revoke. +- Fire reads its revision fresh (`:311`) in the same synchronous call, so it reaches a stale revision only + through another process writing between that read and `commit()`. No test makes that happen without an + injected writer; the check is the same line, and the delete test kills both mutants. +- Test: bus `lifecycle-revoke.test.mjs` T21. A delete raised on revision R and taken; a credential issue for + rev-3 writes the file; the delete refuses `revision-conflict`. The business file's bytes and rev-2's + Credentials entry are unchanged, the vikunja token file is still there with its bytes, the store holds only + it, and the status reads valid. +- Refusal text: `revision-conflict` says the file changed since that revision (CLI `instances.mjs` REASONS, + Console `serve.mjs` INSTANCE_REASONS). Neither says anything was revoked, so no change. The Console's + AGENT_REASONS has no entry for it. +- Mutants for the slot: M76 drops the early check (T21 fails: the token file is removed); M77 moves it after + `beforeWrite` (T21 fails the same way, then refuses). +- Runs, singly or whole as Sage asked: business whole 207 pass, bus whole 525 pass, seat whole 164 pass + (sandbox.test included), cli instances 14, cli lifecycle 16, webui agents 8, all 0 fail. + +Follow-up (option 2, not in b3): run `beforeWrite` inside `writer.write`, under its lock and after the +`organization.mjs:188` check. Until then a window remains: a write landing between the early read and the +writer's lock still lets a fire or delete revoke and then refuse `revision-conflict`, and an `outcome-unknown` +after a revoke still doesn't say the tokens are gone. diff --git a/packages/bus/tests/lifecycle-revoke.test.mjs b/packages/bus/tests/lifecycle-revoke.test.mjs index 464885fe4..8b8c7ccec 100644 --- a/packages/bus/tests/lifecycle-revoke.test.mjs +++ b/packages/bus/tests/lifecycle-revoke.test.mjs @@ -142,3 +142,23 @@ test('T20 delete: with no entry in the running bus, a token file is cleared and assert.equal(f.bytes(), before); assert.deepEqual(f.status('coder'), { gitea: 'not-issued', vikunja: 'valid' }); }); + +test('T21 delete: a write between the raise and the take makes the delete refuse revision-conflict before it revokes anything', async (t) => { + const f = await setup(t); + f.ask('credentials.issue', { agent: 'rev-2', service: 'vikunja', token: TOKEN, date: '2099-01-01' }); + const { request } = f.ask('instances.set', { revision: f.ask('instances').revision, instance: 'rev-2', op: 'delete' }); + assert.equal(f.bus.lifecycleTake({ business: 'demo', request }).go, true); + // A write after the raise: the request's revision is now stale. + const raised = f.bytes(); + f.ask('credentials.issue', { agent: 'rev-3', service: 'gitea', token: 'fake_other_token_9876543210', date: '2099-02-01' }); + assert.ok(f.bytes() !== raised, 'the issue wrote the business file'); + const before = f.bytes(); + const entry = structuredClone(JSON.parse(before).roles['rev-2'].credentials); + assert.equal(f.code(() => f.bus.lifecycleDelete({ business: 'demo', request })), 'revision-conflict'); + // Nothing revoked: the token stays issued, Credentials is unchanged and no store file is removed. + assert.equal(f.bytes(), before); + assert.deepEqual(JSON.parse(f.bytes()).roles['rev-2'].credentials, entry); + assert.equal(readFileSync(join(f.agentDir('rev-2'), 'vikunja'), 'utf8'), TOKEN + '\n'); + assert.deepEqual(readdirSync(f.agentDir('rev-2')), ['vikunja']); + assert.deepEqual(f.status('rev-2'), { gitea: 'not-issued', vikunja: 'valid' }); +}); diff --git a/packages/business/src/instance-settings.mjs b/packages/business/src/instance-settings.mjs index 3a8c6aeae..73364355f 100644 --- a/packages/business/src/instance-settings.mjs +++ b/packages/business/src/instance-settings.mjs @@ -367,6 +367,11 @@ export function instanceSettings({ dir, rolesDir, system, levels = () => null, r } } } + // Row 194 N1 (sage-1634): a delete writes on the revision its request read when it was raised, + // so a write since then makes it stale. The revision is checked here, before the revoke, because + // the writer's own check comes after it. A write between this read and the writer's lock still + // revokes and then refuses (BUILD.md follow-up). + if (writer.read(business).revision !== revision) throw new OrganizationError("revision-conflict", "organization revision changed; read again"); if (beforeWrite) beforeWrite(); const receipt = writer.write(business, next, { revision, actor, context }); return { ...read(business), transaction: receipt.transaction, restart: false };