diff --git a/packages/bus/README.md b/packages/bus/README.md index dc19135af..1726b61b5 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -114,9 +114,10 @@ the task ref for an agent's landing, and the decision's target has to match it. So a decision raised for one can't land the other. land-work accepts only a decision the human resolved through the CLI (`human/cli`). The broker passes `arbiter: true`, which also accepts a `cross-role` -decision its arbiter resolved (REQ-DEC-2). It refuses a decision of class -`gated` with `not-authorized` (REQ-LAND-1): only a role granted -`git.merge.protected` cross-role lands, whoever resolved the decision. +decision its arbiter resolved (REQ-DEC-2). It refuses a decision of any +class but `cross-role` with `not-authorized` (REQ-LAND-1). That covers +`gated` and `within-role`: only a role granted `git.merge.protected` +cross-role lands, whoever resolved the decision. - `{op:'checkLand',cap,args}`: `args` is `{task_ref,decision}`. The caller must hold its claim and the decision: `git.merge.protected`, raised by @@ -127,7 +128,8 @@ decision its arbiter resolved (REQ-DEC-2). It refuses a decision of class latest push (`ref-moved`). A landing with no subject refuses `no-subject`, since an agent has no `--subject` to pass. Returns a ticket with what the host needs: the project, the work ref, the commit, - the verdict and the resolution. It records nothing unless it refuses. + the verdict and the resolution, with its `via` (`cli` or `broker`). It + records nothing unless it refuses. - `{op:'recordLand',ticket,land}`: `land` is `{commit,previous,reviewed, base,patch_id}` once `main` moved. The checks run again inside the transaction. It writes the `action.allowed` for `git.merge.protected` diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 5455b783d..a8f652522 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -807,12 +807,12 @@ export class Broker { }); } // What a landing needs, checked again at recordLand. PRD 1.5 REQ-LAND-1: only a role granted - // git.merge.protected cross-role lands, so a decision of class gated refuses, whoever resolved - // it. A cross-role one may be resolved by the human through the CLI or by its arbiter - // (REQ-DEC-2, landing.mjs `arbiter`). The decision's own project, when it names one, must be - // the push's. + // git.merge.protected cross-role lands, so a decision of any other class refuses, whoever + // resolved it: gated, or within-role, which its own raiser resolved. A cross-role one may be + // resolved by the human through the CLI or by its arbiter (REQ-DEC-2, landing.mjs `arbiter`). + // The decision's own project, when it names one, must be the push's. #landFacts(s, task_ref, decision, cls) { - if (cls === 'gated') fail('not-authorized'); + if (cls !== 'cross-role') fail('not-authorized'); if (this.#spent(s.business, decision)) fail('decision-consumed'); let l; try { @@ -838,6 +838,7 @@ export class Broker { subject: l.subject, choice: l.choice, by: l.by, + via: l.via, resolved_at: l.resolved_at, }; } diff --git a/packages/bus/src/landing.mjs b/packages/bus/src/landing.mjs index 30f8dfe05..f4d05ba66 100644 --- a/packages/bus/src/landing.mjs +++ b/packages/bus/src/landing.mjs @@ -109,6 +109,8 @@ export function readLanding(q, business, id, { into, arbiter = false }) { answer.actor_role === null && answer.actor_run === null && json(answer.body)?.choice === res.choice; + // A cross-role decision routed to an agent closes only by decision.resolve, which takes it from + // the routed role alone (s.role === route_to), so res.by here is the arbiter's run. const arbitrated = arbiter && d.class === 'cross-role' && d.route_to !== 'human' && res.via === 'broker' && !answers.length; if (!human && !arbitrated) refuse('not-human', `resolved by ${res.by} via ${res.via}, ${answers.length} human answer(s)`); @@ -150,6 +152,7 @@ export function readLanding(q, business, id, { into, arbiter = false }) { project: d.project ?? '', choice: res.choice, by: res.by, + via: res.via, resolved_at: res.at, subject: String(merge.text ?? '').split('\n')[0].trim(), }; diff --git a/packages/bus/tests/land.test.mjs b/packages/bus/tests/land.test.mjs index fde6b9bf2..24618c57b 100644 --- a/packages/bus/tests/land.test.mjs +++ b/packages/bus/tests/land.test.mjs @@ -28,12 +28,12 @@ const roles = { reviewer: { authority: { withinRole: ['review.verdict', 'message.send'], crossRole: [] } }, 'merge-gate': { authority: { withinRole: [], crossRole: [MERGE] } }, }; -const demo = (arbiters = { technical: 'pm', delivery: 'pm' }) => ({ +const demo = (arbiters = { technical: 'pm', delivery: 'pm' }, r = roles) => ({ id: 'demo', human: 'jason', arbiters, projects: { stack: { root: '/nonexistent' } }, - roles, + roles: r, }); const options = [ { key: 'yes', text: 'Land the parser fix\nIt is reviewed.' }, @@ -41,10 +41,10 @@ const options = [ ]; const merge = { action: MERGE, target: TASK, question: 'Squash-merge work/vikunja-1-2 into main?', options, recommendation: 'yes', blocking: false }; -function setup(t, { arbiters } = {}) { +function setup(t, { arbiters, roles: r } = {}) { const root = mkdtempSync(join(tmpdir(), 'bus-land-')); const store = new Store(root); - const businesses = { demo: demo(arbiters) }; + const businesses = { demo: demo(arbiters, r) }; const b = new Broker({ store, businesses }); t.after(() => { store.close(); @@ -109,6 +109,7 @@ test('REQ-DEC-2: a merge-gate raise its arbiter resolved lands through the host subject: 'Land the parser fix', choice: 'yes', by: 'pm-run', + via: 'broker', }); assert.deepEqual(w.merges(), []); assert.deepEqual(w.events('action.refused'), []); @@ -200,6 +201,22 @@ test('REQ-LAND-1: a gated merge decision refuses not-authorized, cited and appro assert.deepEqual([...w.merges(), ...w.events('action.refused')], []); }); +// Round 4 (sage-0585): a role granted git.merge.protected within-role resolves its own merge +// raise. That isn't the cross-role grant REQ-LAND-1 asks for, so it refuses at the land too. +test('REQ-LAND-1: a within-role merge grant refuses not-authorized at the land', (t) => { + const w = setup(t, { + roles: { ...roles, 'merge-gate': { authority: { withinRole: [MERGE], crossRole: [] } } }, + }); + w.push(A); + w.approve(A); + const d = w.raise('merge-gate', { choice: 'yes' }); + assert.deepEqual(w.routed(d), { class: 'within-role', route_to: 'merge-gate' }); + assert.deepEqual({ ...w.store.get('SELECT op, choice, via FROM decision_events WHERE decision=?', d.id) }, { op: 'resolved', choice: 'yes', via: 'broker' }); + assert.throws(() => w.check({ task_ref: TASK, decision: d.id }), /not-authorized/); + assert.equal(w.held(), false); + assert.deepEqual([...w.merges(), ...w.events('action.refused')], []); +}); + test('a cross-role merge that routes to the human, because merge-gate is its own arbiter, lands on the human\'s CLI answer', (t) => { const w = setup(t, { arbiters: { technical: 'merge-gate', delivery: 'merge-gate' } }); w.push(A); @@ -208,8 +225,8 @@ test('a cross-role merge that routes to the human, because merge-gate is its own assert.deepEqual(w.routed(d), { class: 'cross-role', route_to: 'human' }); assert.throws(() => w.resolve(d), /human-required/); w.resolve(d, 'yes', 'human'); - const { ticket, by } = w.check({ task_ref: TASK, decision: d.id }); - assert.equal(by, 'jason'); + const { ticket, by, via } = w.check({ task_ref: TASK, decision: d.id }); + assert.deepEqual([by, via], ['jason', 'cli']); w.b.recordLand(ticket, w.land); assert.equal(w.merges()[0].body.class, 'cross-role'); }); @@ -230,9 +247,11 @@ test('a merge option with no first line refuses no-subject, as land-work does', // The codes readLanding gives once #checkAuthority has passed, on the 'main' // side. scripts/test-land-work.sh runs every code on the 'refactor' side. -// not-human can't reach checkLand: a gated decision refuses not-authorized -// first, and the broker resolves a cross-role one the two ways readLanding -// takes with `arbiter`. The REQ-LAND-1 test calls readLanding for it. +// checkLand never gives not-human: #landFacts refuses every class but +// cross-role with not-authorized first, and the broker resolves a cross-role +// decision only the two ways readLanding takes with `arbiter`, the human's CLI +// answer or its arbiter's decision.resolve. The REQ-LAND-1 test calls +// readLanding for not-human. test("checkLand refuses with land-work's codes past the authority check, and a decision raised for refactor never lands", (t) => { const w = setup(t); const check = (d) => () => w.check({ task_ref: TASK, decision: d.id }); diff --git a/packages/cli/README.md b/packages/cli/README.md index 3185eea51..9fde6db6d 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -455,6 +455,8 @@ working repository's `main`, never the project root: the host a ticket (`packages/bus/README.md`). 2. `prepareLand` builds the squash in a worktree under `bus-host/land//` (`packages/seat/README.md`, "Landing on main"). + The message is land-work's. Its Resolved line names the channel from + the ticket's `via`: the human via the CLI, or the arbiter via the broker. 3. The project root has to be a mosaic-stack checkout with `node_modules` (`isMosaicStack`), or `gate-missing`. The root's `node_modules` is linked into the worktree. diff --git a/packages/cli/src/git-socket.mjs b/packages/cli/src/git-socket.mjs index 2a9ef694f..0c13c884d 100644 --- a/packages/cli/src/git-socket.mjs +++ b/packages/cli/src/git-socket.mjs @@ -247,12 +247,16 @@ function dropNetwork(name, env) { } export const LAND_GATE = { classify: isMosaicStack, run: runGate, dropNetwork }; +// How the decision was resolved, by the resolution's via: the human through the CLI, or the +// arbiter through the broker (row 137, REQ-DEC-2). readLanding takes no other. +const RESOLVED_VIA = { cli: "via the CLI", broker: "as its arbiter, via the broker" }; + // The squash's message: land-work's, applied under the git socket. function landMessage(f, { old, base, patch_id }) { return `${f.subject} Squash of ${f.work_ref} (${f.task_ref}) at ${f.commit} onto main ${old}, -the reviewed diff ${base}..${f.commit}. Resolved ${f.choice} by ${f.by} via the CLI at ${f.resolved_at}. +the reviewed diff ${base}..${f.commit}. Resolved ${f.choice} by ${f.by} ${RESOLVED_VIA[f.via] ?? `via ${f.via}`} at ${f.resolved_at}. Decision: ${f.decision} Verdict: ${f.verdict} diff --git a/packages/cli/tests/git-socket.test.mjs b/packages/cli/tests/git-socket.test.mjs index e57d2dd30..5284189c9 100644 --- a/packages/cli/tests/git-socket.test.mjs +++ b/packages/cli/tests/git-socket.test.mjs @@ -667,7 +667,8 @@ async function landSetup(t, { gate = {}, ...opts } = {}) { await w.push("coder"); state.facts = { task_ref: TASK, work_ref: "work/vikunja-1-2", decision: "decision-1", project: "stack", ref: "work/vikunja-1-2", commit: c1, - verdict: "verdict-1", subject: "Land the parser fix", choice: "yes", by: "jason", resolved_at: "2026-10-16T00:00:00.000Z", + verdict: "verdict-1", subject: "Land the parser fix", choice: "yes", by: "jason", via: "cli", + resolved_at: "2026-10-16T00:00:00.000Z", }; const land = () => landWork({ path: w.sock.path, cap: w.roles.coder.cap, task_ref: TASK, decision: "decision-1" }); const git = (...args) => gitIn(w.root, `--git-dir=${w.repo}`, ...args); @@ -701,6 +702,7 @@ test("land: the squash runs the gate in its own worktree, then main moves and th assert.equal(w.git("rev-parse", `${r.commit}^{tree}`), w.git("rev-parse", `${w.c1}^{tree}`)); const msg = w.git("show", "-s", "--format=%B", r.commit); assert.match(msg, /^Land the parser fix\n/); + assert.ok(msg.includes(" Resolved yes by jason via the CLI at 2026-10-16T00:00:00.000Z.\n"), msg); for (const line of [`Decision: decision-1`, `Verdict: verdict-1`, `Reviewed-commit: ${w.c1}`, `Base: main ${w.main}`, `Patch-id: ${r.patch_id}`]) assert.ok(msg.includes(`\n${line}\n`), line); // w.git trims, so the last line has no newline after it. assert.match(msg, /\nApplied-under: git socket land \(bus host\)$/); @@ -732,6 +734,18 @@ test("land: the squash runs the gate in its own worktree, then main moves and th assert.deepEqual(w.logs, []); }); +// Row 137 B1: the squash names the channel that resolved the decision. An arbiter's +// resolution came through the broker, not the CLI. +test("land: an arbiter-resolved landing's squash says the arbiter resolved it through the broker", async (t) => { + const w = await landSetup(t); + w.state.facts = { ...w.state.facts, by: "pm-run", via: "broker" }; + const r = await w.land(); + await w.ended(); + const msg = w.git("show", "-s", "--format=%B", r.commit); + assert.ok(msg.includes(" Resolved yes by pm-run as its arbiter, via the broker at 2026-10-16T00:00:00.000Z.\n"), msg); + assert.ok(!msg.includes("via the CLI"), msg); +}); + test("land: refusals before the gate answer the role at once and leave main and no worktree", async (t) => { // Until `fit` goes false the gate fits; then the host's own check decides, // and the fixture's project is no mosaic-stack checkout.