Skip to content

review-agent: add PAT auth mode and installer onboarding - #82202

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
enxebre:review-agent-pat-auth
Jul 21, 2026
Merged

review-agent: add PAT auth mode and installer onboarding#82202
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
enxebre:review-agent-pat-auth

Conversation

@enxebre

@enxebre enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Cherry-picks generic review-agent extraction from CNTRLPLANE-3853: Extract generic review-agent step registry from HyperShift #82096
  • Adds opt-in PAT authentication mode (REVIEW_AGENT_AUTH_MODE=pat) to the review-agent — single classic PAT for fork creation, push, and PR operations against any public repo
  • Switches hypershift review-agent workflow to PAT mode
  • Adds installer review-agent workflow + trigger + CI jobs
  • Updates onboarding guide to document PAT as works-out-of-the-box option

Test plan

tested via openshift/installer#10700

Note: Last commit temporarily hardcodes PR #10700 for rehearsal and must be reverted before merge.

/hold

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Adds opt-in classic PAT authentication for the generic CI review-agent (REVIEW_AGENT_AUTH_MODE=pat), enabling fork creation/push and PR/comment operations against public GitHub repos in PAT mode (with documented PAT vs GitHub App requirements).
  • Refactors the HyperShift review-agent to be a thin wrapper that delegates to the generic review-agent setup/process/report step-registry components, and updates the HyperShift wrapper wiring/docs accordingly.
  • Switches the HyperShift review-agent workflow to run in PAT mode (REVIEW_AGENT_AUTH_MODE: "pat").
  • Updates installer CI onboarding for openshift/installer:
    • Adds a new installer-review-agent workflow that runs the generic review-agent steps in PAT mode and includes a temporary rehearsal hardcode for REVIEW_AGENT_TARGET_PR: "10700" (to be removed before merge).
    • Adds an installer-review-agent-trigger workflow plus a Gangway-based trigger script to run the installer periodic review-agent and, when possible, post a “view job” link back to the PR as a comment.
    • Extends ci-operator/config/openshift/installer/openshift-installer-main.yaml with a claude-ai-helpers base image entry and new installer periodic/review jobs (periodic-review-agent and optional address-review-comments).
  • Adds/updates review-agent onboarding and step-registry docs/OWNERS, including moving step refs from HyperShift-specific paths to generic review-agent paths.
  • Adjusts the HyperShift review-agent trigger script to address a ShellCheck warning around an unused JOB_STATUS variable during Gangway polling.

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2026
@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR extracts a reusable Review Agent workflow, makes processing repository- and authentication-configurable, updates HyperShift to use generic steps, and adds Installer periodic and on-demand Gangway-triggered review jobs.

Changes

Generic Review Agent and integrations

Layer / File(s) Summary
Generic workflow contracts and onboarding
ci-operator/step-registry/review-agent/...
Adds reusable setup, process, report, workflow, onboarding, metadata, and ownership definitions.
Repository-aware processing and reporting
ci-operator/step-registry/review-agent/process/..., ci-operator/step-registry/review-agent/report/...
Uses configured repositories and authentication for cloning, PR operations, telemetry, prompts, and generated report links.
HyperShift wrapper migration
ci-operator/step-registry/hypershift/review-agent/...
Delegates HyperShift workflow phases to generic refs and condenses its wrapper documentation.

Installer Review Agent integration

Layer / File(s) Summary
Installer workflow and scheduling
ci-operator/config/openshift/installer/..., ci-operator/step-registry/installer/review-agent/...
Adds Installer workflow wiring, image configuration, periodic scheduling, and optional trigger jobs.
Gangway trigger execution
ci-operator/step-registry/installer/review-agent/trigger/...
Starts a periodic Gangway execution, polls for its job URL, and posts a pull-request status comment.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PullRequest
  participant InstallerTrigger
  participant Gangway
  participant InstallerReviewJob
  participant ReviewAgent
  InstallerTrigger->>Gangway: Start periodic review with target PR
  Gangway->>InstallerReviewJob: Execute installer-review-agent
  InstallerReviewJob->>ReviewAgent: Run setup, process, and report phases
  InstallerTrigger->>Gangway: Poll for job URL
  InstallerTrigger->>PullRequest: Post execution comment
Loading

Possibly related PRs

Suggested reviewers: csrwng, celebdor, danilo-gemoli, jmguzik

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
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.
Stable And Deterministic Test Names ✅ Passed Only ci-operator config changed; no test files or Ginkgo titles were added, so there are no unstable test names to flag.
Test Structure And Quality ✅ Passed No Ginkgo test code was modified: the PR changes only YAML, OWNERS, and shell files, and no *_test.go/suite.go files appear in the commit range.
Microshift Test Compatibility ✅ Passed No Ginkgo e2e tests were added; the PR only changes CI config, workflows, docs, and shell scripts, so MicroShift compatibility is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The commit only changes a ci-operator YAML config; no Go/Ginkgo e2e tests were added, so the SNO check is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed PR only adds CI operator workflows/jobs/docs; no deployment manifests, controllers, or scheduling knobs like nodeSelector, affinity, spread constraints, or PDBs were changed.
Ote Binary Stdout Contract ✅ Passed Only changed path is a YAML CI config; no Go/executable process-level code or stdout writes were introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the diff is CI YAML/docs/shell wrappers only, so IPv4/disconnected test compatibility is not applicable.
No-Weak-Crypto ✅ Passed Touched scripts/docs only use PAT/GitHub auth and HTTP requests; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons found.
Container-Privileges ✅ Passed Targeted scans of all changed manifests found no privileged, hostPID/Network/IPC, allowPrivilegeEscalation, or root securityContext settings.
No-Sensitive-Data-In-Logs ✅ Passed Inspected new scripts: PAT/token reads are wrapped with set +x before secrets, and logs only print job IDs/URLs and PR metadata—no direct secrets/PII exposed.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main theme: PAT auth support plus installer onboarding; it is clear and relevant even if it omits some workflow details.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci
openshift-ci Bot requested review from celebdor and csrwng July 21, 2026 09:05
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre, pj-rehearse: unable to determine affected jobs. This could be due to a branch that needs to be rebased. ERROR:

