row 249 (#1751): a grant whose install never returned records hostLineLeft, abort and timeout tests, G8 and the :432 comment (Gizmoduck)

agents/gizmoduck/work/queue-249/BUILD.md

@@ -0,0 +1,82 @@
+# Row 249 build notes (Gizmoduck)
+
+Gitea #1751, Vikunja P19 445 under #436. Brief: docs/plans/2026-10-20_host-credentials.md, section
+"Machine grant row b follow-up: the undo can't prove the remote add has ended" (6de1ebc62). Reviewer
+Duckworth, gate owner Sage. Branch gizmoduck/row249 from origin/refactor bc579f7a8. No file under
+packages/bus/src changed between 6d5545d44 and bc579f7a8, so the brief's line numbers hold on the base.
+Line numbers below are on this branch.
+
+## Correction to the brief
+
+The brief, copying gizmoduck-0139, puts the sweep's host-unknown refusal at broker.mjs:1350. On
+6d5545d44 it is :1349. On this branch it is :1354.
+
+## The change
+
+`#runHostGrant` in packages/bus/src/broker.mjs:
+
+- :943 adds `installed = false` next to `installing`.
+- :956 sets `installed = true` on the line after `await this.#hostGrant.install(...)`, before the
+  `stop()` that follows it. An abort that lands after install returned still ends revoked, with no hint,
+  because install returning means every request it sent has exited.
+- :1025, in the catch, inside `if (installing)` and after the remove: `if (!installed) extra =
+  { hostLineLeft: tag };`. The remove still runs first and its result doesn't matter. A remove that
+  throws sets the same hint as before (:1021).
+
+The rule keys on install not returning, not on `signal.aborted`. That covers an abort (host-run.mjs:136,
+SIGKILL to the local group), a timeout (host-run.mjs:131-134, SIGTERM, then SIGKILL after `graceMs`) and
+a dropped connection, which fails the request with no signal at all. `#hostGrantFailed` (:845) already
+adds "A line tagged X may be left on the host. Run: mosaic hosts sweep <host>" when `hostLineLeft` is
+set (:852), so the message needed no change.
+
+False hints. install is one call that runs `list`, maybe `remove`, then `add` (host-grant.mjs:329-333).
+Any failure inside it now records the hint, including a `list` that exits 1 before any add was sent, and
+an abort that lands before the first ssh starts (the G23 X5 case at "a revoke right after the spend").
+The brief accepts this for an add that exits nonzero. The other cases are the same kind: the line isn't
+on the host, and a sweep that doesn't list the tag drops the hint (G24, "a left tag no longer on the
+host", broker.mjs:1358). Narrowing it would need install to report which request was in flight, which is
+a change to host-grant.mjs's API that the brief doesn't ask for.
+
+## Row a's enroll
+
+The race can't happen there, because enrollment sends nothing to the host. `#runEnrollment`
+(broker.mjs:772-842) writes the record, calls `this.#hostGrant.enroll(business, host)` (:800) and
+commits the fingerprint. `enroll` (host-grant.mjs:247-286) runs ssh-keygen locally and writes the
+private half to its file. It opens no ssh session. The installer line goes on the host by hand: the
+notice tells ops-01 to write it, then to run `mosaic hosts check`. The undo at :835 is `unenroll`, which
+deletes the local key file. There is no remote add to outlive a kill, and no remote undo to race with.
+
+## Tests
+
+packages/bus/tests/grant-executor.test.mjs:
+
+- `lateAdd` (:450) is the shared tail. After the grant ends it asserts the code, `hostLineLeft` on
+  grant.failed, the sweep line in the notice, and that the undo's remove was the last request. Then it
+  writes `release`, so the held add writes its line and exits 0, and waits for the line in `granted`. It
+  checks there's no record, `hosts.status` lists the tag in `hostLineLeft`, `hosts.remove` refuses
+  host-line-left, `hosts.sweep` returns `removed: [tag]` and the line is gone, then `hosts.remove`
+  succeeds.
+- :467, the abort case. `hold-add`, the human approves, the add holds, `abortClaim` aborts the
+  executor's claim. Ends revoked.
+- :479, the timeout case. A 30 s wait is too long for the file, and making the timeout an option would
+  add a src seam, so the test wraps `install` through `opts.wrap` instead. The wrapper passes install its
+  own AbortController signal, waits for `holding`, aborts it, which SIGKILLs the local ssh group as the
+  timer does, then throws as a timed-out request does. The executor's signal isn't aborted, so it ends
+  host-failed. Nothing under src changes for it.
+- G8 (:282) changes. fail-list and fail-add now carry `hostLineLeft` and the sweep line. noop-add fails at
+  verify, after install returned, so it keeps no hint and the old message.
+- The comment at :435 (was :432) now says `skip` makes the held add exit without writing and that a
+  killed session's add can still write. G13 has no such comment.
+
+Both new tests write `release`, so their held stub exits before `t.after` removes the dir. Row 248's dir
+check isn't needed for them.
+
+## Mutant
+
+/mnt/storage/scratch/gizmo-235-out/mutants-249.py, on row 235's runner. C1 removes the :1025 line and
+runs grant-executor.test.mjs. Both new tests and G8 should fail under it. `--check` matches its anchor
+once.
+
+## Runs
+
+Pending Sage's slot: grant-executor.test.mjs whole, then C1. The bus suite runs on Sage's gate.

packages/bus/src/broker.mjs

@@ -940,6 +940,7 @@ export class Broker {
       if (signal.aborted) fail('revoked');
     };
     let installing = false,
+      installed = false,
       registered = false,
       committed = false;
     try {
@@ -952,6 +953,7 @@ export class Broker {
         step('install');
         installing = true;
         await this.#hostGrant.install(business, g.host, record, tag, key.publicLine, signal);
+        installed = true;
         stop();
         await this.#hostGrant.verify(g.host, record, key, signal);
         stop();
@@ -1018,6 +1020,9 @@ export class Broker {
         } catch {
           extra = { hostLineLeft: tag };
         }
+        // An abort or a timeout kills only the local ssh, so an add that hasn't returned may still write
+        // its line after the remove has run. Only install returning proves it ended (row 249).
+        if (!installed) extra = { hostLineLeft: tag };
       }
       try {
         this.#store.transaction(() => this.#hostGrantFailed(business, decision, g, code, extra));

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

@@ -287,14 +287,17 @@ test('G8: a failure at install or verify removes the line and leaves no record,
     const { id, end } = await f.approve();
     assert.equal(end.code, 'host-failed', flag);
     assert.equal(end.tag, f.tag);
-    assert.equal('hostLineLeft' in end, false);
+    // An install that didn't return keeps the sweep hint, since its add may still be running (row 249).
+    // Here the hint is false, and a sweep drops it (G24). noop-add fails at verify, after install returned.
+    const returned = flag === 'noop-add';
+    assert.equal(end.hostLineLeft, returned ? undefined : f.tag, flag);
     assert.equal(f.requests().at(-1), `remove ${USER} ${f.tag}`, flag);
     assert.deepEqual(f.granted(), []);
     assert.equal(f.record(), undefined);
     assert.equal(existsSync(f.keyFile), false);
     assert.equal(f.state(), undefined);
     assert.deepEqual(f.grantEvents().filter((e) => e.grantDecision === id).map((e) => e.action), ['grant.step', 'grant.failed']);
-    assert.match(f.messages(id)[0].body, /failed: host-failed\.$/);
+    assert.match(f.messages(id)[0].body, returned ? /failed: host-failed\.$/ : /Run: mosaic hosts sweep web1$/);
   }
 });
 
@@ -430,7 +433,8 @@ test('G23: a revoke during install ends grant.failed revoked with no key file, n
   await until(() => existsSync(join(f.dir, 'holding')));
   assert.deepEqual(f.credentials.abortClaim('demo/ops', 'host.web1'), { pending: true });
   await until(() => f.ended(d.id));
-  // The held add then exits without writing, as a killed session's would.
+  // `skip` makes the held add exit without writing. A killed session's add can still write; the row 249
+  // tests below cover that.
   f.flag('skip');
   f.flag('release');
   assert.equal(f.ended(d.id).code, 'revoked');
@@ -442,6 +446,57 @@ test('G23: a revoke during install ends grant.failed revoked with no key file, n
   assert.equal(f.credentials.busy('demo/ops', 'host.web1'), false);
 });
 
+// Row 249. An abort and a timeout kill only the local ssh, and sshd sends a non-pty command no SIGHUP,
+// so the held add goes on. Released after the undo's remove, it writes its line and exits 0.
+const lateAdd = async (f, id, code) => {
+  await until(() => f.ended(id));
+  const end = f.ended(id);
+  assert.equal(end.code, code);
+  assert.equal(end.hostLineLeft, f.tag);
+  assert.match(f.messages(id)[0].body, new RegExp(`tagged ${f.tag} may be left on the host\\. Run: mosaic hosts sweep web1$`));
+  assert.equal(f.requests().at(-1), `remove ${USER} ${f.tag}`);
+  f.flag('release');
+  await until(() => f.granted().some((l) => l.endsWith(' ' + f.tag)));
+  assert.equal(f.record(), undefined);
+  assert.deepEqual((await f.call('human', 'hosts.status'))[0].hostLineLeft, [f.tag]);
+  await refusesWith(f.call('human', 'hosts.remove', { host: 'web1' }), 'host-line-left');
+  assert.deepEqual(await f.call('human', 'hosts.sweep', { host: 'web1' }), { host: 'web1', removed: [f.tag], hostLineLeft: [] });
+  assert.deepEqual(f.granted(), []);
+  assert.deepEqual(await f.call('human', 'hosts.remove', { host: 'web1' }), { host: 'web1', removed: true });
+};
+
+test('G23 (row 249): an add that writes after an abort\'s remove leaves hostLineLeft until the sweep', async (t) => {
+  const f = await setup(t);
+  f.flag('hold-add');
+  const d = await f.raise();
+  f.call('human', 'decision.resolve', { id: d.id, choice: 'yes' });
+  await until(() => existsSync(join(f.dir, 'holding')));
+  assert.deepEqual(f.credentials.abortClaim('demo/ops', 'host.web1'), { pending: true });
+  await lateAdd(f, d.id, 'revoked');
+});
+
+// The timeout's kill without its 30 s wait: install gets its own signal, which SIGKILLs the local ssh
+// group as host-run.mjs's timer does, and the call then fails as a timed-out request does.
+test('G23 (row 249): an add that writes after a timed-out install\'s remove leaves hostLineLeft until the sweep', async (t) => {
+  const f = await setup(t, {
+    wrap: (hg) => ({
+      ...hg,
+      install: async (business, host, record, tag, line) => {
+        const timer = new AbortController();
+        const out = hg.install(business, host, record, tag, line, timer.signal);
+        await until(() => existsSync(join(f.dir, 'holding')));
+        timer.abort();
+        await out.catch(() => {});
+        throw new Error('timed out');
+      },
+    }),
+  });
+  f.flag('hold-add');
+  const d = await f.raise();
+  f.call('human', 'decision.resolve', { id: d.id, choice: 'yes' });
+  await lateAdd(f, d.id, 'host-failed');
+});
+
 test('G23: a revoke recorded after the raise and before the start ends step 1 revoked, and a held slot ends host-busy', async (t) => {
   const f = await setup(t);
   const revoked = f.fixtureGrant();