Add docs-cherry-pick and docs-branch-audit skills#73
Closed
slovern wants to merge 1 commit intoredhat-documentation:mainfrom
Closed
Add docs-cherry-pick and docs-branch-audit skills#73slovern wants to merge 1 commit intoredhat-documentation:mainfrom
slovern wants to merge 1 commit intoredhat-documentation:mainfrom
Conversation
Adds two new skills to docs-tools: - docs-cherry-pick: Intelligently cherry-pick documentation changes across branches - docs-branch-audit: Audit which files from a PR, commit, or branch exist in target branches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis PR adds two new documentation skills to the docs-tools plugin: Changes
Sequence DiagramssequenceDiagram
actor User
participant CherryPickScript as cherry_pick.sh
participant GitCLI as Git CLI
participant GitHubAPI as GitHub API
participant AuditScript as branch_audit.sh
participant DeepAuditScript as deep_audit.sh
User->>CherryPickScript: --pr <url> --target <branches>
rect rgba(100, 150, 200, 0.5)
Note over CherryPickScript: Validate Phase
CherryPickScript->>GitHubAPI: Fetch PR metadata & changed files
GitHubAPI-->>CherryPickScript: PR data, commit SHA, file list
CherryPickScript->>GitCLI: Verify repo & commit
CherryPickScript->>CherryPickScript: Store state (commit, branches, files)
end
rect rgba(100, 200, 150, 0.5)
Note over CherryPickScript: Audit Phase
CherryPickScript->>AuditScript: Run branch_audit.sh
AuditScript->>GitCLI: Fetch branches, ls-tree, cat-file checks
AuditScript-->>CherryPickScript: Include/exclude lists per branch
alt if --deep flag set
CherryPickScript->>DeepAuditScript: Run deep_audit.sh
DeepAuditScript->>GitCLI: Compute diffs, count directives
DeepAuditScript-->>CherryPickScript: Confidence levels & issues
end
end
rect rgba(200, 150, 100, 0.5)
Note over CherryPickScript: Apply Phase
loop For each target branch
CherryPickScript->>GitCLI: Fetch branch, create backport branch
CherryPickScript->>GitCLI: Cherry-pick commit
CherryPickScript->>GitCLI: Remove/unstage excluded files
alt Clean cherry-pick
CherryPickScript->>GitCLI: Auto-commit
else Conflicts detected
CherryPickScript-->>User: Exit code 2 + conflict files
User->>User: Resolve conflicts (agent-based)
end
end
end
rect rgba(150, 200, 100, 0.5)
Note over CherryPickScript: Push Phase
CherryPickScript->>CherryPickScript: Generate PR description
CherryPickScript->>GitCLI: Push backport branch to origin
CherryPickScript-->>User: Success / PR links
end
sequenceDiagram
actor User
participant BranchAuditScript as branch_audit.sh
participant GitCLI as Git CLI
participant GitHubAPI as GitHub API
participant DeepAuditScript as deep_audit.sh
User->>BranchAuditScript: --pr <url> --branches <list> [--deep]
rect rgba(100, 150, 200, 0.5)
Note over BranchAuditScript: Extract Source Files
alt PR mode
BranchAuditScript->>GitHubAPI: Fetch PR files
GitHubAPI-->>BranchAuditScript: File list
else Commit mode
BranchAuditScript->>GitCLI: git diff-tree <commit>
GitCLI-->>BranchAuditScript: Changed files
end
BranchAuditScript->>BranchAuditScript: Filter for .adoc files
end
rect rgba(100, 200, 150, 0.5)
Note over BranchAuditScript: Audit Per-Branch
loop For each target branch
BranchAuditScript->>GitCLI: Fetch branch refs
BranchAuditScript->>GitCLI: Build index (git ls-tree)
loop For each source file
BranchAuditScript->>GitCLI: Check if file exists (git cat-file -e)
alt File found
BranchAuditScript->>BranchAuditScript: Add to includes
else Not found
BranchAuditScript->>BranchAuditScript: Fuzzy match basename
alt Match found
BranchAuditScript->>BranchAuditScript: Add to renamed/includes
else No match
BranchAuditScript->>BranchAuditScript: Add to excludes
end
end
end
end
end
rect rgba(200, 150, 100, 0.5)
Note over BranchAuditScript: Deep Audit (if enabled)
alt if --deep flag set
loop For each target branch
BranchAuditScript->>DeepAuditScript: Run with included files
DeepAuditScript->>GitCLI: Compute branch diffs
DeepAuditScript->>GitCLI: Analyze version constructs
DeepAuditScript->>GitCLI: Count structural elements
DeepAuditScript-->>BranchAuditScript: Confidence & issues
end
end
end
rect rgba(150, 200, 100, 0.5)
Note over BranchAuditScript: Output Results
BranchAuditScript-->>User: Text report (per-branch includes/excludes)
BranchAuditScript-->>User: JSON output (if --json)
BranchAuditScript-->>User: Deep audit summaries (if --deep)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
plugins/docs-tools/.claude-plugin/plugin.json (1)
3-3: Consider semantic versioning for new features.Adding two new skills (
docs-cherry-pickanddocs-branch-audit) introduces new functionality, which typically warrants a MINOR version bump (0.0.19 → 0.1.0) rather than a PATCH bump (0.0.19 → 0.0.20) according to semantic versioning conventions. PATCH increments are typically reserved for bug fixes, while MINOR increments signal new backward-compatible features.While the project is in pre-1.0 development where conventions may be more flexible, following semantic versioning helps users understand the nature of changes at a glance.
📦 Suggested semantic version adjustment
- "version": "0.0.20", + "version": "0.1.0",Based on learnings: "Use semantic versioning (MAJOR.MINOR.PATCH) for the version field in plugin.json only"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/.claude-plugin/plugin.json` at line 3, Update the "version" field in plugin.json to follow semantic versioning (MAJOR.MINOR.PATCH) since you added new backward-compatible features: bump from "0.0.20" to "0.1.0" (or appropriate MINOR increase) to reflect the new skills docs-cherry-pick and docs-branch-audit; locate the "version" property in plugins/docs-tools/.claude-plugin/plugin.json and replace the patch-only bump with a MINOR increment so consumers can see feature-level changes.plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh (2)
16-16: Cross-skill script reference should use${CLAUDE_PLUGIN_ROOT}.The reference to
git-pr-readeruses a computed relative path. As per coding guidelines, cross-skill script calls must use${CLAUDE_PLUGIN_ROOT}for consistency and reliability.♻️ Proposed fix
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SKILL_DIR="$(dirname "$SCRIPT_DIR")" -PLUGIN_DIR="$(dirname "$(dirname "$SKILL_DIR")")" -GIT_REVIEW_API="${PLUGIN_DIR}/skills/git-pr-reader/scripts/git_pr_reader.py" +GIT_REVIEW_API="${CLAUDE_PLUGIN_ROOT}/skills/git-pr-reader/scripts/git_pr_reader.py"As per coding guidelines: "Cross-skill and cross-command script calls must use
${CLAUDE_PLUGIN_ROOT}for paths."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh` at line 16, The GIT_REVIEW_API variable uses a relative PLUGIN_DIR path instead of the required cross-skill root; update the assignment of GIT_REVIEW_API to reference the cross-skill root by replacing ${PLUGIN_DIR} with ${CLAUDE_PLUGIN_ROOT} so the path becomes ${CLAUDE_PLUGIN_ROOT}/skills/git-pr-reader/scripts/git_pr_reader.py, ensuring all cross-skill script references follow the `${CLAUDE_PLUGIN_ROOT}` convention.
107-107: Clarify environment variable unsetting syntax.The
GITHUB_TOKEN= GH_TOKEN=syntax is intentional to unset tokens for this command, but shellcheck flags it as confusing. Usingenv -uor explicit empty assignment makes intent clearer.♻️ Proposed fix for clarity
- if GITHUB_TOKEN= GH_TOKEN= gh api "repos/${OWNER_REPO}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename' 2>/dev/null | grep '\.adoc$' > "$CANDIDATE_FILES" && [[ -s "$CANDIDATE_FILES" ]]; then + if env -u GITHUB_TOKEN -u GH_TOKEN gh api "repos/${OWNER_REPO}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename' 2>/dev/null | grep '\.adoc$' > "$CANDIDATE_FILES" && [[ -s "$CANDIDATE_FILES" ]]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh` at line 107, The inline token-unsetting syntax in the gh API invocation (GITHUB_TOKEN= GH_TOKEN= gh api "repos/${OWNER_REPO}/pulls/${PR_NUMBER}/files" ...) is flagged as confusing; update the call in branch_audit.sh to explicitly unset tokens using a clearer form (e.g., use env -u GITHUB_TOKEN -u GH_TOKEN or assign explicit empty values via env) before invoking gh api, and ensure the command still writes filenames to "$CANDIDATE_FILES" and preserves the --paginate --jq '.[].filename' 2>/dev/null | grep '\.adoc$' && [[ -s "$CANDIDATE_FILES" ]] behavior.plugins/docs-tools/skills/docs-branch-audit/SKILL.md (2)
59-75: Add language identifier to fenced code block.The code block showing text output format should have a language identifier for proper rendering and linting compliance.
♻️ Proposed fix
### Text format (default) -``` +```text === Branch Audit: enterprise-4.17 ===🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-branch-audit/SKILL.md` around lines 59 - 75, The fenced code block under the "Text format (default)" section is missing a language identifier; update the opening triple-backtick for that block so it reads ```text to enable correct rendering and linting (i.e., modify the fenced code block that contains "=== Branch Audit: enterprise-4.17 ===" to include the "text" language identifier).
16-43: Script path variable should be${CLAUDE_SKILL_DIR}in skill documentation.Skill documentation should use
${CLAUDE_SKILL_DIR}(the standard skill directory variable) rather than${SKILL_DIR}for consistency with other skills and Claude's execution environment.♻️ Proposed fix
## Usage ```bash # Audit a PR against target branches -bash ${SKILL_DIR}/scripts/branch_audit.sh \ +bash ${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh \ --pr https://github.com/openshift/openshift-docs/pull/106280 \ --branches enterprise-4.17,enterprise-4.16 # Audit a commit -bash ${SKILL_DIR}/scripts/branch_audit.sh \ +bash ${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh \ --commit abc123def \ --branches enterprise-4.17 # Audit from a file list -bash ${SKILL_DIR}/scripts/branch_audit.sh \ +bash ${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh \ --files /tmp/files-to-check.txt \ --branches enterprise-4.17,enterprise-4.16 # JSON output for programmatic use -bash ${SKILL_DIR}/scripts/branch_audit.sh \ +bash ${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh \ --pr https://github.com/openshift/openshift-docs/pull/106280 \ --branches enterprise-4.17 \ --json # Deep audit — check content compatibility for included files -bash ${SKILL_DIR}/scripts/branch_audit.sh \ +bash ${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh \ --pr https://github.com/openshift/openshift-docs/pull/106280 \ --branches enterprise-4.17 \ --deepAs per coding guidelines: "When a skill's own Markdown calls its co-located script, use a relative path from the skill directory."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-branch-audit/SKILL.md` around lines 16 - 43, Replace the incorrect `${SKILL_DIR}` variable with the standard `${CLAUDE_SKILL_DIR}` in the SKILL.md examples that call the co-located script; specifically update every occurrence of `${SKILL_DIR}/scripts/branch_audit.sh` to `${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh` (affecting the PR, commit, files, --json, and --deep examples) while preserving the existing command formatting and line-continuation backslashes so the examples remain runnable.plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh (3)
482-490: Remove unused variables.
source_file_count(line 482) andbackport_stats(line 486) are computed but never used in the script.♻️ Proposed fix
- source_file_count=$(wc -l < "$STATE_DIR/source-files.txt") - # Diff stats comparison - local backport_stats - backport_stats=$(git diff --stat "upstream/${target_branch}...HEAD" 2>/dev/null || echo "") local backport_file_count backport_insertions backport_deletions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh` around lines 482 - 490, The script computes variables source_file_count and backport_stats but never uses them; remove the unused assignments to clean up the code and avoid unnecessary subshells. Specifically, delete or comment out the lines that set source_file_count=$(wc -l < "$STATE_DIR/source-files.txt") and backport_stats=$(git diff --stat "upstream/${target_branch}...HEAD" 2>/dev/null || echo ""), leaving the remaining backport_* computations (backport_file_count, backport_insertions, backport_deletions) intact so diff statistics still work.
28-29: Cross-skill script references should use${CLAUDE_PLUGIN_ROOT}.References to
docs-branch-auditandgit-pr-readerskills use computed relative paths. Cross-skill calls must use${CLAUDE_PLUGIN_ROOT}per coding guidelines.♻️ Proposed fix
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SKILL_DIR="$(dirname "$SCRIPT_DIR")" -PLUGIN_DIR="$(dirname "$(dirname "$SKILL_DIR")")" -BRANCH_AUDIT="${PLUGIN_DIR}/skills/docs-branch-audit/scripts/branch_audit.sh" -GIT_PR_READER="${PLUGIN_DIR}/skills/git-pr-reader/scripts/git_pr_reader.py" +BRANCH_AUDIT="${CLAUDE_PLUGIN_ROOT}/skills/docs-branch-audit/scripts/branch_audit.sh" +GIT_PR_READER="${CLAUDE_PLUGIN_ROOT}/skills/git-pr-reader/scripts/git_pr_reader.py"As per coding guidelines: "Cross-skill and cross-command script calls must use
${CLAUDE_PLUGIN_ROOT}for paths."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh` around lines 28 - 29, The BRANCH_AUDIT and GIT_PR_READER variables are using PLUGIN_DIR-based relative paths; update their assignments to reference the cross-skill root variable `${CLAUDE_PLUGIN_ROOT}` instead of `${PLUGIN_DIR}` so cross-skill script calls follow the guideline (locate the BRANCH_AUDIT and GIT_PR_READER variable definitions in cherry_pick.sh and change the path prefixes to use `${CLAUDE_PLUGIN_ROOT}` when constructing the paths to skills/docs-branch-audit/scripts/branch_audit.sh and skills/git-pr-reader/scripts/git_pr_reader.py).
297-297: Use explicittruefor file truncation.While
> fileworks in bash, shellcheck flags it as unclear. Usingtrue > fileor: > filemakes the intent explicit.♻️ Proposed fix
- > "$STATE_DIR/path-diffs.txt" # clear + : > "$STATE_DIR/path-diffs.txt" # clear🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh` at line 297, Replace the ambiguous file-truncation redirection in the cherry_pick.sh script by using an explicit no-op command before the redirect; change the line that currently does > "$STATE_DIR/path-diffs.txt" to use a clear truncation idiom like : > "$STATE_DIR/path-diffs.txt" (or true > "$STATE_DIR/path-diffs.txt") so the intent is explicit; locate the statement in cherry_pick.sh referencing the STATE_DIR variable and path-diffs.txt and update it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh`:
- Around line 146-159: The loop in branch_audit.sh currently assumes remotes
"upstream" and "origin" when checking branches (see BRANCH_ARRAY handling and
the git rev-parse / git fetch calls); change this to discover remotes first and
use the discovered remote for each branch: run git branch -vv to parse the
tracking remote for each branch (falling back to parsing git remote -v if no
tracking info), set a variable like BRANCH_REMOTE for the branch, then replace
hardcoded "upstream/$branch" and "origin/$branch" with attempts to git rev-parse
and git fetch against the discovered BRANCH_REMOTE (and as a final fallback
iterate available remotes from git remote -v), preserving the existing error
messages and continue behavior.
In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh`:
- Around line 204-212: The script currently hardcodes the git remote name
"upstream" when fetching PR/MR refs (see usage around COMMIT_SHA, pr_number and
platform handling in cherry_pick.sh), which violates guidelines; add a remote
discovery helper (e.g., function discover_remote_or_throw or
get_remote_for_upstream) that inspects `git remote -v` and `git branch -vv` to
find the correct remote to use, and replace all hardcoded "upstream" occurrences
(including the fetch commands used for GitHub pull/${pr_number}/head and GitLab
merge-requests/${pr_number}/head and any other fetch/push operations later in
the script) to use the discovered remote variable, falling back to a clear error
if none found. Ensure the new helper is called before any git fetch/push and
that variables like COMMIT_SHA, pr_number, and platform use the discovered
remote name instead of the literal "upstream."
- Around line 567-574: The script currently pushes by default using the NO_PUSH
variable and git push -u origin "$branch_name"; change this so pushing only
occurs with explicit user permission by either (A) flipping the default to not
push and inverting the flag (introduce a --push flag/PUSH variable and require
PUSH=true to run git push), or (B) keep the --no-push flag but add an
interactive confirmation prompt before running git push (prompt the user, read
response, and only run git push when they answer yes). Update the condition
around git push (the block using NO_PUSH and git push -u origin "$branch_name")
and the flag parsing logic so that push is never performed implicitly.
---
Nitpick comments:
In `@plugins/docs-tools/.claude-plugin/plugin.json`:
- Line 3: Update the "version" field in plugin.json to follow semantic
versioning (MAJOR.MINOR.PATCH) since you added new backward-compatible features:
bump from "0.0.20" to "0.1.0" (or appropriate MINOR increase) to reflect the new
skills docs-cherry-pick and docs-branch-audit; locate the "version" property in
plugins/docs-tools/.claude-plugin/plugin.json and replace the patch-only bump
with a MINOR increment so consumers can see feature-level changes.
In `@plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh`:
- Line 16: The GIT_REVIEW_API variable uses a relative PLUGIN_DIR path instead
of the required cross-skill root; update the assignment of GIT_REVIEW_API to
reference the cross-skill root by replacing ${PLUGIN_DIR} with
${CLAUDE_PLUGIN_ROOT} so the path becomes
${CLAUDE_PLUGIN_ROOT}/skills/git-pr-reader/scripts/git_pr_reader.py, ensuring
all cross-skill script references follow the `${CLAUDE_PLUGIN_ROOT}` convention.
- Line 107: The inline token-unsetting syntax in the gh API invocation
(GITHUB_TOKEN= GH_TOKEN= gh api "repos/${OWNER_REPO}/pulls/${PR_NUMBER}/files"
...) is flagged as confusing; update the call in branch_audit.sh to explicitly
unset tokens using a clearer form (e.g., use env -u GITHUB_TOKEN -u GH_TOKEN or
assign explicit empty values via env) before invoking gh api, and ensure the
command still writes filenames to "$CANDIDATE_FILES" and preserves the
--paginate --jq '.[].filename' 2>/dev/null | grep '\.adoc$' && [[ -s
"$CANDIDATE_FILES" ]] behavior.
In `@plugins/docs-tools/skills/docs-branch-audit/SKILL.md`:
- Around line 59-75: The fenced code block under the "Text format (default)"
section is missing a language identifier; update the opening triple-backtick for
that block so it reads ```text to enable correct rendering and linting (i.e.,
modify the fenced code block that contains "=== Branch Audit: enterprise-4.17
===" to include the "text" language identifier).
- Around line 16-43: Replace the incorrect `${SKILL_DIR}` variable with the
standard `${CLAUDE_SKILL_DIR}` in the SKILL.md examples that call the co-located
script; specifically update every occurrence of
`${SKILL_DIR}/scripts/branch_audit.sh` to
`${CLAUDE_SKILL_DIR}/scripts/branch_audit.sh` (affecting the PR, commit, files,
--json, and --deep examples) while preserving the existing command formatting
and line-continuation backslashes so the examples remain runnable.
In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh`:
- Around line 482-490: The script computes variables source_file_count and
backport_stats but never uses them; remove the unused assignments to clean up
the code and avoid unnecessary subshells. Specifically, delete or comment out
the lines that set source_file_count=$(wc -l < "$STATE_DIR/source-files.txt")
and backport_stats=$(git diff --stat "upstream/${target_branch}...HEAD"
2>/dev/null || echo ""), leaving the remaining backport_* computations
(backport_file_count, backport_insertions, backport_deletions) intact so diff
statistics still work.
- Around line 28-29: The BRANCH_AUDIT and GIT_PR_READER variables are using
PLUGIN_DIR-based relative paths; update their assignments to reference the
cross-skill root variable `${CLAUDE_PLUGIN_ROOT}` instead of `${PLUGIN_DIR}` so
cross-skill script calls follow the guideline (locate the BRANCH_AUDIT and
GIT_PR_READER variable definitions in cherry_pick.sh and change the path
prefixes to use `${CLAUDE_PLUGIN_ROOT}` when constructing the paths to
skills/docs-branch-audit/scripts/branch_audit.sh and
skills/git-pr-reader/scripts/git_pr_reader.py).
- Line 297: Replace the ambiguous file-truncation redirection in the
cherry_pick.sh script by using an explicit no-op command before the redirect;
change the line that currently does > "$STATE_DIR/path-diffs.txt" to use a clear
truncation idiom like : > "$STATE_DIR/path-diffs.txt" (or true >
"$STATE_DIR/path-diffs.txt") so the intent is explicit; locate the statement in
cherry_pick.sh referencing the STATE_DIR variable and path-diffs.txt and update
it accordingly.
🪄 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: Pro
Run ID: 61b29187-171a-416f-933d-8eead1a83b97
📒 Files selected for processing (6)
plugins/docs-tools/.claude-plugin/plugin.jsonplugins/docs-tools/skills/docs-branch-audit/SKILL.mdplugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.shplugins/docs-tools/skills/docs-branch-audit/scripts/deep_audit.shplugins/docs-tools/skills/docs-cherry-pick/SKILL.mdplugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh
Comment on lines
+146
to
+159
| for branch in "${BRANCH_ARRAY[@]}"; do | ||
| branch=$(echo "$branch" | xargs) # trim whitespace | ||
| if ! git rev-parse --verify "remotes/upstream/$branch" >/dev/null 2>&1; then | ||
| echo "Fetching upstream/$branch..." >&2 | ||
| git fetch upstream "$branch" 2>/dev/null || { | ||
| echo "WARNING: Could not fetch upstream/$branch — trying origin/$branch" >&2 | ||
| git fetch origin "$branch" 2>/dev/null || { | ||
| echo "ERROR: Branch $branch not found on upstream or origin" >&2 | ||
| continue | ||
| } | ||
| } | ||
| fi | ||
| done | ||
| fi |
There was a problem hiding this comment.
Hardcoded remote names violate coding guidelines.
The script assumes upstream and origin exist without first discovering available remotes. Per coding guidelines, scripts must discover remotes using git remote -v and git branch -vv.
🔧 Proposed fix - add remote discovery
+# Discover remotes
+UPSTREAM_REMOTE=""
+ORIGIN_REMOTE=""
+while IFS= read -r line; do
+ remote_name=$(echo "$line" | awk '{print $1}')
+ remote_url=$(echo "$line" | awk '{print $2}')
+ if echo "$remote_url" | grep -qE '(openshift|redhat).*docs'; then
+ if [[ -z "$UPSTREAM_REMOTE" ]]; then
+ UPSTREAM_REMOTE="$remote_name"
+ fi
+ fi
+ if [[ -z "$ORIGIN_REMOTE" ]]; then
+ ORIGIN_REMOTE="$remote_name"
+ fi
+done < <(git remote -v | grep fetch)
+
+if [[ -z "$UPSTREAM_REMOTE" ]]; then
+ echo "ERROR: Could not determine upstream remote" >&2
+ exit 1
+fi
+
# Step 2: Fetch remote branches if needed
if [[ "$DRY_RUN" = false ]]; then
IFS=',' read -ra BRANCH_ARRAY <<< "$BRANCHES"
for branch in "${BRANCH_ARRAY[@]}"; do
branch=$(echo "$branch" | xargs) # trim whitespace
- if ! git rev-parse --verify "remotes/upstream/$branch" >/dev/null 2>&1; then
- echo "Fetching upstream/$branch..." >&2
- git fetch upstream "$branch" 2>/dev/null || {
- echo "WARNING: Could not fetch upstream/$branch — trying origin/$branch" >&2
- git fetch origin "$branch" 2>/dev/null || {
+ if ! git rev-parse --verify "remotes/${UPSTREAM_REMOTE}/$branch" >/dev/null 2>&1; then
+ echo "Fetching ${UPSTREAM_REMOTE}/$branch..." >&2
+ git fetch "$UPSTREAM_REMOTE" "$branch" 2>/dev/null || {
+ echo "WARNING: Could not fetch ${UPSTREAM_REMOTE}/$branch — trying ${ORIGIN_REMOTE}/$branch" >&2
+ git fetch "$ORIGIN_REMOTE" "$branch" 2>/dev/null || {
echo "ERROR: Branch $branch not found on upstream or origin" >&2
continue
}As per coding guidelines: "Code should NEVER assume git remote names like 'origin' or 'upstream' without first discovering them using git remote -v and git branch -vv."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/docs-branch-audit/scripts/branch_audit.sh` around
lines 146 - 159, The loop in branch_audit.sh currently assumes remotes
"upstream" and "origin" when checking branches (see BRANCH_ARRAY handling and
the git rev-parse / git fetch calls); change this to discover remotes first and
use the discovered remote for each branch: run git branch -vv to parse the
tracking remote for each branch (falling back to parsing git remote -v if no
tracking info), set a variable like BRANCH_REMOTE for the branch, then replace
hardcoded "upstream/$branch" and "origin/$branch" with attempts to git rev-parse
and git fetch against the discovered BRANCH_REMOTE (and as a final fallback
iterate available remotes from git remote -v), preserving the existing error
messages and continue behavior.
Comment on lines
+204
to
+212
| if [[ "$platform" == "github" ]]; then | ||
| echo "PR is open. Fetching PR head ref..." >&2 | ||
| git fetch upstream "pull/${pr_number}/head:pr-${pr_number}" 2>&1 >&2 || true | ||
| COMMIT_SHA=$(git rev-parse "pr-${pr_number}" 2>/dev/null || echo "$head_sha") | ||
| elif [[ "$platform" == "gitlab" ]]; then | ||
| git fetch upstream "merge-requests/${pr_number}/head:mr-${pr_number}" 2>&1 >&2 || true | ||
| COMMIT_SHA=$(git rev-parse "mr-${pr_number}" 2>/dev/null || echo "$head_sha") | ||
| fi | ||
| echo "WARNING: PR is not merged. Using head commit: $COMMIT_SHA" >&2 |
There was a problem hiding this comment.
Hardcoded remote names violate coding guidelines.
The script hardcodes upstream without first discovering available remotes. This pattern repeats at lines 302-308, 367, 373, 486, and 569.
Consider adding a remote discovery function similar to the one suggested for branch_audit.sh, then using the discovered remote names throughout the script.
As per coding guidelines: "Code should NEVER assume git remote names like 'origin' or 'upstream' without first discovering them using git remote -v and git branch -vv."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh` around
lines 204 - 212, The script currently hardcodes the git remote name "upstream"
when fetching PR/MR refs (see usage around COMMIT_SHA, pr_number and platform
handling in cherry_pick.sh), which violates guidelines; add a remote discovery
helper (e.g., function discover_remote_or_throw or get_remote_for_upstream) that
inspects `git remote -v` and `git branch -vv` to find the correct remote to use,
and replace all hardcoded "upstream" occurrences (including the fetch commands
used for GitHub pull/${pr_number}/head and GitLab
merge-requests/${pr_number}/head and any other fetch/push operations later in
the script) to use the discovered remote variable, falling back to a clear error
if none found. Ensure the new helper is called before any git fetch/push and
that variables like COMMIT_SHA, pr_number, and platform use the discovered
remote name instead of the literal "upstream."
Comment on lines
+567
to
+574
| if [[ "$NO_PUSH" = false ]]; then | ||
| echo "Pushing branch $branch_name..." >&2 | ||
| git push -u origin "$branch_name" 2>&1 | ||
| echo "" >&2 | ||
| echo "Branch pushed: $branch_name" >&2 | ||
| else | ||
| echo "Branch created locally: $branch_name (--no-push)" >&2 | ||
| fi |
There was a problem hiding this comment.
Push defaults to enabled — verify this aligns with user permission requirements.
The script pushes to origin by default unless --no-push is specified. Per coding guidelines, scripts must not push without explicit user permission. The current design requires users to opt-out rather than opt-in.
Consider either:
- Requiring explicit
--pushflag to enable pushing, or - Adding a confirmation prompt before pushing
As per coding guidelines: "Commands and skills must NEVER push to git without explicit user permission."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/docs-cherry-pick/scripts/cherry_pick.sh` around
lines 567 - 574, The script currently pushes by default using the NO_PUSH
variable and git push -u origin "$branch_name"; change this so pushing only
occurs with explicit user permission by either (A) flipping the default to not
push and inverting the flag (introduce a --push flag/PUSH variable and require
PUSH=true to run git push), or (B) keep the --no-push flag but add an
interactive confirmation prompt before running git push (prompt the user, read
response, and only run git push when they answer yes). Update the condition
around git push (the block using NO_PUSH and git push -u origin "$branch_name")
and the flag parsing logic so that push is never performed implicitly.
Member
|
is this different to #48? |
Member
|
Closing as I think this was opened in error. Please reopen if required. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new skills to the docs-tools plugin for intelligent backport operations:
What's included
docs-cherry-pickskill with automated conflict detection and resolution workflowdocs-branch-auditskill with deep content comparison capabilitiesUse cases
docs-branch-audit: Plan cherry-pick backports by identifying which modules are applicable to each release before applying changes.
docs-cherry-pick: Automate backporting of editorial fixes, DITA prep changes, and other documentation updates to multiple enterprise branches while handling version-specific content differences.
Test plan
docs-branch-auditwith a sample PR against enterprise branchesdocs-cherry-pickwith a merged PR🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
docs-branch-auditskill to audit documentation file presence and compatibility across enterprise branches.docs-cherry-pickskill for backporting documentation changes to specified target branches with integrated conflict resolution.Chores