Add /push-pr-update skill for pushing PR revisions

[igerber]

Summary

• Add new /push-pr-update skill for streamlined PR update workflow
• Automatically stages, commits, and pushes changes to existing PR branches
• Triggers AI code review via /ai-review comment (configurable with --no-review)
• Includes same secret scanning safeguards as /submit-pr

Methodology references

• N/A - no methodology changes (this is a tooling/skill addition only)

Validation

• No test changes (Claude Code skill, not library code)
• Manual testing recommended: run /push-pr-update on a branch with uncommitted changes and an open PR

Security / privacy

• Confirm no secrets/PII in this PR: Yes
• Skill includes secret scanning before commits (matches /submit-pr behavior)

---

Generated with Claude Code

[github-actions]

Overall Assessment
Overall assessment: ⚠️ Needs changes

Executive Summary

• No estimator or methodology changes; registry reviewed and not implicated.
• Push step assumes upstream is configured, which can fail or target the wrong remote in some PR setups.
• Default-branch guard only checks main, so repos with other default branches can bypass the intended safety check.
• Reported “Files changed” count lacks a defined computation path when --message is used.

Methodology

• No issues found; PR only adds a tooling command with no estimator/assumption/variance changes (docs/methodology/REGISTRY.md:1).

Code Quality

• Severity: P2 | Impact: git push can fail when upstream is unset or push to the wrong remote in fork workflows, so updates may not reach the PR branch | Fix: detect upstream with git rev-parse --abbrev-ref --symbolic-full-name @{u} and fall back to git push -u origin HEAD, or mirror submit-pr remote resolution to choose the push remote explicitly (.claude/commands/push-pr-update.md:108).

Performance

• No issues found.

Maintainability

• Severity: P3 | Impact: safety guard mentions “default branch” but only checks main, so non-main defaults can slip past the intended abort check | Fix: resolve default branch via gh repo view --json defaultBranchRef or git remote show origin and compare to current branch (.claude/commands/push-pr-update.md:30).

Tech Debt

• No issues found.

Security

• No issues found; secret scanning mirrors /submit-pr.

Documentation/Tests

• Severity: P3 | Impact: output includes “Files changed: ” but no step computes the count when --message is provided, so reporting can be inconsistent | Fix: add a count step such as git diff --cached --name-only | wc -l before reporting (.claude/commands/push-pr-update.md:150).

[igerber]

/ai-review

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: cd19c6fa8a0d4e2410c3ea7b3149775a0ea93536

---

Overall assessment: ⚠️ Needs changes

Executive Summary

• Tooling-only addition; no estimator/methodology code touched (registry reviewed).
• P1: /push-pr-update exits when the working tree is clean even if the branch is ahead, blocking pushes of already-committed updates.
• P1: AI review comment owner/repo is derived from origin, which breaks fork-based PRs and can prevent /ai-review triggers.
• P2: Secret scan prints matching diff lines, risking accidental secret exposure in logs.

Methodology

• Severity: P3. Impact: No estimator or math changes in this PR; registry review not applicable beyond confirmation (docs/methodology/REGISTRY.md). Fix: None.

Code Quality

• Severity: P1. Impact: Clean working tree with unpushed commits is treated as “no changes,” so the command aborts even though the PR has updates to push. Fix: after git status --porcelain is empty, check ahead-of-upstream (git rev-list --count @{u}..HEAD or git status -sb) and proceed to push when ahead > 0; only exit when clean and not ahead. Evidence: .claude/commands/push-pr-update.md:L42-L50.
• Severity: P1. Impact: AI review comment targets the repo parsed from origin, which is incorrect in fork workflows; comments may fail or land on the fork. Fix: derive owner/repo from gh pr view --json url,baseRepository (e.g., baseRepository.nameWithOwner) or parse the PR URL directly. Evidence: .claude/commands/push-pr-update.md:L146-L158.

Performance

• Severity: P3. Impact: No performance issues identified for this doc-only change. Fix: None.

Maintainability

• Severity: P3. Impact: No maintainability issues identified for this diff. Fix: None.

Tech Debt

• Severity: P3. Impact: No tech-debt issues identified for this diff. Fix: None.

Security

• Severity: P2. Impact: git diff --cached | grep -iE ... will echo matching secret values to the console/logs, which can leak sensitive data. Fix: use file-name-only scanning (git diff --cached -G '<regex>' --name-only) or similar approaches that avoid printing secret contents; redact or store matches out-of-band before displaying. Evidence: .claude/commands/push-pr-update.md:L76-L84.

Documentation/Tests

• Severity: P3. Impact: No documentation/test gaps identified beyond the new skill doc itself. Fix: None.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 567ba716c2bda75b5373948483a1bb39bdd817ae

---

Overall assessment: ⚠️ Needs changes

Executive Summary

• P1: Clean working tree with committed changes skips PR lookup, so pushes can occur without verifying an open PR or capturing PR metadata for the review step.
• P1: Branches without an upstream are treated as “no changes,” preventing pushes even when local commits exist.
• Methodology registry reviewed; no estimator/SE/assumption changes in this PR.

