diff --git a/.changeset/gh-simulator-api-faults.md b/.changeset/gh-simulator-api-faults.md new file mode 100644 index 0000000..6d0baf4 --- /dev/null +++ b/.changeset/gh-simulator-api-faults.md @@ -0,0 +1,5 @@ +--- +"github-agent-workflow": patch +--- + +The `gh` simulator for tests is finished. `gh api` answers the REST calls the package makes (a user, an item's comments, editing a comment in place, a PR's files, repository contents and tree, the PR list and one PR, and `update-branch`), a world's `faults` make a matching call fail or let another run get in first, and `tests/lib.test.sh` now runs against the simulator instead of its own stub, so there is one mock of `gh`. The checker's effect model gains `comment_edit`, so a lease edited in place is read back by `diffWorlds`. diff --git a/AGENTS.md b/AGENTS.md index 42b06a4..6410dd2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ skills///SKILL.md exactly three levels deep **Every result a skill can return has a scenario** under `tests/scenarios//`: a world, the inputs, the expected result line and the net effects. `scripts/scenario.mjs` checks each against the skill's contract (scopes, invariants, eligibility oracles), and `npm run scenarios:coverage` lists results with none. A new result line without a scenario fails `npm test`. See `docs/adr/0012-scenarios.md`. -**Every `gh` command a skill uses is listed in `tests/sim/surface.mjs`.** The `gh` simulator (`tests/sim/bin/gh`) answers `gh` from a world file, and a test greps `skills/` and `scripts/` for `gh` calls. A new one fails `npm test` until it is listed as `supported`, `planned` or `out-of-scope`. Use only `--json` fields that `gh` has. See `docs/adr/0014-gh-simulator.md`. +**Every `gh` command a skill uses is listed in `tests/sim/surface.mjs`.** The `gh` simulator (`tests/sim/bin/gh`) answers `gh` from a world file, and a test greps `skills/` and `scripts/` for `gh` calls. A new one fails `npm test` until it is listed as `supported` (and the simulator answers it) or `out-of-scope`. Use only `--json` fields that `gh` has. `tests/lib.test.sh` runs against the same simulator, so there is one mock of `gh`. See `docs/adr/0014-gh-simulator.md`. ## Inputs and defaults diff --git a/docs/adr/0012-scenarios.md b/docs/adr/0012-scenarios.md index ec9eef3..1f738df 100644 --- a/docs/adr/0012-scenarios.md +++ b/docs/adr/0012-scenarios.md @@ -28,7 +28,7 @@ A **scenario** is a world, a skill and its inputs, and the expected outcome: one ## Consequences - A new result line, or a changed write scope, cannot land without a scenario. The old behaviour of the gap-9 bug (a conflicted PR skipped as `no-checks-yet`) is now a failing test. -- The same checker can judge a real run. `diffWorlds(before, after)` in `scripts/scenario.mjs` turns the change a run left behind into the effects the checker takes, and `applyEffects(before, diffWorlds(before, after))` gives `after` back (tested on every golden scenario and on generated worlds). A change no effect can express, such as a title or a body edit of an item that already existed, comes out as an `unmodelled` effect, which the checker rejects unless the contract lists an `unmodelled` write. What a new item is made with (title, body, creation time, author and, for a PR, branch) is part of creating it, so a `create_issue` or `create_pr` effect carries it as `data`, and a `close` carries its `reason`. The `gh` simulator over a world file is described in `0014-gh-simulator.md`. The part that is not built is a runner that drives a model against it. +- The same checker can judge a real run. `diffWorlds(before, after)` in `scripts/scenario.mjs` turns the change a run left behind into the effects the checker takes, and `applyEffects(before, diffWorlds(before, after))` gives `after` back (tested on every golden scenario and on generated worlds). A change no effect can express, such as a title or a body edit of an item that already existed, comes out as an `unmodelled` effect, which the checker rejects unless the contract lists an `unmodelled` write. What a new item is made with (title, body, creation time, author and, for a PR, branch) is part of creating it, so a `create_issue` or `create_pr` effect carries it as `data`, a `close` carries its `reason`, and a comment edited in place is a `comment_edit` (its index among the item's comments, and its new text). The `gh` simulator over a world file is described in `0014-gh-simulator.md`. The part that is not built is a runner that drives a model against it. ## What this does not prove diff --git a/docs/adr/0014-gh-simulator.md b/docs/adr/0014-gh-simulator.md index 3b29e47..268f189 100644 --- a/docs/adr/0014-gh-simulator.md +++ b/docs/adr/0014-gh-simulator.md @@ -2,13 +2,13 @@ ## Status -Accepted. The read paths (#86) and the write paths (#87) are built. `api`, fault injection and the replacement of the stub in `tests/lib.test.sh` (#88) follow, and this record is updated with each. +Accepted and built: the read paths (#86), the write paths (#87), and `api`, fault injection and the replacement of the old stub in `tests/lib.test.sh` (#88). ## Context ADR 0012 checks a scenario against a skill's contract, and `diffWorlds` turns the change a run left behind into the effects the checker takes. What was missing is something to run a skill against. A maintainer who wants to see whether a model follows `shepherd-github-pr` has to point it at a real repository, cannot repeat the run, and cannot make a merge fail or a check hang. -`tests/lib.test.sh` has a stub `gh` for the shell helpers. It covers only what `lib.sh` calls, and it ignores which `--json` fields are asked for. That let `prs_for_issue` ask `gh` for a field it does not have (#89). +`tests/lib.test.sh` had a stub `gh` for the shell helpers. It covered only what `lib.sh` calls, and it ignored which `--json` fields are asked for. That let `prs_for_issue` ask `gh` for a field it does not have (#89). ## Decision @@ -37,16 +37,49 @@ ADR 0012 checks a scenario against a skill's contract, and `diffWorlds` turns th - **`issue reopen` is `unmodelled`.** No effect op reopens an item, so a run that reopens one needs a contract that allows it. Editing a title or body of an existing item is `unmodelled` as well. - **A PR is created from the checkout.** Its branch is `--head` or the current git branch, and its commit is that branch's, so `gh pr create` runs in a git repository. The issues it closes are the existing ones its body names with a closing keyword. +### `api` (#88) + +`gh api` answers the nine REST calls the package makes, and refuses everything else as `unsupported` (graphql, another method on a path, `-F @file`, `--input`): + +| call | answered from | +| --- | --- | +| `GET user` | the world's agent login | +| `GET repos/{owner}/{repo}/issues/N/comments` | the item's comments (an issue's or a PR's), as REST comments with `author_association`, `user.login` and `updated_at` | +| `PATCH repos/{owner}/{repo}/issues/comments/ID -f body=` | edits a stored comment in place | +| `GET .../pulls/N/files` | the PR's `files` (a rename is `{path, previous}`, which gives `previous_filename`) | +| `GET .../contents/PATH?ref=` | the world's `files` (raw with the raw `Accept` header, otherwise a base64 object) | +| `GET .../git/trees/HEAD?recursive=1` | the paths of the world's `files` | +| `GET .../pulls?state=&per_page=&page=` and `GET .../pulls/N` | the world's PRs as REST pull requests, newest first. `author_association` is the PR's `association` (default `NONE`) | +| `PUT .../pulls/N/update-branch -f expected_head_sha=` | gives a `BEHIND` PR a new head and makes it `CLEAN`. A stale sha, a conflict and a PR that is not behind are 422, as on GitHub | + +- **A comment edited in place is an effect.** Since the lease comment of a step is edited rather than added, the checker's effect model gained `comment_edit` (the comment's index and its new text), scoped like `comment`. `diffWorlds` reads an edited comment as one, and `applyEffects` makes it. +- **A comment the simulator derives from a world field (a `lease`, a `readySha`) cannot be edited.** There is no stored comment to change, so the call is `unsupported` and `claim` falls back to posting a comment, as it does when an edit fails. +- **Pagination is not modelled** beyond `per_page` and `page` on the PR list. `--paginate` returns a list whole. + +### Faults (#88) + +A world (and so a scenario's `world:`) may carry `faults: [{match, fail, exit?, before?, times?}]`. + +- A call that starts with `match` (whole words: `pr merge`, `api -X PATCH`, `api user`) fails with the message `fail` and the exit code `exit` (default 1), writes nothing, and is logged with `kind: "fault"`, the index of the fault and a `faultId`. +- `before` is a list of effects applied to the world first. With `fail` it is a failure after something changed. Without `fail` the call goes on, which is how another run gets in ahead of this one (the creation race of `log_issue`). A scenario file is read-only, so `before` needs a world file. +- `times` limits how often a fault fires. Firing is counted in the log, under the fault's definition (`faultId`, a hash of it), so `times` needs `GH_SIM_LOG`. A different fault that is later at the same place in the list starts fresh, and the same fault written again does not. +- Faults are configuration, not state: they are the same in the world a run starts from and the one it leaves, so `diffWorlds` does not see them, and a scenario that carries them is judged as it was (tested on every golden scenario). + +### One mock (#88) + +`tests/lib.test.sh` runs against this simulator and nothing else. It sets a world up with a small `w` (jq over the world file) and a few helpers, runs the functions under test, and reads the result back through `gh`. The old stub is gone. Making the tests as strict as `gh` showed three things the stub let through: the `authorAssociation` field (#89), a repository with a label catalog refuses labels it does not have (the tests now start from the catalog `setup-github-agent-workflow` leaves), and issues and PRs share one number sequence (so a log issue made after PRs 70 to 74 is number 75). The last test of the file fails if any call in the run was `unsupported`, so a helper that swallows the failure of a call the simulator does not cover is still noticed. + ## What it does not do - **It is not GitHub.** Rate limits, pagination, webhooks, merge queues, branch protection and reviews are not modelled. A merge is blocked by the merge state a world gives, nothing else. Some error messages are what `gh` prints and some are close to it (a merge of a closed PR, an unreadable body file). - **Some world fields cannot be shown.** `entries`, `kind`, `openQuestions`, `useCase` and `wontfix` stand for facts a model would read from prose (a log entry, a body), not for GitHub state. `seed.mjs` says which ones a world has. A scenario that depends on them needs a runner to write the prose (#61). - **A world has no branch names, check names or logs.** A PR is on `feat/-pr-`, which matches the pattern `prs_for_issue` reads, unless it names a `branch`. A check is `check-`. The log of a failing run is one fixed line. - **Its field lists are a copy.** They come from one `gh` version and can fall behind. +- **It is slow next to a shell function.** Each call starts Node (about 75 ms, half of it loading `yaml` through the checker). `tests/lib.test.sh` makes about 390 calls and takes about 45 seconds, where the stub took 10. A compile cache saved little (12 ms a call), so it is not used. ## Where the simulator and the skills disagree Following `gh` exactly shows two places where the package assumes something `gh` does not do. -- `lib.sh` asks for `authorAssociation` on a PR, which is not a PR field in any `gh` release (#89). +- `lib.sh` asked for `authorAssociation` on a PR, which is not a PR field in any `gh` release. Fixed in #89, which the simulator made visible. - The shepherd skill's check-state one-liner expects `gh pr checks N --json bucket` to print `[]` for a PR with no checks, and so to answer `no-checks-yet`. `gh` instead exits 1 with `no checks reported on the '' branch` and prints nothing. The skill also says the exit code is non-zero while checks run, which is true without `--json` (8 while pending) and false with it (always 0). The simulator does what `gh` does, and a test records the difference. diff --git a/scripts/scenario.mjs b/scripts/scenario.mjs index 6bf8634..346ad62 100644 --- a/scripts/scenario.mjs +++ b/scripts/scenario.mjs @@ -20,7 +20,7 @@ export const MUTEX_PR = ["agent:needs-work", "agent:ready-for-review", "agent:hu export const LEASE_LABELS = ["agent:triage", "agent:in-progress"]; const REPOSITORY_OPS = ["push", "merge", "delete_branch", "create_pr", "label_create", "label_update", "write_file"]; // `unmodelled` is not something a skill does: diffWorlds reports a change it cannot express with it. -const OPS = ["label_add", "label_remove", "comment", "close", "push", "merge", "delete_branch", "create_pr", "create_issue", "review_request", "label_create", "label_update", "write_file", "unmodelled"]; +const OPS = ["label_add", "label_remove", "comment", "comment_edit", "close", "push", "merge", "delete_branch", "create_pr", "create_issue", "review_request", "label_create", "label_update", "write_file", "unmodelled"]; // ---- repository state: the label set and the agent instructions file ------------------------------ // A world may hold `labels: [{name, description}]` (the repository's label set) and `files: {path: text}`. @@ -185,7 +185,7 @@ export const ORACLES = { export function scopeOf(effect, ctx) { if (effect.op === "unmodelled") return "unmodelled"; if (REPOSITORY_OPS.includes(effect.op)) return "repository"; - if (effect.op === "comment") { + if (effect.op === "comment" || effect.op === "comment_edit") { const t = item(ctx.world, effect.target); if (t && (t.labels ?? []).includes("agent:log")) return "shared"; } @@ -236,6 +236,13 @@ export function applyEffects(world, effects) { if (!t.labels.includes(e.label)) throw new Error(`removes \`${e.label}\` from ${e.target}, which does not carry it`); t.labels = t.labels.filter((l) => l !== e.label); break; case "comment": t.comments.push({ marker: e.marker, text: e.text }); break; + // A comment the run edited in place (a lease): `index` is its place among the item's comments, `text` its new text. + case "comment_edit": { + const c = t.comments[e.index]; + if (!c) throw new Error(`\`comment_edit\` on ${e.target}: it has no comment ${e.index}`); + c.text = e.text; + break; + } case "close": t.state = "closed"; if (e.reason) t.closeReason = e.reason; break; case "push": t.head = e.sha ?? `${t.head}+`; t.pushed = true; break; case "merge": t.state = "merged"; break; @@ -308,8 +315,13 @@ export function diffWorlds(before, after) { const bc = b.comments ?? [], ac = a.comments ?? []; const same = (x, y) => x.marker === y.marker && x.text === y.text; - if (bc.length > ac.length || bc.some((c, i) => !same(c, ac[i]))) odd(target, "comments", bc, ac); - else for (const c of ac.slice(bc.length)) out.push({ op: "comment", target, ...defined({ marker: c.marker, text: c.text }) }); + // A comment that keeps its marker and has new text was edited in place. Any other change to the comments is `unmodelled`. + const edited = (x, y) => y !== undefined && !same(x, y) && x.marker === y.marker && typeof y.text === "string"; + if (bc.length > ac.length || bc.some((c, i) => !same(c, ac[i]) && !edited(c, ac[i]))) odd(target, "comments", bc, ac); + else { + bc.forEach((c, i) => { if (!same(c, ac[i])) out.push({ op: "comment_edit", target, index: i, text: ac[i].text }); }); + for (const c of ac.slice(bc.length)) out.push({ op: "comment", target, ...defined({ marker: c.marker, text: c.text }) }); + } if (a.state !== b.state) { const open = b.state === "open" || b.state === undefined; diff --git a/tests/diff-worlds.test.mjs b/tests/diff-worlds.test.mjs index 70407f6..9b99890 100644 --- a/tests/diff-worlds.test.mjs +++ b/tests/diff-worlds.test.mjs @@ -118,6 +118,23 @@ test("a title or body edited after the item exists is still unmodelled, and a cr assert.deepEqual(unmodelled(W(), edit((w) => { w.issues.push({ number: 3, state: "open", labels: [], comments: [], title: "T", assignee: "x" }); })), ["issue#3 assignee"]); }); +test("a comment edited in place is a comment_edit, and it round-trips", () => { + const before = edit((w) => { w.issues[0].comments = [{ marker: "agent:lease", text: "Claimed " }, { author: "human" }]; }); + const after = edit((w) => { w.issues[0].comments = [{ marker: "agent:lease", text: "Claimed again " }, { author: "human" }]; }); + const diff = diffWorlds(before, after); + assert.deepEqual(diff, [{ op: "comment_edit", target: "issue#1", index: 0, text: "Claimed again " }]); + assert.ok(sameWorld(applyEffects(before, diff), after)); + // an edit and a new comment in one run + const both = edit((w) => { w.issues[0].comments = [{ marker: "agent:lease", text: "x" }, { author: "human" }, { marker: "agent:outcome", text: "done" }]; }); + assert.deepEqual(diffWorlds(before, both).map((e) => [e.op, e.index ?? e.marker]), [["comment_edit", 0], ["comment", "agent:outcome"]]); + assert.ok(sameWorld(applyEffects(before, diffWorlds(before, both)), both)); + // a comment whose marker changed, or that lost its text, is still not an edit + assert.deepEqual(unmodelled(before, edit((w) => { w.issues[0].comments = [{ marker: "agent:outcome", text: "x" }, { author: "human" }]; })), ["issue#1 comments"]); + assert.deepEqual(unmodelled(before, edit((w) => { w.issues[0].comments = [{ marker: "agent:lease" }, { author: "human" }]; })), ["issue#1 comments"]); + assert.deepEqual(unmodelled(before, edit((w) => { w.issues[0].comments = [{ marker: "agent:lease", text: "Claimed " }]; })), ["issue#1 comments"], "a removed comment is not an edit"); + assert.throws(() => applyEffects(before, [{ op: "comment_edit", target: "issue#1", index: 5, text: "x" }]), /has no comment 5/); +}); + test("a close carries its reason, and the reason is not a change of its own", () => { const after = edit((w) => { w.issues[0].state = "closed"; w.issues[0].closeReason = "not planned"; }); const diff = diffWorlds(W(), after); @@ -131,7 +148,8 @@ test("unmodelled: fields, comments, state, branches, removals, new items and the assert.deepEqual(unmodelled(W(), edit((w) => { w.prs[0].checks = ["fail"]; })), ["pr#10 checks"]); assert.deepEqual(unmodelled(W(), edit((w) => { w.prs[0].draft = true; })), ["pr#10 draft"]); assert.deepEqual(unmodelled(W(), edit((w) => { w.prs[0].base = "develop"; })), ["pr#10 base"]); - assert.deepEqual(unmodelled(W(), edit((w) => { w.issues[0].comments[0].text = "edited"; })), ["issue#1 comments"]); + assert.deepEqual(unmodelled(W(), edit((w) => { w.issues[0].comments[0].text = "edited"; })), [], "an edited comment is a comment_edit now"); + assert.deepEqual(diffWorlds(W(), edit((w) => { w.issues[0].comments[0].text = "edited"; })).map((e) => e.op), ["comment_edit"]); assert.deepEqual(unmodelled(W(), edit((w) => { w.issues[0].comments = []; })), ["issue#1 comments"]); assert.deepEqual(unmodelled(edit((w) => { w.issues[0].state = "closed"; }), W()), ["issue#1 state"]); assert.deepEqual(unmodelled(W(), edit((w) => { w.issues[0].state = "merged"; })), ["issue#1 state"]); diff --git a/tests/gh-sim.test.mjs b/tests/gh-sim.test.mjs index 050539a..b0b19fc 100644 --- a/tests/gh-sim.test.mjs +++ b/tests/gh-sim.test.mjs @@ -12,7 +12,8 @@ import { SURFACE, ghCommandsIn } from "./sim/surface.mjs"; import { HANDLERS, run } from "./sim/gh-sim.mjs"; import { loadWorld } from "./sim/world.mjs"; import { seed } from "./sim/seed.mjs"; -import { checkState, applyEffects, diffWorlds, sameWorld, canonicalWorld, checkScenario } from "../scripts/scenario.mjs"; +import { checkState, applyEffects, diffWorlds, sameWorld, canonicalWorld, checkScenario, expandScenario } from "../scripts/scenario.mjs"; +import { contractOf } from "../scripts/scenario-coverage.mjs"; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const GH = join(ROOT, "tests/sim/bin/gh"); @@ -396,7 +397,7 @@ function run2(world, { file = `w-${++n}.yaml`, env = {} } = {}) { const path = join(DIR, file); writeFileSync(path, /\.json$/.test(file) ? JSON.stringify(world) : stringify(world)); const e = { ...process.env, GH_SIM_WORLD: path, GH_SIM_LOG: join(DIR, `log-${path.replace(/\W/g, "")}.jsonl`), GH_SIM_NOW: "2026-11-09T12:00:00Z", ...env }; - const call = (...argv) => { const r = spawnSync(GH, argv, { encoding: "utf8", env: e }); return { out: r.stdout, err: r.stderr, code: r.status }; }; + const call = (...argv) => { const r = spawnSync(GH, argv, { encoding: "utf8", env: e }); return { out: r.stdout, err: r.stderr, code: r.status, json: () => JSON.parse(r.stdout) }; }; return { path, env: e, call, read: () => parse(readFileSync(path, "utf8")), bytes: () => readFileSync(path, "utf8") }; } @@ -725,6 +726,275 @@ test("the effects of every golden scenario, made as gh calls, leave the world th assert.ok(empty >= 40, `${empty} scenarios have no effects`); }); +// ---- api ------------------------------------------------------------------------------------------------------ + +const W3 = () => ({ + repo: "acme/widgets", + files: { "AGENTS.md": "rules\n", ".github/CODEOWNERS": "* @me\n", "src/a.js": "x" }, + issues: [{ number: 5, state: "open", title: "Thing", labels: [], lease: "live", comments: [{ author: "agent", marker: "agent:lease", text: "Claimed " }, { author: "carol", trusted: true, text: "hi" }] }], + prs: [ + { number: 9, state: "open", head: "h1", base: "main", mergeState: "BEHIND", files: ["a.md", "b.md"], author: "dev", association: "MEMBER", body: "", labels: ["bug"], createdAt: "2026-11-02T00:00:00Z", comments: [] }, + { number: 10, state: "merged", head: "h2", files: [], closes: [5], createdAt: "2026-11-03T00:00:00Z", branch: "bugfix/5-x" }, + { number: 11, state: "closed", head: "h3", createdAt: "2026-11-01T00:00:00Z", draft: true }, + ], +}); +const API = (path) => `repos/{owner}/{repo}/${path}`; + +test("api user is the agent", () => { + assert.equal(run2(W3()).call("api", "user", "--jq", ".login").out, "agent-bot\n"); + assert.equal(run2({ ...W3(), user: "bot" }).call("api", "user", "--jq", ".login").out, "bot\n"); +}); + +test("api lists an item's comments as the REST API does, derived ones first, for an issue and for a PR", () => { + const r = run2(W3()); + const rows = r.call("api", API("issues/5/comments"), "--paginate", "--jq", '.[] | [.id, .user.login, .author_association, (.body | length > 0), .created_at == .updated_at] | @tsv').out.trim().split("\n"); + assert.deepEqual(rows, ["5001\tagent-bot\tNONE\ttrue\ttrue", "5002\tagent-bot\tNONE\ttrue\ttrue", "5003\tcarol\tOWNER\ttrue\ttrue"]); + const [c] = r.call("api", API("issues/5/comments")).json(); + assert.deepEqual(Object.keys(c).sort(), ["author_association", "body", "created_at", "html_url", "id", "issue_url", "node_id", "updated_at", "url", "user"]); + assert.equal(c.html_url, "https://github.com/acme/widgets/issues/5#issuecomment-5001"); + assert.equal(c.url, "https://api.github.com/repos/acme/widgets/issues/comments/5001"); + assert.equal(r.call("api", API("issues/9/comments"), "--jq", "length").out, "0\n", "a PR's comments are issue comments"); + assert.equal(r.call("api", API("issues/99/comments")).err, "gh: Not Found (HTTP 404)\n"); + assert.equal(r.call("api", "repos/other/repo/issues/5/comments").err, "gh: Not Found (HTTP 404)\n"); + assert.equal(r.call("api", API("issues/5/comments"), "--jq", ".[0].body | @json").out.startsWith('""); + assert.equal(out.code, 0, out.err); + const c = JSON.parse(out.out); + assert.deepEqual([c.id, c.body.slice(0, 14), c.updated_at], [5002, "Claimed again ", "2026-11-09T12:00:00Z"]); + const w = r.read(); + assert.deepEqual(w.issues[0].comments[0], { author: "agent", marker: "agent:lease", text: "Claimed again ", updatedAt: "2026-11-09T12:00:00Z" }); + assert.equal(w.issues[0].comments.length, 2, "edited, not added"); + assert.deepEqual(diffWorlds(before, w), [{ op: "comment_edit", target: "issue#5", index: 0, text: w.issues[0].comments[0].text }]); + assert.ok(sameWorld(applyEffects(before, diffWorlds(before, w)), w)); + assert.match(r.call("api", API("issues/5/comments"), "--jq", ".[1].updated_at").out, /^2026-11-09T12:00:00Z$/m); + const other = r.call("api", "-X", "PATCH", API("issues/comments/5003"), "-f", "body=changed by the agent"); + assert.equal(other.code, 0, "the simulator does not model who may edit a comment"); +}); + +test("PATCH of a comment that does not exist, without a body, or that is a view of a world field, fails", () => { + const r = run2(W3()), before = r.bytes(); + assert.equal(r.call("api", "-X", "PATCH", API("issues/comments/5999"), "-f", "body=x").err, "gh: Not Found (HTTP 404)\n"); + assert.equal(r.call("api", "-X", "PATCH", API("issues/comments/77001"), "-f", "body=x").err, "gh: Not Found (HTTP 404)\n"); + assert.equal(r.call("api", "-X", "PATCH", API("issues/comments/5002")).err, "gh: Validation Failed (HTTP 422)\n"); + const derived = r.call("api", "-X", "PATCH", API("issues/comments/5001"), "-f", "body=x"); + assert.match(derived.err, /^unsupported: api PATCH of a comment the simulator derives from a world field/); + assert.equal(r.bytes(), before); + const sc = run2({ skill: "s", world: W3(), expect: {} }); + const refused = sc.call("api", "-X", "PATCH", API("issues/comments/5002"), "-f", "body=x"); + assert.equal(refused.code, 2); + assert.match(refused.err, /scenario file/); +}); + +test("api pulls/N/files lists a PR's files, and a PR that is not there is a 404", () => { + const r = run2(W3()); + assert.equal(r.call("api", API("pulls/9/files"), "--paginate", "--jq", ".[] | .filename, (.previous_filename // empty)").out, "a.md\nb.md\n"); + assert.equal(r.call("api", API("pulls/10/files")).out, "[]\n"); + assert.equal(r.call("api", API("pulls/99/files")).err, "gh: Not Found (HTTP 404)\n"); + assert.deepEqual(Object.keys(r.call("api", API("pulls/9/files")).json()[0]).sort(), ["additions", "changes", "deletions", "filename", "sha", "status"]); +}); + +test("a PR's files can name the file a rename came from, and a comment can carry its own association", () => { + const r = run2({ prs: [{ number: 3, state: "open", files: ["a.md", { path: "docs/b.md", previous: "scripts/old.sh" }] }], issues: [{ number: 1, comments: [{ author: "dev", association: "MEMBER", text: "x" }, { author: "dev", trusted: true, association: "NONE", text: "y" }, { author: "dev", trusted: true, text: "z" }] }] }); + assert.equal(r.call("api", API("pulls/3/files"), "--jq", ".[] | .filename, (.previous_filename // empty)").out, "a.md\ndocs/b.md\nscripts/old.sh\n"); + assert.equal(r.call("pr", "view", "3", "--json", "files", "--jq", "[.files[].path]").out, '["a.md","docs/b.md"]\n'); + assert.equal(r.call("api", API("issues/1/comments"), "--jq", "[.[].author_association]").out, '["MEMBER","NONE","OWNER"]\n', "an explicit association wins over trusted"); +}); + +test("api contents is raw with the raw Accept header, a base64 object without it, and a 404 for a file that is not there", () => { + const r = run2(W3()); + assert.equal(r.call("api", "-H", "Accept: application/vnd.github.raw", API("contents/AGENTS.md?ref=main")).out, "rules\n"); + assert.equal(r.call("api", "-H", "Accept: application/vnd.github.raw", API("contents/.github/CODEOWNERS?ref=feat")).out, "* @me\n"); + const j = r.call("api", API("contents/AGENTS.md")).json(); + assert.deepEqual([j.type, j.path, j.encoding, Buffer.from(j.content, "base64").toString()], ["file", "AGENTS.md", "base64", "rules\n"]); + const miss = r.call("api", "-H", "Accept: application/vnd.github.raw", API("contents/nope.md?ref=main")); + assert.deepEqual([miss.code, miss.out, miss.err], [1, "", "gh: Not Found (HTTP 404)\n"]); + assert.equal(run2({}).call("api", "-H", "Accept: application/vnd.github.raw", API("contents/AGENTS.md")).code, 1, "a world with no files has none"); +}); + +test("api git/trees lists the blobs and the directories of the files the world has", () => { + const r = run2(W3()); + assert.equal(r.call("api", API("git/trees/HEAD?recursive=1"), "--jq", '.tree[] | select(.type == "blob") | .path').out, ".github/CODEOWNERS\nAGENTS.md\nsrc/a.js\n"); + assert.equal(r.call("api", API("git/trees/HEAD?recursive=1"), "--jq", '[.tree[] | select(.type == "tree") | .path]').out, '[".github","src"]\n'); + assert.equal(run2({}).call("api", API("git/trees/HEAD?recursive=1"), "--jq", ".tree | length").out, "0\n"); + assert.match(r.call("api", API("git/trees/HEAD")).err, /^unsupported: api GET repos\/acme\/widgets\/git\/trees\/HEAD \(only HEAD with recursive=1\)/); +}); + +test("api pulls lists the PRs as REST objects, newest first, paged, with the author's association", () => { + const r = run2(W3()); + const rows = (q, ...more) => r.call("api", API(`pulls?${q}`), ...more, "--jq", '.[] | [.number, .state, .head.ref, .author_association, .user.login, .merged_at != null, .draft] | @tsv').out.trim().split("\n"); + assert.deepEqual(rows("state=all&per_page=100&page=1"), ["10\tclosed\tbugfix/5-x\tNONE\tagent-bot\ttrue\tfalse", "9\topen\tfeat/9-pr-9\tMEMBER\tdev\tfalse\tfalse", "11\tclosed\tfeat/11-pr-11\tNONE\tagent-bot\tfalse\ttrue"], "newest first, by createdAt"); + assert.deepEqual(rows("state=open").map((x) => x.split("\t")[0]), ["9"]); + assert.deepEqual(rows("state=closed").map((x) => x.split("\t")[0]), ["10", "11"], "closed includes merged, as the API does"); + assert.deepEqual(rows("state=all&per_page=1&page=2").map((x) => x.split("\t")[0]), ["9"], "the second of three, newest first"); + assert.equal(rows("state=all&per_page=1&page=4")[0], "", "a page past the end is empty"); + assert.deepEqual(rows("state=all&per_page=1", "--paginate").length, 3, "--paginate returns them all"); + assert.equal(r.call("api", API("pulls?state=bogus")).err, "gh: Validation Failed (HTTP 422)\n"); + const p = r.call("api", API("pulls/9")).json(); + assert.deepEqual([p.number, p.head.sha, p.base.ref, p.author_association, p.labels.map((l) => l.name), p.html_url], [9, "h1", "main", "MEMBER", ["bug"], "https://github.com/acme/widgets/pull/9"]); + assert.equal(r.call("api", API("pulls/99")).err, "gh: Not Found (HTTP 404)\n"); +}); + +test("update-branch gives a BEHIND PR a new head and brings it up to date, and refuses what GitHub refuses", () => { + const r = run2(W3()), before = structuredClone(r.read()); + const ok = r.call("api", "-X", "PUT", API("pulls/9/update-branch"), "-f", "expected_head_sha=h1"); + assert.equal(ok.code, 0, ok.err); + assert.deepEqual(ok.json(), { message: "Updating pull request branch.", url: "https://api.github.com/repos/acme/widgets/pulls/9" }); + assert.equal(r.call("pr", "view", "9", "--json", "headRefOid,mergeStateStatus", "--jq", '.headRefOid + " " + .mergeStateStatus').out, "h1+ CLEAN\n"); + assert.deepEqual(diffWorlds(before, r.read()), [{ op: "push", target: "pr#9", sha: "h1+" }]); + assert.ok(sameWorld(applyEffects(before, diffWorlds(before, r.read())), r.read())); + assert.equal(r.call("api", "-X", "PUT", API("pulls/9/update-branch")).err, "gh: There are no new commits on the base branch. (HTTP 422)\n", "it is up to date now"); + const cases = [ + [{ mergeState: "BEHIND" }, ["-f", "expected_head_sha=old"], "gh: expected head sha didn't match current head ref. (HTTP 422)"], + [{ mergeState: "DIRTY" }, [], "gh: merge conflict between base and head (HTTP 422)"], + [{ mergeState: "CLEAN" }, [], "gh: There are no new commits on the base branch. (HTTP 422)"], + [{ mergeState: "BEHIND", state: "closed" }, [], "gh: Pull request is not open (HTTP 422)"], + ]; + for (const [patch, flags, want] of cases) { + const w = W3(); Object.assign(w.prs[0], patch); + const x = run2(w), was = x.bytes(); + const out = x.call("api", "-X", "PUT", API("pulls/9/update-branch"), ...flags); + assert.deepEqual([out.code, out.err.trim()], [1, want], JSON.stringify(patch)); + assert.equal(x.bytes(), was, "a refused update changes nothing"); + } + assert.equal(r.call("api", "-X", "PUT", API("pulls/99/update-branch")).err, "gh: Not Found (HTTP 404)\n"); +}); + +test("api refuses what it does not model, and says what", () => { + const r = run2(W3()); + for (const [argv, want] of [ + [["api", "graphql", "-f", "query=x"], /^unsupported: api POST graphql/], + [["api", API("issues/5/labels")], /^unsupported: api GET repos\/acme\/widgets\/issues\/5\/labels/], + [["api", "-X", "DELETE", API("issues/comments/5002")], /^unsupported: api DELETE/], + [["api", "-X", "GET", API("issues/5/comments"), "-f", "per_page=1"], /^unsupported: api GET with -f or -F/], + [["api", API("issues/5/comments"), "-f", "per_page=1"], /^unsupported: api POST repos\/acme\/widgets\/issues\/5\/comments/], + [["api", "-X", "PATCH", API("issues/comments/5002"), "-F", "body=@file"], /^unsupported: api -F @file/], + [["api", API("issues/5/comments"), "--input", "x"], /^unsupported: api --input/], + [["api"], /^unsupported: api without exactly one endpoint/], + [["api", API("issues/5/comments"), "-f", "novalue"], /requires a value separated by an '=' sign/], + ]) { const out = r.call(...argv); assert.equal(out.code, 1, argv.join(" ")); assert.match(out.err, want, argv.join(" ")); } + assert.equal(readFileSync(r.env.GH_SIM_LOG, "utf8").trim().split("\n").filter((l) => JSON.parse(l).kind === "unsupported").length, 8); +}); + +test("no gh command is left as planned: every one is supported or out of scope", () => { + assert.deepEqual(Object.entries(SURFACE).filter(([, st]) => st === "planned"), []); +}); + +// ---- faults ---------------------------------------------------------------------------------------------------- + +test("a fault makes the call it matches fail, changes nothing, and is marked in the log", () => { + const w = W2(); w.faults = [{ match: "pr merge", fail: "X Pull request sim/repo#3 is not mergeable: the base branch policy prohibits the merge." }]; + const r = run2(w), before = r.bytes(); + const out = r.call("pr", "merge", "3", "--squash"); + assert.deepEqual([out.code, out.out, out.err], [1, "", "X Pull request sim/repo#3 is not mergeable: the base branch policy prohibits the merge.\n"]); + assert.equal(r.bytes(), before); + const [call] = readFileSync(r.env.GH_SIM_LOG, "utf8").trim().split("\n").map((l) => JSON.parse(l)); + assert.deepEqual([call.exit, call.kind, call.fault], [1, "fault", 0]); + assert.equal(r.call("pr", "view", "3", "--json", "state", "--jq", ".state").out, "OPEN\n", "a call that does not match is not touched"); + assert.equal(r.call("pr", "merge", "4", "--squash").err.includes("not mergeable"), true, "it matches any PR"); +}); + +test("a fault matches whole words of the call, from the start", () => { + const w = W2(); w.faults = [{ match: "api -X PATCH", fail: "HTTP 500" }, { match: "pr merge 3", fail: "only PR 3" }, { match: "label", fail: "labels down", exit: 4 }]; + const r = run2(w); + assert.equal(r.call("api", "-X", "PATCH", API("issues/comments/1001"), "-f", "body=x").err, "HTTP 500\n"); + assert.equal(r.call("api", "user").code, 0, "a GET is another call"); + assert.equal(r.call("pr", "merge", "3", "--squash").err, "only PR 3\n"); + assert.equal(r.call("pr", "merge", "33", "--squash").err.includes("only PR 3"), false, "pr merge 3 is not pr merge 33"); + const l = r.call("label", "list"); + assert.deepEqual([l.code, l.err], [4, "labels down\n"], "the exit code is the fault's own"); + assert.equal(r.call("labels").err, "unsupported: labels\n", "`label` does not match `labels`"); +}); + +test("times limits a fault, counted in the log, and each fault counts its own", () => { + const w = W2(); w.faults = [{ match: "issue edit", fail: "flaky", times: 2 }, { match: "issue comment", fail: "down", times: 1 }]; + const r = run2(w); + const edit = () => r.call("issue", "edit", "1", "--add-label", "agent:in-progress"); + assert.deepEqual([edit().code, edit().code, edit().code], [1, 1, 0], "it fires twice, and then the call goes through"); + assert.deepEqual(r.read().issues[0].labels, ["ready-for-agent", "agent:in-progress"]); + assert.equal(r.call("issue", "comment", "1", "--body", "x").err, "down\n"); + assert.equal(r.call("issue", "comment", "1", "--body", "x").code, 0); + assert.deepEqual(readFileSync(r.env.GH_SIM_LOG, "utf8").trim().split("\n").map((l) => JSON.parse(l).fault ?? null), [0, 0, null, 1, null]); + const nolog = spawnSync(GH, ["issue", "edit", "1", "--add-label", "x"], { encoding: "utf8", env: { ...r.env, GH_SIM_LOG: "" } }); + assert.deepEqual([nolog.status, /needs GH_SIM_LOG/.test(nolog.stderr)], [2, true]); +}); + +test("a fault is counted by its definition: a different fault at the same place starts fresh, the same one does not", () => { + const w = W2(); w.faults = [{ match: "issue edit", fail: "first", times: 1 }]; + const r = run2(w); + const edit = () => r.call("issue", "edit", "1", "--add-label", "agent:in-progress"); + assert.deepEqual([edit().err, edit().code], ["first\n", 0]); + const rewrite = (faults) => { const x = r.read(); x.faults = faults; writeFileSync(r.path, stringify(x)); }; + rewrite([{ match: "issue edit", fail: "second", times: 1 }]); // another fault, at index 0 again + assert.deepEqual([edit().err, edit().code], ["second\n", 0], "it is not spent by what the first one did"); + rewrite([{ match: "issue edit", fail: "first", times: 1 }]); // the first one, written again + assert.equal(edit().code, 0, "it is still spent"); + const ids = readFileSync(r.env.GH_SIM_LOG, "utf8").trim().split("\n").map((l) => JSON.parse(l)).filter((c) => c.fault !== undefined); + assert.deepEqual(ids.map((c) => c.fault), [0, 0]); + assert.equal(new Set(ids.map((c) => c.faultId)).size, 2); + assert.match(ids[0].faultId, /^[0-9a-f]{10}$/); +}); + +test("a fault's before effects change the world first, as another run getting in ahead of this call", () => { + const w = W2(); w.faults = [{ match: "issue create", before: [{ op: "create_issue", number: 6, labels: ["agent:log"], data: { title: "racing run", body: "" } }], times: 1 }]; + const r = run2(w); + const out = r.call("issue", "create", "--title", "mine", "--body", "b", "--label", "agent:log"); + assert.equal(out.out, "https://github.com/sim/repo/issues/7\n", "the number after the one that got in first"); + assert.deepEqual(r.read().issues.map((i) => i.number), [1, 2, 6, 7]); + assert.equal(JSON.parse(readFileSync(r.env.GH_SIM_LOG, "utf8").trim()).fault, 0, "the call is marked as met by a fault"); + const both = W2(); both.faults = [{ match: "pr merge", before: [{ op: "comment", target: "pr#3", marker: "agent:handoff", text: "a human spoke" }], fail: "changed under you" }]; + const b = run2(both); + assert.equal(b.call("pr", "merge", "3", "--squash").err, "changed under you\n"); + assert.equal(b.read().prs[0].comments.at(-1).text, "a human spoke", "the effects stay when the call fails"); + const sc = run2({ skill: "s", world: both, expect: {} }); + const refused = sc.call("pr", "merge", "3", "--squash"); + assert.equal(refused.code, 2); + assert.match(refused.err, /scenario file/); + assert.equal(run2({ skill: "s", world: { ...W2(), faults: [{ match: "pr merge", fail: "x" }] }, expect: {} }).call("pr", "merge", "3", "--squash").err, "x\n", "a fault that only fails can meet a scenario file"); +}); + +test("a fault written wrong is refused when the world is read", () => { + for (const [fault, want] of [[{ fail: "x" }, /fault 0 needs a `match`/], [{ match: "pr merge" }, /fault 0 needs a `fail` message or `before` effects/], [{ match: "pr merge", fail: "x", times: 0 }, /`times` to be a whole number above 0/], [{ match: "", fail: "x" }, /a `match`/]]) { + const out = run2({ ...W2(), faults: [fault] }).call("issue", "list"); + assert.equal(out.code, 2, JSON.stringify(fault)); + assert.match(out.err, want); + } +}); + +test("a fault does not show in the diff: the world's faults are not state", () => { + const w = W2(); w.faults = [{ match: "pr merge", fail: "nope", times: 1 }]; + const r = run2(w), before = structuredClone(r.read()); + r.call("pr", "merge", "3", "--squash"); r.call("issue", "edit", "1", "--add-label", "agent:in-progress"); + assert.deepEqual(diffWorlds(before, r.read()).map((e) => e.op), ["label_add"]); +}); + +test("a golden scenario whose world carries faults is judged as it was, and the diff does not see them", () => { + let n = 0; + for (const dir of readdirSync(join(ROOT, "tests/scenarios"))) { + for (const f of readdirSync(join(ROOT, "tests/scenarios", dir)).filter((x) => x.endsWith(".yaml"))) { + const sc = expandScenario(parse(readFileSync(join(ROOT, "tests/scenarios", dir, f), "utf8"))); + const withFaults = structuredClone(sc); + withFaults.world = { ...(sc.world ?? {}), faults: [{ match: "pr merge", fail: "not mergeable", times: 1 }, { match: "api -X PATCH", fail: "HTTP 500" }] }; + assert.deepEqual(checkScenario(contractOf(sc.skill), withFaults), [], `${dir}/${f}`); + // The world a run starts from and the one it leaves both carry the faults, so they are no part of what the run did. + const effects = (sc.expect?.effects ?? []).filter((e) => !["unmodelled", "push", "write_file"].includes(e.op) && e.op !== "labels_sync"); + assert.deepEqual(diffWorlds(withFaults.world, applyEffects(withFaults.world, effects)), diffWorlds(sc.world ?? {}, applyEffects(sc.world ?? {}, effects)), `${dir}/${f}`); + n++; + } + } + assert.ok(n >= 80); +}); + // ---- seeding ------------------------------------------------------------------------------------------------- test("seed makes a world file from a scenario, and reports the fields no gh call shows", () => { diff --git a/tests/lib.test.sh b/tests/lib.test.sh index 1dc8ec0..2cd9be0 100644 --- a/tests/lib.test.sh +++ b/tests/lib.test.sh @@ -1,10 +1,10 @@ #!/usr/bin/env bash -# Tests for the shell helpers in agent-workflow-protocol/scripts/lib.sh, against a stub `gh`. +# Tests for the shell helpers in agent-workflow-protocol/scripts/lib.sh, against the `gh` simulator (tests/sim). # bash tests/lib.test.sh # Needs bash, jq, git and Node.js. No network and no real `gh`. set -u -# Fail once, with one line, when a tool the stub `gh` or the tests rely on is missing. Without this +# Fail once, with one line, when a tool the simulator `gh` or the tests rely on is missing. Without this # the suite keeps going and reports dozens of failures that are really one missing tool. for tool in jq git node; do command -v "$tool" >/dev/null 2>&1 || { echo "tests/lib.test.sh needs $tool on PATH" >&2; exit 2; } @@ -15,97 +15,43 @@ LIB="$PKG/skills/meta/agent-workflow-protocol/scripts/lib.sh" WORK="$(mktemp -d)"; trap 'rm -rf "$WORK"' EXIT export STATE="$WORK/state"; mkdir -p "$STATE" "$WORK/bin" "$WORK/repo" -echo '[]' > "$STATE/prs.json"; echo '[]' > "$STATE/issues.json" -# ---- Stub gh: file-backed, covers only the calls lib.sh makes ------------------------------ -cat > "$WORK/bin/gh" <<'STUB' -#!/usr/bin/env bash -cmd=$1; shift -# gh has no author association on a pull request: `--json authorAssociation` is an error for pr list and pr view. -if [ "$cmd" = pr ] && [[ "$*" == *authorAssociation* ]]; then echo 'Unknown JSON field: "authorAssociation"' >&2; exit 1; fi -if [ "$cmd" = pr ] && [ "${1:-}" = list ]; then cat "$STATE/prs.json"; exit 0; fi -# prs.json holds [{number,state:OPEN|MERGED|CLOSED,title,body,headRefName,author:{login},authorAssociation,closing:[numbers]}]. A REST pull request has other names. -restpr='{number,title,body,state:(if .state=="OPEN" then "open" else "closed" end),merged_at:(if .state=="MERGED" then "2026-01-01T00:00:00Z" else null end),head:{ref:.headRefName},user:{login:.author.login},author_association:.authorAssociation}' -jqf() { local f=""; while [ $# -gt 0 ]; do [ "$1" = --jq ] && f=$2; shift; done; jq -r "${f:-.}"; } -# Issues live in $STATE/issues.json: [{number,title,state,labels:[names],createdAt,updatedAt,body}]. -# Unlike a stub that returns a fixed list, this one makes a lookup that finds nothing actually find -# nothing, which is how the "a new log issue on every run" bug got past the tests. +# ---- The gh simulator --------------------------------------------------------------------- +# tests/sim/bin/gh answers `gh` from a world file (the scenarios' format) and logs every call. It is the one mock of +# `gh` in this package, and it is as strict as `gh`: a `--json` field `gh` does not have is an error here too. +# The world is $STATE/world.json. A test sets it up with `w PROGRAM` (jq over the world) and the helpers below, runs the +# functions under test, and reads the result back through `gh` itself. +export GH_SIM_WORLD="$STATE/world.json" GH_SIM_LOG="$STATE/gh.log" +echo '{"issues":[],"prs":[],"files":{}}' > "$GH_SIM_WORLD" +export PATH="$PKG/tests/sim/bin:$PATH" + now_iso() { date -u +%Y-%m-%dT%H:%M:%SZ; } -mk_issue() { # title body label -> prints the new number - local num; num=$(( $(jq 'map(.number) | max // 0' "$STATE/issues.json") + 1 )) - jq --argjson n "$num" --arg t "$1" --arg b "$2" --arg l "$3" --arg c "$(now_iso)" \ - '. + [{number:$n,title:$t,state:"open",labels:[$l],createdAt:$c,updatedAt:$c,body:$b}]' "$STATE/issues.json" > "$STATE/issues.t" - mv "$STATE/issues.t" "$STATE/issues.json"; echo "$num" +w() { jq "$@" "$GH_SIM_WORLD" > "$GH_SIM_WORLD.t" && mv "$GH_SIM_WORLD.t" "$GH_SIM_WORLD"; } # w [--arg k v ...] PROGRAM +items() { # FROM TO -> open issues FROM..TO, those that are not there yet + w --argjson a "$1" --argjson b "$2" '.issues += [range($a; $b + 1) as $n | select([.issues[].number] | index($n) | not) | {number:$n,state:"open",labels:[]}]'; } +ensure() { w --argjson n "$1" 'if ([.issues[].number, .prs[].number] | index($n)) then . else .issues += [{number:$n,state:"open",labels:[]}] end'; } +mkpr() { w --argjson n "$1" 'if (.prs | map(.number) | index($n)) then . else .prs += [{number:$n,state:"open",labels:[]}] end'; } +mk_issue() { # title body label -> prints the new number, as `gh issue create` would number it + local num; num=$(( $(jq '[.issues[].number, .prs[].number] | max // 0' "$GH_SIM_WORLD") + 1 )) + w --argjson n "$num" --arg t "$1" --arg b "$2" --arg l "$3" --arg c "$(now_iso)" '.issues += [{number:$n,title:$t,state:"open",labels:[$l],createdAt:$c,body:$b}]' + echo "$num" } -set_issue() { jq --argjson n "$1" --arg k "$2" --arg v "$3" 'map(if .number==$n then .[$k]=$v else . end)' "$STATE/issues.json" > "$STATE/issues.t"; mv "$STATE/issues.t" "$STATE/issues.json"; } -if [ "$cmd" = label ]; then [ -f "$STATE/labels.json" ] && cat "$STATE/labels.json"; exit 0; fi -if [ "$cmd" = issue ] && [ "${1:-}" = list ]; then - shift; orig=("$@"); lab=""; st=open - while [ $# -gt 0 ]; do case $1 in --label) lab=$2; shift ;; --state) st=$2; shift ;; esac; shift; done - jq -c --arg l "$lab" --arg s "$st" 'map(select(($l=="" or (.labels|index($l))) and ($s=="all" or .state==$s))) | sort_by(-.number)' "$STATE/issues.json" | jqf "${orig[@]}" - exit 0 -fi -if [ "$cmd" = issue ] && [ "${1:-}" = create ]; then - shift; title=""; body=""; lab="" - while [ $# -gt 0 ]; do case $1 in --title) title=$2; shift ;; --body) body=$2; shift ;; --label) lab=$2; shift ;; esac; shift; done - [ -n "${STUB_RACE:-}" ] && mk_issue "racing run" "" "$lab" >/dev/null # another run got in first - echo "https://github.com/o/r/issues/$(mk_issue "$title" "$body" "$lab")"; exit 0 -fi -case "$cmd" in - api) # gh api user --jq F | gh api repos/{owner}/{repo}/issues/N/comments --paginate --jq F - # gh api -X PATCH repos/{owner}/{repo}/issues/comments/ID -f body=TEXT (STUB_PATCH_FAIL=1 fails it) - if [ "$1" = -X ] && [ "${2:-}" = PATCH ]; then - [ -n "${STUB_PATCH_FAIL:-}" ] && exit 1 - id=$(echo "$3" | sed -n 's#.*/issues/comments/\([0-9]*\)$#\1#p'); pb="" - while [ $# -gt 0 ]; do case $1 in -f) pb=${2#body=}; shift ;; esac; shift; done - for cf in "$STATE"/comments_*.json; do - jq -e --argjson i "$id" 'any(.[]; .id==$i)' "$cf" >/dev/null 2>&1 || continue - jq --argjson i "$id" --arg b "$pb" --arg t "$(now_iso)" 'map(if .id==$i then .body=$b | .updated_at=$t else . end)' "$cf" > "$cf.t"; mv "$cf.t" "$cf"; exit 0 - done - exit 1 - fi - if [ "$1" = user ]; then echo '{"login":"agent-bot"}' | jqf "$@"; exit 0; fi - path=""; for a in "$@"; do case $a in repos/*) path=$a ;; esac; done - case $path in - */git/trees/*) [ -f "$STATE/tree.json" ] || exit 1; jqf "$@" < "$STATE/tree.json"; exit 0 ;; - */contents/*) f="$STATE/contents/${path#*/contents/}"; f="${f%%\?*}"; [ -f "$f" ] && { cat "$f"; exit 0; }; exit 1 ;; - */pulls/*/files) n=$(echo "$path" | sed -n 's#.*/pulls/\([0-9]*\)/files#\1#p') - [ -f "$STATE/files_$n.json" ] || exit 1; jqf "$@" < "$STATE/files_$n.json"; exit 0 ;; - */pulls\?*) page=${path##*page=} # 100 to a page, as the REST API pages - jq -c --argjson p "$page" ".[($page-1)*100:$page*100] | map($restpr)" "$STATE/prs.json" | jqf "$@"; exit 0 ;; - */pulls/[0-9]*) n=${path##*/pulls/}; out=$(jq -c --argjson n "$n" ".[]|select(.number==\$n)|$restpr" "$STATE/prs.json") - [ -n "$out" ] || { echo "HTTP 404: Not Found" >&2; exit 1; } - printf '%s\n' "$out" | jqf "$@"; exit 0 ;; - esac - n=$(echo "$1" | sed -n 's#.*/issues/\([0-9]*\)/comments#\1#p') - { cat "$STATE/comments_$n.json" 2>/dev/null || echo '[]'; } | jqf "$@" ;; - issue|pr) - sub=$1; shift; n=$1; shift - lf="$STATE/labels_${cmd}_$n"; touch "$lf" - case "$sub" in - view) if [[ " $* " == *" comments "* ]]; then - jq '{comments: .}' "$STATE/comments_$n.json" | jqf "$@" - elif [ "$cmd" = issue ] && [[ "$*" == *createdAt* ]]; then - jq --argjson n "$n" '.[]|select(.number==$n)' "$STATE/issues.json" | jqf "$@" - elif [ "$cmd" = pr ] && [[ "$*" == *baseRefName* ]]; then - echo '{"baseRefName":"main"}' | jqf "$@" - elif [ "$cmd" = pr ] && [[ " $* " == *closingIssuesReferences* ]]; then - [ -n "${STUB_OLD_GH:-}" ] && { echo 'Unknown JSON field: "closingIssuesReferences"' >&2; exit 1; } # gh before 2.72 - jq --argjson n "$n" '.[]|select(.number==$n)|{closingIssuesReferences:(.closing//[])|map({number:.})}' "$STATE/prs.json" | jqf "$@" - else jq -Rn '{labels:[inputs|select(length>0)|{name:.}]}' "$lf" | jqf "$@"; fi ;; - close) while [ $# -gt 0 ]; do case $1 in --reason) set_issue "$n" closeReason "$2"; shift ;; esac; shift; done - set_issue "$n" state closed ;; - edit) while [ $# -gt 0 ]; do case $1 in - --body-file) set_issue "$n" body "$(cat "$2")"; shift ;; - --add-label) grep -qx -- "$2" "$lf" || echo "$2" >> "$lf"; shift ;; - --remove-label) grep -vx -- "$2" "$lf" > "$lf.t"; mv "$lf.t" "$lf"; shift ;; esac; shift; done ;; - comment) body=""; while [ $# -gt 0 ]; do [ "$1" = --body ] && body=$2; [ "$1" = --body-file ] && body=$(cat "$2"); shift; done - cf="$STATE/comments_$n.json"; [ -f "$cf" ] || echo '[]' > "$cf" - jq --arg b "$body" --arg t "$(now_iso)" --argjson n "$n" '. + [{id:($n*1000+length), body:$b, author_association:"NONE", user:{login:"agent-bot"}, created_at:$t, updated_at:$t}]' "$cf" > "$cf.t"; mv "$cf.t" "$cf" ;; - esac ;; -esac -STUB -chmod +x "$WORK/bin/gh"; export PATH="$WORK/bin:$PATH" +c() { ensure "$1" # c N body [association] [login] [updated_at]: a comment on issue or PR N + w --argjson n "$1" --arg b "$2" --arg a "${3:-OWNER}" --arg l "${4:-maintainer}" --arg t "${5:-}" ' + def add: .comments = ((.comments // []) + [{author:$l, association:$a, text:$b} + (if $t == "" then {} else {createdAt:$t, updatedAt:$t} end)]); + .issues |= map(if .number == $n then add else . end) | .prs |= map(if .number == $n then add else . end)'; } +cr() { ensure "$1" # cr N body created_at [association] [login] + w --argjson n "$1" --arg b "$2" --arg t "$3" --arg a "${4:-OWNER}" --arg l "${5:-maintainer}" ' + def add: .comments = ((.comments // []) + [{author:$l, association:$a, text:$b, createdAt:$t}]); + .issues |= map(if .number == $n then add else . end) | .prs |= map(if .number == $n then add else . end)'; } +setfiles() { local n=$1; shift; mkpr "$n" # setfiles N path...: the files PR N changes + w --argjson n "$n" --argjson f "$(jq -n '$ARGS.positional' --args "$@")" '.prs |= map(if .number == $n then .files = $f else . end)'; } +set_prs() { # PRs as [{number,state:OPEN|MERGED|CLOSED,title,body,headRefName,author:{login},authorAssociation,closing:[numbers]}] on stdin + w --argjson p "$(jq 'map({number, state:(.state | ascii_downcase), title, body, branch:.headRefName, closes:(.closing // []), association:.authorAssociation, author:.author.login} | with_entries(select(.value != null)))')" '.prs = $p'; } +cj() { gh api "repos/{owner}/{repo}/issues/$1/comments" --paginate; } # an item's comments, as the REST API gives them +ij() { gh issue list --state all --limit 1000 --json number,title,state,labels,createdAt,body --jq 'map({number,title,state:(.state | ascii_downcase),labels:(.labels | map(.name)),createdAt,body}) | sort_by(.number)'; } # every issue, oldest first +fault() { w --argjson f "$1" '.faults = $f'; } # fault '[{"match":"api -X PATCH","fail":"HTTP 500"}]': the next matching calls fail +no_faults() { w 'del(.faults)'; } # lib.sh needs to run inside a git repository, but not this one git init -q "$WORK/repo" && cd "$WORK/repo" || exit 1 @@ -114,10 +60,7 @@ pass=0; fail=0 check() { # name expected actual if [ "$2" = "$3" ]; then pass=$((pass+1)); echo " ok $1"; else fail=$((fail+1)); echo " FAIL $1 (expected '$2', got '$3')"; fi } -c() { local cf="$STATE/comments_$1.json"; [ -f "$cf" ] || echo '[]' > "$cf" # c N body [association] [login] [updated_at] - jq --arg b "$2" --arg a "${3:-OWNER}" --arg l "${4:-maintainer}" --arg t "${5:-}" --argjson n "$1" \ - '. + [{id:($n*1000+length), body:$b, author_association:$a, user:{login:$l}} + (if $t == "" then {} else {created_at:$t, updated_at:$t} end)]' "$cf" > "$cf.t"; mv "$cf.t" "$cf"; } -lease_comments() { jq '[.[] | select(.body | contains("" NONE mallory claim 6 issue agent:triage Triaging "1 hour" check "a lease comment from another account is never edited, ours is added" 2 "$(lease_comments 6)" -check "and the other account's comment is unchanged" 1 "$(jq '[.[] | select(.user.login == "mallory" and (.body | startswith("Claimed by Triaging run z")))] | length' "$STATE/comments_6.json")" -STUB_PATCH_FAIL=1 claim 8 issue agent:triage Triaging "1 hour" -STUB_PATCH_FAIL=1 claim 8 issue agent:triage Triaging "1 hour" +check "and the other account's comment is unchanged" 1 "$(cj 6 | jq '[.[] | select(.user.login == "mallory" and (.body | startswith("Claimed by Triaging run z")))] | length')" +fault '[{"match":"api -X PATCH","fail":"gh: Internal Server Error (HTTP 500)"}]' +claim 8 issue agent:triage Triaging "1 hour" +claim 8 issue agent:triage Triaging "1 hour" +no_faults check "if the edit fails, the claim still posts a lease comment" 2 "$(lease_comments 8)" lease_live 8; check "and the item is leased" 0 $? # Edited in place, a lease comment can sit before a newer lease of another step: the last write wins. @@ -217,12 +165,10 @@ check "merged handoff is not a review round" 0 "$(review_rounds 22)" check "merged handoff leaves no open findings" 0 "$(open_findings 22)" echo "Merge lane" -cp "$STATE/prs.json" "$STATE/prs.saved" -cr() { local cf="$STATE/comments_$1.json"; [ -f "$cf" ] || echo '[]' > "$cf" # cr N body created_at [association] [login] - jq --arg b "$2" --arg t "$3" --arg a "${4:-OWNER}" --arg l "${5:-maintainer}" \ - '. + [{body:$b, author_association:$a, user:{login:$l}, created_at:$t}]' "$cf" > "$cf.t"; mv "$cf.t" "$cf"; } -rdy() { echo '{"number":'"$1"',"isDraft":'"${2:-false}"',"labels":[{"name":"agent:ready-for-review"}'"${3:-}"']}'; } -printf '[%s,%s,%s,%s]\n' "$(rdy 50)" "$(rdy 51 true)" "$(rdy 52 false ',{"name":"agent:stuck"}')" "$(rdy 53)" > "$STATE/prs.json" +prs_saved=$(jq -c .prs "$GH_SIM_WORLD") +rdy() { # N [draft] [extra label] -> a PR that carries the ready label + jq -nc --argjson n "$1" --argjson d "${2:-false}" --arg x "${3:-}" '{number:$n,state:"open",draft:$d,labels:(["agent:ready-for-review"] + (if $x == "" then [] else [$x] end))}'; } +w --argjson p "[$(rdy 50),$(rdy 51 true),$(rdy 52 false agent:stuck),$(rdy 53)]" '.prs = $p' READY="ready " cr 50 "$READY" "$(utc_plus '-30 minutes')" cr 51 "$READY" "$(utc_plus '-5 minutes')" @@ -232,18 +178,17 @@ check "a fresh ready PR holds the lane; a draft, a stuck PR and a stranger's mar lane_holder 60 >/dev/null; check "lane_holder returns 0 when held" 0 $? check "a PR is never its own holder" "" "$(lane_holder 50)" lane_holder 50 >/dev/null; check "lane_holder returns 1 when free" 1 $? -rm -f "$STATE/comments_50.json"; cr 50 "$READY" "$(utc_plus '-3 hours')" +w '.prs |= map(if .number == 50 then del(.comments) else . end)'; cr 50 "$READY" "$(utc_plus '-3 hours')" check "a handoff past ready_lane_ttl no longer holds the lane" "" "$(lane_holder 60)" -check "ready_at is the handoff time" "$(jq -r '.[0].created_at' "$STATE/comments_50.json")" "$(ready_at 50)" +check "ready_at is the handoff time" "$(cj 50 | jq -r '.[0].created_at')" "$(ready_at 50)" ready_lane_ttl="4 hours" check "a longer ready_lane_ttl keeps it held" 50 "$(lane_holder 60)" ready_lane_ttl="2 hours" -echo '[]' > "$STATE/prs.json" +w '.prs = []' check "no ready PRs, a free lane" "" "$(lane_holder 60)" -cp "$STATE/prs.saved" "$STATE/prs.json" +w --argjson p "$prs_saved" '.prs = $p' echo "Protected paths and owners" -setfiles() { local n=$1; shift; jq -n '$ARGS.positional | map({filename:.})' --args "$@" > "$STATE/files_$n.json"; } setfiles 70 README.md skills/x/SKILL.md docs/a.md human_merge_paths="" check "no human_merge_paths protects nothing" "" "$(protected_files 70)" @@ -253,76 +198,67 @@ check "a changed file that matches is listed" "skills/x/SKILL.md" "$(protected_f setfiles 71 README.md docs/a.md check "no matching file prints nothing" "" "$(protected_files 71)" protected_files 71 >/dev/null; check "with status 0" 0 $? -jq -n '[{filename:"docs/b.md", previous_filename:"scripts/old.sh"}]' > "$STATE/files_72.json" +mkpr 72; w '.prs |= map(if .number == 72 then .files = [{path:"docs/b.md",previous:"scripts/old.sh"}] else . end)' check "the old name of a renamed file counts" "scripts/old.sh" "$(protected_files 72)" protected_files 79 >/dev/null 2>&1; check "an unreadable file list fails closed (status 2)" 2 $? -echo '[]' > "$STATE/files_73.json" +setfiles 73 protected_files 73 >/dev/null 2>&1; check "an empty file list fails closed too" 2 $? human_merge_paths="!skills/" protected_files 70 >/dev/null 2>&1; rc=$?; check "an unsupported pattern fails (never reads as 'nothing protected')" 1 "$([ $rc -ne 0 ] && echo 1 || echo 0)" human_merge_paths="" -mkdir -p "$STATE/contents/.github" "$STATE/contents/docs" -printf '# owners\n* @root\n/docs/ @docs-team @org/writers\n' > "$STATE/contents/.github/CODEOWNERS" +w --arg t "$(printf '# owners\n* @root\n/docs/ @docs-team @org/writers\n')" '.files[".github/CODEOWNERS"] = $t' setfiles 74 docs/a.md README.md check "owners come from CODEOWNERS on the base branch" "$(printf '@docs-team\n@org/writers\n@root')" "$(codeowners_for 74)" -printf '* @from-docs-dir\n' > "$STATE/contents/docs/CODEOWNERS" +w --arg t "$(printf '* @from-docs-dir\n')" '.files["docs/CODEOWNERS"] = $t' check ".github/CODEOWNERS wins over docs/CODEOWNERS" "$(printf '@docs-team\n@org/writers\n@root')" "$(codeowners_for 74)" -rm -f "$STATE/contents/.github/CODEOWNERS" +w 'del(.files[".github/CODEOWNERS"])' check "and docs/CODEOWNERS is used when it is the only one" "@from-docs-dir" "$(codeowners_for 74)" -rm -rf "$STATE/contents" +w '.files = {}' check "no CODEOWNERS file means no owners" "" "$(codeowners_for 74)" codeowners_for 74 >/dev/null; check "and it is not an error" 0 $? -rm -f "$STATE"/files_*.json echo "Run log" -rm -f "$STATE"/comments_*.json # earlier sections used the same issue numbers -mk_issue() { # title body label -> a log issue in the stub's store, as `gh issue create` would - local num; num=$(( $(jq 'map(.number) | max // 0' "$STATE/issues.json") + 1 )) - jq --argjson n "$num" --arg t "$1" --arg b "$2" --arg l "$3" --arg c "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ - '. + [{number:$n,title:$t,state:"open",labels:[$l],createdAt:$c,updatedAt:$c,body:$b}]' "$STATE/issues.json" > "$STATE/issues.t" - mv "$STATE/issues.t" "$STATE/issues.json"; echo "$num" -} +w '.issues = [] | .prs = []' # earlier sections used the same numbers, and issues and PRs share one sequence old_iso=$(utc_plus "-30 days"); week_iso=$(utc_plus "-3 days"); stale_iso=$(utc_plus "-20 days") legacy_body="Append-only run log: one comment per run. Covers roughly two weeks. Excluded from every workflow step." issues_with() { # number created_at -> a lone legacy log issue (its body carries no dates) - jq -n --argjson n "$1" --arg c "$2" --arg b "$legacy_body" \ - '[{number:$n,title:"Agent log old",state:"open",labels:["agent:log"],createdAt:$c,updatedAt:$c,body:$b}]' > "$STATE/issues.json"; } + w --argjson n "$1" --arg c "$2" --arg b "$legacy_body" '.issues = [{number:$n,title:"Agent log old",state:"open",labels:["agent:log"],createdAt:$c,body:$b}]'; } entry() { # RUN STEP PROBLEMS -> an entry file, the way a runner writes one printf '```yaml\nstep: %s\nrun: %s\nrunner: kilo\noutcome: "1 triaged"\nproblems_count: 0\n```\n**Step:** %s **Run:** %s **Runner:** kilo\n**Outcome:** 1 triaged\n**Problems:** %s\n**Things to improve:** none\n' \ "$2" "$1" "$2" "$1" "$3" > "$WORK/entry-$1.md"; echo "$WORK/entry-$1.md"; } -echo '[]' > "$STATE/issues.json" +w '.issues = []' n1=$(log_issue); n2=$(log_issue); n3=$(log_issue) -check "log_issue makes one issue, not one per call" 1 "$(jq length "$STATE/issues.json")" +check "log_issue makes one issue, not one per call" 1 "$(ij | jq length)" check "log_issue returns the same issue each time" "$n1,$n1" "$n2,$n3" -check "the log issue is labelled agent:log" agent:log "$(jq -r '.[0].labels[0]' "$STATE/issues.json")" -check "the log issue body says when the window rotates" 1 "$(jq -r '.[0].body' "$STATE/issues.json" | grep -c 'Rotates after:\*\* 20')" -check "the log issue body has a Last entry line" 1 "$(jq -r '.[0].body' "$STATE/issues.json" | grep -c 'Last entry:')" -check "title is Agent log YYYY-MM-DD" 1 "$(jq -r '.[0].title' "$STATE/issues.json" | grep -Ec '^Agent log [0-9]{4}-[0-9]{2}-[0-9]{2}$')" +check "the log issue is labelled agent:log" agent:log "$(ij | jq -r '.[0].labels[0]')" +check "the log issue body says when the window rotates" 1 "$(ij | jq -r '.[0].body' | grep -c 'Rotates after:\*\* 20')" +check "the log issue body has a Last entry line" 1 "$(ij | jq -r '.[0].body' | grep -c 'Last entry:')" +check "title is Agent log YYYY-MM-DD" 1 "$(ij | jq -r '.[0].title' | grep -Ec '^Agent log [0-9]{4}-[0-9]{2}-[0-9]{2}$')" e1=$(entry R-1 Triaging "degraded: browser missing") log_run "$e1"; log_run "$e1" -check "log_run posts an entry once, even when called twice" 1 "$(jq length "$STATE/comments_$n1.json")" -check "the entry carries an at: time" 1 "$(jq -r '.[0].body' "$STATE/comments_$n1.json" | grep -c '^at: 20')" -check "the issue body names the last entry" 1 "$(jq -r '.[0].body' "$STATE/issues.json" | grep -c 'Last entry:\*\* 20[0-9:TZ-]* (Triaging, run R-1)')" -check "problems do not open a difficulty issue" 1 "$(jq length "$STATE/issues.json")" +check "log_run posts an entry once, even when called twice" 1 "$(cj $n1 | jq length)" +check "the entry carries an at: time" 1 "$(cj $n1 | jq -r '.[0].body' | grep -c '^at: 20')" +check "the issue body names the last entry" 1 "$(ij | jq -r '.[0].body' | grep -c 'Last entry:\*\* 20[0-9:TZ-]* (Triaging, run R-1)')" +check "problems do not open a difficulty issue" 1 "$(ij | jq length)" log_run "$(entry R-2 Triaging none)" log_run "$(entry R-2 Shepherding none)" -check "another run, and another step of the same run, each get an entry" 3 "$(jq length "$STATE/comments_$n1.json")" +check "another run, and another step of the same run, each get an entry" 3 "$(cj $n1 | jq length)" check "log_status says when the log was last written" 1 "$(log_status | grep -c '^last_entry=20')" printf '%s' '\n\n**Step:** Reviewing **Run:** R-3 **Runner:** kilo\n**Outcome:** 1 merged\n**Problems:** none\n' > "$WORK/escaped.md" log_run "$WORK/escaped.md" -check "an entry written with literal \\n gets real line breaks" 1 "$(jq -r '.[-1].body' "$STATE/comments_$n1.json" | grep -c '^\*\*Outcome:\*\* 1 merged$')" -check "and an at: time" 1 "$(jq -r '.[-1].body' "$STATE/comments_$n1.json" | grep -c '^\*\*At:\*\* 20')" +check "an entry written with literal \\n gets real line breaks" 1 "$(cj $n1 | jq -r '.[-1].body' | grep -c '^\*\*Outcome:\*\* 1 merged$')" +check "and an at: time" 1 "$(cj $n1 | jq -r '.[-1].body' | grep -c '^\*\*At:\*\* 20')" echo "Run log rotation" issues_with 50 "$week_iso" check "an issue inside its window keeps getting entries" 50 "$(log_issue)" -check "and no second issue is made" 1 "$(jq length "$STATE/issues.json")" +check "and no second issue is made" 1 "$(ij | jq length)" issues_with 50 "$old_iso" check "an issue past its window is replaced" 51 "$(log_issue)" -check "the old one is left alone, not closed" open "$(jq -r '.[]|select(.number==50)|.state' "$STATE/issues.json")" +check "the old one is left alone, not closed" open "$(ij | jq -r '.[]|select(.number==50)|.state')" issues_with 50 "$stale_iso" check "20 days old: past the default window of 14" 51 "$(log_issue)" issues_with 50 "$stale_iso" @@ -334,24 +270,28 @@ check "log_lookback_days is still read" 7 "$(log_lookback_days=7 bash -c 'source check "log_window_days wins over the old name" 3 "$(log_lookback_days=7 log_window_days=3 bash -c 'source "$1"; echo "$log_window_days"' _ "$LIB")" check "a window that is not a whole number falls back to 14" 14 "$(node "$AGENT_HELPERS" log-window "two weeks")" -echo '[]' > "$STATE/issues.json" +w '.issues = []' mk_issue "Agent log a" "$legacy_body" agent:log >/dev/null; mk_issue "Agent log b" "$legacy_body" agent:log >/dev/null check "two open windows: the lowest number is the log" 1 "$(log_issue)" -echo '[]' > "$STATE/issues.json" -got=$(STUB_RACE=1 log_issue) +w '.issues = []' +# another run gets its log issue in ahead of ours +w --arg c "$(now_iso)" '.faults = [{match:"issue create", times:1, before:[{op:"create_issue", number:1, labels:["agent:log"], data:{title:"racing run", body:"", createdAt:$c}}]}]' +got=$(log_issue) +no_faults check "a run that lost a creation race uses the other run's issue" 1 "$got" -check "and closes its own duplicate" closed "$(jq -r '.[]|select(.number==2)|.state' "$STATE/issues.json")" +check "and closes its own duplicate" closed "$(ij | jq -r '.[]|select(.number==2)|.state')" echo "Reading the log" -echo '[]' > "$STATE/issues.json"; mk_issue "Agent log" "$legacy_body" agent:log >/dev/null -n9=1 -cat > "$STATE/comments_$n9.json" <<'J' +w '.issues = []'; mk_issue "Agent log" "$legacy_body" agent:log >/dev/null +logc=$(jq 'map({text:.body, createdAt, author:"maintainer", association:"OWNER"})' <<'J' [{"body":"**Step:** Triaging\n**Outcome:** nothing eligible","createdAt":"2999-01-01T00:00:00Z"}, {"body":"**Step:** Triaging\n**Outcome:** nothing eligible\n**Problems:** none\n**Things to improve:** none","createdAt":"2999-01-01T00:00:00Z"}, {"body":"**Step:** Shepherding\n**Outcome:** nothing eligible\n**Problems:** failed: #4 x","createdAt":"2999-01-01T00:00:00Z"}, {"body":"**Step:** Reviewing\n**Outcome:** 1 merged","createdAt":"2999-01-01T00:00:00Z"}, {"body":"**Step:** Exploring\n**Outcome:** 2 filed (too old)","createdAt":"2000-01-01T00:00:00Z"}] J +) +w --argjson c "$logc" '.issues[0].comments = $c' logs=$(read_logs 14) check "idle entry is skipped" 0 "$(grep -c 'Step:\*\* Triaging' <<<"$logs")" check "idle entry that says Problems: none is skipped too" 0 "$(grep -c 'Things to improve' <<<"$logs")" @@ -360,7 +300,7 @@ check "working entry is kept" 1 "$(grep -c 'Step:\*\* Reviewing' <<<"$ check "entry older than the window is dropped" 0 "$(grep -c 'Exploring' <<<"$logs")" echo "Issue <-> PR links" -cat > "$STATE/prs.json" <<'J' +set_prs <<'J' [{"number":50,"state":"OPEN","body":"x\n","headRefName":"feature/7-foo","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, {"number":51,"state":"MERGED","body":"Closes #8.","headRefName":"bugfix/zzz","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, {"number":52,"state":"CLOSED","body":"nothing","headRefName":"feature/9-bar","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, @@ -382,18 +322,20 @@ check "issue_for_pr (Closes)" 8 "$(issue_for_pr 51)" check "a MEMBER's marker is a link" "56 OPEN" "$(prs_for_issue 13)" check "issue_for_pr reads GitHub's own links" 14 "$(issue_for_pr 57)" check "issue_for_pr joins the body and GitHub's links, once each, in numeric order" "3 15 16" "$(issue_for_pr 58 | tr '\n' ' ' | sed 's/ $//')" -check "an old gh without closingIssuesReferences still reads the body" "8" "$(STUB_OLD_GH=1 issue_for_pr 51)" +fault '[{"match":"pr view","fail":"Unknown JSON field: \"closingIssuesReferences\"\nAvailable fields:\n additions"}]' # gh before 2.72 +check "an old gh without closingIssuesReferences still reads the body" "8" "$(issue_for_pr 51)" +no_faults issue_for_pr 999 >/dev/null 2>&1; check "issue_for_pr fails for a PR that is not there" 1 $? -gh pr list --json number,authorAssociation >/dev/null 2>&1; check "gh has no authorAssociation on pr list (the stub refuses it as gh does)" 1 $? +gh pr list --json number,authorAssociation >/dev/null 2>&1; check "gh has no authorAssociation on pr list (the simulator refuses it as gh does)" 1 $? gh pr view 50 --json body,authorAssociation >/dev/null 2>&1; check "gh has no authorAssociation on pr view either" 1 $? jq -n '[range(1;231) | {number:(1000+.),state:"CLOSED",body:"",headRefName:"x",closing:[],authorAssociation:"NONE",author:{login:"z"}}] | .[119] |= (.body="" | .authorAssociation="OWNER") - | .[219] |= (.body="" | .authorAssociation="OWNER")' > "$STATE/prs.json" + | .[9] |= (.body="" | .authorAssociation="OWNER")' | set_prs check "a PR on the second page of PRs is found" "1120 CLOSED" "$(prs_for_issue 20)" check "a PR past the 200 newest is not (the cap is as it was)" "" "$(prs_for_issue 21)" echo "Trust" -cat > "$STATE/prs.json" <<'J' +set_prs <<'J' [{"number":60,"state":"OPEN","body":"","headRefName":"x","closing":[],"authorAssociation":"NONE","author":{"login":"mallory"}}, {"number":61,"state":"OPEN","body":"","headRefName":"x","closing":[],"authorAssociation":"NONE","author":{"login":"agent-bot"}}] J @@ -408,8 +350,8 @@ echo "Shared marker fixtures (the same cases tests/helpers.test.mjs runs)" FIX="$PKG/tests/fixtures/markers.json" for i in $(jq -r 'keys[]' "$FIX"); do name=$(jq -r ".[$i].name" "$FIX"); n=$((900 + i)) - echo '[]' > "$STATE/comments_$n.json" - jq --argjson i "$i" '[.[$i].comments[] | {body, author_association:(.author_association // "OWNER"), user:{login:(.login // "maintainer")}} + (if .updated_at then {created_at:.updated_at, updated_at:.updated_at} else {} end)]' "$FIX" > "$STATE/comments_$n.json" + ensure "$n" + w --argjson n "$n" --argjson c "$(jq --argjson i "$i" '[.[$i].comments[] | {text:.body, association:(.author_association // "OWNER"), author:(.login // "maintainer")} + (if .updated_at then {createdAt:.updated_at, updatedAt:.updated_at} else {} end)]' "$FIX")" '.issues |= map(if .number == $n then .comments = $c else . end)' extra=$(jq -r ".[$i].trusted_authors // [] | join(\",\")" "$FIX") run() { trusted_authors="$extra" bash -c 'source "$1"; shift; "$@"' _ "$LIB" "$@"; } for k in review_rounds open_findings ready_sha shepherd_attempts; do @@ -427,6 +369,7 @@ for i in $(jq -r 'keys[]' "$FIX"); do done echo "Nested calls borrow the caller's lease" +items 1 40 # the run log section started from an empty repository check "lease_held default" "false" "$(bash -c 'source "$1"; echo "$lease_held"' _ "$LIB")" lease_held=true claim 33 issue agent:triage Triaging 1h has_label 33 issue agent:triage; check "claim does nothing when lease_held=true" 1 $? @@ -452,17 +395,21 @@ done echo "Client guidance" -echo '{"tree":[{"type":"blob","path":"README.md"},{"type":"blob","path":"AGENTS.md"},{"type":"blob","path":"docs/CONTRIBUTING.md"},{"type":"blob","path":".github/copilot-instructions.md"},{"type":"blob","path":".github/ISSUE_TEMPLATE/bug.yml"},{"type":"blob","path":"src/AGENTS.md"},{"type":"blob","path":"node_modules/x/AGENTS.md"},{"type":"blob","path":"docs/house-rules.md"},{"type":"tree","path":".github/instructions"}]}' > "$STATE/tree.json" +w '.files = {"README.md":"","AGENTS.md":"","docs/CONTRIBUTING.md":"",".github/copilot-instructions.md":"",".github/ISSUE_TEMPLATE/bug.yml":"","src/AGENTS.md":"","node_modules/x/AGENTS.md":"","docs/house-rules.md":""}' check "client_guidance_files finds the standard files" \ ".github/ISSUE_TEMPLATE/bug.yml,.github/copilot-instructions.md,AGENTS.md,docs/CONTRIBUTING.md,src/AGENTS.md" \ "$(client_guidance_files | paste -sd, -)" check "repo_guidance adds a file the standard list misses" \ ".github/ISSUE_TEMPLATE/bug.yml,.github/copilot-instructions.md,AGENTS.md,docs/CONTRIBUTING.md,docs/house-rules.md,src/AGENTS.md" \ "$(repo_guidance='docs/house-rules.md' client_guidance_files | paste -sd, -)" -echo '[{"name":"evergreen","description":"Recurring enhancement that stays open indefinitely"},{"name":"bug","description":"Something is broken"}]' > "$STATE/labels.json" -printf 'evergreen\n' > "$STATE/labels_issue_90" +w '.labels = [{"name":"evergreen","description":"Recurring enhancement that stays open indefinitely"},{"name":"bug","description":"Something is broken"}] | .issues += [{number:90,state:"open",labels:["evergreen"]}]' check "item_labels prints the label description" "evergreen: Recurring enhancement that stays open indefinitely" "$(item_labels 90 issue)" -printf 'evergreen\nno-description\n' > "$STATE/labels_issue_91" +w '.issues += [{number:91,state:"open",labels:["evergreen","no-description"]}]' check "item_labels keeps a label with no description" "evergreen: Recurring enhancement that stays open indefinitely|no-description: " "$(item_labels 91 issue | paste -sd'|' -)" +echo "The gh simulator" +# A call the simulator does not cover fails, and a helper may swallow the failure. The log does not forget it. +check "no call in this run was one the simulator does not cover" "" "$(jq -r 'select(.kind == "unsupported") | .argv | join(" ")' "$GH_SIM_LOG")" +check "the log recorded the calls the tests made" 1 "$([ "$(wc -l < "$GH_SIM_LOG")" -gt 300 ] && echo 1 || echo 0)" + echo; echo "passed=$pass failed=$fail"; [ "$fail" -eq 0 ] diff --git a/tests/sim/bin/gh b/tests/sim/bin/gh index bb78e37..0e30c9d 100755 --- a/tests/sim/bin/gh +++ b/tests/sim/bin/gh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # The `gh` simulator. Put this directory first on PATH, name a world file in GH_SIM_WORLD, and every # `gh` call is answered from that world and appended to GH_SIM_LOG. See docs/adr/0014-gh-simulator.md. -exec node "$(dirname "${BASH_SOURCE[0]}")/../gh-sim.mjs" "$@" +exec node "${BASH_SOURCE[0]%/*}/../gh-sim.mjs" "$@" diff --git a/tests/sim/gh-sim.mjs b/tests/sim/gh-sim.mjs index 9b076a6..53d1291 100644 --- a/tests/sim/gh-sim.mjs +++ b/tests/sim/gh-sim.mjs @@ -7,10 +7,11 @@ // A command, flag or --json field the simulator does not cover exits 1 with `unsupported: `. import { appendFileSync, readFileSync } from "node:fs"; +import { createHash } from "node:crypto"; import { spawnSync } from "node:child_process"; import { SURFACE } from "./surface.mjs"; import { applyEffects } from "../../scripts/scenario.mjs"; -import { SimError, unsupported, loadWorld, saveWorld, contextOf, view, commentsOf, checksOf, runId, branchOf, createdAt, iso, GH_FIELDS, MODELLED } from "./world.mjs"; +import { SimError, unsupported, loadWorld, saveWorld, contextOf, view, commentsOf, derivedCount, checksOf, runId, branchOf, createdAt, iso, mergeStateOf, restComment, restPr, GH_FIELDS, MODELLED } from "./world.mjs"; // ---- arguments --------------------------------------------------------------------------------------------- @@ -33,7 +34,8 @@ function parseArgs(command, argv, flags) { if (!spec.takes) { out.flags[key] = true; continue; } const value = inline ?? argv[++i]; if (value === undefined) throw new SimError(`flag needs an argument: ${name}`); - if (spec.multi) (out.flags[key] ??= []).push(...value.split(",")); + if (spec.repeat) (out.flags[key] ??= []).push(value); + else if (spec.multi) (out.flags[key] ??= []).push(...value.split(",")); else out.flags[key] = value; } return out; @@ -379,7 +381,7 @@ const prMerge = { ...WRITE, flags: { ...REPO_FLAG, "--squash": { short: "-s" }, if (a.flags["delete-branch"] && !pr.branchDeleted) ctx.world = applyEffects(ctx.world, [{ op: "delete_branch", target: t }]); return { text: "", err: a.flags["delete-branch"] ? `! Pull request ${full} was already merged\n` : "", code: 0 }; } - const merge = pr.mergeState ?? (pr.draft ? "DRAFT" : "CLEAN"); + const merge = mergeStateOf(pr); const blocked = { DIRTY: "the merge commit cannot be cleanly created", BLOCKED: "the base branch policy prohibits the merge", BEHIND: "the head branch is not up to date with the base branch" }[merge]; if (blocked) throw new SimError(`X Pull request ${full} is not mergeable: ${blocked}.\nTo have the pull request merged after all the requirements have been met, add the \`--auto\` flag.`); if (pr.draft || merge === "DRAFT") throw new SimError("GraphQL: Pull request Pull request is still a draft (mergePullRequest)"); @@ -404,6 +406,133 @@ const labelCreate = { ...WRITE, flags: { ...REPO_FLAG, "--color": { short: "-c", return `✓ Label "${name}" ${exists ? "updated" : "created"} in ${ctx.nameWithOwner}\n`; } }; +// ---- api ----------------------------------------------------------------------------------------------------- +// +// The REST calls `lib.sh` and the skills make. Anything else is `unsupported`. Pagination is not modelled: a list is +// one page unless `per_page` and `page` are in the query, and `--paginate` returns it whole. + +const apiError = (code, message) => new SimError(`gh: ${message} (HTTP ${code})`); +const notFound = () => apiError(404, "Not Found"); + +const textOf = (v) => (typeof v === "string" ? v : JSON.stringify(v)); + +/** A list as the API pages it: `per_page` and `page` in the query, unless `--paginate` asks for all of it. */ +function page(list, q, a) { + if (a.flags.paginate) return list; + const per = Number(q.get("per_page") ?? 30), n = Number(q.get("page") ?? 1); + return list.slice((n - 1) * per, n * per); +} + +function requireWritable(ctx, what) { + if (ctx.scenario) throw new SimError(`GH_SIM_WORLD is a scenario file, and the gh simulator does not write to one (${what})`, { code: 2 }); +} + +function updateBranch(ctx, n, fields) { + const pr = findItem(ctx.world, "pr", n); + if (!pr) throw notFound(); + const expected = fields.expected_head_sha; + if (expected !== undefined && expected !== String(pr.head)) throw apiError(422, "expected head sha didn't match current head ref."); + if ((pr.state ?? "open") !== "open") throw apiError(422, "Pull request is not open"); + const merge = mergeStateOf(pr); + if (merge === "DIRTY") throw apiError(422, "merge conflict between base and head"); + if (merge !== "BEHIND") throw apiError(422, "There are no new commits on the base branch."); + requireWritable(ctx, "update-branch"); + // The branch gets a new head, and a PR that was `BEHIND` is up to date (`mergeStateOf` knows a run pushed it). + ctx.world = applyEffects(ctx.world, [{ op: "push", target: `pr#${n}` }]); + return { message: "Updating pull request branch.", url: `https://api.github.com/repos/${ctx.nameWithOwner}/pulls/${n}` }; +} + +function patchComment(ctx, id, fields) { + if (fields.body === undefined) throw apiError(422, "Validation Failed"); + const n = Math.floor((id - 1) / 1000), i = (id - 1) % 1000; + const kind = findItem(ctx.world, "issue", n) ? "issue" : findItem(ctx.world, "pr", n) ? "pr" : null; + const it = kind && findItem(ctx.world, kind, n); + if (!it || i >= commentsOf(ctx, kind, it).length) throw notFound(); + const stored = i - derivedCount(ctx, kind, it); + if (stored < 0) throw unsupported("api PATCH of a comment the simulator derives from a world field (a lease, a handoff, an attempt)"); + requireWritable(ctx, "PATCH"); + const marker = /