test: the gh simulator writes (#87)

.changeset/gh-simulator-write-paths.md

@@ -0,0 +1,5 @@
+---
+"github-agent-workflow": patch
+---
+
+The `gh` simulator for tests now writes. `issue edit|close|create|comment|reopen`, `pr edit|create|comment|merge` and `label create` change the world file the way `gh` would and refuse what `gh` refuses (an unknown label, a merge pinned to the wrong head, a duplicate PR), leaving the file untouched when a call fails. A run's changes are read back by `diffWorlds`. For that, a `create_issue` or `create_pr` effect can carry the new item's title, body, creation time, author and branch as `data`, and a `close` effect its `reason`.

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, comes out as an `unmodelled` effect, which the checker rejects unless the contract lists an `unmodelled` write. The part that is not built is the part that needs infrastructure: a `gh` simulator over a world file, and 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`, 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.
 
 ## What this does not prove
 

docs/adr/0014-gh-simulator.md

@@ -2,7 +2,7 @@
 
 ## Status
 
-Accepted. The read paths are built (#86). The write paths (#87), `api`, fault injection and the replacement of the stub in `tests/lib.test.sh` (#88) follow, and this record is updated with each.
+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.
 
 ## Context
 
@@ -24,9 +24,22 @@ ADR 0012 checks a scenario against a skill's contract, and `diffWorlds` turns th
 
 `issue list|view`, `pr list|view|checks`, `label list`, `repo view`, `auth status` and `run view --log|--log-failed`, with `--json`, `--jq`, `--label`, `--state`, `--limit`, `--search` and `-R`. `--search` matches every word in the title or body, and `closed:>=DATE`. It is not GitHub's search.
 
+### Write paths (#87)
+
+`issue edit|close|create|comment|reopen`, `pr edit|create|comment|merge` and `label create`, with the flags the skills use: `--add-label`, `--remove-label`, `--add-reviewer`, `--title`, `--body`, `--body-file` (a path, or `-` for stdin), `--reason`, `--comment`, `--label`, `--base`, `--head`, `--draft`, `--match-head-commit`, `--squash|--merge|--rebase`, `--delete-branch`, `--color`, `--description` and `--force`.
+
+- **A write is an effect.** Each call is applied with `applyEffects`, the function the checker uses, so a run made of these calls is read back by `diffWorlds` as the effects it had, and `applyEffects(before, diffWorlds(before, after))` gives the world the run left (tested, and tested on the effects of every golden scenario made as `gh` calls). Two things the checker's effect model could not carry were added to it: a created item's title, body, creation time, author and branch travel in the `create_*` effect as `data`, and a `close` carries its `reason`.
+- **A call is checked before it changes anything.** A failing call exits non-zero and never writes. The file is written whole, in its own format, through a temporary file and a rename, so it is never half written. A call that succeeds is the only one that saves.
+- **It is refused like `gh` refuses it.** A label outside the catalog (`'x' not found`, only in a world that has a `labels` catalog), an empty comment, a missing `--title` or `--body`, a second open PR for the same branch, a merge without a method or with two, a merge pinned to a head that is not the head, a merge of a draft, of a PR that is `DIRTY`, `BEHIND` or `BLOCKED`, and an invalid `--reason`.
+- **It never writes to a scenario file.** A write against one exits 2 and says how to make a world file (`seed.mjs`).
+- **Written comments are the agent's own.** The author is the world's agent login (`user`, default `agent-bot`), the time is the call's clock, and the comment's marker is the first `<!-- agent:... -->` in its body. A comment the run just posted is therefore fresh: a ready handoff holds the merge lane.
+- **A merge does not close the issue.** The skills settle an issue after its PR merges (`issue-after-pr.md`), with a keep-open rule that GitHub's own closing would hide, so the simulator leaves it open.
+- **`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.
+
 ## What it does not do
 
-- **It is not GitHub.** Rate limits, pagination, webhooks and merge queues are not modelled.
+- **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/<closed issue>-pr-<N>`, which matches the pattern `prs_for_issue` reads, unless it names a `branch`. A check is `check-<i>`. 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.

scripts/scenario.mjs

@@ -195,10 +195,16 @@ export function scopeOf(effect, ctx) {
 
 /** The item that a `create_pr` or `create_issue` effect adds to the world `w`. */
 export function createdItem(w, e) {
-  if (e.op === "create_pr") return { number: e.number, state: "open", labels: e.labels ?? [], draft: false, head: e.sha ?? "new", base: e.base ?? w.default_branch ?? "main", checks: [], openFindings: 0, closes: e.closes ?? [], comments: [] };
-  return { number: e.number, state: "open", labels: e.labels ?? [], comments: [] };
+  if (e.op === "create_pr") return { ...e.data, number: e.number, state: "open", labels: e.labels ?? [], draft: false, head: e.sha ?? "new", base: e.base ?? w.default_branch ?? "main", checks: [], openFindings: 0, closes: e.closes ?? [], comments: [] };
+  return { ...e.data, number: e.number, state: "open", labels: e.labels ?? [], comments: [] };
 }
 
+/**
+ * What a new item is made with, besides what a `create_*` effect names: its title, its body, when it was created, who
+ * opened it and (for a PR) its branch. A `create_*` effect carries these as `data`. Editing one later is still `unmodelled`.
+ */
+export const CREATION_DATA = ["title", "body", "createdAt", "author", "branch"];
+
 /** Apply effects to a copy of the world. Throws when an effect is impossible. */
 export function applyEffects(world, effects) {
   const w = structuredClone(world);
@@ -230,7 +236,7 @@ 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;
-      case "close": t.state = "closed"; 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;
       case "delete_branch": t.branchDeleted = true; break;
@@ -266,7 +272,7 @@ export function canonicalWorld(world) {
 /** True when two worlds are the same world by `canonicalWorld`. */
 export const sameWorld = (a, b) => isDeepStrictEqual(canonicalWorld(a), canonicalWorld(b));
 
-const MODELLED = { issue: ["number", "labels", "comments", "state"], pr: ["number", "labels", "comments", "state", "head", "pushed", "branchDeleted"] };
+const MODELLED = { issue: ["number", "labels", "comments", "state", "closeReason"], pr: ["number", "labels", "comments", "state", "head", "pushed", "branchDeleted"] };
 const defined = (o) => Object.fromEntries(Object.entries(o).filter(([, v]) => v !== undefined));
 
 /**
@@ -276,7 +282,7 @@ const defined = (o) => Object.fromEntries(Object.entries(o).filter(([, v]) => v
  * `agent:lease` comment are ordinary effects here: the checker adds a claim's marker to a scenario's
  * effects itself, so a caller that passes a diff to `checkScenario` leaves that comment out of it.
  *
- * Anything that no op can express (a changed title, body, checks or base; an edited or removed comment;
+ * Anything that no op can express (a changed title, body, checks or base of an item that already existed; an edited or removed comment;
  * a reopened item; a removed item) becomes `{ op: "unmodelled", target, field, before, after }`.
  * `checkScenario` rejects it unless the contract lists an `unmodelled` write scope. `review_request`
  * changes nothing in a world, so a diff never contains one.
@@ -307,7 +313,7 @@ export function diffWorlds(before, after) {
 
     if (a.state !== b.state) {
       const open = b.state === "open" || b.state === undefined;
-      if (open && a.state === "closed") out.push({ op: "close", target });
+      if (open && a.state === "closed") out.push({ op: "close", target, ...defined({ reason: a.closeReason }) });
       else if (open && kind === "pr" && a.state === "merged") out.push({ op: "merge", target, ...defined({ sha: typeof a.head === "string" ? a.head : undefined }) });
       else odd(target, "state", b.state, a.state);
     }
@@ -332,9 +338,11 @@ export function diffWorlds(before, after) {
       const a = now.get(n);
       if (was.has(n)) { diffItem(kind, was.get(n), a); continue; }
       // a new item: create it, then diff what is left against what the creation gives
+      const data = defined(Object.fromEntries(CREATION_DATA.map((k) => [k, a[k]])));
       const e = kind === "pr"
         ? { op: "create_pr", number: n, labels: [...new Set(a.labels ?? [])], ...defined({ sha: typeof a.head === "string" ? a.head : undefined, base: a.base }), closes: a.closes ?? [] }
         : { op: "create_issue", number: n, labels: [...new Set(a.labels ?? [])] };
+      if (Object.keys(data).length > 0) e.data = data;
       created.push(e);
       const stub = applyEffects({ issues: [], prs: [], default_branch: before.default_branch }, [e]);
       diffItem(kind, (kind === "pr" ? stub.prs : stub.issues)[0], a);

tests/diff-worlds.test.mjs

@@ -101,6 +101,32 @@ test("a change no op can express is reported as unmodelled, with both values", (
   ]);
 });
 
+test("what a new item is made with (title, body, when, by whom, branch) belongs to its create effect, not to unmodelled", () => {
+  const issue = { number: 3, state: "open", labels: ["agent:log"], comments: [], title: "Log", body: "b", createdAt: "2026-11-09T12:00:00Z", author: "bot" };
+  const pr = { number: 11, state: "open", labels: [], draft: false, head: "n", base: "main", checks: [], openFindings: 0, closes: [1], comments: [], title: "T", body: "Closes #1", createdAt: "2026-11-09T12:00:00Z", author: "bot", branch: "feat/1-x" };
+  const after = edit((w) => { w.issues.push(issue); w.prs.push(pr); });
+  const diff = diffWorlds(W(), after);
+  assert.deepEqual(diff.filter((e) => e.op === "unmodelled"), []);
+  assert.deepEqual(diff.find((e) => e.op === "create_issue").data, { title: "Log", body: "b", createdAt: "2026-11-09T12:00:00Z", author: "bot" });
+  assert.deepEqual(diff.find((e) => e.op === "create_pr").data, { title: "T", body: "Closes #1", createdAt: "2026-11-09T12:00:00Z", author: "bot", branch: "feat/1-x" });
+  assert.ok(sameWorld(applyEffects(W(), diff), after), "the item is made as it was");
+  assert.deepEqual(diffWorlds(W(), edit((w) => { w.issues.push({ number: 3, state: "open", labels: [], comments: [] }); })), [{ op: "create_issue", number: 3, labels: [] }], "no data, no `data` key");
+});
+
+test("a title or body edited after the item exists is still unmodelled, and a created item's other fields are too", () => {
+  assert.deepEqual(unmodelled(W(), edit((w) => { w.issues[0].body = "new"; })), ["issue#1 body"]);
+  assert.deepEqual(unmodelled(W(), edit((w) => { w.issues.push({ number: 3, state: "open", labels: [], comments: [], title: "T", assignee: "x" }); })), ["issue#3 assignee"]);
+});
+
+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);
+  assert.deepEqual(diff, [{ op: "close", target: "issue#1", reason: "not planned" }]);
+  assert.ok(sameWorld(applyEffects(W(), diff), after));
+  assert.deepEqual(diffWorlds(W(), edit((w) => { w.issues[0].state = "closed"; })), [{ op: "close", target: "issue#1" }], "no reason, no `reason` key");
+  assert.equal(applyEffects(W(), [{ op: "close", target: "issue#1" }]).issues[0].closeReason, undefined);
+});
+
 test("unmodelled: fields, comments, state, branches, removals, new items and the world itself", () => {
   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"]);