diff --git a/.agents/briefs/implement-fix.md b/.agents/briefs/implement-fix.md index 27e12101a..e19d03c71 100644 --- a/.agents/briefs/implement-fix.md +++ b/.agents/briefs/implement-fix.md @@ -29,10 +29,11 @@ mechanical. Inputs include immutable `base`, `merge-base`, `previous` and test; - a `ci:` line: the named test failed in CI; a line asking to merge the base branch: merge it and resolve the conflicts. - - a `stack:` line: bring the whole stack up to date with `gh stack -sync` (`gh stack rebase` where it stops on a conflict), resolve the - conflicts, push every branch it moved, and list each PR whose branch - needed a resolution. + - a `stack:` line naming a parent branch: in your worktree, `git merge +origin/` into the branch, resolve the conflicts, run the tests + of the files the resolution touched, commit the merge and push. Never + rebase, never force-push, never touch another branch: the orchestrator + carries the merge up the stack with `stack-sync.sh`. 3. Run only the commands under "Tests agents run" in `toolchain.md`. Never run a check listed under "Checks agents never run", nor a tool behind it, from any path: the commit and push hooks run them, and their errors show diff --git a/.agents/scripts/ci-failures.mjs b/.agents/scripts/ci-failures.mjs index 9cfc460e4..d14b03179 100644 --- a/.agents/scripts/ci-failures.mjs +++ b/.agents/scripts/ci-failures.mjs @@ -1,6 +1,6 @@ #!/usr/bin/env node -// `node .agents/scripts/ci-failures.mjs `: the tests that failed in the newest finished run -// of each workflow on the PR's branch. A push cancels the run before it, so while pushes come +// `node .agents/scripts/ci-failures.mjs >`: the tests that failed in the +// newest finished run of each workflow on the PR's branch, or on the named branch. A push cancels the run before it, so while pushes come // faster than CI the head's own checks stay pending and only an older run shows a failure. // // Prints `none`, or for each workflow whose newest finished run failed: ` run at @@ -9,8 +9,8 @@ import { execFileSync } from "node:child_process"; const pr = process.argv[2]; -if (pr === undefined) { - console.error("usage: ci-failures.mjs "); +if (pr === undefined || (pr === "--branch" && process.argv[3] === undefined)) { + console.error("usage: ci-failures.mjs >"); process.exit(2); } const gh = (...args) => @@ -22,7 +22,10 @@ const gh = (...args) => // GitHub's stored logs spell the escape byte as the two characters `^[`. const ANSI = new RegExp(`(?:${String.fromCharCode(27)}|\\^\\[)\\[[0-9;]*m`, "g"); -const branch = JSON.parse(gh("pr", "view", pr, "--json", "headRefName")).headRefName; +const branch = + pr === "--branch" + ? process.argv[3] + : JSON.parse(gh("pr", "view", pr, "--json", "headRefName")).headRefName; const runs = JSON.parse( gh( "run", diff --git a/.agents/scripts/ci-triage.mjs b/.agents/scripts/ci-triage.mjs new file mode 100644 index 000000000..78c63a4f9 --- /dev/null +++ b/.agents/scripts/ci-triage.mjs @@ -0,0 +1,98 @@ +#!/usr/bin/env node +// `node .agents/scripts/ci-triage.mjs `: sorts a PR's CI failures before anyone starts a fix +// run, so a failure the PR did not cause costs a rerun or a stack sync, not an implementer. +// +// For each failed test ci-failures.mjs reports on the PR's branch, one line ` `: +// +// flaky an open `flaky-test` issue names the test's file (testing rule 5): rerun the job +// base the same test failed in the newest finished run on the PR's base branch: the +// branch inherited it; sync the stack (stack-sync.sh) and let CI run again +// reach the test file, or a source file in its directory, is in the PR's diff: a fix run +// unknown none of the above: rerun the failed jobs once; a second failure gets a fix run +// +// Ends with `rerun ` lines for the runs whose failures are all flaky or unknown, and +// `none` when nothing failed. The repo is GH_REPO, else the origin remote's owner/name. +import { execFileSync } from "node:child_process"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const pr = process.argv[2]; +if (pr === undefined || !/^\d+$/.test(pr)) { + console.error("usage: ci-triage.mjs "); + process.exit(2); +} +const here = dirname(fileURLToPath(import.meta.url)); +const run = (cmd, args) => + execFileSync(cmd, args, { + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); +const nwo = + process.env.GH_REPO ?? + run("git", ["remote", "get-url", "origin"]) + .trim() + .replace(/\.git$/, "") + .replace(/.*[/:]([^/:]+\/[^/]+)$/, "$1"); +const api = (path, ...extra) => + JSON.parse(run("gh", ["api", "-X", "GET", `repos/${nwo}/${path}`, ...extra])); + +function failures(args) { + const out = run("node", [join(here, "ci-failures.mjs"), ...args]); + const runs = []; + for (const line of out.split("\n")) { + const head = /^(.+) run (\d+) at (\w+)$/.exec(line); + if (head) runs.push({ workflow: head[1], id: head[2], tests: [] }); + else if (/^\s+\S/.test(line) && runs.length > 0) runs[runs.length - 1].tests.push(line.trim()); + } + return runs; +} +const testFile = (test) => + /^((?:e2e|src|ui)\/[\w./-]+\.(?:test|spec)\.tsx?)/.exec(test)?.[1] ?? null; + +const pull = api(`pulls/${pr}`); +const mine = failures([pr]); +if (mine.length === 0) { + console.log("none"); + process.exit(0); +} +const onBase = new Set(failures(["--branch", pull.base.ref]).flatMap((r) => r.tests)); +const flakyIssues = api( + "issues", + "-f", + "labels=flaky-test", + "-f", + "state=open", + "-f", + "per_page=100", +); +const changed = new Set(); +for (let page = 1; page <= 3; page += 1) { + const files = api(`pulls/${pr}/files`, "-f", "per_page=100", "-f", `page=${page}`); + for (const f of files) changed.add(f.filename); + if (files.length < 100) break; +} +const reached = (file) => { + if (file === null) return false; + if (changed.has(file)) return true; + const dir = dirname(file); + return [...changed].some((f) => dirname(f) === dir && /\.tsx?$/.test(f)); +}; + +const reruns = []; +for (const r of mine) { + let needsFix = false; + for (const test of r.tests) { + const file = testFile(test); + const base = file === null ? "" : file.split("/").pop(); + let cls = "unknown"; + if (flakyIssues.some((i) => base !== "" && `${i.title}\n${i.body ?? ""}`.includes(base))) + cls = "flaky"; + else if (onBase.has(test)) cls = "base"; + else if (reached(file)) cls = "reach"; + if (cls === "reach" || cls === "base") needsFix = true; + console.log(`${cls} ${test} (${r.workflow} run ${r.id})`); + } + if (!needsFix) reruns.push(r.id); +} +for (const id of reruns) console.log(`rerun ${id}`); diff --git a/.agents/scripts/stack-sync.sh b/.agents/scripts/stack-sync.sh new file mode 100755 index 000000000..2206f5a77 --- /dev/null +++ b/.agents/scripts/stack-sync.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env sh +# Brings a feature's PR stack up to date by merging forward, bottom to top, never by rebasing: +# +# .agents/scripts/stack-sync.sh [--dry-run] +# +# The chain is read from GitHub: the top PR's base is the PR below, down to main. For each branch +# in that order, in a temporary detached worktree (never a task's worktree, never local main), +# origin/ is merged into origin/ and the result pushed as a fast-forward of the +# branch. A merge commit needs no force push, so no human has to push, and `settings.json` keeps +# denying `git push --force`. The first conflict stops the sync: it prints `conflict +# ` and the files, removes the worktree, and exits 3; the orchestrator sends that branch +# a fix run whose `stack:` line names the parent to merge, then runs this script again. Prints +# `synced ` or `current ` per branch. --dry-run prints the chain and exits. +# The repo is GH_REPO, else the origin remote's owner/name. +set -eu +top=${1:-} +dry=${2:-} +case $top in '' | *[!0-9]*) echo "usage: stack-sync.sh [--dry-run]" >&2; exit 64 ;; esac +here=$(cd "$(dirname "$0")" && pwd) +root=$(git rev-parse --show-toplevel) +nwo=${GH_REPO:-$(git remote get-url origin | sed -E 's#\.git$##; s#.*[/:]([^/:]+/[^/]+)$#\1#')} +say() { echo "stack-sync: $*" >&2; } + +# why: a cloud session's GitHub proxy refuses GraphQL, so the chain comes from REST, one PR at a time. +chain= +pr=$top +trunk= +while :; do + info=$(gh api -X GET "repos/$nwo/pulls/$pr" -q '"\(.head.ref) \(.base.ref) \(.state) \(.number)"') + set -- $info + [ "$3" = open ] || { say "PR #$4 is $3, not open"; exit 65; } + chain="$1 $chain" + case $2 in + main | master) trunk=$2; break ;; + esac + pr=$(gh api -X GET "repos/$nwo/pulls?head=${nwo%%/*}:$2&state=open" -q '.[0].number // empty') + [ -n "$pr" ] || { say "no open PR for branch $2 (the base of #$4); link the stack first"; exit 65; } +done +say "chain, bottom to top: $trunk -> $(echo "$chain" | sed 's/ $//; s/ / -> /g')" +[ "$dry" != --dry-run ] || exit 0 + +git fetch -q origin +parent=$trunk +for branch in $chain; do + if git merge-base --is-ancestor "origin/$parent" "origin/$branch"; then + echo "current $branch" + parent=$branch + continue + fi + wt=$(mktemp -d "${TMPDIR:-/tmp}/stack-sync.XXXXXX") + rmdir "$wt" + git worktree add -q --detach "$wt" "origin/$branch" + "$here/worktree.sh" --prepare "$wt" >/dev/null 2>&1 || true + if ! git -C "$wt" merge -q --no-edit "origin/$parent" >/dev/null 2>&1; then + files=$(git -C "$wt" diff --name-only --diff-filter=U | tr '\n' ' ') + git -C "$wt" merge --abort || true + git worktree remove --force "$wt" + echo "conflict $branch $parent $files" + exit 3 + fi + git -C "$wt" push -q origin "HEAD:refs/heads/$branch" + sha=$(git -C "$wt" rev-parse --short HEAD) + git worktree remove --force "$wt" + git fetch -q origin "$branch" + echo "synced $branch $sha" + parent=$branch +done diff --git a/.claude/skills/deliver/SKILL.md b/.claude/skills/deliver/SKILL.md index 2178410d1..9ee9951d3 100644 --- a/.claude/skills/deliver/SKILL.md +++ b/.claude/skills/deliver/SKILL.md @@ -146,14 +146,17 @@ they name; never build again or restart review at round 1. `open` in any report: decide, then tell (above). 2. **Review**: `reviewer` with the PR number; it picks the lenses (`docs/internal/REVIEW.md`). Before every review round - and every fix run, run `node .agents/scripts/ci-failures.mjs ` and - `gh pr view --json mergeable` once. The script reads the newest - finished CI run, which is often older than the head: newer pushes cancel - theirs. Each failed test no open `flaky-test` issue names joins the fix - run's `fixes` as `ci: run `, and a conflict as a line - asking to merge the base branch (`stack: gh stack sync` for a stacked - PR). With no fix run due, they get one - before the round. + and every fix run, run `node .agents/scripts/ci-triage.mjs ` and + `gh pr view --json mergeable` once. The triage reads the newest + finished CI run, which is often older than the head (newer pushes cancel + theirs), and sorts each failed test: `flaky` and `unknown` get its + `rerun ` lines run as `gh run rerun --failed`, no fix run; + `base` means the branch inherited the failure: run + `.agents/scripts/stack-sync.sh ` and let CI run again; only + `reach`, and `unknown` red a second time, joins the fix run's `fixes` + as `ci: run `. A conflict joins as `stack: merge +origin/`. With no fix run due, they get one before the + round. 3. **Decide** on the review report: - Incomplete assessment: preserve findings and missing scope; park. It is neither a defect nor a pass. @@ -288,14 +291,26 @@ Runnable: open, `task:ready`, unassigned, and every issue under its Depends on closed or a task of this feature whose PR passed review (open, not a draft). Check the Depends on yourself; no workflow promotes tasks. +- **Before the first build**: the feature's ADR branches fork from an old + `main`, so run `.agents/scripts/stack-sync.sh ` to merge + `main` forward into them, then start `reviewer` on each open ADR PR + (round 1, in the background) while the first task builds. An ADR PR + passes review the way a task's does (step 2, items 3 to 5, ADR-only + counts) and must be ready before the stack merges; its review never + waits for the gate to refuse. - A task's build stage gets `base: `; with no - PR yet, no `base` (it starts from `main`). -- Its review passed (step 2, item 6): `gh pr ready ` and `gh stack link - ` (PR numbers, bottom to top; none below yet: nothing to - link). When its branch did not start from the stack's top (another task - joined meanwhile), one `implementer` fix run with `fixes: stack: gh stack -sync` brings every branch up to date; a conflict it resolved gets a fix - check on that PR. PR body Status: `Gate: waiting for the stack`. + PR yet, no `base` (it starts from `main`). As soon as the build reports + its draft PR, `gh stack link ` (PR numbers, bottom to top; + none below yet: nothing to link): the stack is linked at build time, so + every later sync sees the whole chain. +- Its review passed (step 2, item 6): `gh pr ready `. When its branch + did not start from the stack's top (another task joined meanwhile), run + `.agents/scripts/stack-sync.sh ` yourself: it merges forward, + bottom to top, in temporary worktrees, and never rebases. It stops on + the first conflict (`conflict `): send that + task's branch one `implementer` fix run with `fixes: stack: merge +origin/`, then a fix check on that PR, then the script again. + PR body Status: `Gate: waiting for the stack`. - Deliver up to two runnable tasks at once, each in its own worktree. Start a stage for one as soon as the other's agent is running; never wait on one task to move the other. @@ -306,12 +321,11 @@ sync` brings every branch up to date; a conflict it resolved gets a fix stack stays open and nothing merges; the maintainer decides. **Merging the stack.** When every task has joined the stack and nothing is -in flight, run the gate on the top PR: `merge-pr.sh ` gates every PR -in the stack and merges them all, bottom to top. A conflict with `main` -(another feature merged meanwhile): one `implementer` fix run with `fixes: -stack: gh stack sync`, never one merge run per branch; a conflict it -resolved gets a fix check on that PR; then the gate again. Red CI: the fix -run goes to the lowest red PR, then the same sync carries it up. +in flight, run `.agents/scripts/stack-sync.sh ` once more (another +feature may have merged meanwhile), resolve a conflict as above, then the +gate on the top PR: `merge-pr.sh ` gates every PR in the stack and +merges them all, bottom to top. Red CI: the fix run goes to the lowest red +PR, then the same sync carries it up. Stop when nothing is runnable and nothing is in flight. diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..ea35dcd0e --- /dev/null +++ b/.gitattributes @@ -0,0 +1,4 @@ +# why: every feature adds rows to the ADR index; two features merging add different rows, and +# a union merge keeps both instead of stopping the stack sync on an add/add conflict. The +# src/adr-index.test.ts check keeps the table complete and in order. +docs/internal/adr/README.md merge=union diff --git a/docs/internal/DELIVERY.md b/docs/internal/DELIVERY.md index 1598e63d7..18e52ecad 100644 --- a/docs/internal/DELIVERY.md +++ b/docs/internal/DELIVERY.md @@ -104,8 +104,10 @@ What any change includes without its spec asking is listed once, in you. - **Merge.** Agents merge only through `.agents/scripts/merge-pr.sh` (delivery rule 15). A feature's PRs form one GitHub stack, its ADR PRs at - the bottom, and merge together, so `main` never holds half a feature. A - bug merges on its own. + the bottom, and merge together, so `main` never holds half a feature. + The stack is kept current by `.agents/scripts/stack-sync.sh`, which + merges forward bottom to top and never rebases, so no human has to + force-push. A bug merges on its own. - The same reviews run on a person's PR when you ask; the agent posts the findings as a comment and pushes nothing. diff --git a/docs/internal/adr/README.md b/docs/internal/adr/README.md index 200160649..3abbf11f0 100644 --- a/docs/internal/adr/README.md +++ b/docs/internal/adr/README.md @@ -60,6 +60,7 @@ the status is stale. | [0019](0019-startup-ends-every-lease-whose-device-is-not-running.md) | Startup ends every lease whose device is not running | Accepted; §1–2 narrowed by 0022 for physical devices; §4 narrowed by 0036 (a replay reads the current device) | | [0020](0020-a-requester-may-choose-its-lease-id.md) | A requester may choose its lease id, and the gateway passes it through bare | Accepted | | [0021](0021-a-gateway-dispatch-is-a-probe-and-names-its-fleet-request.md) | A gateway dispatch is a probe, and the gateway records every fleet request's outcome | Accepted — partly implemented: §5 (usage figures) is not built | +| [0022](0022-a-physical-device-is-a-second-device-type-the-core-knows.md) | A physical device is a second device type the core knows | Proposed; amends 0001 §3 and §5, 0015 §3 and 0019 §1–2 for physical devices | | [0023](0023-agent-device-is-served-by-an-optional-edge-module.md) | agent-device is served by an optional edge module, and the simlock lease is the only clock | Accepted; §1 narrowed by 0026 for gateway mode; §3 narrowed by 0030 (no reap while turned off) | | [0024](0024-leasing-calls-lease-end-participants-before-reclaim.md) | Leasing calls lease-end participants before it reclaims a device | Accepted | | [0025](0025-simlock-ships-android-profiles-and-a-model-pairs-only-with-images-that-fit.md) | Simlock ships Android device profiles, and a model pairs only with images that fit it | Accepted | diff --git a/docs/internal/agent-rules/delivery.md b/docs/internal/agent-rules/delivery.md index d76f01eba..385c64f1d 100644 --- a/docs/internal/agent-rules/delivery.md +++ b/docs/internal/agent-rules/delivery.md @@ -205,9 +205,14 @@ is open, done means closed as completed. is the one place a delivery PR is merged from; agents may not run `gh pr merge` themselves. A feature's PRs (its open ADR PRs, then each task's PR in the order its review passed) form one GitHub stack: each - PR targets the branch of the one below, stays open once reviewed, and - the stack merges once every task has joined it, so `main` never holds - part of a feature. The gate then checks every PR in the stack and + PR targets the branch of the one below, is linked into the stack as + soon as its draft PR exists, stays open once reviewed, and the stack + merges once every task has joined it, so `main` never holds part of a + feature. A stack moves by merging forward, bottom to top, through + `.agents/scripts/stack-sync.sh`, never by rebasing: a merge is a + fast-forward push, a rebase a force push nobody may make. The ADR + branch is merged up to `main` and its PR reviewed before the first + task builds. The gate then checks every PR in the stack and merges them together, all or nothing. A task depending on a reviewed task of the same feature starts on top of it: the issue-state workflow counts it as done. The script merges only a ready PR with no diff --git a/src/adr-index.test.ts b/src/adr-index.test.ts new file mode 100644 index 000000000..3c76f986e --- /dev/null +++ b/src/adr-index.test.ts @@ -0,0 +1,29 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +const ADR_DIR = join(dirname(fileURLToPath(import.meta.url)), "..", "docs", "internal", "adr"); + +// why: docs/internal/adr/README.md merges by union (.gitattributes), so two features' rows can +// land in any order and a row can go missing only when nobody wrote it; this proves the table. +describe("the ADR index", () => { + const files = readdirSync(ADR_DIR) + .filter((name) => /^\d{4}-.*\.md$/.test(name)) + .sort(); + const rows = readFileSync(join(ADR_DIR, "README.md"), "utf8") + .split("\n") + .map((line) => /^\| \[(\d{4})\]\(([^)]+)\) \|/.exec(line)) + .filter((match): match is RegExpExecArray => match !== null) + .map((match) => ({ number: match[1], file: match[2] })); + + it("has one row per ADR file, naming that file", () => { + expect(rows.map((row) => row.file).sort()).toEqual(files); + }); + + it("lists the rows in ascending order with no number twice", () => { + const numbers = rows.map((row) => row.number); + expect(numbers).toEqual([...new Set(numbers)].sort()); + }); +});