pr-review-resolver

What this skill does

# PR Review Resolver

Interactively resolve unresolved PR review findings from the canonical pr-review-toolkit comment.

## Contract

Always communicate with the user in Traditional Chinese (`zh-TW`). Process issues one at a time and wait for the user's decision for each issue. Do not decide Fix, Deferred, or N/A yourself.

This skill is a resolver and coordinator, not a review producer and not a single-issue fixer:

- `pr-review-and-document` creates or updates review findings.
- `codex-review-pass` produces read-only review bundles.
- `codex-fix-worker` fixes exactly one selected issue with bounded owned files and reports results back to the resolver.
- `pr-review-resolver` reads unresolved issues, asks the user how to handle each one, coordinates fix workers or inline status decisions, and updates the canonical review comment.

Use `.pr-review-cache/pr-#.json` as the only review state file. Do not create extra cache files or PR comments. Do not commit, push, merge, or directly call `gh api` to update comments.

Find the toolkit root in this order:

1. Use `PR_REVIEW_TOOLKIT_ROOT` when set. This is the supported path.
2. If `PR_REVIEW_TOOLKIT_ROOT` is unset, derive the packaged plugin root from the skill path. This SKILL.md lives at `<root>/codex/skills/<skill-name>/SKILL.md`, so `<root>` is exactly three levels up. Ensure `SKILL_PATH` is set in the environment to the absolute path of this SKILL.md before running the snippet:

   ```bash
   : "${SKILL_PATH:?SKILL_PATH must be set to the absolute path of this SKILL.md}"
   PR_REVIEW_TOOLKIT_ROOT="$(cd "$(dirname "$SKILL_PATH")/../../.." && pwd)"
   ```

   Verify both `<root>/.codex-plugin/plugin.json` and `<root>/scripts/cache-write-comment.sh` exist. If either is missing, treat derivation as failed and proceed to step 3.
3. Stop and ask the dev agent for `PR_REVIEW_TOOLKIT_ROOT`.

Canonicalize the root before using helper scripts:

```bash
PR_REVIEW_TOOLKIT_ROOT="$(cd "$PR_REVIEW_TOOLKIT_ROOT" && pwd)"
```

Use only these scripts for review state:

```bash
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/get-pr-number.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/cache-read-comment.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/cache-write-comment.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/cache-sync.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/extract-content-hash.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/disambiguate-stale-source.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/check-fix-worker-scope.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/parse-validation-entry.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/review-metadata-upgrade.sh"
"${PR_REVIEW_TOOLKIT_ROOT}/scripts/review-metadata-replace.sh"
```

Before beginning the interactive loop, read `references/interaction-example.md`.

## Workflow

1. Get the PR number with `get-pr-number.sh`.
2. Read the canonical review comment:

   ```bash
   set +e
   REVIEW_CONTENT=$("${PR_REVIEW_TOOLKIT_ROOT}/scripts/cache-read-comment.sh" "$PR_NUMBER")
   rc=$?
   set -e

   case $rc in
     0) ;;
     2) echo "No canonical review comment found. Run pr-review-and-document first." >&2; exit 2 ;;
     *) echo "cache-read-comment.sh failed with exit $rc" >&2; exit "$rc" ;;
   esac
   ```

3. Extract the cache `content_hash` for CAS via the shared helper. The helper does file-existence check, jq read with rc capture, regex validation, and triggers `cache-sync.sh` on any failure (with rc propagation and post-recovery validation):

   ```bash
   set +e
   EXPECTED_CONTENT_HASH=$("${PR_REVIEW_TOOLKIT_ROOT}/scripts/extract-content-hash.sh" "$PR_NUMBER")
   rc=$?
   set -e

   case $rc in
     0) ;;
     2) echo "Cache was refreshed; re-read REVIEW_CONTENT and retry from Step 2." >&2; exit 2 ;;
     *) exit "$rc" ;;
   esac
   ```

   Unit-tested in `tests/extract-content-hash-test.sh`.
