From 280255ac89b36ab5f9b71f33f2623714756dfeb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 19:42:50 +0000 Subject: [PATCH 1/5] feat: surface the engine's run diagnostics instead of publishing past them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run that exits zero is not a run that finished cleanly. Core records every degradation it survived — a language server that never started, a language nothing indexed under, naming that stopped answering — in the analysis it writes. Until now this action ignored that field, so a baseline missing a whole language was committed green and a review comment showed a diagram with nothing to say it was short. Read metadata.run_diagnostics back and put it where the reader already is: an annotation per entry on the run page, the list in the sync job summary, and the same list at the top of the review comment, above the diagram rather than under it — a caveat printed below a picture is read after the picture is believed. Entries carry their own remedy. Where nothing on the reader's side would have changed the outcome the remedy is empty, and the block links Discord instead of inventing an instruction nobody can follow. Diagnostics never fail the run: a degraded analysis is still worth having, and the point is that its reader learns it is degraded. An analysis written by an engine that predates the field, or one that never got written at all, reads as silence rather than an error. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x --- README.md | 18 ++++ action.yml | 25 +++++ scripts/action/build-review-comment.sh | 6 ++ scripts/action/sync-summary.sh | 8 ++ scripts/analysis_diagnostics.py | 121 ++++++++++++++++++++++ tests/test_analysis_diagnostics.py | 137 +++++++++++++++++++++++++ 6 files changed, 315 insertions(+) create mode 100755 scripts/analysis_diagnostics.py create mode 100644 tests/test_analysis_diagnostics.py diff --git a/README.md b/README.md index 1d01f82..a6fafbd 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,24 @@ Every run reports what it resolved, so the answer never has to be inferred from - on a configuration failure, an error annotation and — in review mode — a pull request comment with the fix, so the person who has to add the secret sees it where they are. +### Analysis diagnostics + +A run can finish, exit zero, and still have produced a diagram that is missing something: +a language server that never started, a language nothing indexed under, naming that +stopped answering mid-run. Core records each of those in the analysis it writes +(`metadata.run_diagnostics`), and this action reads them back rather than publishing the +result as though nothing happened: + +- a `::warning::` annotation per degradation, on the run page; +- the same list in the job summary (sync) and at the top of the review comment, above the + diagram — a caveat printed under a picture is read after the picture is believed; +- each entry carries what to do about it. Where nothing on your side would have changed + the outcome, it says so and links Discord instead of inventing an instruction. + +Diagnostics never fail the run: a degraded analysis is still worth having, and the whole +point is that you learn it is degraded. The webview reads the same field out of the +analysis it loads, so a diagram opened there carries the same warning. + ## Model selection All model inputs are optional and are passed directly to Core without action-side validation: diff --git a/action.yml b/action.yml index 2d815f7..c4ea45e 100644 --- a/action.yml +++ b/action.yml @@ -496,6 +496,19 @@ runs: retention-days: 30 if-no-files-found: ignore + # An analysis that finished is not an analysis that finished cleanly. The engine + # records every degradation it survived; without this the run is green and the + # committed diagram is short of a language with nothing to say so. + - name: Read analysis diagnostics + id: sync_diagnostics + if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' && steps.sync_analyze.outputs.analysis_path != '' + continue-on-error: true + shell: bash + env: + ANALYSIS_PATH: ${{ steps.sync_analyze.outputs.analysis_path }} + DIAGNOSTICS_OUT: ${{ runner.temp }}/codeboarding-diagnostics.md + run: 'python3 "$GITHUB_ACTION_PATH/scripts/analysis_diagnostics.py" --analysis "$ANALYSIS_PATH" --out "$DIAGNOSTICS_OUT"' + - name: Write sync summary if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' shell: bash @@ -505,6 +518,7 @@ runs: FILES: ${{ steps.sync_commit.outputs.files_written }} STRATEGY: ${{ inputs.sync_strategy }} PR_URL: ${{ steps.sync_commit.outputs.sync_pr_url }} + DIAGNOSTICS_MD: ${{ steps.sync_diagnostics.outputs.markdown_path }} run: "$GITHUB_ACTION_PATH/scripts/action/sync-summary.sh" - name: Analyze pull request @@ -566,6 +580,16 @@ runs: retention-days: 30 if-no-files-found: ignore + - name: Read analysis diagnostics + id: review_diagnostics + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_analyze.outputs.analysis_path != '' + continue-on-error: true + shell: bash + env: + ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }} + DIAGNOSTICS_OUT: ${{ runner.temp }}/codeboarding-diagnostics.md + run: 'python3 "$GITHUB_ACTION_PATH/scripts/analysis_diagnostics.py" --analysis "$ANALYSIS_PATH" --out "$DIAGNOSTICS_OUT"' + - name: Render review diagram id: review_render if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' @@ -622,6 +646,7 @@ runs: BEHIND_BY: ${{ steps.guard.outputs.behind_by }} BASE_REF: ${{ steps.guard.outputs.base_ref }} MERGE_BASE_RESOLVED: ${{ steps.guard.outputs.merge_base_resolved }} + DIAGNOSTICS_MD: ${{ steps.review_diagnostics.outputs.markdown_path }} run: "$GITHUB_ACTION_PATH/scripts/action/build-review-comment.sh" - name: Post review comment diff --git a/scripts/action/build-review-comment.sh b/scripts/action/build-review-comment.sh index d28d0e2..6a8e876 100755 --- a/scripts/action/build-review-comment.sh +++ b/scripts/action/build-review-comment.sh @@ -12,6 +12,12 @@ WEBVIEW_URL="https://app.codeboarding.org/${GITHUB_REPOSITORY}/pull/${PR_NUMBER} BODY="${RUNNER_TEMP}/review-comment.md" printf '### CodeBoarding review\n\n**Status:** %s changed %s\n' "$N_CHANGED" "$COMPONENT_NOUN" > "$BODY" printf '\nSee the full change in [CodeBoarding](%s).\n' "$WEBVIEW_URL" >> "$BODY" +# Above the diagram, not below it: the whole point is that the picture that follows +# is missing something, and a caveat under it is read after the picture is believed. +if [ -n "${DIAGNOSTICS_MD:-}" ] && [ -s "${DIAGNOSTICS_MD}" ]; then + printf '\n' >> "$BODY" + cat "${DIAGNOSTICS_MD}" >> "$BODY" +fi # The diagram compares against the merge base, so commits landed on the base # branch since this PR forked are excluded. Say so rather than hide it. BEHIND="${BEHIND_BY:-0}" diff --git a/scripts/action/sync-summary.sh b/scripts/action/sync-summary.sh index 287b9af..8c2ca8e 100755 --- a/scripts/action/sync-summary.sh +++ b/scripts/action/sync-summary.sh @@ -10,4 +10,12 @@ set -euo pipefail if [ -n "${PR_URL:-}" ]; then echo "- Sync PR: ${PR_URL}" fi + # A baseline that is short a language is committed and read for weeks. The + # bullets above cannot show that, so the engine's own account goes here. + if [ -n "${DIAGNOSTICS_MD:-}" ] && [ -s "${DIAGNOSTICS_MD}" ]; then + echo + echo "#### Analysis diagnostics" + echo + cat "${DIAGNOSTICS_MD}" + fi } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/analysis_diagnostics.py b/scripts/analysis_diagnostics.py new file mode 100755 index 0000000..f3ae9ab --- /dev/null +++ b/scripts/analysis_diagnostics.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +"""Turn an analysis.json's run diagnostics into what the run and the reader see. + +CodeBoarding writes ``metadata.run_diagnostics`` for every degradation it +survived — a language server that never started, a language nothing indexed +under, naming that stopped answering. A run that only checks the exit code +publishes those diagrams as though nothing happened, so this reads them back +and gives the run three things: annotations on the failing step, a Markdown +block for the job summary and the review comment, and a count to branch on. +""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path + +# Where a reader goes when the cause is ours rather than theirs. +DISCORD_URL = "https://discord.gg/T5zHTJYFuy" + +MAX_RENDERED_ENTRIES = 10 + + +def load_entries(analysis_path: Path) -> list[dict]: + """Diagnostic entries from an analysis document, newest schema or none at all. + + An analysis written before the field existed, or by a build that never sets + it, has nothing to say — that is not an error, so it reads as an empty list. + """ + try: + with analysis_path.open(encoding="utf-8") as handle: + document = json.load(handle) + except (OSError, json.JSONDecodeError) as exc: + print(f"::warning::Could not read run diagnostics from {analysis_path}: {exc}", file=sys.stderr) + return [] + + report = document.get("metadata", {}).get("run_diagnostics") + if not isinstance(report, dict): + return [] + entries = report.get("entries") + return [entry for entry in entries if isinstance(entry, dict)] if isinstance(entries, list) else [] + + +def annotations(entries: list[dict]) -> list[str]: + """One workflow annotation per entry, so the run page shows them without scrolling.""" + lines = [] + for entry in entries: + level = "warning" if entry.get("severity") == "degraded" else "notice" + remedy = entry.get("remedy") or f"Not something you can fix — please report it: {DISCORD_URL}" + lines.append(f"::{level}::{entry.get('title', 'Analysis diagnostic')} {entry.get('detail', '')} {remedy}") + return lines + + +def markdown(entries: list[dict]) -> str: + """The block that goes in the job summary and the review comment. + + Degraded entries lead with a GitHub alert, because the point is that the + diagram beneath is missing something and would otherwise be read as complete. + """ + degraded = [entry for entry in entries if entry.get("severity") == "degraded"] + if not entries: + return "" + + lines: list[str] = [] + if degraded: + lines.append("> [!WARNING]") + lines.append( + "> This analysis did not complete cleanly, so the diagram is missing structure " + "a clean run would have had." + ) + lines.append("") + + for entry in entries[:MAX_RENDERED_ENTRIES]: + title = entry.get("title", "Analysis diagnostic") + count = entry.get("count", 1) + repeated = f" (×{count})" if isinstance(count, int) and count > 1 else "" + lines.append(f"- **{title}**{repeated} — {entry.get('detail', '')}") + remedy = entry.get("remedy") + if remedy: + lines.append(f" - {remedy}") + else: + lines.append(f" - Nothing on your side causes this. Please report it on [Discord]({DISCORD_URL}).") + + hidden = len(entries) - MAX_RENDERED_ENTRIES + if hidden > 0: + lines.append(f"- …and {hidden} more, in the run log.") + + return "\n".join(lines) + "\n" + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--analysis", required=True, help="Path to the analysis.json the run produced") + parser.add_argument("--out", required=True, help="File to write the Markdown block to") + args = parser.parse_args(argv) + + analysis_path = Path(args.analysis) + entries = load_entries(analysis_path) if analysis_path.is_file() else [] + degraded = sum(1 for entry in entries if entry.get("severity") == "degraded") + + for line in annotations(entries): + print(line) + + body = markdown(entries) + out_path = Path(args.out) + out_path.parent.mkdir(parents=True, exist_ok=True) + out_path.write_text(body, encoding="utf-8") + + github_output = os.environ.get("GITHUB_OUTPUT") + if github_output: + with open(github_output, "a", encoding="utf-8") as handle: + handle.write(f"degraded={degraded}\n") + handle.write(f"entries={len(entries)}\n") + handle.write(f"markdown_path={out_path if body else ''}\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_analysis_diagnostics.py b/tests/test_analysis_diagnostics.py new file mode 100644 index 0000000..cdec27b --- /dev/null +++ b/tests/test_analysis_diagnostics.py @@ -0,0 +1,137 @@ +"""What the run publishes when the engine finished with less than it should have.""" + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "scripts")) + +import analysis_diagnostics as ad + + +def _entry(**overrides: object) -> dict: + entry = { + "code": "static.language_server_unavailable", + "severity": "degraded", + "title": "CSharp could not be analyzed", + "detail": "Its language server failed to start.", + "remedy": "Install the CSharp toolchain, then run the analysis again.", + "subject": "CSharp", + "count": 1, + } + entry.update(overrides) + return entry + + +def _document(entries: list[dict]) -> dict: + degraded = sum(1 for e in entries if e["severity"] == "degraded") + return { + "metadata": { + "run_diagnostics": { + "version": 1, + "degraded": degraded, + "notices": len(entries) - degraded, + "entries": entries, + } + } + } + + +class LoadEntriesTests(unittest.TestCase): + def _write(self, content: str) -> Path: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + path = Path(tmp.name) / "analysis.json" + path.write_text(content, encoding="utf-8") + return path + + def test_an_analysis_without_the_field_has_nothing_to_say(self): + """Older engines wrote no diagnostics; that is silence, not a failure.""" + self.assertEqual(ad.load_entries(self._write(json.dumps({"metadata": {"depth_cap": 2}}))), []) + + def test_unparseable_json_does_not_take_the_run_down_with_it(self): + self.assertEqual(ad.load_entries(self._write("{not json")), []) + + def test_entries_are_read_back_intact(self): + entries = ad.load_entries(self._write(json.dumps(_document([_entry()])))) + self.assertEqual([e["code"] for e in entries], ["static.language_server_unavailable"]) + + +class AnnotationTests(unittest.TestCase): + def test_a_degraded_entry_annotates_as_a_warning(self): + self.assertTrue(ad.annotations([_entry()])[0].startswith("::warning::")) + + def test_a_notice_stays_a_notice(self): + self.assertTrue(ad.annotations([_entry(severity="notice")])[0].startswith("::notice::")) + + def test_an_entry_nobody_can_act_on_points_at_the_community(self): + self.assertIn(ad.DISCORD_URL, ad.annotations([_entry(remedy="")])[0]) + + +class MarkdownTests(unittest.TestCase): + def test_a_clean_run_renders_nothing(self): + self.assertEqual(ad.markdown([]), "") + + def test_a_degradation_leads_with_an_alert(self): + body = ad.markdown([_entry()]) + self.assertTrue(body.startswith("> [!WARNING]")) + self.assertIn("Install the CSharp toolchain", body) + + def test_notices_alone_do_not_raise_an_alert(self): + body = ad.markdown([_entry(severity="notice")]) + self.assertNotIn("[!WARNING]", body) + self.assertIn("CSharp could not be analyzed", body) + + def test_a_repeated_entry_shows_its_count(self): + self.assertIn("(×3)", ad.markdown([_entry(count=3)])) + + def test_a_long_list_is_capped_and_says_so(self): + entries = [_entry(subject=str(i), title=f"Entry {i}") for i in range(ad.MAX_RENDERED_ENTRIES + 4)] + body = ad.markdown(entries) + self.assertIn("…and 4 more, in the run log.", body) + self.assertNotIn("Entry 12", body) + + +class MainTests(unittest.TestCase): + def _run(self, document: object | None) -> tuple[Path, dict[str, str]]: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + root = Path(tmp.name) + analysis = root / "analysis.json" + if document is not None: + analysis.write_text(json.dumps(document), encoding="utf-8") + out = root / "diagnostics.md" + github_output = root / "github-output" + github_output.write_text("", encoding="utf-8") + os.environ["GITHUB_OUTPUT"] = str(github_output) + self.addCleanup(os.environ.pop, "GITHUB_OUTPUT", None) + + ad.main(["--analysis", str(analysis), "--out", str(out)]) + + outputs = dict( + line.split("=", 1) for line in github_output.read_text(encoding="utf-8").splitlines() if "=" in line + ) + return out, outputs + + def test_a_degraded_run_reports_its_count_and_a_body(self): + out, outputs = self._run(_document([_entry(), _entry(subject="Go", title="Go could not be analyzed")])) + self.assertEqual(outputs["degraded"], "2") + self.assertEqual(outputs["markdown_path"], str(out)) + self.assertTrue(out.read_text(encoding="utf-8")) + + def test_a_clean_run_publishes_an_empty_path_so_callers_skip_the_block(self): + _, outputs = self._run(_document([])) + self.assertEqual(outputs["degraded"], "0") + self.assertEqual(outputs["markdown_path"], "") + + def test_a_missing_analysis_is_not_an_error(self): + """The step runs on always(); an analysis that never got written has no diagnostics.""" + _, outputs = self._run(None) + self.assertEqual(outputs["degraded"], "0") + + +if __name__ == "__main__": + unittest.main() From 6c66dd86645aa83f3a84c9beb85779b91ee05301 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 19:43:44 +0000 Subject: [PATCH 2/5] style: keep the diagnostics block free of em dashes The same title/detail/remedy strings render in the webview, which enforces the house rule with a test, so the prose the action wraps them in should read the same way rather than switching voice between surfaces. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x --- scripts/analysis_diagnostics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/analysis_diagnostics.py b/scripts/analysis_diagnostics.py index f3ae9ab..d6ece19 100755 --- a/scripts/analysis_diagnostics.py +++ b/scripts/analysis_diagnostics.py @@ -48,7 +48,7 @@ def annotations(entries: list[dict]) -> list[str]: lines = [] for entry in entries: level = "warning" if entry.get("severity") == "degraded" else "notice" - remedy = entry.get("remedy") or f"Not something you can fix — please report it: {DISCORD_URL}" + remedy = entry.get("remedy") or f"Nothing on your side causes this; please report it: {DISCORD_URL}" lines.append(f"::{level}::{entry.get('title', 'Analysis diagnostic')} {entry.get('detail', '')} {remedy}") return lines @@ -76,7 +76,7 @@ def markdown(entries: list[dict]) -> str: title = entry.get("title", "Analysis diagnostic") count = entry.get("count", 1) repeated = f" (×{count})" if isinstance(count, int) and count > 1 else "" - lines.append(f"- **{title}**{repeated} — {entry.get('detail', '')}") + lines.append(f"- **{title}**{repeated}: {entry.get('detail', '')}") remedy = entry.get("remedy") if remedy: lines.append(f" - {remedy}") From d2357b06960f908ad1c25c471b0b61147f0a5029 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Fri, 25 Sep 2026 13:18:42 +0200 Subject: [PATCH 3/5] feat: fail the run with a specific comment when the LLM quota runs out The engine now stops on a mid-run quota refusal (exit 3, kind llm_quota_exhausted) or rejected credentials (exit 2, kind llm_auth) instead of writing a folder-named map. analyze_repository.py keeps that verdict in $RUNNER_TEMP/codeboarding-engine-error.json, exits with the engine's code and annotates the specific reason. A new pair of steps turns the record into the job summary and, in review mode, the review's sticky comment, and the generic "Post review failure" stands down when it exists. Delivery and the base publishes already skip on a failed analysis; tests now pin that. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 26 +++++++- action.yml | 31 ++++++++- scripts/action/analyze.sh | 3 + scripts/action/engine_failure.py | 111 +++++++++++++++++++++++++++++++ scripts/analyze_repository.py | 75 ++++++++++++++++++--- tests/test_action_inputs.py | 41 ++++++++++++ tests/test_action_state.py | 58 ++++++++++++++-- tests/test_analyze_repository.py | 74 ++++++++++++++++++++- tests/test_engine_failure.py | 109 ++++++++++++++++++++++++++++++ 9 files changed, 504 insertions(+), 24 deletions(-) create mode 100755 scripts/action/engine_failure.py create mode 100644 tests/test_engine_failure.py diff --git a/README.md b/README.md index b28175f..f4e9e54 100644 --- a/README.md +++ b/README.md @@ -207,13 +207,35 @@ result as though nothing happened: - a `::warning::` annotation per degradation, on the run page; - the same list in the job summary (sync) and at the top of the review comment, above the - diagram — a caveat printed under a picture is read after the picture is believed; + diagram, since a caveat printed under a picture is read after the picture is believed; - each entry carries what to do about it. Where nothing on your side would have changed the outcome, it says so and links Discord instead of inventing an instruction. Diagnostics never fail the run: a degraded analysis is still worth having, and the whole point is that you learn it is degraded. The webview reads the same field out of the -analysis it loads, so a diagram opened there carries the same warning. +analysis it loads, so a diagram opened there carries the same warning. The one +degradation that does fail the run is a used-up LLM quota, below. + +### When the LLM quota runs out + +If the LLM provider refuses the analysis because the token quota is used up (HTTP 402), +CodeBoarding stops rather than publish a map without AI naming. The run fails, and: + +- the annotation on the run page says why (`CodeBoarding LLM quota exhausted`); +- in review mode, the review's sticky comment is replaced with the reason and what to change, + and ends with the machine-readable line carrying `failure=llm_quota_exhausted`; +- the job summary carries the same text; +- nothing is published: sync commits no baseline and uploads no base analysis, and review + posts no diagram. + +On `llm: hosted` the free tier's allowance is per GitHub owner per week and resets Monday +00:00 UTC. To analyze before then, use your own key (`llm: anthropic` with +`anthropic_api_key`, or any provider above), or a CodeBoarding license (`llm: license` with +`license_key`). Credentials the provider rejects outright stop the run the same way, as +`failure=llm_auth`. + +This needs a CodeBoarding release that stops on quota; with an older engine the run +finishes on folder-named components instead. ## Model selection diff --git a/action.yml b/action.yml index a80b3ed..fc2d67b 100644 --- a/action.yml +++ b/action.yml @@ -678,12 +678,37 @@ runs: BODY: ${{ steps.review_body.outputs.path }} run: cat "$BODY" >> "$GITHUB_STEP_SUMMARY" - # Skipped when the run stopped on a credential problem: that path already replaced - # this same sticky comment with the input and secret to fix, and "see the workflow + # The engine stops rather than publish a map without AI naming when the LLM quota runs + # out or its credentials are refused. The run stays failed; this says why, in the job + # summary and (below) in the pull request, instead of "see the workflow logs". + - name: Read engine failure + id: engine_failure + if: failure() && steps.guard.outputs.skip != 'true' && (steps.sync_analyze.outcome == 'failure' || steps.review_analyze.outcome == 'failure') + continue-on-error: true + shell: bash + env: + MODE: ${{ steps.guard.outputs.mode }} + LLM: ${{ inputs.llm }} + PR_NUMBER: ${{ steps.guard.outputs.pr_number }} + HEAD_SHA: ${{ steps.guard.outputs.head_sha }} + run: 'python3 "$GITHUB_ACTION_PATH/scripts/action/engine_failure.py"' + + - name: Report engine failure + if: failure() && steps.engine_failure.outputs.reason != '' && steps.guard.outputs.mode == 'review' && steps.guard.outputs.pr_number != '' + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: ${{ steps.guard.outputs.comment_id }} + number: ${{ steps.guard.outputs.pr_number }} + GITHUB_TOKEN: ${{ inputs.github_token }} + path: ${{ steps.engine_failure.outputs.body_path }} + + # Skipped when the run stopped on a credential problem or an engine refusal: those + # paths replaced this same sticky comment with what to fix, and "see the workflow # logs" posted over the top of it would send the reader hunting for what they had # just been told. - name: Post review failure - if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome != 'success' && steps.llm.outputs.error == '' + if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome != 'success' && steps.llm.outputs.error == '' && steps.engine_failure.outputs.reason == '' continue-on-error: true uses: marocchino/sticky-pull-request-comment@v2 with: diff --git a/scripts/action/analyze.sh b/scripts/action/analyze.sh index 5f246a0..55738af 100755 --- a/scripts/action/analyze.sh +++ b/scripts/action/analyze.sh @@ -6,6 +6,9 @@ if [[ ! "$DEPTH_CAP" =~ ^[1-9][0-9]*$ ]]; then echo "::error::depth_cap must be a positive integer." exit 1 fi +# analyze_repository.py records an engine refusal here for the steps that report it; one +# left by an earlier use of the action in this job is not this run's. +rm -f "$RUNNER_TEMP/codeboarding-engine-error.json" parse_output() { local output="$1" ANALYSIS_MODE="$(awk -F= '$1 == "analysis_mode" {print $2; exit}' <<< "$output")" diff --git a/scripts/action/engine_failure.py b/scripts/action/engine_failure.py new file mode 100755 index 0000000..abc4a06 --- /dev/null +++ b/scripts/action/engine_failure.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +"""Explain a run the engine refused to finish, in the pull request and the job summary. + +analyze_repository.py records the engine's refusal (quota used up, credentials rejected) in +$RUNNER_TEMP/codeboarding-engine-error.json. This renders it for a person: a sticky comment +body in review mode, and the job summary in both modes. The run itself stays failed; this +only replaces "see the workflow logs" with the reason and what to change. + +Writes `reason` (the engine's `kind`, empty when there is nothing to report) and `body_path` +to $GITHUB_OUTPUT. +""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +ERROR_FILE = "codeboarding-engine-error.json" +DOCS = "https://github.com/CodeBoarding/CodeBoarding-action#authentication-and-providers" +HEADINGS = { + "llm_quota_exhausted": "stopped: LLM quota used up", + "llm_auth": "stopped: LLM credentials rejected", +} +OWN_KEY = "use your own LLM key: set `llm` to your provider and pass its key input, for example `llm: anthropic` with `anthropic_api_key`" +LICENSE = "use a CodeBoarding license: `llm: license` with `license_key`" + + +def _quota(llm: str) -> list[str]: + lines = ["The LLM provider refused the analysis because the token quota is used up."] + if llm == "hosted": + lines += [ + "", + "On `llm: hosted` that is CodeBoarding's free tier, whose allowance is per GitHub owner per week and resets Monday 00:00 UTC. To analyze before then, either:", + "", + f"- {OWN_KEY};", + f"- or {LICENSE}.", + ] + elif llm == "license": + lines += [ + "", + f"On `llm: license` that is your CodeBoarding plan's allowance. To analyze before it renews, {OWN_KEY}.", + ] + else: + lines += [ + "", + f"On `llm: {llm}` that is the quota of your own provider account. Raise it with the provider, or {LICENSE}.", + ] + return lines + + +def _auth(llm: str) -> list[str]: + return [ + "The LLM provider rejected this run's credentials.", + "", + f"Check the key or license this workflow passes for `llm: {llm}`; see [Authentication and providers]({DOCS}).", + ] + + +def render(error: dict, env: dict[str, str]) -> str: + kind = error["kind"] + mode = env.get("MODE", "review") + llm = env.get("LLM", "") or "hosted" + lines = [f"### CodeBoarding {mode} · {HEADINGS[kind]}", ""] + lines.append("CodeBoarding stopped instead of publishing a map without AI naming.") + lines += _quota(llm) if kind == "llm_quota_exhausted" else _auth(llm) + lines.append("") + if mode == "sync": + lines.append("The baseline was not updated, and no base analysis was published.") + else: + lines.append("No diagram was posted for this run.") + server = env.get("GITHUB_SERVER_URL", "https://github.com") + repository = env.get("GITHUB_REPOSITORY", "") + run_id = env.get("GITHUB_RUN_ID", "") + lines += [ + "", + f"run [{run_id}]({server}/{repository}/actions/runs/{run_id}) · attempt {env.get('GITHUB_RUN_ATTEMPT', '1')}", + ] + if mode == "review": + # The review comment's machine-readable line, with `failure` in place of the counts a + # stopped run does not have. One line, `key=value`, no spaces in values. + platform = f"https://app.codeboarding.org/{repository}/pull/{env.get('PR_NUMBER', '')}" + lines.append(f"") + return "\n".join(lines) + "\n" + + +def main() -> int: + env = dict(os.environ) + runner_temp = Path(env["RUNNER_TEMP"]) + outputs = [] + try: + error = json.loads((runner_temp / ERROR_FILE).read_text(encoding="utf-8")) + except (OSError, ValueError): + error = None + if isinstance(error, dict) and error.get("kind") in HEADINGS: + body = render(error, env) + path = runner_temp / "codeboarding-engine-failure.md" + path.write_text(body, encoding="utf-8") + summary = env.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as handle: + handle.write(body) + outputs = [f"reason={error['kind']}", f"body_path={path}"] + with open(env["GITHUB_OUTPUT"], "a", encoding="utf-8") as handle: + handle.writelines(line + "\n" for line in outputs) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/analyze_repository.py b/scripts/analyze_repository.py index aba775a..f1ed2d4 100755 --- a/scripts/analyze_repository.py +++ b/scripts/analyze_repository.py @@ -5,6 +5,7 @@ import argparse import json +import os import shutil import subprocess import sys @@ -17,6 +18,61 @@ class AnalysisError(RuntimeError): pass +# The engine's own verdict on a run it refused to finish: exit 3 when the LLM quota ran out, +# exit 2 when the credentials were rejected. Both are aborts by design, because a map drawn +# without the LLM would be published as though it were a real one. +ENGINE_ABORT_TITLES = { + "llm_quota_exhausted": "CodeBoarding LLM quota exhausted", + "llm_auth": "CodeBoarding LLM credentials rejected", +} +ENGINE_ERROR_FILE = "codeboarding-engine-error.json" + + +class EngineAbort(AnalysisError): + def __init__(self, exit_code: int, payload: dict) -> None: + super().__init__(str(payload.get("error") or payload["kind"])) + self.exit_code = exit_code + self.payload = payload + + def annotation(self) -> str: + # Workflow commands end at the first newline, so the engine's message is kept to one line. + message = " ".join(str(self).split()) + return f"::error title={ENGINE_ABORT_TITLES[self.payload['kind']]}::{message}" + + +def _find_json(raw: str) -> object: + """The engine's JSON object, which may follow log lines on stdout.""" + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + payload = None + lines = raw.splitlines() + for index, line in enumerate(lines): + if line.lstrip().startswith("{"): + try: + payload = json.loads("\n".join(lines[index:])) + except json.JSONDecodeError: + continue + if payload is None: + raise exc + return payload + + +def _engine_abort(stdout: str, return_code: int) -> EngineAbort | None: + """Recognise a refusal the engine explained, and keep it for the steps that report it.""" + try: + payload = _find_json(stdout) + except json.JSONDecodeError: + return None + if not isinstance(payload, dict) or payload.get("kind") not in ENGINE_ABORT_TITLES: + return None + runner_temp = os.environ.get("RUNNER_TEMP") + if runner_temp: + record = {**payload, "exitCode": return_code} + (Path(runner_temp) / ENGINE_ERROR_FILE).write_text(json.dumps(record), encoding="utf-8") + return EngineAbort(return_code, payload) + + def _parse_bool(value: object, *, field: str) -> bool: if isinstance(value, bool): return value @@ -34,18 +90,9 @@ def _parse_cli_response(raw: str, working_dir: str) -> tuple[bool, Path | None, raise AnalysisError("CodeBoarding command produced no JSON output") try: - payload = json.loads(raw) + payload = _find_json(raw) except json.JSONDecodeError as exc: - payload = None - lines = raw.splitlines() - for index, line in enumerate(lines): - if line.lstrip().startswith("{"): - try: - payload = json.loads("\n".join(lines[index:])) - except json.JSONDecodeError: - continue - if payload is None: - raise AnalysisError(f"Invalid CodeBoarding JSON response: {exc}") from exc + raise AnalysisError(f"Invalid CodeBoarding JSON response: {exc}") from exc if not isinstance(payload, dict): raise AnalysisError("CodeBoarding JSON response is not an object") @@ -84,6 +131,9 @@ def _run_command(args: list[str], output_dir: Path) -> str: return_code = process.wait() stdout = "".join(stdout_lines) if return_code != 0: + abort = _engine_abort(stdout, return_code) + if abort is not None: + raise abort details = stdout.strip() or f"exit code {return_code}; see command logs above" raise AnalysisError(f"Command failed ({' '.join(args)}): {details}") @@ -140,6 +190,9 @@ def main(argv: list[str] | None = None) -> int: if __name__ == "__main__": try: raise SystemExit(main()) + except EngineAbort as exc: + print(exc.annotation(), file=sys.stderr) + raise SystemExit(exc.exit_code) except AnalysisError as exc: print(f"::error::{exc}", file=sys.stderr) raise SystemExit(1) diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index 4757bc7..d7b68cb 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -139,6 +139,47 @@ def test_a_refused_run_reports_and_then_fails(self) -> None: self.assertLess(report, stop, "the run fails before it explains why") self.assertIn("continue-on-error: true", ACTION[ACTION.index("id: llm") : report]) + def test_an_engine_refusal_is_explained_and_not_buried(self) -> None: + """The engine stops on a used-up quota rather than publish a map without AI naming. + The run stays red; the pull request is told why, in the review's own sticky comment, + and the generic "see the workflow logs" must not be posted over it.""" + read = ACTION.index("- name: Read engine failure") + report = ACTION.index("- name: Report engine failure") + generic = ACTION.index("- name: Post review failure") + for analyze in ("- name: Analyze baseline", "- name: Analyze pull request"): + self.assertLess(ACTION.index(analyze), read) + self.assertLess(read, report) + self.assertLess(report, generic, "the generic step reads the reason, so it must exist first") + + read_block = ACTION[read:report] + self.assertIn("failure()", read_block) + self.assertIn("steps.sync_analyze.outcome == 'failure'", read_block) + self.assertIn("steps.review_analyze.outcome == 'failure'", read_block) + self.assertIn("continue-on-error: true", read_block) + + report_block = ACTION[report:generic] + self.assertIn("steps.engine_failure.outputs.reason != ''", report_block) + self.assertIn("steps.guard.outputs.mode == 'review'", report_block) + self.assertIn("header: ${{ steps.guard.outputs.comment_id }}", report_block) + self.assertIn("path: ${{ steps.engine_failure.outputs.body_path }}", report_block) + + condition = ACTION[generic : ACTION.index("message:", generic)] + self.assertIn("steps.engine_failure.outputs.reason == ''", condition) + + def test_a_failed_analysis_delivers_and_publishes_nothing(self) -> None: + """Delivery and the base publishes run only on success, so a sync the engine refused + commits no baseline and leaves no base for a review to start from.""" + for name in ( + "- name: Deliver baseline", + "- name: Publish baseline analysis", + "- name: Publish baseline analysis for the analyzed commit", + "- name: Publish this analysis for the next run", + ): + start = ACTION.index(name + "\n") + condition = ACTION[start : ACTION.index("\n", ACTION.index("if:", start))] + self.assertNotIn("always()", condition, name) + self.assertNotIn("failure()", condition, name) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 7b5a8aa..83e4d6f 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -32,6 +32,13 @@ if argv[0] == "incremental" and os.environ.get("CB_REQUIRE_FULL") == "true": print(json.dumps({"requiresFullAnalysis": True})) sys.exit(0) +if os.environ.get("CB_ENGINE_ABORT"): + # The engine's refusal contract: a JSON verdict on stdout, no analysis written, exit 3. + print("Analyzing repository...") + print(json.dumps({"mode": argv[0], "error": "LLM quota exhausted: Resource exhausted: token limit reached", + "kind": os.environ["CB_ENGINE_ABORT"], "statusCode": 402, "provider": "openai", + "requiresFullAnalysis": False})) + sys.exit(3) if argv[0] == "full": metadata = {"depth_cap": int(argv[argv.index("--depth-cap") + 1])} with open(analysis, "w") as handle: @@ -176,8 +183,20 @@ def tearDown(self) -> None: self.temp_dir.cleanup() def _analyze(self, **extra: str) -> dict[str, str]: + result = self._invoke(**extra) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + return self._outputs() + + def _outputs(self) -> dict[str, str]: + values: dict[str, str] = {} + for line in self.output.read_text(encoding="utf-8").splitlines(): + key, _, value = line.partition("=") + values[key] = value + return values + + def _invoke(self, **extra: str) -> subprocess.CompletedProcess[str]: self.output.write_text("", encoding="utf-8") - result = subprocess.run( + return subprocess.run( [str(ANALYZE)], env={ "PATH": f"{self.bin_dir}:{os.environ['PATH']}", @@ -203,12 +222,6 @@ def _analyze(self, **extra: str) -> dict[str, str]: text=True, check=False, ) - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - values: dict[str, str] = {} - for line in self.output.read_text(encoding="utf-8").splitlines(): - key, _, value = line.partition("=") - values[key] = value - return values def _engine_calls(self) -> list[dict[str, str]]: return [json.loads(line) for line in self.engine_log.read_text(encoding="utf-8").splitlines()] @@ -328,6 +341,37 @@ def test_sync_without_baseline_uses_configured_depth_directly(self) -> None: self.assertEqual([c["mode"] for c in self._engine_calls()], ["full"]) self.assertEqual(self._engine_calls()[0]["depth"], "4") + def test_a_quota_abort_fails_sync_and_leaves_nothing_to_deliver(self) -> None: + """Deliver baseline and both base publishes read what this step outputs and stages. + A refusal must leave neither, so a map without AI naming is never committed.""" + (self.checkout / ".codeboarding").mkdir() + (self.checkout / ".codeboarding" / "analysis.json").write_text("{}", encoding="utf-8") + result = self._invoke( + ANALYSIS_KIND="sync", FORCE_FULL="true", DEPTH_CAP="2", CB_ENGINE_ABORT="llm_quota_exhausted" + ) + + self.assertEqual(result.returncode, 3, result.stderr or result.stdout) + self.assertIn("::error title=CodeBoarding LLM quota exhausted::", result.stderr) + self.assertEqual(self._outputs(), {}) + self.assertFalse((self.stage_dir / "base").exists()) + error = json.loads((self.runner_temp / "codeboarding-engine-error.json").read_text(encoding="utf-8")) + self.assertEqual((error["kind"], error["exitCode"]), ("llm_quota_exhausted", 3)) + + def test_a_quota_abort_fails_review_and_publishes_no_state(self) -> None: + _state(self.base_dir) + result = self._invoke(CB_ENGINE_ABORT="llm_quota_exhausted") + + self.assertEqual(result.returncode, 3, result.stderr or result.stdout) + self.assertEqual(self._outputs(), {}) + self.assertFalse(self.stage_dir.exists()) + + def test_a_refusal_left_by_an_earlier_run_is_cleared(self) -> None: + stale = self.runner_temp / "codeboarding-engine-error.json" + stale.write_text('{"kind": "llm_quota_exhausted"}', encoding="utf-8") + _state(self.base_dir) + self._analyze() + self.assertFalse(stale.exists()) + def test_a_run_that_stopped_short_of_its_cap_keeps_the_chain(self) -> None: # Core resolves incremental depth from depth_cap, so a realized # depth_level below the cap is not a scope change. diff --git a/tests/test_analyze_repository.py b/tests/test_analyze_repository.py index d394736..defef9d 100644 --- a/tests/test_analyze_repository.py +++ b/tests/test_analyze_repository.py @@ -2,13 +2,16 @@ import io import json +import os +import subprocess import sys import tempfile import unittest from pathlib import Path from unittest.mock import patch -sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "scripts")) +SCRIPTS = Path(__file__).resolve().parent.parent / "scripts" +sys.path.insert(0, str(SCRIPTS)) import analyze_repository as ar @@ -184,5 +187,74 @@ def test_main_rejects_bad_cli_output(self) -> None: ) +class EngineAbortTests(unittest.TestCase): + """The script run as the action runs it, against a stand-in engine that refuses.""" + + def _run(self, exit_code: int, stdout: str) -> tuple[subprocess.CompletedProcess[str], Path]: + tmp = Path(self.enterContext(tempfile.TemporaryDirectory())) + bin_dir = tmp / "bin" + bin_dir.mkdir() + engine = bin_dir / "codeboarding" + engine.write_text( + f"#!/usr/bin/env python3\nimport sys\nsys.stdout.write({stdout!r})\nsys.exit({exit_code})\n", + encoding="utf-8", + ) + engine.chmod(0o755) + (tmp / "repo").mkdir() + runner_temp = tmp / "runner" + runner_temp.mkdir() + result = subprocess.run( + [sys.executable, str(SCRIPTS / "analyze_repository.py"), "incremental"] + + ["--checkout", str(tmp / "repo"), "--output-dir", str(tmp / "out")], + env={"PATH": f"{bin_dir}:{os.environ['PATH']}", "RUNNER_TEMP": str(runner_temp)}, + capture_output=True, + text=True, + check=False, + ) + return result, runner_temp / "codeboarding-engine-error.json" + + @staticmethod + def _verdict(kind: str, status: int) -> str: + payload = { + "mode": "incremental", + "error": "LLM quota exhausted:\nResource exhausted: token limit reached", + "kind": kind, + "statusCode": status, + "provider": "openai", + "requiresFullAnalysis": False, + } + return "Analyzing repository...\n" + json.dumps(payload, indent=2) + "\n" + + def test_quota_exhaustion_keeps_the_engine_exit_code_and_its_reason(self) -> None: + result, error_file = self._run(3, self._verdict("llm_quota_exhausted", 402)) + + self.assertEqual(result.returncode, 3, result.stderr) + self.assertIn( + "::error title=CodeBoarding LLM quota exhausted::LLM quota exhausted: Resource exhausted: token limit reached\n", + result.stderr, + ) + self.assertNotIn("Command failed", result.stderr) + self.assertEqual(result.stdout, "", "no analysis_path for analyze.sh to pick up") + error = json.loads(error_file.read_text(encoding="utf-8")) + self.assertEqual(error["kind"], "llm_quota_exhausted") + self.assertEqual(error["statusCode"], 402) + self.assertEqual(error["exitCode"], 3) + + def test_rejected_credentials_are_reported_by_name(self) -> None: + result, error_file = self._run(2, self._verdict("llm_auth", 401)) + + self.assertEqual(result.returncode, 2, result.stderr) + self.assertIn("::error title=CodeBoarding LLM credentials rejected::", result.stderr) + self.assertEqual(json.loads(error_file.read_text(encoding="utf-8"))["kind"], "llm_auth") + + def test_any_other_failure_keeps_the_generic_path(self) -> None: + for stdout in ("Traceback: boom\n", json.dumps({"error": "boom", "kind": "something_else"})): + with self.subTest(stdout=stdout): + result, error_file = self._run(1, stdout) + self.assertEqual(result.returncode, 1) + self.assertIn("::error::Command failed", result.stderr) + self.assertFalse(error_file.exists()) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_engine_failure.py b/tests/test_engine_failure.py new file mode 100644 index 0000000..c95a45b --- /dev/null +++ b/tests/test_engine_failure.py @@ -0,0 +1,109 @@ +"""scripts/action/engine_failure.py: what a person reads when the engine refused to finish.""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SCRIPT = ROOT / "scripts" / "action" / "engine_failure.py" + +QUOTA = { + "mode": "incremental", + "error": "LLM quota exhausted", + "kind": "llm_quota_exhausted", + "statusCode": 402, + "provider": "openai", + "requiresFullAnalysis": False, + "exitCode": 3, +} + + +class EngineFailureTests(unittest.TestCase): + def _run(self, error: dict | None, **env: str) -> tuple[dict[str, str], str, str]: + tmp = Path(self.enterContext(tempfile.TemporaryDirectory())) + if error is not None: + (tmp / "codeboarding-engine-error.json").write_text(json.dumps(error), encoding="utf-8") + output, summary = tmp / "output", tmp / "summary" + output.write_text("", encoding="utf-8") + result = subprocess.run( + [sys.executable, str(SCRIPT)], + env={ + "PATH": os.environ["PATH"], + "RUNNER_TEMP": str(tmp), + "GITHUB_OUTPUT": str(output), + "GITHUB_STEP_SUMMARY": str(summary), + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_REPOSITORY": "owner/repo", + "GITHUB_RUN_ID": "99", + "GITHUB_RUN_ATTEMPT": "1", + "MODE": "review", + "LLM": "hosted", + "PR_NUMBER": "6", + "HEAD_SHA": "abc123", + **env, + }, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr) + outputs = dict(line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines()) + body = Path(outputs["body_path"]).read_text(encoding="utf-8") if "body_path" in outputs else "" + return outputs, body, summary.read_text(encoding="utf-8") if summary.exists() else "" + + def test_hosted_quota_names_the_reset_and_both_ways_out(self) -> None: + outputs, body, summary = self._run(QUOTA) + + self.assertEqual(outputs["reason"], "llm_quota_exhausted") + self.assertTrue(body.startswith("### CodeBoarding review · stopped: LLM quota used up\n")) + self.assertIn("CodeBoarding stopped instead of publishing a map without AI naming.", body) + self.assertIn("token quota is used up", body) + self.assertIn("per GitHub owner per week and resets Monday 00:00 UTC", body) + self.assertIn("`anthropic_api_key`", body) + self.assertIn("`llm: license` with `license_key`", body) + self.assertEqual(summary, body, "the run page says what the pull request says") + self.assertEqual( + body.rstrip("\n").splitlines()[-1], + "", + ) + + def test_quota_on_your_own_key_points_at_your_provider(self) -> None: + _, body, _ = self._run(QUOTA, LLM="openai") + self.assertIn("On `llm: openai` that is the quota of your own provider account", body) + self.assertNotIn("Monday", body) + + def test_quota_on_a_license_offers_your_own_key(self) -> None: + _, body, _ = self._run(QUOTA, LLM="license") + self.assertIn("your CodeBoarding plan's allowance", body) + self.assertIn("`anthropic_api_key`", body) + + def test_sync_goes_to_the_summary_without_a_comment_marker(self) -> None: + _, body, summary = self._run(QUOTA, MODE="sync", PR_NUMBER="") + self.assertTrue(summary.startswith("### CodeBoarding sync · stopped: LLM quota used up\n")) + self.assertIn("The baseline was not updated, and no base analysis was published.", summary) + self.assertNotIn("