fix(harness): stop PR polling on denied GitHub access (#1864)
This commit is contained in:
@@ -55,10 +55,36 @@ Free-form notes are forbidden outside the markers. The planner parses this verba
|
||||
|
||||
Every value in the report — check names, run_ids, conclusions, counts — must come from command output you actually observed this run. Never guess a check name or infer a run_id. If a command returns empty or errors (output capture fails, `gh` errors, rate limit), do NOT fill the field from memory or a generic CI template: emit the field as `unknown` and state the data-gathering failure in `recommendation:`. A failure you reported honestly is recoverable; a fabricated `ci_failed` entry sends a remediation worker chasing a phantom fix.
|
||||
|
||||
An approval denial, cancellation, or interruption is not a GitHub fetch error.
|
||||
Do not hide it behind a generic `unknown` recommendation or retry it as a
|
||||
transient failure. Follow the access gate below and use its exact terminal
|
||||
recommendation.
|
||||
|
||||
The `claude_summary` line carries the **latest** Claude summary's structured findings table. Pure issue-comment counts (`issue_comments_from_bots`) miss this because the count alone can't tell the parent whether the comment is actionable (e.g. CodeRabbit's "review skipped, too many files" boilerplate ≠ a Claude finding). Use `claude_summary` to drive triage, not the raw count.
|
||||
|
||||
## Procedure
|
||||
|
||||
### GitHub access approval gate
|
||||
|
||||
Before the first command that can access GitHub, check the runtime's current
|
||||
network access. If access is already granted, including in a full-access
|
||||
runtime, run the command normally. If network access requires approval and the
|
||||
runtime supports prompting, request it proactively through the runtime's
|
||||
escalation or approval mechanism; do not run an unapproved probe first and wait
|
||||
for it to fail. This includes `scripts/pr-state`, `scripts/pr-resolve`, and
|
||||
`scripts/run-quiet` when they wrap `gh`, as well as direct `gh pr`, `gh api`,
|
||||
and `gh run` calls.
|
||||
|
||||
If approval is denied, cancelled, or interrupted, stop immediately. Do not run
|
||||
another GitHub command, switch helpers, wait, or ask the planner to relaunch
|
||||
polling. Emit the normal marker-delimited report, set every unobserved
|
||||
GitHub-derived value to `unknown`, and use this exact recommendation:
|
||||
|
||||
`GitHub access requires approval; planner must surface the approval gate to the user and must not relaunch polling.`
|
||||
|
||||
Only errors observed after access was approved and the command actually ran,
|
||||
such as a DNS timeout or GitHub API failure, use the fetch-failure recovery path.
|
||||
|
||||
1. **Resolve PR and conflict state.** Run
|
||||
`gh pr view --json number,url,headRefName,baseRefName,headRefOid,mergeable,mergeStateStatus`
|
||||
(or pass the PR number explicitly). Capture the head SHA and GitHub
|
||||
@@ -186,11 +212,12 @@ The `claude_summary` line carries the **latest** Claude summary's structured fin
|
||||
`rate_limited`, mergeability and local conflict state are known clean, and
|
||||
all required counts are known. The `recommendation:` line is one short
|
||||
sentence chosen from this menu, picking the first that applies:
|
||||
- `"GitHub access requires approval; planner must surface the approval gate to the user and must not relaunch polling."` if the network escalation or approval request was denied, cancelled, or interrupted
|
||||
- `"PR has merge conflicts — planner should assign bounded conflict resolution."` if `mergeable` is `CONFLICTING`, `merge_state_status` is `DIRTY`, or `local_unmerged_entries` is greater than zero
|
||||
- `"CI failed — planner should assign log triage and remediation."` if `ci_failed` is non-empty
|
||||
- `"Claude summary flags <N> blocker(s); planner should assign comment triage and remediation."` if `claude_summary.blockers > 0`
|
||||
- `"Polling timed out with pending items; planner should decide whether to launch another poll."` if any axis hit the cap
|
||||
- `"PR state fetch failed; planner should retry polling."` if a required fetch failed or any required value is `unknown`
|
||||
- `"PR state fetch failed; planner should retry polling."` if an approved request ran but a required fetch failed or any required value is `unknown`
|
||||
- `"Polling is incomplete; planner should continue polling."` if CI or any bot is still pending
|
||||
- `"All checks green; planner should assign triage for <N> unresolved review threads."` if `unresolved_review_threads > 0`
|
||||
- `"All threads resolved; Claude has <N> pending suggestion(s) — planner should assign triage."` if `claude_summary.suggestions > 0`
|
||||
|
||||
@@ -54,6 +54,14 @@ step can retain formatter changes. It does not authorize source or test fixes.
|
||||
recommendation for a new implementer assignment.
|
||||
- For goleak failures after otherwise passing Go tests, loop the affected package with `go test -race -count=10 ./internal/<pkg>/...` and inspect cleanup ownership.
|
||||
- If Go tests fail because `httptest.NewServer` cannot bind loopback in a restricted sandbox, rerun the exact command with normal network/loopback escalation before diagnosing code.
|
||||
- If required filesystem, network, or loopback escalation is unavailable,
|
||||
denied, cancelled, or interrupted, stop. Report verification as blocked,
|
||||
explain that mandatory verification is preventing commit and PR creation,
|
||||
and include a **Required user action** telling the user to enable the
|
||||
runtime's full access mode and retry verification. Do not offer to proceed
|
||||
unverified or imply that the agent or repository host cannot create PRs.
|
||||
Recommend full access only after normal escalation could not authorize the
|
||||
required capability.
|
||||
- For `ENOSPC`, read-only cache, or cache-initialization failures, preserve an existing absolute managed `GOCACHE` and existing lint cache. Move only invocation scratch (`TMPDIR` and quiet logs) to a writable filesystem. If a cache filesystem itself is unusable, relocate only that cache to a persistent agent-owned path outside every worktree; never use a repository-local fallback.
|
||||
- For Rust/Tauri changes, compare `rustc --version` with `apps/desktop/src-tauri/Cargo.toml`'s `rust-version`. Activate a matching installed toolchain without replacing `PATH`; report or request installation if unavailable.
|
||||
- For environment-only failures, rerun the exact command after correcting
|
||||
@@ -67,4 +75,5 @@ step can retain formatter changes. It does not authorize source or test fixes.
|
||||
generation, typecheck, the complete test target, lint, and any scoped Rust
|
||||
tests all pass.
|
||||
|
||||
Do not spawn subagents. Report pass/fail state and blockers to the planner.
|
||||
Do not spawn subagents. Report pass/fail state, blockers, and any required user
|
||||
action to the planner.
|
||||
|
||||
@@ -16,12 +16,11 @@ bounded implementer assignment. It does not run GitHub, test, edit, commit, or
|
||||
push commands directly. If a required worker is unavailable, stop and report
|
||||
the blocked phase.
|
||||
|
||||
Each worker uses only the procedure for its assigned phase: `pr-poller` gathers
|
||||
state, a remediation implementer handles CI logs/code/comments, `verify` runs
|
||||
checks and reports failures, and a delivery implementer commits or pushes an
|
||||
already verified result. Workers never invoke one another.
|
||||
Each worker uses only its assigned phase: polling, remediation, verification, or delivery.
|
||||
Workers never invoke one another.
|
||||
If `pr-poller` recommends `GitHub access requires approval; planner must surface the approval gate to the user and must not relaunch polling.`, show the gate and stop. Do not switch tools or treat denied, cancelled, or interrupted approval as a transient fetch failure.
|
||||
|
||||
> **GitHub tool selection:** This skill uses `gh` CLI commands by default. If `gh` is unavailable or fails, use any available GitHub tools in the environment (e.g. MCP GitHub tools) for PR checks, comments, replies, and reviews. Some operations (reactions, resolving threads, fetching CI logs) may not be available in all environments — skip gracefully.
|
||||
> **GitHub tool selection:** This skill uses `gh` CLI commands by default. After access is approved, if `gh` is unavailable or a request fails, use any available GitHub tools in the environment (e.g. MCP GitHub tools) for PR checks, comments, replies, and reviews. An approval denial, cancellation, or interruption is the terminal gate above, not a reason to switch tools. Some operations (reactions, resolving threads, fetching CI logs) may not be available in all environments — skip gracefully.
|
||||
> **Helper scripts location:** `scripts/pr-state`, `scripts/pr-resolve`, and `scripts/run-quiet` are at the worktree root (`<worktree>/scripts/...`), not under `.agents/skills/pr-fixup/scripts/`.
|
||||
|
||||
## Available skills and subagents
|
||||
@@ -78,15 +77,15 @@ index state. Treat the gate as clean only when `mergeable: MERGEABLE`,
|
||||
`local_unmerged_entries: 0`. If any field reports a conflict, assign a bounded
|
||||
implementer to load `references/merge-conflicts.md`, resolve it, and return
|
||||
targeted verification before continuing. If any gate field is `unknown`,
|
||||
re-poll instead of taking a clean-state shortcut. The planner does not inspect
|
||||
or resolve conflicts directly.
|
||||
re-poll instead of taking a clean-state shortcut. The terminal approval gate
|
||||
above overrides this rule. The planner does not inspect or resolve conflicts directly.
|
||||
|
||||
**Parse the report.** The fields you care about:
|
||||
|
||||
- `ci_failed` — non-empty list of `{name, run_id, conclusion, url}`.
|
||||
`ci_failed: unknown` means CI collection failed, blocks clean state, and
|
||||
requires re-polling. Omission is known empty only when the complete report
|
||||
parsed successfully and its recommendation reports no fetch failure.
|
||||
requires re-polling after an approved fetch failure. The terminal approval
|
||||
gate overrides this rule. Omission is known empty only after a complete report.
|
||||
- `ci_pending` — anything still running when the 20-min cap hit. Only `none` is
|
||||
a known non-pending state.
|
||||
- `bots.<name>` — `done` / `rate_limited` / `pending` / `timeout` / `unknown`.
|
||||
@@ -112,7 +111,7 @@ stop and report the blocked phase.
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
If this command, `scripts/pr-resolve`, or another GitHub helper fails with a transient DNS/network error, retry the parent command with the runtime's required network escalation; treat state as unknown until it succeeds.
|
||||
Request runtime network approval before the first GitHub helper call; denial, cancellation, or interruption stops the workflow, while transient failures from approved commands use bounded retry.
|
||||
|
||||
`scripts/pr-state` accepts flags before or after the PR (`scripts/pr-state --summary <PR>` and `scripts/pr-state <PR> --summary` both work). When parsing output with `jq`, prefer writing the JSON to a temp file first, then running `jq` against that file; this avoids `set -e` surprises and prevents stderr from corrupting a JSON pipe.
|
||||
|
||||
|
||||
@@ -23,6 +23,12 @@ Invoke the `verify` worker in a single call. Wait for it to complete and surface
|
||||
- If verify passes cleanly: report success.
|
||||
- If verify fails: create a bounded remediation assignment from its report and
|
||||
do not proceed with downstream actions that depend on green verification.
|
||||
- If verify reports that required sandbox capabilities could not be authorized,
|
||||
stop before commit, push, or PR creation and surface its required user action.
|
||||
On Codex, tell the user exactly: "Switch the mode selector to
|
||||
`Agent (full access)`, then retry verification." Explain that PR creation is
|
||||
waiting on mandatory verification; do not imply that Codex or GitHub cannot
|
||||
create PRs, ask whether to proceed unverified, or launch downstream workers.
|
||||
|
||||
Do not run verification commands in the planner session. The worker's prompt
|
||||
contains the full procedure in `.agents/agents/verify.md`.
|
||||
@@ -105,6 +111,17 @@ as a sandbox limitation. Rerun the exact command with the runtime's normal
|
||||
network or loopback escalation. Diagnose test code only if the escalated rerun
|
||||
still fails.
|
||||
|
||||
If that escalation is unavailable, denied, cancelled, or interrupted, stop and
|
||||
return a blocked verification report with a **Required user action** section.
|
||||
State that mandatory verification must pass before commit and PR creation can
|
||||
continue. On Codex, the action must say exactly: "Switch the mode selector to
|
||||
`Agent (full access)`, then retry verification." On other runtimes, tell the
|
||||
user to enable the runtime's full filesystem, network, or loopback access as
|
||||
needed, then retry verification. Do not offer to proceed with an unverified PR
|
||||
or describe the blocker as an inability of Codex or the repository host to
|
||||
create one. Recommend full access only after normal escalation could not
|
||||
authorize the required capability in the current mode.
|
||||
|
||||
For desktop Rust changes, compare `rustc --version` with the `rust-version` in
|
||||
`apps/desktop/src-tauri/Cargo.toml` before running the Rust suite. Activate an
|
||||
installed matching rustup toolchain, extending `PATH` rather than replacing it
|
||||
|
||||
Reference in New Issue
Block a user