jason.woltje
158205c5cc
Unclaimed files left in the canonical checkout. Inventory: canonical-cleanup-inventory.md (commit-as-evidence groups). detect-secrets scan clean on 2026-11-02: hits are sha256 pins, git object ids, test sentinels already in tracked tests and unit-file lines.
245 lines
13 KiB
Diff
245 lines
13 KiB
Diff
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/<id>/` (`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.
|