From 6f861a1c0035ba026876d09d374324340c43771d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 04:24:47 +0000 Subject: [PATCH 1/5] refactor(coord): tighten path-claims, dispatcher hooks, shell helpers + PR-workflow note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge cleanup on top of #142's squash-merge. No behavioural changes — 26/26 bridge tests stay green. * path-claims: cache segment-split per registered path so the n×m overlap scan inside register() stops re-splitting; collapse the O(n²) `//` normalisation while-loop into a single `\/+` regex; extract DEFAULT_TTL_S so register() and refresh() share one default; normalise the paths-array iteration into a small helper. * dispatcher: split the path-claims interception into explicit before/after hooks (pathClaimsBefore strips the bridge-only `paths` field before forwarding to the verified backend; pathClaimsAfter registers/refreshes/releases against the response), so the dispatch function reads as envelope-validate → before-hook → fetch → after-hook. * coord-hooks.sh: extract _coord_claim_quiet so coord-claim and coord-worktree share one parse path and interpret "granted" the same way; drop the duplicated python-parse blocks. * CLAUDE.md: add a "PR Workflow" section documenting the squash-merge + follow-up-commit hazard that produced PR #142's ghost-conflict, with the empty-diff diagnostic and the post-merge cleanup recipe. --- .claude/CLAUDE.md | 11 +++++ coord-tui/shell/coord-hooks.sh | 80 +++++++++++++++++++--------------- mcp-bridge/lib/dispatcher.js | 42 +++++++++--------- mcp-bridge/lib/path-claims.js | 71 +++++++++++++++++++----------- 4 files changed, 124 insertions(+), 80 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 5a5a518e..d4c10d8b 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -27,3 +27,14 @@ Adding to this list requires explicit user approval and an unblock condition. Au ### Documentation Format - All docs `.adoc` (AsciiDoc) except GitHub-required files (SECURITY.md, CONTRIBUTING.md, CODE_OF_CONDUCT.md, CHANGELOG.md). + +--- + +## PR Workflow + +This repo squash-merges PRs. Two consequences worth knowing before pushing follow-ups: + +- **Don't pile follow-up commits onto a branch whose PR is in review.** When the PR is squash-merged, `main` gets a new commit with a new SHA. Any commits you pushed after the PR was opened are still on the feature branch, on top of a base that no longer matches `main`. GitHub will then mark the PR as `mergeable_state: "blocked"` and any rebase will produce ghost-conflicts — the conflicting hunks are the *same content*, but git can't tell because the SHAs differ. If you have follow-up work, open it as a new PR off the current `main`. +- **After a squash-merge, delete the feature branch.** It contains pre-squash commits with stale SHAs; reusing it for new work re-creates the ghost-conflict problem. `git checkout main && git pull && git branch -D && git push origin --delete `. + +Diagnostic: if a PR shows `blocked` and `git diff origin/main HEAD` is empty, the PR's content is already on main via squash-merge — close the PR rather than trying to merge it. diff --git a/coord-tui/shell/coord-hooks.sh b/coord-tui/shell/coord-hooks.sh index a1a70383..bb277d7a 100644 --- a/coord-tui/shell/coord-hooks.sh +++ b/coord-tui/shell/coord-hooks.sh @@ -36,6 +36,27 @@ _coord_post() { -d "$payload" 2>/dev/null } +# Quiet claim — echoes the raw backend response, sets the return code +# (0 on success, 1 otherwise). Shared by coord-claim and coord-worktree +# so both interpret "granted" the same way. +_coord_claim_quiet() { + local task="$1" + local tok; tok="$(_coord_token)" + [ -z "$tok" ] && return 2 + local resp + resp=$(_coord_post coord_claim_task \ + "{\"token\":\"$tok\",\"task\":\"$task\"}" 2>/dev/null) + printf '%s' "$resp" + printf '%s' "$resp" | python3 -c " +import sys, json +try: + d = json.load(sys.stdin) + sys.exit(0 if d.get('success') else 1) +except Exception: + sys.exit(1) +" 2>/dev/null +} + _coord_auto_register() { local kind="$1" # Registers silently, writes ~/.cache/coord-tui/peer.env, sets window title. @@ -98,25 +119,27 @@ else: # Claim a task: coord-claim hypatia/my-task coord-claim() { local task="${1:?Usage: coord-claim }" - local tok; tok="$(_coord_token)" - if [ -z "$tok" ]; then - echo "Not registered — run: coord-tui --id --kind claude" >&2; return 1 + local resp rc + resp="$(_coord_claim_quiet "$task")"; rc=$? + if [ $rc -eq 2 ]; then + echo "Not registered — run: coord-tui --id --kind claude" >&2 + return 1 fi - local result - result=$(_coord_post coord_claim_task \ - "{\"token\":\"$tok\",\"task\":\"$task\"}" 2>/dev/null) - echo "$result" | python3 -c " -import sys, json + if [ -z "$resp" ]; then + echo " ✗ Failed (adapter not running?)" + return 1 + fi + printf '%s' "$resp" | TASK="$task" python3 -c " +import sys, os, json d = json.load(sys.stdin) +task = os.environ.get('TASK', '') if d.get('success'): msg = d.get('message','') - if msg == 'granted': - print(f' ✓ Claimed: $task') - else: - print(f' ✗ {msg}') + print(f' ✓ Claimed: {task}' if msg == 'granted' else f' ✗ {msg}') else: print(f' ✗ {d.get(\"error\",\"unknown error\")}') -" 2>/dev/null || echo " ✗ Failed (adapter not running?)" +" 2>/dev/null + return $rc } # Claim a task AND provision an isolated git worktree for it. @@ -149,28 +172,15 @@ coord-worktree() { local branch="agent/${peer}/${safe}" # Claim first — if the backend says no, don't touch the working tree. - local tok; tok="$(_coord_token)" - if [ -n "$tok" ]; then - local claim - claim=$(_coord_post coord_claim_task \ - "{\"token\":\"$tok\",\"task\":\"$task\"}" 2>/dev/null) - local ok - ok=$(echo "$claim" | python3 -c " -import sys, json -try: - d = json.load(sys.stdin) - print('yes' if d.get('success') else 'no') -except Exception: - print('no') -" 2>/dev/null) - if [ "$ok" != "yes" ]; then - echo " ✗ Claim refused — not provisioning worktree." >&2 - echo "$claim" >&2 - return 1 - fi - else - echo " ! No coord token — provisioning worktree without claim." >&2 - fi + local resp rc + resp="$(_coord_claim_quiet "$task")"; rc=$? + case $rc in + 0) ;; # granted + 2) echo " ! No coord token — provisioning worktree without claim." >&2 ;; + *) echo " ✗ Claim refused — not provisioning worktree." >&2 + [ -n "$resp" ] && echo "$resp" >&2 + return 1 ;; + esac mkdir -p "$(dirname "$wt_dir")" if [ -d "$wt_dir" ]; then diff --git a/mcp-bridge/lib/dispatcher.js b/mcp-bridge/lib/dispatcher.js index 741158a8..c9ad5a85 100644 --- a/mcp-bridge/lib/dispatcher.js +++ b/mcp-bridge/lib/dispatcher.js @@ -220,16 +220,7 @@ async function dispatchLocalCoord(toolName, args) { } } - // Advisory path-claims are bridge-layer only — strip from the - // payload forwarded to the verified Zig backend so its schema stays - // unchanged. Backend stays the source of truth for ownership. - let declaredPaths; - let forwarded = args || {}; - if (toolName === "coord_claim_task" && args && Array.isArray(args.paths)) { - declaredPaths = args.paths; - const { paths, ...rest } = args; - forwarded = rest; - } + const { forwarded, ctx } = pathClaimsBefore(toolName, args); try { const res = await fetch(`${LOCAL_COORD_URL}/tools/${toolName}`, { @@ -243,7 +234,7 @@ async function dispatchLocalCoord(toolName, args) { } catch { return { success: false, error: "local-coord-mcp backend returned non-JSON" }; } - return annotatePathClaims(toolName, args, data, declaredPaths); + return pathClaimsAfter(toolName, args, data, ctx); } catch (e) { return { success: false, @@ -253,27 +244,38 @@ async function dispatchLocalCoord(toolName, args) { } } -function annotatePathClaims(toolName, args, data, declaredPaths) { +// Path-claims live entirely at the bridge layer. The verified backend +// has no `paths` field on coord_claim_task, so we strip it before +// forwarding and stash it for the post-response annotate step. +function pathClaimsBefore(toolName, args) { + const a = args || {}; + if (toolName === "coord_claim_task" && Array.isArray(a.paths)) { + const { paths, ...rest } = a; + return { forwarded: rest, ctx: { declaredPaths: paths } }; + } + return { forwarded: a, ctx: {} }; +} + +function pathClaimsAfter(toolName, args, data, ctx) { if (!data || typeof data !== "object") return data; const task = args?.task; switch (toolName) { case "coord_claim_task": { - if (!declaredPaths || !task || data.success === false) return data; - const holder = data.holder || "(unknown)"; - const ttl_s = typeof data.ttl_s === "number" ? data.ttl_s : undefined; + if (!ctx.declaredPaths || !task || data.success === false) return data; const { paths, overlaps } = pathClaims.register({ - task, holder, paths: declaredPaths, ttl_s, + task, + holder: data.holder || "(unknown)", + paths: ctx.declaredPaths, + ttl_s: typeof data.ttl_s === "number" ? data.ttl_s : undefined, }); return { ...data, declared_paths: paths, path_overlap: overlaps }; } - case "coord_progress": { + case "coord_progress": if (task) pathClaims.refresh(task, data.ttl_s); return data; - } - case "coord_report_outcome": { + case "coord_report_outcome": if (task) pathClaims.release(task); return data; - } default: return data; } diff --git a/mcp-bridge/lib/path-claims.js b/mcp-bridge/lib/path-claims.js index a7a5769d..20bf803c 100644 --- a/mcp-bridge/lib/path-claims.js +++ b/mcp-bridge/lib/path-claims.js @@ -4,45 +4,62 @@ // Advisory path-claims for local-coord-mcp. // // The Zig/Idris backend enforces a task-id mutex; this module layers an -// in-bridge advisory map of `task -> {holder, paths, expires_at}` so +// in-bridge advisory map of `task -> {holder, segs, expires_at}` so // `coord_claim_task` can return `path_overlap` hints when two active // claims declare overlapping working-tree paths. Advisory by design: // the backend is the source of truth for ownership; this module never // rejects a claim, it only annotates the response. -const _claims = new Map(); // task -> { holder, paths, expires_at_ms } +const DEFAULT_TTL_S = 300; +const _claims = new Map(); // task -> { holder, paths, segs, expires_at_ms } function normalize(p) { if (typeof p !== "string") return null; let s = p.trim(); if (!s) return null; - s = s.replace(/\\/g, "/"); - while (s.includes("//")) s = s.replace(/\/\//g, "/"); - if (s.endsWith("/") && s.length > 1) s = s.slice(0, -1); + s = s.replace(/\\/g, "/").replace(/\/+/g, "/"); if (s.startsWith("./")) s = s.slice(2); - return s; + if (s.length > 1 && s.endsWith("/")) s = s.slice(0, -1); + return s || null; } -function segments(p) { +function toSegments(p) { return p.split("/").filter((s) => s !== "" && s !== "."); } -// Two paths overlap when one is a segment-prefix of the other (or equal). -// `src/a` overlaps `src/a/b` and `src/a`, but NOT `src/abc`. -export function pathsOverlap(a, b) { - const A = segments(a); - const B = segments(b); +function segmentPrefixMatch(A, B) { const n = Math.min(A.length, B.length); for (let i = 0; i < n; i++) if (A[i] !== B[i]) return false; return true; } +// Two paths overlap when one is a segment-prefix of the other (or equal). +// `src/a` overlaps `src/a/b` and `src/a`, but NOT `src/abc`. +// Kept as a stable public helper for tests and external use. +export function pathsOverlap(a, b) { + return segmentPrefixMatch(toSegments(a), toSegments(b)); +} + function sweep(nowMs = Date.now()) { for (const [task, entry] of _claims) { if (entry.expires_at_ms && entry.expires_at_ms <= nowMs) _claims.delete(task); } } +function ttlMs(ttl_s) { + return (typeof ttl_s === "number" && ttl_s > 0 ? ttl_s : DEFAULT_TTL_S) * 1000; +} + +function normalizePaths(paths) { + if (!Array.isArray(paths)) return []; + const out = []; + for (const p of paths) { + const n = normalize(p); + if (n) out.push(n); + } + return out; +} + /** * Register an advisory path-claim and return overlap warnings with * other *active* claims (excluding the same task by the same holder). @@ -56,32 +73,37 @@ function sweep(nowMs = Date.now()) { */ export function register({ task, holder, paths, ttl_s }) { sweep(); - const norm = Array.isArray(paths) - ? paths.map(normalize).filter(Boolean) - : []; + const norm = normalizePaths(paths); + const newSegs = norm.map(toSegments); + const overlaps = []; for (const [otherTask, other] of _claims) { if (otherTask === task && other.holder === holder) continue; - const hits = []; - for (const a of norm) { - for (const b of other.paths) { - if (pathsOverlap(a, b)) hits.push(a); + const hits = new Set(); + for (let i = 0; i < newSegs.length; i++) { + const a = newSegs[i]; + for (const b of other.segs) { + if (segmentPrefixMatch(a, b)) { + hits.add(norm[i]); + break; + } } } - if (hits.length) { + if (hits.size) { overlaps.push({ task: otherTask, holder: other.holder, paths: other.paths.slice(), - with: Array.from(new Set(hits)), + with: Array.from(hits), }); } } - const ttl = typeof ttl_s === "number" && ttl_s > 0 ? ttl_s : 300; + _claims.set(task, { holder, paths: norm, - expires_at_ms: Date.now() + ttl * 1000, + segs: newSegs, + expires_at_ms: Date.now() + ttlMs(ttl_s), }); return { paths: norm, overlaps }; } @@ -90,8 +112,7 @@ export function register({ task, holder, paths, ttl_s }) { export function refresh(task, ttl_s) { const entry = _claims.get(task); if (!entry) return false; - const ttl = typeof ttl_s === "number" && ttl_s > 0 ? ttl_s : 300; - entry.expires_at_ms = Date.now() + ttl * 1000; + entry.expires_at_ms = Date.now() + ttlMs(ttl_s); return true; } From 9bb7df5c7708efaf5d8907457e5fcdbd74b366b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 04:30:34 +0000 Subject: [PATCH 2/5] test(coord): add path-claims bench harness + just bench-bridge recipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measures register() (the hot path on coord_claim_task) across realistic (10 claims) → stress (1000 claims) population sizes, plus the pathsOverlap leaf primitive and bookkeeping (refresh, list). Bench holds active-claim count stable by reusing one task slot per iter, so the reported numbers reflect overlap-scan cost at the seeded population — not arithmetic-series accumulation. Reference numbers on dev host: 240k ops/s at 10 claims, 3.9k ops/s at 1000 claims, pathsOverlap ~170 ns/op. Numbers are host-dependent; use deltas across commits, not absolute values. --- Justfile | 4 + mcp-bridge/tests/path_claims_bench.js | 132 ++++++++++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 mcp-bridge/tests/path_claims_bench.js diff --git a/Justfile b/Justfile index e308ec0e..5ea928d4 100644 --- a/Justfile +++ b/Justfile @@ -354,6 +354,10 @@ bench: @echo "Running benchmarks..." cd ffi/zig && zig build bench +# Run the mcp-bridge JS benches (path-claims overlap scan, leaf primitives) +bench-bridge: + @node mcp-bridge/tests/path_claims_bench.js + # Run end-to-end integration tests integration: @echo "Running integration tests..." diff --git a/mcp-bridge/tests/path_claims_bench.js b/mcp-bridge/tests/path_claims_bench.js new file mode 100644 index 00000000..e6e69348 --- /dev/null +++ b/mcp-bridge/tests/path_claims_bench.js @@ -0,0 +1,132 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) +// +// Bench harness for mcp-bridge/lib/path-claims.js. +// +// Run: node mcp-bridge/tests/path_claims_bench.js +// Or: just bench-bridge +// +// Path-claims sit on every coord_claim_task call, so the overlap scan +// is on the hot path for multi-agent coordination. Real workloads are +// small (~10-50 active claims), but stress scenarios sanity-check the +// O(n) register scan + the cached segment-split optimisation from the +// post-#142 refactor. Output is stable so commits can be diffed. + +import { + register, + refresh, + release, + list, + pathsOverlap, + _reset, +} from "../lib/path-claims.js"; + +const NS_PER_MS = 1_000_000n; + +function bench(name, iters, fn) { + // Warmup: ~10% of iters or 1k, whichever is smaller. + const warm = Math.min(iters / 10 | 0, 1000); + for (let i = 0; i < warm; i++) fn(i); + const t0 = process.hrtime.bigint(); + for (let i = 0; i < iters; i++) fn(i); + const t1 = process.hrtime.bigint(); + const totalNs = t1 - t0; + const nsPerOp = Number(totalNs) / iters; + const opsPerSec = iters / (Number(totalNs) / 1e9); + return { name, iters, totalMs: Number(totalNs / NS_PER_MS), nsPerOp, opsPerSec }; +} + +function fmtRow({ name, iters, totalMs, nsPerOp, opsPerSec }) { + const ns = nsPerOp < 1000 + ? `${nsPerOp.toFixed(1)} ns/op` + : nsPerOp < 1_000_000 + ? `${(nsPerOp / 1000).toFixed(2)} µs/op` + : `${(nsPerOp / 1_000_000).toFixed(2)} ms/op`; + const ops = opsPerSec > 1e6 + ? `${(opsPerSec / 1e6).toFixed(2)}M ops/s` + : opsPerSec > 1e3 + ? `${(opsPerSec / 1e3).toFixed(1)}k ops/s` + : `${opsPerSec.toFixed(0)} ops/s`; + return ` ${name.padEnd(50)} ${String(iters).padStart(8)} iters ${String(totalMs).padStart(5)} ms ${ns.padStart(14)} ${ops.padStart(14)}`; +} + +function seedClaims(n, pathsPerClaim) { + _reset(); + for (let i = 0; i < n; i++) { + const paths = []; + for (let j = 0; j < pathsPerClaim; j++) { + paths.push(`pkg-${i}/mod-${j}/file-${i}-${j}.js`); + } + register({ task: `seed-${i}`, holder: `peer-${i % 5}`, paths, ttl_s: 3600 }); + } +} + +const SCENARIOS = [ + // Realistic: 10 active claims, declaring 3 paths each. Typical multi- + // agent workstation: 3-5 peers, 1-3 claims apiece. + { name: "register: 10 active claims, 3 new paths", seed: 10, declared: 3, iters: 50_000 }, + // Heavier: 100 claims. Sustained multi-day session. + { name: "register: 100 active claims, 3 new paths", seed: 100, declared: 3, iters: 20_000 }, + // Stress: 1000 claims. Beyond design point — measures graceful scaling. + { name: "register: 1000 active claims, 3 new paths", seed: 1000, declared: 3, iters: 5_000 }, + // Wide declared paths (rare but possible — a sweeping refactor). + { name: "register: 100 active claims, 20 new paths", seed: 100, declared: 20, iters: 5_000 }, +]; + +console.log("path-claims bench (node " + process.version + ")\n"); +console.log(" " + "scenario".padEnd(50) + " iters ms ns/op ops/s"); +console.log(" " + "-".repeat(110)); + +for (const s of SCENARIOS) { + seedClaims(s.seed, 3); + // Pre-build the declared-paths array once so we measure register(), + // not array construction. + const declared = []; + for (let j = 0; j < s.declared; j++) { + declared.push(`pkg-new/feature/file-${j}.js`); + } + // Reuse one task slot — each iter is an update against the same + // seeded population, so active-claim count stays at s.seed and the + // overlap-scan cost is what we actually want to measure. + const result = bench(s.name, s.iters, () => { + register({ + task: "hot-task", + holder: "peer-hot", + paths: declared, + ttl_s: 3600, + }); + }); + console.log(fmtRow(result)); +} + +// Leaf primitive — drives the inner loop in register(). Use long-ish +// paths to exercise the segment comparator under realistic depth. +const aLong = "cartridges/local-coord-mcp/abi/LocalCoord/Identity.idr"; +const bLong = "cartridges/local-coord-mcp/abi/LocalCoord/Auth.idr"; +const aShort = "src/foo"; +const bShort = "src/foo/bar/baz.js"; + +console.log(); +console.log(fmtRow(bench("pathsOverlap: deep diverge at segment 4", 1_000_000, () => { + pathsOverlap(aLong, bLong); +}))); +console.log(fmtRow(bench("pathsOverlap: short prefix match", 1_000_000, () => { + pathsOverlap(aShort, bShort); +}))); + +// refresh/release/list — bookkeeping that fires on coord_progress and +// coord_report_outcome. Measure to catch accidental regressions. +seedClaims(100, 3); +console.log(); +let r = 0; +console.log(fmtRow(bench("refresh (existing claim)", 100_000, () => { + refresh(`seed-${r++ % 100}`, 300); +}))); +seedClaims(100, 3); +let q = 0; +console.log(fmtRow(bench("list (100 active claims)", 50_000, () => { + list(); + q++; +}))); + +console.log("\n (Bench numbers depend on host; use deltas across commits, not absolute values.)"); From f1e646a2d625621bdd0033028ee576693ac7cf12 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 04:33:28 +0000 Subject: [PATCH 3/5] ci(bench): add bench-bridge job to e2e workflow with sticky PR comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs node mcp-bridge/tests/path_claims_bench.js on every PR/push that touches the bridge, uploads the output as an artifact, and posts a sticky PR comment (marker-tag find-or-update) so bench deltas are visible inline across pushes instead of buried in the artifact tarball. Separate job from the existing zig `benchmarks` so the JS bench gets its own runner (no Zig install) and the logs stay untangled. Per-job permissions override (pull-requests: write) keeps workflow-level permissions at read-all — only the comment step needs write. Comment failure is non-fatal (continue-on-error) so a token hiccup or fork PR never gates the bench. Same pattern as hypatia-scan's PR comment step. --- .github/workflows/e2e.yml | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 59e3d389..9ec27c09 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -139,3 +139,70 @@ jobs: name: benchmark-results path: ffi/zig/zig-out/bench* retention-days: 30 + + # ─── Bench Bridge: mcp-bridge JS perf (path-claims) ──────────────── + # Separate job from `benchmarks` above because the bridge has no Zig + # toolchain dependency — keeps logs untangled and lets the JS bench + # run in parallel on its own runner. Posts a sticky PR comment so + # deltas across pushes are visible inline. + bench-bridge: + name: Bench — mcp-bridge (path-claims) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + # Per-job override: only this step needs to write PR comments. + # Workflow-level permissions stay read-all. + pull-requests: write + + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Node + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: '22' + + - name: Run bridge bench + run: node mcp-bridge/tests/path_claims_bench.js | tee bench-bridge.txt + + - name: Upload bridge bench artifact + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: bench-bridge-results + path: bench-bridge.txt + retention-days: 30 + + - name: Sticky PR comment with bench numbers + if: github.event_name == 'pull_request' + # Advisory — a comment failure must never gate the bench job. + # Same reasoning as the hypatia-scan PR-comment step. + continue-on-error: true + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v7 + with: + script: | + const fs = require('fs'); + const MARKER = ''; + const output = fs.readFileSync('bench-bridge.txt', 'utf8'); + const body = `${MARKER}\n## 🏁 path-claims bench\n\nCommit \`${context.sha.slice(0, 7)}\`\n\n
Numbers\n\n\`\`\`\n${output}\n\`\`\`\n
\n\n*Host-dependent — compare deltas across commits, not absolute values.*`; + + const { owner, repo } = context.repo; + const issue_number = context.issue.number; + + const existing = await github.paginate( + github.rest.issues.listComments, + { owner, repo, issue_number, per_page: 100 }, + ); + const prior = existing.find((c) => c.body && c.body.includes(MARKER)); + + if (prior) { + await github.rest.issues.updateComment({ + owner, repo, comment_id: prior.id, body, + }); + } else { + await github.rest.issues.createComment({ + owner, repo, issue_number, body, + }); + } From 5e75c0502df11fb7c179dd9d1c3ab267ec5f986e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 04:38:35 +0000 Subject: [PATCH 4/5] docs(adr): spikes 0015 (backend file-lock) + 0016 (mTLS federation stop-gap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two short ADRs evaluating the remaining survey gaps so the implementation pick is made on documented tradeoffs rather than implementer instinct. * 0015 — Promote bridge-only path-claims to a backend-enforced lock primitive. New P-08 LockSoundness in Idris2 (constructive — segment- prefix mutual exclusion is structurally inductive). Closes the survey's file-lock gap completely. Cost: 2-4 days, but the first non-task-id concept entering the proved backend core. * 0016 — Cross-host federation stop-gap using mTLS + ed25519, sized to match Ruflo's posture. Explicitly positioned as a v1 for the ambitious ADR-0010 (DID + ML-DSA-87 + ML-KEM-1024 + federated quarantine), not a replacement. Loopback bus and Idris2 ABI untouched; cost: 4-6 days; trust bootstrap stays manual on purpose (SSH known_hosts model). Both end with a recommendation block. Verdict: 0016 closes the more user-visible gap and carries lower architectural risk; 0015 is the strongest technical answer but only worth it if "advisory warning is not enough" is a stated requirement. --- .../0015-backend-file-lock-primitive.md | 116 ++++++++++++++++ .../decisions/0016-mtls-federation-stopgap.md | 128 ++++++++++++++++++ 2 files changed, 244 insertions(+) create mode 100644 docs/decisions/0015-backend-file-lock-primitive.md create mode 100644 docs/decisions/0016-mtls-federation-stopgap.md diff --git a/docs/decisions/0015-backend-file-lock-primitive.md b/docs/decisions/0015-backend-file-lock-primitive.md new file mode 100644 index 00000000..137e710d --- /dev/null +++ b/docs/decisions/0015-backend-file-lock-primitive.md @@ -0,0 +1,116 @@ + + + +# 15. Backend-enforced file-lock primitive — spike + +Date: 2026-05-24 + +## Status + +Spike (comparing tradeoffs against ADR-0016; not yet a decision) + +## Context + +The multi-agent MCP survey identified that two comparator servers +(`rinadelph/Agent-MCP`, `AndrewDavidRivers/multi-agent-coordination-mcp`) +ship **hard file locks** at claim time, whereas `local-coord-mcp` has +only the bridge-layer **advisory** path-claims added in PR #142 / PR #143. +The survey marked file-level locks as a clear gap relative to those two. + +This spike evaluates promoting path-claims from "bridge-only advisory +warning" to a **backend-enforced lock primitive** in the verified Idris2 +ABI + Zig FFI. + +## What the change does + +1. Extend the `LocalCoord` Idris2 ABI with a new tool surface: + `coord_lock_paths(token, task, paths[])` and + `coord_unlock_paths(token, task)`. Paths are interned, normalised, and + the backend maintains an authoritative `task → segment[][]` map + alongside the existing claim map. +2. The lock check runs **inside** `coord_claim_task` when `paths` is + present: if any declared path segment-overlaps an existing locked + path held by another peer, the claim is **rejected** (not annotated). + Today's bridge-layer overlap scan becomes a projection of the + backend's authoritative state. +3. New proof obligation **P-08: LockSoundness** in + `cartridges/local-coord-mcp/abi/LocalCoord/Locks.idr`, discharged by + construction: + - **Mutual exclusion** — no two distinct tasks simultaneously hold + overlapping paths (segment-prefix-disjoint). + - **Lock-claim composition** — a granted claim's path-locks survive + until `coord_unlock_paths` or watchdog expiry; never silently + released by a different peer. + - **Watchdog interaction** — when the claim's role-based TTL fires + (P-03 WatchdogTermination), the lock-set is released atomically + with the claim. +4. Cartridge schema bump: `cartridge.json` adds the two new tools and + widens `coord_claim_task` to declare `path_locked` as a rejection + reason in its output schema. Coherence test (`dispatch_test.js`) + already enforces bridge↔cartridge sync, so the bridge tool list and + dispatcher must update in lockstep. + +## Cost + +| Surface | Work | +|---|---| +| Idris2 ABI | New `Locks.idr` module + P-08 discharge. Roughly 200-300 lines of Idris2; the segment-prefix proof reduces to list-prefix induction on `List String`, which is constructive (no new `believe_me`). | +| Zig FFI | `boj_coord_lock_paths` / `boj_coord_unlock_paths` exports + an internal `PathLockTable` (radix tree or sorted-array; flat array is fine at expected scale). ~150 LOC. | +| Bridge | Demote `path-claims.js` to a stateless projection — query the backend's lock table on demand rather than maintaining its own. Or delete it. | +| Cartridge schema | Two new tool entries; one output-shape widening on `coord_claim_task`. | +| Tests | New Idris2 totality proofs (P-08), Zig unit tests, bridge integration tests, bench update. | + +Estimated: 2-4 working days end-to-end. + +## Benefit + +- Closes the survey gap **completely**: backend-enforced locks rather + than an advisory layer. Strongest answer. +- Idris2 proof gives a guarantee neither Agent-MCP nor multi-agent-coord + has — they ship runtime-checked locks, we'd ship a *proved-correct* + lock primitive. Aligns with the AAA Formal posture (P-01..P-07). +- The bridge-layer code shrinks (path-claims.js largely becomes a + pass-through projection), which is a healthy direction. + +## Risks / open questions + +- **Architectural regression risk.** Today's design is explicit: the + verified backend stays minimal and the bridge layers advisory + features. Adding state to the backend (path-locks) is the first time + a non-task-id concept enters the proved core. P-08 must not weaken + the existing proofs — needs careful composition argument. +- **Schema-evolution lock-in.** Once `coord_lock_paths` is in the + cartridge manifest, removing it is a breaking change. Path-semantics + (segment-prefix matching) gets baked into the wire contract. +- **Defines vs. enforces.** What does the backend do when a *non-path* + claim conflicts with a locked path? The current advisory layer is + silent in that case; an enforcer must take a position. +- **Failure modes change.** Today a clashing path is a *warning*; under + this proposal it's a *claim rejection*. Existing rate-limit (5 + rejections / 10 min → 30s cooldown) applies; either way, agents need + retry logic. + +## Verdict + +Strongest technical answer to the survey gap, but the largest +architectural commitment in this branch of work. The proof load +(P-08) is tractable — segment-prefix mutual exclusion is structurally +inductive — but the *design* commitment (locks as a first-class backend +concept) deserves the user's explicit sign-off, not an implementer's +judgement call. + +**Recommendation:** Choose this only if "advisory warning is not +enough" is a stated requirement. Otherwise, prefer ADR-0016 +(cross-host federation stop-gap), which closes a different and arguably +more user-visible survey gap (Ruflo being the only comparator with real +cross-host story) without altering the verified core. + +## Out of scope + +- File-range locks (byte ranges within a file). Not addressed — paths + are still whole-file. The Perforce-style range lock would be a + separate, larger ADR. +- Lock fairness / queueing on contention. This spike rejects on + overlap; a queue would be a P-09 extension. +- Cross-host locks. Locks here are still localhost-bound; combining + with ADR-0010 or ADR-0016 is future work. diff --git a/docs/decisions/0016-mtls-federation-stopgap.md b/docs/decisions/0016-mtls-federation-stopgap.md new file mode 100644 index 00000000..1fefc03a --- /dev/null +++ b/docs/decisions/0016-mtls-federation-stopgap.md @@ -0,0 +1,128 @@ + + + +# 16. mTLS + ed25519 federation stop-gap — spike + +Date: 2026-05-24 + +## Status + +Spike (comparing tradeoffs against ADR-0015; not yet a decision; a +small-scope alternative to the full-fat ADR-0010) + +## Context + +The multi-agent MCP survey identified `ruvnet/ruflo` as the only +comparator shipping a real cross-host story (mTLS + ed25519 federation +across machines). `local-coord-mcp` is currently localhost-only +(`127.0.0.1:7745`) — closing this gap is one of the two outstanding +items from PR #143's spike. + +ADR-0010 already proposes the *ambitious* answer: DID-based identity, +ML-DSA-87 signing, ML-KEM-1024 key exchange, federated quarantine. It +sits at "Proposed (RFC)" status, tracked under epic #87 item 3, and is +a substantial cross-cartridge build effort. + +This spike evaluates a deliberately **smaller** stop-gap: ship +Ruflo-equivalent cross-host federation using mTLS + ed25519 *now*, +without blocking on the post-quantum work in ADR-0010. The two are +compatible: this becomes the v1 transport, ADR-0010 becomes the v2 +upgrade path. + +## What the change does + +1. **Transport.** Add a TLS-terminating endpoint alongside the loopback + bus — `https://:7746/coord/federated/inbox` for inbound + federated envelopes. Loopback bus (`:7745`) stays the default for + intra-machine traffic and is untouched. +2. **Identity.** Each peer generates an ed25519 keypair on first run + (stored under `~/.cache/coord-tui/peer.key`). The public key is the + peer's federated identity; the private key signs outbound envelopes. + No DID method, no PKI hierarchy — leaf-only trust. +3. **Peer trust.** A new file `~/.config/coord-tui/known_peers.toml` + maps peer-id → `{host, port, pubkey}`. Federation only works between + peers that have explicitly added each other (manual key exchange via + `coord-tui --print-pubkey` and a shared channel). No discovery; no + registry. +4. **mTLS.** Both ends of the federated link present client certs + derived from their ed25519 identity keys (SPKI-pinned, not + CA-signed). Connection fails closed if the presented pubkey doesn't + match `known_peers.toml`. +5. **Envelope wrapping.** Outbound coord messages destined for a remote + peer are signed (ed25519 over the canonical envelope bytes), wrapped + in `{from, to, signature, payload}`, sent over the mTLS connection. + Receiver verifies signature against `known_peers.toml`, then injects + into the local bus as if it had arrived locally. +6. **New tools.** `coord_add_peer(peer_id, host, port, pubkey)`, + `coord_remove_peer(peer_id)`, `coord_list_remote_peers()`. No tool + schema change to the existing `coord_send` / `coord_claim_task` + surface — federation is transport-layer. + +## Cost + +| Surface | Work | +|---|---| +| Zig adapter | TLS listener (use `std.crypto.tls`), ed25519 sign/verify (`std.crypto.sign.Ed25519` — already available), known_peers parser, envelope wrap/unwrap. ~400-600 LOC. | +| Idris2 ABI | New `Federation.idr` — but only proof obligations on the *envelope* boundary (signature verification soundness), not on the transport. ~150 lines; reuses existing `SafeHTTP` patterns. Class (J) believe_me may be unavoidable for `std.crypto.tls` opaque primitives — same axiom posture as the existing `Char`/`String` primitives. | +| Bridge | New 3 `coord_*` tool entries in `tools.js` + cartridge.json; dispatcher routes them to the backend. No envelope-shape changes. | +| coord-tui | `--print-pubkey` flag, `~/.config/coord-tui/known_peers.toml` reader, optional `coord-add-peer` shell helper. | +| Tests | Round-trip test on two backend instances on different ports; signature-tampering rejection test; replay-protection (envelope timestamp + nonce). | + +Estimated: 4-6 working days end-to-end. + +## Benefit + +- Closes the survey's "cross-host transport" gap directly: matches + Ruflo's posture, doesn't claim more. +- Additive — the existing loopback bus and Idris2-verified intra-host + ABI are untouched. No proof regression. +- Sets up ADR-0010 as a clean v2 upgrade (swap ed25519→ML-DSA-87, + add ML-KEM, add DID resolution) once that work is prioritised. + Wire format is the same shape; only the cryptographic primitives + change. +- Real user-visible feature — agents on Jonathan's workstation can + coordinate with agents on a build server, a sandbox VM, or a + teammate's machine. + +## Risks / open questions + +- **Trust bootstrap is manual.** Exchanging pubkeys via a shared + channel is the right answer for a v1 (it's how SSH known_hosts + works), but it does mean federation has a per-peer setup cost. ADR- + 0010's DID + cartridge-index resolution removes this; v1 doesn't. +- **No federated quarantine.** Tier-2+ envelopes from a remote peer + hit the *local* master for review — no upstream supervision model. + This is a feature gap relative to ADR-0010, not a bug, but worth + naming. +- **Crypto agility.** ed25519 in a post-quantum world has a finite + shelf life. Estate standards already commit to ML-DSA-87. This + spike's stop-gap framing is honest about that: v1 lasts until ADR- + 0010 lands, not forever. CHANGELOG and README should say so. +- **Idris2 transport-layer proofs.** TLS internals are opaque to the + Idris2 backend-assurance harness — same axiomatisation posture as + the existing 5 `believe_me` sites. Net new principled assumption, + not a regression. + +## Verdict + +Smaller scope than ADR-0015, lower architectural risk than ADR-0010, +and directly closes the only survey gap where `local-coord-mcp` +clearly trails a comparator. The "this is explicitly a stop-gap" +framing keeps the door open for ADR-0010 without making it a +blocker. + +**Recommendation:** Prefer this over ADR-0015 if the goal is "ship a +visible new capability that the survey identified as missing". +Combine with ADR-0015 only if file-locks are a stated user need; +otherwise ADR-0015 is gold-plating relative to the survey's actual +findings. + +## Out of scope + +- DID identity (ADR-0010, v2 upgrade). +- Post-quantum crypto (ADR-0010, v2 upgrade). +- Discovery / registry (deliberate — manual known_peers keeps the + trust story legible). +- Federated quarantine semantics (would land alongside DID work). +- Wire format negotiation for v1↔v2 transition — addressed when + ADR-0010 implementation starts. From a703c8530faa4c839040c9f4023c8651f3f34c6d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 04:43:28 +0000 Subject: [PATCH 5/5] =?UTF-8?q?docs(adr):=20flip=200016=20Spike=20?= =?UTF-8?q?=E2=86=92=20Accepted,=20mark=200015=20Deferred?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User picked 0016 over 0015 after reviewing both spikes. Flip status fields to match: * 0016 — Accepted with a 6-phase implementation plan (identity → sign/verify → mTLS transport → coord tools → Idris2 Federation.idr → tests). Each phase its own PR off main after this ADR lands. Total estimated 4-6 days. * 0015 — Deferred. Stays on file as the design-of-record for a backend-enforced lock primitive should the bridge-layer advisory layer prove insufficient. Reopen by flipping to "Proposed". --- .../0015-backend-file-lock-primitive.md | 11 +++++++++- .../decisions/0016-mtls-federation-stopgap.md | 20 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/docs/decisions/0015-backend-file-lock-primitive.md b/docs/decisions/0015-backend-file-lock-primitive.md index 137e710d..b96f4d7b 100644 --- a/docs/decisions/0015-backend-file-lock-primitive.md +++ b/docs/decisions/0015-backend-file-lock-primitive.md @@ -7,7 +7,16 @@ Date: 2026-05-24 ## Status -Spike (comparing tradeoffs against ADR-0016; not yet a decision) +Deferred (2026-05-24) — ADR-0016 (cross-host federation stop-gap) was +chosen as the next build over this one because it closes a more +user-visible survey gap without altering the verified backend core. +The bridge-layer advisory path-claims shipped in PR #142/#143 remain +the current answer for in-flight conflict signalling. + +This ADR stays on file as the design-of-record for a backend-enforced +lock primitive should "advisory warning is not enough" become a stated +requirement. Reopen by flipping to "Proposed" and scheduling alongside +the next P-0x proof-obligation cycle. ## Context diff --git a/docs/decisions/0016-mtls-federation-stopgap.md b/docs/decisions/0016-mtls-federation-stopgap.md index 1fefc03a..ec090b57 100644 --- a/docs/decisions/0016-mtls-federation-stopgap.md +++ b/docs/decisions/0016-mtls-federation-stopgap.md @@ -7,8 +7,24 @@ Date: 2026-05-24 ## Status -Spike (comparing tradeoffs against ADR-0015; not yet a decision; a -small-scope alternative to the full-fat ADR-0010) +Accepted (2026-05-24) — chosen over ADR-0015 because it closes the +more user-visible survey gap (Ruflo as the only comparator with a +cross-host story) and is additive: loopback bus and Idris2-verified +intra-host ABI stay untouched. Positioned as the v1 transport; +ADR-0010 (DID + ML-DSA-87 + ML-KEM-1024 + federated quarantine) +remains the v2 upgrade path. + +**Implementation plan:** phased rollout, each phase its own PR off +`main` after this ADR lands. + +| Phase | Scope | Estimated | +|---|---|---| +| 1 | Identity foundation — ed25519 keypair, pubkey export, `known_peers.toml` parser, `coord-tui --print-pubkey` flag. No transport. | ~1 day | +| 2 | Envelope sign/verify on the loopback bus (round-trip validation before any wire work). | ~1 day | +| 3 | TLS listener on `:7746` + mTLS handshake gated by `known_peers.toml`. | ~1-2 days | +| 4 | New `coord_add_peer` / `coord_remove_peer` / `coord_list_remote_peers` tools — bridge + cartridge.json sync. | ~0.5 day | +| 5 | Idris2 `Federation.idr` — envelope-boundary signature soundness obligation. | ~1 day | +| 6 | Tests — two-instance round-trip, signature tampering, replay protection. | ~0.5 day | ## Context