could not determine changed registry steps: could not load step registry: workflow/installer-review-agent: parameter "REVIEW_AGENT_TARGET_PR" is overridden in [workflow/installer-review-agent] but not declared in any step
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre, pj-rehearse: unable to determine affected jobs ERROR:

could not determine changed registry steps: could not load step registry: workflow/installer-review-agent: parameter "REVIEW_AGENT_TARGET_PR" is overridden in [workflow/installer-review-agent] but not declared in any step

If the problem persists, please contact Test Platform.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: job(s): periodic-ci-openshift-installer-main-periodic-review-agent either don't exist or were not found to be affected, and cannot be rehearsed

@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: 9

🧹 Nitpick comments (1)
ci-operator/step-registry/installer/review-agent/installer-review-agent-workflow.yaml (1)

15-16: 📐 Maintainability & Code Quality | 🔵 Trivial

Confirm removal of the rehearsal PR hardcode before unholding.

REVIEW_AGENT_TARGET_PR: "10700" is explicitly marked for removal pre-merge, consistent with the PR objectives. Just flagging so it isn't missed when the /hold is lifted.

🤖 Prompt for 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.

In
`@ci-operator/step-registry/installer/review-agent/installer-review-agent-workflow.yaml`
around lines 15 - 16, Remove the hardcoded REVIEW_AGENT_TARGET_PR value for
rehearsal PR 10700 before unholding or merging, including its associated
temporary TODO comment, and restore the workflow’s normal target-PR behavior.
🤖 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 `@ci-operator/step-registry/hypershift/review-agent/README.md`:
- Around line 10-15: Align PAT documentation with the workflow contract: in
ci-operator/step-registry/hypershift/review-agent/README.md lines 10-15, replace
REVIEW_AGENT_FORK_REPO with REVIEW_AGENT_FORK_ORG and document
REVIEW_AGENT_UPSTREAM_REPO; in
ci-operator/step-registry/review-agent/ONBOARDING.md lines 13-20, describe three
environment variables and include REVIEW_AGENT_UPSTREAM_REPO; in
ci-operator/step-registry/hypershift/review-agent/hypershift-review-agent-workflow.yaml
lines 18-19, refer to the fork organization and derived fork rather than a fork
repository.

In
`@ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-commands.sh`:
- Around line 34-37: Update the curl invocations in both retry loops to catch
transport failures instead of allowing set -e to terminate the script. Add
bounded connection and request timeouts, preserve the HTTP status handling for
successful responses, and continue retrying after the configured delay until the
existing retry limit is reached.
- Line 38: Remove the set -x tracing toggles from the credential-handling
script, including the occurrences around the bearer-token calls and lines 60–81,
so tracing remains disabled throughout. If preserving prior tracing state is
required, restore it only when tracing was explicitly enabled, and document the
tracing-disabled scope.
- Around line 97-104: Update the curl invocation in the PR comment-posting flow
to fail on HTTP 4xx/5xx responses and surface the server error, using curl’s
failure and error-display options. Ensure the existing “Comment posted on PR”
message is reached only after curl succeeds, while preserving the current
request payload and tracing behavior.

In
`@ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh`:
- Around line 7-23: Validate REVIEW_AGENT_AUTH_MODE against the explicit allowed
values "app" and "pat" before the mode-specific required-variable checks. Reject
any other value with an error message and nonzero exit, while preserving the
existing validation behavior for valid modes.
- Around line 151-195: Protect the App-mode token configuration in the block
containing GITHUB_TOKEN_FORK and GITHUB_TOKEN_UPSTREAM from shell tracing,
matching the PAT branch’s guard. Add a clear comment, disable tracing only
around the git credential.helper construction and GITHUB_TOKEN export, then
restore the prior tracing state immediately afterward. Keep token generation and
unrelated logging outside this minimal protected scope.
- Around line 109-142: Update the fork-creation POST in the process command to
include REVIEW_AGENT_FORK_ORG as the GitHub API organization field, ensuring the
fork is created where subsequent checks expect it. Add --connect-timeout 10 and
--max-time 30 to both GET requests that check fork existence and readiness,
including the polling request.

In
`@ci-operator/step-registry/review-agent/process/review-agent-process-ref.yaml`:
- Around line 37-46: Update the documentation for REVIEW_AGENT_FORK_REPO and
REVIEW_AGENT_UPSTREAM_REPO to reflect the PAT-mode contract:
REVIEW_AGENT_FORK_REPO is required only in App mode, while PAT mode requires
REVIEW_AGENT_FORK_ORG and may derive the fork URL through
review-agent-process-commands.sh. Keep the existing wrapper workflow and gh CLI
usage descriptions accurate.

In `@ci-operator/step-registry/review-agent/review-agent-workflow.yaml`:
- Around line 20-25: Update the documentation block in
ci-operator/step-registry/review-agent/review-agent-workflow.yaml at lines 20-25
to document both app (default) and pat authentication modes, including each
mode’s required environment variables. Update
ci-operator/step-registry/review-agent/README.md at lines 15-25 with entries for
REVIEW_AGENT_AUTH_MODE, REVIEW_AGENT_FORK_ORG, and optional REVIEW_AGENT_PAT_KEY
alongside the existing variables.

---

Nitpick comments:
In
`@ci-operator/step-registry/installer/review-agent/installer-review-agent-workflow.yaml`:
- Around line 15-16: Remove the hardcoded REVIEW_AGENT_TARGET_PR value for
rehearsal PR 10700 before unholding or merging, including its associated
temporary TODO comment, and restore the workflow’s normal target-PR behavior.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 526e1069-d2ef-4f01-ab45-cdc2dac86b1a

📥 Commits

Reviewing files that changed from the base of the PR and between ada957d and 7575336.

