From cef503cb93fd742a342be3a0e8ff12d24f4d4bca Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 23 Oct 2026 14:35:47 -0500 Subject: [PATCH] Untested: retain library notice after push when publication fails (#1793) --- agents/bubba/work/queue-288/BUILD.md | 34 ++++++++++++++++++++ packages/bus/src/broker.mjs | 28 +++++++++++----- packages/bus/src/runtime.mjs | 1 + packages/bus/tests/library-write.test.mjs | 39 +++++++++++++++++++---- packages/library/README.md | 13 +++++--- packages/library/src/write.mjs | 2 ++ 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/agents/bubba/work/queue-288/BUILD.md b/agents/bubba/work/queue-288/BUILD.md index 97cd138d6..f20da62ed 100644 --- a/agents/bubba/work/queue-288/BUILD.md +++ b/agents/bubba/work/queue-288/BUILD.md @@ -179,3 +179,37 @@ existing plain Gitea-user pattern. The omission at mapping level is valid; human All tests above are newly authored by row288 and remain unrun; no changes to baseline/dependency tests. RED/GREEN and mutants await CLEAR. Proposed fixes are not claimed verified. The new host-only library write module imports the existing pure ID_PATTERN to match the broker's accepted instance ids. + +## 2026-10-23 mos-1520 ruling: post-push notice is required + +Correction to the E4 interpretation above: Mos rejected the known-gap treatment. The requirement +stays: a pushed agent commit has the exact notice even if local publication then fails. This is not +an outbox or retry. The writer now calls the broker's trusted synchronous onPushed callback as soon +as push returns pushed:true, before publication. The callback records the exact library-write notice +and action.allowed in the normal Console-store transaction. Publication failure still answers +outcome-unknown. The notice's Revert can refuse library-revert-not-agent until sync makes that commit +reachable from served; README now says that explicitly and removes the rejected gap description. + +The row's own real-push corruption test now pins the exact notice body with the pushed id, old +served and outcome-unknown; after restoring readable state it pins Revert's refusal before sync and +success after sync. The successful-write adapter fixture calls onPushed too, matching runtime's +writer contract. No existing baseline/dependency test changed (mos-1520 explicitly authorizes this +row's test correction without mos-1272). + +Residual Console-store failure: if notice insertion/routing/its event transaction fails, the broker +calls a separate host log sink with {event:library-notice-store-failed,code:outcome-unknown,business, +instance,op,commit}. It logs no args, path content, credential, raw SQL or exception output. Runtime's +new library.log adapter writes this metadata to stderr. The caller gets outcome-unknown even if +recording action.refused in that same broken store fails. A new real-push test uses temporary SQLite +abort triggers on notice insertion and refusal evidence, checks remote advanced/served unchanged, +no message, the exact external log and outcome-unknown. It does not claim a notice can be stored in +a failed Console store. If the host log sink itself also fails, no durable log can be promised; the +caller still receives outcome-unknown. The runtime log hunk is the required residual-failure wiring. + +Mutant added: move `options.onPushed?.({ project: context.project, path: change.path, commit });` +from before `await publishLibraryWrite(...)` to immediately after that await inside the same try. +Successful operations keep their notice, but the outcome-unknown publication path drops it. The +C2-02 E4 real-push corruption test must fail on its exact notice assertion. Run on CLEAR and restore. + +No Node/tests/build/mutants run. Static git diff --check clean. Della's first284 head0ed4fbab9 is +explicitly incomplete and not a finished dependency; waiting for her complete-head pin before tools. diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 6ceacfc8a..af90f1386 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -5231,13 +5231,21 @@ export class Broker { if (agent) { const op = verb === 'library.doc.delete' ? 'delete' : 'write'; result = await this.#library.writeDoc({ ...context, ...args }, { ...writer, op, - beforePush: () => { this.#processingWork(); this.#libraryWriter(s, project); } }); - try { - this.#store.transaction(() => { - this.#libraryWriteNotice(s, op, result); - this.#event(s, 'action.allowed', { action: 'library.write', class: 'within-role', project, path: result.path, commit: result.commit }); - }); - } catch { fail('outcome-unknown'); } + beforePush: () => { this.#processingWork(); this.#libraryWriter(s, project); }, + onPushed: saved => { + try { + this.#store.transaction(() => { + this.#libraryWriteNotice(s, op, saved); + this.#event(s, 'action.allowed', { action: 'library.write', class: 'within-role', project, path: saved.path, commit: saved.commit }); + }); + } catch { + // A separate metadata-only sink survives a Console store failure. Never log args, + // file text, credentials or raw SQL/exception output. + try { this.#library.log?.({ event: 'library-notice-store-failed', code: 'outcome-unknown', + business: s.business, instance: s.role, op, commit: saved.commit }); } catch {} + fail('outcome-unknown'); + } + } }); } else if (verb === 'library.file.save') { this.#processingWork(); result = await this.#library.saveFile({ ...context, path: args.path, base: args.base, content: args.content }); @@ -5255,7 +5263,11 @@ export class Broker { return result; } catch (error) { const Failure = this.#library?.LibraryError; - throw this.#refused(s, Failure && error instanceof Failure ? new BusError(error.code) : error, request); + const mapped = Failure && error instanceof Failure ? new BusError(error.code) : error; + const refused = this.#refused(s, mapped, request); + // Once a push succeeded the remote effect is uncertain to the caller even if the store + // cannot record refusal evidence. Do not replace that with an ordinary storage refusal. + throw mapped instanceof BusError && mapped.code === 'outcome-unknown' ? mapped : refused; } } // The eight task verbs reach the trusted S3 handler here. It runs outside any transaction, gets the diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs index d4d1d750d..cb17710ac 100644 --- a/packages/bus/src/runtime.mjs +++ b/packages/bus/src/runtime.mjs @@ -316,6 +316,7 @@ export async function startBroker({ store, businesses, library: { + log: entry => process.stderr.write(`library: ${JSON.stringify(entry)}\n`), readFile: context => libraryModule.readLibraryFile({ ...context, dataRoot }), status: context => libraryModule.libraryStatus({ ...context, dataRoot }), sync: (context, { accept }) => processing.work(() => libraryModule.sync({ ...context, dataRoot }, { ...syncOptions(context.business), accept })), diff --git a/packages/bus/tests/library-write.test.mjs b/packages/bus/tests/library-write.test.mjs index 5bec1d703..435b274a7 100644 --- a/packages/bus/tests/library-write.test.mjs +++ b/packages/bus/tests/library-write.test.mjs @@ -14,6 +14,7 @@ import { mappingEntry } from '../../library/tests/helpers.mjs'; import { LibraryError } from '../../library/src/problems.mjs'; import { grantDecision } from './host-fixtures.mjs'; import { world as bareWorld } from '../../library/tests/write-fixture.mjs'; +import { sync } from '../../library/src/sync.mjs'; import { SYNC_STATE_FILE, servedCommit } from '../../library/src/mirror.mjs'; import { writeLibraryDoc, saveLibraryFile, revertLibraryCommit } from '../../library/src/write.mjs'; import { git } from '../../library/tests/helpers.mjs'; @@ -29,10 +30,10 @@ function world(t, options = {}) { options.changeRole?.(roles.bean); const business = { id: 'acme', human: 'jason', vars: { 'bus.tester': true }, projects, roles, arbiters: { technical: 'chief', delivery: 'chief' }, definitions: { companion: {}, 'chief-of-staff': {}, proofreader: {} } }; - const seen = [], held = new Set(); + const seen = [], held = new Set(), logs = []; const result = { project: 'books', path: PATH, commit: NEXT }; - const library = { LibraryError, readFile: async () => ({}), status: async () => ({}), - writeDoc: async (context, options) => { seen.push({ context, options }); options.beforePush(); return result; }, + const library = { LibraryError, log: entry => logs.push(entry), readFile: async () => ({}), status: async () => ({}), + writeDoc: async (context, options) => { seen.push({ context, options }); options.beforePush(); options.onPushed(result); return result; }, saveFile: async context => { seen.push({ context }); return result; }, revert: async context => { seen.push({ context }); return { ...result, reverted: context.commit }; } }; const broker = new Broker({ store, businesses: { acme: business }, library, giteaKeys: { held: (b, a) => held.has(a) } }); @@ -52,7 +53,7 @@ function world(t, options = {}) { }; grant(); const call = (who, verb, args = {}) => Promise.resolve().then(() => broker.request(caps[who], { verb, args })); - return { root, store, broker, caps, call, seen, held, business, grant, library }; + return { root, store, broker, caps, call, seen, held, business, grant, library, logs }; } const args = () => ({ universe: 'harbor', path: PATH, base: SHA, content: 'fixture\n' }); @@ -263,7 +264,7 @@ test('C2-02 F2/E6 runtime: human saves/reverts use editor.user; no login never u }); -test('C2-02 E4: post-push publication failure is outcome-unknown and has no stored notice', async t => { +test('C2-02 E4: post-push publication failure stores the exact notice and returns outcome-unknown', async t => { const bare = await bareWorld(t), f = world(t, { projects: bare.context.business.projects }); const state = join(bare.mirror, SYNC_STATE_FILE), original = readFileSync(state); const previous = await servedCommit(bare.mirror), head = bare.remoteHead(); @@ -279,6 +280,32 @@ test('C2-02 E4: post-push publication failure is outcome-unknown and has no stor { code: 'outcome-unknown' }); assert.notEqual(bare.remoteHead(), head); assert.equal(git(bare.mirror, 'rev-parse', 'served'), previous); - assert.equal(f.store.get("SELECT count(*) n FROM messages WHERE from_role='bean'").n, 0); + const messages = f.store.all("SELECT body FROM messages WHERE from_role='bean' AND to_role='human'"); + assert.deepEqual(messages.map(m => JSON.parse(m.body)), [ + { notice: 'library-write', op: 'write', path: PATH, commit: bare.remoteHead() }]); } finally { writeFileSync(state, original); } + const commit = bare.remoteHead(); + const human = { credential: { ...bare.credential, kind: 'human' }, command: bare.command }; + await assert.rejects(revertLibraryCommit({ ...bare.context, commit }, human), { code: 'library-revert-not-agent' }); + await sync(bare.context, { credential: bare.credential, command: bare.command }); + assert.equal((await revertLibraryCommit({ ...bare.context, commit }, human)).reverted, commit); +}); + + +test('C2-02 mos-1520: failed notice insert logs metadata and returns outcome-unknown even if refusal evidence fails', async t => { + const bare = await bareWorld(t), f = world(t, { projects: bare.context.business.projects }); + const previous = await servedCommit(bare.mirror), head = bare.remoteHead(); + f.library.writeDoc = (context, options) => writeLibraryDoc({ ...context, dataRoot: bare.dataRoot }, + { ...options, credential: { ...options.credential, file: bare.file }, command: bare.command }); + f.store.run(`CREATE TEMP TRIGGER fail_library_notice BEFORE INSERT ON messages + WHEN NEW.from_role='bean' BEGIN SELECT RAISE(ABORT, 'fixture store failure'); END;`); + f.store.run(`CREATE TEMP TRIGGER fail_refusal_log BEFORE INSERT ON events + WHEN NEW.kind='action.refused' BEGIN SELECT RAISE(ABORT, 'fixture evidence failure'); END;`); + await assert.rejects(f.call('bean', 'library.doc.write', { ...args(), base: git(bare.mirror, 'rev-parse', `served:${PATH}`) }), + { code: 'outcome-unknown' }); + assert.notEqual(bare.remoteHead(), head); + assert.equal(await servedCommit(bare.mirror), previous); + assert.equal(f.store.get("SELECT count(*) n FROM messages WHERE from_role='bean'").n, 0); + assert.deepEqual(f.logs, [{ event: 'library-notice-store-failed', code: 'outcome-unknown', business: 'acme', + instance: 'bean', op: 'write', commit: bare.remoteHead() }]); }); diff --git a/packages/library/README.md b/packages/library/README.md index e3295096a..ae14b275f 100644 --- a/packages/library/README.md +++ b/packages/library/README.md @@ -314,7 +314,12 @@ commit remains; callers must inspect status rather than retry blindly. The human push login is `editor.user` (optional in the mapping, required for save/revert), distinct from `user`, which names the fetch bot. An absent editor login refuses `library-human-missing`; no fallback uses -the bot's login. If remote push succeeds but local publication fails, -`outcome-unknown` is returned before a write notice is stored. That -exceptional commit can therefore lack a Console notice; after sync it is -still revertible by commit id. No notice outbox or replay is provided. +the bot's login. Once an agent push succeeds, the broker stores the exact +write notice with that pushed commit id before local publication. If +publication then fails, the agent receives `outcome-unknown` and the +notice remains. Revert can refuse `library-revert-not-agent` until a sync +makes the commit reachable from served. No notice outbox or retry is +needed. If the Console store itself refuses the notice insert, the caller +still receives `outcome-unknown` and a separate metadata-only host log +records `library-notice-store-failed` with the business, instance, operation +and pushed commit id; it contains no file text or credential. diff --git a/packages/library/src/write.mjs b/packages/library/src/write.mjs index fa31d409a..7c79c1d3c 100644 --- a/packages/library/src/write.mjs +++ b/packages/library/src/write.mjs @@ -164,6 +164,8 @@ async function write(context, options, human, revert = false) { if (!result.pushed) { tip = result.tip; continue; } pushed = true; try { + // The broker records the pushed commit even if publication fails next (mos-1520). + options.onPushed?.({ project: context.project, path: change.path, commit }); await publishLibraryWrite(mirror, { mapping, commit, previous: served, readSyncState: lock.readSyncState, now: options.now }); } catch { fail('outcome-unknown'); } return { project: context.project, path: change.path, ...(revert ? { reverted: change.reverted } : {}), commit };