Methodology

• No issues found; change is a tooling/skill doc and does not touch estimators or inference (registry not applicable).

Code Quality

• Severity: P1; Impact: When git status is clean and the branch is ahead, the workflow skips PR lookup, so it can push without confirming an open PR and later steps lack PR number/URL; Fix: always run the PR lookup (or move it ahead of the clean-check) before pushing (.claude/commands/push-pr-update.md:L42-L67).
• Severity: P1; Impact: Missing upstream makes git rev-list ... || echo "0" report zero ahead commits and abort with “Nothing to push,” blocking updates on branches that have commits but no tracking branch; Fix: detect missing upstream explicitly (e.g., git rev-parse @{u}) and treat as “needs push,” then proceed to Section 4 with git push -u (.claude/commands/push-pr-update.md:L42-L56).

Performance

• No issues found.

Maintainability

• No issues found.

Tech Debt

• No issues found.

Security

• No issues found.

Documentation/Tests

• No issues found; doc-only change, no test updates expected.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 110859f606b167ae616a841d837b2de497460f41

---

Findings

• P1 Security: git diff -G uses POSIX ERE, so \s/\s+ do not match whitespace; secrets like api_key = or bearer <token> with spaces can bypass the scan. Impact: secret scanning can miss real credentials, increasing leakage risk. Fix: replace \s with [[:space:]] (e.g., api[_-]?key[[:space:]]*[=:], bearer[[:space:]]+[A-Za-z0-9_-]+) or explicitly switch to a regex engine that supports \s. Location: .claude/commands/push-pr-update.md:L95.

Overall Assessment: ⚠️ Needs changes

Executive Summary

• Secret scan regex likely misses keys with whitespace due to \s in git diff -G.
• No estimator/methodology changes; Methodology Registry unaffected.
• No library runtime code changes; impact is limited to tooling instructions.
• Tests/docs for estimators remain unchanged.

Methodology

• P3: No estimator, math, weighting, variance/SE, or default behavior changes in .claude/commands/push-pr-update.md. Impact: None. Fix: None.

Code Quality

• P3: No library code changes; the diff adds a command/skill doc only. Impact: None. Fix: None.

Performance

• P3: No performance-sensitive paths changed. Impact: None. Fix: None.

Maintainability

• P3: No maintainability issues identified in the added command file. Impact: None. Fix: None.

Tech Debt

• P3: No tech debt introduced beyond standard command documentation. Impact: None. Fix: None.

Security

• P1: \s and \s+ in git diff -G regex do not match whitespace in POSIX ERE, so patterns like api_key = or bearer <token> with spaces can be missed. Impact: secret scan can fail to flag real credentials, increasing leak risk. Fix: replace \s with [[:space:]] (e.g., api[_-]?key[[:space:]]*[=:], bearer[[:space:]]+[A-Za-z0-9_-]+) or explicitly switch to a regex engine that supports \s. Location: .claude/commands/push-pr-update.md:L95.

Documentation/Tests

• P3: No library documentation or tests affected; skill file only. Impact: None. Fix: None.

Open Questions/Assumptions

• Assumption: git diff -G uses POSIX ERE in your environment; if PCRE-style \s is intended, please confirm and adjust the command accordingly.

Change Summary

• Adds .claude/commands/push-pr-update.md describing a workflow for staging, committing, pushing, and optionally triggering AI review.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 723da00d0afdd22ad24145e1a36f6fdf49d12d48

---

Overall assessment: ⚠️ Needs changes

Executive summary:

• No estimator/methodology changes; registry not implicated by this PR.
• P2 security gap: secret scan in /push-pr-update is case-sensitive and can miss uppercase tokens (e.g., API_KEY), weakening safeguards.
• P3 logic: the “no commits” guard for no-upstream branches uses git rev-list --count HEAD, so the “Nothing to push” path is effectively unreachable in non-empty repos.

Methodology

• No findings; changes are confined to .claude/commands tooling docs, not estimators or assumptions.

Code Quality

• P3 | Impact: git rev-list --count HEAD is always >0 in non-empty repos, so the “no commits” abort for no-upstream branches won’t trigger; can lead to unnecessary pushes or misleading messaging. | Concrete fix: compare against the default branch instead (e.g., git rev-list --count <default-branch>..HEAD), or use git log --oneline <default-branch>..HEAD to detect unique commits. | Location: .claude/commands/push-pr-update.md:L62-L69

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings.

Security

• P2 | Impact: case-sensitive secret regex can miss uppercase variants like API_KEY/PASSWORD, reducing the chance of catching secrets before commit. | Concrete fix: make the pickaxe regex case-insensitive (e.g., add --ignore-case if supported, or expand patterns) to align with .claude/commands/submit-pr.md. | Location: .claude/commands/push-pr-update.md:L92-L100

Documentation/Tests

• No findings.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: ff05a0214657dbccba21dbde1a057262631b502b

