From 6b61c724fc1464903677189858e099796f029354 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Tue, 4 Aug 2026 20:45:59 +0100 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20wire=20the=20Spawn=20Drift=20self-h?= =?UTF-8?q?eal=20=E2=80=94=20regenerate=20and=20propose=20a=20sync=20PR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spawn Drift detected drift but nothing regenerated. The templates are declared a generated view, yet the only thing that ever regenerated them was a human typing `/spawn --apply` — so the scheduled leg went red, stayed red, and was cleared by hand. Every green run in its history was a manual dispatch fired 16-19 seconds after such a sync, which is why it could not fail. On schedule/dispatch the workflow now regenerates and opens (or refreshes) a sync PR on each drifted template repo. Deliberately a PR, not a bot push. These repos are force-synced generated views, so an automated push would be a force-push to a published `main`; #118 — a leak that sat public for eight days — is the argument for a human seeing what gets published. That keeps the sanctioned force-push a human act while removing the "nothing regenerates" gap. ## The safety interlock `--check` collapsed every failure into exit 1. Split into: 0 CLEAN published matches the regenerated tree 1 DRIFT content differs — mechanical, safe to PROPOSE 2 UNSAFE UNMATCHED file class or canary hit — a HUMAN DECISION Only exit 1 reaches the PR path. Exit 2 fails the job and opens nothing: a canary hit means the regenerated tree carries live instance content, so a sync PR would be proposing to publish a leak — #118 with a robot doing it. A test asserts UNSAFE outranks DRIFT when both are present. ## A latent bug this surfaced Running the workflow's shell rather than only reading it found that `stamp_complete_index()` (from #120) breaks on a RELATIVE `--write DIR`: the child resolves the script path after chdir'ing to `cwd`, so `--write regenerated` — exactly what a CI step naturally passes — died with "can't open file". Every invocation to date happened to use an absolute path, so it stayed latent. Fixed by resolving, with a regression test that fails without it. ## Verified by executing it, not just reading it Both `run:` blocks were extracted and run under `bash -e` (as Actions runs them) against a CI-shaped fixture, with `git push`/`gh` stubbed: * clean -> no PR * drift -> PR opened; the already-current repo correctly skipped * drift -> existing PR REFRESHED, no duplicate (stable force-updated branch) * canary -> job fails, code=2, PR step's `if` is false, nothing proposed ## Known external dependency Opening a PR on the template repos needs write access there; GITHUB_TOKEN is scoped to PyAutoMind. Uses `secrets.PAT_PYAUTOLABS`, the org's established cross-repo token. Whether that PAT grants write to the two template repos CANNOT be verified from a local session — only a real run reveals it. The step therefore fails with an explicit, actionable message rather than silently doing nothing. Refs #125 Co-Authored-By: Claude Opus 5 --- .github/workflows/spawn_drift.yml | 89 +++++++++++++++++++++++++++ scripts/spawn.py | 35 +++++++++-- tests/test_spawn_template_contract.py | 87 ++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 5 deletions(-) diff --git a/.github/workflows/spawn_drift.yml b/.github/workflows/spawn_drift.yml index 70d6ff36..0ac04612 100644 --- a/.github/workflows/spawn_drift.yml +++ b/.github/workflows/spawn_drift.yml @@ -58,7 +58,96 @@ jobs: git clone --depth 1 "https://x-access-token:${{ secrets.GITHUB_TOKEN }}@github.com/PyAutoLabs/$r" "$r" done - name: Regenerate + diff + id: diff run: | mkdir published mv PyAutoMind-template PyAutoMemory-template published/ + set +e python3 PyAutoMind/scripts/spawn.py --root "$PWD" --check published + code=$? + set -e + echo "code=${code}" >> "$GITHUB_OUTPUT" + case "$code" in + 0) echo "templates are current" ;; + 1) echo "::notice::templates have drifted — proposing a sync PR" ;; + 2) echo "::error::spawn produced an UNSAFE tree (UNMATCHED file class or canary hit). This is a human decision: extend the spec's tables or fix the partition rules. NOT auto-healed — a canary hit means the regenerated tree carries live instance content, so a sync PR would propose publishing a leak." + exit 1 ;; + *) echo "::error::spawn --check exited ${code}, which this workflow does not understand" + exit 1 ;; + esac + + # Regenerate into a clean tree and open (or refresh) one sync PR per + # drifted template repo. Deliberately a PR, not a bot push: these repos + # are force-synced generated views, so an automated push would be a + # force-push to a published `main`. #118 — a leak that sat public for + # eight days — is the argument for a human seeing what gets published. + - name: Propose the sync PR + if: steps.diff.outputs.code == '1' + env: + # GITHUB_TOKEN is scoped to PyAutoMind; writing to the template repos + # needs the org-wide PAT (same one nightly-release.yml uses). + GH_TOKEN: ${{ secrets.PAT_PYAUTOLABS }} + BRANCH: spawn/auto-sync + run: | + if [ -z "${GH_TOKEN:-}" ]; then + echo "::error::PAT_PYAUTOLABS is not set on this repo. The self-heal cannot open a PR on the template repos without a token that can write to them; GITHUB_TOKEN is scoped to PyAutoMind only." + exit 1 + fi + python3 PyAutoMind/scripts/spawn.py --root "$PWD" --write regenerated + + MIND_SHA=$(git -C PyAutoMind rev-parse --short HEAD) + MEMORY_SHA=$(git -C PyAutoMemory rev-parse --short HEAD) + TOP="$PWD" + opened=0 + for name in PyAutoMind-template PyAutoMemory-template; do + if diff -rq "regenerated/$name" "published/$name" \ + --exclude .git --exclude SPAWNED_FROM >/dev/null 2>&1; then + echo "== $name: current, no PR needed" + continue + fi + echo "== $name: drifted, preparing $BRANCH" + work="work/$name" + git clone -q "https://x-access-token:${GH_TOKEN}@github.com/PyAutoLabs/$name" "$work" || { + echo "::error::cannot clone $name with PAT_PYAUTOLABS — check the token grants write to it"; exit 1; } + + # Replace content wholesale; the template IS the generated tree. + ( cd "$work" && find . -mindepth 1 -maxdepth 1 ! -name .git -exec rm -rf {} + ) + cp -a "regenerated/$name/." "$work/" + + cd "$TOP/$work" + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git checkout -q -B "$BRANCH" + git add -A + if git diff --cached --quiet; then + echo " nothing to commit after all"; cd "$TOP"; continue + fi + git commit -q \ + -m "spawn: regenerate from mind@${MIND_SHA} memory@${MEMORY_SHA}" \ + -m "Proposed automatically by the Spawn Drift self-heal (PyAutoMind#125). Review the diff before merging: merging force-syncs this generated view." + # A stable branch, force-updated: a weekly rerun refreshes the open + # PR instead of opening a new one every Monday. + git push -q -f origin "$BRANCH" + + if gh pr view "$BRANCH" --repo "PyAutoLabs/$name" >/dev/null 2>&1; then + echo " refreshed the existing PR" + else + body="$TOP/pr-body.md" + : > "$body" + printf '%s\n' "Opened automatically by the \`Spawn Drift\` self-heal (PyAutoMind#125)." >> "$body" + printf '%s\n' "" >> "$body" + printf '%s\n' "This repo is a **generated view** of the live Mind/Memory. It had drifted from what \`spawn.py\` produces, so this branch carries the regenerated tree." >> "$body" + printf '%s\n' "" >> "$body" + printf '%s\n' "\`spawn --check\` reported **content drift only** (exit 1). Had it reported an UNMATCHED file class or a canary hit (exit 2) no PR would exist - that is a human decision, because a canary hit means the regenerated tree carries live instance content." >> "$body" + printf '%s\n' "" >> "$body" + printf '%s\n' "Review the diff before merging: merging force-syncs this view." >> "$body" + gh pr create --repo "PyAutoLabs/$name" --base main --head "$BRANCH" \ + --title "spawn: sync this generated view with PyAutoMind" \ + --body-file "$body" \ + || { echo "::error::could not open the PR on $name - check PAT_PYAUTOLABS grants write there"; exit 1; } + echo " opened a new PR" + fi + cd "$TOP" + opened=$((opened + 1)) + done + echo "::notice::sync PRs opened/refreshed: ${opened}" diff --git a/scripts/spawn.py b/scripts/spawn.py index d3e22291..3bdb5a5a 100644 --- a/scripts/spawn.py +++ b/scripts/spawn.py @@ -35,6 +35,19 @@ OWNER_PLACEHOLDER = "YOURORG" +# --check exit codes. The split exists so the Spawn Drift self-heal can tell +# "the templates are stale" from "the generator produced something unsafe": +# +# CLEAN 0 published templates match the regenerated tree +# DRIFT 1 content differs — mechanical, safe to PROPOSE as a PR +# UNSAFE 2 UNMATCHED file class or canary hit — a HUMAN DECISION +# +# EXIT_UNSAFE must never be auto-healed. A canary hit means the generated tree +# contains live instance content, so opening a sync PR would be proposing to +# publish a leak — exactly the #118 failure, automated. Both remain non-zero, +# so anything treating the check as a boolean is unaffected. +EXIT_CLEAN, EXIT_DRIFT, EXIT_UNSAFE = 0, 1, 2 + MIND_WORK_TYPES = ( "feature", "bug", "refactor", "docs", "test", "release", "maintenance", "research", "experiment", "triage", @@ -639,6 +652,12 @@ def stamp_complete_index(out_dir): Deliberately NOT a constant here: `lifecycle.py` owns the index format, and a second copy of that text would drift from it. """ + # Absolute, deliberately: the script path is resolved by the child AFTER it + # chdir's to `cwd`, so a relative --write DIR (e.g. `--write regenerated`, + # which is what a CI step naturally passes) made the path unresolvable from + # inside the new cwd and the child died with "can't open file". Every + # invocation to date happened to use an absolute path, so it stayed latent. + out_dir = out_dir.resolve() lifecycle = out_dir / "scripts" / "lifecycle.py" if not lifecycle.exists(): # rules changed; nothing to stamp return @@ -847,9 +866,13 @@ def main(): out_root = Path(args.write) if args.write else Path(tmp) out_root.mkdir(parents=True, exist_ok=True) results = generate_all(root, out_root) - failed = report(results) + # UNSAFE: the generated tree itself cannot be trusted — an unclassified + # file class (UNMATCHED) or leaked instance content (canary). Kept + # strictly apart from DRIFT below; see EXIT_* for why that matters. + unsafe = report(results) + drifted = False - if args.check and not failed: + if args.check and not unsafe: for name in results: problems = diff_trees(out_root / name, Path(args.check) / name) # SPAWNED_FROM records the source commit, which legitimately @@ -858,13 +881,15 @@ def main(): status = "OK" if not problems else f"{len(problems)} drift(s)" print(f"check {name}: {status}") for p in problems: - failed = True + drifted = True print(f" ✗ {p}") - if args.write and not failed: + if args.write and not unsafe: print(f"written: {out_root}") - sys.exit(1 if failed else 0) + if unsafe: + sys.exit(EXIT_UNSAFE) + sys.exit(EXIT_DRIFT if drifted else EXIT_CLEAN) if __name__ == "__main__": diff --git a/tests/test_spawn_template_contract.py b/tests/test_spawn_template_contract.py index 230cf91d..8054981d 100644 --- a/tests/test_spawn_template_contract.py +++ b/tests/test_spawn_template_contract.py @@ -24,6 +24,7 @@ import importlib.util import re import subprocess +import sys from pathlib import Path import pytest @@ -378,6 +379,92 @@ def test_live_complete_index_is_never_copied(tmp_path): assert not (out / "complete" / "2026").exists() +def _workspace_with_published(tmp_path, mind_files=None, published_edit=None): + """A CI-shaped layout: live repos plus a `published/` copy of the templates.""" + mind = tmp_path / "PyAutoMind" + _fake_repo(mind, {**MINIMAL_MIND, **GITHUB_FILES, **(mind_files or {})}) + _fake_repo(tmp_path / "PyAutoMemory", MINIMAL_MEMORY) + published = tmp_path / "published" + published.mkdir() + subprocess.run( + [sys.executable, str(SPAWN_PY), "--root", str(tmp_path), "--write", str(published)], + check=False, capture_output=True, + ) + if published_edit: + published_edit(published) + return published + + +def _check_exit(tmp_path, published): + return subprocess.run( + [sys.executable, str(SPAWN_PY), "--root", str(tmp_path), "--check", str(published)], + capture_output=True, text=True, + ).returncode + + +def test_check_exit_codes_are_the_self_heal_contract(tmp_path): + """`Spawn Drift`'s self-heal branches on these; collapsing them is unsafe. + + 0 clean · 1 content drift (safe to propose) · 2 unsafe tree (human decision). + A canary hit must never reach the PR path — that would automate publishing + a leak, which is #118 with a robot doing it. + """ + published = _workspace_with_published(tmp_path) + assert _check_exit(tmp_path, published) == spawn.EXIT_CLEAN + + # Content drift only. + (published / "PyAutoMind-template" / "README.md").write_text("drifted\n") + assert _check_exit(tmp_path, published) == spawn.EXIT_DRIFT + + +def test_canary_hit_reports_unsafe_not_drift(tmp_path): + """The interlock: a leak must be distinguishable from mechanical drift.""" + token = spawn.CANARY_TOKENS[0] + published = _workspace_with_published( + tmp_path, mind_files={"REFERENCE.md": f"# R\nmentions {token}0946\n"} + ) + assert _check_exit(tmp_path, published) == spawn.EXIT_UNSAFE + + +def test_unmatched_file_class_reports_unsafe_not_drift(tmp_path): + published = _workspace_with_published( + tmp_path, mind_files={"brand_new_thing.md": "unclassified\n"} + ) + assert _check_exit(tmp_path, published) == spawn.EXIT_UNSAFE + + +def test_unsafe_outranks_drift(tmp_path): + """With BOTH problems present, the answer must be UNSAFE. + + If drift won, the self-heal would open a PR from a tree that also carries + leaked content. + """ + token = spawn.CANARY_TOKENS[0] + published = _workspace_with_published( + tmp_path, + mind_files={"REFERENCE.md": f"# R\nmentions {token}0946\n"}, + published_edit=lambda p: (p / "PyAutoMind-template" / "README.md").write_text("drifted\n"), + ) + assert _check_exit(tmp_path, published) == spawn.EXIT_UNSAFE + + +def test_stamping_works_with_a_RELATIVE_output_dir(tmp_path, monkeypatch): + """`--write regenerated` must work, not just `--write /abs/path`. + + The child resolves the script path AFTER chdir'ing to `cwd`, so a relative + out_dir made it unresolvable and the child died with "can't open file". + Every invocation to date happened to pass an absolute path, so it stayed + latent until a CI step naturally wrote to a relative dir. + """ + mind = tmp_path / "PyAutoMind" + _fake_repo(mind, MINIMAL_MIND) + monkeypatch.chdir(tmp_path) + + spawn.generate_mind(mind, Path("out_relative")) # relative, on purpose + + assert (tmp_path / "out_relative" / "complete" / "index.md").exists() + + def test_stamping_is_skipped_when_lifecycle_is_not_kept(tmp_path): """If the rules ever stop KEEPing lifecycle.py, spawn must not crash.""" mind = tmp_path / "PyAutoMind" From f024858156145a825a64ad686384368c59fc43a3 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Tue, 4 Aug 2026 20:47:32 +0100 Subject: [PATCH 2/3] fix: a spawn crash must not be reported as drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python exits 1 on an unhandled exception — the same code as EXIT_DRIFT. The self-heal would therefore read a crash as "the templates are stale" and try to propose a sync PR from whatever partial tree the crash left behind. Not hypothetical: stamp_complete_index() crashing on a relative --write path (fixed in the previous commit) produced exactly this exit-1-that-means-crash, and that is how it was noticed. Unhandled exceptions now exit 3, which the workflow's catch-all rejects. The traceback is still printed, so nothing is hidden. Refs #125 Co-Authored-By: Claude Opus 5 --- scripts/spawn.py | 26 ++++++++++++++++++++++---- tests/test_spawn_template_contract.py | 24 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/scripts/spawn.py b/scripts/spawn.py index 3bdb5a5a..3bb9dc91 100644 --- a/scripts/spawn.py +++ b/scripts/spawn.py @@ -26,6 +26,7 @@ import subprocess import sys import tempfile +import traceback from pathlib import Path # -------------------------------------------------------------------------- @@ -41,12 +42,18 @@ # CLEAN 0 published templates match the regenerated tree # DRIFT 1 content differs — mechanical, safe to PROPOSE as a PR # UNSAFE 2 UNMATCHED file class or canary hit — a HUMAN DECISION +# CRASH 3 unhandled exception — NOT drift (see the __main__ guard) # # EXIT_UNSAFE must never be auto-healed. A canary hit means the generated tree # contains live instance content, so opening a sync PR would be proposing to -# publish a leak — exactly the #118 failure, automated. Both remain non-zero, -# so anything treating the check as a boolean is unaffected. -EXIT_CLEAN, EXIT_DRIFT, EXIT_UNSAFE = 0, 1, 2 +# publish a leak — exactly the #118 failure, automated. +# +# EXIT_CRASH exists because Python exits 1 on an unhandled exception, which is +# indistinguishable from EXIT_DRIFT — a crash would otherwise read as "the +# templates are stale" and the self-heal would propose a PR from whatever +# partial tree the crash left. All of these stay non-zero, so anything treating +# the check as a boolean is unaffected. +EXIT_CLEAN, EXIT_DRIFT, EXIT_UNSAFE, EXIT_CRASH = 0, 1, 2, 3 MIND_WORK_TYPES = ( "feature", "bug", "refactor", "docs", "test", "release", @@ -893,4 +900,15 @@ def main(): if __name__ == "__main__": - main() + try: + main() + except SystemExit: + raise + except BaseException: + # An unhandled exception would otherwise exit 1 — INDISTINGUISHABLE + # from EXIT_DRIFT, so the Spawn Drift self-heal would read a crash as + # "the templates are stale" and try to propose a sync PR from whatever + # partial tree the crash left behind. Exit on a code no caller treats + # as actionable instead; the workflow's catch-all rejects it. + traceback.print_exc() + sys.exit(EXIT_CRASH) diff --git a/tests/test_spawn_template_contract.py b/tests/test_spawn_template_contract.py index 8054981d..0acd4a70 100644 --- a/tests/test_spawn_template_contract.py +++ b/tests/test_spawn_template_contract.py @@ -417,6 +417,30 @@ def test_check_exit_codes_are_the_self_heal_contract(tmp_path): assert _check_exit(tmp_path, published) == spawn.EXIT_DRIFT +def test_a_crash_is_not_reported_as_drift(tmp_path): + """Python exits 1 on an unhandled exception — the same code as EXIT_DRIFT. + + Left alone, the self-heal would read a crash as "the templates are stale" + and try to open a sync PR from whatever partial tree the crash left behind. + A real crash of exactly this kind (`stamp_complete_index` on a relative + path) is what prompted the guard. + """ + broken = tmp_path / "spawn_broken.py" + broken.write_text( + SPAWN_PY.read_text().replace( + " results = generate_all(root, out_root)", + " raise RuntimeError('simulated crash')", + ) + ) + r = subprocess.run( + [sys.executable, str(broken), "--check", str(tmp_path)], + capture_output=True, text=True, + ) + assert r.returncode == spawn.EXIT_CRASH, "a crash must not look like drift" + assert r.returncode != spawn.EXIT_DRIFT + assert "simulated crash" in r.stderr, "the traceback must still be visible" + + def test_canary_hit_reports_unsafe_not_drift(tmp_path): """The interlock: a leak must be distinguishable from mechanical drift.""" token = spawn.CANARY_TOKENS[0] From 7c367874c7bf19614c23ee8506fee3bc589d3a6e Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Tue, 4 Aug 2026 21:03:55 +0100 Subject: [PATCH 3/3] fix: address independent review of the self-heal (6 findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. HIGH — the change broke rule 9, which it had just established one PR ago. Adding the PAT-dependent proposal step to spawn_drift.yml meant the template would ship `secrets.PAT_PYAUTOLABS`, violating the no-configured-secret condition. Confirmed against a post-merge generation. Root cause in the TESTS, not just the rules: the .github fixtures were hand-written miniatures, so `test_no_shipped_workflow_needs_a_configured_secret` was checking a toy workflow that had no PAT while the real one had grown one. The fixtures now READ THE REAL WORKFLOW FILES. Control-tested: shipping spawn_drift.yml again now fails four tests, including the secret check. Rule 9b revised KEEP-with-schedule-stripped -> DROP. The self-heal makes the workflow depend on a PAT and on published *-template repos; a fresh org has neither, so every path in it is unrunnable there. "When in doubt DROP" — the generator and its guards still travel via scripts/ and tests/. The schedule-stripping transform is retired as dead code with its tests. 2. HIGH — `gh pr view` matches merged and closed PRs, so once a sync PR was merged the reused branch would report "refreshed" forever and silently never open another. Now `gh pr list --state open --head`. Both paths dry-run tested: open -> refresh, merged -> opens a new PR. 3. HIGH — every exit-code test compared a subprocess result with constants from the same module, so swapping EXIT_DRIFT and EXIT_UNSAFE would have left them green while the workflow still auto-proposed literal exit 1. Two tests now read the real workflow and pin the CONSUMER against the producer. Control-tested by swapping the constants. 4. MEDIUM — the interlock was incomplete. Fail-closed paths raise SystemExit("message"), which Python turns into exit 1: indistinguishable from EXIT_DRIFT. Those are human decisions like UNMATCHED, so they now map to EXIT_UNSAFE. The earlier crash guard only covered exceptions. 5. MEDIUM — concurrent runs were last-writer-wins on the shared proposal branch. Added a `concurrency` group that queues rather than cancels; a cancelled run could leave a pushed branch with no PR. 6. LOW — `diff` exit 2 means trouble, not differences, but every nonzero status entered the drift path, so an unreadable tree would trigger replacement and force-push. Only exit 1 is drift now; 2 aborts. Refs #125 Co-Authored-By: Claude Opus 5 --- .github/workflows/spawn_drift.yml | 29 +++- docs/pyautobrain/spawn_spec.md | 2 +- scripts/spawn.py | 119 +++------------ tests/test_spawn_template_contract.py | 200 ++++++++++---------------- 4 files changed, 116 insertions(+), 234 deletions(-) diff --git a/.github/workflows/spawn_drift.yml b/.github/workflows/spawn_drift.yml index 0ac04612..a98c9cc9 100644 --- a/.github/workflows/spawn_drift.yml +++ b/.github/workflows/spawn_drift.yml @@ -18,6 +18,13 @@ on: - ".github/workflows/spawn_drift.yml" workflow_dispatch: +# One sync at a time: the proposal branch is shared, so concurrent runs would +# be last-writer-wins on a force-push. Queue rather than cancel — a cancelled +# run could leave a pushed branch with no PR. +concurrency: + group: spawn-drift + cancel-in-progress: false + permissions: contents: read @@ -100,10 +107,19 @@ jobs: TOP="$PWD" opened=0 for name in PyAutoMind-template PyAutoMemory-template; do - if diff -rq "regenerated/$name" "published/$name" \ - --exclude .git --exclude SPAWNED_FROM >/dev/null 2>&1; then + set +e + diff -rq "regenerated/$name" "published/$name" \ + --exclude .git --exclude SPAWNED_FROM >/dev/null + d=$? + set -e + # diff: 0 same, 1 differs, 2 TROUBLE (missing/unreadable tree). + # Only 1 may mean drift; 2 must abort rather than force-push. + if [ "$d" = "0" ]; then echo "== $name: current, no PR needed" continue + elif [ "$d" != "1" ]; then + echo "::error::diff failed ($d) comparing $name — aborting rather than replacing a tree we could not read" + exit 1 fi echo "== $name: drifted, preparing $BRANCH" work="work/$name" @@ -129,8 +145,13 @@ jobs: # PR instead of opening a new one every Monday. git push -q -f origin "$BRANCH" - if gh pr view "$BRANCH" --repo "PyAutoLabs/$name" >/dev/null 2>&1; then - echo " refreshed the existing PR" + open_pr=$(gh pr list --repo "PyAutoLabs/$name" --head "$BRANCH" \ + --state open --json number --jq 'length' 2>/dev/null || echo 0) + # NOT `gh pr view`: it matches merged and closed PRs too, so once a + # sync PR was merged the reused branch would report "refreshed" + # forever and silently never open another one. + if [ "${open_pr:-0}" != "0" ]; then + echo " refreshed the existing open PR" else body="$TOP/pr-body.md" : > "$body" diff --git a/docs/pyautobrain/spawn_spec.md b/docs/pyautobrain/spawn_spec.md index cef9bdf8..747e7ef7 100644 --- a/docs/pyautobrain/spawn_spec.md +++ b/docs/pyautobrain/spawn_spec.md @@ -46,7 +46,7 @@ deliberately, never silently shipped into a template. | 8 | `skills/**`, `policy/**` | KEEP verbatim (`OWNERSHIP.md`, `create_issue/` are generic; `policy/` is org-agnostic safety text) | | 9 | `.github/**` | **Per file, by the succeed-on-a-fresh-repo test below.** Not a blanket rule: owner substitution alone does NOT make a workflow work, because `YOURORG` is a literal placeholder — the template's own `spawn_drift` run failed `repository 'https://github.com/YOURORG/PyAutoMind/' not found`. See rules 9a–9c | | 9a | `.github/workflows/lifecycle_drift.yml` | KEEP verbatim — operates only on its own repo (checkout + local scripts) and contains no owner reference at all, so it needs no substitution and succeeds unmodified in a fresh org. Empirically the one green workflow in the template's run history | -| 9b | `.github/workflows/spawn_drift.yml` | SPECIAL → keep with the `schedule:` trigger **stripped**. The generator machinery is generic and worth shipping, but a fresh org has no published `*-template` repos, so a weekly run would fail until it does. `pull_request` + `workflow_dispatch` remain; a comment says to re-add the schedule once templates are published | +| 9b | `.github/workflows/spawn_drift.yml` | DROP — was "keep with the `schedule:` stripped", revised in #125. The self-heal added there makes this workflow depend on `secrets.PAT_PYAUTOLABS` AND on published `*-template` repos, neither of which a freshly-spawned org has, so **every** path in it is unrunnable there and the secret reference alone breaks the no-configured-secret condition. "When in doubt DROP" applies: an org that later publishes templates can adopt this workflow deliberately, having read it. The template still ships `scripts/spawn.py` + `tests/`, so the generator and its guards travel; only the org-coupled automation does not | | 9c | `.github/workflows/{morning_status,morning_health,arxiv_papers}.yml`, `.github/scripts/**` | DROP — instance automation. They hardcode sibling repo lists, organ-specific workflow names (`PyAutoHeart`/`PyAutoBrain`/`PyAutoHands`), org secrets (`PYAUTO_PAPERS_WEBHOOK_URL`, `CLAUDE_CODE_OAUTH_TOKEN`) and, in `arxiv_fetch.py`, strong-lensing search vocabulary plus dated incident notes. All 13 failing runs in the published template came from these | | 9d | any other `.github/**` | **No catch-all rule — UNMATCHED by design.** A fallback here is fail-*open*: a workflow added to Mind later would ride it into the template carrying whatever schedule and secrets it has, which is precisely the defect 9a–9c fix. A new `.github` file must fail the run and get an explicit entry above, like every other new file class | | 10 | `.claude/**`, `.codex/**` | DROP — agent-discovery symlinks are install artifacts recreated by the PyAutoBrain installer, not source content | diff --git a/scripts/spawn.py b/scripts/spawn.py index 3bb9dc91..45ab56aa 100644 --- a/scripts/spawn.py +++ b/scripts/spawn.py @@ -100,7 +100,11 @@ # # Ordered before the .github/scripts DROP and each other; first match wins. (".github/workflows/lifecycle_drift.yml", "KEEP"), # 9a: self-contained - (".github/workflows/spawn_drift.yml", "SPECIAL:unscheduled"), # 9b + # 9b: DROP (revised in #125). The self-heal makes this workflow depend on + # secrets.PAT_PYAUTOLABS and on published *-template repos; a fresh org has + # neither, so every path in it is unrunnable there and the secret reference + # alone breaks rule 9's no-configured-secret condition. + (".github/workflows/spawn_drift.yml", "DROP"), # 9c — instance automation: sibling repo lists, organ-specific workflow # names, org secrets, strong-lensing vocabulary. Every one of the 13 failing # runs in the published template came from these. @@ -478,104 +482,6 @@ def empty_body(src, rel=None): return header + "\n\n\n" -def unscheduled_workflow_body(src): - """Drop a workflow's `schedule:` trigger (spec rule 9b). - - `spawn_drift.yml` is generic machinery worth shipping, but its scheduled run - clones `/*-template` repos a freshly-spawned org does not have yet — - so on a schedule it would fail weekly and email the new owner. Stripping the - trigger keeps the capability (`workflow_dispatch`, `pull_request`) without - the noise. - - Structural, not textual: only the `schedule:` key that is a DIRECT CHILD of - the top-level `on:` mapping is removed, together with its block. Every other - line is passed through unchanged, except that line endings are normalised to - `\\n` (`splitlines()` discards the originals). - - Three near-misses this scoping exists to avoid, all found by testing rather - than by reading: - * a `schedule:` line inside a `run: |` shell block is script text, not a - trigger — a bare line match rewrote it into comments; - * `on.workflow_call.inputs.schedule` is a legitimate input, not a trigger, - so depth matters, not just "somewhere under `on:`"; - * a comment sitting at the SAME indent as `schedule:` used to end block - consumption early, orphaning the `- cron` line and emitting invalid YAML. - - Anything this cannot handle confidently (flow style, no schedule at all) - raises instead of guessing. - """ - def substantive(idx): - """Next line that is neither blank nor a comment, or None.""" - while idx < len(lines): - s = lines[idx].strip() - if s and not s.startswith("#"): - return idx - idx += 1 - return None - - lines = substitute_owner(src.read_text(errors="replace")).splitlines() - out, i, dropped = [], 0, False - on_child_indent = None # set once we know the `on:` block's child depth - while i < len(lines): - line = lines[i] - stripped = line.lstrip() - indent_here = len(line) - len(stripped) - is_blank_or_comment = (not stripped) or stripped.startswith("#") - - # Enter/leave the top-level `on:` mapping. - if indent_here == 0 and not is_blank_or_comment: - key = stripped.split(":", 1)[0].strip().strip("\"'") - if key == "on" and not stripped.split(":", 1)[1].strip(): - nxt = substantive(i + 1) - on_child_indent = ( - len(lines[nxt]) - len(lines[nxt].lstrip()) if nxt is not None else None - ) - else: - # Any other top-level key ends the block. Flow-style `on: {...}` - # lands here too and never sets a child indent, so it reaches - # the loud failure below rather than being edited blind. - on_child_indent = None - - if ( - on_child_indent is not None - and indent_here == on_child_indent # DIRECT child of `on:` - and stripped.startswith("schedule:") - and not stripped.startswith("#") - ): - pad = " " * indent_here - out.append(pad + "# schedule: removed by spawn — a fresh org has") - out.append(pad + "# no published *-template repos yet, so the run") - out.append(pad + "# would fail until it does. Re-add once you publish.") - i += 1 - # Consume the block. Blanks and comments belong to it only when - # deeper content follows; otherwise they introduce the NEXT key. - while i < len(lines): - s = lines[i].strip() - if not s or s.startswith("#"): - nxt = substantive(i) - if nxt is not None and ( - len(lines[nxt]) - len(lines[nxt].lstrip()) - ) > indent_here: - i += 1 - continue - break - if (len(lines[i]) - len(lines[i].lstrip())) <= indent_here: - break - i += 1 - dropped = True - continue - out.append(line) - i += 1 - if not dropped: - # The trigger this rule exists to remove is gone — the rule is now - # silently a no-op, which is how a guard rots. Fail loudly instead. - raise SystemExit( - f"spawn: {src.name} has no top-level 'on: schedule:' trigger to strip.\n" - f" SPECIAL:unscheduled is now a no-op — re-check spawn_spec.md rule 9b." - ) - return "\n".join(out) + "\n" - - def autonomy_log_body(src=None): """Return the autonomy ledger's schema header WITHOUT reading the source. @@ -628,8 +534,6 @@ def generate_mind(mind_root, out_dir): dest.write_text(substitute_owner(src.read_text(errors="replace"))) elif action == "EMPTY": dest.write_text(empty_body(src, rel)) - elif action == "SPECIAL:unscheduled": - dest.write_text(unscheduled_workflow_body(src)) elif action == "SPECIAL:autonomy_log": dest.write_text(autonomy_log_body(src)) elif action == "SPECIAL:body_map": @@ -902,8 +806,17 @@ def main(): if __name__ == "__main__": try: main() - except SystemExit: - raise + except SystemExit as exc: + # main() exits with an explicit EXIT_* code. Anything else raising + # SystemExit is a fail-closed generator path — `empty_body()` on an + # unmapped EMPTY file, say — which passes a STRING, and Python turns a + # string exit into code 1: indistinguishable from EXIT_DRIFT. Those are + # human decisions, exactly like UNMATCHED, so map them to EXIT_UNSAFE + # rather than letting the self-heal read them as "templates are stale". + if isinstance(exc.code, int) or exc.code is None: + raise + print(exc.code, file=sys.stderr) + sys.exit(EXIT_UNSAFE) except BaseException: # An unhandled exception would otherwise exit 1 — INDISTINGUISHABLE # from EXIT_DRIFT, so the Spawn Drift self-heal would read a crash as diff --git a/tests/test_spawn_template_contract.py b/tests/test_spawn_template_contract.py index 0acd4a70..201bcd97 100644 --- a/tests/test_spawn_template_contract.py +++ b/tests/test_spawn_template_contract.py @@ -78,21 +78,20 @@ def _fake_repo(root, files): # A .github mirroring the real one: two self-contained/generic workflows and # three pieces of instance automation (sibling repo lists, organ workflow # names, org secrets, domain vocabulary). +# The two workflows Mind really ships are read from disk, NOT hand-copied +# miniatures. A stale miniature is how the PAT_PYAUTOLABS reference slipped +# past `test_no_shipped_workflow_needs_a_configured_secret` in #125: the real +# spawn_drift.yml had grown a self-heal step the fixture knew nothing about. +_REAL_WORKFLOWS = Path(__file__).resolve().parents[1] / ".github" / "workflows" + + +def _real(name): + return (_REAL_WORKFLOWS / name).read_text() + + GITHUB_FILES = { - ".github/workflows/lifecycle_drift.yml": ( - "name: Lifecycle Drift\non:\n push:\n branches: [main]\n" - "jobs:\n drift:\n runs-on: ubuntu-latest\n steps:\n" - " - uses: actions/checkout@v4\n" - " - run: python3 scripts/lifecycle.py check\n" - ), - ".github/workflows/spawn_drift.yml": ( - "name: Spawn Drift\non:\n" - " schedule:\n - cron: \"17 6 * * 1\"\n" - " pull_request:\n paths:\n - \"scripts/spawn.py\"\n" - " workflow_dispatch:\n\n" - "jobs:\n drift:\n runs-on: ubuntu-latest\n steps:\n" - " - run: git clone https://github.com/PyAutoLabs/PyAutoMind\n" - ), + ".github/workflows/lifecycle_drift.yml": _real("lifecycle_drift.yml"), + ".github/workflows/spawn_drift.yml": _real("spawn_drift.yml"), ".github/workflows/morning_status.yml": ( "name: digest\non:\n schedule:\n - cron: \"0 6 * * *\"\n" "jobs:\n d:\n runs-on: ubuntu-latest\n steps:\n" @@ -113,6 +112,7 @@ def _fake_repo(root, files): } DROPPED_GITHUB = [ + ".github/workflows/spawn_drift.yml", # rule 9b, revised to DROP in #125 ".github/workflows/morning_status.yml", ".github/workflows/morning_health.yml", ".github/workflows/arxiv_papers.yml", @@ -143,7 +143,7 @@ def test_instance_automation_is_not_shipped(mind_with_github): def test_generic_workflows_are_still_shipped(mind_with_github): """Guard the other direction — rule 9 must not over-drop.""" names = {p.name for p in _shipped_workflows(mind_with_github)} - assert names == {"lifecycle_drift.yml", "spawn_drift.yml"}, names + assert names == {"lifecycle_drift.yml"}, names def test_no_shipped_workflow_runs_on_a_schedule(mind_with_github): @@ -196,15 +196,6 @@ def test_a_new_mind_workflow_is_a_human_decision(tmp_path): assert not (out / ".github" / "workflows" / "brand_new_thing.yml").exists() -def test_unscheduled_transform_fails_loudly_if_it_becomes_a_noop(tmp_path): - """If spawn_drift ever loses its schedule upstream, the rule silently stops - doing anything — that is how a guard rots. It must fail instead.""" - src = tmp_path / "spawn_drift.yml" - src.write_text("name: x\non:\n workflow_dispatch:\njobs: {}\n") - with pytest.raises(SystemExit): - spawn.unscheduled_workflow_body(src) - - MINIMAL_MEMORY = { "README.md": "# Mem\n", "AGENTS.md": "# A\n", "CLAUDE.md": "# C\n", "LICENSE": "MIT\n", ".gitignore": "tmp/\n", "Makefile": "all:\n", @@ -242,108 +233,6 @@ def test_memory_github_is_also_fail_closed(tmp_path): assert (out / ".github" / "workflows" / "validate.yml").exists() -def test_unscheduled_transform_only_touches_the_on_mapping(tmp_path): - """A `schedule:` line inside a `run: |` block is shell, not a trigger. - - The first draft matched any line starting with `schedule:` and rewrote that - shell line into comments — silently mangling the script. The strip is scoped - to the top-level `on:` mapping. - """ - src = tmp_path / "spawn_drift.yml" - src.write_text( - "name: x\non:\n schedule:\n - cron: \"0 6 * * *\"\n workflow_dispatch:\n" - "jobs:\n j:\n runs-on: ubuntu-latest\n steps:\n - run: |\n" - " schedule: not a trigger\n echo done\n" - ) - - out = spawn.unscheduled_workflow_body(src) - - assert "schedule: not a trigger" in out, "the run block was corrupted" - assert "echo done" in out - spec = yaml.safe_load(out) - triggers = spec[True] if True in spec else spec["on"] - assert "schedule" not in triggers and "workflow_dispatch" in triggers - - -@pytest.mark.parametrize( - "body", - [ - # Flow style — a line-based transform cannot safely edit it. - 'name: x\non: {schedule: [{cron: "0 6 * * *"}]}\njobs: {}\n', - # No schedule at all — the rule would be a silent no-op. - "name: x\non:\n workflow_dispatch:\njobs: {}\n", - ], -) -def test_unscheduled_transform_fails_rather_than_guessing(tmp_path, body): - src = tmp_path / "spawn_drift.yml" - src.write_text(body) - with pytest.raises(SystemExit): - spawn.unscheduled_workflow_body(src) - - -@pytest.mark.parametrize( - "body", - [ - # Quoted `on` key — YAML 1.1 turns bare `on` into True, so some repos quote it. - 'name: x\n"on":\n schedule:\n - cron: "0 6 * * *"\n workflow_dispatch:\njobs: {}\n', - # Comment nested inside the schedule block. - 'name: x\non:\n schedule:\n # nightly\n - cron: "0 6 * * *"\n workflow_dispatch:\njobs: {}\n', - # Comment at the SAME indent as `schedule:` — used to end block - # consumption early, orphaning `- cron` and emitting invalid YAML. - 'name: x\non:\n schedule:\n # nightly\n - cron: "0 6 * * *"\n workflow_dispatch:\njobs: {}\n', - # Comment introducing the NEXT key must survive with that key. - 'name: x\non:\n schedule:\n - cron: "0 6 * * *"\n # manual only\n workflow_dispatch:\njobs: {}\n', - # CRLF line endings. - 'name: x\r\non:\r\n schedule:\r\n - cron: "0 6"\r\n workflow_dispatch:\r\njobs: {}\r\n', - ], -) -def test_unscheduled_transform_handles_awkward_yaml(tmp_path, body): - src = tmp_path / "spawn_drift.yml" - src.write_text(body) - out = spawn.unscheduled_workflow_body(src) - spec = yaml.safe_load(out) # must not raise — invalid YAML is the failure - triggers = spec[True] if True in spec else spec["on"] - assert "schedule" not in triggers - assert "workflow_dispatch" in triggers - assert "- cron" not in out, "orphaned cron entry left behind" - - -def test_unscheduled_transform_keeps_a_workflow_call_schedule_input(tmp_path): - """`on.workflow_call.inputs.schedule` is an input, not a trigger. - - Depth matters, not merely "somewhere under `on:`" — only a DIRECT child of - the top-level `on:` mapping is a trigger. - """ - src = tmp_path / "spawn_drift.yml" - src.write_text( - 'name: x\non:\n schedule:\n - cron: "0 6 * * *"\n' - " workflow_call:\n inputs:\n schedule:\n type: string\njobs: {}\n" - ) - - out = spawn.unscheduled_workflow_body(src) - - spec = yaml.safe_load(out) - triggers = spec[True] if True in spec else spec["on"] - assert "schedule" not in triggers, "the trigger should be gone" - assert triggers["workflow_call"]["inputs"]["schedule"]["type"] == "string", ( - "the workflow_call input was deleted along with the trigger" - ) - - -def test_unscheduled_transform_preserves_everything_else(tmp_path): - """Structural strip: only the schedule block goes.""" - src = tmp_path / "spawn_drift.yml" - src.write_text(GITHUB_FILES[".github/workflows/spawn_drift.yml"]) - - spec = yaml.safe_load(spawn.unscheduled_workflow_body(src)) - triggers = spec[True] if True in spec else spec["on"] - - assert "schedule" not in triggers - assert "workflow_dispatch" in triggers - assert triggers["pull_request"]["paths"] == ["scripts/spawn.py"] - assert list(spec["jobs"]) == ["drift"] - - def test_spawn_stamps_the_templates_complete_index(tmp_path): """spawn must run the GENERATED tree's own lifecycle.py, not the live one.""" mind = tmp_path / "PyAutoMind" @@ -417,6 +306,41 @@ def test_check_exit_codes_are_the_self_heal_contract(tmp_path): assert _check_exit(tmp_path, published) == spawn.EXIT_DRIFT +def test_the_workflow_only_proposes_on_the_drift_code(): + """Pin the CONSUMER against the producer, not the producer against itself. + + Every other exit-code test compares a subprocess result with constants + imported from the same module, so swapping `EXIT_DRIFT` and `EXIT_UNSAFE` + would leave them all green while the workflow still auto-proposed literal + exit 1 — now the unsafe one. This reads the real workflow. + """ + wf = yaml.safe_load(_real("spawn_drift.yml")) + steps = {s.get("name"): s for s in wf["jobs"]["drift"]["steps"]} + propose = steps["Propose the sync PR"] + + assert propose["if"] == f"steps.diff.outputs.code == '{spawn.EXIT_DRIFT}'", ( + "the PR step must trigger on EXIT_DRIFT and nothing else — " + f"got {propose['if']!r} against EXIT_DRIFT={spawn.EXIT_DRIFT}" + ) + for unsafe in (spawn.EXIT_UNSAFE, spawn.EXIT_CRASH): + assert f"'{unsafe}'" not in propose["if"], ( + f"exit {unsafe} must never reach the proposal step" + ) + + +def test_the_workflow_handles_every_exit_code_it_can_see(): + """An unhandled code must hit the catch-all, not fall through silently.""" + diff_step = next( + s for s in yaml.safe_load(_real("spawn_drift.yml"))["jobs"]["drift"]["steps"] + if s.get("name") == "Regenerate + diff" + ) + run = diff_step["run"] + arms = set(re.findall(r"^\s*([0-9]+|\*)\)", run, re.MULTILINE)) + for code in (spawn.EXIT_CLEAN, spawn.EXIT_DRIFT, spawn.EXIT_UNSAFE): + assert str(code) in arms, f"exit {code} has no case arm (found {arms})" + assert "*" in arms, "no catch-all arm for unexpected exit codes" + + def test_a_crash_is_not_reported_as_drift(tmp_path): """Python exits 1 on an unhandled exception — the same code as EXIT_DRIFT. @@ -441,6 +365,30 @@ def test_a_crash_is_not_reported_as_drift(tmp_path): assert "simulated crash" in r.stderr, "the traceback must still be visible" +def test_a_fail_closed_SystemExit_is_unsafe_not_drift(tmp_path): + """`SystemExit("message")` also exits 1 — the same code as EXIT_DRIFT. + + The fail-closed generator paths (an unmapped EMPTY file, say) raise exactly + that. They are human decisions like UNMATCHED, so they must report UNSAFE; + otherwise the self-heal reads them as ordinary staleness. + """ + broken = tmp_path / "spawn_failclosed.py" + broken.write_text( + SPAWN_PY.read_text().replace( + " results = generate_all(root, out_root)", + " raise SystemExit('spawn: simulated fail-closed decision')", + ) + ) + r = subprocess.run( + [sys.executable, str(broken), "--check", str(tmp_path)], + capture_output=True, text=True, + ) + assert r.returncode == spawn.EXIT_UNSAFE, ( + f"fail-closed exit must be UNSAFE, got {r.returncode}" + ) + assert "simulated fail-closed decision" in r.stderr + + def test_canary_hit_reports_unsafe_not_drift(tmp_path): """The interlock: a leak must be distinguishable from mechanical drift.""" token = spawn.CANARY_TOKENS[0]