diff --git a/agents/bubba/work/queue-288/BUILD.md b/agents/bubba/work/queue-288/BUILD.md index 6874e1cdf..97cd138d6 100644 --- a/agents/bubba/work/queue-288/BUILD.md +++ b/agents/bubba/work/queue-288/BUILD.md @@ -142,3 +142,40 @@ Additional required doc hunk: packages/library/README.md appends the four verb c lastPush shape, exact notice, lock tradeoff and publication failure behavior. No existing test from the dependency/base has changed. All new/modified tests in this pass were authored by this row. `git diff --check` remains clean. No Node/tests/build/mutants run; 284 and CLEAR remain pending. + +## 2026-10-23 early review follow-up (Webby E1-E6; Ludwig F2) + +Sage sage-3652 approves console-bus.mjs wiring (null option + --send dispatch), confirms editor.user +as the sole human login source and reserves CLEAR for T3/T4 after T2. Ordinary sync()'s separate +post-rename fsync gap stays out of 288 per that ruling; no change to sync()'s existing rollback. + +Honker 283 head 46bad7ea3 merged intact: mapping editor.user is optional, and if present uses the +existing plain Gitea-user pattern. The omission at mapping level is valid; human writes refuse it. + +- Ludwig F2 / Webby E6: confirmed the previous runtime used the fetch bot's mapping.user wrongly. + Runtime now passes mapping.editor.user to humanCredential, never mapping.user. Missing user gives + library-human-missing before Git. New real-runtime cases pin both save and revert with a separate + fetch-bot login, positive pushes under her-gitea-login, and no command for an absent editor login. + The broker's credential.agent is s.role, not args: added an exact assertion and strict-key refusal + cases for caller-supplied agent and credential. +- E1: replaced the business-id RegExp interpolation with a literal startsWith(business.id + '/') + check and ID_PATTERN on only the instance suffix. New acme.co vs acme-co trailer test. +- E2: new broker→bare case holds the seat in the fetch command. beforePush must refuse + library-write-no-grant with all local refs and the remote unchanged and no notice. New mutant: + remove `options.beforePush?.();` in write.mjs; the E2 test must fail. +- E3: new broker cases for expired rotate_by, fire.stopped without fire.done, missing role.publish + project, grant/identity Gitea-user mismatch, and missing identity name/email, for both agent verbs. +- E4: the exceptional publication failure currently returns outcome-unknown before the broker has + a successful result to put in a notice, so a remote agent commit can exist without a Console + notice. There is no durable notice outbox/retry in this row. The trailer commit remains human + revertible by id after sync; this is an explicit known failure-mode gap for the manifest, not + a claim that every failed publication emits a notice. A new real-push test breaks state decoding + after push admission, pins outcome-unknown, old served and no notice. It also found a local error + propagation gap: errors before publishLibraryWrite's internal try now normalize to outcome-unknown + in the writer once push has succeeded, so the caller is never told an ordinary refused-write code. +- E5: 284 still has no build. Tools/8-or-10 snapshot and served-blob integration remain pending. + librarySave is intentionally only an adapter option until C2-11b supplies its route/page. + +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. diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs index 1503f3777..d4d1d750d 100644 --- a/packages/bus/src/runtime.mjs +++ b/packages/bus/src/runtime.mjs @@ -240,7 +240,7 @@ export async function startBroker({ const humanWriteOptions = context => { const mapping = libraryModule.checkMapping(context.business.projects[context.project]); if (!mapping.editor) throw new libraryModule.LibraryError('library-mapping-invalid', 'editor is required'); - return { credential: libraryModule.humanCredential(context, { user: mapping.user, configDir: slotDir }), + return { credential: libraryModule.humanCredential(context, { user: mapping.editor.user, configDir: slotDir }), ...(librarySync ? { command: librarySync.command } : {}) }; }; // Row 194 (#1692, D3): after every write that commits, the broker takes the business as a restart diff --git a/packages/bus/tests/library-write.test.mjs b/packages/bus/tests/library-write.test.mjs index 0bfbbfd7f..5bec1d703 100644 --- a/packages/bus/tests/library-write.test.mjs +++ b/packages/bus/tests/library-write.test.mjs @@ -1,10 +1,11 @@ // Row 288: broker authority, exact notice and strict Console revert API. Fixture grant rows only. import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, rmSync } from 'node:fs'; +import { mkdtempSync, rmSync, mkdirSync, writeFileSync, readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { connect } from 'node:net'; +import { startBroker } from '../src/runtime.mjs'; import { Store } from '../src/store.mjs'; import { Broker } from '../src/broker.mjs'; import { Client } from '../src/client.mjs'; @@ -13,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_STATE_FILE, servedCommit } from '../../library/src/mirror.mjs'; import { writeLibraryDoc, saveLibraryFile, revertLibraryCommit } from '../../library/src/write.mjs'; import { git } from '../../library/tests/helpers.mjs'; @@ -24,6 +26,7 @@ function world(t, options = {}) { const authority = { withinRole: ['message.send', 'library.write'], crossRole: [] }; const roles = { bean: { definition: 'companion', authority, identity: { name: 'Bean', email: 'bean@example.test', gitea: { user: 'bean', url: 'https://git.example.invalid' } }, publish: ['books'] }, chief: { definition: 'chief-of-staff', authority }, quill: { definition: 'proofreader', authority: { withinRole: ['message.send'], crossRole: [] } } }; + 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(); @@ -78,7 +81,9 @@ test('C2-02: writing instance supplies neither ident nor notice text; broker ins assert.ok(f.store.get('SELECT 1 FROM message_threads WHERE message=?', m.id)); assert.equal(f.seen[0].options.identity.name, 'Bean'); assert.equal(f.seen[0].options.credential.user, 'bean'); - await assert.rejects(f.call('bean', 'library.doc.write', { ...args(), author: 'other' }), { code: 'invalid-request' }); + assert.equal(f.seen[0].options.credential.agent, 'bean'); + for (const extra of [{ author: 'other' }, { agent: 'chief' }, { credential: { agent: 'chief' } }]) + await assert.rejects(f.call('bean', 'library.doc.write', { ...args(), ...extra }), { code: 'invalid-request' }); }); test('C2-02 A2: strict human revert args/result and agent human-required refusal', async t => { @@ -175,3 +180,105 @@ test('C2-02 socket: a save name nested in proof cannot enlarge an agent envelope assert.deepEqual(response, { ok: false, error: 'request-too-large' }); assert.equal(f.seen.length, 0); }); + + +test('C2-02 E2: held grant during fetch refuses at beforePush and preserves every ref', async t => { + const bare = await bareWorld(t), f = world(t, { projects: bare.context.business.projects }); + const before = bare.refs(), head = bare.remoteHead(); + const command = (kind, url, options) => { + if (kind === 'fetch') f.held.add('bean'); + return bare.command(kind, url, options); + }; + f.library.writeDoc = (context, options) => writeLibraryDoc({ ...context, dataRoot: bare.dataRoot }, + { ...options, credential: { ...options.credential, file: bare.file }, command }); + await assert.rejects(f.call('bean', 'library.doc.write', { ...args(), base: git(bare.mirror, 'rev-parse', `served:${PATH}`) }), + { code: 'library-write-no-grant' }); + assert.equal(bare.refs(), before); assert.equal(bare.remoteHead(), head); + assert.equal(f.store.get("SELECT count(*) n FROM messages WHERE from_role='bean'").n, 0); +}); + +test('C2-02 E3: expired, firing, missing publish, wrong grant user and missing ident refuse', async t => { + const cases = [ + { name: 'expired', prepare: f => f.store.run("UPDATE gitea_grants SET rotate_by='2000-01-01'"), code: 'library-write-no-grant' }, + { name: 'firing', prepare: f => f.store.run("INSERT INTO events(id,at,business,kind,actor_role,actor_run,subject,body) VALUES('fire-bean',?,'acme','action.allowed','chief','run','bean',?)", + new Date().toISOString(), JSON.stringify({ action: 'fire.stopped' })), code: 'library-write-no-grant' }, + { name: 'publish', changeRole: r => { r.publish = []; }, code: 'library-write-no-grant' }, + { name: 'grant user', prepare: f => f.store.run("UPDATE gitea_grants SET user='other-seat'"), code: 'library-write-no-grant' }, + { name: 'identity name', changeRole: r => { delete r.identity.name; }, code: 'library-write-refused' }, + { name: 'identity email', changeRole: r => { delete r.identity.email; }, code: 'library-write-refused' }, + ]; + for (const c of cases) { + const f = world(t, { changeRole: c.changeRole }); c.prepare?.(f); + for (const verb of ['library.doc.write', 'library.doc.delete']) { + const a = args(); if (verb.endsWith('delete')) delete a.content; + await assert.rejects(f.call('bean', verb, a), { code: c.code }, c.name); + } + assert.equal(f.seen.length, 0); + } +}); + + +test('C2-02 F2/E6 runtime: human saves/reverts use editor.user; no login never uses the fetch bot', { timeout: 30000 }, async t => { + for (const user of [undefined, 'her-gitea-login']) { + const bare = await bareWorld(t); + const agent = await writeLibraryDoc(bare.args(), bare.opts); + bare.entry.user = 'fetch-bot'; + if (user) bare.entry.editor.user = user; + const configDir = join(bare.root, 'config'), slots = join(configDir, 'secrets', 'acme'); + mkdirSync(slots, { recursive: true, mode: 0o700 }); + for (const slot of ['library-human', 'library-gitea']) + writeFileSync(join(slots, slot), 'fixture-runtime-token-288\n', { mode: 0o600 }); + const pushes = []; + const command = (kind, url, options) => { if (kind === 'push') pushes.push(options.user); return bare.command(kind, url, options); }; + const initial = (await bare.state()).lastFetch; + const business = { ...bare.context.business, human: 'jason', roles: { coder: { authority: { withinRole: [], crossRole: [] } } }, + arbiters: { technical: 'coder', delivery: 'coder' } }; + const runtime = await startBroker({ dataRoot: bare.dataRoot, businesses: { acme: business }, + librarySync: { configDir, command, intervalMs: 60000 } }); + try { + const human = runtime.broker.bindHuman({ business: 'acme', human: 'jason', via: 'cli', outsideAgent: true }); + const call = (verb, args) => Promise.resolve().then(() => runtime.broker.request(human, { verb, args })); + // Let the boot fetch finish before inspecting only the human operation's command count. + const deadline = Date.now() + 10000; + while (true) { + const state = await call('library.status', { project: 'books' }); + if (state.lastFetch !== initial) break; + if (Date.now() > deadline) throw Error('boot fetch did not complete'); + await new Promise(resolve => setTimeout(resolve, 10)); + } + const path = 'Book One/Chapters/Chapter 01.md'; + const saves = [ + ['library.file.save', { project: 'books', path, base: git(bare.mirror, 'rev-parse', `served:${path}`), content: 'her edit\n' }], + ['library.revert', { project: 'books', commit: agent.commit }], + ]; + const before = bare.commands.length; + for (const [verb, args] of saves) { + if (user) assert.match((await call(verb, args)).commit, /^[a-f0-9]{40}$/); + else await assert.rejects(call(verb, args), { code: 'library-human-missing' }); + } + assert.deepEqual(pushes, user ? [user, user] : []); + if (!user) assert.equal(bare.commands.length, before); + } finally { await runtime.close(); } + } +}); + + +test('C2-02 E4: post-push publication failure is outcome-unknown and has no stored notice', 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(); + const command = (kind, url, options) => { + // Break only state decoding after push admission; the real remote push still executes. + if (kind === 'push') writeFileSync(state, 'invalid state\n'); + return bare.command(kind, url, options); + }; + f.library.writeDoc = (context, options) => writeLibraryDoc({ ...context, dataRoot: bare.dataRoot }, + { ...options, credential: { ...options.credential, file: bare.file }, command }); + try { + 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(git(bare.mirror, 'rev-parse', 'served'), previous); + assert.equal(f.store.get("SELECT count(*) n FROM messages WHERE from_role='bean'").n, 0); + } finally { writeFileSync(state, original); } +}); diff --git a/packages/library/README.md b/packages/library/README.md index 7758508a0..e3295096a 100644 --- a/packages/library/README.md +++ b/packages/library/README.md @@ -310,3 +310,11 @@ existing five-second lock wait while network Git runs (MVP tradeoff, sage-3628). Publication errors return `outcome-unknown`: served is restored to match old state unless the new state was already published. The remote 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. diff --git a/packages/library/src/write.mjs b/packages/library/src/write.mjs index 8d0c7a0e1..fa31d409a 100644 --- a/packages/library/src/write.mjs +++ b/packages/library/src/write.mjs @@ -3,6 +3,7 @@ import { lstatSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { SAFE, git, gitBytes } from '../../project/src/git.mjs'; import { gitEnv, runGit, GIT_LIMITS } from '../../seat/src/checkout.mjs'; +import { ID_PATTERN } from '../../business/src/vocabulary.mjs'; import { checkMapping } from './mapping.mjs'; import { classify } from './scope.mjs'; import { supportingPath } from './supporting.mjs'; @@ -95,7 +96,8 @@ async function revertChange(mirror, served, context, mapping) { parents = (await git(mirror, ['show', '-s', '--format=%P', commit])).split(' '); } catch { refuse(); } if (parents.length !== 1 || !SHA.test(parents[0]) - || !new RegExp(`^${context.business.id}/[a-z][a-z0-9-]*$`).test(text)) refuse(); + || !text.startsWith(context.business.id + '/') + || !ID_PATTERN.test(text.slice(context.business.id.length + 1))) refuse(); const before = await listTree(mirror, parents[0]), after = await listTree(mirror, commit); const old = new Map(before.map(e => [e.path, e])), current = new Map(after.map(e => [e.path, e])); const changed = [...new Set([...old.keys(), ...current.keys()])].filter(path => @@ -161,7 +163,9 @@ async function write(context, options, human, revert = false) { const result = await push(context, { commit, credential, retried: attempt === 1, writeToken, command: options.command ?? syncCommand }); if (!result.pushed) { tip = result.tip; continue; } pushed = true; - await publishLibraryWrite(mirror, { mapping, commit, previous: served, readSyncState: lock.readSyncState, now: options.now }); + try { + 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 }; } fail('stale-base'); diff --git a/packages/library/tests/write-fixture.mjs b/packages/library/tests/write-fixture.mjs index 81f0c812e..dea1b368b 100644 --- a/packages/library/tests/write-fixture.mjs +++ b/packages/library/tests/write-fixture.mjs @@ -10,7 +10,7 @@ export const PATH = 'Book One/Notes/Agent.md'; export const IDENTITY = { name: 'Bean Fixture', email: 'bean@example.test' }; export const EDITOR = { name: 'Human Fixture', email: 'human@example.test' }; -export async function world(t) { +export async function world(t, { business = 'acme' } = {}) { const f = repo(t, { ...BASE, [PATH]: 'before\n', 'Book One/Notes/Executable.md': { text: 'exec\n', mode: 0o755 }, 'Book One/Notes/Link.md': { link: 'Agent.md' } }); @@ -19,9 +19,9 @@ export async function world(t) { const remote = join(f.root, 'remote.git'); git(f.root, 'clone', '-q', '--bare', '--no-local', f.repo, remote); const dataRoot = join(f.root, 'data'); mkdirSync(dataRoot, { mode: 0o700 }); - const entry = { ...mappingEntry(), user: 'human-fixture', editor: EDITOR }; + const entry = { ...mappingEntry(), user: 'human-fixture', editor: { ...EDITOR } }; entry.universes.harbor.supporting = ['Book One/Notes/']; - const context = { business: { id: 'acme', projects: { books: entry } }, project: 'books', dataRoot }; + const context = { business: { id: business, projects: { books: entry } }, project: 'books', dataRoot }; const file = join(f.root, 'fixture-slot'); writeFileSync(file, 'fixture-write-token-288\n', { mode: 0o600 }); const credential = { kind: 'seat', user: 'bean-fixture', file }; // Keep the production refspec exactly: a '+' mutant on push would overwrite the racing remote. @@ -49,7 +49,7 @@ export async function world(t) { }; const args = (extra = {}) => ({ ...context, universe: 'harbor', path: PATH, base: git(mirror, 'rev-parse', `served:${PATH}`), content: 'after\n', ...extra }); - const opts = { credential, identity: IDENTITY, agent: 'acme/bean', command }; + const opts = { credential, identity: IDENTITY, agent: `${business}/bean`, command }; return { ...f, remote, context, entry, file, credential, command, mirror, refs, remoteHead, state, external, args, opts, race: fn => { beforePush = fn; }, reject: () => { reject = true; }, commands }; } diff --git a/packages/library/tests/write.test.mjs b/packages/library/tests/write.test.mjs index 6294dd7b0..67e329330 100644 --- a/packages/library/tests/write.test.mjs +++ b/packages/library/tests/write.test.mjs @@ -155,3 +155,17 @@ test('C2-02 publication F1: failed state rename restores served to match prior s }); assert.equal((await readSyncState(f.mirror, { branch: 'main' })).served, previous); }); + + +test('C2-02 E1: a dotted business id never matches another business trailer', async t => { + const f = await world(t, { business: 'acme.co' }); + writeFileSync(join(f.repo, PATH), 'foreign seat change\n'); + git(f.repo, 'add', '-A'); git(f.repo, 'commit', '-qm', 'Foreign\n\nMosaic-Agent: acme-co/bean'); + const foreign = git(f.repo, 'rev-parse', 'HEAD'); + git(f.repo, 'push', '-q', f.remote, 'HEAD:main'); + await sync(f.context, { credential: f.credential, command: f.command }); + const before = f.refs(); + await assert.rejects(revertLibraryCommit({ ...f.context, commit: foreign }, + { credential: { ...f.credential, kind: 'human' }, command: f.command }), { code: 'library-revert-not-agent' }); + assert.equal(f.refs(), before); assert.equal(f.remoteHead(), foreign); +});