📒 Files selected for processing (26)
  • ci-operator/config/openshift/installer/openshift-installer-main.yaml
  • ci-operator/step-registry/hypershift/review-agent/README.md
  • ci-operator/step-registry/hypershift/review-agent/hypershift-review-agent-workflow.yaml
  • ci-operator/step-registry/hypershift/review-agent/report/hypershift-review-agent-report-ref.yaml
  • ci-operator/step-registry/hypershift/review-agent/setup/hypershift-review-agent-setup-ref.yaml
  • ci-operator/step-registry/installer/review-agent/installer-review-agent-workflow.yaml
  • ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-commands.sh
  • ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-ref.yaml
  • ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-workflow.yaml
  • ci-operator/step-registry/review-agent/ONBOARDING.md
  • ci-operator/step-registry/review-agent/OWNERS
  • ci-operator/step-registry/review-agent/README.md
  • ci-operator/step-registry/review-agent/process/OWNERS
  • ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh
  • ci-operator/step-registry/review-agent/process/review-agent-process-ref.metadata.json
  • ci-operator/step-registry/review-agent/process/review-agent-process-ref.yaml
  • ci-operator/step-registry/review-agent/report/OWNERS
  • ci-operator/step-registry/review-agent/report/review-agent-report-commands.sh
  • ci-operator/step-registry/review-agent/report/review-agent-report-ref.metadata.json
  • ci-operator/step-registry/review-agent/report/review-agent-report-ref.yaml
  • ci-operator/step-registry/review-agent/review-agent-workflow.metadata.json
  • ci-operator/step-registry/review-agent/review-agent-workflow.yaml
  • ci-operator/step-registry/review-agent/setup/OWNERS
  • ci-operator/step-registry/review-agent/setup/review-agent-setup-commands.sh
  • ci-operator/step-registry/review-agent/setup/review-agent-setup-ref.metadata.json
  • ci-operator/step-registry/review-agent/setup/review-agent-setup-ref.yaml
💤 Files with no reviewable changes (5)
  • ci-operator/step-registry/hypershift/review-agent/setup/hypershift-review-agent-setup-ref.yaml
  • ci-operator/step-registry/review-agent/OWNERS
  • ci-operator/step-registry/hypershift/review-agent/report/hypershift-review-agent-report-ref.yaml
  • ci-operator/step-registry/review-agent/process/OWNERS
  • ci-operator/step-registry/review-agent/report/OWNERS

Comment on lines +10 to +15
| Variable | Value |
|---|---|
| `REVIEW_AGENT_FORK_REPO` | `https://github.com/hypershift-community/hypershift` |
| `REVIEW_AGENT_UPSTREAM_REPO` | `openshift/hypershift` |

The workflow requires secrets in the `test-credentials` namespace:
Credentials use the `hypershift-team-claude-prow` secret (configured in the generic step refs).

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Align the PAT documentation with the actual workflow contract.

The workflow uses REVIEW_AGENT_AUTH_MODE, REVIEW_AGENT_FORK_ORG, and REVIEW_AGENT_UPSTREAM_REPO; PAT mode derives the fork from the organization. The current documentation advertises a fork URL or omits the required upstream repository, which can produce a failed onboarding configuration.

  • ci-operator/step-registry/hypershift/review-agent/README.md#L10-L15: Document REVIEW_AGENT_FORK_ORG and REVIEW_AGENT_UPSTREAM_REPO instead of REVIEW_AGENT_FORK_REPO.
  • ci-operator/step-registry/review-agent/ONBOARDING.md#L13-L20: Change “two env vars” to three and include REVIEW_AGENT_UPSTREAM_REPO.
  • ci-operator/step-registry/hypershift/review-agent/hypershift-review-agent-workflow.yaml#L18-L19: Say “fork organization/derived fork,” not “fork repo.”
📍 Affects 3 files
  • ci-operator/step-registry/hypershift/review-agent/README.md#L10-L15 (this comment)
  • ci-operator/step-registry/review-agent/ONBOARDING.md#L13-L20
  • ci-operator/step-registry/hypershift/review-agent/hypershift-review-agent-workflow.yaml#L18-L19
🤖 Prompt for 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.

In `@ci-operator/step-registry/hypershift/review-agent/README.md` around lines 10
- 15, Align PAT documentation with the workflow contract: in
ci-operator/step-registry/hypershift/review-agent/README.md lines 10-15, replace
REVIEW_AGENT_FORK_REPO with REVIEW_AGENT_FORK_ORG and document
REVIEW_AGENT_UPSTREAM_REPO; in
ci-operator/step-registry/review-agent/ONBOARDING.md lines 13-20, describe three
environment variables and include REVIEW_AGENT_UPSTREAM_REPO; in
ci-operator/step-registry/hypershift/review-agent/hypershift-review-agent-workflow.yaml
lines 18-19, refer to the fork organization and derived fork rather than a fork
repository.

Comment on lines +34 to +37
RESPONSE=$(curl -s -X POST -d "${POST_DATA}" \
-H "Authorization: Bearer $(cat "${TOKEN_FILE}")" \
"${GANGWAY_API}/v1/executions/${PERIODIC_JOB_NAME}" \
-w "\n%{http_code}")

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Retry transport failures instead of exiting immediately.

A DNS, TLS, or connection failure makes these command substitutions non-zero; set -e then aborts the script before either retry loop can retry. Catch curl failures inside both loops, add bounded timeouts, and continue after the configured delay.

Also applies to: 63-66

🤖 Prompt for 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.

In
`@ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-commands.sh`
around lines 34 - 37, Update the curl invocations in both retry loops to catch
transport failures instead of allowing set -e to terminate the script. Add
bounded connection and request timeouts, preserve the HTTP status handling for
successful responses, and continue retrying after the configured delay until the
existing retry limit is reached.

-H "Authorization: Bearer $(cat "${TOKEN_FILE}")" \
"${GANGWAY_API}/v1/executions/${PERIODIC_JOB_NAME}" \
-w "\n%{http_code}")
set -x

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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Keep tracing disabled in this credential-handling script.

The script starts without tracing, but re-enables it after bearer-token calls. Remove these set -x toggles (or restore prior state only when tracing was explicitly enabled); document any necessary tracing-disabled scope.

Also applies to: 60-81

🤖 Prompt for 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.

In
`@ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-commands.sh`
at line 38, Remove the set -x tracing toggles from the credential-handling
script, including the occurrences around the bearer-token calls and lines 60–81,
so tracing remains disabled throughout. If preserving prior tracing state is
required, restore it only when tracing was explicitly enabled, and document the
tracing-disabled scope.

