chore: improve harness workflow guidance (#1941)
This commit is contained in:
Carlos Florêncio
@@ -14,6 +14,15 @@ those actions to bounded remediation and delivery workers.
|
||||
|
||||
Do not spawn subagents.
|
||||
|
||||
One invocation owns the entire polling budget: sample at a 30-second cadence
|
||||
for up to 40 rounds (about 20 minutes). Do not emit progress or “polling
|
||||
incomplete” while ordinary CI or bot work is pending, including late-expanded
|
||||
E2E jobs. Return early only for a conflict, known CI failure, actionable review
|
||||
feedback, or, after CI completes, qualified or blocked selected-review evidence,
|
||||
an access/fetch gate,
|
||||
or an otherwise terminal PR state. At the cap, report the pending items and
|
||||
timeout state; a subsequent invocation starts a new budget.
|
||||
|
||||
## Inputs
|
||||
|
||||
The planner will tell you the PR number (or rely on `gh pr view` against the current branch). If neither is available, return a report with `error=...` and stop.
|
||||
@@ -38,11 +47,11 @@ ci_passed: <count or "unknown">
|
||||
ci_pending: <comma-separated names, "none", or "unknown">
|
||||
checks_evidence: checks_head_sha=<SHA|unknown> snapshot_complete=<true|false|unknown>
|
||||
bots:
|
||||
coderabbit: <done|rate_limited|pending|timeout|unknown> comments=<N or "unknown">
|
||||
greptile: <done|pending|timeout|unknown> reviews=<N or "unknown">
|
||||
claude: <done|pending|timeout|unknown> reviews=<N or "unknown"> path=<app|fork|none>
|
||||
opencode: <done|pending|timeout|unknown> comments=<N or "unknown">
|
||||
cubic: <done|pending|timeout|unknown> reviews=<N or "unknown">
|
||||
coderabbit: <done|rate_limited|not_configured|pending|timeout|unknown> comments=<N or "unknown">
|
||||
greptile: <done|not_configured|pending|timeout|unknown> reviews=<N or "unknown">
|
||||
claude: <done|not_configured|pending|timeout|unknown> reviews=<N or "unknown"> path=<app|fork|none>
|
||||
opencode: <done|not_configured|pending|timeout|unknown> comments=<N or "unknown">
|
||||
cubic: <done|not_configured|pending|timeout|unknown> reviews=<N or "unknown">
|
||||
unresolved_review_threads: <N or "unknown">
|
||||
actionable_issue_comments_from_bots: <N or "unknown">
|
||||
claude_summary: blockers=<N or "unknown"> suggestions=<N or "unknown"> verdict=<ready|ready_with_suggestions|blocked|unknown|none>
|
||||
@@ -115,7 +124,7 @@ such as a DNS timeout or GitHub API failure, use the fetch-failure recovery path
|
||||
```bash
|
||||
scripts/pr-state --trusted-reviewer "$selected_reviewer" <num>
|
||||
```
|
||||
Parse `.checks`, `.checks_head_sha`, `.checks_snapshot_complete`, `.review_threads`, `.unresolved_review_thread_count`, `.reviews`, `.issue_comments`, `.review_evidence`, and `.errors`. Derive `ci_failed`, `ci_pending`, bot terminal states, and whether the latest bot summaries are actionable in the poller from those raw arrays. The PR-view check snapshot is current-head complete only when `checks_snapshot_complete=true`; its `checks_head_sha` must equal both review evidence opening/closing heads before selected-review evidence can qualify. `.review_evidence` is exact-head evidence only when all three heads are known and identical; `current_head_sha` is the final observed head, while `evidence_head_sha` is present only for a stable combined collection. If check snapshot H1 and reviews are stable H2, emit `qualification=unknown` and retry/fall back; never pair their CI and review state. Only records whose `commit_id` equals `evidence_head_sha` qualify; timestamps never prove head coverage. `blocked_exact_current_head_reviews` aggregates every exact-head record with `eligibility=blocked`, regardless of author; old-head blockers are excluded. Use the record's machine-readable `eligibility` and `verdict`: approved is eligible only absent a blocker signal, commented is eligible only with `<!-- kandev-review: clean -->`, nonzero structured blockers, independently labeled Markdown `Blocker:`/`Blockers:` lines with descriptive values, or explicit blocked/changes-requested/action-required/must-fix evidence are blocked. `No blocker(s): ...`, `Blocker(s): 0`, and `| Blocker | 0 |` are nonblocking; later independent positive label lines still win. Dismissed/pending/unknown or ambiguous comments are never eligible. If `.errors` is non-empty, emit `unknown` for affected fields and explain the fetch failure in `recommendation:`. Do not backfill missing values from memory or from a generic CI template.
|
||||
Parse `.checks`, `.checks_head_sha`, `.checks_snapshot_complete`, `.passed_check_count`, `.review_threads`, `.unresolved_review_thread_count`, `.reviews`, `.issue_comments`, `.review_evidence`, and `.errors`. Emit `ci_passed` from the normalized `passed_check_count` only when the snapshot is complete; do not recount raw check rows. A bot is configured only when the PR has its corresponding check, status, review, or comment, or when it is explicitly selected; otherwise emit `not_configured` rather than `pending`. Derive failures and actionable summaries from the remaining raw arrays. The PR-view check snapshot is current-head complete only when `checks_snapshot_complete=true`; its `checks_head_sha` must equal both review evidence opening/closing heads before selected-review evidence can qualify. `.review_evidence` is exact-head evidence only when all three heads are known and identical; `current_head_sha` is the final observed head, while `evidence_head_sha` is present only for a stable combined collection. If check snapshot H1 and reviews are stable H2, emit `qualification=unknown` and retry/fall back; never pair their CI and review state. Only records whose `commit_id` equals `evidence_head_sha` qualify; timestamps never prove head coverage. `blocked_exact_current_head_reviews` aggregates every exact-head record with `eligibility=blocked`, regardless of author; old-head blockers are excluded. Use the record's machine-readable `eligibility` and `verdict`: approved is eligible only absent a blocker signal, commented is eligible only with `<!-- kandev-review: clean -->`, nonzero structured blockers, independently labeled Markdown `Blocker:`/`Blockers:` lines with descriptive values, or explicit blocked/changes-requested/action-required/must-fix evidence are blocked. `No blocker(s): ...`, `Blocker(s): 0`, and `| Blocker | 0 |` are nonblocking; later independent positive label lines still win. Dismissed/pending/unknown or ambiguous comments are never eligible. If `.errors` is non-empty, emit `unknown` for affected fields and explain the fetch failure in `recommendation:`. Do not backfill missing values from memory or from a generic CI template.
|
||||
|
||||
b. **Fallback raw CI status:**
|
||||
```bash
|
||||
@@ -170,7 +179,7 @@ such as a DNS timeout or GitHub API failure, use the fetch-failure recovery path
|
||||
|
||||
d. **Exit conditions:**
|
||||
- With `selected_reviewer`, use `scripts/pr-state --trusted-reviewer "$selected_reviewer" <PR>`. After all CI checks complete, exit when `checks_snapshot_complete=true`, all three evidence heads match, its exact-current-head record has `eligibility=eligible`, and active changes requests, actionable unresolved threads, actionable issue comments, and `blocked_exact_current_head_review_count` are all known zero (`qualification=qualified`). Only the dedicated OpenCode App `${OPENCODE_REVIEW_APP_SLUG}[bot]` additionally requires emitted `trusted_producer=true`; it means both the raw dedicated-App marker predicate and exact authenticated workflow/job success passed. For other explicitly selected reviewers, `trusted_producer` is not a qualification gate. Any missing/failed API state or a latest non-success workflow/job requires local frontier fallback only for the dedicated App path; ambiguous/ineligible selected evidence always requires fallback.
|
||||
- Without `selected_reviewer`, all CI checks completed AND every configured reviewer is in a terminal state (`done` / `rate_limited` / `timeout`) → exit loop.
|
||||
- Without `selected_reviewer`, all CI checks completed AND every configured reviewer is in a terminal state (`done` / `rate_limited` / `timeout`); absent integrations are terminal as `not_configured` → exit loop.
|
||||
- Round 40 reached (≈20 min) → mark any still-pending CI checks under `ci_pending:` and any still-pending bots as `timeout`, then exit loop.
|
||||
|
||||
4. **Fallback only: count unresolved review threads** via GraphQL (single call, not per-round). Skip this when `scripts/pr-state` already returned `.unresolved_review_thread_count` without a `review_threads` error:
|
||||
@@ -216,7 +225,7 @@ such as a DNS timeout or GitHub API failure, use the fetch-failure recovery path
|
||||
7. **Emit the report.** Fill in the shape above exactly. Emit `checks_evidence` from `checks_head_sha`/`checks_snapshot_complete`; when it is not complete, do not describe CI as current-head green. For `review_evidence`, use `reviewer=none qualification=unqualified reviewed_head_sha=<current head> review_state=none eligibility=ineligible verdict=none trusted_producer=unknown active_changes_requested=<N> blocked_exact_current_head_reviews=<N>` when no reviewer was selected. With a selected reviewer, report its latest exact-current-head record's `review_state`, `eligibility`, and `verdict`, and set `trusted_producer` from the canonical raw field. Emit `qualified` only when the PR-view check head plus review opening/closing heads are known, identical, and complete, that record is `eligible`, and active changes-request, actionable-thread, and blocked-exact-review counts are known zero; only the dedicated OpenCode App additionally requires `trusted_producer=true`. For non-OpenCode selected reviewers, neither `trusted_producer=false` nor `unknown` changes generic qualification. Emit `blocked` when any such count is nonzero or the selected record is `blocked`, `unqualified` for an ineligible/ambiguous/no selected record (or a dedicated App without `trusted_producer=true`), and `unknown` when evidence/head data is incomplete. A changed head invalidates every prior record. Use green wording only
|
||||
when the report is complete and every required value is known: `ci_failed`
|
||||
is known empty, `ci_pending` is `none`, every configured reviewer is done or
|
||||
`rate_limited`, mergeability and local conflict state are known clean, and
|
||||
`rate_limited` (and every absent integration is `not_configured`), 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
|
||||
|
||||
@@ -41,6 +41,21 @@ a new round: reassess prior findings and the verdict against the new head, and
|
||||
verify checks or workflow results for that head rather than relying on a PR
|
||||
number or author summary.
|
||||
|
||||
When the base branch advanced after the reviewed head's CI run, validate the
|
||||
actual merge result before declaring the PR ready. Record the latest base and
|
||||
immutable head SHAs, create a temporary worktree from that base, merge the head
|
||||
with `git merge --no-commit --no-ff <head-sha>`, and run focused verification in
|
||||
the merged tree before removing the worktree. GitHub's `mergeable: MERGEABLE`
|
||||
status proves conflict compatibility, not that the merged result was tested.
|
||||
|
||||
For an existing GitHub PR, inspect both `scripts/pr-state --summary <PR>` and
|
||||
`scripts/pr-resolve list <PR>` before treating review feedback as clean. Read
|
||||
the body of any exact-current-head review as well as inline threads: bots can
|
||||
place actionable findings outside the diff. A review's `commit_id` is the
|
||||
current-head signal; timestamps are only collection order. If `pr-state`
|
||||
reports hidden unresolved threads, use `pr-resolve list` to inspect them rather
|
||||
than assuming the filtered list is complete.
|
||||
|
||||
Compare the PR description, checklist, and claimed manual validation with that
|
||||
exact head, especially after a major refactor. Report stale claims separately;
|
||||
they are not verification evidence for the current diff.
|
||||
@@ -58,6 +73,8 @@ Before reviewing implementation details:
|
||||
- Identify missing coverage for happy path, key error paths, edge cases, auth/workspace boundaries, and concurrency/order-sensitive behavior.
|
||||
- For concurrent or event-driven changes, require a deterministic schedule that checks ownership or generation identity, stale-event handling, cancellation, and lock scope. Channel/barrier coordination is preferable to timing sleeps.
|
||||
- For stale-event races, cover both event-before-successor and delayed-old-event-after-successor orderings. Prefer integration coverage for cross-package event or callback paths when practical.
|
||||
- For terminal event streams, block an earlier publication, enqueue a terminal event (for example delete or cancellation), then enqueue a stale update. Assert no later mutation reaches an upserting consumer; queues must tombstone the entity or discard pending work at the terminal boundary.
|
||||
- When completion events lack a stable workload identity, test N outstanding registrations with N completion signals and duplicate delivery. A single-registration test cannot prove that uncorrelated completions retire work correctly. Compare this behavior with the accepted spec or ADR; a passing test that contradicts the contract is still a blocker.
|
||||
- Treat missing tests for new or changed non-UI logic as a blocker unless the change is explicitly untestable and says why.
|
||||
|
||||
### 3. Review for issues
|
||||
@@ -109,6 +126,14 @@ Check every changed file for the following layers. Skip layers that don't apply
|
||||
- Naming clear and consistent with project conventions
|
||||
- Deep nesting (>3 levels) — use early returns
|
||||
|
||||
**Build and platform boundaries:**
|
||||
- For changed Makefiles, shell scripts, or CI path filters, trace each changed
|
||||
target through the shell and platform branches. Distinguish executable naming
|
||||
from recipe-shell syntax; inspect `OS`, `MSYSTEM`, and `SHELL` assumptions.
|
||||
- Use `make -n <changed-target>` for every affected platform branch that is
|
||||
available, and confirm CI invokes the changed target. Include docs or
|
||||
configuration paths when a validator or test reads them.
|
||||
|
||||
**AI slop detection:**
|
||||
- Comments that restate code or narrate obvious steps
|
||||
- Unnecessary try/catch that swallow errors or return silent defaults in trusted internal paths
|
||||
@@ -118,9 +143,8 @@ Check every changed file for the following layers. Skip layers that don't apply
|
||||
|
||||
**Testing (blocker if missing):**
|
||||
- Backend (Go): new or changed functions/methods must have corresponding `*_test.go` tests
|
||||
- Frontend (JS/TS libs only): new utility functions, hooks, API clients, and store slices must have `*.test.ts` tests
|
||||
- We do NOT test React components — skip those
|
||||
- Exceptions: config files, generated code, React component markup
|
||||
- Frontend: new utilities, hooks, API clients, and store slices must have focused tests. Pure React markup may skip a unit test, but behavior-bearing components (conditional status, accessibility, store-derived state, or responsive/mobile variants) need focused `*.test.tsx` coverage and/or E2E. Route responsive user-facing changes through `/mobile-parity`.
|
||||
- Exceptions: config files, generated code, and pure React component markup
|
||||
- Missing tests for new or changed logic is a **blocker** — suggest what tests to add and recommend `/tdd`
|
||||
|
||||
### 4. Report
|
||||
|
||||
@@ -137,6 +137,14 @@ Without this, tests run against stale code and failures are misleading. `make bu
|
||||
4. Use page objects for common interactions; create new ones for new pages
|
||||
5. For GitHub features, use `apiClient.mockGitHub*()` methods to seed mock data
|
||||
|
||||
### Input-modality behavior
|
||||
|
||||
For a touch-specific interaction, use Playwright `.tap()` rather than `.click()`
|
||||
so the app receives a touch `pointerType`. Run focused mobile specs with
|
||||
`-- --project=mobile-chrome`; otherwise Playwright can report no matching tests.
|
||||
After the interaction settles, assert the resulting state and exercise a later
|
||||
mouse or pen entry when the UI maintains hybrid-device pointer state.
|
||||
|
||||
### Visual alignment regressions
|
||||
|
||||
For a UI change whose contract is a rendered size or alignment relationship,
|
||||
|
||||
@@ -57,6 +57,11 @@ manage Kandev platform entities and may be used only when the user explicitly
|
||||
asks to create or manage Kandev tasks or sessions. They are not a delegation
|
||||
fallback. If native harness delegation is unavailable, stop and report it.
|
||||
|
||||
In Codex, a registered `agent_type` is a role override. Spawn it with
|
||||
`fork_turns: "none"` or a bounded recent-turn count, never `fork_turns: "all"`.
|
||||
Put the complete work packet and the paths to read in the initial prompt so the
|
||||
worker has the context that an all-history fork would otherwise provide.
|
||||
|
||||
## Worker Contract
|
||||
|
||||
A worker executes exactly one bounded assignment. It follows the assigned
|
||||
|
||||
@@ -94,8 +94,10 @@ above overrides this rule. The planner does not inspect or resolve conflicts dir
|
||||
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`.
|
||||
Only `done` and `rate_limited` are clean; surface `timeout`, and treat
|
||||
- `bots.<name>` — `done` / `rate_limited` / `not_configured` / `pending` / `timeout` / `unknown`.
|
||||
`not_configured` means the PR exposes no corresponding integration signal and
|
||||
is clean unless that reviewer was explicitly selected. Only `done`,
|
||||
`rate_limited`, and `not_configured` are clean; surface `timeout`, and treat
|
||||
`pending` or `unknown` as incomplete state.
|
||||
- `review_evidence` — contains `reviewer`, `qualification`, `reviewed_head_sha`,
|
||||
`review_state`, `eligibility`, `verdict`, `active_changes_requested`, and
|
||||
@@ -116,7 +118,8 @@ above overrides this rule. The planner does not inspect or resolve conflicts dir
|
||||
- `unresolved_review_threads` and `actionable_issue_comments_from_bots` — drive steps 3-4.
|
||||
Skip to step 5 only when both are known `0`, `ci_failed` is explicitly known
|
||||
empty, `ci_pending` is `none`, the merge-conflict gate is known clean, and
|
||||
every configured reviewer is done or `rate_limited` (or an explicitly selected reviewer is
|
||||
every configured reviewer is done or `rate_limited`, with absent integrations
|
||||
reported as `not_configured` (or an explicitly selected reviewer is
|
||||
`qualified` with no blockers; still run verify + push if you have fixes from
|
||||
earlier).
|
||||
|
||||
|
||||
@@ -91,7 +91,19 @@ explicitly requests task tracking.
|
||||
- Screenshots must use synthetic or redacted data. Reject any asset that
|
||||
exposes secrets, authentication tokens, or personally identifiable
|
||||
information, and stop with a recapture request.
|
||||
- Compress PNGs before embedding: `pngquant --quality 65-90 --ext .png --force apps/web/.pr-assets/*.png`.
|
||||
- Compress PNGs before embedding. Prefer a system `pngquant`; when it is
|
||||
unavailable, use the supported ephemeral fallback rather than skipping
|
||||
compression:
|
||||
```bash
|
||||
if command -v pngquant >/dev/null 2>&1; then
|
||||
pngquant --quality 65-90 --ext .png --force apps/web/.pr-assets/*.png
|
||||
else
|
||||
(
|
||||
cd apps
|
||||
pnpm dlx pngquant-bin@9.0.0 --quality 65-90 --ext .png --force web/.pr-assets/*.png
|
||||
)
|
||||
fi
|
||||
```
|
||||
|
||||
**Embed (GitHub only — image binaries must never merge into `main`):** GitHub has no public API to upload images into a PR body (drag-and-drop is web-UI only), so publish the images on an orphan commit that can never be merged and reference them with SHA-pinned raw URLs:
|
||||
```bash
|
||||
@@ -112,9 +124,21 @@ explicitly requests task tracking.
|
||||
```
|
||||
`gh pr edit` fails on this repo (GraphQL touches the deprecated Projects-classic API). Fall back to REST — build the payload with `jq --rawfile`, never by hand-escaping shell strings:
|
||||
```bash
|
||||
jq -n --rawfile body "<body-file>" '{body: $body}' > /tmp/pr-body-payload.json
|
||||
if command -v rtk >/dev/null 2>&1; then
|
||||
rtk proxy jq -n --rawfile body "<body-file>" '{body: $body}' > /tmp/pr-body-payload.json
|
||||
rtk proxy jq empty /tmp/pr-body-payload.json
|
||||
else
|
||||
jq -n --rawfile body "<body-file>" '{body: $body}' > /tmp/pr-body-payload.json
|
||||
jq empty /tmp/pr-body-payload.json
|
||||
fi
|
||||
gh api --method PATCH repos/:owner/:repo/pulls/<PR_NUMBER> --input /tmp/pr-body-payload.json
|
||||
```
|
||||
**RTK and JSON payloads:** RTK is optional. If it is installed, it
|
||||
summarizes normal stdout, so it must not sit between a JSON producer and a
|
||||
redirected file or another parser. The conditional recipe above keeps the
|
||||
REST fallback byte-preserving whether or not RTK is installed.
|
||||
The same rule applies to command substitutions, `xargs`, and any other
|
||||
consumer that expects unmodified Git or JSON output.
|
||||
|
||||
**Preserve the existing PR description:** The PR body is a shared, mutable
|
||||
document. Preview automation and other bots may add sections after the PR
|
||||
|
||||
@@ -47,7 +47,10 @@ Push the already committed branch to its remote.
|
||||
```bash
|
||||
git push
|
||||
```
|
||||
If the branch has no upstream, use `git push -u origin <branch>`.
|
||||
If the branch has no upstream, use `git push -u origin HEAD` rather than
|
||||
transcribing the branch name. Then verify
|
||||
`git rev-parse HEAD` equals `git rev-parse '@{upstream}'`, and report the
|
||||
branch from `git branch --show-current`.
|
||||
If the branch was rebased or history was rewritten, first confirm the current
|
||||
branch is not `main` or `master`, then use `git push --force-with-lease`.
|
||||
If the branch modifies `.github/workflows/*` and GitHub rejects the push with
|
||||
|
||||
@@ -31,6 +31,20 @@ Entirely in CI via `.github/workflows/release.yml`, triggered by a maintainer fr
|
||||
|
||||
There is no local release script — the entire flow runs in GHA.
|
||||
|
||||
## Release-tag signing configuration
|
||||
|
||||
The release workflow reads signing configuration from the GitHub `release`
|
||||
environment. `RELEASE_GPG_PRIVATE_KEY` and the optional
|
||||
`RELEASE_GPG_PASSPHRASE` are environment secrets. The full 40-character
|
||||
`RELEASE_GPG_FINGERPRINT` is an environment variable, not a secret: the
|
||||
workflow reads `vars.RELEASE_GPG_FINGERPRINT`, so storing it as a secret makes
|
||||
normal-release preflight treat it as missing.
|
||||
|
||||
`.github/release-signing-key.asc` must contain exactly one public primary key
|
||||
whose fingerprint matches that variable; never commit private key material.
|
||||
`backfill_tag` repairs publication for an already-signed existing tag only and
|
||||
does not bypass the normal-release signing checks.
|
||||
|
||||
Desktop signing is automatic. Complete macOS/Windows signing and notarization secrets produce signed artifacts; missing or incomplete signing inputs produce unsigned desktop artifacts and the GitHub release notes get an unsigned-artifact warning. `desktop_validation_only=true` builds artifacts from the current workflow ref for maintainer inspection and skips the release PR, tag, GitHub release, npm publish, Homebrew update, and public container tags.
|
||||
|
||||
## Runtime resolution
|
||||
|
||||
@@ -22,6 +22,15 @@ larger one, then launches a fresh `verify` assignment.
|
||||
|
||||
Invoke the `verify` worker in a single call. Wait for it to complete and surface the result.
|
||||
|
||||
Only one verification worker may run for an artifact at a time. Do not launch
|
||||
a replacement merely because the first worker has not yet returned: overlapping
|
||||
full suites contend for the same checkout and can make both results unreliable.
|
||||
If it appears stalled, use the harness's normal worker-status or interruption
|
||||
mechanism first; start a replacement only after the prior worker is confirmed
|
||||
stopped (and its command/process group is gone when that evidence is exposed).
|
||||
If that cannot be confirmed, report verification as blocked rather than
|
||||
proceeding to push or PR delivery.
|
||||
|
||||
- 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.
|
||||
|
||||
@@ -6,6 +6,22 @@ sandbox_mode = "read-only"
|
||||
developer_instructions = """
|
||||
Pure polling role. Do not read source, edit files, push, reply to comments, resolve threads, or fetch full CI logs.
|
||||
|
||||
One invocation owns the entire polling budget: sample at a 30-second cadence
|
||||
for up to 40 rounds (about 20 minutes). Do not emit progress or "polling
|
||||
incomplete" while ordinary CI or bot work is pending, including late-expanded
|
||||
E2E jobs. Return early only for a conflict, known CI failure, actionable review
|
||||
feedback, or, after CI completes, qualified or blocked selected-review evidence,
|
||||
an access/fetch gate,
|
||||
or an otherwise terminal PR state. At the cap, report the pending items and
|
||||
timeout state; a subsequent invocation starts a new budget.
|
||||
|
||||
Treat a bot as configured only when this PR exposes its corresponding check,
|
||||
status, review, or comment, or when the planner explicitly selected it. Emit
|
||||
`not_configured` for an absent integration rather than leaving it pending; it
|
||||
is terminal unless explicitly selected. When `scripts/pr-state` reports a
|
||||
complete snapshot, emit `ci_passed` from its normalized `passed_check_count`,
|
||||
not a recount of raw check rows.
|
||||
|
||||
Before the first GitHub-dependent helper or gh call, check the inherited runtime policy. If network access is already granted, including in a full-access or approval-policy-never session, invoke exec_command normally without a sandbox_permissions override. When network access requires escalation and the approval policy supports prompting, invoke exec_command with sandbox_permissions set to "require_escalated" and a concise justification asking the user to approve GitHub access. Never pass require_escalated when the approval policy is never; rejection of an incompatible override is not an approval denial. Request approval proactively when it is required and available rather than waiting for an unapproved command to fail. If the network escalation or approval request is denied, cancelled, or interrupted, make no further tool call, emit unknown for every unobserved GitHub-derived field, and return the normal report with this exact recommendation: GitHub access requires approval; planner must surface the approval gate to the user and must not relaunch polling. This is a terminal user gate, not a fetch failure. Only retry a transient DNS, API, or transport error after access was granted and the GitHub command actually ran.
|
||||
|
||||
The planner may supply selected_reviewer=<GitHub login>. Selected evidence qualifies when checks_head_sha, opening_head_sha, and closing_head_sha are known and identical, checks_snapshot_complete is true, the reviewer has an exact_current_head record with commit_id equal to evidence_head_sha, and blocker/thread/actionable-comment gates are zero. Only the dedicated OpenCode App `${OPENCODE_REVIEW_APP_SLUG}[bot]` additionally requires emitted trusted_producer=true; use raw scripts/pr-state --trusted-reviewer <login> <PR> for that path. Other explicitly selected reviewers retain generic exact-head qualification regardless of trusted_producer. Displayed check names never authenticate the App path. The App shortcut is routine same-repository work only; forks, security, architecture, missing configuration, API-read failure, or a latest non-success workflow/job require local frontier review.
|
||||
|
||||
@@ -10,6 +10,22 @@ report including head SHA, GitHub mergeability/merge-state status, and local
|
||||
unmerged-index count. Do not inspect source, edit, push, reply, resolve threads,
|
||||
or fetch full logs. Do not spawn subagents.
|
||||
|
||||
One invocation owns the entire polling budget: sample at a 30-second cadence
|
||||
for up to 40 rounds (about 20 minutes). Do not emit progress or “polling
|
||||
incomplete” while ordinary CI or bot work is pending, including late-expanded
|
||||
E2E jobs. Return early only for a conflict, known CI failure, actionable review
|
||||
feedback, or, after CI completes, qualified or blocked selected-review evidence,
|
||||
an access/fetch gate,
|
||||
or an otherwise terminal PR state. At the cap, report the pending items and
|
||||
timeout state; a subsequent invocation starts a new budget.
|
||||
|
||||
Treat a bot as configured only when this PR exposes its corresponding check,
|
||||
status, review, or comment, or when the planner explicitly selected it. Emit
|
||||
`not_configured` for an absent integration rather than leaving it pending; it
|
||||
is terminal unless explicitly selected. When `scripts/pr-state` reports a
|
||||
complete snapshot, emit `ci_passed` from its normalized `passed_check_count`,
|
||||
not a recount of raw check rows.
|
||||
|
||||
The planner may provide `selected_reviewer=<GitHub login>`. Read raw
|
||||
`scripts/pr-state --trusted-reviewer "$selected_reviewer" <PR>` when selection is set: review evidence is qualified
|
||||
only when `checks_head_sha` plus opening/closing heads match and
|
||||
|
||||
@@ -26,8 +26,22 @@ failure eligible for retry.
|
||||
|
||||
Prefer `scripts/pr-state --summary <PR>` and `scripts/pr-resolve list <PR>`.
|
||||
Include head SHA, GitHub mergeability/merge-state status, and the local unmerged
|
||||
index count from `git ls-files -u`. Use one-shot checks or bounded commands.
|
||||
Report only observed values and return one compact report block.
|
||||
index count from `git ls-files -u`. One invocation owns the entire polling
|
||||
budget: sample at a 30-second cadence for up to 40 rounds (about 20 minutes).
|
||||
Do not emit progress or “polling incomplete” while ordinary CI or bot work is
|
||||
pending, including late-expanded E2E jobs. Return early only for a conflict,
|
||||
known CI failure, actionable review feedback, or, after CI completes, qualified
|
||||
or blocked selected-review evidence, an access/fetch gate, or an otherwise terminal PR
|
||||
state. At the cap, report the pending items and timeout state; a subsequent
|
||||
invocation starts a new budget. Report only observed values in one compact
|
||||
report block.
|
||||
|
||||
Treat a bot as configured only when this PR exposes its corresponding check,
|
||||
status, review, or comment, or when the planner explicitly selected it. Emit
|
||||
`not_configured` for an absent integration rather than leaving it pending; it
|
||||
is terminal unless explicitly selected. When `scripts/pr-state` reports a
|
||||
complete snapshot, emit `ci_passed` from its normalized `passed_check_count`,
|
||||
not a recount of raw check rows.
|
||||
|
||||
The planner may provide `selected_reviewer=<GitHub login>`. For selection,
|
||||
read raw `scripts/pr-state --trusted-reviewer "$selected_reviewer" <PR>`: only known/complete records with matching
|
||||
|
||||
@@ -31,6 +31,8 @@
|
||||
# - since: { head_oid, committed_at } or null
|
||||
# - failed_checks: [{ name, workflow, status, conclusion, run_id, job_id, details_url, target_url }]
|
||||
# - pending_checks: [{ name, workflow, status, run_id, job_id, details_url, target_url }]
|
||||
# - successful_checks: [{ name, workflow, status, conclusion, run_id, job_id, details_url, target_url }]
|
||||
# - passed_check_count: <N>
|
||||
# - checks_head_sha: <SHA or null>
|
||||
# - checks_snapshot_complete: <boolean>
|
||||
# - unresolved_review_thread_count: <N>
|
||||
@@ -483,6 +485,10 @@ summarize_state_json() {
|
||||
and .status != "failure"
|
||||
and .status != "error";
|
||||
|
||||
def successful_check:
|
||||
(.status == "completed" and .conclusion == "success")
|
||||
or (.status == "success" and (.conclusion == null or .conclusion == "success"));
|
||||
|
||||
{
|
||||
pr,
|
||||
since,
|
||||
@@ -498,6 +504,12 @@ summarize_state_json() {
|
||||
| select(pending_status and .conclusion == null)
|
||||
| {name, workflow, status, run_id, job_id, details_url, target_url}
|
||||
],
|
||||
successful_checks: [
|
||||
effective_checks[]
|
||||
| select(successful_check)
|
||||
| {name, workflow, status, conclusion, run_id, job_id, details_url, target_url}
|
||||
],
|
||||
passed_check_count: ([effective_checks[] | select(successful_check)] | length),
|
||||
unresolved_review_thread_count,
|
||||
filtered_review_thread_count,
|
||||
review_evidence,
|
||||
|
||||
@@ -1161,6 +1161,7 @@ test_summary_mode_returns_compact_fixup_state() {
|
||||
assert_jq "summary pending check count" '.pending_checks | length == 2' "$json"
|
||||
assert_jq "summary pending check" '.pending_checks[0] | .name == "claude-review" and .status == "in_progress" and .run_id == "27340000003"' "$json"
|
||||
assert_jq "summary pending status context keeps target url" '.pending_checks[] | select(.name == "external pending") | .status == "pending" and .details_url == null and .target_url == "https://ci.example.test/build/1"' "$json"
|
||||
assert_jq "summary exposes normalized passing checks" '(.passed_check_count == (.successful_checks | length)) and any(.successful_checks[]; .name == "web lint" and .conclusion == "success") and any(.successful_checks[]; .name == "opencode-review-same-repo" and .conclusion == "success")' "$json"
|
||||
assert_jq "summary unresolved count" '.unresolved_review_thread_count == 2' "$json"
|
||||
assert_jq "summary filtered thread count" '.filtered_review_thread_count == 1' "$json"
|
||||
assert_jq "summary unresolved threads" '.unresolved_threads | length == 1' "$json"
|
||||
|
||||
Reference in New Issue
Block a user