diff --git a/agents/gizmoduck/work/queue-264/BUILD.md b/agents/gizmoduck/work/queue-264/BUILD.md index bb968704b..3dfba9c9b 100644 --- a/agents/gizmoduck/work/queue-264/BUILD.md +++ b/agents/gizmoduck/work/queue-264/BUILD.md @@ -170,6 +170,36 @@ Duckworth had no objection to the in-place rebuild and asked for seven checks. W session. New ids come from randomUUID. 7. `thread_one_per_run`. The inner `max(seq)` is per session. +### Duckworth's early look (duckworth-0107) and Sage's ruling (sage-2257) + +Duckworth read 312cf33a1 with git only and found nothing blocking. Round 2 (gizmoduck/row264-r2, +rebased on dcd8d4dda with no shared paths) answers his notes: + +1. Retired-generation replay. Sage ruled it into 264a (sage-2257). `#agent` now fences a thread run + by generation: the session's latest claim must have the generation of the run's own latest + claim (`#runGeneration`). A new sessions.test case has a released run and a revoked run each try + `decision.withdraw`, `message.receive` and `message.read` after another run claims the next + generation. All refuse `not-holder` and every table's row count stays the same. The workspace + and path halves of T2 stay in 264b. +2. `assistant.stop` keeps returning `{thread, run}`. Adding `session` would change the pins in + tester-threads.test and threads.test for a field no caller reads yet. The Console stop in 264e + is the first reader, so the field goes there. +3. `#route` refuses an unknown session with `session-not-found` instead of a TypeError. +4. The README's session section says the `default` retry answers on new threads only, because a + migrated default has op `legacy-3k`. +5. The Row 155 comment at the role.revoke end now says "held thread session". + +### Mutants + +Each runs on a copy of the candidate tree with one edit, and the named test must fail. + +- M1 (sage-2257). In `#agent`, replace `h.generation !== this.#runGeneration(s)` with `false`. + Killed by sessions.test "a released or revoked run's retired generation". +- M2. In `message.receive`, drop the `mt.session=?` filter (the session clause and its argument). + Killed by sessions.test T1, where a1 would take the second session's message. +- M3. In 3j-3k.sql, count every claim row for `generation` instead of `op='claim'` rows only. + Killed by sessions.test T7, generations 1, 1, 1, 2, 2. + ### Dry run Before writing any node test, I ran 3j-3k.sql in an in-memory Python sqlite3 on a 3j fixture diff --git a/packages/bus/README.md b/packages/bus/README.md index 411eccb11..5f1d2b246 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -333,6 +333,15 @@ A thread run receives and replies in its own session, and the chief of staff's r to the session that sent the work. `threadsState` and `threadIdle` keep one row per thread, computed on its default session; 264b's launcher starts the others. +A thread run's claim fences by generation (T2). A run acts only while its session's latest claim +row has the generation of the run's own latest claim. After a release or revoke, once another run +claims the session, the old run's `decision.withdraw`, `message.receive` and `message.read` +refuse `not-holder` and write nothing. + +`thread.create` writes the default session with op `default`, so a `session.create` retry naming +`default` with op `default` answers with it. A migrated thread's default has op `legacy-3k`, and +the same retry there refuses `session-exists`. + The triggers check that a session has its thread's business, instance and owner, that its predecessor and any default, claim, message or open name a session of the same thread, and that a claim's generation is the one its session expects. `delivery_same_session` replaces diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index ae3301b86..b90c33d07 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -812,9 +812,15 @@ export class Broker { if (s.reader) fail('read-only'); if (s.human) fail('agent-required'); const h = this.#own(s); - if (h?.op !== 'claim' || h.holder_run !== s.run) fail('not-holder'); + if (h?.op !== 'claim' || (s.thread ? h.generation !== this.#runGeneration(s) : h.holder_run !== s.run)) fail('not-holder'); return h; } + // Row 264a (#1769, T2): the generation a thread run holds, its latest claim's in its session, or + // null if it never claimed. Once another run claims the session, the run's own generation is + // retired, and #agent refuses its approvals, deliveries and reads. + #runGeneration(s) { + return this.#store.get("SELECT max(generation) AS g FROM thread_claims WHERE session=? AND holder_run=? AND op='claim'", s.session, s.run).g; + } #human(s) { if (s.reader) fail('read-only'); if (!s.human || s.via !== 'cli' || s.outsideAgent !== true) fail('human-required'); @@ -4872,7 +4878,8 @@ export class Broker { identifier(a.decision); this.#consumeAuthority(s, 'role.revoke', { decision: a.decision, target: a.role }); } - // Row 155 (#1649): a thread instance holds a claim per session, so the end revokes each one. + // Row 155 (#1649): a thread instance holds a claim per held thread session (264a), so the end + // revokes each one. if (this.#threaded(s.business, a.role)) { const held = this.#threadHolders(s.business, a.role); if (held.length === 0) fail('not-held'); @@ -5973,7 +5980,7 @@ export class Broker { // Row 264a (#1769): it names one session of the thread, and the session's holder may have it. #route(business, mid, to, session) { if (session) { - const { thread } = this.#store.get('SELECT thread FROM sessions WHERE id=?', session); + const { thread } = this.#store.get('SELECT thread FROM sessions WHERE id=?', session) ?? fail('session-not-found'); this.#store.run('INSERT INTO message_threads(message,thread,session) VALUES(?,?,?)', mid, thread, session); } this.#delivery(mid, 'routed', this.#recipient(business, to, session)); diff --git a/packages/bus/tests/sessions.test.mjs b/packages/bus/tests/sessions.test.mjs index fd0b73655..6614c94e9 100644 --- a/packages/bus/tests/sessions.test.mjs +++ b/packages/bus/tests/sessions.test.mjs @@ -172,6 +172,55 @@ test('T2 REQ-ROLE-2: one holder per session, generations count its claims, and a store.run("INSERT INTO deliveries(message,at,op,holder_run) VALUES(?,?,'delivered',?)", m.id, now(), 'x-run'); }); +test('T2 sage-2257: a released or revoked run\'s retired generation refuses its approval, delivery and read, and writes nothing', (t) => { + const { call, launch, claim, claims, store, A, D } = setup(t); + const tables = store.all("SELECT name FROM sqlite_master WHERE type='table' AND name NOT LIKE 'sqlite_%'").map((r) => r.name); + const counts = () => Object.fromEntries(tables.map((n) => [n, store.get(`SELECT count(*) AS n FROM "${n}"`).n])); + // The run raises a decision, takes one message and leaves a second waiting. + const prepare = (name) => { + launch(name, 'assistant', { thread: A }); + claim(name); + const d = call(name, 'decision.raise', { + action: 'spend', + target: 'demo', + question: 'spend?', + options: [ + { key: 'yes', text: 'Yes' }, + { key: 'no', text: 'No' }, + ], + recommendation: 'no', + blocking: false, + }); + call('human', 'thread.send', { thread: A, body: `for ${name}` }); + const [taken] = call(name, 'message.receive'); + call('human', 'thread.send', { thread: A, body: `waiting after ${name}` }); + return { d, taken }; + }; + const refused = (name, { d, taken }) => { + const before = counts(); + assert.throws(() => call(name, 'decision.withdraw', { id: d.id }), { code: 'not-holder' }, 'approval'); + assert.throws(() => call(name, 'message.receive'), { code: 'not-holder' }, 'delivery'); + assert.throws(() => call(name, 'message.read', { id: taken.id }), { code: 'not-holder' }, 'read'); + assert.deepEqual(counts(), before); + }; + // Released: generation 1 is retired once a2 claims generation 2. + const one = prepare('a1'); + call('a1', 'role.release'); + const two = prepare('a2'); + refused('a1', one); + // Revoked by the human's stop: generation 2 is retired once a3 claims generation 3. + call('human', 'assistant.stop', { instance: 'assistant' }); + prepare('a3'); + refused('a2', two); + assert.deepEqual(claims(D).map((r) => [r.op, r.holder_run, r.generation]), [ + ['claim', 'a1-run', 1], + ['release', 'a1-run', 1], + ['claim', 'a2-run', 2], + ['revoke', 'a2-run', 2], + ['claim', 'a3-run', 3], + ]); +}); + test('session.create is idempotent on its op, refuses a reused op or a taken name, and reaches only the owner\'s threads', (t) => { const { call, store, project, A, D } = setup(t); const first = call('human', 'session.create', { thread: A, name: 'second', op: 'op-1' });