Source: Coding guidelines

Comment on lines +97 to +104
curl -s -X POST \
-H "Authorization: token ${GITHUB_TOKEN_PAT}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/${UPSTREAM_REPO}/issues/${PR_NUMBER}/comments" \
-d "$(jq -n --arg body "$COMMENT_BODY" '{body: $body}')" > /dev/null

$_was_tracing && set -x || true
echo "Comment posted on PR #$PR_NUMBER"

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail when GitHub rejects the PR comment.

curl -s exits successfully for HTTP 4xx/5xx, so a denied or invalid comment is discarded and the script logs success. Use --fail --show-error or validate the HTTP status before printing “Comment posted.”

Proposed fix
-  curl -s -X POST \
+  curl --fail --silent --show-error -X POST \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
curl -s -X POST \
-H "Authorization: token ${GITHUB_TOKEN_PAT}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/${UPSTREAM_REPO}/issues/${PR_NUMBER}/comments" \
-d "$(jq -n --arg body "$COMMENT_BODY" '{body: $body}')" > /dev/null
$_was_tracing && set -x || true
echo "Comment posted on PR #$PR_NUMBER"
curl --fail --silent --show-error -X POST \
-H "Authorization: token ${GITHUB_TOKEN_PAT}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/${UPSTREAM_REPO}/issues/${PR_NUMBER}/comments" \
-d "$(jq -n --arg body "$COMMENT_BODY" '{body: $body}')" > /dev/null
$_was_tracing && set -x || true
echo "Comment posted on PR #$PR_NUMBER"
🤖 Prompt for 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.

In
`@ci-operator/step-registry/installer/review-agent/trigger/installer-review-agent-trigger-commands.sh`
around lines 97 - 104, Update the curl invocation in the PR comment-posting flow
to fail on HTTP 4xx/5xx responses and surface the server error, using curl’s
failure and error-display options. Ensure the existing “Comment posted on PR”
message is reached only after curl succeeds, while preserving the current
request payload and tracing behavior.

Comment on lines +7 to +23
REVIEW_AGENT_AUTH_MODE="${REVIEW_AGENT_AUTH_MODE:-app}"
REVIEW_AGENT_PAT_KEY="${REVIEW_AGENT_PAT_KEY:-gh-pat}"
REVIEW_AGENT_FORK_ORG="${REVIEW_AGENT_FORK_ORG:-}"

