bus: row 236 r2, a revoke during step 7 or a settle ends revoked; sweep fire retry checks the user id (Dewey B1, B2, #1742)

agents/megavolt/work/queue-236/BUILD.md

@@ -64,6 +64,30 @@ Each is a reading of the design where it left room. Dewey may rule otherwise.
 17. X-G7 changes the binding with a fixture decision whose hash differs (row 234's G9 pattern), since the
     broker keeps its own copy of the business. The admin url check takes row 199's `parseGiteaUrl`, which
     accepts the origin with or without a trailing slash.
+18. r2, Dewey B1 (dewey-0781): step 8 and both settle branches (forward, back) check for a `grant.revoked`
+    of the agent after the decision's raise, in the same transaction as their write. A revoke that came
+    while step 7's or the settle's delete awaited took the record and both tokens, so the grant ends
+    `revoked` with the human's notice, as a revoke of a running executor ends it (X-G11), and no record is
+    written back. N9's restart case stays silent: a restart never settles a revoked rotation.
+19. r2, Dewey B2: a sweep retry of `tokens` or `login` checks the user's id first, as the revoke close does,
+    against the `gitea_users` row (it survives a fire). A 404 skips with `gitea-user-gone`, a 200 with
+    another id with `gitea-user-foreign`, any other status leaves the entry. Skips go in `grant.swept` and
+    the sweep answer as `skipped: [{agent, user, id, reason}]`; a skipped entry is not left, so the next
+    sweep does not retry it.
+
+## Named for DEFERRED (r2, Dewey N1-N4)
+
+None is a small local change, so none is in r2.
+
+- N1: an admin rotate cut between the mint and the slot write leaves a token at Gitea that no event names.
+  A `grant.admin rotate-begin {oldName}` before the mint would close it; that is a new event shape.
+- N2: `adminTokenLeft` is `unknown` when `/api/v1/token` gives no name. Which 1.27 F-fact shows the name
+  needs a check against the container, not a code change.
+- N3: admin set or remove records `adminTokenLeft: null`, so a left token drops out of status. Carrying it
+  forward lets `rotate --finish` delete by name under a slot of another account or url; refusing remove
+  while one is left blocks the human's remove when Gitea is down. Either needs a ruling.
+- N4: rotate or remove while a gitea claim is held. Refusing it blocks the human's remove during a run;
+  the running executor keeps the api it began with. Needs a ruling or a line in the README.
 
 ## Q188
 
@@ -93,3 +117,12 @@ were added before any mutant run:
   before its Gitea delete", which holds the DELETE open and reads the file and state.
 - M37 (evidence without its kind filter): "X-G15: a look-alike spend under another kind with the grant writer
   pair is not an open grant".
+
+r2 tests, each red on 8bf442be9 by reading:
+
+- B1: "X-G11: a revoke during a rotation step 7 ends it revoked, and step 8 writes no record back" and
+  "X-G11: a revoke while a sweep settles a rotation, forward or back, ends it revoked with no record".
+  On 8bf442be9 step 8 and the settle write the record and `grant.applied` (forward) or `grant.failed
+  interrupted settled:back` after the revoke deleted it.
+- B2: "X-G14: a sweep retries a fire only for the user it fired: a remade or gone user is skipped and
+  named". On 8bf442be9 the retry fires the remade user (token list, delete, PATCH) and has no `skipped`.

packages/bus/README.md

@@ -228,7 +228,7 @@ an operator inspect content and delivery evidence after an uncertain result.
 | `grants.status` | `{service:"gitea"}`; the human, the tester or the chief (row 236, #1742, #436b r5). The admin slot's url, account and rotate-by date with `rotationDue`, `adminTokenLeft`, the `gitea_users` and `gitea_grants` records, approved decisions not yet run (`spentAtNextStart`), grants `running` or `settlePending`, agents held as `tokenUnknown`, the `serviceLeft` entries a sweep retries, and revokes still `revoking`. No token. |
 | `grants.run` | `{decision}`; the human only. Starts the Gitea executor of a `credential.mint` decision with a gitea grant the human said yes to, as the yes itself does (X1). Step 0's refusals come back with no record: `not-authorized`, `decision-consumed`, `grant-token-unknown`, `grant-busy`, `gitea-not-configured`. Returns `{decision,agent,started:true}`; the outcome is a `grant.applied` or `grant.failed` and a notice. |
 | `grants.revoke` | `{service:"gitea",agent,reason}`; the human, the business's assistant, or the tester when `bus.tester` is exactly `true`. The token leaves memory and its file goes, an executor in flight aborts, then one transaction deletes the `gitea_grants` record and writes `grant.revoked` with the token ids to delete. The deletes run with the admin token after C1 to C4, and `grant.revoke.closed` records `serviceLeft`, a list. `credentials.revoke AGENT gitea` and `credentials.delete` do the same for a stack-made user; delete also deletes every token of the user and sets `prohibit_login` (fire). |
-| `grants.sweep` | `{service:"gitea"}`; the human or the tester. Closes open revokes, settles open rotations again, and retries each `serviceLeft` entry. Writes `grant.sweep` and `grant.swept {left,settled,closed}`. |
+| `grants.sweep` | `{service:"gitea"}`; the human or the tester. Closes open revokes, settles open rotations again, and retries each `serviceLeft` entry. A fire retry (`tokens`, `login`) first checks the user's id as the revoke close does, and a gone or remade user is skipped with `gitea-user-gone` or `gitea-user-foreign`. Writes `grant.sweep` and `grant.swept {left,skipped,settled,closed}`. |
 | `grants.admin.set`, `grants.admin.rotate`, `grants.admin.remove` | The human only (#436b r5 section 3). set takes `{service,url,account,rotateBy,token}`, checks C1 to C4 with the token, then writes the slot under `<configDir>/secrets/grants/<business>/gitea`; a slot already there is `grant-admin-exists`. rotate `{service,rotateBy}` mints a successor admin token and names the old one in `adminTokenLeft`; `{service,finish:true}` deletes it. Rotation voids the approved decisions not yet run, as their `adminSha256` no longer matches. remove drops the slot. Each writes `grant.admin` with no secret. |
 
 Each of the three leaves one receipt in the organization's `changes.jsonl`: the actor and a

packages/bus/src/broker.mjs

@@ -1703,12 +1703,16 @@ export class Broker {
       if (!previous) return 'applied';
       // Step 7, after the new token is live.
       const serviceLeft = (await this.#giteaDelete(api, user, previous.tokenId).catch(() => false)) ? [] : [previous.tokenId];
-      // Step 8.
+      // Step 8. A revoke during step 7 took the record and both tokens: it ends revoked, with no write.
-      this.#store.transaction(() => {
+      return this.#store.transaction(() => {
+        if (this.#giteaRevokedSince(business, decision, agent)) {
+          this.#giteaFailed(business, decision, agent, 'revoked', { user });
+          return 'revoked';
+        }
         write();
         applied(serviceLeft.length ? { serviceLeft } : {});
+        return 'applied';
       });
-      return 'applied';
     } catch (e) {
       if (committed) {
         // Live already: a late abort is an ordinary revoke's to handle. Only a store write failed, so the
@@ -1760,6 +1764,13 @@ export class Broker {
           'ON CONFLICT(business,agent) DO UPDATE SET user=excluded.user,token_id=excluded.token_id,token_name=excluded.token_name,projects=excluded.projects,rotate_by=excluded.rotate_by,decision=excluded.decision',
         business, agent, user, t.tokenId, t.tokenName, JSON.stringify(t.projects), t.rotateBy, t.decision,
       );
+    // A revoke while a branch below awaits took the record and both tokens: the grant ends revoked, as a
+    // revoke of a running executor ends it, and nothing is written back. Run inside the branch's transaction.
+    const revoked = () => {
+      if (!this.#giteaRevokedSince(business, decision, agent)) return false;
+      this.#giteaFailed(business, decision, agent, 'revoked', { user });
+      return true;
+    };
     if (who.status === 200 && who.id === s.tokenId) {
       try {
         this.#giteaKeys.reload(business, agent, g.rotateBy);
@@ -1767,7 +1778,8 @@ export class Broker {
         return 'settle-pending';
       }
       const serviceLeft = (await this.#giteaDelete(api, user, s.previousTokenId)) ? [] : [s.previousTokenId];
-      this.#store.transaction(() => {
+      return this.#store.transaction(() => {
+        if (revoked()) return 'revoked';
         record({ tokenId: s.tokenId, tokenName: s.tokenName, projects: g.target.projects, rotateBy: g.rotateBy, decision });
         this.#grantEvent(
           business,
@@ -1775,16 +1787,17 @@ export class Broker {
           agent,
         );
         this.#grantNotice(business, decision, `The Gitea token rotation of ${agent} (decision ${decision}) settled forward: the new token is live.`);
+        return 'applied';
       });
-      return 'applied';
     }
     if (who.status === 200 && who.id === s.previousTokenId) {
       const serviceLeft = (await this.#giteaDelete(api, user, s.tokenId)) ? [] : [s.tokenId];
-      this.#store.transaction(() => {
+      return this.#store.transaction(() => {
+        if (revoked()) return 'revoked';
         record(s.previous);
         this.#giteaFailed(business, decision, agent, 'interrupted', { user, settled: 'back', ...(serviceLeft.length ? { serviceLeft } : {}) });
+        return 'interrupted';
       });
-      return 'interrupted';
     }
     if (who.status === 401) {
       const serviceLeft = [];
@@ -2099,6 +2112,7 @@ export class Broker {
     }
     const admin = await this.#giteaAdminChecked(business, log);
     const still = [];
+    const skipped = [];
     for (const e of left) {
       if (e.fresh) {
         still.push({ agent: e.agent, user: e.user, id: e.id });
@@ -2107,14 +2121,24 @@ export class Broker {
       let ok = false;
       if (admin && e.user) {
         if (e.id === 'tokens' || e.id === 'login') {
-          const r = await this.#giteaFire(admin.api, e.user);
+          // A fire retry checks the user's id first, as the revoke close does: the gitea_users row
-          ok = !r.includes(e.id);
+          // survives a fire, and a user remade under the same name is not the one that was fired.
+          const u = this.#store.get('SELECT id FROM gitea_users WHERE business=? AND agent=? AND user=?', business, e.agent, e.user);
+          const r = await admin.api.userGet(e.user);
+          const reason = r.status === 404 ? 'gitea-user-gone' : r.status === 200 && (!u || r.json?.id !== u.id) ? 'gitea-user-foreign' : null;
+          if (reason) {
+            skipped.push({ agent: e.agent, user: e.user, id: e.id, reason });
+            continue;
+          }
+          if (r.status === 200) ok = !(await this.#giteaFire(admin.api, e.user)).includes(e.id);
         } else ok = await this.#giteaDelete(admin.api, e.user, e.id).catch(() => false);
       }
       if (!ok) still.push({ agent: e.agent, user: e.user, id: e.id });
     }
-    this.#store.transaction(() => this.#grantEvent(business, { action: 'grant.swept', method: 'gitea', sweep, left: still, settled, closed, by }));
+    this.#store.transaction(() =>
-    return { left: still, settled, closed };
+      this.#grantEvent(business, { action: 'grant.swept', method: 'gitea', sweep, left: still, skipped, settled, closed, by }),
+    );
+    return { left: still, skipped, settled, closed };
   }
   // `mosaic grants status gitea`.
   #giteaStatus(business) {

packages/bus/tests/gitea-grant.test.mjs

@@ -939,6 +939,58 @@ test('X-G11: a revoke unloads memory and removes the file before its Gitea delet
   assert.deepEqual(f.userTokens(), []);
 });
 
+test('X-G11: a revoke during a rotation step 7 ends it revoked, and step 8 writes no record back', async (t) => {
+  const f = await setup(t);
+  await f.approve();
+  const oldId = f.record().token_id;
+  let release;
+  const held = new Promise((r) => (release = r));
+  f.gitea.rule((e) => e.method === 'DELETE' && e.path === `/api/v1/users/${USER}/tokens/${oldId}`, async () => {
+    await held;
+    return { pass: true };
+  });
+  const d = await f.raise();
+  f.call('human', 'decision.resolve', { id: d.id, choice: 'yes' });
+  await until(() => f.gitea.calls().includes(`DELETE /api/v1/users/${USER}/tokens/${oldId}`));
+  const revoke = f.call('human', 'grants.revoke', { service: 'gitea', agent: 'coder', reason: 'stop' });
+  release();
+  await revoke;
+  await until(() => f.ended(d.id));
+  await f.broker.giteaIdle();
+  assert.equal(f.ended(d.id).code, 'revoked');
+  assert.equal(f.grantEvents().some((e) => e.action === 'grant.applied' && e.grantDecision === d.id), false);
+  assert.equal(f.record(), undefined);
+  assert.deepEqual(f.userTokens(), []);
+});
+
+test('X-G11: a revoke while a sweep settles a rotation, forward or back, ends it revoked with no record', async (t) => {
+  for (const forward of [true, false]) {
+    const f = await setup(t);
+    const { live, next, id } = await openRotation(f);
+    if (forward) writeFileSync(f.tokenFile, next.sha1 + '\n');
+    // Forward deletes the old token, back the new one: hold that delete.
+    const target = forward ? live.token_id : next.id;
+    let release;
+    const held = new Promise((r) => (release = r));
+    f.gitea.rule((e) => e.method === 'DELETE' && e.path === `/api/v1/users/${USER}/tokens/${target}`, async () => {
+      await held;
+      return { pass: true };
+    });
+    const sweep = f.call('human', 'grants.sweep', { service: 'gitea' });
+    await until(() => f.gitea.calls().includes(`DELETE /api/v1/users/${USER}/tokens/${target}`));
+    const revoke = f.call('human', 'grants.revoke', { service: 'gitea', agent: 'coder', reason: 'stop' });
+    release();
+    await revoke;
+    const swept = await sweep;
+    await f.broker.giteaIdle();
+    assert.deepEqual(swept.settled, [{ decision: id, outcome: 'revoked' }], forward ? 'forward' : 'back');
+    assert.equal(f.ended(id).code, 'revoked');
+    assert.equal(f.grantEvents().some((e) => e.grantDecision === id && e.settled), false);
+    assert.equal(f.record(), undefined);
+    assert.deepEqual(f.userTokens(), []);
+  }
+});
+
 test('X-G12: a token that reaches wider than its forks fails with the repository named', async (t) => {
   const collab = await setup(t);
   collab.gitea.addRepo('other', 'secret', { collaborators: [USER] });
@@ -1068,6 +1120,36 @@ test('X-G14: a fire whose user is gone or remade skips the fire steps and says w
   assert.equal(g.gitea.users.get(USER).prohibit_login, false);
 });
 
+test('X-G14: a sweep retries a fire only for the user it fired: a remade or gone user is skipped and named', async (t) => {
+  for (const reason of ['gitea-user-foreign', 'gitea-user-gone']) {
+    const f = await setup(t);
+    await f.approve();
+    // The fire's user check answers 500, so both fire steps are left for the sweep.
+    const off = f.gitea.rule((e) => e.method === 'GET' && e.path === `/api/v1/users/${USER}`, { status: 500, json: {} });
+    f.call('human', 'credentials.delete', { agent: 'coder', reason: 'fired' });
+    await until(() => f.grantEvents().some((e) => e.action === 'grant.revoke.closed'));
+    await f.broker.giteaIdle();
+    off();
+    assert.deepEqual(f.grantEvents().find((e) => e.action === 'grant.revoke.closed').serviceLeft, ['tokens', 'login']);
+    f.gitea.removeUser(USER);
+    if (reason === 'gitea-user-foreign') f.gitea.addUser(USER, { scopes: SEAT_SCOPES, tokenName: 'theirs' });
+    const before = f.gitea.requests.length;
+    const swept = await f.call('human', 'grants.sweep', { service: 'gitea' });
+    const after = f.gitea.requests.slice(before).map((r) => `${r.method} ${r.path.split('?')[0]}`);
+    assert.equal(after.some((c) => c.startsWith('PATCH ') || c.startsWith('DELETE ') || c.endsWith('/tokens')), false, reason);
+    assert.deepEqual(swept.left, []);
+    const skipped = [{ agent: 'coder', user: USER, id: 'tokens', reason }, { agent: 'coder', user: USER, id: 'login', reason }];
+    assert.deepEqual(swept.skipped, skipped);
+    assert.deepEqual(f.grantEvents().findLast((e) => e.action === 'grant.swept').skipped, skipped);
+    if (reason === 'gitea-user-foreign') {
+      assert.equal(f.gitea.users.get(USER).prohibit_login, false);
+      assert.deepEqual(f.userTokens().map((x) => x.name), ['theirs']);
+    }
+    // Skipped is not left: the next sweep has nothing to retry.
+    assert.deepEqual((await f.call('human', 'grants.status', { service: 'gitea' })).serviceLeft, []);
+  }
+});
+
 test('X-G14: a revoke Gitea cannot reach stays open, and a restart closes it', async (t) => {
   const f = await setup(t);
   await f.approve();