---

Overall assessment: ⚠️ Needs changes

Executive summary

• Adds /push-pr-update; no estimator or methodology changes (Methodology Registry not impacted).
• Two workflow correctness issues in the new command: false “Nothing to push” on missing default branch, and missing files-changed-count on push-only paths.
• Secret-scanning behavior is skipped for already-committed changes and can be brittle; align and harden.

Methodology

• No findings. (No estimator/assumption changes; Methodology Registry not impacted.)

Code Quality

• P2 — Impact: When <default-branch> is not present locally (e.g., shallow clone), git rev-list --count <default-branch>..HEAD 2>/dev/null || echo "0" masks the error and can incorrectly abort with “Nothing to push.” — Fix: detect missing branch and fall back to origin/<default-branch> or prompt to fetch; avoid || echo "0" masking errors. .claude/commands/push-pr-update.md:L62-L69
• P3 — Impact: In the clean-working-tree path where commits are already ahead, <files-changed-count> is never set, so the final report is incomplete or incorrect. — Fix: compute count from the pushed commit(s) before reporting (e.g., git diff --name-only @{u}..HEAD | wc -l, or <default-branch>..HEAD if no upstream). .claude/commands/push-pr-update.md:L57-L73 and .claude/commands/push-pr-update.md:L181-L199

Performance

• No findings.

Maintainability

• P3 — Impact: Secret-scan regex logic is duplicated between /submit-pr and /push-pr-update and already diverges, which invites drift. — Fix: factor into a shared snippet or align both commands to the same safer approach. .claude/commands/push-pr-update.md:L92-L101 and .claude/commands/submit-pr.md:L136-L140

Tech Debt

• No findings.

Security

• P2 — Impact: Secret scanning is skipped when the working tree is clean but commits are ahead, despite the note claiming the same safeguards as /submit-pr; secrets committed earlier can be pushed without detection. — Fix: add a scan step for committed diffs (e.g., against @{u} or <default-branch>) or clarify the limitation in Notes. .claude/commands/push-pr-update.md:L57-L73 and .claude/commands/push-pr-update.md:L255-L260
• P3 — Impact: git diff --cached -G ... --name-only exits with status 1 on matches; in strict runners this can abort before the warning path executes. — Fix: capture output with || true and branch on output length. .claude/commands/push-pr-update.md:L92-L101

Documentation/Tests

• No findings (docs-only change; no tests expected).

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: f4de4ede8b6d2bce2b37b921468b6781f4713f06

---

Overall assessment: ⚠️ Needs changes

Executive summary:

• P1: No-upstream path assumes origin/<default-branch> exists; when missing (e.g., single-branch clones), it can return a false “nothing to push” and block PR updates (.claude/commands/push-pr-update.md:L62-L76).
• P2: /submit-pr secret scan still prints diff content to stdout; this can leak secrets in logs and diverges from the safer name-only scan used in the new command (.claude/commands/submit-pr.md:L136-L140).
• Methodology registry checked (docs/methodology/REGISTRY.md); no estimator/inference changes in this PR.

Methodology:

• No findings; changes are limited to .claude command docs.

Code Quality:

• Severity: P1; Impact: if origin/<default-branch> is absent, the command can incorrectly abort with “nothing to push,” blocking updates to an existing PR; Fix: verify/fetch <default-branch> from the push remote or fall back to a ref-free ahead check before declaring no changes (.claude/commands/push-pr-update.md:L62-L76).

Performance:

• No findings.

Maintainability:

• No findings.

Tech Debt:

• No findings.

Security:

• Severity: P2; Impact: git diff --cached | grep ... can print secret values into logs; Fix: switch to git diff --cached -G "<regex>" --name-only (as in the new command) and update the wording accordingly (.claude/commands/submit-pr.md:L136-L140).

Documentation/Tests:

• No findings.

Open Questions/Assumptions:

• None.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: d5b7890beba5f76ac8ba7c73c3eab2ccf0e8a844

---

Overall assessment: ✅ Looks good

Executive summary:

• Adds a /push-pr-update workflow with branch/PR validation, secret scanning, push, and optional AI review trigger.
• Updates /submit-pr secret scanning to avoid leaking diff content in logs.
• No estimator or methodology changes; registry cross-check not applicable.
• Minor nits around argument validation and duplicated secret-scan block.

Methodology

• No findings (no estimator/assumption/variance changes in this PR).

Code Quality

• P3 - Impact: --message without a value has undefined behavior and could yield an empty or malformed commit message. Fix: validate --message has non-empty text and abort with a clear error. Ref: .claude/commands/push-pr-update.md:L18-L23.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• P3 - Impact: Secret-scan regex is duplicated across commands; future edits risk drift. Fix: extract a shared snippet or add an explicit “keep in sync” note. Refs: .claude/commands/push-pr-update.md:L96-L155, .claude/commands/submit-pr.md:L136-L171.

Security

• No findings.

Documentation/Tests

• No findings.