# Validate required env vars
if [[ "$REVIEW_AGENT_AUTH_MODE" == "app" ]] && [[ -z "${REVIEW_AGENT_FORK_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_REPO is required in App auth mode (e.g. https://github.com/hypershift-community/hypershift)"
exit 1
fi
if [[ "$REVIEW_AGENT_AUTH_MODE" == "pat" ]] && [[ -z "$REVIEW_AGENT_FORK_ORG" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_ORG is required in PAT auth mode"
exit 1
fi
if [[ -z "${REVIEW_AGENT_UPSTREAM_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_UPSTREAM_REPO is required (e.g. openshift/hypershift)"
exit 1
fi

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate REVIEW_AGENT_AUTH_MODE against an explicit allow-list.

Only "app" and "pat" are ever compared; any other/misspelled value silently falls through to the App-mode branch at line 87 instead of failing fast, since neither if at Line 12/16 raises an error for unrecognized values.

🛡️ Proposed validation
+if [[ "$REVIEW_AGENT_AUTH_MODE" != "app" ]] && [[ "$REVIEW_AGENT_AUTH_MODE" != "pat" ]]; then
+  echo "ERROR: REVIEW_AGENT_AUTH_MODE must be 'app' or 'pat', got: $REVIEW_AGENT_AUTH_MODE"
+  exit 1
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
REVIEW_AGENT_AUTH_MODE="${REVIEW_AGENT_AUTH_MODE:-app}"
REVIEW_AGENT_PAT_KEY="${REVIEW_AGENT_PAT_KEY:-gh-pat}"
REVIEW_AGENT_FORK_ORG="${REVIEW_AGENT_FORK_ORG:-}"
# Validate required env vars
if [[ "$REVIEW_AGENT_AUTH_MODE" == "app" ]] && [[ -z "${REVIEW_AGENT_FORK_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_REPO is required in App auth mode (e.g. https://github.com/hypershift-community/hypershift)"
exit 1
fi
if [[ "$REVIEW_AGENT_AUTH_MODE" == "pat" ]] && [[ -z "$REVIEW_AGENT_FORK_ORG" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_ORG is required in PAT auth mode"
exit 1
fi
if [[ -z "${REVIEW_AGENT_UPSTREAM_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_UPSTREAM_REPO is required (e.g. openshift/hypershift)"
exit 1
fi
REVIEW_AGENT_AUTH_MODE="${REVIEW_AGENT_AUTH_MODE:-app}"
REVIEW_AGENT_PAT_KEY="${REVIEW_AGENT_PAT_KEY:-gh-pat}"
REVIEW_AGENT_FORK_ORG="${REVIEW_AGENT_FORK_ORG:-}"
if [[ "$REVIEW_AGENT_AUTH_MODE" != "app" ]] && [[ "$REVIEW_AGENT_AUTH_MODE" != "pat" ]]; then
echo "ERROR: REVIEW_AGENT_AUTH_MODE must be 'app' or 'pat', got: $REVIEW_AGENT_AUTH_MODE"
exit 1
fi
# Validate required env vars
if [[ "$REVIEW_AGENT_AUTH_MODE" == "app" ]] && [[ -z "${REVIEW_AGENT_FORK_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_REPO is required in App auth mode (e.g. https://github.com/hypershift-community/hypershift)"
exit 1
fi
if [[ "$REVIEW_AGENT_AUTH_MODE" == "pat" ]] && [[ -z "$REVIEW_AGENT_FORK_ORG" ]]; then
echo "ERROR: REVIEW_AGENT_FORK_ORG is required in PAT auth mode"
exit 1
fi
if [[ -z "${REVIEW_AGENT_UPSTREAM_REPO:-}" ]]; then
echo "ERROR: REVIEW_AGENT_UPSTREAM_REPO is required (e.g. openshift/hypershift)"
exit 1
fi
🤖 Prompt for 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.

In
`@ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh`
around lines 7 - 23, Validate REVIEW_AGENT_AUTH_MODE against the explicit
allowed values "app" and "pat" before the mode-specific required-variable
checks. Reject any other value with an error message and nonzero exit, while
preserving the existing validation behavior for valid modes.

Comment on lines +109 to +142
# Ensure the fork exists (auto-fork if needed)
FORK_REPO_NAME="${REVIEW_AGENT_UPSTREAM_REPO#*/}"
echo "Checking if fork ${REVIEW_AGENT_FORK_ORG}/${FORK_REPO_NAME} exists..."
FORK_HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \
-H "Authorization: Bearer ${GITHUB_TOKEN}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/app/installations/${INSTALL_ID}/access_tokens" \
| jq -r '.token'
}
"https://api.github.com/repos/${REVIEW_AGENT_FORK_ORG}/${FORK_REPO_NAME}")
if [ "$FORK_HTTP_CODE" != "200" ]; then
echo "Fork not found (HTTP ${FORK_HTTP_CODE}). Creating fork of ${REVIEW_AGENT_UPSTREAM_REPO}..."
FORK_RESPONSE=$(curl -s -X POST \
--connect-timeout 10 --max-time 30 \
-H "Authorization: Bearer ${GITHUB_TOKEN}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/${REVIEW_AGENT_UPSTREAM_REPO}/forks" \
-d '{"default_branch_only":true}')
FORK_FULL_NAME=$(echo "$FORK_RESPONSE" | jq -r '.full_name // empty' 2>/dev/null)
if [ -z "$FORK_FULL_NAME" ]; then
echo "ERROR: Failed to create fork. API response:"
echo "$FORK_RESPONSE" | head -20
exit 1
fi
echo "Fork creation initiated: ${FORK_FULL_NAME}"
# Poll until ready
WAITED=0
while [ $WAITED -lt 120 ]; do
FORK_HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \
-H "Authorization: Bearer ${GITHUB_TOKEN}" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/${REVIEW_AGENT_FORK_ORG}/${FORK_REPO_NAME}")
[ "$FORK_HTTP_CODE" = "200" ] && break
echo "Waiting for fork to be ready... (${WAITED}s/120s)"
sleep 10
WAITED=$((WAITED + 10))
done

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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '1,220p' ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh

Repository: openshift/release

Length of output: 9001


🏁 Script executed:

grep -nE 'FORK_RESPONSE|FORK_HTTP_CODE|organization|default_branch_only|GITHUB_TOKEN|REVIEW_AGENT_FORK_ORG|REVIEW_AGENT_UPSTREAM_REPO' -n ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh

Repository: openshift/release

Length of output: 3068


🌐 Web query:

GitHub REST API forks create fork organization default_branch_only official docs

💡 Result:

To create a fork using the GitHub REST API, you can send a POST request to the /repos/{owner}/{repo}/forks endpoint [1][2]. When creating a fork, you can use the default_branch_only boolean parameter in the request body to specify that the fork should include only the default branch of the source repository [1][3]. You can also use the organization parameter to specify an organization if you wish to fork the repository into one, rather than to your personal account [1][4]. Example request: curl -L \ -X POST \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer YOUR_TOKEN" \ https://api.github.com/repos/OWNER/REPO/forks \ -d '{ "organization": "your-org-name", "name": "new-fork-name", "default_branch_only": true }' Note that forking a repository is an asynchronous operation, and you may need to wait a short period before the new repository is fully available [1][2]. The organization parameter is optional and is used only when you want to fork into an organization where you have the appropriate permissions [1][5].

Citations:


Create the fork in REVIEW_AGENT_FORK_ORG and bound the GETs.
POST /forks needs an organization field here; without it, GitHub creates the fork under the token owner’s account, while the later existence/poll checks always look for ${REVIEW_AGENT_FORK_ORG}/${FORK_REPO_NAME}. Add --connect-timeout/--max-time to those GETs too so a stalled GitHub request can’t hang the step.

🤖 Prompt for 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.

In
`@ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh`
around lines 109 - 142, Update the fork-creation POST in the process command to
include REVIEW_AGENT_FORK_ORG as the GitHub API organization field, ensuring the
fork is created where subsequent checks expect it. Add --connect-timeout 10 and
--max-time 30 to both GET requests that check fork existence and readiness,
including the polling request.

Comment on lines +151 to +195
else
# App mode: separate fork/upstream tokens
echo "Loading GitHub App auth library..."
if [ ! -f "${SHARED_DIR}/github-app-auth.sh" ]; then
echo "ERROR: github-app-auth.sh not found in SHARED_DIR."
echo "Ensure jira-agent-github-app-auth runs as a pre step."
exit 1
fi
# shellcheck source=/dev/null
source "${SHARED_DIR}/github-app-auth.sh"

INSTALLATION_ID_FORK_FILE="${GITHUB_APP_CREDS_DIR}/installation-id"
INSTALLATION_ID_UPSTREAM_FILE="${GITHUB_APP_CREDS_DIR}/o-h-installation-id"

if [ ! -f "$INSTALLATION_ID_FORK_FILE" ] || [ ! -f "$INSTALLATION_ID_UPSTREAM_FILE" ]; then
echo "GitHub App credentials not yet available in ${GITHUB_APP_CREDS_DIR}"
echo "Available files:"
ls -la "${GITHUB_APP_CREDS_DIR}/" || echo "Directory does not exist"
echo "ERROR: Required credentials are missing. Re-run once secrets are synced."
exit 1
fi

# Generate token for fork (hypershift-community/hypershift) - for pushing branches
echo "Generating GitHub App token for fork..."
GITHUB_TOKEN_FORK=$(generate_github_token "$INSTALLATION_ID_FORK")
if [ -z "$GITHUB_TOKEN_FORK" ] || [ "$GITHUB_TOKEN_FORK" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for fork"
exit 1
fi
echo "Fork token generated successfully"
INSTALLATION_ID_FORK=$(cat "$INSTALLATION_ID_FORK_FILE")
INSTALLATION_ID_UPSTREAM=$(cat "$INSTALLATION_ID_UPSTREAM_FILE")

# Generate token for upstream (openshift/hypershift) - for reading PRs and comments
echo "Generating GitHub App token for upstream..."
GITHUB_TOKEN_UPSTREAM=$(generate_github_token "$INSTALLATION_ID_UPSTREAM")
if [ -z "$GITHUB_TOKEN_UPSTREAM" ] || [ "$GITHUB_TOKEN_UPSTREAM" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for upstream"
exit 1
fi
echo "Upstream token generated successfully"
echo "Generating GitHub App token for fork..."
GITHUB_TOKEN_FORK=$(generate_github_token "$INSTALLATION_ID_FORK")
if [ -z "$GITHUB_TOKEN_FORK" ] || [ "$GITHUB_TOKEN_FORK" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for fork"
exit 1
fi
echo "Fork token generated successfully"

# Configure git to use the fork token for push operations via credential helper
git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
echo "Generating GitHub App token for upstream..."
GITHUB_TOKEN_UPSTREAM=$(generate_github_token "$INSTALLATION_ID_UPSTREAM")
if [ -z "$GITHUB_TOKEN_UPSTREAM" ] || [ "$GITHUB_TOKEN_UPSTREAM" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for upstream"
exit 1
fi
echo "Upstream token generated successfully"

# Export upstream token as GITHUB_TOKEN for gh CLI (used for PR operations)
export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
echo "GitHub App tokens configured successfully"
git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
echo "GitHub App tokens configured successfully"
fi

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

App-mode token exposure: mirror the PAT branch's tracing guard.

Lines 192-193 embed GITHUB_TOKEN_FORK/GITHUB_TOKEN_UPSTREAM into a shell command and export, with no set +x/set -x guard — unlike the PAT branch (lines 96-107), which explicitly disables tracing around the equivalent credential.helper construction. If tracing is ever enabled earlier in the script, this path leaks both App tokens to CI logs.

🔒 Proposed fix
+  [[ $- == *x* ]] && _was_tracing=true || _was_tracing=false
+  set +x
   git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
   export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
   echo "GitHub App tokens configured successfully"
+  $_was_tracing && set -x || true
 fi
As per path instructions, `ci-operator/step-registry/**/*-commands.sh` scripts should "keep the tracing-disabled scope minimal, add clear comments when disabling tracing."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
else
# App mode: separate fork/upstream tokens
echo "Loading GitHub App auth library..."
if [ ! -f "${SHARED_DIR}/github-app-auth.sh" ]; then
echo "ERROR: github-app-auth.sh not found in SHARED_DIR."
echo "Ensure jira-agent-github-app-auth runs as a pre step."
exit 1
fi
# shellcheck source=/dev/null
source "${SHARED_DIR}/github-app-auth.sh"
INSTALLATION_ID_FORK_FILE="${GITHUB_APP_CREDS_DIR}/installation-id"
INSTALLATION_ID_UPSTREAM_FILE="${GITHUB_APP_CREDS_DIR}/o-h-installation-id"
if [ ! -f "$INSTALLATION_ID_FORK_FILE" ] || [ ! -f "$INSTALLATION_ID_UPSTREAM_FILE" ]; then
echo "GitHub App credentials not yet available in ${GITHUB_APP_CREDS_DIR}"
echo "Available files:"
ls -la "${GITHUB_APP_CREDS_DIR}/" || echo "Directory does not exist"
echo "ERROR: Required credentials are missing. Re-run once secrets are synced."
exit 1
fi
# Generate token for fork (hypershift-community/hypershift) - for pushing branches
echo "Generating GitHub App token for fork..."
GITHUB_TOKEN_FORK=$(generate_github_token "$INSTALLATION_ID_FORK")
if [ -z "$GITHUB_TOKEN_FORK" ] || [ "$GITHUB_TOKEN_FORK" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for fork"
exit 1
fi
echo "Fork token generated successfully"
INSTALLATION_ID_FORK=$(cat "$INSTALLATION_ID_FORK_FILE")
INSTALLATION_ID_UPSTREAM=$(cat "$INSTALLATION_ID_UPSTREAM_FILE")
# Generate token for upstream (openshift/hypershift) - for reading PRs and comments
echo "Generating GitHub App token for upstream..."
GITHUB_TOKEN_UPSTREAM=$(generate_github_token "$INSTALLATION_ID_UPSTREAM")
if [ -z "$GITHUB_TOKEN_UPSTREAM" ] || [ "$GITHUB_TOKEN_UPSTREAM" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for upstream"
exit 1
fi
echo "Upstream token generated successfully"
echo "Generating GitHub App token for fork..."
GITHUB_TOKEN_FORK=$(generate_github_token "$INSTALLATION_ID_FORK")
if [ -z "$GITHUB_TOKEN_FORK" ] || [ "$GITHUB_TOKEN_FORK" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for fork"
exit 1
fi
echo "Fork token generated successfully"
# Configure git to use the fork token for push operations via credential helper
git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
echo "Generating GitHub App token for upstream..."
GITHUB_TOKEN_UPSTREAM=$(generate_github_token "$INSTALLATION_ID_UPSTREAM")
if [ -z "$GITHUB_TOKEN_UPSTREAM" ] || [ "$GITHUB_TOKEN_UPSTREAM" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for upstream"
exit 1
fi
echo "Upstream token generated successfully"
# Export upstream token as GITHUB_TOKEN for gh CLI (used for PR operations)
export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
echo "GitHub App tokens configured successfully"
git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
echo "GitHub App tokens configured successfully"
fi
else
# App mode: separate fork/upstream tokens
echo "Loading GitHub App auth library..."
if [ ! -f "${SHARED_DIR}/github-app-auth.sh" ]; then
echo "ERROR: github-app-auth.sh not found in SHARED_DIR."
echo "Ensure jira-agent-github-app-auth runs as a pre step."
exit 1
fi
# shellcheck source=/dev/null
source "${SHARED_DIR}/github-app-auth.sh"
INSTALLATION_ID_FORK_FILE="${GITHUB_APP_CREDS_DIR}/installation-id"
INSTALLATION_ID_UPSTREAM_FILE="${GITHUB_APP_CREDS_DIR}/o-h-installation-id"
if [ ! -f "$INSTALLATION_ID_FORK_FILE" ] || [ ! -f "$INSTALLATION_ID_UPSTREAM_FILE" ]; then
echo "GitHub App credentials not yet available in ${GITHUB_APP_CREDS_DIR}"
echo "Available files:"
ls -la "${GITHUB_APP_CREDS_DIR}/" || echo "Directory does not exist"
echo "ERROR: Required credentials are missing. Re-run once secrets are synced."
exit 1
fi
INSTALLATION_ID_FORK=$(cat "$INSTALLATION_ID_FORK_FILE")
INSTALLATION_ID_UPSTREAM=$(cat "$INSTALLATION_ID_UPSTREAM_FILE")
echo "Generating GitHub App token for fork..."
GITHUB_TOKEN_FORK=$(generate_github_token "$INSTALLATION_ID_FORK")
if [ -z "$GITHUB_TOKEN_FORK" ] || [ "$GITHUB_TOKEN_FORK" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for fork"
exit 1
fi
echo "Fork token generated successfully"
echo "Generating GitHub App token for upstream..."
GITHUB_TOKEN_UPSTREAM=$(generate_github_token "$INSTALLATION_ID_UPSTREAM")
if [ -z "$GITHUB_TOKEN_UPSTREAM" ] || [ "$GITHUB_TOKEN_UPSTREAM" = "null" ]; then
echo "ERROR: Failed to generate GitHub App token for upstream"
exit 1
fi
echo "Upstream token generated successfully"
[[ $- == *x* ]] && _was_tracing=true || _was_tracing=false
set +x
git config --global credential.helper "!f() { echo username=x-access-token; echo password=${GITHUB_TOKEN_FORK}; }; f"
export GITHUB_TOKEN="$GITHUB_TOKEN_UPSTREAM"
echo "GitHub App tokens configured successfully"
$_was_tracing && set -x || true
fi
🤖 Prompt for 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.

In
`@ci-operator/step-registry/review-agent/process/review-agent-process-commands.sh`
around lines 151 - 195, Protect the App-mode token configuration in the block
containing GITHUB_TOKEN_FORK and GITHUB_TOKEN_UPSTREAM from shell tracing,
matching the PAT branch’s guard. Add a clear comment, disable tracing only
around the git credential.helper construction and GITHUB_TOKEN export, then
restore the prior tracing state immediately afterward. Keep token generation and
unrelated logging outside this minimal protected scope.

Source: Path instructions

Comment on lines +37 to +46
- name: REVIEW_AGENT_FORK_REPO
default: ""
documentation: |-
Fork repo URL to clone and push to (e.g. https://github.com/hypershift-community/hypershift).
Set by the team's wrapper workflow.
- name: REVIEW_AGENT_UPSTREAM_REPO
default: ""
documentation: |-
Upstream owner/repo for gh CLI PR operations (e.g. openshift/hypershift).
Set by the team's wrapper workflow.

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the PAT-mode configuration contract.

review-agent-process-commands.sh derives REVIEW_AGENT_FORK_REPO in PAT mode, and the Installer wrapper relies on that behavior. Document it as required only for App mode; PAT mode requires REVIEW_AGENT_FORK_ORG and may derive the fork URL.

Also applies to: 75-81

🤖 Prompt for 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.

In `@ci-operator/step-registry/review-agent/process/review-agent-process-ref.yaml`
around lines 37 - 46, Update the documentation for REVIEW_AGENT_FORK_REPO and
REVIEW_AGENT_UPSTREAM_REPO to reflect the PAT-mode contract:
REVIEW_AGENT_FORK_REPO is required only in App mode, while PAT mode requires
REVIEW_AGENT_FORK_ORG and may derive the fork URL through
review-agent-process-commands.sh. Keep the existing wrapper workflow and gh CLI
usage descriptions accurate.

Comment on lines +20 to +25
Teams onboard by creating a thin wrapper workflow that sets two env vars:
- REVIEW_AGENT_FORK_REPO: Fork repo URL to clone and push to
- REVIEW_AGENT_UPSTREAM_REPO: Upstream owner/repo for gh CLI operations

Teams with a different credential secret create thin ref YAML wrappers
pointing to the generic commands scripts. See ONBOARDING.md.

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Onboarding docs don't mention the new PAT auth mode. Both the workflow's documentation: block and README.md describe onboarding as setting only REVIEW_AGENT_FORK_REPO/REVIEW_AGENT_UPSTREAM_REPO, but the process script (and the installer wrapper that consumes it) now also requires REVIEW_AGENT_AUTH_MODE=pat, REVIEW_AGENT_FORK_ORG, and optionally REVIEW_AGENT_PAT_KEY for PAT-mode teams. The next team onboarding via these docs would miss the PAT path entirely.

  • ci-operator/step-registry/review-agent/review-agent-workflow.yaml#L20-L25: extend the documentation: block to describe both app (default) and pat auth modes and their respective required env vars.
  • ci-operator/step-registry/review-agent/README.md#L15-L25: add a row/section for REVIEW_AGENT_AUTH_MODE, REVIEW_AGENT_FORK_ORG, and REVIEW_AGENT_PAT_KEY alongside the existing table.
📍 Affects 2 files
  • ci-operator/step-registry/review-agent/review-agent-workflow.yaml#L20-L25 (this comment)
  • ci-operator/step-registry/review-agent/README.md#L15-L25
🤖 Prompt for 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.

In `@ci-operator/step-registry/review-agent/review-agent-workflow.yaml` around
lines 20 - 25, Update the documentation block in
ci-operator/step-registry/review-agent/review-agent-workflow.yaml at lines 20-25
to document both app (default) and pat authentication modes, including each
mode’s required environment variables. Update
ci-operator/step-registry/review-agent/README.md at lines 15-25 with entries for
REVIEW_AGENT_AUTH_MODE, REVIEW_AGENT_FORK_ORG, and optional REVIEW_AGENT_PAT_KEY
alongside the existing variables.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: job(s): periodic-ci-openshift-installer-main-periodic-review-agent either don't exist or were not found to be affected, and cannot be rehearsed

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/retest

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: your /pj-rehearse request was not processed because the request waited in queue for longer than 5 minutes. Please retry in a few minutes.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2026
@enxebre
enxebre force-pushed the review-agent-pat-auth branch from aa68f96 to 32cfe89 Compare July 21, 2026 15:14
@enxebre
enxebre force-pushed the review-agent-pat-auth branch from 32cfe89 to ab6a080 Compare July 21, 2026 15:17
@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse periodic-ci-openshift-installer-main-periodic-review-agent

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2026
@enxebre
enxebre force-pushed the review-agent-pat-auth branch from 4374296 to 502ef7f Compare July 21, 2026 16:06
@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse ack

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot openshift-merge-bot Bot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Jul 21, 2026
@patrickdillon

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 21, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2026
enxebre and others added 2 commits July 21, 2026 22:34
Adds opt-in PAT authentication mode (REVIEW_AGENT_AUTH_MODE=pat) to the
review-agent as an alternative to the existing GitHub App flow. In PAT
mode, a single classic PAT handles fork creation, push, and PR operations
against any public repo.

New env vars:
- REVIEW_AGENT_AUTH_MODE: "app" (default) or "pat"
- REVIEW_AGENT_FORK_ORG: GitHub user/org to fork repos into (PAT mode)
- REVIEW_AGENT_PAT_KEY: key name in credential secret for the PAT
- REVIEW_AGENT_TARGET_PR: explicit PR number override

Updates the hypershift review-agent workflow to PAT mode and the
onboarding guide to document PAT as a works-out-of-the-box option.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds review-agent workflow, trigger, and CI job configuration for
openshift/installer using PAT auth mode with jira-solve-bot fork org.

- installer-review-agent workflow (PAT mode, auto-fork)
- installer-review-agent-trigger (gangway + PAT-based PR comment)
- periodic-review-agent and address-review-comments jobs in CI config
- claude-ai-helpers base image for agent container steps
- Generated prowjobs, metadata, and OWNERS files

PR openshift#10700 hardcoded for rehearsal — remove before merge.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@enxebre
enxebre force-pushed the review-agent-pat-auth branch from 502ef7f to aa077bc Compare July 21, 2026 20:36
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2026
@openshift-merge-bot openshift-merge-bot Bot removed the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Jul 21, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@enxebre: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-installer-main-address-review-comments openshift/installer presubmit Presubmit changed
periodic-ci-openshift-installer-main-periodic-review-agent N/A periodic Periodic changed
periodic-ci-openshift-hypershift-main-periodic-review-agent N/A periodic Registry content changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@bryan-cox bryan-cox left a comment

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.

/lgtm

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

openshift-ci Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@enxebre

enxebre commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/pj-rehearse ack

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@enxebre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot openshift-merge-bot Bot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Jul 21, 2026
@openshift-ci

openshift-ci Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@enxebre: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit e6988b2 into openshift:main Jul 21, 2026
18 checks passed
fracappa pushed a commit to fracappa/release that referenced this pull request Jul 22, 2026
…2202)

* review-agent: add PAT auth mode with auto-fork

Adds opt-in PAT authentication mode (REVIEW_AGENT_AUTH_MODE=pat) to the
review-agent as an alternative to the existing GitHub App flow. In PAT
mode, a single classic PAT handles fork creation, push, and PR operations
against any public repo.

New env vars:
- REVIEW_AGENT_AUTH_MODE: "app" (default) or "pat"
- REVIEW_AGENT_FORK_ORG: GitHub user/org to fork repos into (PAT mode)
- REVIEW_AGENT_PAT_KEY: key name in credential secret for the PAT
- REVIEW_AGENT_TARGET_PR: explicit PR number override

Updates the hypershift review-agent workflow to PAT mode and the
onboarding guide to document PAT as a works-out-of-the-box option.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* installer: add review-agent CI jobs with PAT auth

Adds review-agent workflow, trigger, and CI job configuration for
openshift/installer using PAT auth mode with jira-solve-bot fork org.

- installer-review-agent workflow (PAT mode, auto-fork)
- installer-review-agent-trigger (gangway + PAT-based PR comment)
- periodic-review-agent and address-review-comments jobs in CI config
- claude-ai-helpers base image for agent container steps
- Generated prowjobs, metadata, and OWNERS files

PR openshift#10700 hardcoded for rehearsal — remove before merge.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
amogh-redhat pushed a commit to amogh-redhat/release that referenced this pull request Aug 5, 2026
…2202)

* review-agent: add PAT auth mode with auto-fork

Adds opt-in PAT authentication mode (REVIEW_AGENT_AUTH_MODE=pat) to the
review-agent as an alternative to the existing GitHub App flow. In PAT
mode, a single classic PAT handles fork creation, push, and PR operations
against any public repo.

New env vars:
- REVIEW_AGENT_AUTH_MODE: "app" (default) or "pat"
- REVIEW_AGENT_FORK_ORG: GitHub user/org to fork repos into (PAT mode)
- REVIEW_AGENT_PAT_KEY: key name in credential secret for the PAT
- REVIEW_AGENT_TARGET_PR: explicit PR number override

Updates the hypershift review-agent workflow to PAT mode and the
onboarding guide to document PAT as a works-out-of-the-box option.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* installer: add review-agent CI jobs with PAT auth

Adds review-agent workflow, trigger, and CI job configuration for
openshift/installer using PAT auth mode with jira-solve-bot fork org.

- installer-review-agent workflow (PAT mode, auto-fork)
- installer-review-agent-trigger (gangway + PAT-based PR comment)
- periodic-review-agent and address-review-comments jobs in CI config
- claude-ai-helpers base image for agent container steps
- Generated prowjobs, metadata, and OWNERS files

PR openshift#10700 hardcoded for rehearsal — remove before merge.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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. rehearsals-ack Signifies that rehearsal jobs have been acknowledged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants