From c0441585a763a8636ef686bd83c128a79196f46f Mon Sep 17 00:00:00 2001 From: glennfaison Date: Tue, 10 Nov 2026 19:44:07 +0000 Subject: [PATCH 1/3] test: the gh simulator's api and fault injection (#88) Part of #60. gh api answers the nine REST calls lib.sh and the skills make (user, an item's comments, PATCH of a comment, a PR's files, contents, the tree, the PR list, one PR, update-branch). A world's faults make a matching call fail without changing anything, or apply effects first, and are marked in the call log. The checker gets a comment_edit effect, so a lease edited in place is read back by diffWorlds. Co-authored-by: Claude --- scripts/scenario.mjs | 20 +++- tests/diff-worlds.test.mjs | 20 +++- tests/gh-sim.test.mjs | 231 ++++++++++++++++++++++++++++++++++++- tests/sim/gh-sim.mjs | 192 +++++++++++++++++++++++++++--- tests/sim/surface.mjs | 2 +- tests/sim/world.mjs | 40 ++++++- 6 files changed, 483 insertions(+), 22 deletions(-) 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..d1ed034 100644 --- a/tests/gh-sim.test.mjs +++ b/tests/gh-sim.test.mjs @@ -396,7 +396,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 +725,235 @@ 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("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'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"]); +}); + // ---- seeding ------------------------------------------------------------------------------------------------- test("seed makes a world file from a scenario, and reports the fields no gh call shows", () => { diff --git a/tests/sim/gh-sim.mjs b/tests/sim/gh-sim.mjs index 9b076a6..bae6b81 100644 --- a/tests/sim/gh-sim.mjs +++ b/tests/sim/gh-sim.mjs @@ -10,7 +10,7 @@ import { appendFileSync, readFileSync } from "node:fs"; 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 +33,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 +380,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 +405,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 = /" 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/gh-sim.mjs b/tests/sim/gh-sim.mjs index bae6b81..53d1291 100644 --- a/tests/sim/gh-sim.mjs +++ b/tests/sim/gh-sim.mjs @@ -7,6 +7,7 @@ // 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"; @@ -502,7 +503,7 @@ const api = { if (method === "GET" && (g = /^pulls\/(\d+)\/files$/.exec(rest))) { const pr = findItem(ctx.world, "pr", Number(g[1])); if (!pr) throw notFound(); - return out(page((pr.files ?? []).map((filename) => ({ sha: "sim", filename, status: "modified", additions: 0, deletions: 0, changes: 0 })), q, a)); + return out(page((pr.files ?? []).map((f) => (typeof f === "string" ? { sha: "sim", filename: f, status: "modified", additions: 0, deletions: 0, changes: 0 } : { sha: "sim", filename: f.path, previous_filename: f.previous, status: "renamed", additions: 0, deletions: 0, changes: 0 })), q, a)); } if (method === "PUT" && (g = /^pulls\/(\d+)\/update-branch$/.exec(rest))) return out(updateBranch(ctx, Number(g[1]), fields)); if (method === "GET" && (g = /^pulls\/(\d+)$/.exec(rest))) { @@ -547,19 +548,23 @@ export const HANDLERS = { * The fault, if any, that this call meets. A world's `faults` are `{match, fail, exit?, before?, times?}`: a call that * starts with `match` (whole words: `pr merge`, `api -X PATCH`) fails with `fail` and exits `exit` (default 1) without * changing anything. `before` is a list of effects applied to the world first, which is how another run gets in ahead of - * this call. `times` limits how often a fault fires. It counts the log, so it needs GH_SIM_LOG. + * this call. `times` limits how often a fault fires. A fault is known by its definition, so firing is counted in the log + * under that (`faultId`, a hash of the definition), which needs GH_SIM_LOG. Another fault that is later at the same place in + * the list does not inherit the count, and the same fault written again does. */ +const faultId = (f) => createHash("sha1").update(JSON.stringify([f.match, f.fail, f.exit, f.before, f.times])).digest("hex").slice(0, 10); function faultFor(world, argv, env) { const call = argv.join(" "); - const fired = (i) => { + const fired = (id) => { if (!env.GH_SIM_LOG) throw new SimError("GH_SIM_WORLD: a fault with `times` needs GH_SIM_LOG, which is where firing is counted", { code: 2 }); let text = ""; try { text = readFileSync(env.GH_SIM_LOG, "utf8"); } catch { /* no call has been logged yet */ } - return text.split("\n").filter((l) => l && JSON.parse(l).fault === i).length; + return text.split("\n").filter((l) => l && JSON.parse(l).faultId === id).length; }; for (const [i, f] of (world.faults ?? []).entries()) { if (call !== f.match && !call.startsWith(`${f.match} `)) continue; - if (f.times !== undefined && fired(i) >= f.times) continue; - return { i, f }; + const id = faultId(f); + if (f.times !== undefined && fired(id) >= f.times) continue; + return { i, f, id }; } return null; } @@ -569,7 +574,7 @@ export function run(argv, env = process.env) { const words = []; for (const w of argv) { if (w.startsWith("-") || words.length === 2) break; words.push(w); } const command = words.slice(0, 2).join(" ") || words[0]; - let fault; + let fault, id; try { if (!env.GH_SIM_WORLD) throw new SimError("GH_SIM_WORLD is not set: the gh simulator needs a world file", { code: 2 }); const handler = HANDLERS[command] ?? HANDLERS[words[0]]; @@ -579,13 +584,13 @@ export function run(argv, env = process.env) { let world = seeded; const hit = faultFor(world, argv, env); if (hit) { - fault = hit.i; + fault = hit.i; id = hit.id; if (hit.f.before) { if (scenario) throw refuse(" (a fault's `before` effects write)"); world = applyEffects(world, hit.f.before); saveWorld(env.GH_SIM_WORLD, world); } - if (hit.f.fail !== undefined) throw Object.assign(new SimError(hit.f.fail, { code: hit.f.exit ?? 1, kind: "fault" }), { fault }); + if (hit.f.fail !== undefined) throw Object.assign(new SimError(hit.f.fail, { code: hit.f.exit ?? 1, kind: "fault" }), { fault, faultId: id }); } const ctx = contextOf(world, env); ctx.scenario = scenario; @@ -595,10 +600,10 @@ export function run(argv, env = process.env) { const result = handler.run(ctx, a); // Only a call that got this far changes the file. if (writes) saveWorld(env.GH_SIM_WORLD, ctx.world); - return typeof result === "string" ? { stdout: result, code: 0, kind: "ok", fault } : { stdout: result.text, stderr: result.err, code: result.code, kind: "ok", fault }; + return typeof result === "string" ? { stdout: result, code: 0, kind: "ok", fault, faultId: id } : { stdout: result.text, stderr: result.err, code: result.code, kind: "ok", fault, faultId: id }; } catch (e) { if (!(e instanceof SimError)) throw e; - return { stdout: "", stderr: e.message + "\n", code: e.code, kind: e.kind, fault: e.fault ?? fault }; + return { stdout: "", stderr: e.message + "\n", code: e.code, kind: e.kind, fault: e.fault ?? fault, faultId: e.faultId ?? id }; } } @@ -607,7 +612,7 @@ if (import.meta.url === `file://${process.argv[1]}` || process.argv[1]?.endsWith const r = run(argv); if (process.env.GH_SIM_LOG) { const ts = new Date(process.env.GH_SIM_NOW ? Date.parse(process.env.GH_SIM_NOW) : Date.now()).toISOString(); - appendFileSync(process.env.GH_SIM_LOG, JSON.stringify({ ts, argv, exit: r.code, kind: r.kind, ...(r.fault !== undefined ? { fault: r.fault } : {}) }) + "\n"); + appendFileSync(process.env.GH_SIM_LOG, JSON.stringify({ ts, argv, exit: r.code, kind: r.kind, ...(r.fault !== undefined ? { fault: r.fault, faultId: r.faultId } : {}) }) + "\n"); } if (r.stdout) process.stdout.write(r.stdout); if (r.stderr) process.stderr.write(r.stderr); diff --git a/tests/sim/surface.mjs b/tests/sim/surface.mjs index 3e3a6d4..3cd0a70 100644 --- a/tests/sim/surface.mjs +++ b/tests/sim/surface.mjs @@ -1,11 +1,11 @@ // What the `gh` simulator covers. One entry per `gh` command the skills, the scripts and `lib.sh` use. // // supported the simulator answers it -// planned a later part of the simulator will answer it; today it exits `unsupported: ` // out-of-scope the package deliberately does not use it, or the simulator will not model it // // tests/gh-sim.test.mjs greps skills/ for `gh` calls and fails for one that is missing here, so a new -// `gh` call in a skill is noticed. The list of `supported` entries must equal the simulator's handlers. +// `gh` call in a skill is noticed. The `supported` entries must equal the simulator's handlers, and an entry that no +// skill uses is stale. `planned` is not a status any more: every command is supported or deliberately out of scope. export const SURFACE = { "issue list": "supported", diff --git a/tests/sim/world.mjs b/tests/sim/world.mjs index a80f7af..f06613e 100644 --- a/tests/sim/world.mjs +++ b/tests/sim/world.mjs @@ -133,7 +133,7 @@ export function commentsOf(ctx, kind, it) { const base = Date.parse(createdAt(it)); const stored = (it.comments ?? []).map((c) => ({ login: c.author === "agent" ? ctx.user : c.author === "human" || c.author === undefined ? "someone" : c.author, - association: c.trusted ? "OWNER" : "NONE", + association: c.association ?? (c.trusted ? "OWNER" : "NONE"), body: bodyOf(c), at: c.createdAt, updatedAt: c.updatedAt, @@ -195,7 +195,7 @@ const build = { headRefName: branchOf(it), headRefOid: String(it.head ?? ""), baseRefName: it.base ?? ctx.defaultBranch, isCrossRepository: Boolean(it.fork), mergeStateStatus: merge, mergeable: merge === "DIRTY" ? "CONFLICTING" : merge === "UNKNOWN" ? "UNKNOWN" : "MERGEABLE", - files: (it.files ?? []).map((path) => ({ path, additions: 0, deletions: 0 })), changedFiles: (it.files ?? []).length, additions: 0, deletions: 0, + files: (it.files ?? []).map((f) => ({ path: typeof f === "string" ? f : f.path, additions: 0, deletions: 0 })), changedFiles: (it.files ?? []).length, additions: 0, deletions: 0, closingIssuesReferences: (it.closes ?? []).map((n) => ({ number: n, url: `https://github.com/${ctx.nameWithOwner}/issues/${n}`, repository: { name: ctx.name, owner: { login: ctx.owner } } })), reviewRequests: [], reviewDecision: "", statusCheckRollup: checks.map((c) => ({ __typename: "CheckRun", name: c.name, workflowName: c.workflow, status: c.bucket === "pending" ? "IN_PROGRESS" : "COMPLETED", conclusion: c.bucket === "pending" ? "" : c.state, detailsUrl: c.link, startedAt: c.startedAt, completedAt: c.completedAt })), From 3117ce27400ecb42c4504fbec035bdb81e5bf3b9 Mon Sep 17 00:00:00 2001 From: glennfaison Date: Wed, 11 Nov 2026 01:01:18 +0000 Subject: [PATCH 3/3] test: say what the simulator costs, and drop a fork from the wrapper (#88) Co-authored-by: Claude --- docs/adr/0014-gh-simulator.md | 1 + tests/sim/bin/gh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/adr/0014-gh-simulator.md b/docs/adr/0014-gh-simulator.md index f405328..268f189 100644 --- a/docs/adr/0014-gh-simulator.md +++ b/docs/adr/0014-gh-simulator.md @@ -75,6 +75,7 @@ A world (and so a scenario's `world:`) may carry `faults: [{match, fail, exit?, - **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 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" "$@"