chore: improve harness feedback workflow (#1561)
This commit is contained in:
Carlos Florêncio
@@ -44,6 +44,10 @@ Use rules for always-on or path-scoped behavior, not long workflows. Dynamic rul
|
||||
|
||||
OpenCode uses `AGENTS.md` for project rules. It also supports Claude-compatible fallbacks such as `CLAUDE.md` when no `AGENTS.md` exists.
|
||||
|
||||
## Automation Coverage
|
||||
|
||||
When creating or updating automation that classifies harness artifacts, derive the allowlist from the platform references and include platform-native instruction/config roots, not only the repo's current source-of-truth roots. Include the existing `.agents`, `.augment`, `.claude`, `.codex`, and `.opencode` agent/skill roots plus platform-native surfaces such as `.claude/settings.json`, `.claude/rules/**/*.md`, `.claude/commands/**/*.md`, `.cursor/rules/**/*.mdc`, `.cursor/skills/**/SKILL.md`, and `.opencode/skills/**/SKILL.md`.
|
||||
|
||||
## Validation
|
||||
|
||||
After instruction edits:
|
||||
|
||||
@@ -60,6 +60,8 @@ Every UI feature should end with one of these:
|
||||
- existing mobile Playwright test explicitly identified as covering the changed behavior
|
||||
- written justification for no mobile test, limited to impossible-to-test infrastructure gaps
|
||||
|
||||
For frontend changes that are purely state/data normalization inside an existing component and do not alter rendered layout, touch behavior, scrolling, navigation, or viewport-dependent interaction, targeted unit/component tests plus an explicit note can satisfy mobile parity. New mobile Playwright coverage is not required for that narrow case.
|
||||
|
||||
Good mobile tests assert real user outcomes, not only visibility. Prefer:
|
||||
|
||||
- open feature from mobile navigation and complete primary action
|
||||
|
||||
@@ -61,7 +61,7 @@ If available, invoke the `pr-poller` subagent with the PR number (or let it reso
|
||||
- `bots.<name>` — `done` / `rate_limited` / `pending` / `timeout`. Anything in `done` or `rate_limited` has had its chance; treat the rest as missing data, not a blocker.
|
||||
- `unresolved_review_threads` and `issue_comments_from_bots` — drive steps 3-4. If both are 0 and `ci_failed` is empty, skip to step 5 (still run verify + push if you have fixes from earlier).
|
||||
|
||||
**E2E CI outlasts the poller.** The pr-poller caps at ~20 minutes. The E2E matrix (10 shards × 2 projects) often runs longer. GitHub can expand E2E matrix jobs late; if pending checks briefly drop near zero and then jump when E2E shards appear, keep treating that as normal pending CI unless a shard reports failure. If the report shows `ci_pending` with only E2E/lint jobs and `ci_failed` is empty, re-invoke pr-poller once those jobs finish — do not spin a manual `gh pr checks` loop in the parent. If the cap hits with E2E still pending, report "CI in progress" to the user instead of blocking, and include the exact pending shard names from `ci_pending`.
|
||||
**E2E CI outlasts the poller.** The pr-poller caps at ~20 minutes. Standard E2E shards plus container shards often run longer. GitHub can expand E2E matrix jobs late, and the shard matrix may appear only after the build job finishes; if pending checks briefly drop near zero and then jump when E2E shards appear, keep treating that as normal pending CI unless a shard reports failure. If the report shows `ci_pending` with only E2E/lint jobs and `ci_failed` is empty, re-invoke pr-poller once those jobs finish — do not spin a manual `gh pr checks` loop in the parent. If the cap hits with E2E still pending, report "CI in progress" to the user instead of blocking, and include the exact pending shard names from `ci_pending`.
|
||||
|
||||
**Do not fetch poll output yourself** — that is what burns context. The report is the only thing that enters your context.
|
||||
|
||||
@@ -86,13 +86,15 @@ The summary output contains:
|
||||
- `unresolved_review_thread_count` — total unresolved thread count on the PR, including older unresolved threads outside the current-head filter
|
||||
- `filtered_review_thread_count` — informational count for the current-head-filtered review-thread view; it can include resolved or historical comments, so do not treat it as a blocker by itself
|
||||
- `unresolved_threads` — compact current-head inline review state to triage in this fixup pass
|
||||
- `hidden_unresolved_threads` — compact unresolved threads outside the current-head filter when historical unresolved threads exist
|
||||
- `hidden_unresolved_threads` — compact unresolved threads outside the current-head filter when historical unresolved threads exist; treat these as actionable blockers, not ignorable history
|
||||
- `errors` — data-gathering failures; treat affected fields as unknown instead of reconstructing them ad hoc
|
||||
|
||||
If `scripts/pr-state --summary <PR>` briefly returns `branch:"unknown"` or reports that `gh pr view` failed while `gh pr view <PR>` works directly, treat it as a transient state-resolution issue. Re-run the explicit PR-number command once and fall back to direct `gh pr view <PR>` / targeted `gh run view` checks for that pass instead of assuming the PR state is unavailable.
|
||||
|
||||
If `scripts/pr-state --summary <PR>` returns `since: null` with `errors` like `repo` or `since`, but still includes valid `failed_checks`, `pending_checks`, and `unresolved_review_thread_count`, use the check/thread data for that poll and rerun once on the next cadence. Do not discard usable CI state just because head-commit metadata lookup failed.
|
||||
|
||||
If `scripts/pr-state --summary <PR>` returns valid `failed_checks` and `pending_checks` but `unresolved_review_thread_count: null` with an error such as `{"source":"review_threads","message":"gh api graphql reviewThreads failed"}`, use the check data for that poll but treat review-thread state as unknown. Do not report review as clean or blocked from that snapshot; rerun `scripts/pr-state --summary <PR>` on the next cadence to recover thread state. Before a final status, retry `scripts/pr-state --summary <PR>` once; if review thread state still errors, run `scripts/pr-resolve list <PR>` directly. Only report "no unresolved review threads" after one of those succeeds.
|
||||
|
||||
If `scripts/pr-state --summary <PR>` fails with `jq: Argument list too long`, do not debug the summary script during fixup. Split the fallback checks:
|
||||
|
||||
```bash
|
||||
@@ -100,9 +102,17 @@ gh pr checks <PR>
|
||||
scripts/pr-resolve list <PR>
|
||||
```
|
||||
|
||||
`gh pr checks` gives CI status only; `scripts/pr-resolve list` is still required for unresolved inline review threads.
|
||||
If the checks table is long or mixed with request diagnostics, keep only
|
||||
actionable rows:
|
||||
|
||||
If `unresolved_review_thread_count` is nonzero but `unresolved_threads` is empty, run `scripts/pr-resolve list <PR>` before acting. `pr-state` can briefly report total unresolved count from historical state while current-head unresolved threads are empty; `pr-resolve list` is the authoritative actionable thread set for fixup work. If `scripts/pr-state --summary` reports a nonzero unresolved count but `scripts/pr-resolve show <PR> <THREAD_ID>` says the listed thread is `resolved: true`, treat the summary as stale state. Wait briefly, rerun `scripts/pr-state --summary <PR>`, and do not reply again to an already-resolved thread.
|
||||
```bash
|
||||
gh pr checks <PR> 2>/dev/null | awk -F '\t' '$2 == "pending" || $2 == "fail" {print}'
|
||||
scripts/pr-resolve list <PR>
|
||||
```
|
||||
|
||||
`gh pr checks` gives CI status only; `scripts/pr-resolve list` is still required for unresolved inline review threads. `gh pr checks` can exit nonzero when checks are pending or failed (for example exit code 8) while still printing usable status rows. Treat its table output as valid state, not automatically as a shell/tool failure; parse the rows for `fail`, `pending`, and `pass`, and avoid `set -e` flows unless you intentionally capture or ignore the exit code.
|
||||
|
||||
If `unresolved_review_thread_count` is nonzero but `unresolved_threads` is empty, or if `hidden_unresolved_threads` is non-empty, run `scripts/pr-resolve list <PR>` before acting. Fetch each full body with `scripts/pr-state --comment <comment_id>` or `scripts/pr-resolve show <PR> <THREAD_ID_OR_COMMENT_ID>`; hidden threads can contain valid current blockers even when the current-head filtered view is empty. `pr-state` can briefly report total unresolved count from historical state while current-head unresolved threads are empty; `pr-resolve list` is the authoritative actionable thread set for fixup work. If `scripts/pr-state --summary` reports a nonzero unresolved count but `scripts/pr-resolve show <PR> <THREAD_ID>` says the listed thread is `resolved: true`, treat the summary as stale state. Wait briefly, rerun `scripts/pr-state --summary <PR>`, and do not reply again to an already-resolved thread.
|
||||
|
||||
Poll at a 30s cadence with a **20 min cap**. Prefer one-shot `scripts/pr-state --summary <PR>` checks, or a bounded command that can finish naturally, over long inline shell loops. In this environment, a running non-TTY loop may not receive Ctrl-C through `write_stdin`, so avoid `while sleep ...; do ...; done` polling in the main session. To wait one cadence without accidentally immediate-polling, use:
|
||||
|
||||
@@ -112,6 +122,19 @@ bash -lc 'sleep 30; scripts/pr-state --summary <PR>'
|
||||
|
||||
Stop early if any required check fails. If the cap hits and only E2E shards are still pending with no failures or unresolved comments, report "CI in progress" instead of continuing to watch indefinitely, and include the exact pending shard names from `pending_checks`. If the user explicitly asks to poll until CI is green, continue past the normal pending-E2E stopping point with bounded one-shot `sleep N; scripts/pr-state --summary <PR>` checks; stop immediately on any `failed_checks`, and continue until `failed_checks: []`, `pending_checks: []`, and `unresolved_review_thread_count: 0`. Do not run `gh pr checks --watch` in the main session unless the runtime can keep the watcher isolated and automatically clean it up. If you do use `gh pr checks --watch`, keep watching until the command exits; GitHub can expand matrix jobs after an initial aggregate "Build" check passes, so the first green build/lint/test rows are not necessarily terminal.
|
||||
|
||||
For E2E-only pending phases, summarize counts before dumping long shard lists into context:
|
||||
|
||||
```bash
|
||||
scripts/pr-state --summary <PR> > /tmp/prstate-<PR>.json
|
||||
jq '{failed_checks, pending_count:(.pending_checks|length), pending_by_workflow:(.pending_checks|group_by(.workflow)|map({workflow:.[0].workflow,count:length,statuses:(map(.status)|unique)})), unresolved_review_thread_count, errors}' /tmp/prstate-<PR>.json
|
||||
```
|
||||
|
||||
When stopping at the cap, print the exact pending names from the saved snapshot:
|
||||
|
||||
```bash
|
||||
jq -r '.pending_checks[] | "\(.status) | \(.name)"' /tmp/prstate-<PR>.json
|
||||
```
|
||||
|
||||
If a user interrupts a long manual poll loop (`sleep`, `gh pr checks`, or `scripts/pr-state`), check for leftover polling processes before switching tasks and terminate only the processes you started.
|
||||
|
||||
Use raw mode only when debugging an odd GitHub state:
|
||||
@@ -130,6 +153,16 @@ Mark task 2 as in_progress.
|
||||
|
||||
Prefer `scripts/pr-state <PR>` for this cross-check before falling back to raw `gh` output. It already gives you raw checks with normalized names plus extracted `run_id`s in one JSON snapshot.
|
||||
|
||||
When local verification passes but CI fails, reproduce the exact command from the failed job log before changing code. Backend lint can run stricter CI-only modes such as:
|
||||
|
||||
```bash
|
||||
golangci-lint run ./... --new-from-rev="${BASE_SHA}" --timeout=5m
|
||||
```
|
||||
|
||||
Do not assume plain `make lint` covers that failure; `--new-from-rev` can surface new-code-only lint failures such as complexity or cyclomatic threshold issues. After fixing backend lint, rerun the exact CI-shaped command from `apps/backend` before pushing.
|
||||
|
||||
If `Run Backend Tests` fails and the visible error is a report step such as `Generate test report` failing with `ENOENT: no such file or directory, open 'apps/backend/test-results.json'`, treat that as a downstream symptom. Search earlier in the saved CI log for `Run golangci-lint`, `.go:<line>:<col>:`, `1 issues:`, or `Process completed with exit code 1`, then reproduce with the CI-shaped `golangci-lint ... --new-from-rev=<base> --timeout=5m` command instead of debugging the missing report file.
|
||||
|
||||
For each entry in the report's `ci_failed:` list:
|
||||
|
||||
1. Use the `run_id` from the report (the poller already extracted these — don't re-run `gh pr checks`).
|
||||
@@ -142,84 +175,19 @@ For each entry in the report's `ci_failed:` list:
|
||||
```bash
|
||||
job_id="<failed job id from scripts/pr-state or gh run view --json jobs>"
|
||||
gh api repos/:owner/:repo/actions/jobs/"$job_id"/logs > /tmp/kandev-job.log
|
||||
rg -n "(Error:|FAIL|Failed|Timed out|Timeout|\\.spec\\.ts)" /tmp/kandev-job.log
|
||||
rg -n "(Error:|FAIL|Failed|Timed out|Timeout|\\.spec\\.ts|ghcr\\.io|docker/login-action)" /tmp/kandev-job.log
|
||||
```
|
||||
Then inspect targeted line ranges or downloaded artifacts rather than streaming the full log into context.
|
||||
The job-log endpoint may return plain text rather than a zip; inspect the saved file directly before assuming it needs `unzip`. Then inspect targeted line ranges or downloaded artifacts rather than streaming the full log into context.
|
||||
If fetching logs fails with a transient GitHub or DNS error such as `lookup api.github.com ... i/o timeout` or `error connecting to api.github.com`, wait one short cadence and retry once before treating the logs as unavailable or debugging from stale local output.
|
||||
If `gh run view --log-failed` says logs are unavailable because the workflow is still running, wait for the workflow/report job to finish before retrying.
|
||||
3. Read the relevant source files at the failing lines (use `Read` with `offset`/`limit`, not `cat`)
|
||||
4. Fix the issues (lint errors, test failures, type errors, etc.)
|
||||
|
||||
**If the failure looks unfamiliar or the cause isn't obvious from the log, check CI history on the branch before diving into the code:**
|
||||
|
||||
```bash
|
||||
gh run list --branch <branch> --workflow "<workflow name>" --limit 10 --json conclusion,headSha,createdAt,databaseId
|
||||
```
|
||||
|
||||
On long-lived PRs that get rebased/squashed, prior SHAs on the same branch often passed the same workflow. A `passing → failing` boundary tells you the regression is isolated to the most recent rework — diff against the last passing SHA (`git diff <last-passing-sha>..HEAD`) instead of against `main` to narrow the search dramatically.
|
||||
|
||||
**Recognize a cancelled concurrency-duplicate before reading any logs.** A required check with `conclusion=cancelled` — often annotated *"Canceling since a higher priority waiting request … exists"*, with 0s job durations and **unexpanded** `${{ matrix.* }}` job names (a "Merge reports" job may exit 1 for missing blobs) — is a concurrency-group artifact from a superseded run, **not a real failure**. GitHub re-triggers PR workflows when the base branch moves (e.g. a release lands on main), cancelling the in-flight run. Confirm the *non-cancelled* run for the **same head SHA** passed (`gh run list --workflow "<name>" --json headSha,conclusion,databaseId`), then trigger a single clean run (rebase onto main + force-push, or `gh run rerun <id>`) instead of debugging code.
|
||||
|
||||
**Recognize transient GitHub/API failures in semantic PR title checks.** If the
|
||||
`pr-title` / `amannn/action-semantic-pull-request` check fails with transport
|
||||
or response parsing errors such as `invalid json response body ... Unexpected
|
||||
end of JSON input`, treat it as infrastructure, not a title failure. Confirm
|
||||
the PR title is still valid Conventional Commits, rerun the failed job once,
|
||||
then re-check:
|
||||
|
||||
```bash
|
||||
gh run rerun <run-id> --failed
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
**Recognize E2E container setup failures before debugging tests.** If an E2E
|
||||
Containers shard fails during `Install Playwright system deps` before tests run,
|
||||
check the job log for apt repository failures. A failure like
|
||||
`packages.microsoft.com ... 403 Forbidden` from `playwright install-deps` is
|
||||
infrastructure/package-repo related, not an app or test failure. Record it as
|
||||
infra or rerun the failed job after code-related checks are clean.
|
||||
|
||||
**E2E test failures require special handling:**
|
||||
|
||||
If any failing check is an E2E test (Playwright):
|
||||
|
||||
1. Read the `/e2e` skill (`SKILL.md`) for debugging guidance, test patterns, and run commands
|
||||
2. Follow the "Debugging failures" section — read error output, check failure screenshots in `e2e/test-results/`, classify the failure (test logic, frontend, backend)
|
||||
3. For failed shards, identify the exact failing spec/test from logs before making changes.
|
||||
4. Fix the root cause. **Never increase timeouts to fix flaky tests** — find the real issue
|
||||
5. Confirm fixes pass locally before pushing. If CI logs name a specific failed test, run that exact spec/test first before any full shard run; shard-level runs can fail on unrelated existing flakes and obscure whether the reported PR failure reproduces. This still applies when the failure is a test encoding old intended behavior: run the exact failing test locally before changing the expectation, then rerun the exact test after updating it. If it still fails, treat that as a product/backend regression signal, not just a stale test. Build required artifacts first if global setup needs them, then run the targeted Playwright command. Wrap with `scripts/run-quiet`:
|
||||
```bash
|
||||
scripts/run-quiet build -- make build-backend build-web
|
||||
scripts/run-quiet e2e -- bash -c 'cd apps && pnpm --filter @kandev/web e2e -- tests/path/to/failing.spec.ts'
|
||||
scripts/run-quiet e2e -- bash -c 'cd apps && pnpm --filter @kandev/web e2e -- tests/path/to/failing.spec.ts -g "exact failing test title"'
|
||||
```
|
||||
Run the specific failing test file(s) or test title first, not the full suite or full shard. Only proceed to the verify/commit/push step after the reported failure passes locally.
|
||||
|
||||
If the failed E2E spec is unrelated to the PR diff, the exact failing test passes locally via `pnpm e2e:run`, and there are no unresolved review threads or other failed checks, rerun the failed GitHub job once:
|
||||
|
||||
```bash
|
||||
gh run rerun <run-id> --failed
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
After rerunning, poll `scripts/pr-state --summary <PR>` until terminal using the bounded polling rules above.
|
||||
|
||||
If the failed specs are in unrelated existing areas and no changed code plausibly affects that surface, record the failure as unrelated in the PR fixup summary instead of changing unrelated tests.
|
||||
|
||||
When a UI copy rename is intentional, search E2E specs for old visible text before debugging deeper. Prefer updating assertions to the new user-facing label while keeping stable routes unchanged when route compatibility is intentional.
|
||||
|
||||
When `gh pr checks` shows many pending E2E matrix shards and no failures, inspect the workflow once for a compact remaining-jobs list instead of repeatedly dumping the full checks table:
|
||||
|
||||
```bash
|
||||
gh run view <run-id> --json status,conclusion,jobs \
|
||||
--jq '{status, conclusion, remaining: [.jobs[] | select(.status != "completed" or .conclusion != "success") | {name, status, conclusion}]}'
|
||||
```
|
||||
|
||||
**Don't dismiss a repeated failure as "flaky".** If the same shard or test fails 2+ poll iterations in a row, stop polling and do two cheap checks instead:
|
||||
|
||||
- **Compare per-shard runtime vs `main`.** `gh run list --branch main --workflow "<name>" --limit 5 --json databaseId` then `gh run view <id> --json jobs` and diff started/completed timestamps against the PR's run. A shard that takes e.g. 216s on main and 616s on the PR is real test failures + retries pushing past the job's `timeout-minutes` cap, not infrastructure variance. "Cancelled" at exactly the timeout boundary almost always means this.
|
||||
- **Reproduce the failing spec locally** (step 4 above). CI logs hide in-DOM React render errors that show up immediately in the local `e2e/test-results/<test>/error-context.md`. A single local run (~5-10 min) routinely unlocks fixes that would otherwise burn 3+ CI cycles of speculative "rerun and hope".
|
||||
|
||||
Recommend a merge over green-pending-flake-rerun only after both checks pass.
|
||||
If the failure is unfamiliar, looks like infrastructure, or involves E2E, load
|
||||
`references/ci-troubleshooting.md` before changing code. It covers branch
|
||||
history checks, cancelled concurrency duplicates, semantic-title transport
|
||||
failures, Vitest runner crashes, E2E container setup failures, and local E2E
|
||||
reproduction.
|
||||
|
||||
Mark task 2 as completed.
|
||||
|
||||
@@ -241,6 +209,8 @@ gh pr view <number> --json comments
|
||||
When `scripts/pr-resolve list <PR>` returns a `comment_id`, prefer fetching the exact review comment over bulk-fetching all PR comments/reviews if only one or two unresolved threads are actionable:
|
||||
|
||||
```bash
|
||||
scripts/pr-state --comment <comment_id>
|
||||
# Fallback if the helper is unavailable:
|
||||
gh api repos/:owner/:repo/pulls/comments/<comment_id> --jq .body
|
||||
```
|
||||
|
||||
@@ -259,6 +229,10 @@ gh api repos/:owner/:repo/issues/<PR>/comments \
|
||||
|
||||
OpenCode valid-empty output is represented by `<!-- opencode-review:no-findings -->`; absence of inline review comments does not mean OpenCode skipped review.
|
||||
|
||||
If an automated review check fails with a tool diagnostic instead of actionable code findings, classify it separately from review feedback. Fetch the failed job log and any diagnostic comment, but do not invent code changes for infrastructure output such as `OpenCode did not produce parseable findings`. Keep polling or rerun as appropriate until the required check is no longer failed.
|
||||
|
||||
When CodeQL opens an inline security review thread, resolving the thread is only half the work. The PR is not clean until the corresponding CodeQL check reruns successfully on the new head and `failed_checks` is empty.
|
||||
|
||||
**Verify before implementing.** Do not blindly accept review feedback — evaluate each comment technically:
|
||||
|
||||
For each comment:
|
||||
@@ -313,30 +287,40 @@ scripts/pr-resolve list <PR>
|
||||
scripts/pr-resolve show <PR> <THREAD_ID>
|
||||
scripts/pr-resolve show <PR> <COMMENT_ID>
|
||||
|
||||
# Reply syntax is: scripts/pr-resolve reply <PR> <comment_id> <thread_id> <body>
|
||||
# Reply syntax is:
|
||||
# scripts/pr-resolve reply <PR> <comment_id> <thread_id> <body>
|
||||
# scripts/pr-resolve reply <PR> <comment_id> <thread_id> --body-file <path>
|
||||
# Note the order: list prints <thread_id> then <comment_id>, but reply takes
|
||||
# <comment_id> before <thread_id>. If reply returns "Parent comment not found",
|
||||
# first check whether those two IDs were swapped.
|
||||
#
|
||||
# Reply + resolve + +1 (same call whether you're agreeing or pushing back —
|
||||
# the body text conveys which):
|
||||
scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> "Fixed — monotonic counter via useRef. See commit abc1234."
|
||||
scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> "Acknowledged; the strict source check was relaxed for E2E. Tracking as a follow-up."
|
||||
scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> 'Fixed via monotonic counter in useRef. See commit abc1234.'
|
||||
printf '%s\n' 'Acknowledged; the strict source check was relaxed for E2E. Tracking as a follow-up.' > /tmp/pr-reply.txt
|
||||
scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> --body-file /tmp/pr-reply.txt
|
||||
```
|
||||
|
||||
If `scripts/pr-resolve reply` fails mid-flight, run `scripts/pr-resolve show <PR> <THREAD_ID>` and `scripts/pr-resolve list <PR>` before retrying. Confirm whether the reply landed and only resolve/reaction failed, so you do not post a duplicate reply.
|
||||
When calling `scripts/pr-resolve reply`, avoid double-quoted bodies that
|
||||
contain Markdown backticks, `$...`, or shell metacharacters; the shell will
|
||||
execute or expand them before the script sees the text. Prefer single quotes
|
||||
for simple bodies, `--body-file` for multi-sentence replies, or plain text
|
||||
without Markdown code formatting. If quoting is awkward, shorten the reply and
|
||||
reference the commit SHA.
|
||||
|
||||
If `scripts/pr-resolve reply` fails mid-flight, run `scripts/pr-resolve show <PR> <THREAD_ID>` and `scripts/pr-resolve list <PR>` before retrying. Confirm whether the reply landed and only resolve/reaction failed, so you do not post a duplicate reply. When replying to several threads, check every command result; retry individual failures caused by transient GitHub, DNS, or API errors before treating that thread as still actionable.
|
||||
|
||||
`scripts/pr-resolve list` only prints previews. Before deciding whether a thread is valid, stale, duplicate, or already fixed, fetch the full comment/thread body:
|
||||
|
||||
```bash
|
||||
scripts/pr-resolve show <PR> <THREAD_ID_OR_COMMENT_ID>
|
||||
|
||||
# Repo helper for a single review comment:
|
||||
scripts/pr-state --comment <comment_id>
|
||||
|
||||
# Raw fallback for a single review comment:
|
||||
gh api repos/:owner/:repo/pulls/comments/<comment_id> --jq '{body,path,line,commit_id,html_url}'
|
||||
|
||||
# Repo helper fallback for a single review comment:
|
||||
scripts/pr-state --comment <comment_id>
|
||||
|
||||
# Raw fallback for all PR review comments when the helper only supports list/reply:
|
||||
gh api repos/:owner/:repo/pulls/<PR>/comments --paginate
|
||||
|
||||
@@ -348,6 +332,8 @@ Bots may post duplicate or stale review threads against the previous head immedi
|
||||
|
||||
For valid comments: read the file, implement the fix, then call `pr-resolve reply` with a body that names the commit or the file:line of the fix.
|
||||
|
||||
If the valid fix is PR-description prose rather than code, edit the PR body as part of the fixup before replying. If `gh pr edit <PR> --body-file <file>` fails with the GitHub Projects classic deprecation / `repository.pullRequest.projectCards` GraphQL error, do not retry the same wrapper. Use the REST body fallback documented in `/pr`, or fetch the PR node ID (`gh pr view <PR> --json id --jq .id`) and update the body through GraphQL `updatePullRequest`.
|
||||
|
||||
If the fix moves, splits, or deletes tests or source files, check both the original and destination paths before committing. Use `rg` for symbols or imports that were removed from the original file, and run targeted lint/typecheck commands that cover every touched file. File splits can leave missing imports or stale references in the file that stayed behind even when the moved test passes.
|
||||
|
||||
For skipped comments (already addressed, nitpick, wrong, outdated): call `pr-resolve reply` with a body that explains why. Examples:
|
||||
@@ -431,6 +417,7 @@ Mark task 5 as in_progress.
|
||||
```
|
||||
|
||||
If formatting changes files, re-run the affected checks after reviewing the diff.
|
||||
When combining format and test commands in one shell invocation, keep path roots consistent. For example, run `gofmt -w apps/backend/...` from the repo root, but run `go test ./internal/...` from `apps/backend`; do not chain root-relative `gofmt` paths inside an `apps/backend` working directory.
|
||||
|
||||
2. Stage and commit the fixes directly. Use a descriptive Conventional Commits message, e.g.:
|
||||
```
|
||||
@@ -439,6 +426,8 @@ Mark task 5 as in_progress.
|
||||
fix: address review feedback and fix CI failures
|
||||
```
|
||||
|
||||
Before staging, remove generated helper caches such as `.github/scripts/__pycache__/` if local Python scripts or `make test` created them. Do not include those caches in fixup commits.
|
||||
|
||||
For conflict-fix PRs, after merging or rebasing `origin/main`, check unresolved review threads again before pushing because conflict-resolution diffs can trigger new comments:
|
||||
```bash
|
||||
scripts/pr-resolve list <PR>
|
||||
@@ -467,6 +456,8 @@ After the push, CI restarts and bots may re-review. Use `pr-poller` again when a
|
||||
- The final re-check must include `scripts/pr-state --summary <PR>` (or a fresh pr-poller report), not only CI status. New review threads can arrive after a fresh push even when the previous unresolved count was zero.
|
||||
- If `ci_failed:` is empty AND `unresolved_review_threads: 0` AND `issue_comments_from_bots: 0` (no new bot comments to address) → keep polling until the equivalent `scripts/pr-state --summary <PR>` fields are also clean: `failed_checks: []`, `pending_checks: []`, and `unresolved_review_thread_count: 0`.
|
||||
- Treat a clean intermediate state as provisional while bot checks are still pending. Reviewer services can add new unresolved threads after CI is mostly green, so after resolving bot feedback and pushing a fixup, wait for both `pending_checks: []` and `unresolved_review_thread_count: 0` before declaring done.
|
||||
- In direct-command fallback, do not report the PR as review-clean while harness lint or bot review checks such as `Harness files pass lint`, `Claude Code Review`, `OpenCode Code Review`, CodeRabbit, Greptile, or cubic are still queued or in progress, unless the bounded poll cap is reached. Keep polling those review/lint checks until terminal, or explicitly report that review is still pending and may create new threads.
|
||||
- If only long-running checks such as CodeQL, preview deploy, or E2E remain pending after full local verification is green, `failed_checks: []`, and `unresolved_review_thread_count: 0`, you may stop at the 20-minute cap or 3-iteration loop limit and report the exact pending checks. Continue immediately if any check fails or a new unresolved thread appears.
|
||||
- If a pushed fix restarts CI while previous checks were already in progress, continue using `scripts/pr-state --summary <PR>`. Treat `filtered_review_thread_count` as informational when `unresolved_review_thread_count` is `0`; do not re-open or re-handle already-resolved historical/current-head filtered threads. Current-head CI can legitimately reset to queued after a push, especially for long E2E, CodeQL, or preview jobs. If checks remain pending when the 20-min cap or 3-iteration loop limit is reached, report the exact pending checks instead of waiting indefinitely unless the user explicitly asks to keep waiting.
|
||||
- If new CI failures appeared from the latest commit → loop back to task 2 and reset task 2-5 to `in_progress` as needed.
|
||||
- If new review comments appeared after the push → loop back to task 3.
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
# PR Fixup CI Troubleshooting
|
||||
|
||||
Load this reference from `/pr-fixup` when a failing CI check is unfamiliar,
|
||||
looks like infrastructure, or involves E2E.
|
||||
|
||||
## Narrow Unfamiliar Failures
|
||||
|
||||
If the failure looks unfamiliar or the cause is not obvious from the log, check
|
||||
CI history on the branch before diving into code:
|
||||
|
||||
```bash
|
||||
gh run list --branch <branch> --workflow "<workflow name>" --limit 10 --json conclusion,headSha,createdAt,databaseId
|
||||
```
|
||||
|
||||
On long-lived PRs that get rebased or squashed, prior SHAs on the same branch
|
||||
often passed the same workflow. A `passing -> failing` boundary tells you the
|
||||
regression is isolated to the most recent rework. Diff against the last passing
|
||||
SHA (`git diff <last-passing-sha>..HEAD`) instead of against `main`.
|
||||
|
||||
## Infrastructure Failures
|
||||
|
||||
**Cancelled concurrency duplicates:** A required check with
|
||||
`conclusion=cancelled`, 0s job durations, unexpanded `${{ matrix.* }}` job
|
||||
names, or a "Canceling since a higher priority waiting request ..." annotation
|
||||
is usually a superseded GitHub run, not a code failure. Confirm the
|
||||
non-cancelled run for the same head SHA passed, then trigger one clean run
|
||||
(rebase onto main + force-push, or `gh run rerun <id>`).
|
||||
|
||||
**Semantic PR title transport failures:** If `pr-title` /
|
||||
`amannn/action-semantic-pull-request` fails with transport or response parsing
|
||||
errors such as `invalid json response body ... Unexpected end of JSON input`,
|
||||
treat it as infrastructure. Confirm the PR title is valid Conventional
|
||||
Commits, rerun once, then re-check:
|
||||
|
||||
```bash
|
||||
gh run rerun <run-id> --failed
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
**Vitest runner/runtime crashes after passing suites:** If `Run Frontend Lint,
|
||||
Tests, and Build` or another Vitest-based job logs all test suites passing and
|
||||
then exits from a Node/V8 fatal crash such as `FATAL ERROR: v8::ToLocalChecked
|
||||
Empty MaybeLocal` or `node::cjs_lexer::Parse`, rerun the failed job once:
|
||||
|
||||
```bash
|
||||
gh run rerun <run-id> --failed
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
Only debug code if the rerun fails with an actual lint, test, or build error.
|
||||
|
||||
**E2E container setup failures:** If an E2E Containers shard fails during setup
|
||||
before tests run, check for dependency or registry failures. Patterns such as
|
||||
`packages.microsoft.com ... 403 Forbidden`, `docker/login-action@v3`,
|
||||
`Error response from daemon: Get "https://ghcr.io/v2/"`, `ghcr.io/token`,
|
||||
`context deadline exceeded`, or `Client.Timeout exceeded while awaiting
|
||||
headers` are infrastructure/package-registry issues, not app or test failures.
|
||||
If GitHub rejects `gh run rerun <run-id> --failed` while the workflow is still
|
||||
active, wait for the workflow/report job to finish and retry.
|
||||
|
||||
## E2E Failures
|
||||
|
||||
If any failing check is an E2E test:
|
||||
|
||||
1. Read the `/e2e` skill for debugging guidance, test patterns, and commands.
|
||||
2. Identify the exact failing spec/test from logs before changing code.
|
||||
3. Fix the root cause; never increase timeouts to hide flakes.
|
||||
4. Run the exact failed spec/title locally before a full shard. CI logs hide
|
||||
in-DOM React render errors that often show up in
|
||||
`e2e/test-results/<test>/error-context.md`.
|
||||
|
||||
Useful focused commands:
|
||||
|
||||
```bash
|
||||
scripts/run-quiet build -- make build-backend build-web
|
||||
scripts/run-quiet e2e -- bash -c 'cd apps && pnpm --filter @kandev/web e2e -- tests/path/to/failing.spec.ts'
|
||||
scripts/run-quiet e2e -- bash -c 'cd apps && pnpm --filter @kandev/web e2e -- tests/path/to/failing.spec.ts -g "exact failing test title"'
|
||||
```
|
||||
|
||||
If the failed E2E spec is unrelated to the PR diff, the exact failing test
|
||||
passes locally via `pnpm e2e:run`, and there are no unresolved review threads
|
||||
or other failed checks, rerun the failed GitHub job once and poll until terminal:
|
||||
|
||||
```bash
|
||||
gh run rerun <run-id> --failed
|
||||
scripts/pr-state --summary <PR>
|
||||
```
|
||||
|
||||
When a UI copy rename is intentional, search E2E specs for old visible text
|
||||
before debugging deeper. Prefer updating assertions to the new label while
|
||||
keeping stable routes unchanged when route compatibility is intentional.
|
||||
|
||||
For repeated failures, do not dismiss them as flaky. Compare per-shard runtime
|
||||
against recent `main` runs and reproduce the exact failing spec locally. A
|
||||
shard that is much slower on the PR than on `main`, or cancelled exactly at the
|
||||
job timeout boundary, usually indicates real test failures plus retries.
|
||||
|
||||
For pending E2E matrix shards, inspect the workflow once for a compact list
|
||||
instead of repeatedly dumping the full checks table:
|
||||
|
||||
```bash
|
||||
gh run view <run-id> --json status,conclusion,jobs \
|
||||
--jq '{status, conclusion, remaining: [.jobs[] | select(.status != "completed" or .conclusion != "success") | {name, status, conclusion}]}'
|
||||
```
|
||||
@@ -42,6 +42,8 @@ description: Commit, push, and create a PR. Default is ready-for-review with aut
|
||||
|
||||
3. **Push** the branch to origin with `-u` to set upstream tracking.
|
||||
|
||||
If the branch modifies `.github/workflows/*` and GitHub rejects the push with a message like `refusing to allow an OAuth App to create or update workflow ... without workflow scope`, treat it as push authentication/scope, not a code or branch-protection failure. Retry with an SSH remote when available, for example `git push git@github.com:<owner>/<repo>.git <branch>`, or tell the user the token needs `workflow` scope.
|
||||
|
||||
4. **Create the PR.** Use `--draft` flag if the user requested draft mode, otherwise create as ready-for-review.
|
||||
|
||||
**PR title** must follow Conventional Commits format (see `/commit` for full rules). CI validates via `pr-title.yml` — the PR title becomes the squash-merge commit used for release notes.
|
||||
@@ -72,7 +74,7 @@ description: Commit, push, and create a PR. Default is ready-for-review with aut
|
||||
|
||||
After pushing review fixes, interpret `scripts/pr-state --summary <PR>` thread counts carefully. The command filters thread details by the latest head commit, so `filtered_review_thread_count` can include resolved historical threads from the current filtered view. Treat `unresolved_review_thread_count` as the blocker. For example, a re-check may show `unresolved_review_thread_count: 0` and `filtered_review_thread_count: 3`; do not turn the filtered historical count into new unresolved work.
|
||||
|
||||
A ready PR may still end with "CI pending" after fixup when no checks have failed and no review threads remain unresolved, especially after a late fixup push restarts CodeQL, E2E, or preview jobs. When PR checks include long E2E shard queues, continue fixing failed checks and unresolved review threads, but it is acceptable to report the PR as ready locally once full local verification is green, CodeQL/security alerts are intentionally handled, required non-E2E checks are passing, and only queued/in-progress E2E shards remain with no failures. Do not wait indefinitely; include the exact pending checks from the final re-check in the response.
|
||||
A ready PR may still end with "CI pending" after fixup when no checks have failed and no review threads remain unresolved, especially after a late fixup push restarts CodeQL, E2E, or preview jobs. Continue fixing failed checks and unresolved review threads, but it is acceptable to report the PR as ready locally once full local verification is green, `failed_checks: []`, `unresolved_review_thread_count: 0`, and only queued/in-progress long-running checks remain. This includes CodeQL and preview deploy as well as E2E shards; do not wait indefinitely. Include the exact pending checks from the final re-check in the response, and stop immediately if a pending check fails or a new unresolved thread appears.
|
||||
|
||||
6. **PR screenshots:** After creating the PR, check if `apps/web/.pr-assets/manifest.json` exists. If it does:
|
||||
- Read the manifest to list available screenshots/GIFs
|
||||
|
||||
@@ -40,6 +40,12 @@ Commit any pending changes and push to the remote branch.
|
||||
If the branch has no upstream, use `git push -u origin <branch>`.
|
||||
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
|
||||
a message like `refusing to allow an OAuth App to create or update workflow
|
||||
... without workflow scope`, treat it as push authentication/scope, not a code
|
||||
or branch-protection failure. Retry with an SSH remote when available, for
|
||||
example `git push git@github.com:<owner>/<repo>.git <branch>`, or tell the
|
||||
user the token needs `workflow` scope.
|
||||
|
||||
4. **Report** the pushed commit hash and branch.
|
||||
|
||||
|
||||
@@ -40,3 +40,22 @@ In `apps/cli/src/runtime.ts`, the CLI locates its bundled runtime via:
|
||||
1. `KANDEV_BUNDLE_DIR` env var (set by Homebrew wrapper, used by tests).
|
||||
2. Installed `@kdlbs/runtime-{platform}` npm package via `require.resolve()`.
|
||||
3. `--runtime-version <tag>` cache fallback (debug only — downloads from GitHub).
|
||||
|
||||
## Runtime helper binary checklist
|
||||
|
||||
When adding, renaming, or removing bundled helper binaries such as `agentctl-<goos>-<goarch>`, update every packaging surface in the same PR:
|
||||
|
||||
- backend build targets and scripts
|
||||
- Docker/runtime image copy steps
|
||||
- `.github/workflows/release.yml` bundle, macOS signing, and notarization loops
|
||||
- `scripts/release/prepare-desktop-runtime.sh`
|
||||
- `scripts/release/verify-desktop-runtime.sh`
|
||||
- `scripts/release-desktop.test.sh`
|
||||
- `apps/desktop/AGENTS.md` runtime resource list
|
||||
|
||||
Verify with the helper build plus release-runtime tests, for example:
|
||||
|
||||
```bash
|
||||
make -C apps/backend build-agentctl-remote
|
||||
bash scripts/release-desktop.test.sh
|
||||
```
|
||||
|
||||
@@ -19,6 +19,10 @@ go.work
|
||||
# Dependency directories
|
||||
vendor/
|
||||
|
||||
# Python helper caches
|
||||
__pycache__/
|
||||
*.py[cod]
|
||||
|
||||
# IDE and editor files
|
||||
.idea/
|
||||
*.swp
|
||||
|
||||
@@ -18,7 +18,7 @@ apps/
|
||||
|
||||
- **Package manager**: `pnpm` workspace (run from `apps/`, not repo root)
|
||||
- **Backend**: Go with Make (`make -C apps/backend test|lint|build`)
|
||||
- **Frontend**: Vite/React SPA (`cd apps && pnpm --filter @kandev/web dev|build:vite|lint|typecheck`)
|
||||
- **Frontend**: Vite/React SPA (`cd apps && pnpm --filter @kandev/web dev|build:vite|lint`; for direct web typecheck use `cd apps/web && pnpm run typecheck`)
|
||||
- **Desktop**: Tauri shell (`cd apps && pnpm --filter @kandev/desktop build|e2e`; Rust tests from `apps/desktop/src-tauri`)
|
||||
- **UI**: Shadcn components via `@kandev/ui`
|
||||
- **E2E**: Playwright (`cd apps/web && pnpm e2e`). The `containers` project (gated on `KANDEV_E2E_CONTAINERS=1`, formerly `docker`) covers both the Docker executor and the SSH executor — anything that needs a real Docker daemon on the host lives there. See `apps/web/e2e/README.md`.
|
||||
@@ -87,6 +87,10 @@ For PR review/fixup workflows, prefer the repo helpers before manually querying
|
||||
|
||||
When a Kandev system message references an MCP tool that is not visible in the active tool list, use the runtime's tool discovery mechanism, such as `tool_search` when available, before falling back to a less specific workflow. Some task messaging and platform helpers are exposed on demand.
|
||||
|
||||
### Kandev Task Creation
|
||||
|
||||
When creating follow-up or delegated Kandev work from an active task, use `create_task_kandev` with `parent_id: "self"` when the work is related. That preserves workspace, workflow, repository, agent profile, and executor context from the current task. For genuinely unrelated top-level tasks, do not rely on workspace defaults when the user expects continuity; explicitly preserve the current task's `agent_profile_id` / `executor_profile_id`, or ask if the intended profile is ambiguous.
|
||||
|
||||
### Third-party integrations
|
||||
|
||||
Jira and Linear are the model (per-workspace credentials, 90s auth-health poller via `internal/integrations/healthpoll`, settings page with status banner). New integrations should **reuse the shared shapes** rather than copying either. Full layout, file conventions, and Jira-vs-Linear divergence notes in `apps/backend/internal/integrations/AGENTS.md` and the `/add-integration` skill — load either when scaffolding a new integration.
|
||||
|
||||
@@ -159,6 +159,8 @@ and can leak ACP subprocesses.
|
||||
- `sprites` - Sprites cloud environment
|
||||
- `remote_docker`, `remote_vps`, `k8s` - Planned
|
||||
|
||||
**Remote SSH executor platforms:** Treat supported remote OS/arch values as an end-to-end contract. Platform probe/normalization, lifecycle support checks, agentctl helper resolution, platform default shell, SSH readiness endpoints, frontend response types, and tests must stay aligned. Preserve raw unsupported platform details in user-facing errors, but use normalized values for supported-platform matching. Keep shell defaults platform-aware: Darwin defaults to `zsh`, Linux defaults to `bash`, unless an explicit shell is saved.
|
||||
|
||||
## Execution Flow
|
||||
|
||||
```text
|
||||
|
||||
@@ -17,6 +17,10 @@ If the new feedback is not actionable, do not modify files, do not commit, and d
|
||||
Non-actionable feedback includes summaries, status updates, no-finding reports, duplicated or
|
||||
previously addressed comments, rate-limit notices, and review diagnostics that do not request a
|
||||
concrete code or test change. In that case, reply only with a short summary that there is nothing actionable to address.
|
||||
Pending-only PR snapshots are also non-actionable: when `failed_checks: []` and there are zero
|
||||
unresolved review threads, but checks are still queued or running, do not modify files, do not run
|
||||
local verification, do not commit, and do not poll indefinitely. Reply that CI is still in progress
|
||||
and include the pending check names if they were provided.
|
||||
|
||||
Only make code changes when there is a concrete failing check, actionable review request, or
|
||||
reproducible issue that needs a fix. Do not push a commit merely to acknowledge feedback.
|
||||
|
||||
@@ -72,6 +72,14 @@ For rebasing or finishing PRs written against the old Next.js runtime, follow
|
||||
|
||||
Use subscription hooks only; the WS client auto-deduplicates.
|
||||
|
||||
When changing task lifecycle WS handlers (`task.updated`, `task.deleted`,
|
||||
`task.state_changed`), check both kanban and Office surfaces. Archive/delete
|
||||
events may need to update kanban caches, `tasks.activeTaskId` / session pin
|
||||
state, recent/sidebar prefs, Office refetch triggers such as
|
||||
`setOfficeRefetchTrigger("tasks")`, and route redirects for `/t/:id`,
|
||||
`/tasks/:id`, and `/office/tasks/:id`. Add focused tests for every affected
|
||||
surface.
|
||||
|
||||
## Component conventions
|
||||
|
||||
- **Framework adapters during Next removal:** Client components should import
|
||||
@@ -102,6 +110,30 @@ Use subscription hooks only; the WS client auto-deduplicates.
|
||||
and Playwright's `getByTestId` only matches one attribute name — the
|
||||
`data-legacy-testid` alias lets existing specs keep selecting the element
|
||||
while the migration is in flight.
|
||||
- **Dockview session panel activation:** session chat panels can become active
|
||||
through tab pointer/keyboard events, global tab-cycling shortcuts,
|
||||
reopen/menu actions, and Dockview close controls. When changing
|
||||
`tasks.activeSessionId` or active-session sync, audit all of those paths. Use
|
||||
store state in addition to Dockview `api.isActive`; the current session's chat
|
||||
tab may be Dockview-inactive while Files/Changes is active. Same-session
|
||||
clicks must not leave stale activation intent, and Dockview
|
||||
`.dv-default-tab-action` close controls should be treated as close/delete
|
||||
actions rather than session-switch intent.
|
||||
- **GitHub PR status UI:** visual PR/CI status surfaces should use the shared
|
||||
helpers in `apps/web/components/github/pr-task-icon.tsx`
|
||||
(`hasPRChecksPassedForDisplay`, `hasPRChecksInProgressForDisplay`, and
|
||||
`hasPRChecksPassedWithoutReviewWaitForDisplay`) instead of re-deriving status
|
||||
from `checks_state`, `checks_total`, or `checks_passing` locally. Aggregate
|
||||
check counts are a display-only fallback when `checks_state` is empty; they may
|
||||
make chips or task icons render passed/in-progress, but must not enable merge
|
||||
actions. Merge readiness must use `isPRReadyToMerge`, which requires GitHub's
|
||||
explicit `checks_state === "success"` rollup. When changing PR status behavior,
|
||||
update both `pr-task-icon.test.ts` and `pr-status-chip.test.tsx`.
|
||||
- **Task repository labels:** user-facing task/card repo chips should display a
|
||||
stable repo slug or name (`owner/repo` when known, otherwise the repo name),
|
||||
not a local filesystem path. Local clone paths or folder paths belong in
|
||||
hover/title/tooltip metadata. Tasks with no repository, or only a non-repo
|
||||
local folder, should not render a repo chip.
|
||||
|
||||
## Code-quality limits
|
||||
|
||||
|
||||
+18
-3
@@ -11,6 +11,7 @@
|
||||
# and URL. Use before deciding how to answer a preview from list.
|
||||
#
|
||||
# scripts/pr-resolve reply <PR> <comment_id> <thread_id> <body>
|
||||
# scripts/pr-resolve reply <PR> <comment_id> <thread_id> --body-file <path>
|
||||
# Reply to the thread starter, resolve the thread, and add a +1
|
||||
# reaction in one call. Use this for both "fixed" replies and
|
||||
# pushback replies — the reply body conveys whether you agreed
|
||||
@@ -37,7 +38,7 @@ case "$cmd" in
|
||||
;;
|
||||
esac
|
||||
|
||||
if [[ "$cmd" == "reply" && $# -eq 4 ]]; then
|
||||
if [[ "$cmd" == "reply" && ( $# -eq 4 || $# -eq 5 ) ]]; then
|
||||
if [[ "$2" == PRRT_* ]]; then
|
||||
echo "reply expects comment_id before thread_id; got a review thread ID in the comment_id position" >&2
|
||||
exit 1
|
||||
@@ -46,6 +47,15 @@ if [[ "$cmd" == "reply" && $# -eq 4 ]]; then
|
||||
echo "reply expects thread_id after comment_id; got a non-review-thread ID in the thread_id position" >&2
|
||||
exit 1
|
||||
fi
|
||||
if [[ $# -eq 4 && "$4" == "--body-file" ]]; then
|
||||
echo "reply --body-file requires a path" >&2
|
||||
exit 1
|
||||
fi
|
||||
if [[ $# -eq 5 ]]; then
|
||||
[[ "$4" == "--body-file" ]] || usage
|
||||
[[ -f "$5" ]] || { echo "body file not found: $5" >&2; exit 1; }
|
||||
[[ -s "$5" ]] || { echo "body file is empty: $5" >&2; exit 1; }
|
||||
fi
|
||||
fi
|
||||
|
||||
read -r OWNER REPO < <(gh repo view --json owner,name --jq '"\(.owner.login) \(.name)"')
|
||||
@@ -150,8 +160,13 @@ case "$cmd" in
|
||||
;;
|
||||
|
||||
reply)
|
||||
[ $# -eq 4 ] || usage
|
||||
PR=$1; COMMENT_ID=$2; THREAD_ID=$3; BODY=$4
|
||||
[[ $# -eq 4 || $# -eq 5 ]] || usage
|
||||
PR=$1; COMMENT_ID=$2; THREAD_ID=$3
|
||||
if [[ $# -eq 5 ]]; then
|
||||
BODY=$(<"$5")
|
||||
else
|
||||
BODY=$4
|
||||
fi
|
||||
gh api -X POST "repos/$OWNER/$REPO/pulls/$PR/comments/$COMMENT_ID/replies" \
|
||||
-f body="$BODY" --silent
|
||||
gh api graphql -f query='mutation($t: ID!) { resolveReviewThread(input: {threadId: $t}) { thread { isResolved } } }' \
|
||||
|
||||
@@ -54,3 +54,23 @@ if grep -q "non-review-thread ID in the thread_id position" "$TMP_DIR/stderr"; t
|
||||
else
|
||||
fail "non-thread id in thread position has clear error"
|
||||
fi
|
||||
|
||||
empty_file="$TMP_DIR/empty.txt"
|
||||
: >"$empty_file"
|
||||
if run_invalid_reply 456 PRRT_xyz --body-file "$empty_file"; then
|
||||
fail "empty body file fails"
|
||||
fi
|
||||
if grep -q "body file is empty" "$TMP_DIR/stderr"; then
|
||||
pass "empty body file has clear error"
|
||||
else
|
||||
fail "empty body file has clear error"
|
||||
fi
|
||||
|
||||
if run_invalid_reply 456 PRRT_xyz --body-file; then
|
||||
fail "missing body file path fails"
|
||||
fi
|
||||
if grep -q "requires a path" "$TMP_DIR/stderr"; then
|
||||
pass "missing body file path has clear error"
|
||||
else
|
||||
fail "missing body file path has clear error"
|
||||
fi
|
||||
|
||||
+25
-11
@@ -165,7 +165,7 @@ fi
|
||||
|
||||
extract_checks_json() {
|
||||
local status_json=$1
|
||||
jq -cn --argjson checks_in "$status_json" '
|
||||
jq -cn --slurpfile checks_in <(printf '%s\n' "$status_json") '
|
||||
def lower_or_null:
|
||||
if . == null or . == "" then null else ascii_downcase end;
|
||||
def extract($re; $field):
|
||||
@@ -191,7 +191,7 @@ extract_checks_json() {
|
||||
elif .conclusion == "cancelled" then 5
|
||||
else 6
|
||||
end;
|
||||
$checks_in
|
||||
$checks_in[0]
|
||||
| to_entries
|
||||
| map(.value + {_index: .key} | {
|
||||
type: (.["__typename"] // null),
|
||||
@@ -220,8 +220,8 @@ extract_checks_json() {
|
||||
extract_issue_comments_json() {
|
||||
local comments_json=$1
|
||||
local since=$2
|
||||
jq -cn --argjson comments "$comments_json" --arg since "$since" '
|
||||
$comments
|
||||
jq -cn --slurpfile comments <(printf '%s\n' "$comments_json") --arg since "$since" '
|
||||
$comments[0]
|
||||
| map(select($since == "" or ((.createdAt // .created_at // "") > $since)))
|
||||
| map({
|
||||
author: (.author.login // "unknown"),
|
||||
@@ -235,8 +235,8 @@ extract_issue_comments_json() {
|
||||
extract_reviews_json() {
|
||||
local reviews_api_json=$1
|
||||
local since=$2
|
||||
jq -cn --argjson reviews "$reviews_api_json" --arg since "$since" '
|
||||
$reviews
|
||||
jq -cn --slurpfile reviews <(printf '%s\n' "$reviews_api_json") --arg since "$since" '
|
||||
$reviews[0]
|
||||
| map(select($since == "" or ((.submitted_at // "") > $since)))
|
||||
| map({
|
||||
author: (.user.login // "unknown"),
|
||||
@@ -250,14 +250,14 @@ extract_reviews_json() {
|
||||
extract_review_threads_json() {
|
||||
local threads_nodes_json=$1
|
||||
local since=$2
|
||||
jq -cn --argjson threads "$threads_nodes_json" --arg since "$since" '
|
||||
jq -cn --slurpfile threads <(printf '%s\n' "$threads_nodes_json") --arg since "$since" '
|
||||
def relevant_comments:
|
||||
if $since == "" then
|
||||
(.comments.nodes // [])
|
||||
else
|
||||
[(.comments.nodes // [])[] | select((.createdAt // "") > $since)]
|
||||
end;
|
||||
$threads
|
||||
$threads[0]
|
||||
| map(. as $thread | (relevant_comments) as $comments | select($comments | length > 0) | {
|
||||
thread_id: .id,
|
||||
is_resolved: (.isResolved // false),
|
||||
@@ -373,7 +373,9 @@ fetch_review_threads_nodes_json() {
|
||||
local repo=$2
|
||||
local pr_number=$3
|
||||
local cursor=''
|
||||
local nodes='[]'
|
||||
local nodes_file
|
||||
nodes_file="$(mktemp)"
|
||||
printf '[]\n' >"$nodes_file"
|
||||
|
||||
while :; do
|
||||
local cursor_args=()
|
||||
@@ -410,12 +412,23 @@ fetch_review_threads_nodes_json() {
|
||||
}
|
||||
}' \
|
||||
-f owner="$owner" -f repo="$repo" -F pr="$pr_number" "${cursor_args[@]}")"; then
|
||||
rm -f "$nodes_file"
|
||||
return 1
|
||||
fi
|
||||
|
||||
local page_nodes
|
||||
page_nodes="$(jq -c '.data.repository.pullRequest.reviewThreads.nodes // []' <<<"$page_json")"
|
||||
nodes="$(jq -cn --argjson prev "$nodes" --argjson page "$page_nodes" '$prev + $page')"
|
||||
local next_nodes_file
|
||||
next_nodes_file="$(mktemp)"
|
||||
if ! jq -cn \
|
||||
--slurpfile prev "$nodes_file" \
|
||||
--slurpfile page <(printf '%s\n' "$page_nodes") \
|
||||
'$prev[0] + $page[0]' >"$next_nodes_file"; then
|
||||
add_error "review_threads" "failed to merge paginated review threads"
|
||||
rm -f "$next_nodes_file" "$nodes_file"
|
||||
return 1
|
||||
fi
|
||||
mv "$next_nodes_file" "$nodes_file"
|
||||
|
||||
local has_next
|
||||
has_next="$(jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage // false' <<<"$page_json")"
|
||||
@@ -430,7 +443,8 @@ fetch_review_threads_nodes_json() {
|
||||
fi
|
||||
done
|
||||
|
||||
printf '%s\n' "$nodes"
|
||||
cat "$nodes_file"
|
||||
rm -f "$nodes_file"
|
||||
}
|
||||
|
||||
repo_owner=''
|
||||
|
||||
+1
-1
@@ -110,7 +110,7 @@ grep_for_tag() {
|
||||
gh-run|ci|gh-run-view)
|
||||
# GitHub Actions log lines: "JOB\tSTEP\t...". Pull anything that
|
||||
# mentions error/failed/FAIL, plus the surrounding job context line.
|
||||
grep -nE '^\S+\s+\S+\s+.*(error|failed|FAIL|Error:|exit code)' "$PLAIN"
|
||||
grep -nE '^\S+\s+\S+\s+.*(error|failed|FAIL|Error:|exit code|golangci-lint run|[A-Za-z0-9_./-]+\.go:[0-9]+:[0-9]+:|[0-9]+ issues?:|apps/backend/test-results\.json|Process completed with exit code 1)' "$PLAIN"
|
||||
;;
|
||||
*)
|
||||
grep -nE '(error|Error|FAIL|panic:|Failed|✘|✗)' "$PLAIN"
|
||||
|
||||
Reference in New Issue
Block a user