diff --git a/AGENTS.md b/AGENTS.md index bca2b108..63ba2aef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -160,33 +160,52 @@ Like the other PyAuto repos, PyAutoBrain runs from its checkout (no pip install) it resolves the sibling `pyauto-heart` and `autohands` binaries from PATH or the `~/Code/PyAutoLabs/` checkouts. -## Remote sessions: three lines that pay for themselves - -Measured in a web/mobile container, where this file is loaded and little else is. - -- **Tests run 3.5x faster in parallel** — 4 cores, subprocess-heavy suites, no - single slow test: this repo's 554 tests are 96s on one core and 28s on four. - Use `python3 -m pytest -q -n auto`. -- **A session holding several organs registers no SessionStart hook.** Claude - Code reads project hooks from the project directory, which in that layout is - the repos' *parent*, not a repo. So knock on the door yourself in the first - turn: `bash PyAutoMind/scripts/session_bootstrap.sh` (add `--check` to report - only). `bin/pyauto-brain` already calls it before every verb. The symptom of - skipping it is a pytest that is not this workspace's — collection - `ImportError`s naming `yaml`, or `No module named pytest`. + +## Remote sessions (Claude Code on web and mobile) + +Three facts, all measured in a web/mobile container, where this file is loaded +and little else is. They ride in every organ because a session may hold any +subset of them — and the session that needs this most is the one holding +several, which is exactly the session no hook fires in. + +- **Bootstrap in the first turn, unconditionally** — before the first test + command, not as a remedy once something looks wrong: + + ``` + bash PyAutoMind/scripts/session_bootstrap.sh # ~10s cold, ~1s warm + bash PyAutoMind/scripts/session_bootstrap.sh --check # report only + ``` + + A session holding several organs registers no SessionStart hook — Claude Code + reads project hooks from the project directory, which in that layout is the + repos' *parent*, not a repo — so nothing has set this session up. It was once + phrased as a remedy keyed to `No module named pytest` or collection + `ImportError`s naming `yaml`; that symptom stopped appearing when the + container image moved to Python 3.12, while the environment is still wrong in + ways that read like a bad command rather than a stale session (`pytest -n + auto` → `unrecognized arguments: -n`). The bootstrap also **unshallows the + clones**: a remote session clones shallow, and `git merge-base --is-ancestor` + then answers "not an ancestor" for a commit whose ancestry is merely absent — + the answer the ship and close-out procedures act on when proving a branch + merged. + +- **Then run the suite in parallel.** 4 cores, subprocess-heavy suites, no + single slow test: about 3.5x. `python3 -m pytest -q -n auto`, with + `pytest-xdist` supplied by the bootstrap above. + - **There is no `gh`, and installing one does not help.** A remote session reaches GitHub through the `mcp__github__*` tools, already scoped to the - session's repos. `gh` itself is two seconds away (`apt-get install gh`) and - is a trap: it authenticates — `gh api user` returns your login — and then - 403s every repo-scoped call, because the egress proxy serves neither the - REST repo paths nor GraphQL beyond a pinned set of PR-review operations. So - `gh pr list`, `gh issue list`, `gh repo view` and `gh api repos/...` all - fail *after* the install, which is worse than the honest `command not - found`. Read [`PyAutoBrain/skills/GITHUB_ACCESS.md`](skills/GITHUB_ACCESS.md) - at the top of any run that touches GitHub; it maps each `gh` operation onto - its MCP tool. Spell that path from the workspace root, as written: a session - holding several organs is cwd'd at the repos' *parent*, so a bare `skills/…` - reads as a missing file rather than a missing repo prefix. + session's repos. `gh` installs in two seconds and is a trap: it authenticates, + then 403s every repo-scoped call, because the egress proxy serves neither the + REST repo paths nor GraphQL beyond a pinned set of PR-review operations — a + binary that looks healthy and fails everything that matters. It also defeats + the surface probe, which keys off `gh auth status`. Read + `PyAutoBrain/skills/GITHUB_ACCESS.md` at the top of any run that touches + GitHub; it maps each `gh` operation onto its MCP tool. Spell that path from + the workspace root, as written: a multi-organ session is cwd'd at the repos' + *parent*, so a bare `skills/…` reads as a missing file rather than a missing + repo prefix. + ## The command surface (Brain implicit) diff --git a/skills/GITHUB_ACCESS.md b/skills/GITHUB_ACCESS.md index eae08858..1c2504ac 100644 --- a/skills/GITHUB_ACCESS.md +++ b/skills/GITHUB_ACCESS.md @@ -113,10 +113,21 @@ leave it that way. | Create a branch | `gh api -X POST .../git/refs` | `create_branch` | | Releases | `gh release view/list` | `get_latest_release`, `list_releases`, `get_release_by_tag` | | Who am I | `gh api user` | `get_me` | +| Be woken by CI / comments on a PR | *(no equivalent — a CLI polls)* | `subscribe_pr_activity`, `unsubscribe_pr_activity` | Tool names are given unprefixed; the harness exposes them as `mcp__github__`. If a name is not loaded, `ToolSearch` fetches its schema. +That last row is the one place the MCP surface **beats** `gh` rather than +merely matching it. A CLI can only ask again; a subscribed session is woken by +the check-suite rollup, so waiting for CI costs one turn per event instead of +one turn per poll. Take it wherever a run would otherwise sit in a loop +(`/prm` step 3), and drop the subscription when the PR merges. Webhooks are +best-effort — a *successful* rollup is the event most likely to go missing — so +pair it with a `send_later` check-in (claude-code-remote) where that tool +exists, and re-read the PR's real state on every wake rather than trusting the +event to be the whole story. + ## What the MCP surface cannot do - **Delete a remote branch.** There is no MCP tool for it, and in a proxied web diff --git a/skills/prm/prm.md b/skills/prm/prm.md index 889c56e6..9e2eae44 100644 --- a/skills/prm/prm.md +++ b/skills/prm/prm.md @@ -43,6 +43,27 @@ command -v gh >/dev/null 2>&1 && gh auth status >/dev/null 2>&1 \ `/prm` **deletes no remote branch on any surface** (step 5.6), so nothing here needs the second probe that step used to carry. +### The `mcp` lane, step by step + +Every GitHub call a close-out makes, so a mobile run reads this page and the +mapping page only if something here is missing. Names take the +`mcp__github__` prefix (the wake tools are the harness's own); `ToolSearch` +fetches a schema that is not loaded. + +| Step | Call | +|---|---| +| 1 resolve | `list_pull_requests` (state `open`), or `pull_request_read` `get` for a known number. Mind's `active.md` via `get_file_contents` | +| 2 judge CI | `pull_request_read` `get_check_runs` — every check on the head commit, in one call. Then `pull_request_read` `get` for `mergeable` / `merge_state_status` | +| 2 by workflow | `actions_list` `list_workflow_runs` **has no `head_sha` filter** (`workflow_runs_filter` is actor/branch/event/status only): filter by `branch`, then match `head_sha` yourself, or you will judge a stale run. Legs: `actions_list` `list_workflow_jobs`, `resource_id` = run id | +| 3 wait | `subscribe_pr_activity` (owner, repo, pullNumber), then end the turn; `send_later` for the fallback check-in | +| 3 red | `get_job_logs` with `run_id`, `failed_only: true`, `return_content: true` — **before** anything else, the blob is purged | +| 4 merge | `merge_pull_request` (`merge_method: "merge"`), then `pull_request_read` `get` to confirm `MERGED`; `unsubscribe_pr_activity` | +| 5.2 issue | `add_issue_comment`, then `issue_write` `update` with `state: "closed"` and `state_reason: "completed"` — an unset reason is what leaves an issue reading "closed as not planned" | +| 5.1 proof | git, not GitHub — and `--is-ancestor` needs an unshallowed clone (`reference.md` §1) | + +Steps 5.3-5.7 touch no GitHub API at all; what they need is a checkout, and the +note at the foot of step 5 says what to do without one. + ## 1. Resolve the target PR(s) In order: explicit argument → current branch (`gh pr view --json`) → the claimed @@ -64,9 +85,21 @@ every run for the sha and every job in it, and treat anything not `completed` as ## 3. Wait, or stop -- **Pending** — poll every ~90s, one compact line per poll (`3/4 legs done`), - capped at ~30 min; then report where it stands rather than spinning. Under - `--no-wait`, skip the loop entirely: report and stop. +- **Pending** — wait by the cheapest mechanism the surface has. Under + `--no-wait`, do neither: report and stop. + + - **Be woken (mobile, web — preferred there).** Where the harness exposes + `subscribe_pr_activity`, call it once per target PR and **end the turn**. + CI completions and review comments then wake the session: one turn per + event instead of one turn per 90s, and the phone can be put down between + them. Pair it with a `send_later` check-in ~15 min out where that tool + exists — a *successful* rollup is the event most likely to arrive late or + not at all, and the check-in is what stops a green PR sitting unmerged. + On every wake re-judge from step 2 (an event is a reason to look, never + evidence of green) and re-arm the check-in until the merge lands. + - **Poll (local CLI, or no wake tools).** Every ~90s, one compact line per + poll (`3/4 legs done`), capped at ~30 min; then report where it stands + rather than spinning. - **Red** — **fetch the failing job's log immediately** (GitHub purges the blob; once purged the failure is unnameable forever), quote the failing step, and stop. Do not merge, do not re-run, do not "wait for the flake to pass". Offer @@ -84,6 +117,10 @@ Green on every leg → merge, in this order: 2. `gh pr merge --merge` per PR (`-R owner/repo` when you have no checkout), then confirm the state is `MERGED` — queued or auto-merge is not merged. Never `--delete-branch`: it takes the local branch too, orphaning a task worktree. +3. **Drop any subscription step 3 took.** Once every target PR reads `MERGED`, + `unsubscribe_pr_activity` each one and cancel the `send_later` check-in. A + subscription outliving its merge wakes later sessions on a PR nobody is + driving. Never force, never override a protection, never rewrite history. If a merge is refused by GitHub, report the reason verbatim and stop. diff --git a/skills/prm/reference.md b/skills/prm/reference.md index f1c49e9f..4b71d6b4 100644 --- a/skills/prm/reference.md +++ b/skills/prm/reference.md @@ -123,6 +123,14 @@ git rev-list --count origin/main..origin/feature/ # want 0 `UNMERGED` with no open PR → stop the close-out and report which repo. Squash merges break `--is-ancestor`; fall back to the PR's `state=MERGED`. +**On a shallow clone this answer is a lie, not a verdict.** A remote session +clones shallow (measured 2026-08-27: PyAutoMind arrived with 78 of 4478 +commits), and `--is-ancestor` reports "not an ancestor" for a commit whose +ancestry is merely *absent* — so a merged branch reads `UNMERGED` and the +close-out stops on nothing. `session_bootstrap.sh` unshallows every checkout, +which is one more reason it is the first thing a session runs; if you did not +run it, `git rev-parse --is-shallow-repository` before trusting this step. + ### 2. Issue: comment, then close `gh issue close` prints its usage string and exits non-zero in this gh (2.4.0) — diff --git a/tests/test_gh_surface.py b/tests/test_gh_surface.py index 3df6e309..a22a2073 100644 --- a/tests/test_gh_surface.py +++ b/tests/test_gh_surface.py @@ -139,3 +139,44 @@ def test_helper_locates_itself_without_readlink(tmp_path): capture_output=True, text=True, env=env, timeout=60) assert "command not found" not in r.stderr, r.stderr assert "GITHUB_ACCESS.md" in r.stderr + + +def test_waiting_for_ci_prefers_being_woken_over_polling(): + """A poll costs a turn every 90s; a wake costs one per event. + + The MCP surface is usually a *reduced* `gh`, so the one operation where it + is strictly better is easy to leave unused. `/prm` is where a run would + otherwise sit in a loop, so its wait step must offer the subscription + first, and the mapping page must carry the tool that provides it. + """ + access = ACCESS_PAGE.read_text() + for tool in ("subscribe_pr_activity", "unsubscribe_pr_activity"): + assert tool in access, f"{tool} is unmapped" + + prm = (SKILLS / "prm" / "prm.md").read_text() + wait = prm.split("## 3. Wait, or stop")[1].split("## 4.")[0] + assert "subscribe_pr_activity" in wait, "the wait step never mentions waking" + assert wait.index("subscribe_pr_activity") < wait.index("~90s"), ( + "polling is offered before the cheaper wake path") + # And the subscription must be dropped, or later sessions wake on a PR + # nobody is driving. + assert "unsubscribe_pr_activity" in prm.split("## 4.")[1] + + +def test_the_close_out_carries_its_own_mcp_calls(): + """The mobile lane must not cost two pages to read. + + `/prm` is ~4k words of `gh` mechanics; a run without `gh` used to load them + AND the mapping page, then translate step by step. The calls that close a + task out are few and stable enough to name inline, so the mapping page is + the index, not a required second read. + """ + prm = (SKILLS / "prm" / "prm.md").read_text() + lane = prm.split("### The `mcp` lane")[1].split("## 1.")[0] + for call in ("pull_request_read", "actions_list", "get_job_logs", + "merge_pull_request", "add_issue_comment", "issue_write", + "subscribe_pr_activity", "get_file_contents"): + assert call in lane, f"the mcp lane never names {call}" + # The two traps that cost a run its turns, not just a lookup. + assert "head_sha" in lane, "nothing warns that runs cannot be filtered by sha" + assert "state_reason" in lane, "closing an issue without a reason mislabels it"