mosaicstack/stack
25
0 Fork 1
Code Issues 404 Pull Requests 13 Packages Projects Releases 15 Wiki Activity
Files
org-redact6
stack/agents/darkwing/work/queue-93/r1/build.patch
T
jason.woltje 158205c5cc lead: commit darkwing's untracked evidence (348 files: queue-137 queue-141 queue-150 queue-162 queue-165 queue-166 queue-167 queue-169 queue-179 queue-180 queue-181 queue-182 queue-185 queue-186 queue-187 queue-188 queue-190 queue-93) 
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.
2026-11-02 15:05:14 -06:00

2798 lines
147 KiB
Diff
Raw Permalink Blame History

diff --git a/packages/bus/README.md b/packages/bus/README.md
index 527768b1a..793694bae 100644
--- a/packages/bus/README.md
+++ b/packages/bus/README.md
@@ -73,9 +73,11 @@ S6 adds launch ops on the same IPC channel, one reply each
- `{op:'identity',cap}`: the business, role and run a capability names.
- `{op:'authorizeLaunch',cap,instance}`: `authorize(cap,'role.launch',
{target:instance})`; a refusal is also recorded as `action.refused`.
-- `{op:'refuse',cap,code}`: records a refusal the host decided itself
- (instance list, capacity, a failed spawn) as `action.refused` against the
- caller, the code only.
+- `{op:'refuse',cap,code,subject?}`: records a refusal the host decided
+ itself (instance list, capacity, a failed spawn, a git socket check) as
+ `action.refused` against the caller, the code only. A task ref as
+ `subject` files the refusal on that task's trail, as a refused verb's
+ does; any other subject, or none, records null (row 93).
- `{op:'endLaunch',record}`: `{business,run,reason,exitCode?}` for a run
this broker bound. It writes `session.ended` `{reason,exitCode,released}`,
releases the run's claim if it still holds it, and drops every capability
@@ -85,6 +87,25 @@ S6 adds launch ops on the same IPC channel, one reply each
- `{op:'credentialStatus',business,role}`: the role's `{service,date,state}`
rows, never a token.
+Row 93 (#1587) adds three ops for the host's git socket
+(`packages/cli/src/git-socket.mjs`). A refusal from any of them is recorded
+as `action.refused` on the task's trail.
+
+- `{op:'checkPush',cap,task_ref}`: `checkPush(cap,task_ref)` checks the
+ caller's `git.push.working` authority before the host takes a push's
+ bundle. It records nothing unless it refuses.
+- `{op:'recordPush',cap,push}`: `push` is `{task_ref,project,branch,ref,
+ commit,previous}` once the work ref moved. It checks the authority again
+ and writes the `action.allowed` for `git.push.working`, with the task as
+ subject and `{project,branch,ref,commit,previous}` in its body. That event
+ is the push record: the only `action.allowed` for `git.push.working` that
+ names a commit. The project must be one of the business's. The host moves
+ the ref back when this refuses.
+- `{op:'reviewFetch',cap,args}`: `args` is `{task_ref,request}`. The caller
+ must hold `review.verdict` and be the role the `review.requested` event was
+ addressed to (`request-not-found` otherwise). Returns the request's
+ `{project,commit,ref}`, which the host bundles for the reviewer.
+
`{op:'close'}`, SIGTERM and SIGINT close the socket and DB and remove the owned
writer lock. Losing the host IPC channel closes with exit 2. Startup or host
protocol failures return a static error code and exit 2, never a raw exception.
@@ -123,6 +144,8 @@ an operator inspect content and delivery evidence after an uncertain result.
| `message.send` | `{to,body,class?,in_reply_to?,corrects?,decision?}`; returns `{id,request}`. Human instruction's `request` is its `human.input` event ID. |
| `message.receive` | No args; up to 100 undelivered messages for current holder, including originating `request` for human instructions. |
| `message.read` | `{id}`; recipient holder's acknowledgement; no new holder can acknowledge old holder's delivery. |
+| `review.request` | `{task_ref,commit,to,note?,decision?}`; action `review.request` (row 93, #1587). `commit` must be the task's latest pushed commit: an older one refuses `stale-commit`, any other `unknown-commit`. `to` must be another role holding `review.verdict` (`unknown-role`, `self-review`, `not-a-reviewer`). Writes a `review.requested` event `{task_ref,project,ref,commit,to,message}` and sends `to` a REVIEW-REQUEST message naming it. Returns `{request,message,commit,ref}`. |
+| `review.verdict` | `{task_ref,request,verdict,body,decision?}`; action `review.verdict`. `verdict` is `approve` or `changes`. `request` must be a `review.requested` event on the task addressed to the caller's role (`request-not-found`), with no verdict yet (`verdict-exists`). Writes a `review.verdict` event `{task_ref,request,commit,verdict,message}` and sends the requesting role a REVIEW-RESULT in reply to the request's message. Returns `{verdict,message,commit}`. |
| `event.emit` | `{kind:'action.allowed',body:{action:'routine',target?},subject?}`; routine observations only. |
| `launch.revoke`, `launch.restore` | No args; outside-agent human CLI only. |
| `inbox` | No args; plain JSON projection scoped to capability's business. |
@@ -337,7 +360,18 @@ it is not itself an external service executor. An adapter must not treat a prior
check as authority after an await/holder change. Task/credential/poll registration
and serialized external operations are S3's integration work, not a generic
user-defined-handler endpoint in S2. `recordEvent` stamps identity, uses a
-transaction, and cannot emit human input or human launch control events.
+transaction, and cannot emit human input or human launch control events,
+or the review loop's `review.requested` and `review.verdict` events, or an
+`action.allowed` for `git.push.working`. Only the verbs and host ops above
+write those. The review loop adds no event kind and no schema change.
+
+`authorize(cap,'task.close',{target,verdict})` checks the verdict on a task
+with a recorded push, inside the same transaction (row 93): it
+must be the id of a `review.verdict` event on that task
+(`verdict-not-found`), with verdict `approve` (`verdict-not-approve`), for
+the task's latest pushed commit (`verdict-stale`). A refusal rolls the
+`action.allowed` back. On a task with no push the verdict stays a citation
+the broker doesn't check.
Task refs in action/review bodies require matching `subject`; all task events
are also guarded by the pinned schema. `task.created.body.request` must name an
existing same-business human input, with a requirement ID.
diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs
index 09a18633e..5e874ad3e 100644
--- a/packages/bus/src/broker.mjs
+++ b/packages/bus/src/broker.mjs
@@ -85,6 +85,12 @@ function identifier(x) {
return x;
}
const taskRef = (x) => typeof x === 'string' && /^vikunja:[1-9][0-9]*\/[1-9][0-9]*$/.test(x);
+// A git commit id, SHA-1 or SHA-256.
+const oid = (x) => typeof x === 'string' && /^[0-9a-f]{40}([0-9a-f]{24})?$/.test(x);
+const VERDICTS = ['approve', 'changes'];
+// Row 93 (#1587): the review loop's evidence, written only by the verbs below. A push is an
+// action.allowed for git.push.working that names its commit, written only by recordPush.
+const REVIEW_KINDS = ['review.requested', 'review.verdict'];
export class Broker {
#store;
@@ -232,10 +238,11 @@ export class Broker {
}
// Trusted launcher API: records a refusal the host decided itself (instance list, capacity,
// credentials) as action.refused against the caller, the way request() records its own.
- refuse(cap, code) {
+ // A task ref as subject files the refusal on that task's trail, as request() does.
+ refuse(cap, code, subject = null) {
const s = this.#session(cap);
if (!/^[a-z-]{1,64}$/.test(code)) fail('invalid-request');
- return this.#refused(s, new BusError(code), null).code;
+ return this.#refused(s, new BusError(code), { args: { task_ref: subject } }).code;
}
// Trusted human transport invokes only AFTER checking its CLI process and launch ancestry.
bindHuman(record) {
@@ -422,7 +429,8 @@ export class Broker {
return { class: cls, decision: d.id };
}
// Caller owns the transaction: authority, consumption and effect commit together.
- #consumeAuthority(s, action, context = {}) {
+ // detail adds to the action.allowed body: a push's commit (row 93).
+ #consumeAuthority(s, action, context = {}, detail = {}) {
const result = this.#checkAuthority(s, action, context);
// decision.raise records context, not permission consumed by an executor.
if (
@@ -437,22 +445,111 @@ export class Broker {
this.#event(
s,
'action.allowed',
- { action, ...result, target: context.target ?? null },
+ { ...detail, action, ...result, target: context.target ?? null },
context.target ?? null,
);
return result;
}
// Trusted S3 handler calls inside its own operation; not a generic socket action executor.
+ // Row 93 (#1587): task.close on a task with a recorded push also needs
+ // context.verdict, checked by #closeVerdict; a refusal there rolls the action.allowed back.
authorize(cap, action, context = {}) {
const s = this.#session(cap);
- return this.#store.transaction(() => this.#consumeAuthority(s, action, context));
+ return this.#store.transaction(() => {
+ const result = this.#consumeAuthority(s, action, context);
+ if (action === 'task.close') this.#closeVerdict(s, context.target, context.verdict);
+ return result;
+ });
+ }
+ // A task with a pushed commit closes only on an approve verdict for the latest push. A task with
+ // no push closes as before, its verdict a citation the broker doesn't check.
+ #closeVerdict(s, task, verdict) {
+ const push = this.#latestPush(s.business, task);
+ if (!push) return;
+ const v = id(verdict)
+ ? this.#store.get(
+ "SELECT body FROM events WHERE business=? AND kind='review.verdict' AND id=? AND subject=?",
+ s.business,
+ verdict,
+ task,
+ )
+ : null;
+ if (!v) fail('verdict-not-found');
+ const body = JSON.parse(v.body);
+ if (body.verdict !== 'approve') fail('verdict-not-approve');
+ if (body.commit !== push.commit) fail('verdict-stale');
+ }
+ // A push is the action.allowed recordPush wrote, the only one for git.push.working with a commit.
+ #latestPush(business, task) {
+ const r = this.#store.get(
+ "SELECT body FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.action')='git.push.working' AND json_type(body,'$.commit')='text' AND subject=? ORDER BY seq DESC LIMIT 1",
+ business,
+ task,
+ );
+ return r ? JSON.parse(r.body) : null;
+ }
+ // Row 93: the bus host's git socket asks before it takes a push's bytes. Nothing is recorded
+ // unless it refuses: the push is recorded, and its authority consumed, once the ref moved.
+ checkPush(cap, task_ref) {
+ const s = this.#session(cap);
+ return this.#store.transaction(() => {
+ this.#agent(s);
+ if (!taskRef(task_ref)) fail('invalid-request');
+ return this.#checkAuthority(s, 'git.push.working', { target: task_ref });
+ });
+ }
+ // Row 93: records a push the git socket made (packages/seat/src/working.mjs) as the action.allowed
+ // for git.push.working, with the commit. A refusal here moves the ref back. Trusted host only.
+ recordPush(cap, push) {
+ const s = this.#session(cap);
+ return this.#store.transaction(() => {
+ this.#agent(s);
+ keys(push, ['task_ref', 'project', 'branch', 'ref', 'commit', 'previous'], ['task_ref', 'project', 'branch', 'ref', 'commit', 'previous']);
+ if (
+ !taskRef(push.task_ref) ||
+ !Object.hasOwn(this.#business(s.business).projects ?? {}, push.project) ||
+ !oid(push.commit) ||
+ (push.previous !== null && !oid(push.previous))
+ )
+ fail('invalid-request');
+ string(push.branch, 200);
+ string(push.ref, 200);
+ const { task_ref, project, branch, ref, commit, previous } = push;
+ this.#consumeAuthority(s, 'git.push.working', { target: task_ref }, { project, branch, ref, commit, previous });
+ return { task_ref, commit };
+ });
+ }
+ // Row 93: what the git socket's fetch for a reviewer may send, from the review.requested event the
+ // reviewer cites. The caller must hold review.verdict and be the request's reviewer. Trusted host only.
+ reviewFetch(cap, { task_ref, request } = {}) {
+ const s = this.#session(cap);
+ return this.#store.transaction(() => {
+ this.#agent(s);
+ if (!taskRef(task_ref) || !id(request)) fail('invalid-request');
+ if (this.#classification(s, 'review.verdict') === 'gated') fail('not-authorized');
+ const r = this.#reviewRequest(s, task_ref, request);
+ return { project: r.project, commit: r.commit, ref: r.ref };
+ });
+ }
+ // A review.requested event on the task, addressed to this session's role.
+ #reviewRequest(s, task, request) {
+ const r = this.#store.get(
+ "SELECT actor_role, body FROM events WHERE business=? AND kind='review.requested' AND id=? AND subject=?",
+ s.business,
+ request,
+ task,
+ );
+ const body = r ? JSON.parse(r.body) : null;
+ if (!body || body.to !== s.role) fail('request-not-found');
+ return { ...body, from: r.actor_role };
}
// Trusted adapters only. No agent socket route reaches this method.
recordEvent(cap, { kind, body, subject = null }) {
const s = this.#session(cap);
return this.#store.transaction(() => {
this.#agent(s);
- if (['human.input', 'launch.revoked', 'launch.restored'].includes(kind)) fail('reserved-event');
+ if (['human.input', 'launch.revoked', 'launch.restored', ...REVIEW_KINDS].includes(kind)) fail('reserved-event');
+ if (kind === 'action.allowed' && body?.action === 'git.push.working') fail('reserved-event');
return this.#event(s, kind, body, subject);
});
}
@@ -873,6 +970,83 @@ export class Broker {
this.#delivery(m.id, 'read', s.human ? null : this.#holder(s.business, s.role));
return { read: true };
}
+ // Row 93 (#1587): the coder asks a reviewer role to review the task's latest pushed commit.
+ case 'review.request': {
+ keys(a, ['task_ref', 'commit', 'to', 'note', 'decision'], ['task_ref', 'commit', 'to']);
+ if (!taskRef(a.task_ref) || !oid(a.commit)) fail('invalid-request');
+ const withinRole = this.#classification(s, 'review.request') === 'within-role';
+ this.#consumeAuthority(s, 'review.request', {
+ decision: withinRole ? null : (a.decision ?? null),
+ target: a.task_ref,
+ });
+ if (a.note !== undefined) string(a.note, 16384);
+ if (!Object.hasOwn(b.roles, a.to)) fail('unknown-role');
+ if (a.to === s.role) fail('self-review');
+ const reviewer = b.roles[a.to].authority;
+ if (![...reviewer.withinRole, ...reviewer.crossRole].includes('review.verdict')) fail('not-a-reviewer');
+ const push = this.#latestPush(s.business, a.task_ref);
+ if (push?.commit !== a.commit) {
+ const older = this.#store.get(
+ "SELECT 1 FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.action')='git.push.working' AND subject=? AND json_extract(body,'$.commit')=?",
+ s.business,
+ a.task_ref,
+ a.commit,
+ );
+ fail(older ? 'stale-commit' : 'unknown-commit');
+ }
+ const mid = randomUUID();
+ const eid = this.#event(
+ s,
+ 'review.requested',
+ { task_ref: a.task_ref, project: push.project, ref: push.ref, commit: a.commit, to: a.to, message: mid },
+ a.task_ref,
+ );
+ this.#reviewMessage(s, mid, {
+ to: a.to,
+ cls: 'REVIEW-REQUEST',
+ decision: withinRole ? null : (a.decision ?? null),
+ body: `Review request ${eid} for ${a.task_ref}: commit ${a.commit} on ${push.ref} in project ${push.project}. Fetch it with fetch_review, then answer with review_verdict citing this request.${a.note ? '\n\n' + a.note : ''}`,
+ });
+ return { request: eid, message: mid, commit: a.commit, ref: push.ref };
+ }
+ // Row 93: the reviewer's answer to one review.request, sent back to the role that asked.
+ case 'review.verdict': {
+ keys(a, ['task_ref', 'request', 'verdict', 'body', 'decision'], ['task_ref', 'request', 'verdict', 'body']);
+ if (!taskRef(a.task_ref) || !id(a.request) || !VERDICTS.includes(a.verdict)) fail('invalid-request');
+ const withinRole = this.#classification(s, 'review.verdict') === 'within-role';
+ this.#consumeAuthority(s, 'review.verdict', {
+ decision: withinRole ? null : (a.decision ?? null),
+ target: a.task_ref,
+ });
+ string(a.body, 32768);
+ const r = this.#reviewRequest(s, a.task_ref, a.request);
+ // One verdict per request: a changed mind needs a new request, so a close can't cite an
+ // approve that a later verdict on the same request took back.
+ if (
+ this.#store.get(
+ "SELECT 1 FROM events WHERE business=? AND kind='review.verdict' AND subject=? AND json_extract(body,'$.request')=?",
+ s.business,
+ a.task_ref,
+ a.request,
+ )
+ )
+ fail('verdict-exists');
+ const mid = randomUUID();
+ const eid = this.#event(
+ s,
+ 'review.verdict',
+ { task_ref: a.task_ref, request: a.request, commit: r.commit, verdict: a.verdict, message: mid },
+ a.task_ref,
+ );
+ this.#reviewMessage(s, mid, {
+ to: r.from,
+ cls: 'REVIEW-RESULT',
+ in_reply_to: r.message,
+ decision: withinRole ? null : (a.decision ?? null),
+ body: `Review verdict ${eid} on ${a.task_ref}, commit ${r.commit}: ${a.verdict}. A task_close cites this verdict id.\n\n${a.body}`,
+ });
+ return { verdict: eid, message: mid, commit: r.commit };
+ }
case 'launch.revoke':
case 'launch.restore': {
keys(a, []);
@@ -1055,6 +1229,25 @@ export class Broker {
decision,
);
}
+ // The message a review verb sends, routed like message.send's.
+ #reviewMessage(s, mid, { to, cls, in_reply_to = null, decision = null, body }) {
+ this.#store.run(
+ 'INSERT INTO messages(id,at,business,from_role,from_run,to_role,class,in_reply_to,decision,corrects,body) VALUES(?,?,?,?,?,?,?,?,?,?,?)',
+ mid,
+ this.#now(),
+ s.business,
+ s.role,
+ s.run,
+ to,
+ cls,
+ in_reply_to,
+ decision,
+ null,
+ body,
+ );
+ const h = this.#holder(s.business, to);
+ this.#delivery(mid, 'routed', h?.op === 'claim' ? h : null);
+ }
#delivery(mid, op, h, transport = null) {
this.#store.run(
'INSERT INTO deliveries(message,at,op,holder_run,transport,address) VALUES(?,?,?,?,?,?)',
diff --git a/packages/bus/src/process.mjs b/packages/bus/src/process.mjs
index 68276a03b..0cb3f3e05 100644
--- a/packages/bus/src/process.mjs
+++ b/packages/bus/src/process.mjs
@@ -2,12 +2,25 @@
// never command arguments, stdout or service-token-bearing environment variables.
import { startBroker } from './runtime.mjs';
import { BusError } from './broker.mjs';
-const LAUNCH_OPS = new Set(['identity', 'authorizeLaunch', 'refuse', 'endLaunch', 'credentialStatus']);
+const LAUNCH_OPS = new Set([
+ 'identity',
+ 'authorizeLaunch',
+ 'refuse',
+ 'endLaunch',
+ 'credentialStatus',
+ 'checkPush',
+ 'recordPush',
+ 'reviewFetch',
+]);
function launchOp(m) {
if (m.op === 'identity') return runtime.identity(m.cap);
if (m.op === 'authorizeLaunch') return runtime.authorizeLaunch(m.cap, m.instance);
- if (m.op === 'refuse') return runtime.refuse(m.cap, m.code);
+ if (m.op === 'refuse') return runtime.refuse(m.cap, m.code, m.subject);
if (m.op === 'endLaunch') return runtime.endLaunch(m.record);
+ // Row 93 (#1587): the git socket's ops.
+ if (m.op === 'checkPush') return runtime.checkPush(m.cap, m.task_ref);
+ if (m.op === 'recordPush') return runtime.recordPush(m.cap, m.push);
+ if (m.op === 'reviewFetch') return runtime.reviewFetch(m.cap, m.args);
return runtime.credentialStatus(m.business, m.role);
}
// The host's request id comes back on the reply, so a late reply can't
diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs
index e72f65d6f..a4912853f 100644
--- a/packages/bus/src/runtime.mjs
+++ b/packages/bus/src/runtime.mjs
@@ -75,15 +75,22 @@ export async function startBroker({
});
// S6 launcher operations. The host decides capacity and the instance list; the broker
// authorizes role.launch and keeps the evidence, refusals included.
- function authorizeLaunch(cap, instance) {
+ // A refusal is recorded against the caller, as request() records one.
+ function refusing(cap, fn, subject = null) {
try {
- return broker.authorize(cap, 'role.launch', { target: instance });
+ return fn();
} catch (e) {
if (!(e instanceof BusError)) throw e;
- broker.refuse(cap, e.code);
+ broker.refuse(cap, e.code, subject);
throw e;
}
}
+ const authorizeLaunch = (cap, instance) =>
+ refusing(cap, () => broker.authorize(cap, 'role.launch', { target: instance }));
+ // Row 93 (#1587): the host's git socket. A push is checked before the host takes the bundle
+ // and recorded once the ref moved; a reviewer's fetch is checked against its review.requested.
+ const checkPush = (cap, task_ref) => refusing(cap, () => broker.checkPush(cap, task_ref), task_ref);
+ const reviewFetch = (cap, args) => refusing(cap, () => broker.reviewFetch(cap, args), args?.task_ref);
// Metadata only (service, date, state); the token stays in Credentials.
function credentialStatus(business, role) {
return credentials
@@ -99,7 +106,10 @@ export async function startBroker({
endLaunch: (r) => broker.endLaunch(r),
identity: (cap) => broker.identity(cap),
authorizeLaunch,
- refuse: (cap, code) => broker.refuse(cap, code),
+ checkPush,
+ recordPush: (cap, push) => refusing(cap, () => broker.recordPush(cap, push), push?.task_ref),
+ reviewFetch,
+ refuse: (cap, code, subject) => broker.refuse(cap, code, subject),
credentialStatus,
launches: bound,
readers: readCaps,
diff --git a/packages/bus/tests/broker.test.mjs b/packages/bus/tests/broker.test.mjs
index d5d53d9e6..5dcf091c7 100644
--- a/packages/bus/tests/broker.test.mjs
+++ b/packages/bus/tests/broker.test.mjs
@@ -197,7 +197,7 @@ test('task action subjects and linked decision trail are complete and ordered',
for (let i = 1; i < rows.length; i++)
assert.ok(rows[i].at > rows[i - 1].at, 'broker write order has unique timestamps');
assert.throws(
- () => b.recordEvent(c, { kind: 'review.verdict', body: { task_ref: 'vikunja:3/1' } }),
+ () => b.recordEvent(c, { kind: 'action.refused', body: { task_ref: 'vikunja:3/1' } }),
/task-subject-required/,
);
});
diff --git a/packages/bus/tests/review.test.mjs b/packages/bus/tests/review.test.mjs
new file mode 100644
index 000000000..9a11b8034
--- /dev/null
+++ b/packages/bus/tests/review.test.mjs
@@ -0,0 +1,213 @@
+// Row 93 (#1587): the review loop's broker side. The host's git socket
+// checks a push (checkPush) and records it once the ref moved (recordPush), the
+// coder asks for a review (review.request, a review.requested event),
+// the reviewer's fetch is checked against that request (reviewFetch), the
+// reviewer answers (review.verdict), and task.close on a pushed task needs an
+// approve verdict for the latest push.
+
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { Store } from '../src/store.mjs';
+import { Broker } from '../src/broker.mjs';
+
+const TASK = 'vikunja:1/2';
+const A = 'a'.repeat(40);
+const B = 'b'.repeat(40);
+const businesses = {
+ demo: {
+ id: 'demo',
+ human: 'jason',
+ arbiters: { technical: 'cto', delivery: 'pm' },
+ projects: { stack: { root: '/nonexistent' } },
+ roles: {
+ pm: { authority: { withinRole: ['message.send', 'task.close'], crossRole: [] } },
+ cto: { authority: { withinRole: ['message.send', 'review.request'], crossRole: [] } },
+ coder: {
+ authority: { withinRole: ['task.update.assigned', 'git.push.working', 'review.request', 'message.send'], crossRole: [] },
+ },
+ reviewer: { authority: { withinRole: ['review.verdict', 'message.send', 'task.update.assigned'], crossRole: [] } },
+ second: { authority: { withinRole: ['review.verdict'], crossRole: [] } },
+ },
+ },
+};
+
+function setup(t) {
+ const root = mkdtempSync(join(tmpdir(), 'bus-review-'));
+ const store = new Store(root);
+ const b = new Broker({ store, businesses });
+ t.after(() => {
+ store.close();
+ rmSync(root, { recursive: true, force: true });
+ });
+ const caps = {};
+ for (const role of Object.keys(businesses.demo.roles)) {
+ caps[role] = b.bindLaunch({ business: 'demo', role, run: `${role}-run`, harness: 'pi' });
+ b.request(caps[role], { verb: 'role.claim', args: {} });
+ }
+ const call = (role, verb, args) => b.request(caps[role], { verb, args });
+ // What the git socket does for a push: check, then record once the ref moved.
+ const push = (commit, previous = null, task_ref = TASK) => {
+ b.checkPush(caps.coder, task_ref);
+ return b.recordPush(caps.coder, { task_ref, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit, previous });
+ };
+ const events = (kind) => store.all('SELECT * FROM events WHERE kind=? ORDER BY seq', kind);
+ const pushes = () => events('action.allowed').filter((e) => JSON.parse(e.body).action === 'git.push.working');
+ return { b, store, caps, call, push, events, pushes };
+}
+
+test('a push is recorded as the action.allowed for git.push.working on the task, and only the host records one', (t) => {
+ const { b, caps, push, events, pushes } = setup(t);
+ // The check before the host takes the bundle records nothing unless it refuses.
+ b.checkPush(caps.coder, TASK);
+ assert.equal(events('action.allowed').length, 0);
+ assert.throws(() => b.checkPush(caps.reviewer, TASK), /decision-required/);
+ assert.throws(() => b.checkPush(caps.coder, 'gitea:1'), /invalid-request/);
+ assert.deepEqual(push(A), { task_ref: TASK, commit: A });
+ const [e] = pushes();
+ assert.equal(e.subject, TASK);
+ assert.equal(e.actor_role, 'coder');
+ assert.deepEqual(JSON.parse(e.body), {
+ project: 'stack',
+ branch: 'feat',
+ ref: 'work/vikunja-1-2',
+ commit: A,
+ previous: null,
+ action: 'git.push.working',
+ class: 'within-role',
+ target: TASK,
+ });
+ // A role without the action can't have a push recorded either.
+ assert.throws(
+ () => b.recordPush(caps.reviewer, { task_ref: TASK, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit: B, previous: A }),
+ /decision-required/,
+ );
+ const good = { task_ref: TASK, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit: B, previous: A };
+ for (const bad of [{ ...good, project: 'other' }, { ...good, commit: 'x' }, { ...good, previous: 'x' }, { ...good, task_ref: 'gitea:1' }]) assert.throws(() => b.recordPush(caps.coder, bad), /invalid-request/);
+ const { previous, ...short } = good;
+ assert.throws(() => b.recordPush(caps.coder, short), /invalid-request/);
+ for (const kind of ['review.requested', 'review.verdict'])
+ assert.throws(() => b.recordEvent(caps.coder, { kind, body: { task_ref: TASK }, subject: TASK }), /reserved-event/);
+ assert.throws(
+ () => b.recordEvent(caps.coder, { kind: 'action.allowed', body: { action: 'git.push.working', target: TASK, commit: B }, subject: TASK }),
+ /reserved-event/,
+ );
+ assert.equal(pushes().length, 1);
+});
+
+test('review.request names the latest pushed commit and sends the reviewer a REVIEW-REQUEST', (t) => {
+ const { store, call, push, events } = setup(t);
+ push(A);
+ const r = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer', note: 'please' });
+ assert.equal(r.commit, A);
+ assert.equal(r.ref, 'work/vikunja-1-2');
+ const [e] = events('review.requested');
+ assert.equal(e.id, r.request);
+ assert.equal(e.subject, TASK);
+ assert.deepEqual(JSON.parse(e.body), { task_ref: TASK, project: 'stack', ref: 'work/vikunja-1-2', commit: A, to: 'reviewer', message: r.message });
+ const m = store.get('SELECT * FROM messages WHERE id=?', r.message);
+ assert.equal(m.to_role, 'reviewer');
+ assert.equal(m.from_role, 'coder');
+ assert.equal(m.class, 'REVIEW-REQUEST');
+ assert.match(m.body, new RegExp(`Review request ${r.request} for vikunja:1/2: commit ${A}`));
+ assert.match(m.body, /please$/);
+});
+
+test('review.request refuses a commit that isn\'t the latest push, a non-reviewer and a role without the action', (t) => {
+ const { call, push, events } = setup(t);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }), /unknown-commit/);
+ push(A);
+ push(B, A);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }), /stale-commit/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: 'c'.repeat(40), to: 'reviewer' }), /unknown-commit/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: 'vikunja:1/3', commit: B, to: 'reviewer' }), /unknown-commit/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'nobody' }), /unknown-role/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'pm' }), /not-a-reviewer/);
+ assert.throws(() => call('cto', 'review.request', { task_ref: TASK, commit: B, to: 'cto' }), /self-review/);
+ assert.throws(() => call('reviewer', 'review.request', { task_ref: TASK, commit: B, to: 'second' }), /decision-required/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: 'B', to: 'reviewer' }), /invalid-request/);
+ assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer', extra: 1 }), /invalid-request/);
+ assert.equal(events('review.requested').length, 0);
+ // A refusal rolls its action.allowed back, and is recorded as action.refused.
+ assert.equal(events('action.allowed').filter((e) => JSON.parse(e.body).action === 'review.request').length, 0);
+ assert.ok(events('action.refused').length >= 10);
+ // Another role that holds review.request asks too.
+ assert.equal(call('cto', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer' }).commit, B);
+});
+
+test('reviewFetch answers only the addressed reviewer, for the request it cites', (t) => {
+ const { b, caps, call, push } = setup(t);
+ push(A);
+ const { request } = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
+ assert.deepEqual(b.reviewFetch(caps.reviewer, { task_ref: TASK, request }), { project: 'stack', commit: A, ref: 'work/vikunja-1-2' });
+ assert.throws(() => b.reviewFetch(caps.coder, { task_ref: TASK, request }), /not-authorized/);
+ assert.throws(() => b.reviewFetch(caps.pm, { task_ref: TASK, request }), /not-authorized/);
+ assert.throws(() => b.reviewFetch(caps.second, { task_ref: TASK, request }), /request-not-found/);
+ assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: 'vikunja:1/3', request }), /request-not-found/);
+ assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: TASK, request: 'no-such-request' }), /request-not-found/);
+ assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: TASK }), /invalid-request/);
+});
+
+test('review.verdict answers one request once, to the role that asked', (t) => {
+ const { store, call, push, events } = setup(t);
+ push(A);
+ const req = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
+ assert.throws(() => call('coder', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'mine' }), /decision-required/);
+ assert.throws(() => call('second', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'not mine' }), /request-not-found/);
+ assert.throws(() => call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'maybe', body: 'x' }), /invalid-request/);
+ const v = call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'changes', body: 'fix the test' });
+ assert.equal(v.commit, A);
+ const [e] = events('review.verdict');
+ assert.equal(e.id, v.verdict);
+ assert.deepEqual(JSON.parse(e.body), { task_ref: TASK, request: req.request, commit: A, verdict: 'changes', message: v.message });
+ const m = store.get('SELECT * FROM messages WHERE id=?', v.message);
+ assert.equal(m.to_role, 'coder');
+ assert.equal(m.class, 'REVIEW-RESULT');
+ assert.equal(m.in_reply_to, req.message);
+ assert.match(m.body, /: changes\. A task_close cites this verdict id\.\n\nfix the test$/);
+ assert.throws(() => call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'changed my mind' }), /verdict-exists/);
+ assert.equal(events('review.verdict').length, 1);
+});
+
+test('task.close on a pushed task needs an approve verdict for the latest push', (t) => {
+ const { b, caps, call, push, events } = setup(t);
+ const close = (verdict, target = TASK) => b.authorize(caps.pm, 'task.close', { target, verdict });
+ // No push: the verdict is a citation the broker doesn't check.
+ assert.equal(close('anything').class, 'within-role');
+ push(A);
+ const allowed = () => events('action.allowed').filter((e) => JSON.parse(e.body).action === 'task.close').length;
+ const before = allowed();
+ assert.throws(() => close(undefined), /verdict-not-found/);
+ assert.throws(() => close('no-such-verdict'), /verdict-not-found/);
+ const r1 = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
+ const changes = call('reviewer', 'review.verdict', { task_ref: TASK, request: r1.request, verdict: 'changes', body: 'no' });
+ assert.throws(() => close(changes.verdict), /verdict-not-approve/);
+ // A verdict on another task doesn't count for this one.
+ push(A, null, 'vikunja:1/3');
+ const other = call('coder', 'review.request', { task_ref: 'vikunja:1/3', commit: A, to: 'reviewer' });
+ const otherOk = call('reviewer', 'review.verdict', { task_ref: 'vikunja:1/3', request: other.request, verdict: 'approve', body: 'ok' });
+ assert.throws(() => close(otherOk.verdict), /verdict-not-found/);
+ const r2 = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
+ const approve = call('reviewer', 'review.verdict', { task_ref: TASK, request: r2.request, verdict: 'approve', body: 'ok' });
+ // A push after the approval makes it stale.
+ push(B, A);
+ assert.throws(() => close(approve.verdict), /verdict-stale/);
+ assert.equal(allowed(), before, 'each refused close rolled its action.allowed back');
+ const r3 = call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer' });
+ const fresh = call('reviewer', 'review.verdict', { task_ref: TASK, request: r3.request, verdict: 'approve', body: 'ok' });
+ assert.equal(close(fresh.verdict).class, 'within-role');
+ assert.equal(allowed(), before + 1);
+});
+
+test('a refusal the host decided is filed on the task it names, and only a task ref counts', (t) => {
+ const { b, caps, events } = setup(t);
+ assert.equal(b.refuse(caps.coder, 'non-fast-forward', TASK), 'non-fast-forward');
+ assert.equal(b.refuse(caps.coder, 'invalid-request', 'gitea:1'), 'invalid-request');
+ assert.equal(b.refuse(caps.coder, 'invalid-request'), 'invalid-request');
+ assert.deepEqual(
+ events('action.refused').map((e) => [e.subject, JSON.parse(e.body).code]),
+ [[TASK, 'non-fast-forward'], [null, 'invalid-request'], [null, 'invalid-request']],
+ );
+});
diff --git a/packages/cli/README.md b/packages/cli/README.md
index 29406721b..d37f2d3f4 100644
--- a/packages/cli/README.md
+++ b/packages/cli/README.md
@@ -191,7 +191,8 @@ No capability appears in argv, stdout, the environment or a file.
uses: `bindLaunch(record)` binds a launched run's process identity
(`pid`, `startTime`) and returns `{business, run, cap}`; `op(message)` sends
one of the broker process's launch ops (`identity`, `authorizeLaunch`,
-`refuse`, `endLaunch`, `credentialStatus`); `beforeClose(fn)` runs `fn`
+`refuse`, `endLaunch`, `credentialStatus`, and for the git socket
+`checkPush`, `recordPush`, `reviewFetch`); `beforeClose(fn)` runs `fn`
before the broker closes, so the launcher stops its sessions first.
Requests to the broker process go one at a time. Each carries an id, and
the broker echoes it on the reply. If a reply doesn't arrive within 10 s, or
@@ -306,6 +307,42 @@ the recorded start time and that its command line is the runner's, then
sends SIGTERM until it exits (30 s). A stale entry prints "not running" and
exits 0; an unknown run exits 2.
+### Git socket
+
+The launcher also serves `<dataRoot>/bus-host/git.sock` (0600,
+`src/git-socket.mjs`, row 93, #1587). A coder's runner pushes work to the
+business's working repository through it, and a reviewer's runner fetches a
+commit it was asked to review (`push_working` and `fetch_review`,
+`packages/harness/README.md`). The sandbox binds it only for a role that
+holds git.push.working or review.verdict, and a session's `session.json`
+names it as `gitSocket` only then.
+
+Only a git bundle crosses. The host never runs git in a session's clone
+and never opens a path in its workspace; `packages/seat/README.md` ("The
+working repositories") has what it does with the bundle. A request is one
+JSON line of at most 4096 bytes:
+
+| Op | Request | Reply |
+|---|---|---|
+| push | `{cap, op: "push", task_ref, project, branch, size}` | `{ok: true, send: <size>}`; the runner sends exactly that many bundle bytes; then `{ok: true, result: {ref, commit, previous}}` |
+| fetch | `{cap, op: "fetch", task_ref, request, have}` | `{ok: true, result: {project, ref, commit, size}}`, then that many bytes of a bundle whose one ref is `refs/mosaic/review` |
+
+The host checks the caller's identity (a role of this business), then the
+request (task ref, project, branch name, a size of at most 32 MiB), then
+the broker: `checkPush` for a push, `reviewFetch` for a fetch. Only
+then does it read bundle bytes. The broker records a push with
+`recordPush` under the repository's lock once the ref moved; when it
+refuses, the ref goes back. A refusal is `{ok: false, error: <code>}` and
+ends the connection. The host's own refusals go through the broker's
+`refuse` with the task ref as subject, so each is an `action.refused`
+event on the task's trail. Eight connections are served at once; more
+refuse `git-busy`.
+
+A bundle file lives in `bus-host/` (0700) under a random name and is
+deleted on every path. On close the launcher drops the git socket's
+connections and waits for any push in flight to finish under its lock
+before the broker stops.
+
### Trackers
`config.trackers[<business>]` is `{baseUrl, project, pollSeconds,
diff --git a/packages/cli/src/git-socket.mjs b/packages/cli/src/git-socket.mjs
new file mode 100644
index 000000000..5ac6b208e
--- /dev/null
+++ b/packages/cli/src/git-socket.mjs
@@ -0,0 +1,262 @@
+// The host's git socket (#1587), `<dataRoot>/bus-host/git.sock` (0600). A
+// coder's runner pushes its work to the business's working repository
+// through it, and a reviewer's runner fetches a commit it was asked to
+// review. The bundle is the only thing that crosses: the host never runs git
+// in a session's clone and never opens a path in a session's workspace
+// (packages/seat/src/working.mjs says why). The sandbox binds the socket only
+// for a role that holds git.push.working or review.verdict.
+//
+// Every connection starts with one JSON line of at most 4096 bytes.
+//
+// push {"cap", "op": "push", "task_ref", "project", "branch", "size"}
+// The host answers {"ok": true, "send": <size>} or a refusal. On
+// "send" the runner writes exactly <size> bytes of a v2 git bundle,
+// and the host answers {"ok": true, "result": {ref, commit,
+// previous}} or a refusal.
+// fetch {"cap", "op": "fetch", "task_ref", "request", "have": [<oid>...]}
+// The host answers {"ok": true, "result": {project, ref, commit,
+// size}} followed by <size> bytes of a bundle whose one ref is
+// refs/mosaic/review, or a refusal.
+//
+// A refusal is {"ok": false, "error": "<code>"}, and the connection ends.
+// The host checks, in order: identity (a role of this business), the request
+// (task ref, project, branch, size, at most 32 MiB: bundle-too-large), then
+// the broker: git.push.working for a push, the review.requested for a fetch
+// (packages/bus/src/broker.mjs). Only then does it take any bundle bytes.
+// A push is recorded, as the action.allowed for git.push.working with its
+// commit, under the repository's lock once the ref moved; when the broker
+// refuses the record, the ref goes back.
+// The host's own refusals go through the broker's refuse(), so each is an
+// action.refused event on the task's trail; the broker records its own.
+//
+// A bundle file lives in bus-host/ (0700) under a random name, and the host
+// deletes it on every path.
+
+import { randomBytes } from "node:crypto";
+import { closeSync, createReadStream, existsSync, lstatSync, openSync, rmSync, statSync, writeSync, chmodSync } from "node:fs";
+import { createServer } from "node:net";
+import { join } from "node:path";
+import { pipeline } from "node:stream/promises";
+import { gitSocketPath } from "../../seat/src/session.mjs";
+import { BUNDLE_MAX, FSCK, HAVE_MAX, push, review, validBranch, workRef, WorkingError } from "../../seat/src/working.mjs";
+import { CliError } from "./errors.mjs";
+
+const HEADER_MAX = 4096;
+const HEADER_TIMEOUT_MS = 10000;
+// The longest the host waits between two chunks of a bundle.
+const BODY_IDLE_MS = 30000;
+// Connections served at once; more refuse git-busy.
+const MAX_ACTIVE = 8;
+const CAP = /^[0-9a-f]{64}$/;
+const OID = /^[0-9a-f]{40}([0-9a-f]{24})?$/;
+
+// A refusal the host decided; it goes to the broker's refuse().
+class Refusal extends Error {
+ constructor(code, message = code) {
+ super(message);
+ this.code = code;
+ }
+}
+// A refusal the broker decided and recorded itself.
+class Recorded extends Error {
+ constructor(code) {
+ super(code);
+ this.code = code;
+ }
+}
+
+// Resolves { req, rest } once a whole first line came; rest is what followed it.
+function readHeader(socket) {
+ return new Promise((resolve, reject) => {
+ let buf = Buffer.alloc(0);
+ const done = (err, value) => {
+ socket.off("data", onData);
+ socket.off("end", onGone);
+ socket.off("close", onGone);
+ socket.off("timeout", onGone);
+ socket.setTimeout(0);
+ socket.pause();
+ err ? reject(err) : resolve(value);
+ };
+ const onGone = () => done(new Refusal("invalid-request", "no request line"));
+ const onData = (chunk) => {
+ buf = Buffer.concat([buf, chunk]);
+ const nl = buf.indexOf(0x0a);
+ if (nl < 0 ? buf.length > HEADER_MAX : nl > HEADER_MAX) return done(new Refusal("invalid-request", "request line too long"));
+ if (nl < 0) return;
+ let req;
+ try {
+ req = JSON.parse(buf.subarray(0, nl).toString("utf8"));
+ } catch {
+ return done(new Refusal("invalid-request", "request line isn't JSON"));
+ }
+ done(null, { req, rest: buf.subarray(nl + 1) });
+ };
+ socket.on("data", onData);
+ socket.once("end", onGone);
+ socket.once("close", onGone);
+ socket.setTimeout(HEADER_TIMEOUT_MS);
+ socket.once("timeout", onGone);
+ });
+}
+
+// Exactly `size` bytes from the socket into a new file (0600).
+function readBody(socket, file, size) {
+ return new Promise((resolve, reject) => {
+ const fd = openSync(file, "wx", 0o600);
+ let got = 0;
+ let settled = false;
+ const done = (err) => {
+ if (settled) return;
+ settled = true;
+ socket.off("data", onData);
+ socket.off("end", onShort);
+ socket.off("close", onShort);
+ socket.off("timeout", onShort);
+ socket.setTimeout(0);
+ socket.pause();
+ closeSync(fd);
+ err ? reject(err) : resolve();
+ };
+ const onShort = () => done(new Refusal("invalid-request", `the bundle ended after ${got} of ${size} bytes`));
+ const onData = (chunk) => {
+ if (got + chunk.length > size) return done(new Refusal("invalid-request", "more bytes than the request's size"));
+ try {
+ writeSync(fd, chunk);
+ } catch (e) {
+ return done(e);
+ }
+ got += chunk.length;
+ if (got === size) done();
+ };
+ socket.on("data", onData);
+ socket.once("end", onShort);
+ socket.once("close", onShort);
+ socket.setTimeout(BODY_IDLE_MS);
+ socket.once("timeout", onShort);
+ socket.resume();
+ });
+}
+
+// host: startHost()'s handle; business: loadBusiness() output. bundleMax and
+// fsck are for the tests.
+export function createGitSocket({ host, business: b, dataRoot, env = process.env, log = () => {}, bundleMax = BUNDLE_MAX, fsck = FSCK }) {
+ const path = gitSocketPath(dataRoot);
+ const dir = join(dataRoot, "bus-host");
+ let server = null;
+ let closed = false;
+ const sockets = new Set();
+ const active = new Set();
+
+ const op = async (message) => {
+ try {
+ return await host.op(message);
+ } catch (e) {
+ throw new Recorded(e.code ?? "not-authorized");
+ }
+ };
+ const project = (id) => (typeof id === "string" && Object.hasOwn(b.projects, id) ? b.projects[id] : null);
+
+ async function doPush(socket, cap, req, rest, file) {
+ const { task_ref, branch, size } = req;
+ const p = project(req.project);
+ if (!p || !validBranch(branch) || !Number.isSafeInteger(size) || size < 1) throw new Refusal("invalid-request");
+ workRef(task_ref);
+ if (size > bundleMax) throw new Refusal("bundle-too-large", `${size} bytes, over ${bundleMax}`);
+ // The runner waits for "send"; bytes before it break the protocol.
+ if (rest.length) throw new Refusal("invalid-request", "bundle bytes before send");
+ await op({ op: "checkPush", cap, task_ref });
+ socket.write(`${JSON.stringify({ ok: true, send: size })}\n`);
+ await readBody(socket, file, size);
+ const result = await push({
+ dataRoot, business: b.id, project: req.project, root: p.root, taskRef: task_ref, branch, bundle: file, env, fsck,
+ record: (r) => op({ op: "recordPush", cap, push: { task_ref, project: req.project, branch, ...r } }),
+ });
+ socket.end(`${JSON.stringify({ ok: true, result })}\n`);
+ }
+
+ async function doFetch(socket, cap, req, file) {
+ const { task_ref, request, have = [] } = req;
+ if (typeof request !== "string" || !Array.isArray(have) || have.length > HAVE_MAX || have.some((h) => typeof h !== "string" || !OID.test(h))) throw new Refusal("invalid-request");
+ workRef(task_ref);
+ const r = await op({ op: "reviewFetch", cap, args: { task_ref, request } });
+ const p = project(r.project);
+ if (!p) throw new Refusal("unknown-project");
+ const made = await review({ dataRoot, business: b.id, project: r.project, root: p.root, taskRef: task_ref, commit: r.commit, have, out: file, env });
+ const size = statSync(file).size;
+ if (size > bundleMax) throw new Refusal("bundle-too-large", `${size} bytes, over ${bundleMax}`);
+ socket.write(`${JSON.stringify({ ok: true, result: { project: r.project, ref: made.ref, commit: made.commit, size } })}\n`);
+ await pipeline(createReadStream(file), socket);
+ }
+
+ async function serve(socket) {
+ let cap = null;
+ let subject = null;
+ let file = null;
+ const fail = (code) => {
+ if (!socket.destroyed) socket.end(`${JSON.stringify({ ok: false, error: code })}\n`);
+ };
+ try {
+ const { req, rest } = await readHeader(socket);
+ if (typeof req?.cap !== "string" || !CAP.test(req.cap)) return fail("unauthenticated");
+ if (closed) return fail("host-closing");
+ let who;
+ try {
+ who = await host.op({ op: "identity", cap: req.cap });
+ } catch (e) {
+ return fail(e.code ?? "unauthenticated");
+ }
+ cap = req.cap;
+ if (typeof req.task_ref === "string") subject = req.task_ref;
+ if (!who.role || who.business !== b.id) throw new Refusal("not-a-role");
+ file = join(dir, `${randomBytes(8).toString("hex")}.bundle`);
+ if (req.op === "push") await doPush(socket, cap, req, rest, file);
+ else if (req.op === "fetch") await doFetch(socket, cap, req, file);
+ else throw new Refusal("invalid-request", "op is push or fetch");
+ } catch (e) {
+ if (e instanceof Recorded) return fail(e.code);
+ const code = e instanceof Refusal || e instanceof WorkingError ? e.code : "git-failed";
+ if (code === "git-failed") log(`git socket: ${e.message}`);
+ if (cap) await host.op({ op: "refuse", cap, code, subject }).catch(() => {});
+ fail(code);
+ } finally {
+ if (file) rmSync(file, { force: true });
+ }
+ }
+
+ async function listen() {
+ if (existsSync(path)) {
+ if (!lstatSync(path).isSocket()) throw new CliError(`${path} exists and isn't a socket`, 3);
+ rmSync(path);
+ }
+ server = createServer((socket) => {
+ sockets.add(socket);
+ socket.once("close", () => sockets.delete(socket));
+ socket.on("error", () => {});
+ if (active.size >= MAX_ACTIVE) return socket.end(`${JSON.stringify({ ok: false, error: "git-busy" })}\n`);
+ const job = serve(socket).catch((e) => log(`git socket: ${e.message}`));
+ active.add(job);
+ job.finally(() => active.delete(job));
+ });
+ // bus-host/ is 0700, so the moment before the chmod exposes nothing.
+ await new Promise((resolve, reject) => {
+ server.once("error", reject);
+ server.listen(path, resolve);
+ });
+ chmodSync(path, 0o600);
+ }
+
+ // A push whose bundle already came finishes, under its lock, while the
+ // broker still runs: the launcher's close() runs before the host stops it.
+ async function close() {
+ closed = true;
+ if (!server) return;
+ const stopped = new Promise((r) => server.close(r));
+ for (const socket of sockets) socket.destroy();
+ await Promise.allSettled([...active]);
+ await stopped;
+ rmSync(path, { force: true });
+ }
+
+ return { path, listen, close };
+}
diff --git a/packages/cli/src/launcher.mjs b/packages/cli/src/launcher.mjs
index ab8503957..c671725c8 100644
--- a/packages/cli/src/launcher.mjs
+++ b/packages/cli/src/launcher.mjs
@@ -34,6 +34,10 @@
//
// `bus start <business> --pm` launches launch.by the same way with
// launchedBy = the business's human and no caller capability.
+//
+// listen() also serves the git socket (src/git-socket.mjs, row 93), which
+// the sandbox binds for a role that holds git.push.working or
+// review.verdict; session.json names it as gitSocket.
import { spawnSync } from "node:child_process";
import { createServer } from "node:net";
@@ -47,6 +51,7 @@ import {
endReason,
family,
hasSessionCredential,
+ gitSocketPath,
launchSocketPath,
newRun,
readSessions,
@@ -61,6 +66,7 @@ import {
} from "../../seat/src/session.mjs";
import { BWRAP, sandboxProblem } from "../../seat/src/sandbox.mjs";
import { CheckoutError, ensureCheckouts } from "../../seat/src/checkout.mjs";
+import { createGitSocket } from "./git-socket.mjs";
import { cmdlineOf, startTimeOf } from "../../seat/src/proc.mjs";
import { REPO, rolesDir } from "./config.mjs";
import { CliError } from "./errors.mjs";
@@ -95,8 +101,9 @@ export function claudeVersion(env = process.env) {
}
// host: startHost()'s handle. tracker: true when the broker has task verbs for
-// the business. adapters, bwrap, readOnly, sessionDefaults, versions and
-// stopTimeoutMs exist for the tests; the command line never sets them.
+// the business. adapters, bwrap, readOnly, sessionDefaults, versions,
+// stopTimeoutMs and git (createGitSocket options) exist for the tests; the
+// command line never sets them.
export function createLauncher({
host,
system,
@@ -108,6 +115,7 @@ export function createLauncher({
sessionDefaults = {},
versions = null,
stopTimeoutMs = STOP_TIMEOUT_MS,
+ git = {},
log = (l) => process.stderr.write(`mosaic-bus: ${l}\n`),
}) {
const dataRoot = system.dataRoot;
@@ -129,6 +137,7 @@ export function createLauncher({
let closed = false;
let server = null;
const sockets = new Set();
+ const gitSocket = createGitSocket({ host, business: b, dataRoot, env, log, ...git });
const record = (entry) => {
try {
@@ -206,6 +215,9 @@ export function createLauncher({
claudeVersion: v.claude,
});
const { manifest, ...files } = bundle.files;
+ // Row 93: the git socket, for a role that pushes work or reviews it.
+ const held = [...resolved.limits.authority.withinRole, ...resolved.limits.authority.crossRole];
+ const git = held.includes("git.push.working") || held.includes("review.verdict") ? gitSocketPath(dataRoot) : null;
writeSessionFile(runDir, {
...sessionDefaults,
business: b.id,
@@ -217,6 +229,7 @@ export function createLauncher({
model: model.model,
brokerSocket: host.path,
launchSocket: launchSocketPath(dataRoot),
+ gitSocket: git,
workspace,
sessionDir,
adapter: adapters[model.harness],
@@ -231,6 +244,7 @@ export function createLauncher({
harness: model.harness,
brokerSocket: host.path,
launchSocket: resolved.launch ? launchSocketPath(dataRoot) : null,
+ gitSocket: git,
claudeBin: env.MOSAIC_CLAUDE_BIN || null,
readOnly,
};
@@ -415,6 +429,7 @@ export function createLauncher({
server.listen(path, resolve);
});
chmodSync(path, 0o600);
+ await gitSocket.listen();
}
// SIGTERM every runner, wait, then end what's left. Runs before the host
@@ -429,6 +444,7 @@ export function createLauncher({
await stopped;
rmSync(launchSocketPath(dataRoot), { force: true });
}
+ await gitSocket.close();
await chain;
// Again every second: a runner still starting has no handler yet, and as
// pid 1 of its namespace it never sees a signal sent before it has one.
diff --git a/packages/cli/tests/git-socket.test.mjs b/packages/cli/tests/git-socket.test.mjs
new file mode 100644
index 000000000..e401ec1bf
--- /dev/null
+++ b/packages/cli/tests/git-socket.test.mjs
@@ -0,0 +1,316 @@
+// Row 93 (#1587): the host's git socket inside a real bus host, driven by the
+// runner's own git tools (packages/harness/src/git-tools.mjs) over real
+// clones. A coder pushes, asks for a review, the reviewer fetches the commit
+// and answers, all as the two roles' launches. No sandbox: the socket and the
+// tools are what is under test, and each test checks that the project root is
+// unchanged and that no bundle file is left in bus-host/ or either tmp.
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import { createHash, randomBytes } from "node:crypto";
+import { connect } from "node:net";
+import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { Client } from "../../bus/src/client.mjs";
+import { configDir, loadBusiness } from "../../business/src/index.mjs";
+import { gitEnv } from "../../seat/src/checkout.mjs";
+import { gitSocketPath } from "../../seat/src/session.mjs";
+import { workingRepo } from "../../seat/src/working.mjs";
+import { fetchReview, pushWorking } from "../../harness/src/git-tools.mjs";
+import { bootConfig, loadSystem, rolesDir } from "../src/config.mjs";
+import { createGitSocket } from "../src/git-socket.mjs";
+import { startHost, startTimeOf } from "../src/host.mjs";
+import { fixture } from "./helpers.mjs";
+
+const TASK = "vikunja:1/2";
+
+function gitIn(cwd, ...args) {
+ const r = spawnSync("git", ["-c", "user.name=test", "-c", "[email protected]", "-C", cwd, ...args], { env: gitEnv(), encoding: "utf8" });
+ assert.equal(r.status, 0, r.stderr);
+ return r.stdout.trim();
+}
+function commit(dir, text, file = "file.txt") {
+ writeFileSync(join(dir, file), `${text}\n`);
+ gitIn(dir, "add", file);
+ gitIn(dir, "commit", "-q", "-m", text);
+ return gitIn(dir, "rev-parse", "HEAD");
+}
+function fingerprint(dir) {
+ const h = createHash("sha256");
+ const walk = (d) => {
+ for (const name of readdirSync(d).sort()) {
+ const p = join(d, name);
+ const st = lstatSync(p);
+ h.update(`${p}\0${st.mode}\0`);
+ if (st.isDirectory()) walk(p);
+ else if (st.isSymbolicLink()) h.update(readlinkSync(p));
+ else if (st.isFile()) h.update(readFileSync(p));
+ }
+ };
+ walk(dir);
+ return h.digest("hex");
+}
+const bundles = (dir) => (existsSync(dir) ? readdirSync(dir).filter((n) => n.endsWith(".bundle")) : []);
+
+// A host for business acme, its git socket, and the coder, reviewer and pm
+// bound and claimed. Each role has a workspace with a clone of the project.
+async function setup(t, git = {}) {
+ // One teardown, in order: stop the socket and the host, check, then remove
+ // the tree. (t.after hooks run first-registered first.)
+ const root = mkdtempSync(join(tmpdir(), "mosaic-git-"));
+ const down = [];
+ t.after(async () => {
+ try {
+ for (const fn of down) await fn();
+ } finally {
+ rmSync(root, { recursive: true, force: true });
+ }
+ });
+ const f = fixture(root, "acme");
+ mkdirSync(f.dataRoot, { mode: 0o700 });
+ const project = f.doc.projects.stack.root;
+ gitIn(project, "init", "-q", "-b", "main");
+ const main = commit(project, "one");
+ const before = fingerprint(project);
+ const system = loadSystem({ env: f.env });
+ const host = await startHost({ boot: bootConfig({ system, businessId: "acme", env: f.env }), business: "acme", log: () => {} });
+ down.push(() => host.close(0));
+ const logs = [];
+ const sock = createGitSocket({ host, business: loadBusiness("acme", { dir: configDir(f.env), rolesDir: rolesDir(f.env) }), dataRoot: f.dataRoot, env: f.env, log: (l) => logs.push(l), ...git });
+ down.unshift(() => sock.close());
+ await sock.listen();
+ const roles = {};
+ for (const role of ["coder", "reviewer", "pm", "cto"]) {
+ const { cap } = await host.bindLaunch({ business: "acme", role, run: `${role}-test`, harness: "pi", pid: process.pid, startTime: startTimeOf(process.pid) });
+ const client = new Client({ path: host.path, cap });
+ await client.call("role.claim");
+ const workspace = join(root, "ws", role);
+ const tmpDir = join(root, "tmp", role);
+ mkdirSync(workspace, { recursive: true });
+ mkdirSync(tmpDir, { recursive: true });
+ gitIn(workspace, "clone", "-q", project, "stack");
+ roles[role] = { cap, client, workspace, tmp: tmpDir, clone: join(workspace, "stack") };
+ }
+ // Checked after every test: the root as it was, and no bundle anywhere.
+ down.push(() => {
+ assert.equal(fingerprint(project), before, "the project root is unchanged");
+ for (const dir of [join(f.dataRoot, "bus-host"), ...Object.values(roles).map((r) => r.tmp)]) assert.deepEqual(bundles(dir), [], `no bundle left in ${dir}`);
+ });
+ const push = (role, args = {}) => pushWorking({ path: sock.path, cap: roles[role].cap, workspace: roles[role].workspace, tmp: roles[role].tmp, task_ref: TASK, project: "stack", branch: "feat", ...args });
+ const fetch = (role, request, args = {}) => fetchReview({ path: sock.path, cap: roles[role].cap, workspace: roles[role].workspace, tmp: roles[role].tmp, task_ref: TASK, request, ...args });
+ const repo = workingRepo(f.dataRoot, "acme", "stack");
+ const refs = () => (existsSync(repo) ? gitIn(root, `--git-dir=${repo}`, "for-each-ref", "--format=%(refname) %(objectname)") : "");
+ const trail = async (kind, subject = TASK) => (await roles.cto.client.call("trail", { subject })).filter((r) => r.table === "events" && r.kind === kind);
+ return { root, f, project, main, host, sock, roles, push, fetch, repo, refs, trail, logs };
+}
+
+const rejects = (p, code) => assert.rejects(p, (e) => e.code === code || assert.fail(`${e.code}: ${e.message}, wanted ${code}`));
+
+// One raw exchange on the git socket.
+function raw(path, text) {
+ return new Promise((resolve, reject) => {
+ const s = connect(path);
+ let buf = "";
+ s.on("connect", () => s.write(text));
+ s.on("data", (b) => (buf += b));
+ s.on("end", () => resolve(JSON.parse(buf.split("\n")[0])));
+ s.on("error", reject);
+ });
+}
+
+test("the review loop: the coder pushes and asks, the reviewer fetches and approves, and the task's trail holds each step", async (t) => {
+ const w = await setup(t);
+ assert.equal(statSync(w.sock.path).mode & 0o777, 0o600);
+ assert.equal(w.sock.path, gitSocketPath(w.f.dataRoot));
+ const { coder, reviewer } = w.roles;
+ gitIn(coder.clone, "checkout", "-q", "-b", "feat");
+ const c1 = commit(coder.clone, "two");
+ assert.deepEqual(await w.push("coder"), { ref: "work/vikunja-1-2", commit: c1, previous: null });
+ assert.equal(gitIn(coder.clone, "rev-parse", "refs/remotes/working/work/vikunja-1-2"), c1);
+ assert.equal(w.refs(), `refs/heads/main ${w.main}\nrefs/heads/work/vikunja-1-2 ${c1}`);
+ // Nothing new: the clone remembers the push.
+ await rejects(w.push("coder"), "nothing-to-push");
+
+ const req = await coder.client.call("review.request", { task_ref: TASK, commit: c1, to: "reviewer" });
+ const got = await w.fetch("reviewer", req.request);
+ assert.deepEqual(got, { project: "stack", ref: "working/work/vikunja-1-2", commit: c1, clone: reviewer.clone });
+ assert.equal(gitIn(reviewer.clone, "rev-parse", "refs/remotes/working/work/vikunja-1-2"), c1);
+ assert.equal(gitIn(reviewer.clone, "show", "-s", "--format=%s", c1), "two");
+ const v = await reviewer.client.call("review.verdict", { task_ref: TASK, request: req.request, verdict: "changes", body: "one more" });
+ assert.equal(v.commit, c1);
+
+ // The coder answers with a fast-forward; the bundle carries only the new commit.
+ const c2 = commit(coder.clone, "three");
+ assert.deepEqual(await w.push("coder"), { ref: "work/vikunja-1-2", commit: c2, previous: c1 });
+ const req2 = await coder.client.call("review.request", { task_ref: TASK, commit: c2, to: "reviewer" });
+ assert.equal((await w.fetch("reviewer", req2.request)).commit, c2);
+ const ok = await reviewer.client.call("review.verdict", { task_ref: TASK, request: req2.request, verdict: "approve", body: "good" });
+
+ // A push is the action.allowed for git.push.working that names its commit.
+ const pushes = (await w.trail("action.allowed")).filter((e) => e.body.action === "git.push.working");
+ assert.deepEqual(pushes.map((e) => [e.actor_role, e.body.commit, e.body.previous, e.body.branch]), [["coder", c1, null, "feat"], ["coder", c2, c1, "feat"]]);
+ assert.deepEqual((await w.trail("review.requested")).map((e) => e.id), [req.request, req2.request]);
+ assert.deepEqual((await w.trail("review.verdict")).map((e) => [e.id, e.body.verdict, e.body.commit]), [[v.verdict, "changes", c1], [ok.verdict, "approve", c2]]);
+ assert.deepEqual(w.logs, []);
+});
+
+test("refusals: no action, non-fast-forward, an unknown commit or request, and a push for another task moves only that task's ref", async (t) => {
+ const w = await setup(t);
+ const { coder, reviewer, pm } = w.roles;
+ gitIn(coder.clone, "checkout", "-q", "-b", "feat");
+ const c1 = commit(coder.clone, "two");
+ await w.push("coder");
+ const refs = w.refs();
+
+ // The reviewer and pm hold no git.push.working; the broker refuses before the host takes a byte.
+ for (const role of ["reviewer", "pm"]) {
+ gitIn(w.roles[role].clone, "checkout", "-q", "-b", "feat");
+ commit(w.roles[role].clone, `${role} work`);
+ await rejects(w.push(role), "decision-required");
+ }
+ assert.equal(w.refs(), refs);
+
+ // A rewritten branch.
+ gitIn(coder.clone, "update-ref", "-d", "refs/remotes/working/work/vikunja-1-2");
+ gitIn(coder.clone, "reset", "-q", "--hard", w.main);
+ commit(coder.clone, "other two");
+ await rejects(w.push("coder"), "non-fast-forward");
+ assert.equal(w.refs(), refs);
+
+ // The same branch for another task lands on that task's ref and leaves this one.
+ const other = await w.push("coder", { task_ref: "vikunja:1/3" });
+ assert.equal(other.ref, "work/vikunja-1-3");
+ assert.match(w.refs(), new RegExp(`refs/heads/work/vikunja-1-2 ${c1}`));
+
+ await rejects(coder.client.call("review.request", { task_ref: TASK, commit: "e".repeat(40), to: "reviewer" }), "unknown-commit");
+ const req = await coder.client.call("review.request", { task_ref: TASK, commit: c1, to: "reviewer" });
+ await rejects(w.fetch("reviewer", "no-such-request"), "request-not-found");
+ await rejects(w.fetch("reviewer", req.request, { task_ref: "vikunja:1/3" }), "request-not-found");
+ // The coder and pm hold no review.verdict.
+ await rejects(w.fetch("coder", req.request), "not-authorized");
+ await rejects(w.fetch("pm", req.request), "not-authorized");
+ assert.equal(existsSync(join(reviewer.clone, ".git", "refs", "remotes", "working")), false);
+
+ // Each refusal is an action.refused event on the task, from the role that tried.
+ const refused = (await w.trail("action.refused")).map((e) => `${e.actor_role} ${e.body.code}`);
+ for (const r of ["reviewer decision-required", "pm decision-required", "coder non-fast-forward", "coder unknown-commit", "reviewer request-not-found", "coder not-authorized", "pm not-authorized"])
+ assert.ok(refused.includes(r), `${r} in ${refused.join(", ")}`);
+});
+
+test("the size cap refuses a push before any byte", async (t) => {
+ const w = await setup(t, { bundleMax: 64 });
+ gitIn(w.roles.coder.clone, "checkout", "-q", "-b", "feat");
+ commit(w.roles.coder.clone, "two");
+ await rejects(w.push("coder"), "bundle-too-large");
+ assert.equal(w.refs(), "");
+ assert.deepEqual((await w.trail("action.allowed")).filter((e) => e.body.action === "git.push.working"), []);
+});
+
+test("a fetch over the cap refuses before any byte reaches the reviewer", async (t) => {
+ const w = await setup(t, { bundleMax: 1024 * 1024 });
+ const { coder } = w.roles;
+ gitIn(coder.clone, "checkout", "-q", "-b", "feat");
+ // Two pushes of 600 KiB of noise each pass a 1 MiB cap; the reviewer has
+ // neither, so its bundle carries both and is over.
+ writeFileSync(join(coder.clone, "big"), randomBytes(600 * 1024));
+ gitIn(coder.clone, "add", "big");
+ gitIn(coder.clone, "commit", "-q", "-m", "big");
+ const c = (await w.push("coder")).commit;
+ writeFileSync(join(coder.clone, "big"), randomBytes(600 * 1024));
+ gitIn(coder.clone, "commit", "-q", "-am", "bigger");
+ const c2 = (await w.push("coder")).commit;
+ assert.notEqual(c, c2);
+ const req = await coder.client.call("review.request", { task_ref: TASK, commit: c2, to: "reviewer" });
+ await rejects(w.fetch("reviewer", req.request), "bundle-too-large");
+ assert.equal(existsSync(join(w.roles.reviewer.clone, ".git", "refs", "remotes", "working")), false);
+});
+
+test("the socket refuses a request with no capability, an unknown one, and a bad request line", async (t) => {
+ const w = await setup(t);
+ assert.deepEqual(await raw(w.sock.path, "not json\n"), { ok: false, error: "invalid-request" });
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ op: "push" })}\n`), { ok: false, error: "unauthenticated" });
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap: "f".repeat(64), op: "push" })}\n`), { ok: false, error: "unauthenticated" });
+ const cap = w.roles.coder.cap;
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap, op: "merge" })}\n`), { ok: false, error: "invalid-request" });
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap, op: "push", task_ref: TASK, project: "nope", branch: "feat", size: 10 })}\n`), { ok: false, error: "invalid-request" });
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap, op: "push", task_ref: "gitea:1", project: "stack", branch: "feat", size: 10 })}\n`), { ok: false, error: "invalid-request" });
+ // Bytes before "send" are refused, not read.
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap, op: "push", task_ref: TASK, project: "stack", branch: "feat", size: 10 })}\n0123456789`), { ok: false, error: "invalid-request" });
+ assert.deepEqual(await raw(w.sock.path, `${JSON.stringify({ cap, op: "fetch", task_ref: TASK, request: "x", have: ["nope"] })}\n`), { ok: false, error: "invalid-request" });
+ // A push whose bytes have a bundle's header and then no ref line git can read.
+ const junk = await new Promise((resolve, reject) => {
+ const s = connect(w.sock.path);
+ let buf = "";
+ s.on("connect", () => s.write(`${JSON.stringify({ cap, op: "push", task_ref: TASK, project: "stack", branch: "feat", size: 20 })}\n`));
+ s.on("data", (b) => {
+ buf += b;
+ if (buf === `${JSON.stringify({ ok: true, send: 20 })}\n`) s.write("# v2 git bundle\nzzz\n");
+ });
+ s.on("end", () => resolve(buf.trim().split("\n").pop()));
+ s.on("error", reject);
+ });
+ assert.deepEqual(JSON.parse(junk), { ok: false, error: "invalid-bundle" });
+ assert.equal(w.refs(), `refs/heads/main ${w.main}`);
+});
+
+test("a rigged clone runs nothing: hooks, fsmonitor, a filter, a gitlink and a symlinked .git/config leave no marker", async (t) => {
+ const w = await setup(t);
+ const { coder, reviewer } = w.roles;
+ const marks = join(w.root, "marks");
+ mkdirSync(marks);
+ gitIn(coder.clone, "checkout", "-q", "-b", "feat");
+ commit(coder.clone, "two");
+ // A gitlink whose .gitmodules would run a command if anything cloned it.
+ // (An `update = !cmd` line fsck refuses outright; the end of the test pushes one.)
+ writeFileSync(join(coder.clone, ".gitmodules"), `[submodule "sub"]\n\tpath = sub\n\turl = ext::sh -c touch% ${marks}/submodule\n`);
+ gitIn(coder.clone, "add", ".gitmodules");
+ gitIn(coder.clone, "update-index", "--add", "--cacheinfo", `160000,${w.main},sub`);
+ gitIn(coder.clone, "commit", "-q", "-m", "gitlink");
+ writeFileSync(join(coder.clone, ".gitattributes"), "* filter=evil\n");
+ gitIn(coder.clone, "add", ".gitattributes");
+ gitIn(coder.clone, "commit", "-q", "-m", "attributes");
+ const tipCommit = gitIn(coder.clone, "rev-parse", "HEAD");
+ // On a second branch, a .gitmodules with a command as its update setting.
+ gitIn(coder.clone, "checkout", "-q", "-b", "bad");
+ writeFileSync(join(coder.clone, ".gitmodules"), `[submodule "sub"]\n\tpath = sub\n\turl = ./sub\n\tupdate = !touch ${marks}/update\n`);
+ gitIn(coder.clone, "add", ".gitmodules");
+ gitIn(coder.clone, "commit", "-q", "-m", "update command");
+ gitIn(coder.clone, "checkout", "-q", "feat");
+ // Rigged only now, so the test's own git calls above ran none of it.
+ const rig = (clone, name) => {
+ const hooks = join(w.root, `hooks-${name}`);
+ mkdirSync(hooks);
+ for (const hook of ["reference-transaction", "pre-push", "post-checkout", "post-merge", "post-rewrite", "pre-auto-gc", "fsmonitor-watchman"]) {
+ writeFileSync(join(hooks, hook), `#!/bin/sh\ntouch '${marks}/${name}-${hook}'\nexit 0\n`);
+ chmodSync(join(hooks, hook), 0o755);
+ mkdirSync(join(clone, ".git", "hooks"), { recursive: true });
+ writeFileSync(join(clone, ".git", "hooks", hook), `#!/bin/sh\ntouch '${marks}/${name}-local-${hook}'\nexit 0\n`);
+ chmodSync(join(clone, ".git", "hooks", hook), 0o755);
+ }
+ const config = join(w.root, `config-${name}`);
+ writeFileSync(
+ config,
+ `${readFileSync(join(clone, ".git", "config"), "utf8")}[core]\n\thooksPath = ${hooks}\n\tfsmonitor = ${hooks}/fsmonitor-watchman\n[filter "evil"]\n\tclean = touch ${marks}/${name}-clean\n\tsmudge = touch ${marks}/${name}-smudge\n\tprocess = touch ${marks}/${name}-process\n[uploadpack]\n\tpackObjectsHook = touch ${marks}/${name}-pack\n[gc]\n\tauto = 1\n`,
+ );
+ renameSync(join(clone, ".git", "config"), join(w.root, `config-${name}.orig`));
+ symlinkSync(config, join(clone, ".git", "config"));
+ };
+ rig(coder.clone, "coder");
+ rig(reviewer.clone, "reviewer");
+ const pushed = await w.push("coder");
+ assert.equal(pushed.commit, tipCommit);
+ const req = await coder.client.call("review.request", { task_ref: TASK, commit: pushed.commit, to: "reviewer" });
+ assert.equal((await w.fetch("reviewer", req.request)).commit, pushed.commit);
+ assert.deepEqual(readdirSync(marks), []);
+ // The gitlink came across as a tree entry and nothing more.
+ assert.match(gitIn(reviewer.clone, "-c", "core.hooksPath=/dev/null", "ls-tree", pushed.commit, "sub"), /^160000 commit /);
+ assert.equal(existsSync(join(reviewer.clone, "sub", ".git")), false);
+ assert.deepEqual(readdirSync(marks), []);
+ // The update command: the host's fsck refuses the bundle and the work ref stays.
+ const landed = w.refs();
+ await rejects(w.push("coder", { branch: "bad" }), "invalid-bundle");
+ assert.equal(w.refs(), landed);
+ assert.deepEqual(readdirSync(marks), []);
+});
diff --git a/packages/cli/tests/launcher.test.mjs b/packages/cli/tests/launcher.test.mjs
index d86dbb64e..14e996dfe 100644
--- a/packages/cli/tests/launcher.test.mjs
+++ b/packages/cli/tests/launcher.test.mjs
@@ -13,7 +13,7 @@ import { chmodSync, existsSync, mkdirSync, readFileSync, readdirSync, renameSync
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import { Client } from "../../bus/src/client.mjs";
-import { launchLogFile, launchSocketPath, readSessions, Registry, SESSION_CREDENTIALS, sessionsFile, workspaceDir } from "../../seat/src/session.mjs";
+import { gitSocketPath, launchLogFile, launchSocketPath, readSessions, Registry, SESSION_CREDENTIALS, sessionsFile, workspaceDir } from "../../seat/src/session.mjs";
import { sandboxProblem } from "../../seat/src/sandbox.mjs";
import { gitEnv } from "../../seat/src/checkout.mjs";
import { bootConfig, loadSystem } from "../src/config.mjs";
@@ -150,6 +150,9 @@ test("bus start --pm: the PM runs under its own PID namespace, registered, with
assert.equal(session.launchedBy, "jason");
assert.equal(session.brokerSocket, host.path);
assert.equal(session.launchSocket, sock);
+ // Row 93: the PM neither pushes nor reviews, so no git socket; the launcher serves one.
+ assert.equal(session.gitSocket, null);
+ assert.equal(statSync(gitSocketPath(f.dataRoot)).mode & 0o777, 0o600);
const manifest = JSON.parse(readFileSync(join(runDir, "bundle", "manifest.json"), "utf8"));
assert.equal(manifest.piVersion, "0.85.1");
assert.equal(manifest.claudeVersion, null);
@@ -218,6 +221,11 @@ test("the PM launches a coder through its launch tool; the coder answers; refusa
const bound = (pid) => readFileSync(`/proc/${pid}/cmdline`, "utf8").split("\0").includes(sock);
assert.equal(bound(readSessions(f.dataRoot).find((s) => s.instance === "pm").pid), true);
assert.equal(bound(coder.pid), false);
+ // Row 93: and only a role that pushes or reviews has the git socket.
+ const gitBound = (pid) => readFileSync(`/proc/${pid}/cmdline`, "utf8").split("\0").includes(gitSocketPath(f.dataRoot));
+ assert.equal(gitBound(coder.pid), true);
+ assert.equal(gitBound(readSessions(f.dataRoot).find((s) => s.instance === "pm").pid), false);
+ assert.equal(JSON.parse(readFileSync(join(coder.runDir, "session.json"), "utf8")).gitSocket, gitSocketPath(f.dataRoot));
// The coder claimed its role and answers its own messages.
const hello = await client.call("message.send", { to: "coder", class: "REQUEST", body: "hello" });
diff --git a/packages/harness/README.md b/packages/harness/README.md
index 7c040c660..6f87ac731 100644
--- a/packages/harness/README.md
+++ b/packages/harness/README.md
@@ -88,6 +88,32 @@ file outside the workspace, which the gate refuses to read (row 89, #1583;
tasks, 50 at a time, without descriptions. `get_task` takes a `task_ref`
and returns that task in full. `packages/bus/README.md` has the reply.
+The review loop (row 93, #1587) adds four tools:
+
+- `push_working` (git.push.working holders with a clone) pushes a local
+ branch of one clone to the task's work branch,
+ `work/vikunja-<project>-<task>`, in the business's working repository.
+ The host takes only a fast-forward of what the task pushed before. The
+ reply is `{ref, commit, previous}`.
+- `request_review` (review.request) names that commit and a reviewer role.
+ The broker refuses a commit that isn't the task's latest push.
+- `fetch_review` (review.verdict holders with a clone) fetches the commit
+ a review request names into the clone of its project, as
+ `refs/remotes/working/<work ref>`. Nothing is merged or checked out.
+- `review_verdict` (review.verdict) records `approve` or `changes`, one per
+ request.
+
+`request_review` and `review_verdict` are broker verbs. `push_working`
+(verb `working.push`) and `fetch_review` (verb `working.fetch`) are not:
+the runner answers them itself through `git-tools.mjs`, which moves the
+commits as a git bundle over the host's git socket
+(`packages/cli/src/git-socket.mjs`). git there runs with no system or
+global config and no hooks, the fetch has fsck on, and the bundle file in
+the run's `/tmp` is deleted on every path. A session without a clone (no
+`projects` in `typedTools`) gets neither of the two. `task_close` cites the
+verdict id `review_verdict` returned; for a task with pushed work the
+broker takes only an `approve` verdict for the latest push.
+
## The gate
`gate.mjs` `decide(policy, tool, input)` answers one tool call: the
@@ -145,7 +171,9 @@ it never touches the disk and isn't in the environment or argv.
and the rest of `FOUNDER_ENV`), or a service the role needs with no
usable role token, exits 20 before the role is claimed.
2. `role.claim`.
-3. The tool socket serves the typed tools.
+3. The tool socket serves the typed tools. `launch` goes to the host's
+ launch socket, `push_working` and `fetch_review` to its git socket, and
+ every other tool to the broker.
4. One harness turn per received message, through the adapter
(`/bin/sh <adapter.sh>`: the repository keeps adapters 0644 and only the
image sets the mode), in its own process group, under the wall clock
@@ -202,7 +230,17 @@ These are limits, not bugs, and nothing in this package claims otherwise.
connect to it; it still needs a capability to do anything there. The
tool socket is in the run directory and acts with the session's own
capability only. The launch socket is bound in only for the role whose
- launch tool calls it.
+ launch tool calls it. The git socket is bound in only for a role that
+ holds git.push.working or review.verdict, and every request on it needs
+ a capability the broker then checks.
+- **Git tool timeouts disagree.** git in `git-tools.mjs` may run for 300 s,
+ but the tool call itself (`callTool`) gives up after 60 s and the tool
+ socket after 150 s. A push or fetch slower than 60 s reaches the model
+ as a timeout while the runner still finishes it. When it landed, the
+ clone's `refs/remotes/working` already has the commit, so pushing again
+ refuses `nothing-to-push`; `trail` on the task shows the
+ push, an `action.allowed` for `git.push.working`. Bundles are capped at 32 MiB, which keeps this
+ rare.
- **PID namespace.** Each session's runner is pid 1 of its own namespace
(bwrap `--unshare-pid --as-pid-1`), with its own IPC namespace. A
session sees no other process, so it can't signal or `ptrace` one by
diff --git a/packages/harness/src/bundle.mjs b/packages/harness/src/bundle.mjs
index 678c8749f..728eef797 100644
--- a/packages/harness/src/bundle.mjs
+++ b/packages/harness/src/bundle.mjs
@@ -80,6 +80,12 @@ function prompt({ contract, resolved, session, tools, untyped }) {
// Row 90 (#1584): PM fault 10 (#228 c91).
"- Only the human stops a run, with `mosaic stop`. If you think a run should stop, raise a role.revoke decision with blocking false and the role instance as target, naming the run and saying why.",
];
+ // Row 93 (#1587): the review loop, for the roles that have its tools.
+ const named = new Set(tools.map((t) => t.name));
+ if (named.has("push_working"))
+ lines.push("- Review loop: commit on a branch of your clone, push_working it, then request_review with the commit it returned. Work that isn't pushed can't be reviewed, and a task with pushed work closes only on an approve verdict for its latest push. After a changes verdict, commit the fixes, push again and ask again.");
+ if (named.has("fetch_review"))
+ lines.push("- Reviews: a REVIEW-REQUEST names a request id. fetch_review it, read the commit in your clone, and answer with review_verdict citing the request. One verdict per request.");
if (untyped.length) lines.push(`- Actions you hold that have no bus tool yet: ${untyped.join(", ")}. Say so instead of working around it.`);
return lines.join("\n") + "\n";
}
@@ -99,7 +105,7 @@ function prompt({ contract, resolved, session, tools, untyped }) {
// Returns { manifest, files } with absolute paths.
export function buildBundle({ dir, resolved, contract, arbiter = false, tracker = false, credentials = [], session, piVersion = null, claudeVersion = null }) {
const model = sessionModel(resolved);
- const tools = typedTools({ resolved, arbiter, tracker });
+ const tools = typedTools({ resolved, arbiter, tracker, projects: (session.checkouts ?? []).map((c) => c.project) });
const untyped = untypedActions(resolved, tools);
mkdirSync(dir, { recursive: true, mode: 0o700 });
const write = (name, text) => {
diff --git a/packages/harness/src/git-tools.mjs b/packages/harness/src/git-tools.mjs
new file mode 100644
index 000000000..57b0bfa7e
--- /dev/null
+++ b/packages/harness/src/git-tools.mjs
@@ -0,0 +1,209 @@
+// The session side of the host's git socket (#1587,
+// packages/cli/src/git-socket.mjs). The runner calls these for the
+// push_working and fetch_review tools, inside the sandbox, in the session's
+// own clones under its workspace. Commits leave and arrive only as bundles:
+//
+// push `git bundle create` of the named branch, leaving out what the
+// clone's origin and working remotes already have, written to the
+// run's /tmp; the host takes it once the broker authorized the push.
+// fetch the host sends a bundle of the commit the review request names;
+// the runner fetches it with fsck on into
+// refs/remotes/working/<work ref> of that project's clone.
+//
+// The bundle files are deleted on every path. git runs with no system or
+// global config and no hooks, as the host's does.
+
+import { spawn } from "node:child_process";
+import { randomBytes } from "node:crypto";
+import { closeSync, createReadStream, existsSync, openSync, readdirSync, rmSync, statSync, writeSync } from "node:fs";
+import { connect } from "node:net";
+import { join } from "node:path";
+import { once } from "node:events";
+
+// The host's cap (BUNDLE_MAX and HAVE_MAX in packages/seat/src/working.mjs;
+// a test holds them equal). Kept here so the runner doesn't load the host's module.
+export const BUNDLE_MAX = 32 * 1024 * 1024;
+export const HAVE_MAX = 32;
+const PROJECT = /^[a-z0-9][a-z0-9._-]{0,63}$/;
+const WORK_REF = /^work\/vikunja-[1-9][0-9]*-[1-9][0-9]*$/;
+const OID = /^[0-9a-f]{40}([0-9a-f]{24})?$/;
+const TIMEOUT_MS = 300_000;
+const SAFE = [
+ "-c", "core.hooksPath=/dev/null",
+ "-c", "core.fsmonitor=false",
+ "-c", "protocol.allow=never",
+ "-c", "protocol.file.allow=always",
+ "-c", "submodule.recurse=false",
+];
+
+const failure = (code, message = code) => Object.assign(new Error(message), { code });
+
+// The same rule as validBranch in packages/seat/src/working.mjs.
+export const validBranch = (branch) =>
+ typeof branch === "string" && /^[A-Za-z0-9][A-Za-z0-9._\/-]{0,199}$/.test(branch) && !/\.\.|\/\/|\/\.|@\{|\.lock(\/|$)|[./]$/.test(branch);
+
+function git(args, env = process.env) {
+ return new Promise((resolve) => {
+ const child = spawn("git", [...SAFE, ...args], {
+ env: { PATH: env.PATH ?? "/usr/bin:/bin", HOME: "/nonexistent", GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: "/dev/null", GIT_TERMINAL_PROMPT: "0", LC_ALL: "C" },
+ timeout: TIMEOUT_MS,
+ killSignal: "SIGKILL",
+ stdio: ["ignore", "pipe", "pipe"],
+ });
+ let out = "";
+ let err = "";
+ child.stdout.setEncoding("utf8").on("data", (d) => (out += d));
+ child.stderr.setEncoding("utf8").on("data", (d) => (err += d));
+ child.on("error", (e) => resolve({ ok: false, out, err, why: e.code ?? e.message }));
+ child.on("close", (code, signal) => resolve({ ok: code === 0, out, err, why: signal ? `killed by ${signal}` : err.trim().split("\n").pop() || `exit ${code}` }));
+ });
+}
+
+function clone(workspace, project) {
+ if (!PROJECT.test(project ?? "")) throw failure("invalid-request", "not a project id");
+ const dir = join(workspace, project);
+ if (!existsSync(join(dir, ".git"))) throw failure("no-clone", `no clone of ${project} in the workspace`);
+ return dir;
+}
+
+// One connection to the git socket: JSON lines and raw bytes, read in order.
+function open(path) {
+ if (typeof path !== "string" || !path) throw failure("not-authorized", "this session has no git socket");
+ const socket = connect(path);
+ let buf = Buffer.alloc(0);
+ let gone = null;
+ let wake = null;
+ const poke = () => wake?.();
+ socket.on("data", (c) => {
+ buf = buf.length ? Buffer.concat([buf, c]) : c;
+ poke();
+ });
+ socket.on("end", () => {
+ gone ??= "the host closed the connection";
+ poke();
+ });
+ socket.on("error", (e) => {
+ gone ??= `git socket: ${e.code ?? e.message}`;
+ poke();
+ });
+ socket.setTimeout(TIMEOUT_MS, () => {
+ gone ??= "git socket timed out";
+ socket.destroy();
+ poke();
+ });
+ const wait = () => new Promise((r) => (wake = () => ((wake = null), r())));
+ return {
+ socket,
+ async line() {
+ for (;;) {
+ const nl = buf.indexOf(0x0a);
+ if (nl >= 0) {
+ const text = buf.subarray(0, nl).toString("utf8");
+ buf = buf.subarray(nl + 1);
+ try {
+ return JSON.parse(text);
+ } catch {
+ throw failure("invalid-response", "the host's reply isn't JSON");
+ }
+ }
+ if (buf.length > 65536) throw failure("invalid-response", "the host's reply line is too long");
+ if (gone) throw failure("outcome-unknown", gone);
+ await wait();
+ }
+ },
+ async bytes(size, fd) {
+ let got = 0;
+ for (;;) {
+ if (buf.length) {
+ const take = buf.subarray(0, size - got);
+ writeSync(fd, take);
+ got += take.length;
+ buf = buf.subarray(take.length);
+ }
+ if (got === size) return;
+ if (gone) throw failure("outcome-unknown", gone);
+ await wait();
+ }
+ },
+ close: () => socket.destroy(),
+ };
+}
+
+const refused = (r) => {
+ if (r?.ok === false) throw failure(/^[a-z-]{1,64}$/.test(r.error ?? "") ? r.error : "refused");
+ if (r?.ok !== true) throw failure("invalid-response");
+ return r;
+};
+
+// await pushWorking({ path, cap, workspace, task_ref, project, branch, tmp })
+// Resolves the host's { ref, commit, previous }.
+export async function pushWorking({ path, cap, workspace, task_ref, project, branch, tmp = "/tmp", env = process.env }) {
+ const dir = clone(workspace, project);
+ if (!validBranch(branch)) throw failure("invalid-request", "not a branch name");
+ if (!(await git(["-C", dir, "rev-parse", "-q", "--verify", `refs/heads/${branch}^{commit}`], env)).ok) throw failure("no-such-branch", `no branch ${branch} in the ${project} clone`);
+ const file = join(tmp, `push-${randomBytes(8).toString("hex")}.bundle`);
+ try {
+ const made = await git(["-C", dir, "bundle", "create", "-q", file, `refs/heads/${branch}`, "--not", "--remotes=origin", "--remotes=working"], env);
+ if (!made.ok) throw failure(/empty bundle/.test(made.err) ? "nothing-to-push" : "bundle-failed", made.why);
+ const size = statSync(file).size;
+ if (size > BUNDLE_MAX) throw failure("bundle-too-large", `${size} bytes, over ${BUNDLE_MAX}`);
+ const conn = open(path);
+ try {
+ conn.socket.write(`${JSON.stringify({ cap, op: "push", task_ref, project, branch, size })}\n`);
+ refused(await conn.line());
+ for await (const chunk of createReadStream(file)) if (!conn.socket.write(chunk)) await once(conn.socket, "drain");
+ const result = refused(await conn.line()).result;
+ // The clone remembers what the working repository has, so the next
+ // push sends only what is new. Best effort: the push already landed.
+ if (WORK_REF.test(result?.ref ?? "") && OID.test(result.commit ?? "")) await git(["-C", dir, "update-ref", `refs/remotes/working/${result.ref}`, result.commit], env);
+ return result;
+ } finally {
+ conn.close();
+ }
+ } finally {
+ rmSync(file, { force: true });
+ }
+}
+
+// await fetchReview({ path, cap, workspace, task_ref, request, tmp })
+// Resolves { project, ref, commit, clone }; ref is the clone's remote ref.
+export async function fetchReview({ path, cap, workspace, task_ref, request, tmp = "/tmp", env = process.env }) {
+ // Commits the host may leave out: what the clones' remotes already hold.
+ const have = new Set();
+ for (const project of projectsIn(workspace)) {
+ const r = await git(["-C", join(workspace, project), "for-each-ref", "--format=%(objectname)", "refs/remotes/origin", "refs/remotes/working"], env);
+ if (r.ok) for (const oid of r.out.split("\n")) if (OID.test(oid) && have.size < HAVE_MAX) have.add(oid);
+ }
+ const file = join(tmp, `fetch-${randomBytes(8).toString("hex")}.bundle`);
+ try {
+ const conn = open(path);
+ let result;
+ try {
+ conn.socket.write(`${JSON.stringify({ cap, op: "fetch", task_ref, request, have: [...have] })}\n`);
+ result = refused(await conn.line()).result;
+ if (!PROJECT.test(result?.project ?? "") || !WORK_REF.test(result.ref ?? "") || !OID.test(result.commit ?? "") || !Number.isSafeInteger(result.size) || result.size < 1 || result.size > BUNDLE_MAX)
+ throw failure("invalid-response");
+ const fd = openSync(file, "wx", 0o600);
+ try {
+ await conn.bytes(result.size, fd);
+ } finally {
+ closeSync(fd);
+ }
+ } finally {
+ conn.close();
+ }
+ const dir = clone(workspace, result.project);
+ const into = `refs/remotes/working/${result.ref}`;
+ const fetched = await git(["-c", "transfer.fsckObjects=true", "-C", dir, "fetch", "-q", "--no-tags", "--no-write-fetch-head", "--no-recurse-submodules", "--no-auto-maintenance", file, `+refs/mosaic/review:${into}`], env);
+ if (!fetched.ok) throw failure("fetch-failed", fetched.why);
+ const tip = await git(["-C", dir, "rev-parse", "-q", "--verify", `${into}^{commit}`], env);
+ if (tip.out.trim() !== result.commit) throw failure("fetch-failed", "the fetched commit isn't the one the host named");
+ return { project: result.project, ref: `working/${result.ref}`, commit: result.commit, clone: dir };
+ } finally {
+ rmSync(file, { force: true });
+ }
+}
+
+// The project clones in a workspace: each directory with a project id's
+// name and a .git.
+const projectsIn = (workspace) => readdirSync(workspace).filter((name) => PROJECT.test(name) && existsSync(join(workspace, name, ".git")));
diff --git a/packages/harness/src/runner.mjs b/packages/harness/src/runner.mjs
index c8943244b..00595542b 100644
--- a/packages/harness/src/runner.mjs
+++ b/packages/harness/src/runner.mjs
@@ -9,8 +9,9 @@
// 2. role.claim; a refusal exits 21.
// 3. The tool socket (0600) serves the harness's typed tools: a name in
// tools.json is forwarded with the runner's capability, to the broker
-// or, for `launch`, to the host's launch socket. The harness never sees
-// the capability.
+// or, for `launch`, to the host's launch socket, or, for push_working
+// and fetch_review, to the host's git socket (git-tools.mjs). The
+// harness never sees the capability.
// 4. One harness turn per received message, through the adapter, in its
// own process group under a wall clock (S0 line 4). The turn's stdout
// goes back to the sender as a RESULT, except for an incoming RESULT,
@@ -30,6 +31,7 @@ import { pathToFileURL } from "node:url";
import { Client } from "../../bus/src/client.mjs";
import { SOURCES } from "./bundle.mjs";
import { claudeBuiltins } from "./gate.mjs";
+import { fetchReview, pushWorking } from "./git-tools.mjs";
export const EXIT = Object.freeze({ stopped: 0, founder: 20, claim: 21, session: 22, broker: 23, usage: 2 });
@@ -112,8 +114,10 @@ function launchCall(path, request, timeout = 120_000) {
});
}
-// Serve the typed tools on `path`. Returns the server.
-function toolServer({ path, tools, client, launch }) {
+// Serve the typed tools on `path`. Returns the server. `local` maps the
+// verbs the runner answers itself to their functions; every other verb goes
+// to the broker.
+function toolServer({ path, tools, client, local }) {
const byName = new Map(tools.map((t) => [t.name, t]));
rmSync(path, { force: true });
const server = createServer((socket) => {
@@ -135,7 +139,7 @@ function toolServer({ path, tools, client, launch }) {
if (!tool) return answer({ ok: false, error: "unknown-tool" });
const args = request.args && typeof request.args === "object" && !Array.isArray(request.args) ? request.args : {};
try {
- const result = tool.verb === "launch" ? await launch(args) : await client.call(tool.verb, args);
+ const result = Object.hasOwn(local, tool.verb) ? await local[tool.verb](args) : await client.call(tool.verb, args);
answer({ ok: true, result });
} catch (e) {
answer({ ok: false, error: /^[a-z-]{1,64}$/.test(e.code ?? "") ? e.code : "tool-failed" });
@@ -296,7 +300,11 @@ export async function main(runDir, { stdin = process.stdin, env = process.env, l
path: session.toolSocket,
tools,
client,
- launch: (args) => launchCall(session.launchSocket, { cap, instance: args.instance }),
+ local: {
+ launch: (args) => launchCall(session.launchSocket, { cap, instance: args.instance }),
+ "working.push": (args) => pushWorking({ path: session.gitSocket, cap, workspace: session.workspace, task_ref: args.task_ref, project: args.project, branch: args.branch }),
+ "working.fetch": (args) => fetchReview({ path: session.gitSocket, cap, workspace: session.workspace, task_ref: args.task_ref, request: args.request }),
+ },
});
const turnEnv = adapterEnv(session, env);
diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs
index a568a8082..5f27a6bf9 100644
--- a/packages/harness/src/tools.mjs
+++ b/packages/harness/src/tools.mjs
@@ -1,6 +1,7 @@
// Typed tools: the only way a managed session acts on the bus. Each tool
// maps one-to-one onto a broker verb (or, for `launch`, onto the host's
-// launch socket), and a session gets a tool only for an action its resolved
+// launch socket, and for push_working and fetch_review, onto the host's git
+// socket through git-tools.mjs), and a session gets a tool only for an action its resolved
// instance holds within-role or cross-role, plus the reads and
// raise_decision every agent has. The broker still decides every call;
// a tool's presence is not permission.
@@ -64,18 +65,20 @@ const TASK_TOOLS = {
state: str("Board state", { enum: ["todo", "in-progress", "in-review", "blocked"] }),
comment: str("Comment to add"),
}, ["task_ref"]],
- "task.close": ["task_close", "Close a task after its review verdict.", { ...COMMON, verdict: str("Citation of the review verdict") }, ["task_ref", "verdict"]],
+ "task.close": ["task_close", "Close a task after its review verdict. A task with pushed work closes only on an approve verdict for its latest push.", { ...COMMON, verdict: str("The review verdict id review_verdict returned, as the REVIEW-RESULT message names it") }, ["task_ref", "verdict"]],
};
const MESSAGE_CLASSES = ["REQUEST", "ASSIGNMENT", "REVIEW-REQUEST", "REVIEW-RESULT", "RESULT", "INFO", "DECISION", "REACTION"];
-// typedTools({ resolved, arbiter, tracker }) -> [{ name, verb, action, description, parameters }]
+// typedTools({ resolved, arbiter, tracker, projects }) -> [{ name, verb, action, description, parameters }]
// resolved resolveInstance(...) output for the session's instance
// arbiter true when the business names this instance as an arbiter
// tracker true when the broker has task verbs for the business
+// projects the project ids the session has a clone of
// `verb` is the broker verb; the `launch` tool has verb "launch" and goes
-// to the host instead.
-export function typedTools({ resolved, arbiter = false, tracker = false }) {
+// to the host instead, and "working.push" and "working.fetch" go to the
+// host's git socket.
+export function typedTools({ resolved, arbiter = false, tracker = false, projects = [] }) {
const held = new Set([...resolved.limits.authority.withinRole, ...resolved.limits.authority.crossRole]);
const tools = [];
const add = (name, verb, action, description, parameters) => tools.push({ name, verb, action, description, parameters });
@@ -98,6 +101,37 @@ export function typedTools({ resolved, arbiter = false, tracker = false }) {
if (held.has(action)) add(name, action, action, description, obj(properties, required));
}
}
+ // Row 93 (#1587): the review loop. Pushes and fetches need a clone to work in.
+ if (held.has("git.push.working") && projects.length) {
+ add("push_working", "working.push", "git.push.working", "Push a branch of your clone to the business's working repository as the task's work branch, work/vikunja-<project>-<task>. Only a fast-forward of what the task already pushed is taken. Commit first; the reply names the pushed commit, which request_review takes.", obj({
+ task_ref: TASK_REF,
+ project: str("The project whose clone holds the branch", { enum: [...projects] }),
+ branch: str("Local branch to push, such as the branch you committed on"),
+ }, ["task_ref", "project", "branch"]));
+ }
+ if (held.has("review.request")) {
+ add("request_review", "review.request", "review.request", "Ask a reviewer role to review the task's latest pushed commit. The reviewer gets a REVIEW-REQUEST message, and its verdict comes back to you as a REVIEW-RESULT.", obj({
+ task_ref: TASK_REF,
+ commit: str("The commit push_working returned", { pattern: "^[0-9a-f]{40}([0-9a-f]{24})?$" }),
+ to: str("Reviewer role instance"),
+ note: str("What to look at, for the reviewer"),
+ decision: COMMON.decision,
+ }, ["task_ref", "commit", "to"]));
+ }
+ if (held.has("review.verdict")) {
+ if (projects.length)
+ add("fetch_review", "working.fetch", null, "Fetch the commit a review request names into your clone of its project, as the remote branch the reply names. Check it out or diff it there; nothing is merged.", obj({
+ task_ref: TASK_REF,
+ request: str("The review request id from the REVIEW-REQUEST message"),
+ }, ["task_ref", "request"]));
+ add("review_verdict", "review.verdict", "review.verdict", "Answer a review request: approve, or changes with what must change. One verdict per request; it goes back to the role that asked.", obj({
+ task_ref: TASK_REF,
+ request: str("The review request id from the REVIEW-REQUEST message"),
+ verdict: str("Your verdict", { enum: ["approve", "changes"] }),
+ body: str("The review: what you checked and what you found"),
+ decision: COMMON.decision,
+ }, ["task_ref", "request", "verdict", "body"]));
+ }
add("raise_decision", "decision.raise", null, "Ask for a decision before a gated or cross-role action, or when you need a choice made. The answer arrives as a message.", obj({
action: str("The action the decision is about", { enum: [...ACTIONS] }),
question: str("The question, with enough context to answer it"),
diff --git a/packages/harness/tests/bundle.test.mjs b/packages/harness/tests/bundle.test.mjs
index a343200ff..6a17a0dcd 100644
--- a/packages/harness/tests/bundle.test.mjs
+++ b/packages/harness/tests/bundle.test.mjs
@@ -120,6 +120,23 @@ test("the prompt names the project clones in the workspace, and none when there
assert.deepEqual(line(build(t, resolvedFor(t, "pm"), { session: session([]) })), []);
});
+// Row 93 (#1587): the review loop's prompt lines, for the roles with its tools.
+test("the prompt explains the review loop to a role with push_working, and reviews to one with fetch_review", (t) => {
+ const lines = (b) => readFileSync(b.files.prompt, "utf8").split("\n").filter((l) => /^- (Review loop|Reviews): /.test(l));
+ const dir = scratch(t);
+ const session = { run: "run-1", launchedBy: "pm", workspace: join(dir, "ws"), toolSocket: join(dir, "tools.sock"), turnMarker: join(dir, "turn.done"), checkouts: [{ project: "stack", path: join(dir, "ws", "stack"), outcome: "cloned", branch: "main" }] };
+ const coder = lines(build(t, resolvedFor(t, "coder"), { session }));
+ assert.equal(coder.length, 1);
+ assert.match(coder[0], /^- Review loop: commit on a branch of your clone, push_working it, then request_review/);
+ assert.match(coder[0], /closes only on an approve verdict for its latest push/);
+ const reviewer = lines(build(t, resolvedFor(t, "reviewer", (d) => (d.roles.reviewer.vars = { harness: "pi" })), { session }));
+ assert.equal(reviewer.length, 1);
+ assert.match(reviewer[0], /^- Reviews: a REVIEW-REQUEST names a request id\. fetch_review it/);
+ assert.deepEqual(lines(build(t, resolvedFor(t, "pm"), { session })), []);
+ // Without a clone neither git tool exists, so neither line is there.
+ assert.deepEqual(lines(build(t, resolvedFor(t, "coder"))), []);
+});
+
test("a bundle is written once: an existing file refuses", (t) => {
const dir = scratch(t);
mkdirSync(join(dir, "bundle"));
diff --git a/packages/harness/tests/tools.test.mjs b/packages/harness/tests/tools.test.mjs
index c8fdfe467..232c3db4b 100644
--- a/packages/harness/tests/tools.test.mjs
+++ b/packages/harness/tests/tools.test.mjs
@@ -55,15 +55,16 @@ test("list_requirements reads the requirement ids, for a task.create holder with
test("a coder gets no launch, no resolve_decision, and no task tools without a tracker", (t) => {
const coder = resolvedFor(t, "coder");
const tools = typedTools({ resolved: coder });
- assert.deepEqual(names(tools), ["send_message", "raise_decision", "list_agents", "inbox", "trail"]);
- // Held actions without a tool are listed, so the manifest shows them.
- assert.deepEqual(untypedActions(coder, tools), ["task.update.assigned", "git.push.working", "review.request", "task.scope.change"]);
+ assert.deepEqual(names(tools), ["send_message", "request_review", "raise_decision", "list_agents", "inbox", "trail"]);
+ // Held actions without a tool are listed, so the manifest shows them. With
+ // no clone to push from, push_working is one of them.
+ assert.deepEqual(untypedActions(coder, tools), ["task.update.assigned", "git.push.working", "task.scope.change"]);
});
test("a coder with a tracker gets its own task tools and no task_reassign", (t) => {
const coder = resolvedFor(t, "coder");
assert.deepEqual(names(typedTools({ resolved: coder, tracker: true })), [
- "send_message", "task_scope_change", "task_update_assigned",
+ "send_message", "task_scope_change", "task_update_assigned", "request_review",
"raise_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task",
]);
});
@@ -76,7 +77,45 @@ test("launch only when the business's launch block names the instance as launche
test("an action outside the instance's authority has no tool", (t) => {
const coder = resolvedFor(t, "coder", (d) => (d.roles.coder.vars["limits.authority"] = ["review.request"]));
- assert.deepEqual(names(typedTools({ resolved: coder, tracker: true })), ["raise_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task"]);
+ assert.deepEqual(names(typedTools({ resolved: coder, tracker: true, projects: ["stack"] })), ["request_review", "raise_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task"]);
+});
+
+// Row 93 (#1587): the review loop's tools, by authority, and the git ones only with a clone.
+test("the review loop: push_working and request_review for the coder, fetch_review and review_verdict for the reviewer, none for the PM", (t) => {
+ const coder = resolvedFor(t, "coder");
+ const tools = typedTools({ resolved: coder, tracker: true, projects: ["stack", "site"] });
+ assert.deepEqual(names(tools), [
+ "send_message", "task_scope_change", "task_update_assigned", "push_working", "request_review",
+ "raise_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task",
+ ]);
+ assert.deepEqual(untypedActions(coder, tools), []);
+ const push = tools.find((x) => x.name === "push_working");
+ assert.deepEqual([push.verb, push.action], ["working.push", "git.push.working"]);
+ assert.deepEqual(push.parameters.required, ["task_ref", "project", "branch"]);
+ assert.deepEqual(push.parameters.properties.project.enum, ["stack", "site"]);
+ const ask = tools.find((x) => x.name === "request_review");
+ assert.deepEqual([ask.verb, ask.action], ["review.request", "review.request"]);
+ assert.deepEqual(ask.parameters.required, ["task_ref", "commit", "to"]);
+ assert.equal(ask.parameters.properties.commit.pattern, "^[0-9a-f]{40}([0-9a-f]{24})?$");
+
+ const reviewer = resolvedFor(t, "reviewer");
+ const rtools = typedTools({ resolved: reviewer, tracker: true, projects: ["stack"] });
+ assert.deepEqual(names(rtools), ["send_message", "task_update_assigned", "fetch_review", "review_verdict", "raise_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task"]);
+ const fetch = rtools.find((x) => x.name === "fetch_review");
+ assert.deepEqual([fetch.verb, fetch.action], ["working.fetch", null]);
+ assert.deepEqual(fetch.parameters.required, ["task_ref", "request"]);
+ const verdict = rtools.find((x) => x.name === "review_verdict");
+ assert.deepEqual([verdict.verb, verdict.action], ["review.verdict", "review.verdict"]);
+ assert.deepEqual(verdict.parameters.properties.verdict.enum, ["approve", "changes"]);
+ assert.deepEqual(verdict.parameters.required, ["task_ref", "request", "verdict", "body"]);
+ // No clone, no fetch; the verdict still has its tool.
+ assert.deepEqual(names(typedTools({ resolved: reviewer, projects: [] })).filter((n) => /review/.test(n)), ["review_verdict"]);
+
+ const pm = names(typedTools({ resolved: resolvedFor(t, "pm"), arbiter: true, tracker: true, projects: ["stack"] }));
+ for (const n of ["push_working", "request_review", "fetch_review", "review_verdict"]) assert.ok(!pm.includes(n), n);
+ const close = typedTools({ resolved: resolvedFor(t, "pm"), tracker: true }).find((x) => x.name === "task_close");
+ assert.match(close.description, /closes only on an approve verdict for its latest push/);
+ for (const tool of [...tools, ...rtools]) assert.equal(tool.parameters.additionalProperties, false);
});
test("callTool: one JSON line out, the result back, a refusal rejects", async (t) => {
diff --git a/packages/seat/README.md b/packages/seat/README.md
index 3797f503f..e993b9f09 100644
--- a/packages/seat/README.md
+++ b/packages/seat/README.md
@@ -184,11 +184,13 @@ each at its own host path; `binds()` returns them with the reason for each,
and the file's header comment has the table.
- Read-only: `/usr`, `/etc` and the host's `/bin`, `/lib`, `/lib64`,
- `/sbin` links into `/usr`; the ten repository files the runner loads
+ `/sbin` links into `/usr`; the twelve repository files the runner loads
(`RUNNER_FILES`); the adapter; for pi the repository's `node_modules`,
for Claude Code the `MOSAIC_CLAUDE_BIN` binary when it lives outside
- `/usr`; node when it lives outside `/usr`; the broker socket, and the
- launch socket for the role whose launch tool calls it.
+ `/usr`; node when it lives outside `/usr`; the broker socket, the
+ launch socket for the role whose launch tool calls it, and the host's
+ git socket (`bus-host/git.sock`) for a role that holds
+ git.push.working or review.verdict.
- Read-write: the instance's workspace and the run directory. The run's
`bundle/` and `session.json` are bound read-only again on top.
- Private: `/tmp` is the run's `tmp/`, `HOME` the run's empty `home/`, and
@@ -289,6 +291,57 @@ or the clone fails) rejects with a `CheckoutError` naming the project, and
the temp directory is removed. The launcher refuses the launch with
`checkout-failed` (`packages/cli/README.md`, "Sessions").
+### The working repositories (#1587)
+
+A coder's pushed work goes to one bare repository per business and
+project, `<dataRoot>/working/<business>/<project>.git`, which only the bus
+host writes (`src/working.mjs`). It isn't the project's root and it isn't
+a forge: no credential, no remote, and no sandbox binds it. A task's work
+lands on `refs/heads/work/vikunja-<project>-<task>`, a name the host makes
+from the task ref; the session only picks which branch of its bundle to
+take.
+
+Commits arrive as a git bundle over the host's git socket
+(`packages/cli/src/git-socket.mjs`), never by the host reading the
+session's clone. A live session can change its clone between any check and
+any git call, so the host doesn't run git there at all. It reads the
+bundle as it would a fetch from an untrusted remote.
+
+A push runs under the repository's lock, in this order:
+
+1. The bundle starts with the v2 header; v3 can carry a filter. Otherwise
+ `invalid-bundle`.
+2. `main` is fetched again from the project's root, which the host trusts.
+ A root it can't read refuses `root-unreadable`.
+3. `git bundle verify`. A prerequisite the repository lacks refuses
+ `missing-prerequisite`; anything else wrong is `invalid-bundle`.
+4. The bundle has the named branch, or `branch-not-in-bundle`.
+5. That one ref is fetched with `transfer.fsckObjects` into a temp ref
+ under `refs/mosaic/incoming/`. A broken object refuses `invalid-bundle`.
+6. The commit shares history with `main`, or `unrelated-history`.
+7. The temp ref moves to the work ref with no `+`, so git takes only a
+ fast-forward (`non-fast-forward`).
+8. The broker records the push, an `action.allowed` for
+ `git.push.working` naming the commit. When it
+ refuses, the work ref goes back to where it was.
+
+The temp ref is deleted on every path, and so is the bundle file in
+`bus-host/`. No other ref moves, and the root is only read.
+
+For a review, under the same lock, the host fetches `main` again and
+writes a bundle of the requested commit with one ref,
+`refs/mosaic/review`. It leaves out the commits the reviewer says its
+clones already have (at most 32, `HAVE_MAX`), but only those the working
+repository knows. A commit the repository doesn't have refuses
+`unknown-commit`.
+
+A bundle is at most 32 MiB (`BUNDLE_MAX`) both ways, or `bundle-too-large`.
+For a push the git socket checks the size the request names before it
+reads a byte; a review bundle over the cap isn't sent. Host git here runs as
+it does for the clones: no system or global config, hooks at `/dev/null`,
+`core.fsmonitor` off, only the `file` transport, no submodule recursion
+and no auto-maintenance.
+
## Exit codes
`launch` exits with the launch script's own code once the script runs.
diff --git a/packages/seat/src/sandbox.mjs b/packages/seat/src/sandbox.mjs
index 88db8db1e..274a570f1 100644
--- a/packages/seat/src/sandbox.mjs
+++ b/packages/seat/src/sandbox.mjs
@@ -8,7 +8,9 @@
// the repository files the runner loads; the adapter; for pi
// the repository's node_modules, for Claude Code its pinned
// binary; node when it lives outside /usr; the broker socket,
-// and the launch socket for the role that may launch
+// the launch socket for the role that may launch, and the
+// host's git socket for a role holding git.push.working or
+// review.verdict (#1587)
// read-write the instance's workspace and the run directory, except the
// run's bundle/ and session.json, which stay read-only
// private /tmp (the run's tmp/), HOME (the run's home/), /dev, /proc
@@ -46,6 +48,7 @@ export const RUNNER_FILES = Object.freeze([
"packages/harness/src/bundle.mjs",
"packages/harness/src/gate.mjs",
"packages/harness/src/tools.mjs",
+ "packages/harness/src/git-tools.mjs",
"packages/harness/src/claude-gate.mjs",
"packages/harness/src/mcp-server.mjs",
"packages/harness/src/pi-extension.mjs",
@@ -86,9 +89,10 @@ function systemBinds(node = process.execPath) {
// The binds for one session, in mount order; each says why the session
// needs it. paths: the run directory, the instance's workspace, the adapter,
// the harness ("pi" or "claude-code"), the broker socket, the launch socket
-// (null for a role that may not launch), the Claude Code binary
+// (null for a role that may not launch), the git socket (null for a role
+// that neither pushes nor reviews), the Claude Code binary
// (MOSAIC_CLAUDE_BIN, or null) and node. `readOnly` adds files, for tests only.
-export function binds({ runDir, workspace, adapter, harness, brokerSocket, launchSocket = null, claudeBin = null, node = process.execPath, readOnly = [] }) {
+export function binds({ runDir, workspace, adapter, harness, brokerSocket, launchSocket = null, gitSocket = null, claudeBin = null, node = process.execPath, readOnly = [] }) {
const out = systemBinds(node);
for (const f of RUNNER_FILES) out.push({ kind: "ro", src: f, why: "a repository file the runner loads" });
out.push({ kind: "ro", src: adapter, why: "the harness adapter the runner starts each turn" });
@@ -99,6 +103,7 @@ export function binds({ runDir, workspace, adapter, harness, brokerSocket, launc
}
out.push({ kind: "ro", src: brokerSocket, why: "the broker socket the runner claims and calls through" });
if (launchSocket) out.push({ kind: "ro", src: launchSocket, why: "the launch socket, for the role whose launch tool calls it" });
+ if (gitSocket) out.push({ kind: "ro", src: gitSocket, why: "the host's git socket, for push_working and fetch_review" });
for (const f of readOnly) out.push({ kind: "ro", src: f, why: "a test fixture" });
out.push(
{ kind: "rw", src: workspace, why: "the instance's workspace, where the harness works" },
diff --git a/packages/seat/src/session.mjs b/packages/seat/src/session.mjs
index eb5fa18b1..ff79ee827 100644
--- a/packages/seat/src/session.mjs
+++ b/packages/seat/src/session.mjs
@@ -42,6 +42,8 @@ export const SESSIONS_VERSION = 1;
export const sessionsFile = (dataRoot) => join(dataRoot, "bus-host", "sessions.json");
export const launchSocketPath = (dataRoot) => join(dataRoot, "bus-host", "launch.sock");
+// Row 93 (#1587): packages/cli/src/git-socket.mjs.
+export const gitSocketPath = (dataRoot) => join(dataRoot, "bus-host", "git.sock");
export const runsDir = (dataRoot, business) => join(dataRoot, "launches", business);
export const launchLogFile = (dataRoot, business) => join(dataRoot, "launches", `${business}.jsonl`);
export const workspaceDir = (dataRoot, business, instance) => join(dataRoot, "workspaces", business, instance);
diff --git a/packages/seat/src/working.mjs b/packages/seat/src/working.mjs
new file mode 100644
index 000000000..62b4c73f0
--- /dev/null
+++ b/packages/seat/src/working.mjs
@@ -0,0 +1,240 @@
+// The working repositories (#1587): one bare git repository per business
+// and project at `<dataRoot>/working/<business>/<project>.git`, which only
+// the bus host writes. A coder's push lands there as
+// `refs/heads/work/vikunja-<project>-<task>`, and a reviewer fetches from
+// there. It isn't the project's root and it isn't a forge; no credential and
+// no remote goes into it, and no sandbox binds it.
+//
+// Commits move as git bundles. The session's runner makes the bundle in its
+// own clone, inside its sandbox, and sends the bytes over the host's git
+// socket (packages/cli/src/launcher.mjs). The host never runs git in a
+// session's clone and never opens a path in a session's workspace: a live
+// session can change its clone between any check and any git call, so no
+// check of the clone would hold. Host git reads only the bundle, as a fetch
+// from an untrusted remote, with fsck on.
+//
+// Every host git call here runs with no system or global config, hooks at
+// /dev/null, fsmonitor off, only local transports and no submodule
+// recursion (gitEnv and SAFE), in a repository the host made itself.
+//
+// push, under the repository's lock:
+// 1. the bundle file starts with the v2 header (v3 can carry a filter)
+// 2. main is fetched again from the project's root, which the host trusts
+// 3. `bundle verify`: a prerequisite the repository lacks refuses
+// missing-prerequisite
+// 4. the bundle has the named branch (branch-not-in-bundle)
+// 5. that one ref is fetched with transfer.fsckObjects into a temp ref;
+// a broken object refuses invalid-bundle
+// 6. the commit shares history with main (unrelated-history)
+// 7. the temp ref moves to the work ref with no `+`, so git refuses
+// anything but a fast-forward (non-fast-forward)
+// 8. record() runs; when it throws, the work ref goes back
+// The temp ref is deleted on every path. The caller owns the bundle file and
+// deletes it.
+//
+// review, under the same lock: main again from the root, then a bundle of
+// the requested commit that leaves out the commits the reviewer said it has
+// (those the repository knows), written to the file the caller names.
+
+import { spawn } from "node:child_process";
+import { randomBytes } from "node:crypto";
+import { closeSync, lstatSync, mkdirSync, openSync, readSync, renameSync, rmSync } from "node:fs";
+import { dirname, join } from "node:path";
+import { gitEnv, GIT_TIMEOUT_MS } from "./checkout.mjs";
+
+// The largest bundle the host takes or sends.
+export const BUNDLE_MAX = 32 * 1024 * 1024;
+// The most commits a reviewer may say it has.
+export const HAVE_MAX = 32;
+const V2 = "# v2 git bundle\n";
+const OID = /^[0-9a-f]{40}([0-9a-f]{24})?$/;
+const TASK_REF = /^vikunja:([1-9][0-9]*)\/([1-9][0-9]*)$/;
+const PROJECT = /^[a-z0-9][a-z0-9._-]{0,63}$/;
+
+export class WorkingError extends Error {
+ constructor(code, message = code) {
+ super(message);
+ this.code = code;
+ }
+}
+const refuse = (code, message) => {
+ throw new WorkingError(code, message);
+};
+
+// Ahead of every git subcommand. Command-line config wins over any file.
+const SAFE = [
+ "-c", "core.hooksPath=/dev/null",
+ "-c", "core.fsmonitor=false",
+ "-c", "protocol.allow=never",
+ "-c", "protocol.file.allow=always",
+ "-c", "submodule.recurse=false",
+];
+// Ahead of the fetch from a bundle. Tests replace it to show it matters.
+export const FSCK = ["-c", "transfer.fsckObjects=true"];
+
+function git(args, env, { pre = [] } = {}) {
+ return new Promise((resolve) => {
+ const child = spawn("git", [...SAFE, ...pre, ...args], { env: gitEnv(env), timeout: GIT_TIMEOUT_MS, killSignal: "SIGKILL", stdio: ["ignore", "pipe", "pipe"] });
+ let out = "";
+ let err = "";
+ child.stdout.setEncoding("utf8").on("data", (d) => (out += d));
+ child.stderr.setEncoding("utf8").on("data", (d) => (err += d));
+ child.on("error", (e) => resolve({ ok: false, out, err, why: e.code ?? e.message }));
+ child.on("close", (code, signal) => resolve({ ok: code === 0, out, err, why: signal ? `killed by ${signal}` : err.trim().split("\n").pop() || `exit ${code}` }));
+ });
+}
+
+// The branch name a task's work goes to. A task ref holds a colon, which a
+// ref name can't.
+export function workRef(taskRef) {
+ const m = TASK_REF.exec(taskRef ?? "");
+ if (!m) refuse("invalid-request", "not a task ref");
+ return `work/vikunja-${m[1]}-${m[2]}`;
+}
+
+// A branch name the session gives: it only picks a ref inside the bundle.
+export function validBranch(branch) {
+ return (
+ typeof branch === "string" &&
+ /^[A-Za-z0-9][A-Za-z0-9._\/-]{0,199}$/.test(branch) &&
+ !/\.\.|\/\/|\/\.|@\{|\.lock(\/|$)|[./]$/.test(branch)
+ );
+}
+
+export const workingRepo = (dataRoot, business, project) => join(dataRoot, "working", business, `${project}.git`);
+
+// One chain per repository: a push and a fetch never overlap in one repo.
+const locks = new Map();
+function locked(repo, fn) {
+ const run = (locks.get(repo) ?? Promise.resolve()).then(fn);
+ const tail = run.catch(() => {});
+ locks.set(repo, tail);
+ tail.then(() => {
+ if (locks.get(repo) === tail) locks.delete(repo);
+ });
+ return run;
+}
+
+// The bare repository, made on first use in a temp directory beside it and
+// renamed into place. One that exists must be a real directory.
+async function ensure(repo, env) {
+ try {
+ if (!lstatSync(repo).isDirectory()) refuse("working-repo-invalid", `${repo} isn't a directory`);
+ return;
+ } catch (e) {
+ if (e instanceof WorkingError) throw e;
+ if (e.code !== "ENOENT") refuse("working-repo-invalid", `${repo} can't be read: ${e.code}`);
+ }
+ mkdirSync(dirname(repo), { recursive: true, mode: 0o700 });
+ const tmp = join(dirname(repo), `.init-${randomBytes(4).toString("hex")}`);
+ try {
+ const r = await git(["init", "-q", "--bare", "--template=", tmp], env);
+ if (!r.ok) refuse("working-repo-invalid", `git init failed: ${r.why}`);
+ renameSync(tmp, repo);
+ } finally {
+ rmSync(tmp, { recursive: true, force: true });
+ }
+}
+
+// main from what the root has checked out. The root is the owner's; it isn't
+// a session's.
+async function seed(repo, root, env) {
+ const r = await git([`--git-dir=${repo}`, "fetch", "-q", "--no-tags", "--no-write-fetch-head", "--no-recurse-submodules", "--no-auto-maintenance", root, "+HEAD:refs/heads/main"], env);
+ if (!r.ok) refuse("root-unreadable", `main couldn't be fetched from ${root}: ${r.why}`);
+}
+
+function header(file) {
+ const fd = openSync(file, "r");
+ try {
+ const b = Buffer.alloc(V2.length);
+ return readSync(fd, b, 0, b.length, 0) === b.length && b.toString("latin1") === V2;
+ } finally {
+ closeSync(fd);
+ }
+}
+
+const tip = async (repo, ref, env) => {
+ const r = await git([`--git-dir=${repo}`, "rev-parse", "-q", "--verify", `${ref}^{commit}`], env);
+ return r.ok ? r.out.trim() : null;
+};
+
+// await push({ dataRoot, business, project, root, taskRef, branch, bundle, record, env, fsck })
+// Resolves { ref, commit, previous }; rejects with WorkingError. `record`
+// runs under the lock once the ref moved, and a throw moves it back. `fsck`
+// is for the tests.
+export function push({ dataRoot, business, project, root, taskRef, branch, bundle, record = async () => {}, env = process.env, fsck = FSCK }) {
+ if (!PROJECT.test(project ?? "")) refuse("invalid-request", "not a project id");
+ if (!validBranch(branch)) refuse("invalid-request", "not a branch name");
+ const ref = `refs/heads/${workRef(taskRef)}`;
+ const repo = workingRepo(dataRoot, business, project);
+ return locked(repo, async () => {
+ if (!header(bundle)) refuse("invalid-bundle", "not a v2 git bundle");
+ await ensure(repo, env);
+ await seed(repo, root, env);
+ const at = [`--git-dir=${repo}`];
+ // Not -q: with it, git 2.55 drops the list of missing prerequisites.
+ const verify = await git([...at, "bundle", "verify", bundle], env);
+ if (!verify.ok) {
+ if (/lacks these prerequisite/.test(verify.err)) refuse("missing-prerequisite", "the bundle needs commits the working repository doesn't have");
+ refuse("invalid-bundle", verify.why);
+ }
+ const heads = await git([...at, "bundle", "list-heads", bundle], env);
+ if (!heads.ok) refuse("invalid-bundle", heads.why);
+ if (!heads.out.split("\n").some((l) => l.split(" ")[1] === `refs/heads/${branch}`)) refuse("branch-not-in-bundle");
+ const incoming = `refs/mosaic/incoming/${randomBytes(8).toString("hex")}`;
+ try {
+ const fetched = await git([...at, "fetch", "-q", "--no-tags", "--no-write-fetch-head", "--no-recurse-submodules", "--no-auto-maintenance", bundle, `refs/heads/${branch}:${incoming}`], env, { pre: fsck });
+ if (!fetched.ok) refuse("invalid-bundle", fetched.why);
+ const commit = await tip(repo, incoming, env);
+ if (!commit) refuse("invalid-bundle", "the branch isn't a commit");
+ if (!(await git([...at, "merge-base", commit, "refs/heads/main"], env)).ok) refuse("unrelated-history", "the commit shares no history with main");
+ const previous = await tip(repo, ref, env);
+ // From the repository into itself, by its path (`.` would be the cwd), and
+ // not -q, which drops the "(non-fast-forward)" reason.
+ const moved = await git([...at, "fetch", "--no-tags", "--no-write-fetch-head", "--no-auto-maintenance", repo, `${incoming}:${ref}`], env);
+ if (!moved.ok) refuse(/\(non-fast-forward\)/.test(moved.err) ? "non-fast-forward" : "push-failed", moved.why);
+ try {
+ await record({ ref: workRef(taskRef), commit, previous });
+ } catch (e) {
+ const back = previous ? ["update-ref", ref, previous, commit] : ["update-ref", "-d", ref, commit];
+ await git([...at, ...back], env);
+ throw e;
+ }
+ return { ref: workRef(taskRef), commit, previous };
+ } finally {
+ await git([...at, "update-ref", "-d", incoming], env);
+ }
+ });
+}
+
+// await review({ dataRoot, business, project, root, taskRef, commit, have, out, env })
+// Writes a bundle of `commit` to `out`, with one ref, refs/mosaic/review.
+// `have` lists commits the reviewer's clone holds; those the repository
+// doesn't know are skipped. Resolves { ref, commit }.
+export function review({ dataRoot, business, project, root, taskRef, commit, have = [], out, env = process.env }) {
+ if (!PROJECT.test(project ?? "") || !OID.test(commit ?? "")) refuse("invalid-request");
+ if (!Array.isArray(have) || have.length > HAVE_MAX || have.some((h) => !OID.test(h ?? ""))) refuse("invalid-request", "have must list commit ids");
+ const repo = workingRepo(dataRoot, business, project);
+ return locked(repo, async () => {
+ try {
+ if (!lstatSync(repo).isDirectory()) refuse("working-repo-invalid");
+ } catch (e) {
+ if (e instanceof WorkingError) throw e;
+ refuse("unknown-commit", "the working repository doesn't exist");
+ }
+ await seed(repo, root, env);
+ const at = [`--git-dir=${repo}`];
+ if (!(await tip(repo, commit, env))) refuse("unknown-commit");
+ const known = [];
+ for (const h of have) if (await tip(repo, h, env)) known.push(h);
+ const temp = "refs/mosaic/review";
+ try {
+ if (!(await git([...at, "update-ref", temp, commit], env)).ok) refuse("fetch-failed");
+ const made = await git([...at, "bundle", "create", "-q", out, temp, ...(known.length ? ["--not", ...known] : [])], env);
+ if (!made.ok) refuse(/empty bundle/.test(made.err) ? "nothing-to-fetch" : "fetch-failed", made.why);
+ } finally {
+ await git([...at, "update-ref", "-d", temp], env);
+ }
+ return { ref: workRef(taskRef), commit };
+ });
+}
diff --git a/packages/seat/tests/sandbox.test.mjs b/packages/seat/tests/sandbox.test.mjs
index 704d20323..9d029c6db 100644
--- a/packages/seat/tests/sandbox.test.mjs
+++ b/packages/seat/tests/sandbox.test.mjs
@@ -63,9 +63,23 @@ test("every bind says why; the bundle and session file stay read-only over the r
}
assert.deepEqual(list.at(-1), { kind: "rw", src: join(runDir, "tmp"), dest: "/tmp", why: list.at(-1).why });
assert.equal(at(join(root, "launch.sock")), -1, "no launch socket for a role that may not launch");
+ assert.equal(at(join(root, "git.sock")), -1, "no git socket for a role that neither pushes nor reviews");
assert.ok(!list.some((b) => b.src === realpathSync(join(REPO, "node_modules"))), "Claude Code gets no node_modules");
});
+// Row 93 (#1587): the git socket is bound read-only, before the workspace, when the launcher passes one.
+test("the host's git socket is bound only when given, read-only and before the writable binds", (t) => {
+ const root = scratch(t);
+ const args = { runDir: join(root, "run"), workspace: join(root, "ws"), adapter: join(root, "a.sh"), harness: "pi", brokerSocket: join(root, "bus.sock") };
+ const list = binds({ ...args, gitSocket: join(root, "git.sock") });
+ const at = (src) => list.findIndex((b) => b.src === src);
+ assert.equal(list[at(join(root, "git.sock"))].kind, "ro");
+ assert.match(list[at(join(root, "git.sock"))].why, /push_working and fetch_review/);
+ assert.ok(at(join(root, "git.sock")) < at(join(root, "ws")));
+ assert.equal(binds(args).findIndex((b) => b.src === join(root, "git.sock")), -1);
+ assert.ok(RUNNER_FILES.includes(join(REPO, "packages/harness/src/git-tools.mjs")));
+});
+
// The founder's files and the other sessions' state, as stand-ins. The
// /dev/shm file and the SysV segment are the host's, made for this test and
// removed after it.
diff --git a/packages/seat/tests/working.test.mjs b/packages/seat/tests/working.test.mjs
new file mode 100644
index 000000000..68a6de1f7
--- /dev/null
+++ b/packages/seat/tests/working.test.mjs
@@ -0,0 +1,280 @@
+// The working repositories (src/working.mjs, #1587), against real git
+// repositories in a temp directory. The bundles are made the way a
+// session's runner makes them (packages/harness/src/git-tools.mjs). Every
+// case also checks that the root's files and refs are byte for byte what
+// they were, that no temp ref is left, and that only the task's own ref moved.
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import { createHash } from "node:crypto";
+import { existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { gitEnv } from "../src/checkout.mjs";
+import { push, review, validBranch, workingRepo, workRef, WorkingError } from "../src/working.mjs";
+
+const TASK = "vikunja:1/2";
+const REF = "refs/heads/work/vikunja-1-2";
+
+function tmp(t) {
+ const dir = mkdtempSync(join(tmpdir(), "mosaic-working-"));
+ t.after(() => rmSync(dir, { recursive: true, force: true }));
+ return dir;
+}
+
+function git(cwd, ...args) {
+ const r = spawnSync("git", ["-c", "user.name=t", "-c", "[email protected]", ...args], { cwd, encoding: "utf8", env: gitEnv() });
+ assert.equal(r.status, 0, `git ${args.join(" ")}: ${r.stderr}`);
+ return r.stdout.trim();
+}
+
+function commit(dir, text) {
+ writeFileSync(join(dir, "README"), `${text}\n`);
+ git(dir, "add", "README");
+ git(dir, "commit", "-q", "-m", text);
+ return git(dir, "rev-parse", "HEAD");
+}
+
+function fingerprint(dir) {
+ const h = createHash("sha256");
+ const walk = (d) => {
+ for (const name of readdirSync(d).sort()) {
+ const p = join(d, name);
+ const st = lstatSync(p);
+ h.update(`${p}\0${st.mode}\0`);
+ if (st.isDirectory()) walk(p);
+ else if (st.isSymbolicLink()) h.update(readlinkSync(p));
+ else if (st.isFile()) h.update(readFileSync(p));
+ }
+ };
+ walk(dir);
+ return h.digest("hex");
+}
+
+// A root on main with one commit, a coder's clone of it, and the data root.
+function world(t) {
+ const dir = tmp(t);
+ const root = join(dir, "root");
+ mkdirSync(root);
+ git(root, "init", "-q", "-b", "main");
+ const first = commit(root, "one");
+ const clone = join(dir, "clone");
+ git(dir, "clone", "-q", root, clone);
+ const dataRoot = join(dir, "data");
+ mkdirSync(dataRoot, { mode: 0o700 });
+ const repo = workingRepo(dataRoot, "acme", "stack");
+ let n = 0;
+ // A bundle of `branch` the way the runner makes one.
+ const bundle = (branch = "feat", from = clone) => {
+ const file = join(dir, `b${n++}.bundle`);
+ git(from, "bundle", "create", "-q", file, `refs/heads/${branch}`, "--not", "--remotes=origin", "--remotes=working");
+ return file;
+ };
+ const refs = () => (existsSync(repo) && lstatSync(repo).isDirectory() ? git(dir, `--git-dir=${repo}`, "for-each-ref", "--format=%(refname) %(objectname)") : "");
+ const go = (file, extra = {}) => push({ dataRoot, business: "acme", project: "stack", root, taskRef: TASK, branch: "feat", bundle: file, ...extra });
+ return { dir, root, first, clone, dataRoot, repo, bundle, refs, go };
+}
+
+// A refusal with `code`, and the root, the working repository's refs and its
+// temp refs as they were.
+async function refuses(w, fn, code) {
+ const root = fingerprint(w.root);
+ const refs = w.refs();
+ await assert.rejects(async () => fn(), (e) => e instanceof WorkingError && e.code === code);
+ assert.equal(fingerprint(w.root), root, "the root is unchanged");
+ if (refs) assert.equal(w.refs(), refs, "no ref of the working repository moved");
+ assert.doesNotMatch(w.refs(), /refs\/mosaic\//, "no temp ref is left");
+}
+
+async function lands(w, fn) {
+ const root = fingerprint(w.root);
+ const r = await fn();
+ assert.equal(fingerprint(w.root), root, "the root is unchanged");
+ assert.doesNotMatch(w.refs(), /refs\/mosaic\//, "no temp ref is left");
+ return r;
+}
+
+test("workRef and validBranch take only what a ref name can hold", () => {
+ assert.equal(workRef("vikunja:12/345"), "work/vikunja-12-345");
+ for (const bad of ["vikunja:0/1", "vikunja:1", "gitea:1/2", "vikunja:1/2\n", null]) assert.throws(() => workRef(bad), WorkingError);
+ for (const ok of ["feat", "fix/thing-2", "a.b_c"]) assert.equal(validBranch(ok), true, ok);
+ for (const bad of ["", "-x", "a..b", "a//b", "a/.b", "a@{1}", "x.lock", "a/", "a.", "a b", "a:b", "a~1", "a".repeat(201)]) assert.equal(validBranch(bad), false, bad);
+});
+
+test("a push lands as the task's work ref, a fast-forward lands after it, and main is the root's", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ const c1 = commit(w.clone, "two");
+ const seen = [];
+ const r1 = await lands(w, () => w.go(w.bundle(), { record: async (r) => seen.push(r) }));
+ assert.deepEqual(r1, { ref: "work/vikunja-1-2", commit: c1, previous: null });
+ assert.deepEqual(seen, [r1]);
+ assert.equal(w.refs(), `refs/heads/main ${w.first}\n${REF} ${c1}`);
+
+ // The runner's clone remembers the push, so the next bundle carries only c2.
+ git(w.clone, "update-ref", "refs/remotes/working/work/vikunja-1-2", c1);
+ const c2 = commit(w.clone, "three");
+ const file = w.bundle();
+ assert.match(git(w.dir, "bundle", "list-heads", file), new RegExp(c2));
+ const r2 = await lands(w, () => w.go(file));
+ assert.deepEqual(r2, { ref: "work/vikunja-1-2", commit: c2, previous: c1 });
+ assert.equal(w.refs(), `refs/heads/main ${w.first}\n${REF} ${c2}`);
+});
+
+test("a rewritten branch refuses non-fast-forward and the work ref stays", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "two");
+ await w.go(w.bundle());
+ git(w.clone, "reset", "-q", "--hard", w.first);
+ commit(w.clone, "other two");
+ await refuses(w, () => w.go(w.bundle()), "non-fast-forward");
+});
+
+test("a branch with no history in common with main refuses unrelated-history", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "--orphan", "feat");
+ git(w.clone, "rm", "-q", "-rf", ".");
+ commit(w.clone, "alone");
+ await refuses(w, () => w.go(w.bundle()), "unrelated-history");
+ assert.equal(w.refs(), `refs/heads/main ${w.first}`);
+});
+
+test("a bundle that needs commits the working repository lacks refuses missing-prerequisite", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ const local = commit(w.clone, "never pushed");
+ commit(w.clone, "on top");
+ const file = join(w.dir, "partial.bundle");
+ git(w.clone, "bundle", "create", "-q", file, "refs/heads/feat", "--not", local);
+ await refuses(w, () => w.go(file), "missing-prerequisite");
+});
+
+test("main is fetched from the root again before each push, so a clone of a newer root pushes", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "two");
+ await w.go(w.bundle());
+ // The root moves on; a second coder clones it and branches from there.
+ const newer = commit(w.root, "root two");
+ const other = join(w.dir, "other");
+ git(w.dir, "clone", "-q", w.root, other);
+ git(other, "checkout", "-q", "-b", "feat");
+ const c = commit(other, "from newer");
+ const r = await lands(w, () => push({ dataRoot: w.dataRoot, business: "acme", project: "stack", root: w.root, taskRef: "vikunja:1/3", branch: "feat", bundle: w.bundle("feat", other) }));
+ assert.equal(r.commit, c);
+ assert.match(w.refs(), new RegExp(`refs/heads/main ${newer}`));
+});
+
+test("a root moved back after the clone leaves the push's base unknown: missing-prerequisite", async (t) => {
+ const w = world(t);
+ commit(w.root, "root two");
+ git(w.clone, "pull", "-q", "--ff-only");
+ git(w.root, "reset", "-q", "--hard", w.first);
+ git(w.root, "reflog", "expire", "--expire=now", "--all");
+ git(w.root, "gc", "-q", "--prune=now");
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "three");
+ await refuses(w, () => w.go(w.bundle()), "missing-prerequisite");
+});
+
+test("a commit with a broken object refuses invalid-bundle, and only because fsck is on", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ const tree = git(w.clone, "rev-parse", "HEAD^{tree}");
+ const raw = `tree ${tree}\nparent ${w.first}\nauthor nobody\ncommitter t <[email protected]> 1700000000 +0000\n\nbad\n`;
+ const r = spawnSync("git", ["hash-object", "-t", "commit", "-w", "--literally", "--stdin"], { cwd: w.clone, input: raw, encoding: "utf8", env: gitEnv() });
+ assert.equal(r.status, 0, r.stderr);
+ git(w.clone, "update-ref", "refs/heads/feat", r.stdout.trim());
+ const file = w.bundle();
+ await refuses(w, () => w.go(file), "invalid-bundle");
+ assert.doesNotMatch(w.refs(), /work\//);
+ // The same bundle with no fsck lands: the refusal above is fsck's.
+ const landed = await w.go(file, { fsck: [] });
+ assert.equal(landed.commit, r.stdout.trim());
+});
+
+test("only a v2 bundle with the named branch is read", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "two");
+ const v2 = w.bundle();
+ const v3 = join(w.dir, "v3.bundle");
+ writeFileSync(v3, readFileSync(v2, "latin1").replace("# v2 git bundle\n", "# v3 git bundle\n@object-format=sha1\n"), "latin1");
+ await refuses(w, () => w.go(v3), "invalid-bundle");
+ const junk = join(w.dir, "junk.bundle");
+ writeFileSync(junk, "# v2 git bundle\nnot a bundle\n");
+ await refuses(w, () => w.go(junk), "invalid-bundle");
+ await refuses(w, () => push({ dataRoot: w.dataRoot, business: "acme", project: "stack", root: w.root, taskRef: TASK, branch: "other", bundle: v2 }), "branch-not-in-bundle");
+});
+
+test("a branch named main, or named for another task, moves only this task's work ref", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "work/vikunja-9-9");
+ const c = commit(w.clone, "two");
+ git(w.clone, "branch", "-f", "main", c);
+ for (const branch of ["main", "work/vikunja-9-9"]) {
+ const r = await lands(w, () => push({ dataRoot: w.dataRoot, business: "acme", project: "stack", root: w.root, taskRef: TASK, branch, bundle: w.bundle(branch) }));
+ assert.equal(r.ref, "work/vikunja-1-2");
+ }
+ assert.equal(w.refs(), `refs/heads/main ${w.first}\n${REF} ${c}`);
+});
+
+test("a record that throws moves the work ref back, or removes a new one", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "two");
+ const boom = async () => {
+ throw new Error("not recorded");
+ };
+ await assert.rejects(w.go(w.bundle(), { record: boom }), /not recorded/);
+ assert.equal(w.refs(), `refs/heads/main ${w.first}`);
+ const c1 = (await w.go(w.bundle())).commit;
+ commit(w.clone, "three");
+ await assert.rejects(w.go(w.bundle(), { record: boom }), /not recorded/);
+ assert.equal(w.refs(), `refs/heads/main ${w.first}\n${REF} ${c1}`);
+});
+
+test("a working repository path that isn't a directory refuses", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ commit(w.clone, "two");
+ mkdirSync(join(w.dataRoot, "working", "acme"), { recursive: true });
+ writeFileSync(w.repo, "");
+ await refuses(w, () => w.go(w.bundle()), "working-repo-invalid");
+});
+
+test("review bundles a pushed commit as refs/mosaic/review, leaving out what the reviewer has", async (t) => {
+ const w = world(t);
+ git(w.clone, "checkout", "-q", "-b", "feat");
+ const c1 = commit(w.clone, "two");
+ await w.go(w.bundle());
+ const out = join(w.dir, "review.bundle");
+ const args = { dataRoot: w.dataRoot, business: "acme", project: "stack", root: w.root, taskRef: TASK, commit: c1, out };
+ assert.deepEqual(await lands(w, () => review({ ...args, have: [w.first, "f".repeat(40)] })), { ref: "work/vikunja-1-2", commit: c1 });
+ assert.equal(git(w.dir, "bundle", "list-heads", out), `${c1} refs/mosaic/review`);
+ // Only the one commit: the reviewer has main.
+ assert.match(git(w.dir, `--git-dir=${w.repo}`, "bundle", "verify", out), /requires this ref/);
+ rmSync(out);
+ await refuses(w, () => review({ ...args, commit: "e".repeat(40) }), "unknown-commit");
+ await refuses(w, () => review({ ...args, have: ["x"] }), "invalid-request");
+ await refuses(w, () => review({ ...args, have: Array(33).fill(w.first) }), "invalid-request");
+ await refuses(w, () => review({ ...args, have: [c1] }), "nothing-to-fetch");
+ assert.equal(existsSync(out), false);
+ assert.doesNotMatch(w.refs(), /refs\/mosaic/);
+});
+
+test("review of a project with no working repository refuses unknown-commit", async (t) => {
+ const w = world(t);
+ await refuses(w, () => review({ dataRoot: w.dataRoot, business: "acme", project: "stack", root: w.root, taskRef: TASK, commit: w.first, out: join(w.dir, "x.bundle") }), "unknown-commit");
+ assert.equal(existsSync(w.repo), false);
+});
+
+test("the runner's git tools hold the host's limits and branch rule", async () => {
+ const tools = await import("../../harness/src/git-tools.mjs");
+ const working = await import("../src/working.mjs");
+ assert.equal(tools.BUNDLE_MAX, working.BUNDLE_MAX);
+ assert.equal(tools.HAVE_MAX, working.HAVE_MAX);
+ for (const b of ["feat", "a/b", "-x", "a..b", "a.lock", "a/", "x".repeat(200), "x".repeat(201)]) assert.equal(tools.validBranch(b), working.validBranch(b), b);
+});
diff --git a/packages/tasks/src/verbs.mjs b/packages/tasks/src/verbs.mjs
index cef5ae445..dfd6bb1d3 100644
--- a/packages/tasks/src/verbs.mjs
+++ b/packages/tasks/src/verbs.mjs
@@ -159,7 +159,8 @@ export function verbs(ctx) {
if (before.task.done) fail('task-done');
const work = plan.check(before, at);
await located(plan.others ?? []);
- ctx.broker.authorize(cap, verb, { decision: plan.decision, target: args.task_ref });
+ // Row 93 (#1587): task.close's verdict goes to the broker, which checks it on a pushed task.
+ ctx.broker.authorize(cap, verb, { decision: plan.decision, target: args.task_ref, ...(verb === 'task.close' ? { verdict: args.verdict } : {}) });
let failure = null;
let landed = 0;
try {
@@ -363,7 +364,8 @@ export function verbs(ctx) {
}
case 'task.close': {
keys(args, [...common, 'verdict'], ['task_ref', 'verdict']);
- // The queue holds review verdicts in slice 1. This is a citation, recorded and not checked.
+ // A review.verdict event id. On a task with a recorded push the broker checks it
+ // (row 93); on any other task it is a citation, recorded and not checked.
const verdict = text(args.verdict, 256, 1);
plan.check = (_before, { id, project, view }) => ({
steps: [
diff --git a/packages/tasks/tests/verbs.test.mjs b/packages/tasks/tests/verbs.test.mjs
index 251317d39..91b26ec8b 100644
--- a/packages/tasks/tests/verbs.test.mjs
+++ b/packages/tasks/tests/verbs.test.mjs
@@ -237,6 +237,38 @@ test('task.close needs a verdict; after it every verb refuses with task-done', a
await assert.rejects(w.call(coder, 'task.update.assigned', { task_ref, percent_done: 1 }), refusedWith('task-done'));
await assert.rejects(w.call(pm, 'task.close', { task_ref, verdict: 'again' }), refusedWith('task-done'));
});
+test('a task with pushed work closes only citing an approve verdict for its latest push, and a refusal writes nothing', async (t) => {
+ // Row 93 (#1587): the verb hands its verdict to the broker's check.
+ const w = await ready(t, { business: (b) => (b.projects = { stack: { root: '/nonexistent' } }) });
+ const pm = w.agent('pm');
+ const coder = w.agent('coder');
+ const reviewer = w.agent('reviewer');
+ const { task_ref } = await task(w);
+ const push = (commit, previous) => {
+ w.broker.checkPush(coder, task_ref);
+ w.broker.recordPush(coder, { task_ref, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-1', commit, previous });
+ };
+ const verdict = (commit, v) => {
+ const r = w.broker.request(coder, { verb: 'review.request', args: { task_ref, commit, to: 'reviewer' } });
+ return w.broker.request(reviewer, { verb: 'review.verdict', args: { task_ref, request: r.request, verdict: v, body: v } }).verdict;
+ };
+ const A = 'a'.repeat(40);
+ const B = 'b'.repeat(40);
+ push(A, null);
+ const writes = () => w.fake.requests.filter((r) => r.method !== 'GET').length;
+ const before = writes();
+ await assert.rejects(w.call(pm, 'task.close', { task_ref, verdict: 'queue:38' }), refusedWith('verdict-not-found'));
+ await assert.rejects(w.call(pm, 'task.close', { task_ref, verdict: verdict(A, 'changes') }), refusedWith('verdict-not-approve'));
+ const stale = verdict(A, 'approve');
+ push(B, A);
+ await assert.rejects(w.call(pm, 'task.close', { task_ref, verdict: stale }), refusedWith('verdict-stale'));
+ assert.equal(writes(), before, 'no refused close reached the tracker');
+ assert.equal(w.fake.task(1).done, false);
+ const fresh = verdict(B, 'approve');
+ await w.call(pm, 'task.close', { task_ref, verdict: fresh });
+ assert.equal(w.fake.task(1).done, true);
+ assert.deepEqual(w.events('task.closed')[0].body, { verdict: fresh });
+});
test('a lost answer is settled by a re-read and never retried', async (t) => {
const w = await ready(t);
const pm = w.agent('pm');
diff --git a/roles/coder.md b/roles/coder.md
index 768ef907b..9163046c0 100644
--- a/roles/coder.md
+++ b/roles/coder.md
@@ -7,11 +7,18 @@ assigned to you and hand finished work to review.
- Work only on tasks assigned to you. Update a task's state and comments
as the work moves.
-- Push only to the project's working branch. Protected branches belong
- to the human.
+- Push only with `push_working`. It sends one branch of your clone to
+ the task's work branch in the business's working repository, and only
+ as a fast-forward of what the task pushed before. Protected branches
+ belong to the human.
- Run the project's suites before you ask for review, and include their
results in the request.
-- Request a review when a task's acceptance evidence is complete.
+- Request a review when a task's acceptance evidence is complete. Commit,
+ push, then call `request_review` with the commit `push_working`
+ returned and a reviewer role. Work that isn't pushed can't be reviewed.
+- After a `changes` verdict, fix the findings, push again and ask again.
+ A task with pushed work closes only on an `approve` verdict for its
+ latest push.
## Authority
diff --git a/roles/pm.md b/roles/pm.md
index 534220823..522ac9c0e 100644
--- a/roles/pm.md
+++ b/roles/pm.md
@@ -21,6 +21,10 @@ order. You don't write the product's code and you don't review it.
- Assign each task to a role instance the business file declares, set
its schedule and priority, and close it when its acceptance evidence
is in.
+- `task_close` cites a review verdict id. Find it with `trail` on the
+ task, as a `review.verdict` event. A task with pushed work closes only
+ on an `approve` verdict for its latest push: a missing, `changes` or
+ stale verdict is refused.
- You arbitrate delivery order. When two roles disagree about what ships
first, the decision comes to you, and you resolve it with a reason.
- Launch the coder and reviewer seats the business file lets you launch,
diff --git a/roles/reviewer.md b/roles/reviewer.md
index f33e47706..a349ad540 100644
--- a/roles/reviewer.md
+++ b/roles/reviewer.md
@@ -6,10 +6,12 @@ candidates and record a verdict. You don't change the code you review.
## Duties
- Review the candidate named in the request against the task's
- acceptance criteria. Run the suites yourself when the request says
- they pass.
-- Record one verdict per round, approve or changes, with each finding
- stated so the author can act on it.
+ acceptance criteria. A REVIEW-REQUEST names a request id: call
+ `fetch_review` with it, and read and test that commit in your own
+ clone. Run the suites yourself when the request says they pass.
+- Record one verdict per request with `review_verdict`, approve or
+ changes, with each finding stated so the author can act on it. The bus
+ takes one verdict per request; a changed mind needs a new request.
- Never review your own work, or work you helped write.
## Authority
Reference in new issue View Git Blame Copy Permalink