Skip to content

fix(utils): use absolute path for check_replied.py in address-reviews - #315

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
bryan-cox:fix/check-replied-script-path
Feb 6, 2026
Merged

openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
bryan-cox:fix/check-replied-script-path

Conversation

@bryan-cox

@bryan-cox bryan-cox commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The check_replied.py script path in the address-reviews skill was relative (plugins/utils/scripts/check_replied.py), which only resolves when the working directory contains a plugins/ directory
  • In practice, the skill runs from whatever project the user is working in, so the script is never found (exit code 2)
  • When the script fails, the agent proceeds without the duplicate check, leading to duplicate replies (e.g. 4 duplicate bot replies to a single nit comment)

Changes

  1. Use find ~ -name "check_replied.py" -path "*/utils/scripts/*" to dynamically locate the script regardless of install method or location (marketplace, manual clone, symlink, etc.)
  2. Explicitly error when the script cannot be found instead of silently continuing
  3. Instruct the agent to stop and investigate on exit code 2 rather than proceeding without the duplicate check

Test plan

  • Run /utils:address-reviews on a PR in a non-ai-helpers repo and verify the check_replied.py script is found and runs correctly
  • Verify linter passes (make lint)
  • Verify version bump is correct (0.0.4 → 0.0.5)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Bumped utils plugin version to 0.0.5.
  • Documentation

    • Clarified address-review guidance: validation now dynamically locates the check, explicitly prevents posting on failed validation and asks users to investigate, while preserving the existing "already replied" skip behavior.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2026
…ss-reviews

The script path was relative (`plugins/utils/scripts/check_replied.py`),
which only resolves when the working directory contains a plugins/
directory. In practice, the skill runs from whatever project the user is
working in, so the script is never found (exit code 2).

Use `find ~/.claude/plugins` to dynamically locate the script regardless
of whether the plugin is installed via marketplace or as a local plugin.
Also explicitly error when the script cannot be found, and instruct the
agent to stop and investigate on exit code 2 rather than proceeding
without the duplicate check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bryan-cox
bryan-cox force-pushed the fix/check-replied-script-path branch from f1ea125 to d442f3b Compare February 6, 2026 16:13
@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Version bump for the utils plugin from 0.0.4 to 0.0.5 in manifests and docs, plus replacement of hardcoded check_replied.py calls in the address-reviews command with dynamic discovery and explicit handling of a new exit code 2 (abort on checker failure).

Changes

