Skip to content

feat(#860): make fix agent multi-forge (GitHub + GitLab) - #872

Merged
ggallen merged 1 commit into
mainfrom
agent/860-fix-multi-forge
Aug 21, 2026
Merged

feat(#860): make fix agent multi-forge (GitHub + GitLab)#872
ggallen merged 1 commit into
mainfrom
agent/860-fix-multi-forge

Conversation

@fullsend-ai-coder

@fullsend-ai-coder fullsend-ai-coder Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds GitLab support to the fix agent using the established three-file lib pattern (fix-ops.lib.sh dispatcher + github-fix-ops.lib.sh + gitlab-fix-ops.lib.sh) from the code and triage agents.

Key changes:

  • Forge ops libs: New dispatcher and per-forge operation libraries with forge_* functions for PR/MR head ref lookup, push remote setup, token management, label operations, and comment posting
  • post-fix.src.sh: Replaced all direct gh CLI calls with forge-dispatched forge_* functions; sources fix-ops.lib.sh for automatic forge selection
  • pre-fix.src.sh: Converted from standalone pre-fix.sh to bundled source with forge-neutral workspace and PATH handling via forge_get_workspace_dir and forge_append_path
  • harness/fix.yaml: Added complete forge.gitlab section (policy, skills, host_files, env) and enriched forge.github with FULLSEND_FORGE, skills, and host_files
  • process-fix-result.py: Forge-aware PR/MR comment posting (gh pr comment for GitHub, curl POST to notes API for GitLab)
  • Network policy: policies/gitlab/fix.yaml allows curl/git/node/pre-commit (no gh), GitLab API hosts, and package registries
  • Skills split: skills/fix-review/ shared methodology kept at top level; forge-specific CLI recipes moved to github/ and gitlab/ subdirs
  • Env normalization: env/github/fix.env and env/gitlab/fix.env map forge-specific input vars to uniform names

Shellcheck fixes (from prior attempts):

  • Added # shellcheck disable=SC2153 for PR_NUMBER env var references alongside pr_number local variables in post-failure-report.lib.sh and post-fix.src.sh
  • Added # shellcheck disable=SC2030,SC2031 for intentional subshell exports in post-fix-test.sh

Testing

  • All existing post-fix-test.sh tests pass (push retry, pre-commit retry, result resolution, branch mismatch, numeric validation, security integration)
  • New GitLab test: gitlab-api-failure-fails-closed verifies curl-based API failure triggers fail-closed behavior
  • New GitLab test: gitlab-api-failure-fails-closed-no-gh-calls verifies no gh binary invocations in GitLab mode
  • process-fix-result-test.py — all 30 tests pass
  • make check-bundle passes (bundled scripts match source)
  • shellcheck passes on all modified scripts (only info-level SC1091/SC2001 remain)

Closes #860

Post-script verification

  • Branch is not main/master (agent/860-fix-multi-forge)
  • Secret scan passed (gitleaks — 107b558888fb5c3eb76e2ac4e1754d33a65214e5..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 18, 2026 15:02
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 18, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:04 PM UTC · Completed 3:24 PM UTC

Commit: 815f336 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/ (13 files), skills/fix-review/ (3 files) — This PR modifies 16 files under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and explains the rationale (adding GitLab support to the fix agent using the established three-file lib pattern). Human approval is always required for protected-path changes, regardless of context.

Low

  • [token-exposure] scripts/lib/gitlab-fix-ops.lib.sh:59 — _gitlab_api() passes GITLAB_TOKEN via --header on the curl command line, exposing it in /proc/PID/cmdline. The Python counterpart (process-fix-result.py) uses --config - (stdin) to avoid this. This matches the existing pattern in gitlab-code-ops.lib.sh and is consistent with the codebase, not a regression.

  • [token-masking] scripts/lib/gitlab-fix-ops.lib.sh:76 — forge_mask_token is a no-op on GitLab CI since ::add-mask:: is GHA-specific. This is intentional (the code comment notes it), but GITLAB_TOKEN should be configured as a "masked" CI/CD variable in GitLab project settings to compensate.

  • [code-organization] scripts/lib/github-fix-ops.lib.sh:16 — The gha_echo fallback uses a sanitization implementation distinct from the two existing patterns in the codebase. The fallback is guarded by declare -F and rarely fires at runtime (the canonical gha_echo from post-failure-report.lib.sh loads first in bundled scripts), so impact is minimal.

  • [bot-detection-bypass] scripts/lib/fix-ops.lib.sh:24 — is_bot_user for GitLab checks the _bot suffix, a naming convention without platform enforcement (unlike GitHub's reserved [bot]). A human user named john_bot would be misclassified. Security impact is limited: misclassification only affects iteration cap logic (lower bot cap of 5 vs human cap of 10).

  • [edge-case] scripts/lib/gitlab-fix-ops.lib.sh — forge_validate_pr_url regex requires {2,} path segments after the host, rejecting root-level GitLab projects. Negligible given the allowlist restriction to gitlab.com and gitlab.cee.redhat.com.

  • [privilege-scope] harness/fix.yaml — The GitLab sandbox receives GITLAB_TOKEN. The gitlab-fix-ops.lib.sh header documents minimum scope as api (read/write), while the harness comment states the token is "read-only scoped." The network policy (policies/gitlab/fix.yaml) enforces read-only at the network level, but the documentation is contradictory.

  • [test-adequacy] scripts/post-fix-test.sh — No test verifies is_bot_user with GitLab-style bot usernames (e.g., project_123_bot). The bot detection logic affects iteration cap enforcement and trigger source normalization.

Previous run

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/, skills/ — 16 of 22 changed files are under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 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

  • [stale-inline-docs] scripts/pre-fix.src.sh — The header comment for TRIGGER_SOURCE says "usernames ending in [bot] are bot triggers" which only describes the GitHub convention. post-fix.src.sh was updated to "GitHub: [bot] suffix; GitLab: _bot suffix" but pre-fix.src.sh was not. The HUMAN_INSTRUCTION comment also retains the GitHub-only wording.

  • [code-organization] harness/fix.yaml — The forge.github and forge.gitlab blocks omit pre_script and post_script, relying on top-level inheritance. This deviates from code.yaml and review.yaml which explicitly set pre_script/post_script in every forge block. If the runtime supports top-level inheritance, this is a valid simplification; if not, the GitLab forge will lack scripts at runtime.

  • [code-organization] harness/fix.yaml — PUSH_TOKEN, PUSH_TOKEN_SOURCE, and REPO_FULL_NAME were promoted from forge-specific env.runner blocks to the common env.runner section. Several sandbox vars were similarly promoted. This diverges from the code.yaml pattern where these variables remain forge-specific, creating a cross-agent inconsistency.

  • [token-isolation] harness/fix.yaml — GITLAB_TOKEN with api scope is passed into the sandbox, unlike GitHub where GH_TOKEN is a separate read-only scoped token. This follows the established code agent pattern (code.yaml uses the same approach) and the network policy enforces read-only access as the primary control.

  • [error-handling-gap] scripts/lib/gitlab-fix-ops.lib.sh — _gitlab_api passes GITLAB_TOKEN via --header on the command line, making it visible in /proc/*/cmdline. The Python implementation (process-fix-result.py) uses --config stdin to avoid this. This follows the established shell pattern in gitlab-code-ops.lib.sh but is inconsistent with the Python path.

  • [token-masking] scripts/lib/gitlab-fix-ops.lib.sh — forge_mask_token() is a no-op on native GitLab CI (GITHUB_ACTIONS not set). This follows the established pattern, relying on GitLab CI's native variable masking. The comment correctly notes ::add-mask:: is GHA-specific.

  • [naming-convention] scripts/lib/fix-ops.lib.sh — is_bot_user() is placed in the dispatcher rather than in forge-specific files. No other dispatcher (code-ops.lib.sh, triage-ops.lib.sh, etc.) defines cross-forge helpers. This is a reasonable design choice since the function needs forge context, but departs from the convention where dispatchers contain only the case-switch.

Previous run (2)

Review

Findings

Medium

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 16 files under protected paths (governance and infrastructure): agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/lib/fix-ops.lib.sh, scripts/lib/github-fix-ops.lib.sh, scripts/lib/gitlab-fix-ops.lib.sh, scripts/post-fix-test.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/pre-fix.sh, scripts/pre-fix.src.sh, scripts/process-fix-result-test.py, scripts/process-fix-result.py, skills/fix-review/SKILL.md, skills/fix-review/github/SKILL.md, skills/fix-review/gitlab/SKILL.md. The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-fix-ops.lib.sh:24 — The GitLab fallback gha_echo function (gha_echo() { echo "::${1}::${2:-}"; }) does not sanitize arguments for :: sequences or %0A/%0D encoded newlines, unlike the GitHub version in github-fix-ops.lib.sh which strips these. In pre-fix.src.sh, only fix-ops.lib.sh is sourced (no post-failure-report.lib.sh), so the unsanitized version is the only gha_echo available when FULLSEND_FORGE=gitlab. While :: workflow commands are inert on GitLab CI (they are GitHub Actions-specific), the inconsistency creates a defense-in-depth gap. This matches the existing pattern in gitlab-code-ops.lib.sh and is not a regression.
    Remediation: Apply the same sanitization in the GitLab fallback gha_echo that exists in the GitHub version for consistency.

  • [token-exposure] scripts/lib/gitlab-fix-ops.lib.sh:33 — _gitlab_api passes GITLAB_TOKEN via --header "PRIVATE-TOKEN: ${GITLAB_TOKEN}" as a direct command-line argument to curl, making it visible in /proc/*/cmdline and ps output. By contrast, process-fix-result.py correctly uses --config - with stdin to avoid token exposure. This is a pre-existing pattern across all gitlab-*-ops.lib.sh files in the codebase, not a regression introduced by this PR.
    Remediation: Refactor _gitlab_api to pass the token via curl's --config - (stdin) pattern, matching process-fix-result.py.

Low

  • [token-masking-gap] scripts/lib/gitlab-fix-ops.lib.sh:113 — forge_mask_token is a no-op on GitLab CI since ::add-mask:: is GHA-specific and GitLab CI has no equivalent in-script masking command. This is a deliberate design choice matching the existing gitlab-code-ops.lib.sh pattern.

  • [naming-convention] scripts/lib/gitlab-fix-ops.lib.sh:28 — The internal API helper is named _gitlab_api, matching 4 of 5 existing agents (prioritize, retro, review, triage). The code agent uses the namespaced _gitlab_code_api. Since bundled scripts only source one agent's ops lib, no runtime collision occurs, but namespacing to _gitlab_fix_api would be more defensive.

  • [code-organization] harness/fix.yaml:62 — Several env vars (PUSH_TOKEN, TRIGGER_SOURCE, GIT_AUTHOR_*, etc.) are hoisted to top-level env.runner/env.sandbox blocks rather than duplicated per forge block as in code.yaml. This follows retro.yaml's pattern and is functionally equivalent since the harness merges top-level and forge-specific env blocks. PUSH_TOKEN correctly remains in env.runner only (never enters sandbox).

  • [fail-open] scripts/lib/gitlab-fix-ops.lib.sh:56 — forge_validate_pr_url uses two-step validation (regex then host allowlist case statement). If the case statement were accidentally removed, the regex alone would accept any host. The same two-step pattern exists across all gitlab-*-ops.lib.sh files, and the network policy provides an additional defense layer.

Previous run (3)

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/ (10 files), skills/ (3 files) — This PR modifies 16 files under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and provides clear rationale for the changes (multi-forge support for the fix agent). Human approval is always required for protected-path changes, regardless of context.

Low

  • [GHA workflow command injection] scripts/lib/gitlab-fix-ops.lib.sh:26 — The fallback gha_echo does not sanitize its message argument, unlike the GitHub equivalent which strips :: sequences and encoded newlines. In pre-fix.sh (GitLab mode), the unsanitized fallback is active since post-failure-report.lib.sh is not sourced. The attack surface is limited: interpolated values are regex-validated before use, and the cross-forge scenario (GitLab mode on GHA) is architecturally unusual. Consistent with existing gitlab-code-ops.lib.sh pattern.
    Remediation: Apply the same sanitization for defense-in-depth.

  • [token masking gap] scripts/lib/gitlab-fix-ops.lib.sh:93 — forge_mask_token() is a no-op outside GitHub Actions. On native GitLab CI, tokens are not masked via ::add-mask::. This follows the existing pattern in gitlab-code-ops.lib.sh and gitlab-retro-ops.lib.sh — a known limitation, not a regression.
    Remediation: Document that tokens must be configured as masked CI/CD variables in GitLab project settings.

  • [error-handling-idiom] scripts/lib/gitlab-fix-ops.lib.sh:33 — _gitlab_api() does not include an inline GITLAB_HOST guard, unlike gitlab-prioritize-ops.lib.sh. The callers (post-fix.src.sh) perform extensive GITLAB_HOST validation before any API call. Consistent with gitlab-code-ops.lib.sh which also omits this guard.
    Remediation: Consider adding a guard inside _gitlab_api() for defense-in-depth.

  • [token handling] harness/fix.yaml:64 — PUSH_TOKEN and PUSH_TOKEN_SOURCE are moved from per-forge forge.github.env.runner to the shared env.runner block, deviating from harness/code.yaml where these remain per-forge. Functionally safe (env.runner vars never enter the sandbox).

  • [naming-convention] scripts/lib/gitlab-fix-ops.lib.sh:25 — The gha_echo fallback passes only $2, while github-fix-ops uses shift + $*. Multi-word messages may be silently truncated. Consistent with existing gitlab-code-ops.lib.sh.

  • [stale-doc] docs/fix.md:157 — The new "Multi-forge support" section does not document PR_URL as a forge-neutral variable or the GitLab host allowlist pattern (three-file sync requirement), unlike the equivalent section in docs/code.md.

Info

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

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/* (13 files), skills/fix-review/* (3 files) — This PR modifies 16 files under protected paths (agents/, harness/, policies/, scripts/, skills/). Human approval is always required for protected-path changes, regardless of context. The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and explains the rationale for multi-forge support.

  • [api-shape-pattern] scripts/lib/gitlab-fix-ops.lib.sh:30 — The _gitlab_fix_api helper passes the GitLab token via --config - (stdin here-string), while every other GitLab ops file in the codebase (gitlab-code-ops.lib.sh, gitlab-review-ops.lib.sh, gitlab-retro-ops.lib.sh, gitlab-triage-ops.lib.sh, gitlab-prioritize-ops.lib.sh) passes it via --header. The --config - approach is more secure (prevents token from appearing in /proc/*/cmdline), but should be adopted codebase-wide rather than diverging in one agent.
    Remediation: Use --header "PRIVATE-TOKEN: ${GITLAB_TOKEN}" to match the existing pattern, or adopt --config - codebase-wide in a separate PR.

Low

  • [error-handling-gap] scripts/lib/gitlab-fix-ops.lib.sh:91 — forge_set_push_remote does not validate that GITLAB_HOST is non-empty before constructing the push URL. Current callers always set GITLAB_HOST before calling this function, but a guard would provide defense-in-depth.

  • [token-masking-gap] scripts/lib/gitlab-fix-ops.lib.sh:104 — forge_mask_token is a no-op on GitLab CI runners (guards on GITHUB_ACTIONS=true). This is a pre-existing design choice shared with other agents (e.g., gitlab-retro-ops.lib.sh), not a regression.

  • [naming-convention] scripts/lib/gitlab-fix-ops.lib.sh:28 — Internal helper _gitlab_fix_api deviates from the _gitlab_api naming convention used in 4 of 5 other GitLab ops files. File-scoped, so no cross-file impact.

  • [stale-comment] scripts/post-fix.src.sh:39, scripts/pre-fix.src.sh:15 — TRIGGER_SOURCE comment says "forge username" but still only documents the GitHub [bot] convention, omitting the GitLab _bot suffix used by is_bot_user().

  • [test-adequacy] scripts/post-fix-test.sh — No explicit test for GitLab forge_set_push_remote URL format (oauth2:token@host/repo.git) or forge_setup_push_token exporting GITLAB_TOKEN. The happy-path integration test provides indirect coverage.

  • [edge-case] scripts/lib/fix-ops.lib.sh:26 — is_bot_user uses _bot$ regex for GitLab bots, which could match legitimate human usernames. Documented as a deliberate simplification in agents/fix.md.

  • [error-handling-idiom] scripts/lib/gitlab-fix-ops.lib.sh:24 — gha_echo fallback uses multi-line sanitization (stripping ::, %0A, %0D), while the established pattern in other GitLab ops files uses a simple one-liner. The new version is more thorough.

  • [fail-open-risk] scripts/post-fix.src.sh:80 — REPO_FULL_NAME validation else branch accepts nested paths for non-GitHub forges. Mitigated by :? expansion and fix-ops.lib.sh case statement ensuring FULLSEND_FORGE is always github or gitlab.

  • [api-shape-pattern] scripts/lib/gitlab-fix-ops.lib.sh:95 — forge_mask_token() accepts a parameter, while gitlab-retro-ops.lib.sh's version is parameterless. The parameterized version is functionally necessary for the fix agent (masking PUSH_TOKEN).

  • [code-organization] scripts/lib/fix-ops.lib.sh:24 — is_bot_user defined in the dispatch wrapper rather than forge-specific files. Minor deviation from the pattern where dispatchers contain only the case statement.

Previous run (5)

Review

Findings

Medium

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

  • [scope-creep] scripts/lib/gitlab-code-ops.lib.sh:33 — The --config stdin token handling changes to gitlab-code-ops.lib.sh and gitlab-triage-ops.lib.sh (plus their bundled outputs post-code.sh, pre-code.sh, post-triage.sh, pre-triage.sh) are security improvements to the code and triage agents, not the fix agent. Issue Make fix agent multi-forge (GitHub + GitLab) #860 authorizes only fix agent multi-forge work. Consider documenting in the PR description that these cross-agent changes were intentionally bundled as an opportunistic security improvement, or splitting them to a separate PR.

Low

  • [token-masking] scripts/lib/gitlab-fix-ops.lib.sh:109 — forge_mask_token is a no-op on GitLab CI by design (the ::add-mask:: mechanism is GHA-specific). Tokens must be masked via GitLab CI/CD variable settings instead. The code comment documents this intentional behavior, but no script-level defense-in-depth exists for the GitLab forge.

  • [field-ordering] harness/fix.yaml:78 — The forge.github and forge.gitlab block field ordering differs from the established pattern in harness/code.yaml (e.g., pre_script/post_script placed before providers/openshell; policy placed after pre_script/post_script instead of first). Consider reordering to match.

Previous run (6)

Review

Findings

Medium

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

Low

  • [token-masking] scripts/lib/gitlab-fix-ops.lib.sh:130 — forge_mask_token() on the GitLab path is a no-op when GITHUB_ACTIONS is not true. GitLab CI handles token masking through CI/CD variable settings rather than runtime log commands. The inline comment documents this behavior. Low risk since the --config stdin pattern throughout the PR prevents token exposure in process listings.

  • [test-coverage] scripts/post-fix-test.sh:1066 — GitLab integration tests use a two-segment REPO_FULL_NAME (test-group/test-project). No integration test exercises a deeply nested GitLab group path (e.g., group/subgroup/project) to verify URL-encoding of REPO_ENCODED works correctly end-to-end through the curl API URL construction.
    Remediation: Add a GitLab integration test case with a multi-segment REPO_FULL_NAME.

  • [env-contract] harness/fix.yaml — FULLSEND_FORGE is now required by both pre-fix.sh and post-fix.sh. Consumers extending this harness via base: composition must ensure FULLSEND_FORGE is set. This matches the established pattern in code.yaml.

  • [host-allowlist-consistency] scripts/lib/gitlab-fix-ops.lib.sh:94 — The GitLab host allowlist is maintained in three separate locations (gitlab-fix-ops.lib.sh, process-fix-result.py, policies/gitlab/fix.yaml). Cross-referencing comments are present, but synchronization remains manual.

  • [naming-convention] skills/fix-review/gitlab/SKILL.md — Uses --config - pattern for token passing while the shared skills/gitlab-forge/SKILL.md uses --header directly. The --config - pattern is more secure; the shared skill was not updated in this PR.

Previous run (7)

Review

Findings

Medium

  • [harness-structure-inconsistency] harness/fix.yaml — The PR removes pre_script and post_script from the forge.github block without adding them to forge.gitlab. The top-level pre_script/post_script remain, but every other multi-forge harness (code.yaml, triage.yaml) redundantly specifies these in each forge block. If the harness runtime does not fall through to top-level scripts when forge-specific ones are absent, the scripts will not execute for either forge.
    Remediation: Add pre_script: scripts/pre-fix.sh and post_script: scripts/post-fix.sh to both forge.github and forge.gitlab blocks, matching the pattern in code.yaml and triage.yaml.

Low

  • [token-masking-gap] scripts/lib/gitlab-fix-ops.lib.sh — forge_mask_token on GitLab CI is a no-op because GITHUB_ACTIONS is not true. The code has a comment acknowledging this design choice (::add-mask:: is GHA-specific) and uses defense-in-depth (--config stdin for tokens, sanitized logging). GitLab CI provides platform-level variable masking.

  • [scope-creep] scripts/lib/gitlab-triage-ops.lib.sh, scripts/lib/gitlab-code-ops.lib.sh — The PR modifies triage and code agent GitLab ops libs to change curl token passing from --header to --config stdin. These files are not fix-agent files. The change is a security improvement (prevents token exposure in /proc/*/cmdline) that establishes a consistent pattern across all GitLab ops libs.

  • [function-signature-inconsistency] scripts/lib/github-fix-ops.lib.sh:17 — The gha_echo fallback in the new fix-ops libs uses a sanitizing implementation (strips ::, %0A, %0D) that differs from the existing fallback in gitlab-code-ops.lib.sh (simple one-liner). In production, post-failure-report.lib.sh defines a comprehensive gha_echo before any ops lib loads, so these fallbacks rarely execute.

  • [test-adequacy] scripts/post-fix-test.sh — GitLab integration tests pre-set REPO_ENCODED and GITLAB_HOST explicitly, partly bypassing the derivation logic in post-fix.src.sh. The key security validations (host mismatch, invalid host, missing PR_URL) are covered by separate dedicated tests.

  • [bot-user-detection] scripts/lib/fix-ops.lib.sh:22 — is_bot_user for GitLab uses _bot$ suffix matching. A human user with a username ending in _bot would be classified as a bot (applying bot iteration cap 5 instead of human cap 10). This is fail-safe (more restrictive) and matches the documented convention.

  • [token-handling-pattern] scripts/lib/gitlab-fix-ops.lib.sh — Missing _gitlab_fix_api_with_status variant that other GitLab ops libs provide. No current callers need it — the fix agent's API usage (get MR head ref, post comments, manage labels) is simpler than code/triage agents.

Info

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 23 files under protected paths. Issue Make fix agent multi-forge (GitHub + GitLab) #860 authorizes these changes as part of the fix agent multi-forge implementation. Note: intent-coherence analysis confirms authorization via linked issue. Human approval is still required for protected-path changes.

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

Previous run (8)

Review

Findings

Medium

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — 23 files in this PR modify governance and infrastructure paths (agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/lib/fix-ops.lib.sh, scripts/lib/github-fix-ops.lib.sh, scripts/lib/gitlab-fix-ops.lib.sh, and 17 others). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [token-masking-gap] scripts/lib/gitlab-fix-ops.lib.sh — forge_mask_token is a no-op on GitLab CI since ::add-mask:: is GHA-specific. This is intentional per the issue description (pattern sync: port triage agent changes from scaffold #8: "no-op, harmless on GitLab CI") and documented in the code. GitLab CI's native masked-variable feature is expected to handle masking at the project settings level.

  • [scope-creep] scripts/lib/gitlab-triage-ops.lib.sh, scripts/lib/gitlab-code-ops.lib.sh — Token-passing pattern changed from --header to --config stdin in the triage and code agent GitLab ops libs, which are outside issue Make fix agent multi-forge (GitHub + GitLab) #860's fix-agent scope. The change maintains consistency with the new gitlab-fix-ops.lib.sh pattern and is a security improvement (tokens no longer appear in /proc/*/cmdline), but the generated scripts post-triage.sh, pre-code.sh, and post-code.sh are also affected.

  • [naming-convention] scripts/lib/gitlab-fix-ops.lib.sh:24, scripts/lib/github-fix-ops.lib.sh:18 — gha_echo fallback definitions use a multi-line sanitizing implementation that differs from the simpler one-liner in gitlab-code-ops.lib.sh. The new version is closer to the canonical sanitizing version in post-failure-report.lib.sh and is justified by the fix-agent scripts' sourcing order (fix-ops is sourced before post-failure-report).

  • [bot-user-detection-bypass] scripts/lib/fix-ops.lib.sh:24 — The GitLab bot detection pattern _bot$ is broader than GitHub's [bot]$. A human user whose username ends in _bot would get the stricter bot iteration cap (safe direction), while a service account not ending in _bot would get the less strict human cap. This convention is documented in agents/fix.md and skills/fix-review/SKILL.md.

  • [test-coverage] scripts/post-fix-test.sh — GitLab integration tests mock curl globally with pattern-based dispatch. New API calls added to the post-fix script in the future would silently succeed via the catch-all exit 0.

Previous run (9)

Review

Findings

Medium

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 23 files under protected paths (governance and infrastructure). The changes are authorized by issue Make fix agent multi-forge (GitHub + GitLab) #860 (make fix agent multi-forge) and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

  • [scope-creep] scripts/lib/gitlab-triage-ops.lib.sh, scripts/lib/gitlab-code-ops.lib.sh — The PR applies a security improvement (switching token passing from --header to --config stdin) to the triage and code agent GitLab ops libs, which is outside the fix-agent scope authorized by issue Make fix agent multi-forge (GitHub + GitLab) #860. The change is beneficial (prevents token exposure in /proc/*/cmdline) but modifies infrastructure for agents not covered by the issue. Consider expanding issue Make fix agent multi-forge (GitHub + GitLab) #860's scope to cover this cross-cutting improvement, or splitting it into a separate PR.

  • [pattern-inconsistency] scripts/lib/github-fix-ops.lib.sh:18 — The gha_echo fallback in both fix-ops libs uses a multi-line sanitizing implementation (stripping ::, %0A, %0D), while the established pattern in gitlab-code-ops.lib.sh uses a simple one-liner (gha_echo() { echo "::${1}::${2:-}"; }). Both work correctly, but the inconsistency between agents' fallback behavior is worth aligning.

Low

  • [test-inadequate] scripts/post-fix-test.sh:653 — The GitLab happy-path integration test pre-sets REPO_ENCODED and GITLAB_HOST, bypassing the PR_URL-based derivation logic in post-fix.src.sh. A test variant that relies solely on PR_URL derivation would increase confidence.

  • [test-inadequate] scripts/post-fix-test.sh:926 — Pre-fix validation tests only exercise FULLSEND_FORGE=gitlab. No tests for pre-fix.src.sh with FULLSEND_FORGE=github, so the GitHub-specific REPO_FULL_NAME validation is untested.

  • [code-organization] scripts/lib/gitlab-fix-ops.lib.sh:113 — forge_mask_token() is defined in both fix-ops libs but never called. post-fix.src.sh directly uses inline ::add-mask:: masking instead of the forge function. This is dead code.

  • [code-organization] scripts/pre-fix.src.sh:31 — is_bot_user() is defined identically in pre-fix.src.sh and post-fix.src.sh. The codebase pattern is to place shared helpers in .lib.sh files. Consider extracting to fix-ops.lib.sh.

  • [api-shape] scripts/lib/github-fix-ops.lib.sh:70 — forge_get_pr_head_ref() hardcodes PUSH_TOKEN fallback internally rather than accepting a token parameter like the code-ops counterpart (forge_get_default_branch). Minor API shape inconsistency between agent libraries.

  • [error-handling-idiom] scripts/pre-fix.src.sh:48 — Validation errors use raw echo '::error::...' while other parts of the same file use the sanitizing gha_echo helper. Inconsistent approach to workflow command output.

  • [edge-case] agents/fix.md:64 — The GitLab bot detection heuristic uses _bot$ suffix, which could produce false positives for human usernames ending in _bot. The risk is minimal given GitLab's naming conventions, and the pattern is applied consistently across all code paths.

  • [logic-error] scripts/lib/gitlab-fix-ops.lib.sh:78 — forge_get_pr_head_ref depends on REPO_ENCODED being set by the caller before invocation. The contract is documented in the file header, and all current call sites satisfy it, but there is no defensive local check.

  • [naming-convention] scripts/lib/gitlab-fix-ops.lib.sh:68 — Section header # --- PR/MR operations --- appears in both GitHub and GitLab fix-ops files. The GitHub code-ops pattern omits /MR since GitHub does not have merge requests.

Info

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

Review

Findings

Medium

  • [token-isolation] harness/fix.yaml — The GitLab forge section passes the same GITLAB_TOKEN to both runner (env.runner) and sandbox (env.sandbox). On GitHub, the sandbox receives a separate, read-only GH_TOKEN while only the runner gets PUSH_TOKEN. For GitLab, the network policy (policies/gitlab/fix.yaml, access: read-only) is the sole enforcement layer for sandbox read-only behavior — there is no token-level isolation. Consider provisioning separate read-only and read-write GitLab tokens, or documenting that the network policy is the sole enforcement boundary for GitLab sandbox isolation.

Low

  • [harness-comment-staleness] harness/fix.yaml:9 — The top-level header comment (lines 9–11) still uses pre-multi-forge wording. The forge-section comment was updated in this PR, but the top-level comment was not.
  • [naming-convention] scripts/lib/github-fix-ops.lib.sh:18 — The gha_echo fallback in the new fix-ops libs uses a sanitizing implementation (stripping ::, %0A, %0D) that diverges from the existing one-liner fallback in gitlab-code-ops.lib.sh. The new version is more secure; consider upgrading the older code-ops lib to match.
  • [logic-error] scripts/lib/github-fix-ops.lib.sh:33 — The GitHub forge_validate_pr_url regex accepts PR number 0 ([0-9]+$) while the GitLab equivalent correctly requires a positive integer ([1-9][0-9]*$). Mitigated by upstream PR_NUMBER validation in pre-fix.src.sh and post-fix.src.sh, but the inconsistency could surface if the function is used in a different context.

Info

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 23 files under protected paths. Note: intent-coherence analysis cites issue Make fix agent multi-forge (GitHub + GitLab) #860 as explicit authorization for this change. Human approval is still required for protected-path changes.
  • [provenance-warning] — Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.
Previous run (11)

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/*, skills/* — This PR modifies 17 files under protected paths (agents/, harness/, policies/, scripts/, skills/). The linked issue (Make fix agent multi-forge (GitHub + GitLab) #860) and PR description provide clear rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [pattern-inconsistency] scripts/lib/github-fix-ops.lib.sh:18 — The gha_echo fallback uses a multi-line sanitizing implementation that differs from the one-liner in gitlab-code-ops.lib.sh. The multi-line form is actually a security hardening (prevents workflow command injection via :: and %0A/%0D stripping) — the inconsistency favors the new code. Consider aligning the older one-liner to the hardened form.

  • [edge-case] scripts/lib/gitlab-fix-ops.lib.sh:47 — forge_validate_pr_url regex uses [1-9][0-9]* (rejects MR IID 0) while gitlab-review-ops.lib.sh uses [0-9]+. The stricter pattern is correct (GitLab IIDs start at 1) but the cross-agent inconsistency could cause confusion.

  • [error-handling] scripts/lib/gitlab-fix-ops.lib.sh:97 — forge_mask_token emits ::add-mask:: which is GHA-specific and a no-op on GitLab CI. Documented as harmless in the code.

  • [workflow-command-injection] scripts/lib/gitlab-fix-ops.lib.sh:39 — gha_echo sanitizes message body but not the lvl parameter. All call sites pass string literals — not exploitable in current code.

  • [scope-creep] scripts/lib/gitlab-code-ops.lib.sh — The PR also retrofits gitlab-code-ops.lib.sh and gitlab-triage-ops.lib.sh with the curl --config stdin token-passing pattern, which is outside the stated issue scope but is a small, consistent security improvement.

  • [code-organization] scripts/post-fix.src.sh:55 — SCRIPT_DIR aliased to SCRIPT_DIR_POST with SC2034 disable but no comment explaining the indirection. A # Used by fix-ops.lib.sh comment would clarify.

  • [code-organization] scripts/lib/gitlab-fix-ops.lib.sh:54 — Cross-reference comment "matches the secure pattern in process-fix-result.py" is also applied to gitlab-code-ops.lib.sh and gitlab-triage-ops.lib.sh where it is slightly misleading since those agents do not use process-fix-result.py.

  • [pattern-inconsistency] scripts/post-fix-test.sh — GitLab tests use several one-off named functions rather than the reusable parameterized test runner pattern from existing tests.

Previous run (12)

Review

Findings

Medium

  • [workflow-command-injection] scripts/lib/github-fix-ops.lib.sh:18 — The GitHub forge's gha_echo function passes its second argument directly to echo without sanitizing GHA workflow command metacharacters (::, %0A, %0D). The GitLab forge's gha_echo in gitlab-fix-ops.lib.sh correctly sanitizes these. An attacker who controls values interpolated into gha_echo calls could inject additional workflow commands. Note: the unsanitized pattern is pre-existing in other *-ops.lib.sh files; the GitLab fix-ops introduces a new, more secure variant.
    Remediation: Apply the same sanitization pattern from gitlab-fix-ops.lib.sh to github-fix-ops.lib.sh.

  • [API-shape-inconsistency] scripts/lib/github-fix-ops.lib.sh:68 — forge_create_label omits --force, inconsistent with every other github-*-ops.lib.sh file (github-code-ops.lib.sh, github-triage-ops.lib.sh, github-review-ops.lib.sh). Without --force, gh label create fails silently (via || true) when the label already exists rather than updating its description/color. This diverges from the established contract where forge_create_label is idempotent and can update label metadata.
    Remediation: Add --force to the gh label create call.

  • [protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 23 files under protected paths (agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, 17 files under scripts/, 3 files under skills/). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and the description explains the rationale for multi-forge support. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling-gap] scripts/lib/gitlab-fix-ops.lib.sh:89 — forge_get_pr_head_ref temporarily swaps GITLAB_TOKEN with PUSH_TOKEN for an elevated API call using a manual save/restore pattern. If execution is interrupted between swap and restore, the token remains elevated. Practical risk is minimal since the post-script already has PUSH_TOKEN available, but the GitHub counterpart scopes the override to a single command line, which is safer.

  • [scope-creep] scripts/lib/gitlab-triage-ops.lib.sh:37 — This PR modifies the triage and code agent GitLab ops (gitlab-triage-ops.lib.sh, gitlab-code-ops.lib.sh) to switch from --header to --config stdin for token passing. While a reasonable security hardening related to the fix agent work, it is outside the strict scope of issue Make fix agent multi-forge (GitHub + GitLab) #860. The scope extension is minor and reduces security debt.

  • [edge-case] scripts/lib/gitlab-fix-ops.lib.sh:65 — forge_validate_pr_url's regex accepts MR IID 0 ([0-9]+). Not exploitable since PR_NUMBER is separately validated to require [1-9], but inconsistent with the overall validation contract.

  • [token-handling] scripts/lib/gitlab-fix-ops.lib.sh:96 — forge_mask_token uses ::add-mask:: which is GHA-specific and ineffective on GitLab CI. Tokens should be configured as masked CI variables at the GitLab project level for defense-in-depth.

  • [gha-echo-inconsistency] scripts/lib/gitlab-fix-ops.lib.sh:33 — The gha_echo fallback uses an inline sanitization pattern unique to this file, creating a third variant across the codebase (simple passthrough in gitlab-code-ops.lib.sh and github-fix-ops.lib.sh, sanitized in gitlab-fix-ops.lib.sh, and the sanitize_gha_log_output approach in post-failure-report.lib.sh).

Previous run (13)

Review

Findings

High

  • [missing function definition] scripts/pre-fix.src.sh:160 — On the GitHub forge path (FULLSEND_FORGE=github), pre-fix.src.sh calls gha_echo at top-level script execution (lines 160–162 and 183) but gha_echo is never defined for that path. The script only sources fix-ops.lib.sh, which dispatches to github-fix-ops.lib.sh; neither defines gha_echo. The GitLab path works because gitlab-fix-ops.lib.sh conditionally defines gha_echo. Under set -euo pipefail, if a target repo has .pre-commit-config.yaml but the companion resolve/install scripts are missing, the script crashes with gha_echo: command not found. The base-branch pre-fix.sh used raw echo "::warning::" and did not have this problem. The same issue affects the bundled pre-fix.sh.
    Remediation: Add a conditional gha_echo definition to github-fix-ops.lib.sh matching the pattern in gitlab-fix-ops.lib.sh: if ! declare -F gha_echo >/dev/null 2>&1; then gha_echo() { ... }; fi. Then re-run make script-build to regenerate pre-fix.sh.

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/ (16 files), skills/ (3 files) — 23 of 29 changed files are under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 which explicitly authorizes making the fix agent multi-forge, and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [token masking gap] scripts/lib/gitlab-fix-ops.lib.sh:698 — forge_mask_token() emits ::add-mask:: which is GHA-specific and a no-op on GitLab CI (acknowledged in code comment). When running on GitLab CI, token values will not be masked by this function. The --config - pattern prevents tokens from appearing in /proc/*/cmdline, and GitLab CI natively supports masked variables. This is a defense-in-depth documentation gap.
    Remediation: Document that GitLab CI variables configured as masked: true are required for PUSH_TOKEN and GITLAB_TOKEN.

  • [fallback-definition-inconsistency] scripts/lib/gitlab-fix-ops.lib.sh:23 — The gha_echo fallback in gitlab-fix-ops.lib.sh uses a multi-line sanitizing implementation while gitlab-code-ops.lib.sh uses a simpler one-liner. When the authoritative gha_echo from post-failure-report.lib.sh is loaded, these fallbacks are never reached. The fix-ops version is more secure; the inconsistency is a code hygiene issue.

  • [bot user detection broadening] scripts/pre-fix.src.sh:574 — The is_bot_user function was broadened to match _bot$ for GitLab. The suffix pattern is broad — any user whose username ends in _bot gets the stricter bot iteration cap (5 instead of 10). GitLab bot users conventionally use this suffix, so false positives are unlikely but possible.

  • [stale-doc] docs/fix.md:118 — The "What the agent does not read" section says "The fix agent does not read GitHub Actions logs" but the fix agent now supports GitLab CI. The rest of docs/fix.md was updated for multi-forge support but this sentence was missed.
    Remediation: Change "GitHub Actions logs" to "CI logs" to be forge-neutral.

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 (14)

Review

Findings

Medium

  • [api-contract] scripts/lib/gitlab-fix-ops.lib.sh:86 — The GitLab forge_get_pr_head_ref uses the ambient GITLAB_TOKEN for the API call, whereas the GitHub equivalent (github-fix-ops.lib.sh) explicitly overrides with PUSH_TOKEN via GH_TOKEN="${PUSH_TOKEN:-${GH_TOKEN:-}}". In post-fix.src.sh, forge_get_pr_head_ref is called (line 162) before forge_setup_push_token upgrades the token (line 428). If the runner's GITLAB_TOKEN has insufficient scope to read MR metadata, the head-ref lookup will fail — a behavioral divergence from the GitHub path.
    Remediation: In forge_get_pr_head_ref in gitlab-fix-ops.lib.sh, temporarily override GITLAB_TOKEN with PUSH_TOKEN for the API call, matching the GitHub pattern.

  • [error-handling-gap] scripts/post-fix.src.sh:59 — The ERR trap (trap 'report_post_failure_to_pr' ERR) is set on line 84, but fix-ops.lib.sh is sourced on line 59 — before the trap is armed. If FULLSEND_FORGE is invalid, fix-ops.lib.sh calls exit 1 before the trap can catch it, silently terminating the post-script without posting a failure comment to the PR.
    Remediation: Move source fix-ops.lib.sh after the ERR trap setup (line 84), or add : "${FULLSEND_FORGE:?...}" before the source to produce a detectable error.

  • [protected-path] Multiple protected files modified: agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, 17 files under scripts/, and 3 files under skills/fix-review/. The PR links to issue Make fix agent multi-forge (GitHub + GitLab) #860 and provides clear rationale for all protected-path changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [missing-validation] scripts/pre-fix.src.sh:22 — FULLSEND_FORGE is not validated before sourcing fix-ops.lib.sh, which can exit 1 on invalid values. Other pre-scripts (e.g., pre-review.src.sh) use : "${FULLSEND_FORGE:?...}" for cleaner early-failure.

  • [test-inadequate] scripts/process-fix-result-test.py:509 — TestGitLabPostComment does not test the missing-GITLAB_TOKEN path. The code raises ValueError when gitlab_token is empty, but no test exercises this branch.

  • [edge-case] scripts/post-fix.src.sh:111 — When FULLSEND_FORGE=gitlab and PR_URL is empty, forge_validate_pr_url emits a misleading error ending with a colon and no value.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-fix-ops.lib.sh:23 — The fallback gha_echo sanitizes :: and %0A/%0D but omits ANSI escape sequences. In pre-fix.src.sh, post-failure-report.lib.sh is not sourced, so this weaker fallback is active. Practical risk is low because interpolated values are regex-validated.

  • [GHA-workflow-command-injection] scripts/lib/gitlab-code-ops.lib.sh:25 — Pre-existing fallback gha_echo in this file performs no sanitization. This PR touches the file for curl token improvements but the unsanitized fallback is unchanged.

  • [code-organization] scripts/pre-fix.src.sh:27 — is_bot_user is duplicated identically in pre-fix.src.sh and post-fix.src.sh. Could be factored into fix-ops.lib.sh.

  • [naming-convention] scripts/pre-fix.src.sh:151 — Uses raw echo "::warning::..." workflow commands instead of gha_echo, while post-fix.src.sh uses gha_echo consistently.

  • [code-organization] scripts/post-fix.src.sh:55 — SCRIPT_DIR is set to SCRIPT_DIR_POST on line 55, then re-derived identically via cd/dirname on line 433. The re-derivation is redundant.

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 (15)

Review

Findings

Medium

  • [protected-path] agents/fix.md, harness/fix.yaml, policies/gitlab/fix.yaml, scripts/*, skills/* — This PR modifies 23 files under governance/infrastructure protected paths (agents/, harness/, policies/, scripts/, skills/). The linked issue Make fix agent multi-forge (GitHub + GitLab) #860 and PR description provide clear authorization context for these changes. Human approval is always required for protected-path changes.

Low

  • [missing cross-check] scripts/post-fix.src.sh — The GitLab validation block validates PR_URL format and GITLAB_HOST match but does not cross-check REPO_FULL_NAME or PR_NUMBER against PR_URL, unlike pre-fix.src.sh which performs both checks. Defense-in-depth favors parity across pre- and post-scripts.
  • [false-positive risk in bot detection] scripts/pre-fix.src.sh:29 — is_bot_user uses _bot$ as a GitLab bot suffix heuristic, which could match legitimate human usernames. Consequences are safe-side (lower iteration cap). Documented as deliberate in agents/fix.md.
  • [missing format validation] scripts/post-fix.src.sh — Validates PR_NUMBER format but not REPO_FULL_NAME format (pre-fix.src.sh validates both). Low risk given pre-fix sequencing guarantee.
  • [Secrets handling] scripts/lib/gitlab-fix-ops.lib.sh:93 — forge_mask_token emits ::add-mask:: which is a no-op on GitLab CI. Tokens should be configured as masked CI/CD variables in GitLab settings.
  • [inconsistent-fallback-helper] scripts/lib/gitlab-fix-ops.lib.sh:23 — The gha_echo fallback in gitlab-fix-ops.lib.sh uses a sanitizing implementation that differs from the simpler one-liner in gitlab-code-ops.lib.sh. The new sanitizing version is the better pattern; peer files should be updated to match.

Info

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

Review

Findings

Medium

  • [naming-convention] skills/fix-review/github/SKILL.md:3 — The forge-specific skill frontmatter uses name: fix-review, which does not include the forge suffix. The established pattern in this codebase (e.g., skills/pr-review/github/SKILL.md uses name: pr-review-github) appends the forge name to distinguish forge-specific variants from the base skill.
    Remediation: Change name: fix-review to name: fix-review-github.

  • [naming-convention] skills/fix-review/gitlab/SKILL.md:3 — Same issue: the forge-specific skill uses name: fix-review instead of name: fix-review-gitlab, matching the base skill's name exactly and breaking the convention used by pr-review-gitlab.
    Remediation: Change name: fix-review to name: fix-review-gitlab.

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

Low

  • [stale-reference] skills/fix-review/SKILL.md:431 — The PR systematically renamed "GitHub username" to "forge username" for TRIGGER_SOURCE across agents/fix.md, docs/fix.md, and earlier sections of this file, but missed one occurrence in the structured output section where it still says "(the GitHub username)".
    Remediation: Change "(the GitHub username)" to "(the forge username)".

  • [token-masking] scripts/lib/gitlab-fix-ops.lib.sh:96 — forge_mask_token() uses GHA-specific ::add-mask:: which is a no-op on GitLab CI (the code documents this with a comment). GITLAB_TOKEN should be configured as a masked CI/CD variable in GitLab project settings for equivalent log protection.

  • [edge-case] scripts/lib/gitlab-fix-ops.lib.sh:35 — _gitlab_fix_api reads the token via stdin heredoc consumed by --config -. If a caller passes args also needing stdin (e.g., --data @-), the call would conflict. No current callers do this; same pattern exists in code and triage ops libs.

  • [secrets-handling] scripts/lib/gitlab-fix-ops.lib.sh:82 — forge_set_push_remote() embeds the token in the git remote URL. If a subsequent git command fails and prints the remote URL, the token may appear in logs. The GitHub equivalent has the same pattern, and callers use print_sanitized_gha_log to mitigate.

  • [scope-creep] scripts/lib/gitlab-code-ops.lib.sh — Changes to existing GitLab ops libs (gitlab-code-ops.lib.sh, gitlab-triage-ops.lib.sh) apply curl --config stdin security hardening consistently across all GitLab curl wrappers. These are outside the fix-agent scope but are reasonable consistency improvements.

  • [scope-creep] scripts/lib/post-failure-report.lib.sh — Shellcheck disable comment (SC2153) added, unrelated to the multi-forge feature. Benign.

  • [error-handling-idiom] scripts/lib/gitlab-fix-ops.lib.sh:23 — The gha_echo fallback uses a more defensive sanitizing pattern (stripping ::, %0A, %0D) than gitlab-code-ops.lib.sh's simpler echo pattern. The inconsistency means the two GitLab ops files use different shapes for the same function.

Previous run (17)

Review

Findings

Medium

  • [protected-path] agents/fix.md — This PR modifies 23 files under protected governance/infrastructure paths (agents/, harness/, policies/, scripts/, skills/). The PR is authorized by issue Make fix agent multi-forge (GitHub + GitLab) #860 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

  • [error-message-idiom] scripts/lib/fix-ops.lib.sh:22 — The fallback error message says "set FULLSEND_FORGE to 'github' or 'gitlab'" but the established pattern in code-ops.lib.sh and review-ops.lib.sh is "pass --forge <github|gitlab> or set FULLSEND_FORGE".
    Remediation: Align the error message with the established pattern.

  • [stale-documentation] docs/network-policy.md:100 — The parenthetical "(code agent also has a GitLab equivalent under policies/gitlab/)" is now stale. This PR adds policies/gitlab/fix.yaml, so the fix agent also has a GitLab equivalent.
    Remediation: Change to "(both agents also have GitLab equivalents under policies/gitlab/)".

Low

  • [GHA-workflow-command-injection] scripts/lib/gitlab-fix-ops.lib.sh:163 — The gha_echo fallback does not sanitize arguments for :: sequences or encoded newlines. In practice, the risk is minimal: in post-fix.src.sh the sanitizing version from post-failure-report.lib.sh takes precedence, and in pre-fix.src.sh no user-controlled values flow through gha_echo. Consider aligning the fallback with the sanitized pattern for defense-in-depth.

  • [token-masking-gap] scripts/lib/gitlab-fix-ops.lib.sh:133 — forge_mask_token() uses ::add-mask:: which is GitHub Actions-specific. On GitLab CI this is a no-op — tokens are not masked in job logs. The --config - pattern prevents /proc exposure, and GitLab CI's native variable masking (project settings) is the standard mitigation.

  • [bot-detection-expansion] scripts/post-fix.src.sh:69 — is_bot_user() now matches _bot$ suffix for GitLab bots. Any username ending in _bot would be subject to the stricter bot iteration cap (5 vs 10). Impact is limited to iteration budgeting.

  • [host-validation-consistency] scripts/lib/gitlab-fix-ops.lib.sh:98 — The GitLab host allowlist is maintained in three separate locations (gitlab-fix-ops.lib.sh, process-fix-result.py, policies/gitlab/fix.yaml) with cross-reference comments but no single source of truth. Future additions could miss one location.

Info

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

Review

Findings

Low

  • [missing function implementation] scripts/lib/github-fix-ops.lib.sh — github-fix-ops.lib.sh does not implement forge_get_workflow_run_url, but gitlab-fix-ops.lib.sh does. The analogous pair (github-code-ops.lib.sh / gitlab-code-ops.lib.sh) both implement this function. post-failure-report.lib.sh has a fallback that produces correct GitHub URLs when the function is absent, so there is no runtime error, but the interface contract is incomplete.
    Remediation: Add forge_get_workflow_run_url to the Workspace operations section of github-fix-ops.lib.sh, matching github-code-ops.lib.sh.

  • [gha_echo definition inconsistency] scripts/lib/gitlab-fix-ops.lib.sh:24 — The gha_echo fallback uses a third distinct implementation (manual percent-encoding of %, newlines, ::) that differs from both gitlab-code-ops.lib.sh (simple one-liner) and post-failure-report.lib.sh (sanitize_gha_log_output). In the fix pipeline, post-failure-report.lib.sh is sourced first, so this fallback is effectively dead code during normal operation.
    Remediation: Adopt one of the existing gha_echo implementations for consistency.

  • [missing input validation] scripts/process-fix-result.py — _post_comment_gitlab validates gitlab_host is non-empty and checks it against the allowlist, but does not validate that gitlab_token is non-empty before constructing the PRIVATE-TOKEN header. Mitigated because forge_setup_push_token sets GITLAB_TOKEN from the already-validated PUSH_TOKEN.
    Remediation: Add a guard: if not gitlab_token: raise ValueError('GITLAB_TOKEN is not set').

  • [incomplete-enumeration] FEATURES.md:94 — The enumeration of generated/bundled scripts now omits scripts/pre-fix.sh. The PR adds pre-fix.src.sh to BUNDLE_SRCS and generates pre-fix.sh, but FEATURES.md's list does not include it.
    Remediation: Add scripts/pre-fix.sh to the enumeration in FEATURES.md.

  • [new-env-contract] harness/fix.yaml — FULLSEND_FORGE is now required by fix-ops.lib.sh (exits with error if unset or unrecognized). External systems invoking the fix harness outside the forge block mechanism must set FULLSEND_FORGE=github. Note: docs/fix.md already documents this variable.

  • [token-handling] scripts/lib/gitlab-fix-ops.lib.sh:131 — forge_mask_token emits ::add-mask:: which is a no-op on GitLab CI. Token masking on GitLab depends on CI-level variable configuration. Mitigated by the --config stdin pattern preventing token exposure in process listings.

  • [bot-user detection] scripts/lib/gitlab-fix-ops.lib.sh — is_bot_user matches _bot suffix. On GitLab, human users could create usernames ending in _bot and would receive the lower bot iteration cap (5 instead of 10). Fails safe since bot triggers have fewer iterations, not more.

  • [scope-expansion] scripts/lib/gitlab-triage-ops.lib.sh, scripts/lib/gitlab-code-ops.lib.sh — PR modifies existing GitLab ops libs to change curl token passing from --header to --config stdin. Not in the issue's file list but is a reasonable security coherence improvement aligning all GitLab API helpers.

Info

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

  • [protected-path] — 24 of 27 changed files are under protected paths (agents/, harness/, policies/, scripts/, skills/). Note: intent-coherence review cites issue Make fix agent multi-forge (GitHub + GitLab) #860 as explicit authorization for this change. Human approval is still required for protected-path changes.

Labels: PR adds GitLab support to the fix agent, which is a new capability (enhancement).

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen

ggallen commented Aug 18, 2026

Copy link
Copy Markdown
Member

All review findings addressed in the amended commit:

High:

  • missing-runtime-globals — Added GITLAB_HOST and REPO_ENCODED derivation from validated PR_URL early in post-fix.src.sh (with host mismatch check against any pre-set GITLAB_HOST).

Medium:

  • incomplete-forge-conversion (x2) — Replaced GITHUB_WORKSPACE references in Section 2 (pre-commit tool fallback) and Section 5 (process-fix-result.py lookup) with forge_get_workspace_dir().
  • token-exposure — Added ::add-mask::${GITLAB_TOKEN} after the PUSH_TOKEN mask.
  • token-exfiltration (post-fix.src.sh) — Added forge_validate_pr_url + host cross-check block (mirrors post-code.src.sh pattern).
  • token-exfiltration (process-fix-result.py) — Added ALLOWED_GITLAB_HOSTS allowlist validation; removed silent fallback to gitlab.com (now raises ValueError).
  • test-inadequate — Added gitlab-happy-path test before the API failure test, added PR_URL to test env.
  • stale-doc (x2) — Updated PR diff references to "Forge-specific skill", added FULLSEND_FORGE to Variables table.
  • missing-doc — Added "Multi-forge support" section to docs/fix.md.
  • protected-path — Acknowledged, human approval required.

Low:

  • skill-naming-consistency (x2) — Changed both fix-review-github and fix-review-gitlab to fix-review.
  • gha-workflow-command-injection — Sanitized the gha_echo fallback in gitlab-fix-ops.lib.sh (escapes %, newlines, :: sequences).
  • stale-doc (x2) — Updated URL support table for forge-neutral language; noted GitLab URL behavior.

@ggallen
ggallen dismissed fullsend-ai-review[bot]’s stale review August 18, 2026 15:44

All findings addressed in amended commit 5a36919.

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 815f336 to ed0c101 Compare August 19, 2026 14:26
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:28 PM UTC · Completed 2:46 PM UTC

Commit: ed0c101 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment enhancement New feature or request labels Aug 19, 2026
@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from ed0c101 to 5d94a02 Compare August 19, 2026 14:52
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 2:53 PM UTC · Ended 2:58 PM UTC

Commit: 5d94a02 · View workflow run →

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 5d94a02 to 8cc1180 Compare August 19, 2026 14:57
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 2:59 PM UTC · Ended 3:08 PM UTC

Commit: 8cc1180 · View workflow run →

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 8cc1180 to 7bbcdab Compare August 19, 2026 15:07
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:10 PM UTC · Completed 3:30 PM UTC

Commit: 7bbcdab · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 7bbcdab to 22d4fff Compare August 19, 2026 15:34
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Commit: 22d4fff · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 22d4fff to 8c42598 Compare August 19, 2026 16:00
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:02 PM UTC · Completed 4:19 PM UTC

Commit: 8c42598 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 8c42598 to 8fc6d8f Compare August 19, 2026 16:22
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:24 PM UTC · Completed 4:46 PM UTC

Commit: 8fc6d8f · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen ggallen mentioned this pull request Aug 20, 2026
Closed
fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from e5dc730 to baafde7 Compare August 20, 2026 21:35
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:37 PM UTC · Ended 9:51 PM UTC

Commit: baafde7 · View workflow run →

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from baafde7 to fd90ec0 Compare August 20, 2026 21:51
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:53 PM UTC · Completed 10:33 PM UTC

Commit: fd90ec0 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from fd90ec0 to 639a633 Compare August 20, 2026 22:39
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:41 PM UTC · Completed 11:19 PM UTC

Commit: 639a633 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 639a633 to 3266fd0 Compare August 20, 2026 23:21
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:23 PM UTC · Completed 11:43 PM UTC

Commit: 3266fd0 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Sign in to view

@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 3266fd0 to 2e0340f Compare August 20, 2026 23:46
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:48 PM UTC · Completed 12:07 AM UTC

Commit: 2e0340f · View workflow run →

@ggallen @claude
feat(#807): make fix agent multi-forge (GitHub + GitLab)
f92b08e 
Signed-off-by: Greg Allen <gallen@redhat.com>
Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ggallen
ggallen force-pushed the agent/860-fix-multi-forge branch from 2e0340f to f92b08e Compare August 21, 2026 00:13
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:15 AM UTC · Completed 12:33 AM UTC

Commit: f92b08e · View workflow run →

fullsend-ai-review[bot]
fullsend-ai-review Bot reviewed Aug 21, 2026
View reviewed changes

@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.

Comment thread scripts/lib/gitlab-fix-ops.lib.sh
Comment thread scripts/lib/gitlab-fix-ops.lib.sh
Comment thread scripts/lib/github-fix-ops.lib.sh
Comment thread scripts/lib/fix-ops.lib.sh
@ggallen
ggallen added this pull request to the merge queue Aug 21, 2026
Merged via the queue into main with commit 9b85a9a Aug 21, 2026
78 checks passed
@ggallen
ggallen deleted the agent/860-fix-multi-forge branch August 21, 2026 00:53
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:54 AM UTC · Completed 1:06 AM UTC

Commit: f92b08e · View workflow run →

@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 21, 2026

Copy link
Copy Markdown

PR #872 (feat: make fix agent multi-forge) took 3 days from issue creation to merge, involving ~34 agent runs across triage (3), code (4), review (~25), and fix (2) stages. The dominant failure mode was shellcheck violations caught by the post-script's authoritative pre-commit check: the code agent failed 3 times and the fix agent failed 2 times before the human took over manual fixes. The code agent only succeeded after the human provided explicit step-by-step instructions to run make script-build then pre-commit run --all-files.

The review agent found one genuine high-severity bug (missing gha_echo definition on the GitHub path in pre-fix.src.sh) and ~20 actionable findings across 18 review cycles, but also produced significant noise: ~115 inline comments with only ~18% driving actual code changes. Key quality issues included contradictory findings across cycles (advising --config - then flagging it as inconsistent, wanting --header back, then flagging --header as token exposure) and re-raising already-acknowledged findings 5–10 times each (token masking no-op, bot detection heuristic).

Several existing open issues cover the review quality problems observed here:

One novel proposal targets the root cause of the 5 shellcheck-related agent failures.

Proposals filed

@fullsend-ai-retro fullsend-ai-retro Bot mentioned this pull request Aug 21, 2026
Open
@fullsend-ai-coder fullsend-ai-coder Bot mentioned this pull request Aug 21, 2026
Open
3 tasks
@ggallen ggallen mentioned this pull request Aug 21, 2026
Closed
32 tasks
@fullsend-ai-retro fullsend-ai-retro Bot mentioned this pull request Aug 23, 2026
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ggallen ggallen ggallen approved these changes
+1 more reviewer
@fullsend-ai-review fullsend-ai-review[bot] fullsend-ai-review[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@ggallen ggallen

Labels

enhancement New feature or request fix-agent ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make fix agent multi-forge (GitHub + GitLab)

1 participant

@ggallen