fix(ci): address BM Bossbot PR feedback

Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

.github/workflows/bm-bossbot.yml

@@ -32,6 +32,9 @@ jobs:
     name: BM Bossbot Review
     if: github.event.pull_request.draft != true
     runs-on: ubuntu-latest
+    outputs:
+      pr_number: ${{ steps.pr.outputs.pr_number }}
+      head_ref: ${{ steps.pr.outputs.head_ref }}
 
     steps:
       - name: Checkout trusted base ref
@@ -83,13 +86,21 @@ jobs:
           metadata="${RUNNER_TEMP}/bm-bossbot-pr.json"
           diff_file="${RUNNER_TEMP}/bm-bossbot-pr.diff"
           prompt_file="${RUNNER_TEMP}/bm-bossbot-prompt.md"
+          review_file="${RUNNER_TEMP}/bm-bossbot-review.json"
+          max_diff_bytes=120000
 
           gh pr view "${PR_NUMBER}" \
             --repo "${GITHUB_REPOSITORY}" \
             --json number,title,body,author,headRefName,headRefOid,baseRefName,labels,files,commits,reviewDecision,mergeStateStatus,isDraft \
             > "${metadata}"
           gh pr diff "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --patch > "${diff_file}"
 
+          diff_bytes="$(wc -c < "${diff_file}" | tr -d '[:space:]')"
+          diff_truncated=false
+          if [ "${diff_bytes}" -gt "${max_diff_bytes}" ]; then
+            diff_truncated=true
+          fi
+
           cat .github/basic-memory/bm-bossbot-review.md > "${prompt_file}"
           {
             echo ""
@@ -103,16 +114,42 @@ jobs:
             echo "### Diff"
             echo ""
             echo '```diff'
-            head -c 120000 "${diff_file}"
+            if [ "${diff_truncated}" = "true" ]; then
+              echo "[Diff omitted: ${diff_bytes} bytes exceeds BM Bossbot's ${max_diff_bytes} byte review limit.]"
+            else
+              cat "${diff_file}"
+            fi
             echo ""
             echo '```'
           } >> "${prompt_file}"
 
+          if [ "${diff_truncated}" = "true" ]; then
+            jq -n \
+              --arg sha "${HEAD_SHA}" \
+              --argjson bytes "${diff_bytes}" \
+              --argjson max_bytes "${max_diff_bytes}" \
+              '{
+                reviewed_head_sha: $sha,
+                review_complete: false,
+                verdict: "needs_human",
+                blocking_findings: [
+                  {
+                    title: "Diff exceeds BM Bossbot review limit",
+                    body: "The PR diff is \($bytes) bytes, exceeding the deterministic \($max_bytes) byte review limit. A human review is required or the PR must be split before BM Bossbot can approve."
+                  }
+                ],
+                nonblocking_findings: [],
+                summary: "BM Bossbot did not approve because the PR diff exceeded the deterministic review limit."
+              }' > "${review_file}"
+          fi
+
           echo "prompt_file=${prompt_file}" >> "${GITHUB_OUTPUT}"
-          echo "review_file=${RUNNER_TEMP}/bm-bossbot-review.json" >> "${GITHUB_OUTPUT}"
+          echo "review_file=${review_file}" >> "${GITHUB_OUTPUT}"
+          echo "diff_truncated=${diff_truncated}" >> "${GITHUB_OUTPUT}"
 
       - name: Run BM Bossbot review with Codex
         id: codex
+        if: steps.context.outputs.diff_truncated != 'true'
         uses: openai/codex-action@v1
         with:
           openai-api-key: ${{ secrets.OPENAI_API_KEY }}
@@ -133,13 +170,31 @@ jobs:
             --repo "${GITHUB_REPOSITORY}" \
             --run-url "${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"
 
+  assets:
+    name: BM Bossbot Assets
+    needs: review
+    if: needs.review.result == 'success'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: write
+      pull-requests: write
+
+    steps:
+      - name: Checkout trusted base ref
+        uses: actions/checkout@v6
+        with:
+          ref: ${{ github.event.pull_request.base.ref || github.ref }}
+          fetch-depth: 1
+
+      - name: Set up uv
+        uses: astral-sh/setup-uv@v3
+
       - name: Generate non-gating PR infographic
-        if: success()
         continue-on-error: true
         env:
           OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
           GH_TOKEN: ${{ github.token }}
-          PR_NUMBER: ${{ steps.pr.outputs.pr_number }}
+          PR_NUMBER: ${{ needs.review.outputs.pr_number }}
         run: |
           set -euo pipefail
           gh pr view "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --json body --jq '.body // ""' > "${RUNNER_TEMP}/bm-bossbot-pr-body.md"
@@ -150,12 +205,11 @@ jobs:
             --output "docs/assets/infographics/pr-${PR_NUMBER}.webp"
 
       - name: Publish non-gating PR infographic
-        if: success()
         continue-on-error: true
         env:
           GH_TOKEN: ${{ github.token }}
