Skip to content

feat(#814): make review agent multi-forge (GitHub + GitLab) - #815

Merged
ggallen merged 1 commit into
mainfrom
agent/814-review-multi-forge
Aug 19, 2026
Merged

feat(#814): make review agent multi-forge (GitHub + GitLab)#815
ggallen merged 1 commit into
mainfrom
agent/814-review-multi-forge

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Add multi-forge (GitHub + GitLab) support to the review agent, following the patterns established by the triage agent in PR #686.

Infrastructure

  • Forge ops layer: review-ops.lib.sh dispatcher + github-review-ops.lib.sh (gh CLI) + gitlab-review-ops.lib.sh (curl/GitLab API) providing forge_* functions for URL validation, PR queries, review posting, label management, comments, and stale-head detection
  • Script conversion: pre-review.sh and post-review.sh converted to .src.sh source files that use forge ops, bundled via make script-build

Configuration

  • Per-forge env files (env/github/review.env, env/gitlab/review.env)
  • Per-forge sandbox policies (policies/github/review.yaml, policies/gitlab/review.yaml)
  • Per-forge pr-review skill variants (skills/pr-review/github/SKILL.md, skills/pr-review/gitlab/SKILL.md)
  • Updated harness/review.yaml with forge.gitlab section mirroring the triage pattern

Env var naming

Following the triage pattern: GITHUB_PR_URL → forge-neutral PR_URL (set by harness per forge), FULLSEND_FORGE added for dispatch.

Upstream dependency

GitLab review posting depends on fullsend post-review --forge gitlab support (fullsend-ai/fullsend#6240). The gitlab-review-ops.lib.sh passes --forge gitlab to fullsend post-review — this will work once the upstream change lands.

Testing

  • All 12 pre-review tests pass with updated env vars
  • All 71 post-review integration tests pass (severity filtering, label actions, protected paths, draft PRs, validated dirs, body content)
  • make check-bundle confirms bundled scripts match source
  • Pre-existing post-retro test failures (10 tests) confirmed on main — unrelated to this change

Closes #814

Post-script verification

  • Branch is not main/master (agent/814-review-multi-forge)
  • Secret scan passed (gitleaks — 693ed83bbd5289e1ef13d6c7a3772f61efc43c86..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 15, 2026 02:41
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 15, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:42 AM UTC · Completed 3:02 AM UTC

Commit: c5b0ed7 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [breaking-cli] scripts/post-review.sh — The script's required environment variable contract has changed in a backward-incompatible way. Previously required: PR_NUMBER (validated as integer) and REPO_FULL_NAME (required). Now required: PR_URL and FULLSEND_FORGE. Any external caller (fullsend CLI dispatch, per-repo harness overrides using base: composition) that supplies the old variables without PR_URL and FULLSEND_FORGE will fail immediately. The harness YAML changes handle the standard path, but the upstream dependency on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 must land first or simultaneously.
    Remediation: Ensure Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 is merged before this PR. Verify all consumers of post-review.sh supply PR_URL and FULLSEND_FORGE.

  • [breaking-cli] scripts/pre-review.sh — Same backward-incompatible contract change: old required vars (PR_NUMBER, REPO_FULL_NAME, GITHUB_PR_URL) replaced by PR_URL and FULLSEND_FORGE. The old script also cross-validated URL-extracted values against independently-provided env vars; the new script derives everything from PR_URL.
    Remediation: Same as post-review.sh — coordinate with Make fullsend post-review multi-forge (add --forge flag) fullsend#6240.

Medium

  • [breaking-cli] scripts/lib/github-review-ops.lib.sh:68forge_post_review() now passes --forge github (or --forge gitlab) to the fullsend CLI. This is a new required argument that the deployed fullsend binary must support. If the deployed version does not support --forge, the post-review step will fail. The PR explicitly declares this dependency on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240.
    Remediation: Ensure the deployed fullsend CLI version supports --forge before these scripts are exercised in production.

  • [protected-path] harness/review.yaml — This PR modifies 18 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and provides rationale for the changes. Human approval is required for protected-path changes regardless of review outcome.
    Protected files: .github/scripts/select-eval-agents-test.sh, AGENTS.md, agents/review.md, harness/review.yaml, policies/github/review.yaml, policies/gitlab/review.yaml, scripts/lib/github-review-ops.lib.sh, scripts/lib/gitlab-review-ops.lib.sh, scripts/lib/review-ops.lib.sh, scripts/post-review-test.sh, scripts/post-review.sh, scripts/post-review.src.sh, scripts/pre-review-test.sh, scripts/pre-review.sh, scripts/pre-review.src.sh, skills/pr-review/SKILL.md, skills/pr-review/github/SKILL.md, skills/pr-review/gitlab/SKILL.md.

  • [permission-expansion] policies/github/review.yaml — New sandbox policy for the GitHub forge. Well-structured: read-only API access, binary allowlist restricts to gh and node (excluding curl to prevent raw HTTP access with the injected GH_TOKEN), GraphQL read-only enforcement. No issues found — flagged as a mandatory permission-declaring file finding.

  • [permission-expansion] policies/gitlab/review.yaml — New sandbox policy for the GitLab forge. Allows curl (not gh) with read-only enforcement on two allowed hosts (gitlab.com, gitlab.cee.redhat.com). allow_encoded_slash: true is appropriate for GitLab project paths. No issues found.

Low

  • [breaking-config] harness/review.yaml:17skills/issue-labels/github removed from top-level skills: array, moved to forge.github.skills. Downstream harnesses using base: composition that override at the top level may need to adjust skill references to target forge.<platform>.skills.

  • [scope-creep] agents/review.mdGITHUB_ISSUE_URL input variable removed entirely. However, the old harness never passed GITHUB_ISSUE_URL to the sandbox (forge.github.env.sandbox had no entry for it). The variable was documented but never wired — removing dead documentation is reasonable scope.

  • [architectural-coherence] harness/review.yaml:36pre_script and post_script removed from top-level config, kept only under forge.github and forge.gitlab. The triage agent retains top-level scripts as a fallback, but triage supports non-forge invocations (jira). The review agent is exclusively PR/MR-scoped and always requires a forge context, making top-level scripts unnecessary.

  • [pattern-inconsistency] scripts/lib/github-review-ops.lib.sh:59forge_parse_pr_url() sets global REPO while harness env blocks still pass REPO_FULL_NAME. The old pre-review script cross-validated these; the new script derives everything from PR_URL. Architecturally intentional — PR_URL is now the single source of truth.

  • [secret-exposure] scripts/lib/gitlab-review-ops.lib.sh:10_gitlab_api() passes REVIEW_TOKEN via --header 'PRIVATE-TOKEN: ...' on the curl command line, visible in process listings (/proc/<pid>/cmdline). The GitHub forge avoids this by using GH_TOKEN as an environment variable. Risk is mitigated by ephemeral runners and ::add-mask:: in the post-script.

  • [fail-open] scripts/lib/gitlab-review-ops.lib.sh:41forge_get_pr_state() returns UNKNOWN for unrecognized GitLab states. The post-review script handles UNKNOWN by skipping review (exit 0) with a warning — intentionally fail-open to avoid blocking reviews when the GitLab API is temporarily unavailable.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

Medium

  • [GHA-workflow-command-injection] scripts/lib/github-review-ops.lib.sh:5forge_validate_pr_url() echoes raw ${PR_URL} to stderr without sanitization on the error path. The GitLab implementation correctly uses _gha_sanitize("${PR_URL}"). Since validation failure means PR_URL did not match the expected regex, it could contain GHA workflow command injection sequences (::set-env::, %0A).
    Remediation: Apply _gha_sanitize to ${PR_URL} in the error message, matching the GitLab implementation.

  • [breaking-env-var-rename] agents/review.mdGITHUB_PR_URL env var renamed to PR_URL in the sandbox, GITHUB_ISSUE_URL removed. The harness maps ${GITHUB_PR_URL}PR_URL internally, preserving runner-level compatibility. Downstream repos with custom env.sandbox overrides referencing GITHUB_PR_URL will need to update.
    Remediation: Document the rename; confirm no downstream repos reference GITHUB_PR_URL in custom env.sandbox blocks.

  • [breaking-harness-structure] harness/review.yaml — Top-level pre_script/post_script removed and relocated into forge.github/forge.gitlab sections. skills/issue-labels/github moved from top-level skills: to forge.github.skills. Downstream repos overriding at root level via base: composition will need to override at the forge level instead.
    Remediation: Verify harness engine merges forge-level overrides correctly. Document the structural change for downstream consumers.

  • [new-required-env-var] harness/review.yaml — New required FULLSEND_FORGE env var. Both pre-review.sh and post-review.sh abort if unset. The harness sets it automatically per forge section.

  • [new-required-env-var] scripts/post-review.sh — Required env vars changed from PR_NUMBER + REPO_FULL_NAME to PR_URL + FULLSEND_FORGE. Script now derives PR_NUMBER and REPO from PR_URL via forge_parse_pr_url().

  • [protected-path] This PR modifies files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [naming-convention] scripts/lib/gitlab-review-ops.lib.sh:19_gitlab_api() function name collides with scripts/lib/gitlab-triage-ops.lib.sh. The established pattern is to namespace (e.g., _gitlab_code_api() in gitlab-code-ops.lib.sh). Not sourced together today, but inconsistent with the convention.

  • [gitlab-api-pagination] scripts/lib/gitlab-review-ops.lib.sh:106forge_get_recent_redispatch_comments fetches only 100 notes without pagination, unlike the GitHub --paginate implementation. Low practical risk (100+ notes in a 5-minute window is extreme).

  • [skill-interface-change] skills/pr-review/SKILL.md — Orchestrator now delegates CLI commands to forge-specific sub-skills rather than embedding gh commands inline. The harness loads the correct forge sub-skill.

  • [error-handling-idiom] scripts/lib/gitlab-review-ops.lib.sh:35 — GitLab forge_validate_pr_url() sanitizes URL in error messages with _gha_sanitize, but other validate_*_url() functions in the codebase do not. Inconsistency resolves if the GHA-workflow-command-injection finding's remediation is applied.

Previous run (2)

Review

Findings

Medium

  • [cli-interface-change] scripts/lib/github-review-ops.lib.sh:55forge_post_review now passes --forge github (or --forge gitlab) to fullsend post-review. This flag did not exist in the prior invocation and is added to both the GitHub and GitLab paths. If the fullsend CLI in the sandbox image does not yet support --forge, the post-review step will fail for both forges.
    Remediation: Ensure the fullsend CLI version in the sandbox image supports --forge before merging. Coordinate with Make fullsend post-review multi-forge (add --forge flag) fullsend#6240.

  • [stale-doc] docs/review.md:5 — The opening description still says "evaluates pull requests" while the rest of the file was updated to use "PR/MR" terminology for multi-forge support.
    Remediation: Change to "evaluates pull requests and merge requests" or "evaluates PRs/MRs".

  • [protected-path] This PR modifies 18 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and the description explains the rationale. Human approval is always required for protected-path changes.

Low

  • [test-adequacy] scripts/post-review-test.sh:979 — The mock curl for GitLab tests always returns a non-empty label list regardless of the page query parameter, causing forge_list_repo_labels to loop through all 50 max pages with duplicate output. Tests pass but are slower than necessary.

  • [naming-convention] scripts/lib/gitlab-review-ops.lib.sh:32_gitlab_api() name collides with the same function in gitlab-triage-ops.lib.sh. The code agent avoids this with _gitlab_code_api(). While both are never sourced together at runtime, the inconsistency is a latent risk.

  • [env-file-consistency] env/github/review.env, env/gitlab/review.env — These env files export behavior-tuning vars (REVIEW_FINDING_SEVERITY_THRESHOLD, REVIEW_SKIP_AUTHORS) that are already set in the harness env.sandbox block. Existing code.env and triage.env files only export forge-bridging variables.

  • [env-var-rename] harness/review.yaml:81 — The sandbox-visible env var changed from GITHUB_PR_URL to PR_URL. The env/github/review.env file bridges the rename (PR_URL="${GITHUB_PR_URL}"), but external consumers referencing GITHUB_PR_URL inside the sandbox would break.

  • [GHA-sanitization] scripts/lib/gitlab-review-ops.lib.sh:127forge_validate_pr_url() echoes raw PR_URL to stderr without _gha_sanitize on validation failure. Low risk since PR_URL is set by the harness, not user content.

  • [stale-doc] agents/fix.md:47 — References gh pr review --body which is GitHub-specific. The fix agent has not been made multi-forge yet, so this is informational for follow-up.

  • [required-env-var-change] scripts/post-review.sh — The required env var contract changed from {REVIEW_TOKEN, PR_NUMBER, REPO_FULL_NAME} to {REVIEW_TOKEN, PR_URL, FULLSEND_FORGE}. Standard harness deployments are unaffected but direct script callers must update.

Previous run (3)

Review

Findings

High

  • [cross-repo CLI contract] scripts/lib/github-review-ops.lib.sh:59 — The fullsend post-review CLI invocation now includes a new required --forge flag (--forge github and --forge gitlab). Previously, the CLI was called without --forge. This is a cross-repo contract change: the fullsend CLI in fullsend-ai/fullsend must support the --forge flag before this PR is merged, otherwise all review post-scripts (both GitHub and GitLab) will fail at runtime. The PR body notes this dependency (fullsend-ai/fullsend#6240).
    Remediation: Verify that the fullsend CLI already supports --forge on post-review, or coordinate a simultaneous release. If the CLI does not yet support it, merge the CLI change first.

Medium

  • [env var contract rename] harness/review.yaml:66GITHUB_PR_URL is renamed to PR_URL in both forge.github.env.runner and forge.github.env.sandbox. The host-side source variable ${GITHUB_PR_URL} is preserved, but any downstream consumer reading the sandbox-side or runner-side env var by the old name GITHUB_PR_URL will break.
    Remediation: Audit fullsend-ai/fullsend for code reading GITHUB_PR_URL from sandbox/runner env. Update to PR_URL in coordination.

  • [logic error] scripts/lib/gitlab-review-ops.lib.sh:86forge_get_pr_files uses the /merge_requests/:iid/changes endpoint, which may truncate the changes array for large MRs without pagination. A truncated file list in the protected-path check could cause the post-script to approve a PR that touches protected files, since those files might be in the truncated portion. The GitHub equivalent (gh pr view --json files) handles pagination internally.
    Remediation: Check the overflow field in the response and refuse to approve if truncated, or use the /merge_requests/:iid/diffs endpoint with pagination.

  • [permission-expansion] policies/gitlab/review.yaml:29 — The GitLab sandbox policy permits the curl binary, whereas the GitHub policy explicitly excludes curl to prevent raw HTTP access with injected tokens. The GitHub policy comment documents this as an intentional security boundary. With curl and GITLAB_TOKEN in the sandbox, the agent has raw HTTP access to allowed GitLab hosts. The sandbox network policy (access: read-only) is the compensating control.
    Remediation: Verify this is the intended design trade-off and document it. Consider whether the sandbox GITLAB_TOKEN can be scoped more narrowly.

  • [removed top-level harness fields] harness/review.yaml:32 — Top-level pre_script and post_script are removed and moved into forge.<platform> sections. Non-forge invocations (e.g., --print mode) that rely on top-level resolution will not find the scripts. The scripts themselves also now require FULLSEND_FORGE, so they would fail in a non-forge context regardless.
    Remediation: Verify that the fullsend CLI correctly resolves pre_script/post_script from forge-specific sections.

  • [protected-path] harness/review.yaml — This PR modifies 18 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR is linked to issue Make review agent multi-forge (GitHub + GitLab) #814 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error handling gap] scripts/lib/gitlab-review-ops.lib.sh:131 — GitLab forge functions emit ::warning:: GHA workflow commands on failure. On GitLab CI runners, these render as raw text instead of annotations. Functionally harmless but inconsistent with the forge abstraction.

  • [permission-expansion] harness/review.yaml:89 — The same GITLAB_TOKEN serves both sandbox (read) and runner post-script (write) roles. On GitHub, GH_TOKEN (sandbox) and REVIEW_TOKEN (runner) can be distinct. The sandbox network policy enforces read-only access as the compensating control.

  • [removed input contract] agents/review.md:23GITHUB_ISSUE_URL is removed from agent inputs without a forge-neutral replacement. This was an optional, likely unused input — the forge-specific skills now provide issue context commands. Confirm no downstream harness files inject this variable.

  • [naming-convention-inconsistency] scripts/lib/gitlab-review-ops.lib.sh:32_gitlab_api() uses REVIEW_TOKEN for the PRIVATE-TOKEN header, while gitlab-triage-ops.lib.sh uses GITLAB_TOKEN. Deliberate design choice (harness maps REVIEW_TOKEN: "${GITLAB_TOKEN}") but worth noting for maintainers.

  • [edge case] scripts/lib/gitlab-review-ops.lib.sh:22 — GitLab host allowlist is hardcoded to gitlab.com and gitlab.cee.redhat.com. Self-hosted instances on other hostnames are rejected. Consistent with the sandbox network policy.

  • [fail-open] scripts/lib/gitlab-review-ops.lib.sh:61forge_get_pr_info() falls back to {state: "UNKNOWN", isDraft: false} on API failure. The UNKNOWN state triggers exit 0 in the post-script, silently skipping the review. Graceful degradation trade-off, consistent with the GitHub path.


Labels: PR adds new GitLab forge support capability to the review agent.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (4)

Review

Findings

High

  • [env-var-contract-break] scripts/post-review.sh — The script's required env var interface changed from {REVIEW_TOKEN, PR_NUMBER, REPO_FULL_NAME} to {REVIEW_TOKEN, PR_URL, FULLSEND_FORGE}. Any external consumer invoking this script with the old variable set (without PR_URL and FULLSEND_FORGE) will fail. The breaking change is intentional and the harness mediates it via forge.github.env.runner.PR_URL: '${GITHUB_PR_URL}'.
    Remediation: Ensure fullsend-ai/fullsend is updated in lockstep. Document the migration in release notes.

  • [env-var-contract-break] scripts/pre-review.sh — Same contract break: required env vars changed from {PR_NUMBER, REPO_FULL_NAME, GITHUB_PR_URL} to {PR_URL, FULLSEND_FORGE}. The old GITHUB_PR_URL is no longer directly consumed — it's mapped through forge.github.env.runner.
    Remediation: Coordinate deployment so the harness update lands at the same time. Update documentation referencing old env vars.

Medium

  • [upstream-dependency] scripts/lib/github-review-ops.lib.sh:99forge_post_review calls fullsend post-review --forge <github|gitlab>. The --forge flag depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240, which has not shipped. If this PR is merged before the upstream change, both forge paths will fail at runtime.
    Remediation: Merge Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 first or ensure atomic release.

  • [env-var-contract-break] harness/review.yaml — Top-level pre_script, post_script, and skills/issue-labels/github were removed and moved into forge sections. Downstream harnesses using base: composition that override these at the top level will find no base value to inherit.
    Remediation: Document the structural change in release notes. Audit downstream harness consumers.

  • [fail-open] scripts/post-review.sh:414 — When forge_get_pr_info fails (API error), the post-review script treats UNKNOWN state as a safe skip (exit 0). API unreachability could prevent the review from posting. Mitigated by the harness reporting the run as successful without a posted review, which is observable.

  • [protected-path] 18 of 23 changed files are under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling-gap] scripts/lib/gitlab-review-ops.lib.sh:41forge_get_pr_files uses || return which can abort under set -e instead of producing empty output like the GitHub implementation. The failure mode is fail-closed (safe), but inconsistent with the GitHub path.

  • [edge-case] scripts/lib/gitlab-review-ops.lib.sh:23 — GitLab forge_validate_pr_url regex does not handle URLs with trailing slashes. Minor robustness concern since the harness controls URL construction.

  • [inconsistent-error-handling] scripts/lib/gitlab-review-ops.lib.sh:47forge_get_pr_state returns empty string on failure, while forge_get_pr_info returns {state: "UNKNOWN", isDraft: false}. Inconsistent failure modes between related functions.

  • [test-coverage-gap] scripts/pre-review-test.sh:334 — GitLab pre-review tests do not cover the missing REVIEW_TOKEN code path (no-token scenario).

  • [naming-convention] scripts/lib/gitlab-review-ops.lib.sh:39_gitlab_api() uses REVIEW_TOKEN for the PRIVATE-TOKEN header, while the established triage pattern (gitlab-triage-ops.lib.sh) uses GITLAB_TOKEN.

  • [coherence-documentation-drift] AGENTS.md:158 — Text references forge.github.env.runner blocks (singular forge) but PR_URL now appears in both forge.github and forge.gitlab env blocks.

  • [GHA-workflow-command] scripts/lib/gitlab-review-ops.lib.sh:145 — GitLab forge path uses GHA-specific ::warning:: workflow commands, which are inert on GitLab CI runners. Sanitization is correct but diagnostic output will not surface on GitLab.

  • [provenance-warning] Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (5)

Review

Findings

Medium

  • [protected-path] — This PR modifies 18 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and provides detailed justification. Human approval is required for protected-path changes regardless of review outcome.

  • [breaking-config] harness/review.yaml — Top-level pre_script, post_script, and skills entries moved into forge.github/forge.gitlab sections. Downstream repos using base: composition that override these at the top level will need to restructure into forge.<platform> sections.

  • [breaking-config] harness/review.yaml — New required env var FULLSEND_FORGE. Scripts now fail with FULLSEND_FORGE must be set if absent. The harness sets it automatically; direct invocations need updating.

  • [breaking-api] agents/review.md — Input contract renames GITHUB_PR_URL to PR_URL internally. The harness bridge (forge.github.env.sandbox: PR_URL: "${GITHUB_PR_URL}") preserves the external contract, but custom harness overrides referencing old variable names need updating.

  • [breaking-api] scripts/post-review.sh — Required env vars change from PR_NUMBER/REPO_FULL_NAME to PR_URL/FULLSEND_FORGE. Values now derived from PR_URL via forge_parse_pr_url().

  • [edge-case] scripts/lib/gitlab-review-ops.lib.sh:86forge_get_recent_redispatch_comments fetches only the first 100 notes without pagination (GitHub uses --paginate). The sort=desc mitigates this, but high-traffic MRs could miss the redispatch marker.

  • [naming-convention] scripts/lib/gitlab-review-ops.lib.sh:38_gitlab_api() name collides with gitlab-triage-ops.lib.sh. Other ops libraries use namespaced names (e.g., _gitlab_code_api()).
    Remediation: Rename to _gitlab_review_api().

Low

  • [permission-expansion] policies/github/review.yaml — New sandbox policy. Correctly restricts GitHub API to read-only, excludes curl from binary allowlist.

  • [permission-expansion] policies/gitlab/review.yaml — New sandbox policy for GitLab. Read-only API access on allowed hosts, binaries limited to curl and node.

  • [edge-case] scripts/lib/gitlab-review-ops.lib.sh:49forge_get_pr_files uses /changes endpoint returning full diff content. May timeout for very large MRs (bounded by 30s --max-time).

  • [breaking-api] agents/review.mdGITHUB_ISSUE_URL input removed (was optional).

  • [data-exposure] env/gitlab/review.envGITLAB_TOKEN exported to sandbox. Constrained by network policy to allowed hosts with read-only enforcement.

  • [test-adequacy] scripts/pre-review-test.sh — GitLab tests do not cover URL validation rejection path.

  • [scope-note] harness/review.yaml — Structural difference from triage: removes top-level pre_script/post_script (triage retains them). Both approaches valid.

  • [pattern-inconsistency] env/github/review.env — Exports 8 variables including passthroughs; established pattern is 2–3 forge-specific mappings only.

  • [injection-vuln] scripts/lib/gitlab-review-ops.lib.sh:22 — URL construction validated at both code and network policy layers. No injection vector found.

Previous run (6)

Review

Findings

Medium

  • [protected-path] .github/scripts/*, AGENTS.md, agents/review.md, harness/review.yaml, policies/*, scripts/*, skills/* — This PR modifies 20 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 and provides detailed justification for the multi-forge conversion. Human approval is always required for protected-path changes, regardless of context.

Low

  • [incomplete-test-coverage] scripts/pre-review-test.sh — No GitLab-specific pre-review test cases. The post-review-test.sh adds GitLab tests, but pre-review-test.sh only tests the GitHub forge. The GitLab pre-review path (URL validation, state check, author skip) is untested.
    Remediation: Add GitLab-specific test cases that set FULLSEND_FORGE=gitlab, PR_URL to a GitLab MR URL, and use a mock curl.

  • [GHA-workflow-command-injection] scripts/pre-review.src.sh — The _SAFE_AUTHOR sanitization uses only ${PR_AUTHOR//::/ } which strips :: sequences but does not strip newlines, % characters, or ANSI escape sequences. The existing _gha_sanitize helper (already in scope from review-ops.lib.sh) handles all of these.
    Remediation: Replace with _SAFE_AUTHOR="$(_gha_sanitize "${PR_AUTHOR}")" for consistent sanitization.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-review-ops.lib.sh:49 — The forge_get_pr_state function's *) fallthrough passes unrecognized GitLab MR state values through without sanitization. This value is interpolated into ::notice:: in pre-review.sh.
    Remediation: Normalize unknown states to UNKNOWN consistent with forge_get_pr_info.

  • [env-file-pattern] env/github/review.env, env/gitlab/review.env — Both env files export 8 variables, most of which are also set in the forge.*.env.sandbox section of harness/review.yaml. Existing env files for other agents export 2–3 vars. The overlap creates a dual source of truth.

  • [naming-convention] scripts/lib/gitlab-review-ops.lib.sh:28 — The internal API helper _gitlab_api() matches gitlab-triage-ops.lib.sh but diverges from the namespaced convention _gitlab_code_api() in gitlab-code-ops.lib.sh. No runtime collision (separate bundles), but two competing naming patterns exist in the codebase.

Previous run (7)

Review

Findings

Medium

  • [protected-path] harness/review.yaml — This PR modifies 18 files under protected paths (.github/, AGENTS.md, agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge changes. Human approval is always required for protected-path changes.

  • [breaking-env-rename] harness/review.yaml:72 — The env var GITHUB_PR_URL in forge.github.env.runner has been renamed to PR_URL. Downstream repos extending this harness via base: composition that reference GITHUB_PR_URL in their own env overrides will silently lose the value. The env/github/review.env bridge file maps the transition, and the triage agent established this rename pattern in PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Preserve GITHUB_PR_URL alongside PR_URL for one release cycle, or document as a breaking change.

  • [breaking-env-removal] harness/review.yaml:75GITHUB_PR_URL key removed from forge.github.env.sandbox, replaced by PR_URL. The env bridge file handles the mapping, but downstream agent definitions referencing $GITHUB_PR_URL by exact name would need updating.
    Remediation: Add GITHUB_PR_URL as deprecated alias, or document following triage precedent.

  • [breaking-structural-move] harness/review.yaml:36 — Top-level pre_script and post_script removed; now only inside forge.github and forge.gitlab. Scripts require FULLSEND_FORGE and PR_URL so cannot function without forge context, but this diverges from the triage harness which keeps both top-level and forge-section entries.
    Remediation: Document whether base: composition children with top-level overrides still work, or align with triage pattern.

  • [API contract / pagination misuse] scripts/lib/gitlab-review-ops.lib.sh:106forge_get_pr_files() applies pagination to the GitLab /merge_requests/:iid/changes endpoint, which returns a single object with embedded changes array — not a list endpoint. The page/per_page parameters are ignored by the API. Defensive coding (deduplication + sort -u) masks this, producing correct output. The endpoint is also deprecated since GitLab 15.7.
    Remediation: Replace pagination loop with single API call. Consider migrating to /projects/:id/merge_requests/:iid/diffs.

  • [docs-currency] docs/review.md:87 — The Variables table does not document FULLSEND_FORGE, a new required env var introduced by this PR. Both docs/code.md and docs/triage.md document it in their respective tables.
    Remediation: Add FULLSEND_FORGE row to the Variables table.

  • [new-required-env] harness/review.yaml:60 — Scripts now require PR_URL and FULLSEND_FORGE (fatal if unset), replacing the previous PR_NUMBER/REPO_FULL_NAME requirements. Downstream callers invoking scripts outside the harness will need updating.

Low

  • [breaking-skill-removal] harness/review.yaml:21skills/issue-labels/github removed from top-level skills: array, moved to forge.github.skills. The triage harness already follows this pattern.

  • [SSRF / Host allowlist] scripts/lib/gitlab-review-ops.lib.sh:29_gitlab_api() relies on GITLAB_HOST set by forge_parse_pr_url(), with host validation in a separate function. Currently safe; sandbox network policy provides defense-in-depth.
    Remediation: Consider consolidating host validation into forge_parse_pr_url().

  • [test adequacy] scripts/pre-review-test.sh — No GitLab forge test cases added to pre-review tests. Post-review tests do include GitLab coverage. GitLab-specific pre-review behavior (URL validation, host allowlist) is untested.

  • [error handling] scripts/lib/gitlab-review-ops.lib.sh:106forge_get_pr_files() pipes through sort -u, causing API errors to be silently swallowed (exit code reflects sort, not the API call).

  • [Token exposure] harness/review.yaml:96 — GitLab forge uses the same GITLAB_TOKEN for both runner (write) and sandbox (read intent), unlike GitHub which separates tokens. Sandbox network policy enforces read-only access.

  • [docs-currency] agents/fix.md:47 — Describes review agent posting via gh pr review --body, now GitHub-specific only.

  • [docs-currency] skills/fix-review/SKILL.md:135 — States review agent posts via gh pr review --body, now GitHub-specific only.

  • [code-organization] LOCAL.md:53 — Spurious blank line creates double blank line between comment blocks.

  • [code-organization] scripts/lib/gitlab-review-ops.lib.sh:31 — Double blank line between _gitlab_api() and URL handling section.

  • [scope-creep] harness/review.yaml — Harness restructuring (removing top-level entries, moving to forge sections) is architecturally correct and consistent with the triage agent pattern.

Previous run (8)

Review

Findings

Medium

  • [authorization] harness/review.yaml — The GitLab sandbox env block passes GITLAB_TOKEN into the sandbox — the same token used for write mutations on the runner (REVIEW_TOKEN is set to GITLAB_TOKEN). Unlike the GitHub forge where a separate read-only GH_TOKEN is passed to the sandbox, the GitLab forge has the network policy (access: read-only) as the sole control preventing write mutations from the sandbox.
    Remediation: Consider issuing a separate read-only scoped GitLab token (read_api scope) for the sandbox, or document that the network policy is the sole control.

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:109forge_post_review() in both forge ops files calls fullsend post-review --forge <github|gitlab>. The --forge flag is a new CLI argument that depends on an upstream fullsend CLI change. If the upstream change has not landed, both GitHub and GitLab review workflows will fail at the post-review step.
    Remediation: Coordinate the merge with the upstream fullsend CLI change that adds --forge support.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [breaking-change] harness/review.yaml — Structural changes to harness contract: pre_script, post_script moved from top-level into forge.github.* and forge.gitlab.* sections; skills/issue-labels/github moved to forge.github.skills; policy is now per-forge; new env vars FULLSEND_FORGE and PR_URL required. Downstream repos with base: composition will encounter these changes only when bumping their pinned SHA.
    Remediation: Document migration path in release notes.

  • [api-contract] scripts/lib/gitlab-review-ops.lib.sh:111forge_get_pr_files() uses page-based pagination against the GitLab MR /changes endpoint, but this endpoint returns all changes as a nested array within a single JSON response object. The duplicate-detection guard correctly prevents an infinite loop, but one unnecessary extra API call is always made.

  • [reliability] scripts/lib/gitlab-review-ops.lib.sh:130 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

  • [test-inadequate] scripts/post-review-test.sh:450 — All integration tests set FULLSEND_FORGE="github" exclusively. No tests exercise the GitLab forge path, meaning gitlab-review-ops.lib.sh functions (URL parsing, state normalization, label management, comment posting) have no test coverage.

  • [edge-case] scripts/lib/gitlab-review-ops.lib.sh:66forge_get_pr_state(), forge_get_pr_author(), and forge_get_pr_info() each independently call the same /merge_requests/${PR_NUMBER} endpoint. In the pre-review flow, two identical API calls are made sequentially where one would suffice.

Info

  • [stale-doc] agents/fix.md:47 — References gh pr review --body to describe how the review agent posts findings. With multi-forge support, this is now incomplete (GitLab posts via the GitLab REST API, not gh).

  • [stale-doc] skills/fix-review/SKILL.md:135 — States the review agent posts findings as a single gh pr review --body comment. Now incomplete given GitLab support.

Previous run (9)

Review

Findings

Medium

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:101forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. The --forge flag is new for BOTH GitHub and GitLab. If the upstream change has not landed, both GitHub and GitLab review workflows will fail at the post-review step.
    Remediation: Verify that Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 has merged and the --forge flag works for both github and gitlab values before merging this PR.

  • [breaking-change] harness/review.yaml — Structural breaking changes to harness contract: (1) Environment variable renamed GITHUB_PR_URLPR_URL in forge.github.env sections. (2) pre_script, post_script, skills/issue-labels/github moved from top-level into forge.github.* and new forge.gitlab.* sections. (3) New forge-specific env files (env/github/review.env, env/gitlab/review.env) and policy files (policies/github/review.yaml, policies/gitlab/review.yaml). Downstream repos with base: composition will need to update overrides when bumping their pinned SHA.
    Remediation: Document migration path in release notes.

  • [authorization] harness/review.yaml — The GitLab sandbox env block passes GITLAB_TOKEN into the sandbox — the same token used for write mutations on the runner (REVIEW_TOKEN is set to GITLAB_TOKEN). Unlike the GitHub forge where a separate read-only GH_TOKEN is passed to the sandbox, the GitLab forge has the network policy (access: read-only) as the sole control preventing write mutations from the sandbox.
    Remediation: Consider issuing a separate read-only scoped GitLab token for the sandbox, or document that the network policy is the sole control.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [api-contract] scripts/lib/gitlab-review-ops.lib.sh:111forge_get_pr_files() uses page-based pagination against the GitLab MR /changes endpoint, but this endpoint returns all changes as a nested array within a single JSON response object. The duplicate-detection guard correctly prevents an infinite loop, but one unnecessary extra API call is always made.

  • [reliability] scripts/lib/gitlab-review-ops.lib.sh:112 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

  • [injection] scripts/lib/gitlab-review-ops.lib.sh:112 — In forge_close_pr() and forge_post_comment(), the comment/body parameter is passed to curl via --data-urlencode. While double-quoted (safe from word splitting) and current callers only pass static or internally-constructed strings, the function interface accepts arbitrary strings.

Previous run (10)

Review

Findings

Medium

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:101forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. The --forge flag is new for BOTH GitHub and GitLab. If the upstream change has not landed, both GitHub and GitLab review workflows will fail at the post-review step.
    Remediation: Verify that Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 has merged and the --forge flag works for both github and gitlab values before merging this PR.

  • [breaking-change] harness/review.yaml — Environment variable renamed: GITHUB_PR_URLPR_URL. Structural changes: policy, pre_script, post_script, skills, host_files moved from top-level to forge.github.*/forge.gitlab.* sections. File paths changed (env/review.envenv/github/review.env, policies/review.yamlpolicies/github/review.yaml). Downstream repos with base: composition will need to update overrides when bumping their pinned SHA. Follows the pattern from triage agent PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Document migration path in release notes.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [api-contract] scripts/lib/gitlab-review-ops.lib.sh:117forge_get_pr_files() uses page-based pagination against the GitLab MR /changes endpoint, but this endpoint returns all changes as a nested array within a single JSON response object. The duplicate-detection guard correctly prevents an infinite loop, but one unnecessary extra API call is always made.

  • [authorization] scripts/lib/gitlab-review-ops.lib.sh:112 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

  • [stale-doc] agents/prioritize.md:16 — The prioritize agent still uses GITHUB_ISSUE_URL directly instead of the forge-neutral ISSUE_URL pattern. This creates documentation inconsistency with the triage and code agents which already use ISSUE_URL.
    Remediation: Update agents/prioritize.md to use ISSUE_URL or acknowledge it is GitHub-only until multi-forge support is added.

Previous run (11)

Review

Findings

Medium

  • [secrets-handling] harness/review.yaml:96 — The GitLab forge section injects GITLAB_TOKEN with api scope (read+write) into the sandbox. On GitLab, curl IS the permitted binary in policies/gitlab/review.yaml, and the network policy's access: read-only is the sole control preventing write mutations via curl with the full-scope api token.
    Remediation: Inject a separate read-only scoped GitLab token (read_api scope) for the sandbox.

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:101forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. If the upstream change has not landed, GitLab review workflows will fail at the post-review step.
    Remediation: Ensure Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 is merged before deploying.

  • [breaking-change] harness/review.yaml — Structural changes for multi-forge support: policy, pre_script, post_script, and forge-specific skills/host_files moved from top-level to forge.github.*/forge.gitlab.* sections. File paths changed (env/review.envenv/github/review.env, policies/review.yamlpolicies/github/review.yaml). Env var key renamed GITHUB_PR_URLPR_URL. Downstream repos with base: composition will need to update overrides when bumping their pinned SHA. Follows the pattern from triage agent PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Document migration path in release notes.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/lib/gitlab-review-ops.lib.sh:93forge_get_pr_info() returns {state: "UNKNOWN", isDraft: false} on API failure. The caller in post-review.src.sh produces a clear warning and exits gracefully (exit 0).

  • [api-contract] scripts/lib/gitlab-review-ops.lib.sh:117forge_get_pr_files() uses page-based pagination against the GitLab MR /changes endpoint, but this endpoint returns all changes in a single response object. The duplicate-detection guard prevents an infinite loop but makes one unnecessary extra API call.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-review-ops.lib.sh:154 — In forge_add_label(), the warning message interpolates the label name without sanitization. Goes to stderr (not GHA-parsed stdout), but inconsistent with GitHub counterpart which uses _gha_sanitize().

  • [GHA-workflow-command-injection] scripts/post-review.src.sh:148 — Inconsistent sanitization approaches between the label validation loop (LA_LABEL=${LA_LABEL//::/:}) and _gha_sanitize() (sed 's/::/%3A%3A/g'). The regex allowlist at line 154 independently blocks most injection payloads, but the multi-layered inconsistency creates fragility.

  • [SSRF-host-validation] scripts/lib/gitlab-review-ops.lib.sh:55 — Both forge_validate_pr_url() and forge_parse_pr_url() independently validate the GitLab host against the allowlist. Correctly implemented as defense-in-depth.

  • [authorization] scripts/lib/gitlab-review-ops.lib.sh:112 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

Previous run (12)

Review

Findings

Medium

  • [secrets-handling] harness/review.yaml:99 — The GitLab forge section injects GITLAB_TOKEN with api scope (read+write) into the sandbox. On GitHub, the sandbox policy blocks curl from the binary allowlist, providing defense-in-depth alongside the network policy's access: read-only. On GitLab, curl is allowed (needed for API access), so the network policy is the sole control preventing write mutations.
    Remediation: Inject a separate read-only scoped GitLab token (read_api scope) for the sandbox.

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:129forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. If the upstream change has not landed, GitLab review workflows will fail at the post-review step.
    Remediation: Ensure Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 is merged before deploying.

  • [breaking-change] harness/review.yaml — Structural changes for multi-forge support: policy, pre_script, post_script, and forge-specific skills/host_files moved from top-level to forge.github.*/forge.gitlab.* sections. File paths changed (env/review.envenv/github/review.env, policies/review.yamlpolicies/github/review.yaml). Env var key renamed GITHUB_PR_URLPR_URL. Downstream repos with base: composition will need to update overrides when bumping their pinned SHA. Follows the pattern from triage agent PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Document migration path in release notes.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/lib/gitlab-review-ops.lib.sh:93forge_get_pr_info() returns {state: "UNKNOWN", isDraft: false} on API failure. The caller in post-review.src.sh now produces a clear warning and exits gracefully (exit 0). Improved from prior review.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-review-ops.lib.sh:136 — In forge_add_label(), the warning message on failure interpolates the label name without sanitization. Goes to stderr (not GHA-parsed stdout), but inconsistent with GitHub counterpart which uses _gha_sanitize().

  • [GHA-workflow-command-injection] scripts/post-review.src.sh:148 — Inconsistent sanitization approaches between the label validation loop (LA_LABEL=${LA_LABEL//::/:}) and _gha_sanitize() (sed 's/::/%3A%3A/g'). The regex allowlist at line 154 independently blocks most injection payloads, but the multi-layered inconsistency creates fragility.

  • [SSRF-host-validation] scripts/lib/gitlab-review-ops.lib.sh:55 — Both forge_validate_pr_url() and forge_parse_pr_url() independently validate the GitLab host against the allowlist. Correctly implemented as defense-in-depth. forge_parse_pr_url() duplicates the validation (the triage-ops equivalent does not), but the duplication is a deliberate safety net.

  • [authorization] scripts/lib/gitlab-review-ops.lib.sh:144 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

Previous run (13)

Review

Findings

Medium

  • [secrets-handling] harness/review.yaml:89 — The GitLab forge section injects GITLAB_TOKEN with api scope (read+write) into the sandbox. On GitHub, the sandbox policy blocks curl from the binary allowlist, providing defense-in-depth alongside the network policy's access: read-only. On GitLab, curl is allowed (needed for API access), so the network policy is the sole control preventing write mutations.
    Remediation: Inject a separate read-only scoped GitLab token (read_api scope) for the sandbox.

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:129forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. If the upstream change has not landed, GitLab review workflows will fail at the post-review step.
    Remediation: Ensure Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 is merged before deploying.

  • [breaking-change] harness/review.yaml — Structural changes for multi-forge support: policy, pre_script, post_script, and forge-specific skills/host_files moved from top-level to forge.github.*/forge.gitlab.* sections. File paths changed (env/review.envenv/github/review.env, policies/review.yamlpolicies/github/review.yaml). Env var key renamed GITHUB_PR_URLPR_URL. Downstream repos with base: composition will need to update overrides when bumping their pinned SHA. Follows the pattern from triage agent PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Document migration path in release notes.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/lib/gitlab-review-ops.lib.sh:93forge_get_pr_info() returns {state: "UNKNOWN", isDraft: false} on API failure. The caller in post-review.src.sh now produces a clear warning and exits gracefully (exit 0). Improved from prior review.

  • [GHA-workflow-command-injection] scripts/post-review.src.sh:148 — Inconsistent sanitization approaches between the label validation loop (LA_LABEL=${LA_LABEL//::/:}) and _gha_sanitize() (sed 's/::/%3A%3A/g'). The regex allowlist at line 154 independently blocks most injection payloads, but the multi-layered inconsistency creates fragility.

  • [SSRF-host-validation] scripts/lib/gitlab-review-ops.lib.sh:55 — Both forge_validate_pr_url() and forge_parse_pr_url() independently validate the GitLab host against the allowlist. Correctly implemented as defense-in-depth.

  • [authorization] scripts/lib/gitlab-review-ops.lib.sh:144 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

  • [naming-consistency] scripts/lib/github-review-ops.lib.sh:133forge_list_labels() diverges from the established triage-ops naming convention forge_list_repo_labels(). Both github-triage-ops.lib.sh and gitlab-triage-ops.lib.sh use forge_list_repo_labels().

  • [code-organization] scripts/lib/gitlab-review-ops.lib.sh:68forge_parse_pr_url() duplicates the GitLab host allowlist validation already performed by forge_validate_pr_url(). The triage-ops equivalent does NOT duplicate the host validation.

  • [incomplete-doc] FEATURES.md:94 — The generated-scripts list enumerates six scripts but does not include scripts/pre-review.sh and scripts/post-review.sh, which this PR converts to generated bundles built from .src.sh sources via make script-build.

Previous run (14)

Review

Findings

Medium

  • [error-handling] scripts/lib/github-review-ops.lib.sh:45 — GitHub forge_get_pr_info() has no error handling. If gh pr view fails, the function propagates the non-zero exit code. Under set -euo pipefail, this terminates the post-review script immediately, bypassing the UNKNOWN state handler (post-review.src.sh:54) that was added for exactly this scenario. The GitLab counterpart catches the failure and returns {state: "UNKNOWN", isDraft: false}, enabling graceful skip.
    Remediation: Add error handling matching the GitLab pattern: gh pr view ... 2>/dev/null || { jq -n '{state: "UNKNOWN", isDraft: false}'; return; }.

  • [secrets-handling] harness/review.yaml:89 — The GitLab forge section injects GITLAB_TOKEN with api scope (read+write) into the sandbox. On GitHub, the sandbox policy blocks curl from the binary allowlist, providing defense-in-depth alongside the network policy's access: read-only. On GitLab, curl is allowed (needed for API access), so the network policy is the sole control preventing write mutations.
    Remediation: Inject a separate read-only scoped GitLab token (read_api scope) for the sandbox.

  • [upstream-dependency] scripts/lib/gitlab-review-ops.lib.sh:129forge_post_review() calls fullsend post-review --forge gitlab, which depends on Make fullsend post-review multi-forge (add --forge flag) fullsend#6240. If the upstream change has not landed, GitLab review workflows will fail at the post-review step.
    Remediation: Ensure Make fullsend post-review multi-forge (add --forge flag) fullsend#6240 is merged before deploying.

  • [breaking-change] harness/review.yaml — Structural changes for multi-forge support: policy, pre_script, post_script, and forge-specific skills/host_files moved from top-level to forge.github.*/forge.gitlab.* sections. File paths changed (env/review.envenv/github/review.env, policies/review.yamlpolicies/github/review.yaml). Env var key renamed GITHUB_PR_URLPR_URL. Downstream repos with base: composition will need to update overrides when bumping their pinned SHA. Follows the pattern from triage agent PR feat: make triage agent multi-forge (GitHub + GitLab) #686.
    Remediation: Document migration path in release notes.

  • [protected-path] agents/review.md, harness/review.yaml, policies/, scripts/, skills/, .github/, AGENTS.md — This PR modifies 18 files under protected paths (agents/, harness/, policies/, scripts/, skills/, .github/, AGENTS.md). The PR links to issue Make review agent multi-forge (GitHub + GitLab) #814 which authorizes the multi-forge work, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [logic-error] scripts/lib/gitlab-review-ops.lib.sh:99jq -r '.draft // false' uses jq's alternative operator which treats false as falsy, producing the correct result by coincidence. Functionally correct in all cases but relies on the coincidental equivalence of both branches.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-review-ops.lib.sh:189 — Inconsistent sanitization approaches between the label validation loop (LA_LABEL=${LA_LABEL//::/:}) and _gha_sanitize(). The defense chain currently blocks injection, but the multi-layered inconsistency creates fragility.

  • [naming-consistency] scripts/lib/gitlab-review-ops.lib.sh:98_gha_sanitize() references "GHA" (GitHub Actions) but is called from GitLab forge operations. The ::warning:: workflow command syntax emitted by forge_add_label() is GHA-specific and will appear as literal text in GitLab CI logs.

  • [error-handling] scripts/lib/gitlab-review-ops.lib.sh:93forge_get_pr_info() returns {state: "UNKNOWN", isDraft: false} on API failure. The caller now produces a clear warning and exits gracefully. Improved from prior review.

  • [edge-case] scripts/lib/gitlab-review-ops.lib.sh:123forge_get_pr_files() pagination uses duplicate detection and sort -u. On pre-15.7 GitLab instances without pagination support, the heuristic correctly breaks. Net result is correct.

  • [test-adequacy] scripts/post-review-test.sh:1450 — No GitLab test for forge_get_pr_info API failure path (UNKNOWN state handler). The curl mock always succeeds.

  • [test-adequacy] scripts/pre-review-test.sh:334 — No GitLab test for closed/merged MR state-check path. The curl mock always returns opened.

  • [SSRF-host-validation] scripts/lib/gitlab-review-ops.lib.sh:55 — Both forge_validate_pr_url() and forge_parse_pr_url() now independently validate the GitLab host against the allowlist. Improved from prior review.

  • [authorization] scripts/lib/gitlab-review-ops.lib.sh:144 — Non-atomic close-with-comment on GitLab: note post and state change are separate API calls with || true. If the note succeeds but the close fails, the MR has a misleading comment without being closed.

  • [incomplete-doc] docs/review.md:18 — The Triggers section uses "PR" exclusively without mentioning GitLab MRs. The Variables table in the same file was updated to use "PR/MR", creating terminology inconsistency.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment review-agent labels Aug 15, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from c5b0ed7 to 8864cf8 Compare August 15, 2026 12:25
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:26 PM UTC · Completed 12:44 PM UTC

Commit: 8864cf8 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 8864cf8 to 7e97bca Compare August 15, 2026 12:48
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:50 PM UTC · Completed 1:06 PM UTC

Commit: 7e97bca · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 7e97bca to 1327121 Compare August 15, 2026 17:13
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:14 PM UTC · Completed 5:33 PM UTC

Commit: 1327121 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 1327121 to 20d17b7 Compare August 15, 2026 17:40
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:42 PM UTC · Completed 6:00 PM UTC

Commit: 20d17b7 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 15, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 20d17b7 to b62861d Compare August 15, 2026 18:03
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:04 PM UTC · Completed 6:23 PM UTC

Commit: b62861d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 15, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from b62861d to 0fa7dfb Compare August 15, 2026 18:32
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:33 PM UTC · Completed 6:48 PM UTC

Commit: 0fa7dfb · View workflow run →

@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 7175f79 to 501c42b Compare August 18, 2026 22:14
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:16 PM UTC · Completed 10:38 PM UTC

Commit: 501c42b · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 18, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 501c42b to 2bb686f Compare August 18, 2026 23:07
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:09 PM UTC · Completed 11:28 PM UTC

Commit: 2bb686f · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the enhancement New feature or request label Aug 18, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 2bb686f to 6412b50 Compare August 18, 2026 23:34
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:36 PM UTC · Completed 11:54 PM UTC

Commit: 6412b50 · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself August 18, 2026 23:54

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 18, 2026
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 6412b50 to 4a07bcb Compare August 19, 2026 00:00
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:02 AM UTC · Completed 12:19 AM UTC

Commit: 4a07bcb · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ggallen
ggallen force-pushed the agent/814-review-multi-forge branch from 4a07bcb to f3fe5b5 Compare August 19, 2026 00:21
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:23 AM UTC · Completed 12:41 AM UTC

Commit: f3fe5b5 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.


Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • scripts/lib/github-review-ops.lib.sh:68: [medium] breaking-cli

forge_post_review() passes --forge github/gitlab to the fullsend CLI. This new required argument must be supported by the deployed fullsend binary. Depends on fullsend-ai/fullsend#6240.

Suggested fix: Ensure the deployed fullsend CLI supports --forge before exercising these scripts in production.

  • harness/review.yaml (file-level): Line 17 · [low] breaking-config

skills/issue-labels/github removed from top-level skills array, moved to forge.github.skills. Downstream harnesses using base: composition may need to adjust skill references.

  • harness/review.yaml:36: [low] architectural-coherence

pre_script and post_script removed from top-level config. Unlike triage (which supports non-forge invocations), the review agent always requires a forge context, making top-level scripts unnecessary.

  • scripts/lib/github-review-ops.lib.sh:59: [low] pattern-inconsistency

forge_parse_pr_url() sets global REPO while harness env blocks pass REPO_FULL_NAME. The old cross-validation was dropped. Architecturally intentional — PR_URL is now the single source of truth.

  • scripts/lib/gitlab-review-ops.lib.sh:10: [low] secret-exposure

_gitlab_api() passes REVIEW_TOKEN via --header on the curl command line, visible in /proc//cmdline. Mitigated by ephemeral runners and ::add-mask:: in the post-script.

Suggested fix: Consider passing token via netrc file, curl config, or stdin to avoid process listing visibility.

  • scripts/lib/gitlab-review-ops.lib.sh:41: [low] fail-open

forge_get_pr_state() returns UNKNOWN for unrecognized GitLab states. Post-review script handles UNKNOWN by skipping review (exit 0) with a warning — intentionally fail-open to avoid blocking reviews during API unavailability.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 19, 2026
@ggallen
ggallen added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit 816b89b Aug 19, 2026
56 of 58 checks passed
@ggallen
ggallen deleted the agent/814-review-multi-forge branch August 19, 2026 01:38
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 19, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ❌ Failure · Started 1:40 AM UTC · Completed 1:40 AM UTC

Commit: f3fe5b5 · View workflow run →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ready-for-review Triggers review agent dispatch review-agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make review agent multi-forge (GitHub + GitLab)

1 participant