Cohort / File(s) Summary
Version Bump
​.claude-plugin/marketplace.json, docs/data.json, plugins/utils/.claude-plugin/plugin.json
Updated the utils plugin version value from 0.0.4 to 0.0.5 in registry, docs, and the plugin manifest.
Script Invocation Enhancement
plugins/utils/commands/address-reviews.md
Replaced hardcoded python3 plugins/utils/scripts/check_replied.py with dynamic discovery of the script under the user home (*/utils/scripts/*), invoke via python3 "$CHECK_REPLIED", and add handling for exit code 2 (abort when checker not found/failed); preserved existing exit code 1 behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)
Check name Status Explanation Resolution
Git Push Safety Rules ❌ Error Pull request contains force push operation (git push --force-with-lease) on line 143 without user confirmation, violating Git Push Safety Rules #1 and #3. Replace force push with standard git push and add explicit user confirmation step before any push operation.
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and specifically describes the main change: fixing the use of absolute paths for check_replied.py in the address-reviews documentation file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
No Real People Names In Style References ✅ Passed No real people's names found in plugin commands, skill documentation, or style references within the modified files.
No Assumed Git Remote Names ✅ Passed The pull request does not introduce any hardcoded git remote name assumptions. Modifications are limited to duplicate prevention section using dynamic discovery without referencing git remote names.
No Untrusted Mcp Servers ✅ Passed The pull request contains only version bumps and a fix to address-reviews.md using shell find commands; no MCP server installations from any sources.
Ai-Helpers Overlap Detection ✅ Passed PR contains targeted bug fix for address-reviews command with no new functionality or overlap with existing ai-helpers content or other open PRs.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@bryan-cox

Copy link
Copy Markdown
Contributor Author

Updated to use find ~ -name "check_replied.py" -path "*/utils/scripts/*" instead of searching ~/.claude/plugins directly. This resolves the script relative to wherever it exists under the user's home directory, so it works regardless of install method or location.

Note: I initially tried using ${CLAUDE_PLUGIN_ROOT} (the officially documented Claude Code env var for plugin path resolution), which is the cleanest approach and already used by git, workspaces, and metrics plugins in this repo. However, there's a known bug where CLAUDE_PLUGIN_ROOT doesn't get set when executing bash from command markdown files — it only works in JSON configs (hooks, MCP, LSP). Until that's fixed upstream, the find ~ approach is the most portable option.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@plugins/utils/commands/address-reviews.md`:
- Around line 196-199: The current FIND pipeline that sets CHECK_REPLIED by
scanning the entire home tree for check_replied.py is unsafe and
non-deterministic; change the search to only probe known plugin install roots
(the trusted plugin install directories), use find with constrained depth and
-type f (and -executable if appropriate), pipe through sort for determinism, and
then assign the first match to CHECK_REPLIED; ensure the subsequent python3
"$CHECK_REPLIED" <owner> <repo> <pr_number> <comment_id> --type <type> still
runs the selected file and fail with a clear error if no match is found.

Comment thread plugins/utils/commands/address-reviews.md
… path

Try CLAUDE_PLUGIN_ROOT first (future-proof for when Claude Code fixes
the env var in markdown commands), then fall back to searching
~/.claude/plugins with constrained scope. Errors explicitly when the
script cannot be found.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bryan-cox
bryan-cox force-pushed the fix/check-replied-script-path branch from f7f32b8 to ca7b7ca Compare February 6, 2026 18:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@plugins/utils/commands/address-reviews.md`:
- Around line 196-201: The fallback find logic for locating check_replied.py
(variable CHECK_REPLIED) currently only searches under ~/.claude/plugins; update
the fallback to also search Cursor installs by adding a second find path that
looks for ~/.cursor/commands/*/plugins/utils/scripts/check_replied.py (or
include both find patterns in one command) so that if the first path isn't
present the script can be found under Cursor's install layout; ensure the final
existence check ([ -z "$CHECK_REPLIED" ] || [ ! -f "$CHECK_REPLIED" ]) and the
error/exit behavior remain unchanged and still reference CHECK_REPLIED and the
target filename check_replied.py.

Comment thread plugins/utils/commands/address-reviews.md
@bryan-cox

Copy link
Copy Markdown
Contributor Author

@celebdor — addressing your concern about plugins not necessarily living at ~/.claude/plugins:

The latest commit (d442f3b) uses CLAUDE_PLUGIN_ROOT as the primary lookup path. This is a Claude Code env var that resolves to the actual install directory of the plugin at runtime, so it works regardless of where the plugin is installed. If CLAUDE_PLUGIN_ROOT is set and the script exists there, no find is needed.

The find ~/.claude/plugins fallback exists because of an upstream bug (anthropics/claude-code#9354) where CLAUDE_PLUGIN_ROOT is not populated when bash is executed from command markdown files — it only works in JSON-based configs (hooks, MCP, LSP). Once that bug is fixed, the fallback becomes dead code.

This was tested end-to-end by running /utils:address-reviews from a hypershift worktree (not the ai-helpers repo) against PR #7654 — it successfully located check_replied.py and posted replies without duplicates.

One remaining gap: the CI job (review-agent in openshift/release) doesn't use the plugin system at all, so neither CLAUDE_PLUGIN_ROOT nor ~/.claude/plugins applies there. A companion change to set CLAUDE_PLUGIN_ROOT explicitly in the CI job config is being prepared separately.

@celebdor

celebdor commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2026
@openshift-ci

openshift-ci Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, celebdor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit d05efe2 into openshift-eng:main Feb 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants