diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23ab6003e..9fa0df28a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,6 +110,8 @@ jobs: run: zig run scripts/lint.zig - name: scripts/zb (build lock) run: sh scripts/test-zb.sh + - name: scripts/check-kickoff + run: python3 scripts/test-check-kickoff.py test: name: Tests (${{ matrix.name }}) diff --git a/docs/AGENT_WORK.md b/docs/AGENT_WORK.md index 0be937bd9..d12e49b57 100644 --- a/docs/AGENT_WORK.md +++ b/docs/AGENT_WORK.md @@ -12,7 +12,7 @@ take it; nothing here depends on which model. branch (`/`). 3. The `running:*` label is the lock: the first claim wins. `agent-work.yml` removes a second `running:*` label from the issue, with a comment, and the worker that lost picks another. -4. The worker opens a draft PR on its first push saying `Part of #N`, and works as +4. The worker opens a draft PR on its first push saying `Part of fizzyedit/work#N`, and works as `CONTRIBUTING.md` says. 5. A claim with no live PR after 24 hours is released: the daily sweep removes `running:*`, comments, and the issue is available again (`agent` stays). A live PR is an open PR in this @@ -25,7 +25,7 @@ removes it, on issues and on pull requests, unless its latest application was by maintain/admin permission (a failed permission lookup counts as no permission). The daily sweep does the same for any `merge-ok` the label-time run missed. Nothing auto-merges. -## Kickoff template (the issue body, 1,200 characters at most) +## Kickoff template (the issue body, 1,600 characters at most) ``` Task: @@ -34,11 +34,18 @@ Builds on: Branch: / Files/area: Gates: scripts/check-fmt.sh; scripts/zb build, scripts/zb build test, scripts/zb build test-integration, scripts/zb build check-web, scripts/zb build test-sdk-version (read "N/N steps succeeded") -Rules: follow AGENTS.md, CONTRIBUTING.md, docs/STYLE.md and the AGENTS.md of each directory touched. One PR, <=800 lines outside tests/docs. If the SDK fingerprint moves, record it and leave sdk_version alone. +Rules: follow AGENTS.md, CONTRIBUTING.md, docs/STYLE.md and the AGENTS.md of each directory touched. One PR, <=800 lines outside tests/docs. PRs say "Part of fizzyedit/work#N". If the SDK fingerprint moves, record it and leave sdk_version alone. Done: draft PR on first push; gates pass; PR body says what was run and how verified (test, tape, measurement). Notes: ``` +Before assigning a worker, run `scripts/check-kickoff ` (issue numbers +are read from `fizzyedit/work` through `gh`). It reports every missing or empty template +field, bodies over 1,600 characters, invalid branch names, `scripts/zb zig build` in Gates, +and Rules without a `fizzyedit/work#N` reference. It accepts the template above and GitHub +issue-form `### Field` headings. The template is the source of the required field list. +Run `python3 scripts/test-check-kickoff.py` for the validator tests. + ## Trust boundary - Labels are applied only by maintainers and triagers (triage access or higher). Anyone else cannot make diff --git a/scripts/check-kickoff b/scripts/check-kickoff new file mode 100755 index 000000000..d2a5fdad6 --- /dev/null +++ b/scripts/check-kickoff @@ -0,0 +1,79 @@ +#!/usr/bin/env python3 +"""Validate kickoff files or fizzyedit/work issues against the documented template.""" + +import re +import subprocess +import sys +from pathlib import Path + +TEMPLATE = Path(__file__).resolve().parents[1] / "docs/AGENT_WORK.md" +MAX_CHARACTERS = 1600 + + +def template_fields(): + text = TEMPLATE.read_text(encoding="utf-8") + section = text.split("## Kickoff template", 1)[1].split("```", 2)[1] + fields = re.findall(r"^([^:\n]+):", section, re.MULTILINE) + if not fields or len(fields) != len(set(fields)): + raise ValueError("kickoff template has no fields or duplicate fields") + return fields + + +def validate(body, fields): + values = {} + current = None + for line in body.splitlines(): + # GitHub issue forms emit headings; pasted kickoffs use Label: value. + heading = re.fullmatch(r"### (.+?)\s*", line) + label, separator, value = line.partition(":") + name = heading[1] if heading else label + if name in fields and (heading or separator): + current = name + values.setdefault(name, []).append("" if heading else value) + elif current: + values[current].append(line) + values = {name: "\n".join(lines).strip() for name, lines in values.items()} + failures = [] + for name in fields: + if not values.get(name) or values[name] == "_No response_": + failures.append(f"{name}: missing field or empty value") + if len(body) > MAX_CHARACTERS: + failures.append(f"Body: {len(body)} characters exceeds {MAX_CHARACTERS}") + if values.get("Branch") and not re.fullmatch(r"[a-z0-9-]+/[a-z0-9._-]+", values["Branch"]): + failures.append("Branch: must match ^[a-z0-9-]+/[a-z0-9._-]+$") + if re.search(r"scripts/zb\s+zig\s+build", values.get("Gates", "")): + failures.append("Gates: use scripts/zb build, not scripts/zb zig build") + if not re.search(r"fizzyedit/work#(?:[1-9][0-9]*|N)\b", values.get("Rules", "")): + failures.append("Rules: must require a PR reference to fizzyedit/work#N") + return failures + + +def main(): + if len(sys.argv) != 2: + print("usage: scripts/check-kickoff ", file=sys.stderr) + return 2 + source = sys.argv[1] + try: + fields = template_fields() + if re.fullmatch(r"[1-9][0-9]*", source): + body = subprocess.run( + ["gh", "issue", "view", source, "--repo", "fizzyedit/work", "--json", "body", "--jq", ".body"], + check=True, capture_output=True, text=True, + ).stdout.removesuffix("\n") + else: + body = Path(source).read_text(encoding="utf-8") + failures = validate(body, fields) + except (OSError, ValueError, IndexError, subprocess.CalledProcessError) as error: + print(f"check-kickoff: {source}: {error}", file=sys.stderr) + if isinstance(error, subprocess.CalledProcessError) and error.stderr: + print(error.stderr.strip(), file=sys.stderr) + return 2 + for failure in failures: + print(f"{source}: {failure}", file=sys.stderr) + if not failures: + print(f"{source}: kickoff passes") + return bool(failures) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test-check-kickoff.py b/scripts/test-check-kickoff.py new file mode 100644 index 000000000..0c5db5819 --- /dev/null +++ b/scripts/test-check-kickoff.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +"""Exercise kickoff validation and its file and issue-number interfaces.""" + +import importlib.machinery +import importlib.util +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + +SCRIPT = Path(__file__).with_name("check-kickoff") +loader = importlib.machinery.SourceFileLoader("kickoff", str(SCRIPT)) +spec = importlib.util.spec_from_loader(loader.name, loader) +kickoff = importlib.util.module_from_spec(spec) +loader.exec_module(kickoff) + + +class KickoffTests(unittest.TestCase): + def setUp(self): + self.fields = kickoff.template_fields() + self.values = {name: "example" for name in self.fields} + self.values.update(Branch="ci/check-kickoff", Gates="scripts/zb build", + Rules="PRs say Part of fizzyedit/work#42") + + def body(self): + return "\n".join(f"{name}: {value}" for name, value in self.values.items()) + + def test_valid_and_form(self): + self.assertEqual(kickoff.validate(self.body(), self.fields), []) + form = "\n\n".join(f"### {name}\n\n{value}" for name, value in self.values.items()) + self.assertEqual(kickoff.validate(form, self.fields), []) + + def test_each_missing_or_empty_field(self): + for name in self.fields: + for value in (None, "", "_No response_"): + with self.subTest(name=name, value=value): + saved = self.values.pop(name) + if value is not None: + self.values[name] = value + failures = kickoff.validate(self.body(), self.fields) + self.assertTrue(any(f.startswith(name + ":") for f in failures)) + self.values[name] = saved + + def test_all_failures_reported(self): + self.values.update(Branch="Bad branch", Gates="scripts/zb zig build", Rules="no reference") + failures = kickoff.validate(self.body() + "x" * 1600, self.fields) + self.assertEqual({f.split(":")[0] for f in failures}, {"Branch", "Gates", "Rules", "Body"}) + + def test_length_boundary_and_unicode(self): + body = self.body() + "\né" * 10 + body += "x" * (1600 - len(body)) + self.assertEqual(kickoff.validate(body, self.fields), []) + self.assertTrue(any(f.startswith("Body:") for f in kickoff.validate(body + "x", self.fields))) + + def test_branch_and_multiline_rules(self): + for branch in ("ci/a._-9", "ci/A", "ci/a/b", "ci/", "ci/a extra"): + self.values["Branch"] = branch + failures = kickoff.validate(self.body(), self.fields) + self.assertEqual(bool(failures), branch != "ci/a._-9") + self.values["Branch"] = "ci/check" + self.values["Rules"] = '\n- PRs say "Part of fizzyedit/work#N".' + self.assertEqual(kickoff.validate(self.body(), self.fields), []) + + def test_template_fields_are_live(self): + with tempfile.TemporaryDirectory() as directory: + template = Path(directory) / "AGENT_WORK.md" + template.write_text("## Kickoff template\n```\nExtra: value\n```\n") + original = kickoff.TEMPLATE + try: + kickoff.TEMPLATE = template + self.assertEqual(kickoff.template_fields(), ["Extra"]) + self.assertIn("Extra: missing field or empty value", + kickoff.validate("", kickoff.template_fields())) + finally: + kickoff.TEMPLATE = original + + def test_cli_file_and_issue(self): + with tempfile.TemporaryDirectory() as directory: + body = Path(directory) / "body" + body.write_text(self.body()) + result = subprocess.run([str(SCRIPT), str(body)], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr) + gh = Path(directory) / "gh" + gh.write_text('#!/bin/sh\n[ "$*" = "issue view 7 --repo fizzyedit/work --json body --jq .body" ] || exit 9\ncat "$KICKOFF_BODY"\n') + gh.chmod(0o755) + env = dict(os.environ, PATH=directory + os.pathsep + os.environ["PATH"], KICKOFF_BODY=str(body)) + result = subprocess.run([str(SCRIPT), "7"], env=env, capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr) + body.write_text("Task: incomplete") + result = subprocess.run([str(SCRIPT), str(body)], capture_output=True, text=True) + self.assertEqual(result.returncode, 1) + self.assertIn("Branch:", result.stderr) + gh.write_text('#!/bin/sh\nprintf "lookup failed\\n" >&2\nexit 1\n') + result = subprocess.run([str(SCRIPT), "7"], env=env, capture_output=True, text=True) + self.assertEqual(result.returncode, 2) + self.assertIn("lookup failed", result.stderr) + result = subprocess.run([str(SCRIPT), str(body) + ".missing"], capture_output=True, text=True) + self.assertEqual(result.returncode, 2) + + +if __name__ == "__main__": + unittest.main()