From b811ccc1bcf66d8bb89bda45c6ba84201fe359bd Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Tue, 1 Sep 2026 11:58:14 -0700 Subject: [PATCH 1/2] Enable explain-open in Codex --- ai/codex/excluded-skills.txt | 1 - ai/skills/explain-open/SKILL.md | 15 +++++++-------- ai/tests/test-ai-installers.sh | 2 ++ ai/tests/test-canonical-skills.sh | 7 +++++-- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ai/codex/excluded-skills.txt b/ai/codex/excluded-skills.txt index c0c65b4..03c874a 100644 --- a/ai/codex/excluded-skills.txt +++ b/ai/codex/excluded-skills.txt @@ -2,7 +2,6 @@ analyze-permissions # Workflows that depend on Claude-only slash commands or external Claude skills. -explain-open go review-fix-cycle diff --git a/ai/skills/explain-open/SKILL.md b/ai/skills/explain-open/SKILL.md index b72388e..a96efb6 100644 --- a/ai/skills/explain-open/SKILL.md +++ b/ai/skills/explain-open/SKILL.md @@ -1,7 +1,6 @@ --- name: explain-open description: Explain each open or skipped code-review item in plain English, weigh what happens on each side of the decision, and give a recommendation. -compatibility: Designed for Claude Code (or similar products) argument-hint: "[|||]" model: sonnet metadata: @@ -17,17 +16,17 @@ Code reviews leave two kinds of loose ends: items explicitly flagged as open que - No argument — use the code review already visible in this conversation. This is the common case: you just ran a review and want the loose ends explained before deciding. - `` or `` — look up that PR's review artifacts. - `` — look up that branch's review artifacts. -- `` — read a specific review file directly (e.g. a saved `/review-code` output or `.notes/review-skipped.md`). +- `` — read a specific review file directly (e.g. a saved `review-code` output or `.notes/review-skipped.md`). ## Step 1: Gather the Open and Skipped Items ### If no argument was given -Scan back through this conversation for code review activity — output from `/review-code`, `/address-pr-reviews`, `/review-fix-cycle`, an ad hoc review, or PR comment triage. +Scan back through this conversation for code review activity — output from `review-code`, `address-pr-reviews`, `review-fix-cycle`, an ad hoc review, or PR comment triage. - If exactly one review is visible, use it. - If more than one review appears (e.g., you reviewed one PR earlier, then separately reviewed another), use only the most recent one. Items from an earlier review are likely stale or about a different target, and mixing them in produces a confusing, ungrounded list. If it's genuinely unclear which of several reviews is the current one, ask which target to use rather than guessing or merging both. -- If the conversation has been compacted or summarized and you can't recover the specific findings (file, line, exact wording) from what's actually in context, say so rather than filling in detail from a vague summary. Suggest re-running the review, or invoking `/explain-open ` to read the saved review file directly. +- If the conversation has been compacted or summarized and you can't recover the specific findings (file, line, exact wording) from what's actually in context, say so rather than filling in detail from a vague summary. Suggest re-running the review, or invoking the skill with the target (`$explain-open ` in Codex or `/explain-open ` in Claude Code) to read the saved review file directly. Pull out every item that fits either bucket: @@ -54,22 +53,22 @@ If nothing in the conversation fits either bucket, say so plainly and stop. Don' - **It's a GitHub PR URL** (`https://github.com///pull/`) — resolve it: ```bash - ~/.dotfiles/bin/detect-pr.sh --json "$ARGUMENTS" + ~/.dotfiles/bin/detect-pr.sh --json "" ``` This always exits 0 and prints JSON. If `error` is null, use `org`, `repo`, and `pr_number` from the result: the identifier for step 3 is `pr-`, passed alongside `--org --repo ` (the PR may belong to a different repo than the current checkout). If `error` is set, tell the user the PR reference looks malformed and stop; don't fall through to treating it as a branch name. - **It's a bare integer** (e.g. `456`) — use it directly as the identifier in step 3, with no `--org`/`--repo`. Don't call `detect-pr.sh`; `review-file-path.sh` resolves bare PR numbers against the current git checkout on its own. - - **Anything else** — treat it as a branch name. Use `$ARGUMENTS` directly as the identifier in step 3, with no `--org`/`--repo`. Don't call `detect-pr.sh` here: it only resolves PR URLs or numbers and rejects everything else, so calling it first just adds a step that's guaranteed to fail. + - **Anything else** — treat it as a branch name. Use the provided argument directly as the identifier in step 3, with no `--org`/`--repo`. Don't call `detect-pr.sh` here: it only resolves PR URLs or numbers and rejects everything else, so calling it first just adds a step that's guaranteed to fail. 3. Locate the review file: ```bash - ~/.claude/skills/review-code/scripts/review-file-path.sh [--org --repo ] + ~/.agents/skills/review-code/scripts/review-file-path.sh [--org --repo ] ``` - (Include `--org`/`--repo` only for the PR-URL case above.) Parse the JSON output. If `file_exists` is true, read `file_path` and pull out `` `question` `` findings plus any `` `suggestion` ``/`` `nit` `` findings not marked as fixed. + If `review-code` is not installed there, tell the user that target lookup requires that skill and stop. Include `--org`/`--repo` only for the PR-URL case above. Parse the JSON output. If `file_exists` is true, read `file_path` and pull out `` `question` `` findings plus any `` `suggestion` ``/`` `nit` `` findings not marked as fixed. 4. Check for a skipped-items log in the repo: diff --git a/ai/tests/test-ai-installers.sh b/ai/tests/test-ai-installers.sh index 8474004..6327445 100755 --- a/ai/tests/test-ai-installers.sh +++ b/ai/tests/test-ai-installers.sh @@ -163,6 +163,8 @@ if [[ -n "$enabled_skill" ]] && run_installer "$CODEX_INSTALLER" --skills-only; check_eq "Codex skill symlinks point at the canonical source" \ "$(resolved_target "$FAKE_HOME/.agents/skills/$enabled_skill")" \ "${REPO_ROOT}/ai/skills/$enabled_skill" + check "Codex installs explain-open" \ + test -L "$FAKE_HOME/.agents/skills/explain-open" else fail "Codex skill installation succeeds" fi diff --git a/ai/tests/test-canonical-skills.sh b/ai/tests/test-canonical-skills.sh index fa7dea7..6c301a6 100755 --- a/ai/tests/test-canonical-skills.sh +++ b/ai/tests/test-canonical-skills.sh @@ -168,9 +168,12 @@ for skill_dir in "${skill_dirs[@]}"; do is_excluded_skill "$skill_name" "$EXCLUSIONS" && continue skill_file="$skill_dir/SKILL.md" [[ -f "$skill_file" ]] || continue - # Drop absolute references first: their tails look exactly like relative ones, and + # Drop supported absolute references first: their tails look exactly like relative ones, and # scanning for both at once reports every cross-skill path as a missing local file. - body=$(sed 's#~/\.dotfiles/[A-Za-z0-9._/-]*##g' "$skill_file") + body=$(sed -E \ + -e 's#~/\.dotfiles/[A-Za-z0-9._/-]*##g' \ + -e 's#~/\.agents/skills/review-code/[A-Za-z0-9._/-]*##g' \ + "$skill_file") while IFS= read -r ref; do [[ -z "$ref" || -e "$skill_dir/$ref" ]] && continue echo "FAIL: $skill_file references $ref, which does not exist in the skill" From 6b55ec7aa6e9de1bdefca687172c1a186f90287f Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Tue, 1 Sep 2026 13:18:48 -0700 Subject: [PATCH 2/2] Address PR review feedback --- ai/tests/test-canonical-skills.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ai/tests/test-canonical-skills.sh b/ai/tests/test-canonical-skills.sh index 6c301a6..747067e 100755 --- a/ai/tests/test-canonical-skills.sh +++ b/ai/tests/test-canonical-skills.sh @@ -172,7 +172,7 @@ for skill_dir in "${skill_dirs[@]}"; do # scanning for both at once reports every cross-skill path as a missing local file. body=$(sed -E \ -e 's#~/\.dotfiles/[A-Za-z0-9._/-]*##g' \ - -e 's#~/\.agents/skills/review-code/[A-Za-z0-9._/-]*##g' \ + -e 's#~/\.agents/skills/review-code/scripts/review-file-path\.sh([^A-Za-z0-9._/-]|$)#\1#g' \ "$skill_file") while IFS= read -r ref; do [[ -z "$ref" || -e "$skill_dir/$ref" ]] && continue