-          PR_NUMBER: ${{ steps.pr.outputs.pr_number }}
-          HEAD_REF: ${{ steps.pr.outputs.head_ref }}
+          PR_NUMBER: ${{ needs.review.outputs.pr_number }}
+          HEAD_REF: ${{ needs.review.outputs.head_ref }}
         run: |
           set -euo pipefail
           asset_path="docs/assets/infographics/pr-${PR_NUMBER}.webp"

scripts/generate_infographic.py

@@ -45,7 +45,8 @@ def validate_output_path(path: Path, *, repo_root: Path | None = None) -> Path:
     output = path.resolve()
     allowed_root = (root / "docs" / "assets" / "infographics").resolve()
     if not output.is_relative_to(allowed_root):
-        raise ValueError(f"Output path must be under {allowed_root.relative_to(root)}")
+        allowed_path = allowed_root.relative_to(root).as_posix()
+        raise ValueError(f"Output path must be under {allowed_path}")
     if output.suffix != ".webp":
         raise ValueError("Output path must end with .webp")
     return output

tests/ci/test_bm_bossbot_workflow.py

@@ -29,6 +29,10 @@ def test_bm_bossbot_uses_safe_pull_request_target_gate() -> None:
     assert permissions["pull-requests"] == "write"
     assert permissions["statuses"] == "write"
 
+    asset_permissions = workflow["jobs"]["assets"]["permissions"]
+    assert asset_permissions["contents"] == "write"
+    assert asset_permissions["pull-requests"] == "write"
+
 
 def test_bm_bossbot_workflow_never_checks_out_untrusted_head() -> None:
     workflow = _workflow()
@@ -65,6 +69,24 @@ def test_bm_bossbot_workflow_has_deterministic_status_steps() -> None:
     assert "uv run --script scripts/bm_bossbot_status.py finalize" in WORKFLOW_PATH.read_text(
         encoding="utf-8"
     )
+
+
+def test_bm_bossbot_assets_are_non_gating_and_separate_from_review_job() -> None:
+    workflow = _workflow()
+    review_steps = workflow["jobs"]["review"]["steps"]
+    asset_job = workflow["jobs"]["assets"]
+    asset_steps = asset_job["steps"]
+
+    assert asset_job["needs"] == "review"
+    assert asset_job["if"] == "needs.review.result == 'success'"
+    assert not any(step["name"] == "Generate non-gating PR infographic" for step in review_steps)
+    assert not any(step["name"] == "Publish non-gating PR infographic" for step in review_steps)
+
+    generate = next(step for step in asset_steps if step["name"] == "Generate non-gating PR infographic")
+    publish = next(step for step in asset_steps if step["name"] == "Publish non-gating PR infographic")
+
+    assert generate["continue-on-error"] is True
+    assert publish["continue-on-error"] is True
     assert "uv run --script scripts/generate_pr_infographic.py" in WORKFLOW_PATH.read_text(
         encoding="utf-8"
     )
@@ -73,6 +95,21 @@ def test_bm_bossbot_workflow_has_deterministic_status_steps() -> None:
     assert "BM_INFOGRAPHIC_PROVENANCE:end" in WORKFLOW_PATH.read_text(encoding="utf-8")
 
 
+def test_bm_bossbot_rejects_oversized_diffs_without_partial_approval() -> None:
+    workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8")
+    workflow = _workflow()
+    steps = workflow["jobs"]["review"]["steps"]
+    run_codex = next(step for step in steps if step["name"] == "Run BM Bossbot review with Codex")
+
+    assert "max_diff_bytes=120000" in workflow_text
+    assert "diff_truncated=true" in workflow_text
+    assert "review_complete: false" in workflow_text
+    assert 'verdict: "needs_human"' in workflow_text
+    assert "Diff exceeds BM Bossbot review limit" in workflow_text
+    assert run_codex["if"] == "steps.context.outputs.diff_truncated != 'true'"
+    assert "head -c 120000" not in workflow_text
+
+
 def test_bm_bossbot_prompt_references_engineering_style_and_json_bullets() -> None:
     prompt = PROMPT_PATH.read_text(encoding="utf-8")
 

tests/scripts/test_generate_pr_infographic.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 
 import pytest
+from click import unstyle
 from typer.testing import CliRunner
 
 from scripts import generate_infographic, generate_pr_infographic
@@ -20,16 +21,17 @@ def test_infographic_scripts_are_uv_typer_entrypoints() -> None:
 
 def test_generate_pr_infographic_cli_help_exposes_useful_options() -> None:
     result = CliRunner().invoke(generate_pr_infographic.app, ["--help"])
+    help_text = unstyle(result.output)
 
     assert result.exit_code == 0
-    assert "--pr-number" in result.output
-    assert "--pr-body-file" in result.output
-    assert "--output" in result.output
-    assert "--theme" in result.output
-    assert "--visual-format" in result.output
-    assert "--provenance-output" in result.output
-    assert "--print-prompt" in result.output
-    assert "--dry-run" in result.output
+    assert "--pr-number" in help_text
+    assert "--pr-body-file" in help_text
+    assert "--output" in help_text
+    assert "--theme" in help_text
+    assert "--visual-format" in help_text
+    assert "--provenance-output" in help_text
+    assert "--print-prompt" in help_text
+    assert "--dry-run" in help_text
 
 
 def test_extract_bossbot_summary_from_pr_body() -> None: