diff --git a/.changeset/structured-write-allowances-groups.md b/.changeset/structured-write-allowances-groups.md new file mode 100644 index 0000000..68e8d9c --- /dev/null +++ b/.changeset/structured-write-allowances-groups.md @@ -0,0 +1,5 @@ +--- +"github-agent-workflow": patch +--- + +The remaining contracts (the four group skills, `reconcile-github-issues`, `file-github-issues`, `explore-github-repo` and `setup-github-agent-workflow`) now list what they may write as `allow` entries, which completes ADR 0017: every contract that writes is checked by what it writes and not only by where. An `allow` for `create_issue` and `create_pr` now has a required `labels` list, so "new issues, with no labels" and "a PR labelled agent:needs-work" are checked, and `implement-github-issue` is tightened to match. The wording of the contracts is unchanged. diff --git a/docs/adr/0017-structured-write-allowances.md b/docs/adr/0017-structured-write-allowances.md index 941c23d..bcc53e5 100644 --- a/docs/adr/0017-structured-write-allowances.md +++ b/docs/adr/0017-structured-write-allowances.md @@ -2,7 +2,7 @@ ## Status -Accepted. Steps 1 and 2 of 3 are built (#67): the schema, the checker and `shepherd-github-pr` as the worked example, then the other item skills. The rest move a scope at a time, so each step can be reviewed. +Accepted and built (#67), in three steps so each could be reviewed: the schema, the checker and `shepherd-github-pr` as the worked example, then the other item skills, then the group, sweep, file and setup skills. Every contract that writes now lists its writes as `allow` entries. ## Context @@ -22,6 +22,7 @@ claimed: ``` - `op` is required and is one of the effect ops the checker models, except `unmodelled`. +- `labels` is required for `create_issue` and `create_pr`: the labels the new item may carry, each a name or a glob. Every label it is made with must match one, and `[]` means none. - `label` is required for `label_add` and `label_remove`, and is a name or a glob. `*` is the only wildcard. Use `*` alone to allow any label, which is then a visible choice and not a gap. - `marker` is required for `comment` and `comment_edit`: a name, a glob, or `null` for a comment with no marker (a run-log entry, a one-line comment on a linked issue). A `comment_edit` carries only an index, so its marker is the marker of the comment it edits in the world the run started from. A comment that cannot be found matches nothing. @@ -37,7 +38,8 @@ claimed: ## What it does not do -- **`create_pr`, `create_issue`, `close` and `review_request` are matched by op alone.** What a new item is labelled with, or what a PR is closed for, is not constrained yet. The schema rejects `label` and `marker` on them, so they cannot be mistaken for checked. +- **`close` and `review_request` are matched by op alone.** What an issue is closed for, or who a review is requested from, is not constrained. The schema rejects `label`, `marker` and `labels` on them, so they cannot be mistaken for checked. +- **A created item's other fields are unconstrained.** Only its labels are matched. Its title, body, parent and the PR's base are not. - **The golden scenarios still come from the same reading of the skill.** An `allow` made from the prose and a scenario made from the prose can share a mistake. Running a model (ADR 0016) is what tests that. - **It checks effects, not their order or count.** One extra comment with an allowed marker passes. @@ -48,8 +50,14 @@ claimed: - **The outcome comment has no fixed marker.** The skill text does not name one and the goldens use both `agent:outcome` and none, so triage allows both. Pinning it is a skill change and not part of this one. - **Removing a state label is listed only where the text does it.** Triage may remove `needs-triage` and its own lease, not `needs-info` (its `never` list says only a human does). An issue that already carries a state label is ineligible, so triage never swaps one. +## Notes from migrating the group and file skills + +- **A group's `allow` is declarative for now.** Scenarios already forbid a group from doing item work, with an earlier and stricter rule, so its `shared` entry is exercised by the contract tests and by real runs through `eval`, not by the goldens (they have no effects). +- **`reconcile-github-issues` may add any one of the four state labels and remove the others.** Its repair picks the target by a priority order that the checker does not model, so the list permits the whole mutex set and the repair sentence says so. It never lists `needs-triage`, a lease label to add, or a lease marker. +- **`setup-github-agent-workflow` is migrated too**, though it was not in the plan: its two entries map onto `label_create`, `label_update` and `write_file`. + ## Migration 1. The schema, the checker and `shepherd-github-pr`. Done. 2. The other item skills: `triage-github-issue`, `implement-github-issue`, `review-github-pr`. Done. -3. The group, sweep and file skills: the `-issues` and `-prs` groups, `reconcile-github-issues`, `file-github-issues` and `explore-github-repo`. +3. The four group skills (one run-log entry each), `reconcile-github-issues`, `file-github-issues`, `explore-github-repo` and `setup-github-agent-workflow`. Done. This step added `labels`, so "new issues, with no labels" is checked, and `implement-github-issue`'s PR (`agent:needs-work` only) and sub-issues (none) are now pinned too. diff --git a/scripts/contracts.mjs b/scripts/contracts.mjs index 315ebc8..a42847a 100644 --- a/scripts/contracts.mjs +++ b/scripts/contracts.mjs @@ -21,6 +21,7 @@ const ITEM_OPS = ["label_add", "label_remove", "comment", "comment_edit", "close export const ALLOW_OPS = [...ITEM_OPS, "create_issue", ...REPOSITORY_OPS]; const LABEL_OPS = ["label_add", "label_remove"]; const MARKER_OPS = ["comment", "comment_edit"]; +const CREATE_OPS = ["create_issue", "create_pr"]; // The ops that can land in each scope: scopeOf puts REPOSITORY_OPS in `repository`, a comment on the run log in `shared`, // and a `create_issue` always in `other`. export const SCOPE_OPS = { @@ -51,7 +52,7 @@ export function allowsOf(list) { function checkAllow(a, scope) { if (!a || typeof a !== "object" || Array.isArray(a)) return ["an `allow` must be a mapping with an `op`"]; const out = []; - for (const k of Object.keys(a)) if (!["op", "label", "marker"].includes(k)) out.push(`\`allow\` has unknown key \`${k}\``); + for (const k of Object.keys(a)) if (!["op", "label", "marker", "labels"].includes(k)) out.push(`\`allow\` has unknown key \`${k}\``); if (!ALLOW_OPS.includes(a.op)) return [...out, `\`allow.op\` must be one of ${ALLOW_OPS.join(", ")}`]; if (SCOPE_OPS[scope] && !SCOPE_OPS[scope].includes(a.op)) out.push(`\`${a.op}\` can never land in \`${scope}\`, so this entry would allow nothing`); if (LABEL_OPS.includes(a.op)) { @@ -60,6 +61,9 @@ function checkAllow(a, scope) { if (MARKER_OPS.includes(a.op)) { if (!("marker" in a) || (a.marker !== null && !isText(a.marker))) out.push(`\`${a.op}\` needs a \`marker\`: a name, a glob such as \`agent:*\`, or \`null\` for a comment with no marker`); } else if ("marker" in a) out.push(`\`marker\` applies only to ${MARKER_OPS.join(" and ")}, not \`${a.op}\``); + if (CREATE_OPS.includes(a.op)) { + if (!Array.isArray(a.labels) || !a.labels.every(isText)) out.push(`\`${a.op}\` needs \`labels\`: the labels the new item may carry, each a name or a glob. \`[]\` means none`); + } else if (a.labels !== undefined) out.push(`\`labels\` applies only to ${CREATE_OPS.join(" and ")}, not \`${a.op}\``); return out; } diff --git a/scripts/scenario.mjs b/scripts/scenario.mjs index 52b7ff0..3af7ab3 100644 --- a/scripts/scenario.mjs +++ b/scripts/scenario.mjs @@ -232,6 +232,7 @@ export function allowedBy(effect, allows, ctx) { for (const a of allows) { if (a.op !== effect.op) continue; if (a.label !== undefined && !(typeof effect.label === "string" && globMatches(a.label, effect.label))) continue; + if (a.labels !== undefined && !(effect.labels ?? []).every((l) => a.labels.some((g) => globMatches(g, l)))) continue; if ("marker" in a) { const m = markerOf(effect, ctx); if (a.marker === null ? m !== null : !(typeof m === "string" && globMatches(a.marker, m))) continue; @@ -241,7 +242,7 @@ export function allowedBy(effect, allows, ctx) { return null; } -const describe = (x) => [x.op, x.label !== undefined && `\`${x.label}\``, "marker" in x && (x.marker === null ? "(no marker)" : `\`${x.marker}\``)].filter(Boolean).join(" "); +const describe = (x) => [x.op, x.label !== undefined && `\`${x.label}\``, x.labels !== undefined && (x.labels.length ? `with only ${x.labels.map((l) => `\`${l}\``).join(", ")}` : "with no labels"), "marker" in x && (x.marker === null ? "(no marker)" : `\`${x.marker}\``)].filter(Boolean).join(" "); /** * Why the contract does not allow an effect, or null when it does. First the scope must be one `effects.writes` lists. diff --git a/skills/meta/setup-github-agent-workflow/SKILL.md b/skills/meta/setup-github-agent-workflow/SKILL.md index 95fcb2d..fed906a 100644 --- a/skills/meta/setup-github-agent-workflow/SKILL.md +++ b/skills/meta/setup-github-agent-workflow/SKILL.md @@ -32,8 +32,12 @@ effects: reads: ["`gh auth status`, `gh repo view` and `gh label list`", "the tools and the skills available to the runner", "the repository's agent instructions file"] writes: repository: - - "The labels in `references/labels.md`: created when missing, and the `agent:` ones reset to their description." - - "The `## GitHub workflow` section of the agent instructions file." + - text: "The labels in `references/labels.md`: created when missing, and the `agent:` ones reset to their description." + allow: + - { op: label_create } + - { op: label_update } + - text: "The `## GitHub workflow` section of the agent instructions file." + allow: { op: write_file } external: ["The probes for `gh`, `jq`, `node` and the external skills."] never: - "Write a settings file. A repository's values belong in the prompt or the agent instructions." diff --git a/skills/pipeline/explore-github-repo/SKILL.md b/skills/pipeline/explore-github-repo/SKILL.md index 5690801..7c9add7 100644 --- a/skills/pipeline/explore-github-repo/SKILL.md +++ b/skills/pipeline/explore-github-repo/SKILL.md @@ -32,9 +32,13 @@ effects: reads: [the live app (when app_url is set and the browser stack is present), the code, open issues, recent Exploring log entries] writes: other: - - "New unlabelled issues, and evidence comments on duplicates, through file-github-issues (unless dry_run)." + - text: "New unlabelled issues, and evidence comments on duplicates, through file-github-issues (unless dry_run)." + allow: + - { op: create_issue, labels: [] } + - { op: comment, marker: null } shared: - - "One run-log entry." + - text: "One run-log entry." + allow: { op: comment, marker: null } external: ["A browser session against app_url, when in scope."] never: - "Apply, remove or change any label." diff --git a/skills/pipeline/file-github-issues/SKILL.md b/skills/pipeline/file-github-issues/SKILL.md index cd1e761..e4add49 100644 --- a/skills/pipeline/file-github-issues/SKILL.md +++ b/skills/pipeline/file-github-issues/SKILL.md @@ -50,8 +50,10 @@ effects: reads: [open issues, recently closed issues (dedupe_closed_days), ".out-of-scope/", the repository's own guidance and issue templates] writes: other: - - "New issues, with no labels (unless dry_run)." - - "One evidence comment on an open issue that reports the same problem (unless dry_run)." + - text: "New issues, with no labels (unless dry_run)." + allow: { op: create_issue, labels: [] } + - text: "One evidence comment on an open issue that reports the same problem (unless dry_run)." + allow: { op: comment, marker: null } never: - "Apply, remove or change any label." - "Comment on, close or edit an issue other than the evidence comment in the dedupe step." diff --git a/skills/pipeline/implement-github-issue/SKILL.md b/skills/pipeline/implement-github-issue/SKILL.md index bd9f7ba..df481ac 100644 --- a/skills/pipeline/implement-github-issue/SKILL.md +++ b/skills/pipeline/implement-github-issue/SKILL.md @@ -80,12 +80,12 @@ effects: - text: "A feature or bugfix branch pushed to origin, with commits." allow: { op: push } - text: "One PR against pr_base_branch, labelled agent:needs-work." - allow: { op: create_pr } + allow: { op: create_pr, labels: [agent:needs-work] } other: - text: "agent:needs-work on an existing OPEN agent PR for the issue." allow: { op: label_add, label: agent:needs-work } - text: "New sub-issues through file-github-issues when the issue is split, each with no labels, each linked to the issue as a sub-issue." - allow: { op: create_issue } + allow: { op: create_issue, labels: [] } shared: - text: "One run-log entry (when write_log is true)." allow: { op: comment, marker: null } diff --git a/skills/pipeline/implement-github-issues/SKILL.md b/skills/pipeline/implement-github-issues/SKILL.md index ba533f5..b8c0b4f 100644 --- a/skills/pipeline/implement-github-issues/SKILL.md +++ b/skills/pipeline/implement-github-issues/SKILL.md @@ -31,7 +31,8 @@ effects: reads: [open issues labelled ready-for-agent] writes: shared: - - "One run-log entry." + - text: "One run-log entry." + allow: { op: comment, marker: null } never: - "Implement an issue itself, or claim one. Only implement-github-issue claims." - "Wait for checks or fix CI. shepherd-github-prs does that." diff --git a/skills/pipeline/reconcile-github-issues/SKILL.md b/skills/pipeline/reconcile-github-issues/SKILL.md index 4e9e5e5..f6cf425 100644 --- a/skills/pipeline/reconcile-github-issues/SKILL.md +++ b/skills/pipeline/reconcile-github-issues/SKILL.md @@ -46,14 +46,47 @@ effects: reads: [open issues with their labels and comments, "prs_for_issue", "markers", "children_of", the state of each sub-issue, item labels, "lease_live", the repository's own guidance] writes: other: - - "Closes an issue whose PR merged, with a comment, and removes agent:pr-open and ready-for-agent from it (unless dry_run)." - - "Closes a split issue whose sub-issues are all closed as completed, with a comment, and removes ready-for-agent from it. A keep-open one is not closed and swaps ready-for-agent for ready-for-human, with a comment (unless dry_run)." - - "Swaps ready-for-agent for ready-for-human, with a comment, on a split issue that has a sub-issue closed as not planned (unless dry_run)." - - "Swaps agent:pr-open or ready-for-agent for ready-for-human, with a comment, when every PR was closed unmerged (unless dry_run)." - - "Repairs conflicting state labels in one edit per issue, with a comment when it falls back to ready-for-human (unless dry_run)." - - "Removes a stale agent:triage or agent:in-progress label (unless dry_run)." + - text: "Closes an issue whose PR merged, with a comment, and removes agent:pr-open and ready-for-agent from it (unless dry_run)." + allow: + - { op: close } + - { op: comment, marker: null } + - { op: label_remove, label: agent:pr-open } + - { op: label_remove, label: ready-for-agent } + - text: "Closes a split issue whose sub-issues are all closed as completed, with a comment, and removes ready-for-agent from it. A keep-open one is not closed and swaps ready-for-agent for ready-for-human, with a comment (unless dry_run)." + allow: + - { op: close } + - { op: comment, marker: null } + - { op: label_remove, label: ready-for-agent } + - { op: label_add, label: ready-for-human } + - text: "Swaps ready-for-agent for ready-for-human, with a comment, on a split issue that has a sub-issue closed as not planned (unless dry_run)." + allow: + - { op: comment, marker: null } + - { op: label_remove, label: ready-for-agent } + - { op: label_add, label: ready-for-human } + - text: "Swaps agent:pr-open or ready-for-agent for ready-for-human, with a comment, when every PR was closed unmerged (unless dry_run)." + allow: + - { op: comment, marker: null } + - { op: label_remove, label: agent:pr-open } + - { op: label_remove, label: ready-for-agent } + - { op: label_add, label: ready-for-human } + - text: "Repairs conflicting state labels in one edit per issue, with a comment when it falls back to ready-for-human (unless dry_run). The target is any one of the four state labels, and every other one is removed." + allow: + - { op: label_add, label: needs-info } + - { op: label_add, label: ready-for-agent } + - { op: label_add, label: ready-for-human } + - { op: label_add, label: agent:pr-open } + - { op: label_remove, label: needs-info } + - { op: label_remove, label: ready-for-agent } + - { op: label_remove, label: ready-for-human } + - { op: label_remove, label: agent:pr-open } + - { op: comment, marker: null } + - text: "Removes a stale agent:triage or agent:in-progress label (unless dry_run)." + allow: + - { op: label_remove, label: agent:triage } + - { op: label_remove, label: agent:in-progress } shared: - - "One run-log entry (when write_log is true)." + - text: "One run-log entry (when write_log is true)." + allow: { op: comment, marker: null } never: - "Triage, implement, review or merge anything, or change code." - "Claim an issue, or repair the labels of one that has a live lease." diff --git a/skills/pipeline/review-github-prs/SKILL.md b/skills/pipeline/review-github-prs/SKILL.md index 529cf7b..c2c7b81 100644 --- a/skills/pipeline/review-github-prs/SKILL.md +++ b/skills/pipeline/review-github-prs/SKILL.md @@ -31,7 +31,8 @@ effects: reads: [open non-draft PRs labelled agent:ready-for-review] writes: shared: - - "One run-log entry." + - text: "One run-log entry." + allow: { op: comment, marker: null } never: - "Review or merge a PR itself, or claim one. Only review-github-pr claims." calls: [] diff --git a/skills/pipeline/shepherd-github-prs/SKILL.md b/skills/pipeline/shepherd-github-prs/SKILL.md index 9a88f50..9a8ebd5 100644 --- a/skills/pipeline/shepherd-github-prs/SKILL.md +++ b/skills/pipeline/shepherd-github-prs/SKILL.md @@ -32,7 +32,8 @@ effects: reads: [open PRs labelled agent:needs-work, their check state] writes: shared: - - "One run-log entry." + - text: "One run-log entry." + allow: { op: comment, marker: null } never: - "Shepherd a PR itself, or claim one. Only shepherd-github-pr claims." - "Wait for CI to finish." diff --git a/skills/pipeline/triage-github-issues/SKILL.md b/skills/pipeline/triage-github-issues/SKILL.md index 524d979..b61f61c 100644 --- a/skills/pipeline/triage-github-issues/SKILL.md +++ b/skills/pipeline/triage-github-issues/SKILL.md @@ -32,7 +32,8 @@ effects: reads: [open issues with their labels] writes: shared: - - "One run-log entry." + - text: "One run-log entry." + allow: { op: comment, marker: null } never: - "Triage an issue itself, or change code or PRs." - "Claim an issue. Only triage-github-issue claims." diff --git a/tests/contracts.test.mjs b/tests/contracts.test.mjs index f0845b5..cac03ea 100644 --- a/tests/contracts.test.mjs +++ b/tests/contracts.test.mjs @@ -362,3 +362,19 @@ test("entryParts and allowsOf read both forms", () => { assert.deepEqual(allowsOf(["prose", { text: "T", allow: [{ op: "push" }, { op: "merge" }] }]), [{ op: "push" }, { op: "merge" }]); assert.deepEqual(allowsOf(["only", "prose"]), []); }); + +// ---- `labels` on create_issue and create_pr ---------------------------------------------------------------------------- +const otherWith = (block) => ITEM.replace('claimed: ["A label."]', `claimed: ["A label."]\n other:\n${block}`); + +test("a create op needs `labels`: a list of names or globs, and `[]` means none", () => { + assert.deepEqual(run([item(otherWith(` - text: "New issues with no labels."\n allow: { op: create_issue, labels: [] }`))]), []); + assert.deepEqual(run([item(otherWith(` - text: "A labelled issue."\n allow: { op: create_issue, labels: [bug, "agent:*"] }`))]), []); + assert.ok(run([item(otherWith(` - text: "x"\n allow: { op: create_issue }`))]).some((e) => /`create_issue` needs `labels`/.test(e))); + assert.ok(run([item(otherWith(` - text: "x"\n allow: { op: create_issue, labels: bug }`))]).some((e) => /`create_issue` needs `labels`/.test(e))); + assert.ok(run([item(otherWith(` - text: "x"\n allow: { op: create_issue, labels: [""] }`))]).some((e) => /`create_issue` needs `labels`/.test(e))); +}); + +test("`labels` on an op that creates nothing is rejected", () => { + assert.ok(allowErrors(` - text: "x"\n allow: { op: label_add, label: a, labels: [a] }`).some((e) => /`labels` applies only to create_issue and create_pr/.test(e))); + assert.ok(allowErrors(` - text: "x"\n allow: { op: comment, marker: null, labels: [] }`).some((e) => /`labels` applies only to/.test(e))); +}); diff --git a/tests/scenarios.test.mjs b/tests/scenarios.test.mjs index 6243441..2f6b9cd 100644 --- a/tests/scenarios.test.mjs +++ b/tests/scenarios.test.mjs @@ -4,7 +4,7 @@ import { readFileSync, readdirSync, existsSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { parse } from "yaml"; -import { extractContract } from "../scripts/contracts.mjs"; +import { extractContract, allowsOf } from "../scripts/contracts.mjs"; import { coverage, contractOf, rerunGaps } from "../scripts/scenario-coverage.mjs"; import { checkScenario, allowedBy, globMatches, markerOf, expandScenario, labelCatalog, syncLabels, LABEL_CATALOG, diffWorlds, applyEffects, matchesReturn, checkState, ORACLES, ORACLE_REQUIRES } from "../scripts/scenario.mjs"; @@ -901,3 +901,67 @@ for (const [file, extra, why] of FORBIDDEN) { assert.ok(problems(contractOf(sc.skill), sc).some((p) => notAllowed.test(p)), JSON.stringify(extra)); }); } + +// ---- `labels` on a create, and the group, sweep and file skills --------------------------------------------------------- +test("allowedBy: a created item may carry only the labels the entry lists", () => { + const ctx = { world: { issues: [], prs: [] }, claimed: new Set() }; + const made = (labels) => ({ op: "create_issue", number: 9, ...(labels ? { labels } : {}) }); + assert.ok(allowedBy(made(), [{ op: "create_issue", labels: [] }], ctx), "no labels at all"); + assert.ok(allowedBy(made([]), [{ op: "create_issue", labels: [] }], ctx)); + assert.equal(allowedBy(made(["bug"]), [{ op: "create_issue", labels: [] }], ctx), null, "`[]` allows none"); + assert.ok(allowedBy(made(["bug"]), [{ op: "create_issue", labels: ["bug", "enhancement"] }], ctx)); + assert.equal(allowedBy(made(["bug", "wontfix"]), [{ op: "create_issue", labels: ["bug"] }], ctx), null, "every label must be listed"); + assert.ok(allowedBy(made(["agent:stuck"]), [{ op: "create_issue", labels: ["agent:*"] }], ctx)); + assert.ok(allowedBy({ op: "create_pr", number: 3, labels: ["agent:needs-work"] }, [{ op: "create_pr", labels: ["agent:needs-work"] }], ctx)); + assert.equal(allowedBy({ op: "create_pr", number: 3, labels: ["agent:needs-work", "x"] }, [{ op: "create_pr", labels: ["agent:needs-work"] }], ctx), null); +}); + +test("an issue filed with a label, or a PR opened with the wrong one, is refused", () => { + const filed = find("file-github-issues/limit-defers-the-rest.yaml"); + filed.expect.effects[0].labels = ["ready-for-agent"]; + const found = problems(contractOf("file-github-issues"), filed).filter((p) => notAllowed.test(p)); + assert.equal(found.length, 1); + assert.match(found[0], /create_issue with only `ready-for-agent`/); + assert.match(found[0], /It allows:.*create_issue with no labels/); + const pr = find("implement-github-issue/an-interrupted-pass-does-not-open-a-second-pr.yaml"); + const made = pr.expect.effects.find((e) => e.op === "create_pr"); + if (made) { made.labels = ["agent:ready-for-review"]; assert.ok(problems(contractOf("implement-github-issue"), pr).some((p) => notAllowed.test(p))); } +}); + +const FORBIDDEN_3 = [ + ["file-github-issues/a-duplicate-gets-a-comment.yaml", { op: "label_add", target: "issue#7", label: "wontfix" }, "filing never labels an existing issue"], + ["file-github-issues/a-duplicate-gets-a-comment.yaml", { op: "comment", target: "issue#7", marker: "agent:handoff" }, "evidence comments carry no marker"], + ["file-github-issues/a-duplicate-gets-a-comment.yaml", { op: "close", target: "issue#7" }, "filing closes nothing"], + ["reconcile-github-issues/merged-pr-closes-the-issue.yaml", { op: "label_add", target: "issue#12", label: "wontfix" }, "reconcile never applies wontfix"], + ["reconcile-github-issues/merged-pr-closes-the-issue.yaml", { op: "label_remove", target: "issue#12", label: "needs-triage" }, "reconcile leaves needs-triage to triage"], + ["reconcile-github-issues/stale-lease-is-cleared.yaml", { op: "comment", target: "issue#13", marker: "agent:lease" }, "reconcile writes no lease"], + ["reconcile-github-issues/stale-lease-is-cleared.yaml", { op: "label_add", target: "issue#13", label: "agent:triage" }, "reconcile never claims an issue"], +]; +for (const [file, extra, why] of FORBIDDEN_3) { + test(`${file.split("/")[0]}: ${why}`, () => { + const sc = find(file); + assert.deepEqual(problems(contractOf(sc.skill), sc), [], "the golden scenario is clean first"); + sc.expect.effects.push(extra); + assert.ok(problems(contractOf(sc.skill), sc).some((p) => notAllowed.test(p)), JSON.stringify(extra)); + }); +} + +test("setup-github-agent-workflow: it writes labels and one file, and pushes nothing", () => { + const sc = find("setup-github-agent-workflow/an-empty-repository-gets-every-label.yaml"); + assert.deepEqual(problems(contractOf(sc.skill), sc), [], "the golden scenario is clean first"); + sc.expect.effects.push({ op: "push", target: "pr#1" }); + assert.ok(problems(contractOf(sc.skill), sc).some((p) => notAllowed.test(p))); +}); + +test("a group skill's one write is an unmarked run-log comment, and nothing else", () => { + const ctx = { world: { issues: [{ number: 40, state: "open", labels: ["agent:log"], comments: [] }], prs: [] }, claimed: new Set() }; + for (const skill of ["triage-github-issues", "implement-github-issues", "review-github-prs", "shepherd-github-prs"]) { + const writes = contractOf(skill).effects.writes; + assert.deepEqual(Object.keys(writes), ["shared"], skill); + const allows = allowsOf(writes.shared); + assert.ok(allowedBy({ op: "comment", target: "issue#40" }, allows, ctx), `${skill}: the log entry`); + assert.equal(allowedBy({ op: "comment", target: "issue#40", marker: "agent:handoff" }, allows, ctx), null, `${skill}: a marker`); + assert.equal(allowedBy({ op: "label_add", target: "issue#40", label: "agent:log" }, allows, ctx), null, `${skill}: a label`); + assert.equal(allowedBy({ op: "close", target: "issue#40" }, allows, ctx), null, `${skill}: a close`); + } +});