fix-pr-tests

Files (1)

• SKILL.md

SKILL.md (first lines)

> **Tool mapping (OpenCode):** This skill uses:
> - `question` tool for user prompts (not `AskUserQuestion`)
> - `task` tool for subagents (not `Agent` or `Task` subagent syntax)
> - `skill({ name: "..." })` to load other skills (not `Skill(skill="...", args="...")`)
> - `codegraph_explore`, `codegraph_search`, `codegraph_context` for codebase exploration (preferred)
> - `todowrite` for phase/progress tracking
> - `grep`, `glob`, `read`, `bash` as fallback when `.codegraph/` not initialized
>
> **Context rule:** This skill supersedes any prior skill instructions. Follow ONLY these instructions now. Read the user's goal and any mode/include/skip from the conversation context.

Read `mode=`, `include=`, and `skip=` from the conversation context. Default to the mode specified in this skill if not provided.


# Fix PR Tests

A failing PR test means **one** of three things. Get the classification right before touching code — fixing the wrong side either ships a real bug masked by a "fixed" test, or wastes time changing prod code when the test was just stale.

---

## Phase 1 — Locate the failure

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

1. Resolve the PR: explicit `<PR#>` from the user, otherwise `gh pr view --json number,headRefName` for the current branch. If neither resolves, stop and ask.
2. List failing checks: `gh pr checks <PR#>` — note every failing job (don't fix the first one and assume the rest are duplicates).
3. For each failing job, fetch the run ID and pull logs:
   ```
   gh run view <run-id> --log-failed
   ```
   `--log-failed` skips the green steps. Read the actual assertion message, file path, and line — not just "tests failed".
4. Extract: failing test name(s), file path, assertion/error message, stack frames pointing into project code.

**Checkpoint — do not proceed without:** test name, file:line, exact error. If logs are truncated or missing, fetch the full log with `gh run view <run-id> --log` before guessing.

---

## Phase 2 — Reproduce locally

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

Run **only the failing test** first, not the suite. Goals: confirm it fails the same way locally, and get a fast iteration loop.

Before reproducing, ask: what could differ between local and CI? (env vars, timezone, seed, parallelism, OS, FS case-sensitivity).

- Detect runner from project (`package.json` scripts, `pytest.ini`, `go.mod`, etc.) — do not assume.
- Use the runner's single-test filter (`vitest run <file> -t "<name>"`, `pytest path::test_name`, `go test -run TestName ./pkg`, etc.).
- If it **passes locally** but fails in CI: this is a CI-environment signal (env var, timezone, fixture seed, parallelism, OS). Do NOT edit the test to make CI green — investigate the env delta first. Common deltas: `TZ`, `CI=true` branches, `Date.now()` / random seeds, file-system case sensitivity, network egress.
- If it **fails locally** the same way: continue to Phase 3.
- If it **fails locally differently**: the local environment is also broken — note both failures; the CI one is still the source of truth.

---

## Phase 3 — Classify (real bug vs. stale test vs. flake)

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

This is the load-bearing decision. Before classifying, read both the test AND the production code it exercises.

Ask, in order:

1. **Is this a flake?** Re-run the single test 3× locally. If it alternates pass/fail with no code change → flake. Look for: `Date.now()`, `Math.random()`, unseeded fixtures, timing-based assertions, ordering assumptions on unordered collections, shared global state across tests. Fix the **flakiness source** (seed it, freeze time, sort before compare) — never paper over with retries unless the underlying nondeterminism is genuinely external.

2. **Did the test's intent change recently?** `git log -p <test-file>` and `git log -p <prod-file>` over the PR's commit range. If the PR intentionally changed behavior and the test still encodes the old behavior → **stale test**. The fix is to update the test's expected values to match the new, intentional behavior.

3. **Did production code regress?** If the test encodes a contract (public API shape, documented behavior, business rule) and the PR's prod-code change broke it → **real bug**. The fix is in the production code; the test stays as-is.

4. **Ambiguous?** When you cannot tell whether the new behavior is intentional, **stop and ask the user**. Do not guess. Call the `question` tool to present: "Test `X` expects `A`, code now returns `B`. Is `B` the intended new behavior (I'll update the test) or a regression (I'll fix the code)?"

**Checkpoint — write down the classification and one-sentence justification before editing anything.**

---

## Phase 4 — Fix

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

Apply the fix to the side the classification picked. Keep the diff minimal.

- **Real bug:** fix production code. Do not also "tighten" the test in the same change.
- **Stale test:** update only the assertions that reflect the changed behavior. If you find yourself rewriting the whole test, the classification was probably wrong — re-check Phase 3.
- **Flake:** fix the source of nondeterminism in the test or fixture. Adding a `retry(3)` is not a fix.

After fixing one failure, **scan sibling tests** for the same root cause before pushing. One regression often breaks N tests; fixing one and pushing means another red CI in 5 minutes.

---

## Phase 5 — Verify, then push

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

1. Re-verify: failing test → containing file → full suite only if the fix touched shared code (utilities, types, schemas, config).
2. Commit with a message that names the failure and the classification: e.g. `fix: handle null user in formatName (regression caught by formatName.test.ts)` or `test: update formatName expected output for new locale rule`.
3. Push to the PR branch. Then `gh pr checks <PR#> --watch` (or poll) until CI re-runs. If still red, return to Phase 1 with the new logs — do not assume a second failure is the same as the first.

---

## NEVER

- **NEVER edit a test to make it pass without classifying first**
  **Instead:** Complete Phase 3 and write down the classification before opening the test file.
  **Why:** Editing the assertion is the fastest way to make CI green and the fastest way to ship a real regression. The test was the alarm; silencing the alarm doesn't fix the fire.

- **NEVER fix the prod code AND the test in the same change when only one was wrong**
  **Instead:** Pick one side based on Phase 3. If both genuinely need to change (e.g., intentional API rename), say so explicitly in the commit message.
  **Why:** Touching both sides together hides which one was the actual defect and prevents future bisects from isolating cause.

- **NEVER push a fix without reproducing the failure locally first**
  **Instead:** Phase 2 must show the same failure on your machine before Phase 4. If it won't reproduce, that's the bug to investigate, not a reason to push speculatively.
  **Why:** Push-and-pray burns CI minutes, pollutes the PR's commit history with "try again" commits, and misses CI-environment-specific bugs that won't be caught by speculative fixes.

- **NEVER classify a single failure as a flake without re-running 3×**
  **Instead:** Re-run locally with the same seed/env at least three times. Only call it a flake if it actually alternates.
  **Why:** "Probably flaky" is the most common cover story for a real intermittent bug. Calling a deterministic failure a flake means the bug ships and re-surfaces under load.

- **NEVER skip sibling-test scan after fixing one failure**
  **Instead:** After Phase 4, grep the codebase for tests that touch the same prod symbol/file before pushing.
  **Why:** A single root cause usually breaks multiple tests. Fixing only the first one named in logs guarantees a second red CI run on the same PR.

- **NEVER edit `.github/workflows/` or CI config to make a test pass**

From Acauhi99/opencode-agent-system, MIT.

More from Acauhi99/opencode-agent-system