Skip to content

fix(ci): stop blocking PRs with tool harness enforcement#184

Merged
joryirving merged 2 commits into
mainfrom
fix/ai-pr-review-tuning
Jun 5, 2026
Merged

fix(ci): stop blocking PRs with tool harness enforcement#184
joryirving merged 2 commits into
mainfrom
fix/ai-pr-review-tuning

Conversation

@joryirving

Copy link
Copy Markdown
Collaborator

Problem

The AI PR review workflow has tool_failure_enforcement: true, which means the action will force request_changes on PRs when the tool harness cannot gather sufficient evidence. Since the action is pinned to v1.0.18 (pre-PR #121 which fixed fail-closed semantics), enforcement here is actually broken — but even when it works, tool harness enforcement is too strict for a general-purpose review where the model may not always need or be able to call tools.

For repos using publish_mode: review_verdict with allow_approve: true, this blocks PRs from being merged when the review can't collect tool evidence.

Change

Removes tool_failure_enforcement and tool_min_successful_requests from the workflow inputs. The action defaults ("false" and "0") apply instead, making the tool harness explicitly best-effort:

  • The AI review still runs and evaluates code changes
  • The tool harness still attempts to collect evidence when the planner decides to
  • But tool failures no longer force request_changes
  • Remaining tools (standards file, linked sources, image digest analysis, etc.) are unaffected

Related

Same fix applied across all misospace repos to unblock PR automation.

The AI PR review workflow has tool harness enforcement enabled
(tool_failure_enforcement: true), which blocks PRs when the model
cannot gather tool evidence. This makes tool harness explicitly
best-effort so that the AI review still runs and evaluates code,
but tool failures no longer force request_changes.

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Recommendation: Approve

This PR removes tool_failure_enforcement and tool_min_successful_requests from the AI PR review workflow, making the tool harness explicitly best-effort. The change is targeted, well-reasoned, and aligns with the stated problem.

Change-by-Change Findings

.github/workflows/ai-pr-review.yaml

Functional changes (removals):

  • Removed tool_failure_enforcement: 'true' — this was forcing request_changes when the tool harness couldn't gather sufficient evidence
  • Removed tool_min_successful_requests: '1' — this required at least 1 successful tool request

Cosmetic changes (quote normalization):

  • All workflow input values normalized from single quotes ('8') to double quotes ("8") — this is standard YAML convention and does not affect action behavior

Effect

With publish_mode: review_verdict and allow_approve: 'true' retained, removing these enforcement parameters allows the AI review to proceed and potentially approve PRs even when the tool harness cannot collect evidence. The action defaults ("false" and "0") apply instead.

Standards Compliance

No repository standards file (AGENTS.md) applies directly to CI workflow configuration. The change is a workflow tuning adjustment with no impact on:

  • Game code (main.gd, game_state.gd)
  • Build/test pipelines (test.yml, release.yml)
  • Godot toolchain configuration
  • Project architecture or UX

Linked Issue Fit

No linked issue provided. The PR body is self-contained and describes the problem clearly: tool harness enforcement was too strict for a general-purpose review workflow, and with review_verdict + allow_approve mode, tool failures were incorrectly blocking PR merges.

Evidence Provider Findings

No evidence providers configured for this repository.

Tool Harness Findings

The tool harness output confirms the scenario this PR addresses:

  • Planned requests: 0 — the harness couldn't plan any tool calls
  • Planning warning: Could not parse planning response as JSON — evidence gathering failed

Under the previous configuration, this state would have forced request_changes on the PR. With this change, such tool harness failures become non-blocking.

Unknowns / Needs Verification

  1. PR #121 context: The PR body references "pre-PR #121 which fixed fail-closed semantics" but no linked issue or PR #121 details are available. The action is pinned to v1.0.18 via commit hash f838c5b49d72d11dd33cfee6e29b85a28b5aa8df. If the fail-closed semantics fix mentioned is not present in this version, the enforcement removal is even more critical.

  2. Consistency across misospace repos: The PR body states "Same fix applied across all misospace repos" — this is good for consistency but cannot be verified in this single-repo scan.

Conclusion

The change is low-risk (workflow configuration only), solves the stated problem directly, and the tool harness findings validate the need for this adjustment. Recommend approval.

@joryirving
joryirving enabled auto-merge June 5, 2026 17:18
@joryirving
joryirving merged commit 4acda1b into main Jun 5, 2026
4 checks passed
@joryirving
joryirving deleted the fix/ai-pr-review-tuning branch June 5, 2026 17:19

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Recommendation: Approve

This PR cleanly removes tool_failure_enforcement and tool_min_successful_requests from the AI PR review workflow, making the tool harness explicitly best-effort rather than blocking.

Change-by-Change Findings

Change Assessment
Removed tool_failure_enforcement: 'true' Correct — eliminates forced request_changes when tool harness cannot gather evidence
Removed tool_min_successful_requests: '1' Correct — removes minimum successful tool request requirement
Quote normalization (single → double) Cosmetic only — YAML accepts both, no functional impact

Standards Compliance

  • Workflow conventions: Action version remains pinned (v1.0.18) with full SHA commit hash. This follows existing patterns in the repository for pinned action versions.
  • AGENTS.md applicability: No AGENTS.md conventions apply to GitHub Actions workflow configuration. The PR targets CI infrastructure, not game code or game logic.
  • No regressions introduced: The change only removes restrictive parameters; all other tool settings (tool_max_requests, tool_planning_timeout_sec, tool_planning_max_context_bytes, tool_planning_max_tokens, tool_max_response_bytes, tool_allowed_gh_api_repos, tool_request_timeout_sec, tool_enable_for_forks) remain intact and unchanged.

Linked Issue Fit

No linked issues found. The PR body is self-contained and clearly articulates:

  • Problem: tool_failure_enforcement: true blocks PRs with publish_mode: review_verdict + allow_approve: true when tool harness cannot collect evidence
  • Solution: Remove enforcement parameters, relying on action defaults ("false" and "0") to make tool harness best-effort
  • Scope: Single workflow file modification with no side effects

Evidence Provider Findings

No evidence providers configured for this repository.

Tool Harness Findings

No blocking findings. The tool harness reported "Could not parse planning response as JSON" which is a non-blocking informational message.

Unknowns / Needs Verification

None — the diff is minimal, self-explanatory, and addresses the stated problem directly.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant