fix: make CI review advisory by default (#23)

[fatherlinux]

Closes #23.

Problem

The reusable review workflow ran gatehouse --stdin --comment, which exits 1 on critical/high findings and submits a REQUEST_CHANGES review. When a consumer wired it as a required status check, a non-deterministic LLM finding could block an otherwise-green PR. That's exactly what happened on crunchtools/mcp-trentina#56 — three runs produced three different sets of blocking HIGH findings, every one a verifiable false positive, forcing an admin-merge.

The crunchtools constitution (§XII) already declares Gatehouse an advisory gate. The implementation just didn't honor it. This makes it so.

Changes

• review.yml: new blocking input (default false) → the workflow passes --advisory unless a caller explicitly opts in with blocking: true. The check now exits 0 by construction.
• review.py / github.py: in advisory mode, post a plain COMMENT review instead of REQUEST_CHANGES (which otherwise lingers as an unresolved review even when the check is green).
• examples/gatehouse.yml: corrected the guidance that told adopters to mark both jobs required. Only the workflow-protection guard job is a blocking gate; the review job must not be required.
• Local CLI behavior is unchanged — gatehouse still exits 1 on blocking findings for local/pre-commit gating. The fix is scoped to CI, where the incident occurred.
• Docs: README "GitHub Actions" section, CHANGELOG, gatehouse-local constitution note.

Scope notes

• Issue option feat: detect secret exfiltration in GitHub Actions workflow files #4 (deterministic head-scoped diffing) needs no code change. The workflow already feeds gh pr diff (net merge-base vs head); BASE_REF/GATEHOUSE_CONTEXT_REF only scopes styleguide/constitution loading, not the reviewed diff.
• Options Add Documentation agent #2 (corroboration gate) and Add Constitution agent #3 (dismissible findings) are deliberately left out — advisory-by-default resolves the incident; those are larger follow-ups if blocking is ever re-enabled.
• Constitution §XII strengthening ships as a separate PR in crunchtools/constitution.

Test

ruff ✓ mypy ✓ pytest ✓ (126 passed, incl. a new test asserting advisory mode posts COMMENT, not REQUEST_CHANGES).

🤖 Generated with Claude Code

[github-actions]

Gatehouse found 1 issues (1 high)

[github-actions]

HIGH (Documentation): The run_review function's behavior regarding GitHub PR review posting and exit codes has changed with the introduction of the advisory parameter's interaction with has_blocking. Specifically, in advisory mode, the function now always posts a COMMENT review and exits with code 0, even if blocking findings are present. The docstring for run_review does not appear to have been updated to reflect this significant change in its public interface behavior.

Suggestion: Update the docstring for the run_review function to clearly explain how the advisory parameter affects both the function's exit code and the type of GitHub PR review (COMMENT vs REQUEST_CHANGES) that is posted.

Evidence:

    has_blocking = _has_blocking_findings(all_results)
    request_changes = has_blocking and not advisory
    exit_code = 1 if request_changes else 0
    print_summary(all_results, exit_code)

    if comment:
        # In advisory mode, post a plain COMMENT review — never REQUEST_CHANGES,
        # which would still register as an unresolved review on the PR.
        await post_pr_review(all_results, request_changes)

[gemini-code-assist]

Code Review

This pull request updates the reusable GitHub Actions review workflow to be advisory by default, meaning it posts plain comments and exits with code 0 unless the new blocking: true input is explicitly configured. It also updates the documentation, CLI constitution, and tests to align with this change. The review feedback correctly identifies two issues: an indentation error in the commented-out YAML example in examples/gatehouse.yml that would cause syntax errors if uncommented, and an outdated version reference (@v0.2.0) in the README.md example that does not support the newly introduced blocking parameter.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The commented-out indentation for with: and blocking: true is incorrect. If a user uncomments these lines by removing the comment prefix # , they will get:

      with:
        blocking: true

Since uses: is indented at 4 spaces, with: must also be indented at 4 spaces to be a valid sibling key. The current commented-out lines have 6 spaces of indentation for with:, which will cause a YAML syntax/validation error in GitHub Actions.

Please adjust the commented-out indentation so that removing # results in correct alignment.

    # with:
    #   blocking: true

[gemini-code-assist]

The example snippet references @v0.2.0 of the reusable workflow. However, the blocking input is a new feature being introduced in this PR (which is post-v0.3.0). If a user copies this snippet and runs it with @v0.2.0, GitHub Actions will fail with a validation error because the blocking input does not exist in that version.

Please update the reference to @main or the upcoming version tag (e.g., @v0.4.0) so that the documented feature is actually available.

Suggested change

	uses: crunchtools/gatehouse/.github/workflows/review.yml@v0.2.0
	uses: crunchtools/gatehouse/.github/workflows/review.yml@main

[github-actions]

Gatehouse found 2 issues (2 low)

[github-actions]

LOW (Consistency Check): An explanatory comment for the test assertion was removed, reducing the clarity and maintainability of the test. The comment clarified the condition under which a change review is requested.

Suggestion: Restore the comment # Blocking findings with advisory=False → request a change review. above the assertion to clarify its intent.

Evidence:

    639    |     mock_post.assert_awaited_once()
    640    |     assert mock_post.call_args.args[1] is True

[github-actions]

LOW (Consistency Check): An explanatory comment for the test assertion was removed, reducing the clarity and maintainability of the test. The comment clarified that advisory mode suppresses change requests.

Suggestion: Restore the comment # Advisory suppresses REQUEST_CHANGES → request_changes must be False. above the assertion to clarify its intent.

Evidence:

    672    |     mock_post.assert_awaited_once()
    673    |     assert mock_post.call_args.args[1] is False

[github-actions]

Gatehouse found 1 issues (1 high)

[github-actions]

HIGH (Documentation): The run_review function's behavior regarding its exit code and the type of GitHub PR review posted has changed significantly with the introduction of the 'advisory by default' logic. The advisory parameter now directly controls whether blocking findings result in an exit code of 1 and a 'REQUEST_CHANGES' review, or an exit code of 0 and a 'COMMENT' review. The function's docstring (not shown in the diff, but assumed to exist for a public function) should be updated to clearly explain this new behavior.

Suggestion: Update the docstring for the run_review function to explicitly describe how the advisory parameter influences the function's return value (exit code) and the type of GitHub PR review (COMMENT vs. REQUEST_CHANGES) that is posted. For example, clarify that if advisory is True, the function will always return 0 and post a COMMENT review, regardless of blocking findings.

Evidence:

  266    |     has_blocking = _has_blocking_findings(all_results)
  267 [+]|     request_changes = has_blocking and not advisory
  268 [+]|     exit_code = 1 if request_changes else 0
  269    |     print_summary(all_results, exit_code)
  270    | 
  271    |     if comment:
  272 [+]|         await post_pr_review(all_results, request_changes)
  273    | 
  274    |     return exit_code