row 239 (#1745): design draft 1. beforeWrite runs inside writer.write under the lock after the revision and validation checks; a refusal after the revoke is marked and the bus names it revision-conflict-revoked or outcome-unknown-revoked, with CLI, Console and fire-notice texts; injected-writer fire test T1, T2-T6, mutants M1-M3e; Q1-Q3 for Duckworth; doc only (sage-2792)
This commit is contained in:
jason.woltje committed
1 parent
546366f4af
commit
cb3a456989
1 file changed
+159
@@ -0,0 +1,159 @@
|
||||
# Row 239 design: the revoke under the writer's lock, and a fire test
|
||||
|
||||
Brief: docs/plans/2026-10-20_agent-lifecycle.md, "Brief: row 194 follow-up, the revoke under the writer's
|
||||
lock and a fire test" (blob 3b3b3b2b9), from Duckworth's row 194 b3 N1 and mos-1158. Base aca16404c (row
|
||||
194 landed as fd6dd327c, row 281 as e479521d3). Builder Honker, reviewer Duckworth, gate owner Sage.
|
||||
Draft 1, 2026-10-22.
|
||||
|
||||
## Today
|
||||
|
||||
- `instance-settings.mjs` `commit()` runs the checks, then compares the caller's revision with a fresh
|
||||
`writer.read` (row 194 b3, sage-1634), then calls `beforeWrite` (the fire's or the delete's credential
|
||||
revoke), then `writer.write`.
|
||||
- `writer.write` (`organization.mjs`) takes the lock, runs `recover`, checks the revision again (`:188`),
|
||||
validates, prepares, checks the bytes once more before publication (`:221`), renames and commits.
|
||||
- So two windows are left.
|
||||
- A write that lands between `commit()`'s read and the writer's lock still gets a revoke and then a
|
||||
`revision-conflict`.
|
||||
- Any refusal after the revoke ran (the `:221` check, or an I/O failure the bus reports as
|
||||
`outcome-unknown`) uses the same text as one before it, so nothing says the tokens may be gone.
|
||||
- Row 281 left this path alone. Its fire job (`broker.mjs` `#fireJob`) runs step G (grants), then step B,
|
||||
which is this same `write('instance.fire')`, and reports a step B failure as `fire.failed` along with a
|
||||
human notice that names the code. Its delete job only does step G. The delete itself still commits
|
||||
through `lifecycleDelete` → `instances.set` op delete → `commit()`.
|
||||
|
||||
## What changes
|
||||
|
||||
### 1. `beforeWrite` runs inside `writer.write`, under the lock (organization.mjs)
|
||||
|
||||
`write(id, document, { revision, actor, detail, context, beforeWrite = null })`. The order under the lock:
|
||||
|
||||
1. `recover`, read, then the `:188` revision check. A refusal here revokes nothing.
|
||||
2. Projects-protected and both validations. A refusal here revokes nothing either. I put `beforeWrite`
|
||||
after these rather than straight after `:188`, so a document the writer would refuse anyway never
|
||||
costs a revoke. Q1 asks Duckworth to confirm this.
|
||||
3. `beforeWrite()`. If it throws (today `credential-revoke-failed`), the error propagates as it is and
|
||||
nothing has been written, as now.
|
||||
4. Prepare, the `:221` check, rename and commit, wrapped so that any error thrown after step 3 gets
|
||||
`revoked = true` set on it before it is rethrown. That covers `revision-conflict` from `:221`, an
|
||||
`organization-refused` from `log()`, a barrier throw, and an fs error.
|
||||
|
||||
`record()` and the other writers (`launch-settings`, `plugin-settings`, `credentialVerbs`) pass no
|
||||
`beforeWrite` and behave as before.
|
||||
|
||||
### 2. commit() passes beforeWrite through (instance-settings.mjs)
|
||||
|
||||
`commit()` passes `beforeWrite` to `writer.write` and no longer calls it itself. The row 194 b3 early check
|
||||
stays. It costs one read, keeps T21's refusal before any validation work, and is no longer what protects
|
||||
the revoke. Its comment is rewritten to say so, and the "(BUILD.md follow-up)" sentence goes.
|
||||
|
||||
`instanceSettings({ ..., writer = organizationWriter({ dir, rolesDir }) })` takes an optional writer, so a
|
||||
test can inject one (the brief's "injected writer"). `runtime.mjs` passes none.
|
||||
|
||||
### 3. The bus names a refusal after a revoke (broker.mjs `#organizationFail`)
|
||||
|
||||
The bus answers with a code only, and the CLI and Console look the text up by code. So the "may already be
|
||||
revoked" signal has to be a code. Two new codes:
|
||||
|
||||
- `revision-conflict-revoked`: `e.revoked && e.code === 'revision-conflict'`.
|
||||
- `outcome-unknown-revoked`: `e.revoked` with any other code. The write may or may not have landed, since a
|
||||
failure after the rename is recovered as committed.
|
||||
|
||||
Both are checked before the existing mappings. An error without `revoked` maps exactly as today, so a
|
||||
refusal before any revoke keeps today's code and text. Q2 asks about the names.
|
||||
|
||||
### 4. The texts
|
||||
|
||||
- CLI `instances.mjs` REASONS:
|
||||
- `revision-conflict-revoked`: "the business file changed before the write, after the agent's tokens
|
||||
were revoked; the tokens may already be revoked and the instance can't use them; read the instances
|
||||
again and run the same command again, which is safe".
|
||||
- `outcome-unknown-revoked`: "the write's outcome is unknown and the agent's tokens may already be
|
||||
revoked; read the instances before trying again".
|
||||
- Console `serve.mjs` AGENT_REASONS: the same two, in the Console's wording ("delete again").
|
||||
- `#fireJob`'s `fire.failed` notice: when the code ends in `-revoked`, it adds "Its credential tokens may
|
||||
already be revoked." The chief's synchronous fire (no grants) answers with the code, as every chief verb
|
||||
does.
|
||||
- `transport.mjs` EXIT: `outcome-unknown-revoked` exits 1, as `outcome-unknown` does. Console
|
||||
`bus.mjs` STATUS: `revision-conflict-revoked` 409, `outcome-unknown-revoked` 503. A delete's code
|
||||
reaches both as a closed request's `refusal`, so these two only matter for a direct verb answer.
|
||||
|
||||
## Tests
|
||||
|
||||
- **T1, the brief's fire test** (business `instance-settings.test.mjs`). The fire uses an injected writer
|
||||
that wraps the real one. Its `write` makes one hand edit of the business file (a vars change on another
|
||||
instance) and then calls the real `write`, so the edit lands after `commit()`'s early check and before
|
||||
the lock's `:188` check. Expected:
|
||||
- the fire refuses `revision-conflict` without `revoked`;
|
||||
- the revoke spy has no calls;
|
||||
- the business file's bytes are the hand edit's;
|
||||
- changes.jsonl has no new prepared entry.
|
||||
- **T2** (same file). A fire on a real writer built with `barrier: stage => stage === 'prepared' && edit()`.
|
||||
The revoke runs, then `:221` refuses. Expected: `revision-conflict` with `revoked === true`, one revoke
|
||||
call, the file's bytes are the edit's, and the last receipt is `aborted`.
|
||||
- **T3** (same file). The same, with a `prepared` barrier that throws. Expected: the error has
|
||||
`revoked === true`, one revoke call, and the file unchanged. A delete through the same writer gives the
|
||||
same result (T3b).
|
||||
- **T4** (bus `fire-revokes-grants.test.mjs`, its injected `f.org`). `f.org.fireError` is a
|
||||
`revision-conflict` with `revoked: true`. Expected:
|
||||
- the job's `fire.failed` carries `revision-conflict-revoked`, and the inbox message says "may already
|
||||
be revoked";
|
||||
- the same with no `grants` (the synchronous fire) refuses `revision-conflict-revoked`;
|
||||
- a plain `revision-conflict` still maps to `revision-conflict` and the message lacks the sentence.
|
||||
- T4b: `outcome-unknown-revoked` for an fs-style error with `revoked`.
|
||||
- **T5** (cli `lifecycle.test.mjs` or `instances.test.mjs`, whichever already reads a closed delete's
|
||||
refusal). A delete closed with `revision-conflict-revoked` prints the new text, and one closed with
|
||||
`revision-conflict` prints today's.
|
||||
- **T6** (webui `agents.test.mjs`). The same pair for the Console's reason.
|
||||
- T21 (bus `lifecycle-revoke.test.mjs`) stays as it is, for delete.
|
||||
|
||||
## Mutants
|
||||
|
||||
| Mutant | Change | Killed by |
|
||||
| --- | --- | --- |
|
||||
| M1 | `beforeWrite` before the lock: `commit()` calls `beforeWrite()` itself and passes none to `write` | T1 (the revoke spy has a call) |
|
||||
| M2 | The fire refusal dropped: the `:188` revision check removed | T1 (the write lands over the hand edit) |
|
||||
| M3a | The mark dropped: no `revoked = true` in the catch | T2, T3, and T4 through the codes |
|
||||
| M3b | `#organizationFail` ignores `revoked` | T4 |
|
||||
| M3c | CLI text: the two REASONS entries removed | T5 |
|
||||
| M3d | Console text: the two AGENT_REASONS entries removed | T6 |
|
||||
| M3e | The notice: the `fire.failed` sentence removed | T4 |
|
||||
|
||||
M3a-M3e together are the brief's "drop the may already be revoked text".
|
||||
|
||||
## Files
|
||||
|
||||
- `packages/business/src/organization.mjs`
|
||||
- `packages/business/src/instance-settings.mjs`
|
||||
- `packages/business/tests/instance-settings.test.mjs`
|
||||
- `packages/bus/src/broker.mjs`
|
||||
- `packages/bus/tests/fire-revokes-grants.test.mjs`
|
||||
- `packages/cli/src/instances.mjs`
|
||||
- `packages/cli/src/transport.mjs`
|
||||
- one cli test file
|
||||
- `packages/webui/src/serve.mjs`
|
||||
- `packages/webui/src/bus.mjs`
|
||||
- `packages/webui/tests/agents.test.mjs`
|
||||
- This packet.
|
||||
|
||||
## Gate
|
||||
|
||||
As the brief says:
|
||||
|
||||
- Duckworth approves the manifest.
|
||||
- The bus, business, cli and webui suites and every `scripts/test-*.sh` are green on Sage's slot, with
|
||||
the touched test files run whole.
|
||||
- M1-M3e are killed.
|
||||
|
||||
Before the in-review move I grep the tests for the changed comment and error text, and for hash pins on the
|
||||
changed files.
|
||||
|
||||
## Questions for Duckworth
|
||||
|
||||
- **Q1.** Should `beforeWrite` run after the writer's validations (my pick) or straight after `:188` (the
|
||||
brief's literal place)? Running it later means a document the writer would refuse revokes nothing.
|
||||
- **Q2.** Are two new codes, `revision-conflict-revoked` and `outcome-unknown-revoked`, the right way to
|
||||
carry the signal? The alternative is to give `BusError` a detail field, which every CLI and Console
|
||||
reader would then have to learn. The codes are additive and no existing code changes meaning.
|
||||
- **Q3.** The brief's mutant "drop the fire refusal": I read it as removing the `:188` check (M2). If you
|
||||
meant the fire test's own refusal, say so and I'll redefine it.
|
||||
Reference in new issue
Block a user