docs: record PR fixup harness learnings (#1416)

This commit is contained in:
Carlos Florêncio
2026-06-17 23:38:37 +01:00
committed by GitHub
parent 1f5a551579
commit 130ec19587
10 changed files with 217 additions and 23 deletions
+3 -3
View File
@@ -110,13 +110,13 @@ The `claude_summary` line carries the **latest** Claude summary's structured fin
- `done` if a matching review exists OR if `statusCheckRollup` shows the `cubic · AI code reviewer` check completed
- else `pending`
- **OpenCode** (`github-actions[bot]`, inline comments plus fallback issue comment):
- **OpenCode** (`github-actions[bot]`, usually inline comments via a trusted wrapper; fallback issue comments are optional and may be absent):
```bash
gh pr view <num> --json statusCheckRollup
gh pr view <num> --json comments --jq '.comments[] | select(.author.login == "github-actions" and (.body | startswith("## OpenCode review")))'
```
- `done` if `statusCheckRollup` shows `opencode-review-same-repo`, `opencode-review-fork`, or the `OpenCode Code Review` workflow completed
- also `done` if a fallback issue comment starts with `## OpenCode review`
- `done` primarily from `statusCheckRollup` when `opencode-review-same-repo`, `opencode-review-fork`, or the `OpenCode Code Review` workflow completed
- also `done` if a fallback issue comment starts with `## OpenCode review`, but do not require that comment
- else `pending`
d. **Exit conditions:**
+1
View File
@@ -258,6 +258,7 @@ Tests are grouped by feature area in subdirectories under `tests/`. When creatin
- **Test through the UI, not the API.** E2E tests verify user-facing behavior. Don't write tests that only call the API and assert the response -- those are integration tests. Instead, navigate to the page, interact with UI elements, and assert what the user sees.
- **Verify persistence with page reload.** After changing a setting or creating data, reload the page (`testPage.reload()`) and assert the state is still correct. This catches hydration bugs and SSR/client mismatches.
- **Seed via API, assert via UI.** Use `apiClient` to set up preconditions quickly, but always verify the result by opening the page and checking the DOM.
- **Workflow/session invariants.** For session-primary/profile behavior, prefer polling backend state with `apiClient.listTaskSessions(taskId)` for invariants such as `agent_profile_id`, `is_primary`, `state`, and session count, then add UI assertions as secondary evidence. UI tab markers can lag or be absent when the backend invariant is the behavior under test.
- **Scope terminal helpers to the active panel.** Terminal/mobile helpers must avoid document-wide `.xterm` or `terminal-xterm-host` selectors because multiple terminal panels can be mounted at once. Scope locators through `data-testid="terminal-panel"` and prefer the visible or latest panel for `page.evaluate` helpers.
- **Scope Dockview preview polling to visible panels.** Hidden or stale Dockview panels can remain mounted and produce false positives if helpers scan all matching custom elements globally. For `diffs-container`, filter candidate elements by visible layout box and computed visibility before reading shadow DOM text.
- **Poll before Dockview cleanup.** If an E2E helper uses `window.__dockviewApi__`, wait or poll for the API to be attached before acting. A one-shot `if (!api) return` cleanup can silently skip cleanup during page initialization and leak prior preview panels into later assertions.
+49 -4
View File
@@ -84,11 +84,15 @@ The summary output contains:
- `failed_checks` — actionable non-green checks with `name`, `workflow`, `status`, `conclusion`, `run_id`, `job_id`, `details_url`, and `target_url`
- `pending_checks` — still-running or queued checks with `name`, `workflow`, `status`, `run_id`, `job_id`, `details_url`, and `target_url`
- `unresolved_review_thread_count` — total unresolved thread count on the PR, including older unresolved threads outside the current-head filter
- `filtered_review_thread_count` and `unresolved_threads` — compact current-head inline review state to triage in this fixup pass
- `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
- `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>` fails with `jq: Argument list too long`, do not debug the summary script during fixup. Split the fallback checks:
```bash
@@ -98,9 +102,15 @@ 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 `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 `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.
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. 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`. 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.
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:
```bash
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.
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.
@@ -149,6 +159,25 @@ On long-lived PRs that get rebased/squashed, prior SHAs on the same branch often
**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):
@@ -157,7 +186,7 @@ If any failing check is an E2E test (Playwright):
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. Build required artifacts first if global setup needs them, then run the targeted Playwright command. Wrap with `scripts/run-quiet`:
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'
@@ -176,6 +205,15 @@ After rerunning, poll `scripts/pr-state --summary <PR>` until terminal using the
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.
@@ -218,6 +256,8 @@ Then classify:
- **Nitpick or preference** — subjective style not covered by linters. Skip unless the reviewer insists.
- **Wrong or outdated** — misunderstands the code, refers to old state, or is technically incorrect. Push back with reasoning.
For review comments about CI/workflow changes, account for `pull_request_target` trust boundaries. Source agent instructions from the base commit or trusted workflow content, not PR-head harness/config files; keep `GH_TOKEN` out of direct model execution when a trusted wrapper can post comments afterward; and validate model-produced comment targets against the computed diff scope before posting.
**Push back when:**
- The suggestion breaks existing functionality
- The reviewer lacks full context (explain what they're missing)
@@ -257,6 +297,9 @@ 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>
# 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):
@@ -264,6 +307,8 @@ scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> "Fixed — monotonic coun
scripts/pr-resolve reply <PR> <COMMENT_ID> <THREAD_ID> "Acknowledged; the strict source check was relaxed for E2E. Tracking as a follow-up."
```
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.
`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
+2 -1
View File
@@ -54,7 +54,6 @@ description: Commit, push, and create a PR. Default is ready-for-review with aut
- Remove all HTML comments/placeholders from the final body.
- Do NOT add tool attribution footers.
- Before creating the PR, self-check that the final body has no `<!--`, no empty required sections, and no placeholder text.
```bash
test -f .github/pull_request_template.md
# Build /tmp/pr-body.md from the template, using comments as instructions
@@ -66,6 +65,8 @@ description: Commit, push, and create a PR. Default is ready-for-review with aut
5. **If ready (not draft):** Run `/pr-fixup` to wait for CI checks and CodeRabbit, Greptile, Claude, OpenCode, and cubic review feedback, fix any failures or valid comments, and push.
Immediately after creating the PR, run `scripts/pr-state --summary <PR>` once. Automated review comments and required-check failures can arrive quickly; if comments or failures appear, switch into the `/pr-fixup` flow instead of treating PR creation as complete.
CodeRabbit issue comments that only report rate limits or exhausted usage credits are informational. They should not block PR completion when other review threads are resolved and checks are otherwise passing or pending.
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.
+39 -1
View File
@@ -23,17 +23,55 @@ Do NOT run the verification commands yourself in the main session when the helpe
Use this only when the runtime does not permit delegated helpers/subagents. Run the full pipeline directly from the repository root and report the exact commands and results:
```bash
# If the branch is behind main, rebase first:
# Fresh worktrees share .git/ but not apps/node_modules.
if [ ! -d apps/node_modules ]; then
(cd apps && pnpm install --frozen-lockfile)
fi
# If the branch is behind main, rebase first.
git fetch origin main
git merge-base --is-ancestor origin/main HEAD || echo "branch is behind origin/main"
git rebase origin/main
# make typecheck uses the top-level Makefile path and can bypass package
# pretypecheck hooks, so generate web metadata before typecheck.
make fmt
node apps/web/scripts/generate-release-notes.mjs
node apps/web/scripts/generate-changelog.mjs
make typecheck
make test
make lint
```
Before rebasing, check whether `origin/main` is already an ancestor of `HEAD`.
If `git rebase origin/main` fails because there are unstaged tracked changes,
stash only the relevant tracked changes for the intended commit, rebase, then
pop the stash before running `make fmt/typecheck/test/lint`. Do not stash
unrelated user changes blindly. Resolve conflicts before continuing
verification.
If `make fmt` changes files, review the diff and continue with the remaining commands. If any command fails, fix the issue and re-run the failed command; for formatter-caused changes, re-run any affected checks before reporting success.
If `make typecheck` still fails because `apps/web/generated/changelog.json` or
`apps/web/generated/release-notes.json` is missing, regenerate them and rerun
`make typecheck`:
```bash
(cd apps/web && node scripts/generate-release-notes.mjs)
(cd apps/web && node scripts/generate-changelog.mjs)
```
When verifying the web package directly, prefer:
```bash
(cd apps/web && pnpm run typecheck)
```
That package script runs `pretypecheck` and regenerates
`generated/changelog.json` / `generated/release-notes.json`. If troubleshooting
the web package directly, prefer the package-local script over workspace-filter
forms so TypeScript runs in the intended package context.
If the aggregate `make lint` wrapper stalls or does not provide useful progress, run the backend and frontend lint checks directly instead and record the substitution in your result:
```bash
+2
View File
@@ -80,6 +80,8 @@ Every code change must include tests for new or changed logic. Backend: `*_test.
### GitHub Operations
Skills use `gh` CLI by default. If a `gh` command fails (not installed, not authenticated, etc.), use whatever GitHub tools are available in the environment (MCP GitHub tools, API tools, etc.) to accomplish the same operation. The goal is the same — the tool may differ.
For PR review/fixup workflows, prefer the repo helpers before manually querying GitHub/GraphQL: `scripts/pr-state --summary <PR>` for checks and unresolved-thread state, `scripts/pr-resolve list <PR>` for actionable unresolved review threads, and `scripts/pr-resolve reply <PR> <comment_id> <thread_id> "<body>"` to reply, resolve, and react in one call.
### 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.
+17
View File
@@ -31,6 +31,23 @@ usage() {
cmd=$1; shift
case "$cmd" in
-h | --help | help)
usage 0
;;
esac
if [[ "$cmd" == "reply" && $# -eq 4 ]]; 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
fi
if [[ "$3" != PRRT_* ]]; then
echo "reply expects thread_id after comment_id; got a non-review-thread ID in the thread_id position" >&2
exit 1
fi
fi
read -r OWNER REPO < <(gh repo view --json owner,name --jq '"\(.owner.login) \(.name)"')
case "$cmd" in
+56
View File
@@ -0,0 +1,56 @@
#!/usr/bin/env bash
set -euo pipefail
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
SCRIPT="$ROOT_DIR/scripts/pr-resolve"
pass() {
printf 'ok - %s\n' "$1"
}
fail() {
printf 'not ok - %s\n' "$1" >&2
exit 1
}
TMP_DIR="$(mktemp -d)"
cleanup() {
rm -rf "$TMP_DIR"
}
trap cleanup EXIT
mkdir -p "$TMP_DIR/bin"
printf '#!/usr/bin/env bash\necho "gh should not be called" >&2\nexit 99\n' >"$TMP_DIR/bin/gh"
chmod +x "$TMP_DIR/bin/gh"
run_invalid_reply() {
PATH="$TMP_DIR/bin:$PATH" "$SCRIPT" reply 123 "$@" 2>"$TMP_DIR/stderr"
}
if PATH="$TMP_DIR/bin:$PATH" "$SCRIPT" --help >"$TMP_DIR/help" 2>"$TMP_DIR/stderr"; then
if grep -q "scripts/pr-resolve list <PR>" "$TMP_DIR/help"; then
pass "--help prints usage without gh"
else
fail "--help prints usage without gh"
fi
else
fail "--help exits zero"
fi
if run_invalid_reply PRRT_bad 456 body; then
fail "thread id in comment position fails"
fi
if grep -q "review thread ID in the comment_id position" "$TMP_DIR/stderr"; then
pass "thread id in comment position has clear error"
else
fail "thread id in comment position has clear error"
fi
if run_invalid_reply 456 789 body; then
fail "non-thread id in thread position fails"
fi
if grep -q "non-review-thread ID in the thread_id position" "$TMP_DIR/stderr"; then
pass "non-thread id in thread position has clear error"
else
fail "non-thread id in thread position has clear error"
fi
+45 -14
View File
@@ -10,6 +10,7 @@
# - since: { head_oid, committed_at } or null
# - checks: [{ type, name, workflow, status, conclusion, details_url, target_url, run_id, job_id }]
# - review_threads: [{ thread_id, is_resolved, path, comment_id, comment_created_at, author, body_preview }]
# - hidden_unresolved_threads: [{ thread_id, comment_id, author, path, body_preview }]
# - unresolved_review_thread_count: <N>
# - filtered_review_thread_count: <N>
# - reviews: [{ author, state, submitted_at, html_url }]
@@ -25,6 +26,7 @@
# - unresolved_review_thread_count: <N>
# - filtered_review_thread_count: <N>
# - unresolved_threads: [{ thread_id, comment_id, author, path, body_preview }]
# - hidden_unresolved_threads: [{ thread_id, comment_id, author, path, body_preview }]
# - summary_notes: [<short human-readable notes>]
# - errors: [{ source, message }]
#
@@ -84,6 +86,7 @@ errors='[]'
pr_json='{}'
checks_json='[]'
review_threads_json='[]'
hidden_unresolved_threads_json='[]'
reviews_json='[]'
issue_comments_json='[]'
since_json='null'
@@ -257,6 +260,7 @@ summarize_state_json() {
| select(.is_resolved == false)
| {thread_id, comment_id, author, path, body_preview}
],
hidden_unresolved_threads: (.hidden_unresolved_threads // []),
summary_notes: (
[
if (
@@ -433,6 +437,16 @@ fi
if [[ -n "$repo_owner" && -n "$repo_name" && -n "$resolved_pr_number" ]]; then
if threads_nodes_json="$(fetch_review_threads_nodes_json "$repo_owner" "$repo_name" "$resolved_pr_number")"; then
review_threads_json="$(extract_review_threads_json "$threads_nodes_json" "$since")"
all_review_threads_json="$(extract_review_threads_json "$threads_nodes_json" "")"
hidden_unresolved_threads_json="$(jq -cn \
--slurpfile all <(printf '%s\n' "$all_review_threads_json") \
--slurpfile filtered <(printf '%s\n' "$review_threads_json") \
'
($filtered[0] | map(.thread_id)) as $filtered_ids
| $all[0]
| map(select(.is_resolved == false and (.thread_id as $id | $filtered_ids | index($id) | not)))
| map({thread_id, comment_id, author, path, body_preview})
')"
unresolved_review_thread_count="$(jq -r '[.[] | select(.isResolved == false)] | length' <<<"$threads_nodes_json")"
filtered_review_thread_count="$(jq -r 'length' <<<"$review_threads_json")"
else
@@ -444,26 +458,43 @@ elif [[ -z "$resolved_pr_number" ]]; then
add_error "review_threads" "skipped: resolved PR identity unavailable"
fi
state_tmp_dir="$(mktemp -d)"
cleanup_state_tmp() {
rm -rf "$state_tmp_dir"
}
trap cleanup_state_tmp EXIT
printf '%s\n' "$pr_json" >"$state_tmp_dir/pr.json"
printf '%s\n' "$since_json" >"$state_tmp_dir/since.json"
printf '%s\n' "$checks_json" >"$state_tmp_dir/checks.json"
printf '%s\n' "$review_threads_json" >"$state_tmp_dir/review_threads.json"
printf '%s\n' "$hidden_unresolved_threads_json" >"$state_tmp_dir/hidden_unresolved_threads.json"
printf '%s\n' "$reviews_json" >"$state_tmp_dir/reviews.json"
printf '%s\n' "$issue_comments_json" >"$state_tmp_dir/issue_comments.json"
printf '%s\n' "$errors" >"$state_tmp_dir/errors.json"
state_json="$(jq -cn \
--argjson pr "$pr_json" \
--argjson since "$since_json" \
--argjson checks "$checks_json" \
--argjson review_threads "$review_threads_json" \
--argjson reviews "$reviews_json" \
--argjson issue_comments "$issue_comments_json" \
--slurpfile pr "$state_tmp_dir/pr.json" \
--slurpfile since "$state_tmp_dir/since.json" \
--slurpfile checks "$state_tmp_dir/checks.json" \
--slurpfile review_threads "$state_tmp_dir/review_threads.json" \
--slurpfile hidden_unresolved_threads "$state_tmp_dir/hidden_unresolved_threads.json" \
--slurpfile reviews "$state_tmp_dir/reviews.json" \
--slurpfile issue_comments "$state_tmp_dir/issue_comments.json" \
--argjson unresolved_review_thread_count "$unresolved_review_thread_count" \
--argjson filtered_review_thread_count "$filtered_review_thread_count" \
--argjson errors "$errors" \
--slurpfile errors "$state_tmp_dir/errors.json" \
'{
pr: $pr,
since: $since,
checks: $checks,
review_threads: $review_threads,
pr: $pr[0],
since: $since[0],
checks: $checks[0],
review_threads: $review_threads[0],
hidden_unresolved_threads: $hidden_unresolved_threads[0],
unresolved_review_thread_count: $unresolved_review_thread_count,
filtered_review_thread_count: $filtered_review_thread_count,
reviews: $reviews,
issue_comments: $issue_comments,
errors: $errors
reviews: $reviews[0],
issue_comments: $issue_comments[0],
errors: $errors[0]
}')"
if [[ "$SUMMARY" == "1" ]]; then
+3
View File
@@ -505,6 +505,8 @@ test_summary_mode_returns_compact_fixup_state() {
assert_jq "summary filtered thread count" '.filtered_review_thread_count == 1' "$json"
assert_jq "summary unresolved threads" '.unresolved_threads | length == 1' "$json"
assert_jq "summary unresolved thread fields" '.unresolved_threads[0] | .thread_id == "PRRT_1" and .comment_id == 112 and .author == "greptile-apps[bot]"' "$json"
assert_jq "summary hidden unresolved threads" '.hidden_unresolved_threads | length == 1' "$json"
assert_jq "summary hidden unresolved thread fields" '.hidden_unresolved_threads[0] | .thread_id == "PRRT_2" and .comment_id == 222 and .author == "cubic-dev-ai[bot]"' "$json"
assert_jq "summary omits raw arrays" 'has("checks") | not' "$json"
assert_jq "summary no errors" '.errors == []' "$json"
pass "--summary returns compact fixup state"
@@ -524,6 +526,7 @@ test_summary_all_flag_includes_historical_unresolved_threads() {
assert_jq "summary all filtered thread count" '.filtered_review_thread_count == 2' "$json"
assert_jq "summary all keeps historical first comment" '.unresolved_threads[] | select(.thread_id == "PRRT_1") | .comment_id == 111' "$json"
assert_jq "summary all unresolved threads count" '.unresolved_threads | length == 2' "$json"
assert_jq "summary all has no hidden unresolved threads" '.hidden_unresolved_threads == []' "$json"
pass "--summary --all includes historical unresolved thread comments"
}