4. Parse unresolved items from the review content.
5. For each unresolved issue, one at a time:
   - Present the issue in Traditional Chinese.
   - Show source (`Claude`, `Gemini`, or `Codex`), title, file references, problem, and fix suggestion.
   - Read the referenced source files and verify whether the issue still exists.
   - Explain available options in Traditional Chinese.
   - Ask the user to choose: Fix, Deferred, N/A, or Skip for now.
   - Wait for the user's decision before moving to the next issue.
6. For user-approved fixes:
   - **Session setup (once per resolver session, before the first fix-worker dispatch):** initialize the session-scoped `OWNED_FILES` bash array that Step 9's scope check reads. Without this, Step 9 either errors under `set -u` ("OWNED_FILES[@]: parameter not set") or silently fail-opens under `set +u`:

     ```bash
     declare -a OWNED_FILES=()
     ```

     If this resolver session is resumed in a fresh shell (model context reset, new bash subshell), the `OWNED_FILES` array state is lost. The only safe action then is to abort this session and restart from Step 1; do NOT re-declare `OWNED_FILES=()` and continue, as the empty array would not reflect prior fix-worker dispatches and Step 9 would treat every subsequent write as unexpected (or fail-open, depending on shell flags).

   - Identify owned files from the issue and source inspection.
   - Check for overlap with any in-progress fix worker. Do not run two workers with overlapping owned files concurrently.
   - Spawn or invoke bounded fix work using the managed-only `codex-fix-worker` contract. Provide PR number, source, issue title, file references, user decision, and owned files.
   - Track `{issue title, source, owned files, worker id/status}` in memory.
   - Append the worker's owned files to the session-scoped `OWNED_FILES` array (initialized in Session setup above):

     ```bash
     # Each time a fix-worker is dispatched with owned files A B C:
     OWNED_FILES+=(A B C)
     ```

   - Continue discussing later issues only when doing so does not require the same files.
7. For Deferred or N/A decisions:
   - Record the reason from the user.
   - Add a self-contained source comment near the relevant code when appropriate:
     ```text
     Design Decision: <complete reason; do not rely on PR links>
     ```
   - Mark the item as `⏭️ Deferred` or `⏭️ N/A` in the planned comment update.
8. After all issues have decisions, collect fix worker results. Parse the worker's `Status:` line (see `codex-fix-worker` Output Contract); do not infer success from the presence of `Files changed:` alone:
   - `Status: success`: cross-check every `Validation:` entry by passing it to `${PR_REVIEW_TOOLKIT_ROOT}/scripts/parse-validation-entry.sh '<entry>'`. The parser distinguishes two valid shapes by its first stdout line:

     1. Run the parser; capture its rc. If rc != 0, the entry is malformed — treat the entire worker output as `partial`.
     2. Read the parser's first stdout line. If it equals the literal `none-possible`, this is a form-2 entry (`none possible: <reason>`) and counts as success-eligible — proceed to the next entry without further check.
     3. Otherwise interpret the first stdout line as a signed integer rc. If rc == 0, the entry counts as success-eligible. If rc != 0, treat the entire worker output as `partial`.
     4. Only mark `✅ Fixed` when every entry survives steps 1-3 as success-eligible.

     Workers that violate the safety net (em-dash separator, malformed shape, hidden non-zero exit, whitespace-only cmd, multiple `-- exit` boundaries) are rejected by the parser at step 1; their `Status: success` self-report does not bypass the cross-check.
   - `Status: partial`: report the validation gap in Traditional Chinese and ask the user whether to accept-as-fixed, retry, or defer.
   - `Status: failed`: report the error in Traditional Chinese and ask whether to retry, defer, or mark N/A. Do not mark `✅ Fixed`.
9. Validate modified file scope via the shared helper. The helper is byte-exact for paths containing spaces, literal newlines, non-ASCII bytes, and forces per-file enumeration of untracked-dir contents (`-uall`) so workers cannot hide