Skip to content

CNTRLPLANE-3677: fix(openshift-developer): use CLAUDE_PLUGIN_ROOT with fallback for check_replied.py path - #588

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift-eng:mainfrom
enxebre:fix/address-review-pr-check-replied-path
Jun 30, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
openshift-eng:mainfrom
enxebre:fix/address-review-pr-check-replied-path

Conversation

@enxebre

@enxebre enxebre commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix address-review-pr skill failing with No such file or directory for check_replied.py in CI and non-standard plugin install locations
  • Use CLAUDE_PLUGIN_ROOT env var as primary path, with fallback search across ~/.claude/plugins and /tmp
  • Mirrors the fix pattern established in PR fix(utils): use absolute path for check_replied.py in address-reviews #315 for the utils plugin

Test plan

  • Run /openshift-developer:address-review-pr in a standard Claude Code environment
  • Run in CI where plugins install under /tmp

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved duplicate-reply detection by directly invoking the duplicate-prevention script, avoiding failures caused by dynamic script discovery.
  • Chores
    • Updated the OpenShift Developer plugin version to 1.1.6 in the embedded marketplace data and plugin metadata.

@openshift-ci
openshift-ci Bot requested review from bentito and stbenjam June 29, 2026 21:54
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 29, 2026
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c6405437-f04d-42f1-8eab-7fcbe0efcbbd

📥 Commits

Reviewing files that changed from the base of the PR and between 656d5ac and 959784e.

📒 Files selected for processing (5)
  • .claude-plugin/marketplace.json
  • docs/index.html
  • plugins/openshift-developer/.claude-plugin/plugin.json
  • plugins/openshift-developer/skills/address-review-pr/SKILL.md
  • plugins/openshift-developer/skills/address-review-pr/scripts/check_replied.py
✅ Files skipped from review due to trivial changes (3)
  • plugins/openshift-developer/.claude-plugin/plugin.json
  • .claude-plugin/marketplace.json
  • docs/index.html

Walkthrough

The openshift-developer plugin version is bumped from 1.1.5 to 1.1.6 in the plugin manifest, marketplace data, and embedded docs. The address-review-pr skill also changes how it invokes check_replied.py for duplicate prevention.

Changes

openshift-developer updates

Layer / File(s) Summary
Version metadata bump
plugins/openshift-developer/.claude-plugin/plugin.json, .claude-plugin/marketplace.json, docs/index.html
Updates the openshift-developer version field from 1.1.5 to 1.1.6 in the plugin manifest, marketplace entry, and embedded docs data.
Duplicate-prevention command target
plugins/openshift-developer/skills/address-review-pr/SKILL.md
Changes the duplicate-prevention step to run check_replied.py directly from ${CLAUDE_SKILL_DIR}/scripts/check_replied.py instead of locating it with a find-based path lookup.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested labels

lgtm, ok-to-test

Suggested reviewers

  • theobarberbany
  • cblecker

Important

Pre-merge checks failed

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

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
No Assumed Git Remote Names ❌ Error SKILL.md still hardcodes upstream/origin in rebase and push verification logic, including BASE_REMOTE:-origin and git ls-remote origin. Replace literal remote names with discovered remotes/tracking branches and avoid origin/upstream in commands or docs unless they were first determined.
Git Push Safety Rules ❌ Error SKILL.md still instructs an unconditional git push in Step 4b, with no explicit user-confirmation gate before pushing. Add an explicit ask-for-approval step before any push, or remove push automation from the skill entirely. Keep main/master and force-push blocked.
✅ Passed checks (8 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main functional change to check_replied.py path resolution in the openshift-developer skill.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Real People Names In Style References ✅ Passed Touched plugin/docs files only update versions and path resolution; no real-person names or style-reference examples appear in commands or skill text.
No Untrusted Mcp Servers ✅ Passed PR only bumps versions and changes a local script path; no new MCP server installs or dependencies were added in the touched files.
Ai-Helpers Overlap Detection ✅ Passed PASS: The only open PR touching address-review-pr/SKILL.md is PR #588 itself; no other open PR shows semantically similar titles with overlapping paths.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/openshift-developer/skills/address-review-pr/SKILL.md`:
- Around line 230-235: The fallback in the address-review-pr skill currently
searches /tmp for check_replied.py and then executes the first match, which can
run an untrusted script. Update the CHECK_REPLIED lookup logic to only use
trusted plugin locations, preferably under CLAUDE_PLUGIN_ROOT or other
user-owned plugin roots, and remove /tmp from the search path before the python3
invocation. Keep the existing validation that the script exists before running
it, but ensure the resolved path cannot come from a world-writable directory.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b626000f-9172-4c92-9760-f0bf98f0bc3e

📥 Commits

Reviewing files that changed from the base of the PR and between efcdc26 and 656d5ac.

📒 Files selected for processing (4)
  • .claude-plugin/marketplace.json
  • docs/index.html
  • plugins/openshift-developer/.claude-plugin/plugin.json
  • plugins/openshift-developer/skills/address-review-pr/SKILL.md

Comment thread plugins/openshift-developer/skills/address-review-pr/SKILL.md Outdated
```bash
SKILL_DIR="$(dirname "$(find ~/.claude/plugins -type f -path "*/openshift-developer/skills/address-review-pr/check_replied.py" 2>/dev/null | sort | head -1)")"
python3 "$SKILL_DIR/check_replied.py" <owner> <repo> <pr_number> <comment_id> --type <type>
CHECK_REPLIED="${CLAUDE_PLUGIN_ROOT}/skills/address-review-pr/check_replied.py"

@stbenjam stbenjam Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, Claude fixed this bug some time ago, this shouldn't be needed anymore. Did you try putting the file in scripts/? It is where the agentskills spec asks them to go, it shouldn't be in the skill root.

@enxebre enxebre Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Occasional error is

  Error: Exit code 2
  python3: can't open file '/tmp/hypershift/./check_replied.py': [Errno 2] No such file or directory

Updated according to https://code.claude.com/docs/en/skills#generate-visual-output

Move check_replied.py into the skill's scripts/ subdirectory and
reference it via ${CLAUDE_SKILL_DIR}, which resolves correctly
regardless of install location.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@enxebre
enxebre force-pushed the fix/address-review-pr-check-replied-path branch from 2e08116 to 959784e Compare June 30, 2026 07:18
@stbenjam

Copy link
Copy Markdown
Member

/lgtm

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

openshift-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre, stbenjam

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

@stbenjam

Copy link
Copy Markdown
Member

I was hoping it would work without reference to Claude-specific paths. I think I usually just tell it to run <script name> and it finds it, but if it doesn't work this is fine

@openshift-merge-bot
openshift-merge-bot Bot merged commit 5f26b20 into openshift-eng:main Jun 30, 2026
6 checks passed
@enxebre enxebre changed the title fix(openshift-developer): use CLAUDE_PLUGIN_ROOT with fallback for check_replied.py path CNTRLPLANE-3677: fix(openshift-developer): use CLAUDE_PLUGIN_ROOT with fallback for check_replied.py path Jun 30, 2026
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