row 249: row 235 follow-up, a grant whose install never returned records hostLineLeft, since the remote add can outlive an abort or a timeout (#1751)

An abort or a timeout kills only the local ssh. A non-pty sshd command gets no SIGHUP, so the remote
add can still write its line after the undo's remove. The broker now marks install done only after
`await install(...)` returns. When it never returned, the undo records hostLineLeft with the tag, so
grant.failed and status name the line, hosts.remove refuses until `mosaic hosts sweep` clears it.
The :432 test comment now matches. Row a's enroll does no remote add, so it has no such race.

Candidate gizmoduck/row249-on-8a04 05595794b (Gizmoduck), manifest 3a969bb136bc, approved by
Duckworth in round 1 (rev 2889-2890 request c30068, verdict c30070). Gate on Sage's slot 14:53Z
(sage-2026): grant-executor.test.mjs whole 39/0 and bus 598/0 with no stub left, and C1, which
drops the `if (!installed)` branch, fails G8 and both G23 tests. Webby's N1 on X5 `sleep 30` is
deferred by Duckworth (DEFERRED.md).
This commit is contained in:
jason.woltje committed 2026-10-21 09:54:29 -05:00
1 parent 8ca064158a
commit 9de5a57abd
3 files changed
+161 -3

No files matched your search

+98
View File
@@ -0,0 +1,98 @@
# 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-on-8a04, on row 248's landing 8a04f4aa4 (sage-2012).
No file under packages/bus/src changed between 6d5545d44 and 8a04f4aa4, so the brief's src 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` (:451) 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.
- :468, the abort case. `hold-add`, the human approves, the add holds, `abortClaim` aborts the
executor's claim. Ends revoked.
- :480, 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 (:283) 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 :436 (:432 at 6d5545d44) 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 (:67, :72) is the backstop.
## 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
On Sage's grant sage-2015, at 8457af076 (7cee1a1ab plus this file), TMPDIR /mnt/storage/scratch/g249t:
| Run | Window | Result |
|---|---|---|
| grant-executor.test.mjs whole | 13:29:01-13:29:21Z | 39 of 39 pass |
| C1 | 13:29:21-13:29:40Z | killed: both row 249 tests and G8 fail on `hostLineLeft` undefined |
After C1 the worktree was clean and broker.mjs matched HEAD. After the runs, no node, sshd or sh process had a
cwd under the worktree or g249t, and no `fake/add` or `fake/remove` stub was running. The bus suite runs on
Sage's gate.
## Rebase onto 8a04f4aa4 (sage-2012)
The first commit, 02bfd184b, sat on bc579f7a8 as gizmoduck/row249, which was never pushed. Row 248 landed
as 8a04f4aa4, changing grant-executor.test.mjs at :54, :67 and :72. gizmoduck/row249-on-8a04 starts at
8a04f4aa4 and cherry-picks 02bfd184b with no conflict. 248's lines stay as landed. The patch-id is the
same on both (c33823941e61). Test line numbers moved down by one, and this file was updated for that.
+5
View File
@@ -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));
+58 -3
View File
@@ -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();