From 520378fe480302d45a60e8d2fd07328604d9de93 Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:15:33 +0000 Subject: [PATCH 1/6] fix(report-failure): dedup outage comments per run across matrix legs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A matrix workflow (e.g. review-reviewers, 5 legs) invokes report-failure.sh once per leg on an outage, every leg sharing one GITHUB_RUN_ID. The existing reconcile handles the create-create race (duplicate *issues*) but nothing dedups the append path, so each leg posted its own near-identical row — flooding the tend-outage issue with 5-6 comments all citing the same run. Guard the comment on the run not already being recorded (body or existing comment), then reconcile symmetrically to the issue path: keep the earliest comment citing this run, delete later duplicates. Convergent across concurrently-jittered legs. --- shared/steps/report-failure.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/shared/steps/report-failure.sh b/shared/steps/report-failure.sh index cc73608f..0a238c76 100755 --- a/shared/steps/report-failure.sh +++ b/shared/steps/report-failure.sh @@ -49,8 +49,34 @@ sleep $((RANDOM % 30)) EXISTING=$(gh issue list --label "$LABEL" --state open --json number --jq '.[0].number // empty') if [ -n "$EXISTING" ]; then + # Per-run comment dedup. A matrix workflow invokes this script once per leg, + # every leg sharing one GITHUB_RUN_ID (and thus one RUN_URL). Without a guard + # each leg appends its own near-identical row and floods the issue (a 5-leg + # matrix failing during an outage → 5 comments all citing the same run). + # Skip if this run is already recorded — whether in the issue body (a leg of + # this same run seeded the issue) or in an existing comment. + if gh issue view "$EXISTING" --json body,comments \ + --jq '.body + "\n" + ([.comments[].body] | join("\n"))' \ + | grep -qF "$RUN_URL"; then + echo "Run ${GITHUB_RUN_ID} already recorded on #${EXISTING} — skipping duplicate comment" + exit 0 + fi printf '%s\n' "$TABLE" > /tmp/comment.md gh issue comment "$EXISTING" -F /tmp/comment.md + + # The check-then-act above still races across concurrently-jittered legs: two + # can both read no matching row before either posts. Reconcile to one row per + # run — keep the earliest comment citing this RUN_URL, delete later dups. + # Convergent, mirroring the issue reconcile below: every racing leg sorts the + # same way and computes the same keeper, so deleting an already-deleted + # comment is a harmless 404. + sleep 5 + gh api "repos/${GITHUB_REPOSITORY}/issues/${EXISTING}/comments?per_page=100" \ + --jq "[.[] | select(.body | contains(\"${RUN_URL}\"))] | sort_by(.created_at) | .[1:] | .[].id" \ + | while read -r DUP_ID; do + [ -z "$DUP_ID" ] && continue + gh api -X DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${DUP_ID}" 2>/dev/null || true + done else printf '%s\n\n%s\n\n%s\n' \ "The bot failed to process a request. This issue tracks failures until the underlying cause is resolved." \ From 86f39ccbde15d929e9bb255e11d4d152a5f6c70d Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 04:21:56 +0000 Subject: [PATCH 2/6] fix(report-failure): match generated-row anchor so dedup never touches human comments --- shared/steps/report-failure.sh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/shared/steps/report-failure.sh b/shared/steps/report-failure.sh index 0a238c76..2e2d33c9 100755 --- a/shared/steps/report-failure.sh +++ b/shared/steps/report-failure.sh @@ -54,10 +54,13 @@ if [ -n "$EXISTING" ]; then # each leg appends its own near-identical row and floods the issue (a 5-leg # matrix failing during an outage → 5 comments all citing the same run). # Skip if this run is already recorded — whether in the issue body (a leg of - # this same run seeded the issue) or in an existing comment. + # this same run seeded the issue) or in an existing comment. Match the full + # generated-row anchor, not the bare URL, so a human comment that merely + # mentions the run URL can't suppress the bot's own row (and so a longer run + # id that has this one as a prefix can't false-positive). if gh issue view "$EXISTING" --json body,comments \ --jq '.body + "\n" + ([.comments[].body] | join("\n"))' \ - | grep -qF "$RUN_URL"; then + | grep -qF "[workflow run](${RUN_URL})"; then echo "Run ${GITHUB_RUN_ID} already recorded on #${EXISTING} — skipping duplicate comment" exit 0 fi @@ -69,10 +72,12 @@ if [ -n "$EXISTING" ]; then # run — keep the earliest comment citing this RUN_URL, delete later dups. # Convergent, mirroring the issue reconcile below: every racing leg sorts the # same way and computes the same keeper, so deleting an already-deleted - # comment is a harmless 404. + # comment is a harmless 404. Select on the full generated-row anchor, not the + # bare URL, so only the bot's auto-generated rows are ever eligible for + # deletion (no human content is touched). sleep 5 gh api "repos/${GITHUB_REPOSITORY}/issues/${EXISTING}/comments?per_page=100" \ - --jq "[.[] | select(.body | contains(\"${RUN_URL}\"))] | sort_by(.created_at) | .[1:] | .[].id" \ + --jq "[.[] | select(.body | contains(\"[workflow run](${RUN_URL})\"))] | sort_by(.created_at) | .[1:] | .[].id" \ | while read -r DUP_ID; do [ -z "$DUP_ID" ] && continue gh api -X DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${DUP_ID}" 2>/dev/null || true From 834734ec36804db32089f679a77b900587c00195 Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Fri, 7 Aug 2026 07:24:15 +0000 Subject: [PATCH 3/6] fix(report-failure): paginate the reconcile, and cover it with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue comments come back oldest-first, so the unpaginated `?per_page=100` read returned only the oldest page. Past 100 comments the rows this run and its racing sibling just posted are not in the response at all and the reconcile silently no-ops — on exactly the flooded trackers it exists for (#831 reached 77 rows in one outage). `--paginate` alone doesn't fix it: `gh` applies `--jq` per page, so each page would keep its own earliest comment and `sort_by | .[1:]` would delete the keeper. `--slurp` refuses `--jq`, so the filter moves downstream with `add` flattening the pages. Tests cover both halves of the dedup against a fake `gh`, alongside the existing `rate-limit-preflight.sh` and `mark-notification-read.sh` suites: the guard skipping a run already recorded, the reconcile converging two racing legs to one row, the anchor scoping that keeps a human comment naming the run out of the delete set, and the pagination case — which is the only one of the five that fails against the pre-fix script. --- generator/tests/test_shared_steps.py | 260 +++++++++++++++++++++++++++ shared/steps/report-failure.sh | 14 +- 2 files changed, 272 insertions(+), 2 deletions(-) diff --git a/generator/tests/test_shared_steps.py b/generator/tests/test_shared_steps.py index 7d603abb..1cb4275c 100644 --- a/generator/tests/test_shared_steps.py +++ b/generator/tests/test_shared_steps.py @@ -575,3 +575,263 @@ def test_rate_limit_reopens_rather_than_refiling( assert not any(c.startswith("issue create") for c in calls), ( f"filed a second pause issue instead of reopening #42: {calls}" ) + + +REPORT_FAILURE = REPO_ROOT / "shared" / "steps" / "report-failure.sh" + +OUTAGE_TITLE = "Bot temporarily unavailable" +OUTAGE_LABEL = "tend-outage" +# The anchor `run_issue_anchor` builds from the fixture's server/repo/run id. +# Both dedup matchers select on it, so it is what the fixtures have to carry. +RUN_LINK = "[workflow run](https://github.com/owner/repo/actions/runs/12345)" +POSTED_AT = "2026-01-02T12:00:00Z" + +# `gh` stand-in for the outage reporter. Same shape as the rate-limit fake — +# fixtures in, the script's own filters doing the work — with two additions the +# comment dedup needs. `issue comment` appends to the comment fixture, so the +# reconcile that follows sees the row this run just posted, and the comment +# list is chunked into pages of 100 only when `--slurp` asks for them: an +# unpaginated read gets the oldest page alone, exactly as the API serves it. +FAKE_GH_REPORT_FAILURE = r"""#!/usr/bin/env bash +printf '%s\n' "$*" >> "$GH_CALLS" + +jq_expr="" +slurp="" +prev="" +for arg in "$@"; do + [ "$prev" = "--jq" ] && jq_expr="$arg" + [ "$arg" = "--slurp" ] && slurp=1 + prev="$arg" +done + +emit() { + if [ -n "$jq_expr" ]; then + printf '%s' "$1" | jq -r "$jq_expr" + else + printf '%s' "$1" + fi +} + +case "$1 $2" in + "issue list") emit "$(cat "$OPEN_ISSUES_JSON")" ;; + "issue view") emit "$(cat "$KEEPER_JSON")" ;; + "issue comment") + body=$(cat) + printf '%s\n' "$body" >> "$COMMENT_BODIES" + jq -c --arg b "$body" --arg t "$POSTED_AT" \ + '. + [{id: ((map(.id) | max // 0) + 1), created_at: $t, body: $b}]' \ + "$ISSUE_COMMENTS_JSON" > "$ISSUE_COMMENTS_JSON.tmp" + mv "$ISSUE_COMMENTS_JSON.tmp" "$ISSUE_COMMENTS_JSON" + ;; + "issue create") + cat > /dev/null + echo "https://github.com/owner/repo/issues/${FAKE_NEW_ISSUE}" + ;; + "issue close" | "label create") ;; + *) + case "$*" in + *"/comments?per_page=100"*) + # Paged the way the endpoint pages, whether or not the caller asked + # for every page: `--slurp` gets the array of pages, a plain read gets + # the oldest 100 alone. Both go through `emit`, so a caller passing + # `--jq` has its own filter applied to what it actually received. + if [ -n "$slurp" ]; then + emit "$(jq -c '[_nwise(100)]' "$ISSUE_COMMENTS_JSON")" + else + emit "$(jq -c '.[0:100]' "$ISSUE_COMMENTS_JSON")" + fi + ;; + *"-X DELETE"*) ;; + *) exit 1 ;; + esac + ;; +esac +""" + + +@pytest.fixture +def report_failure_env(tmp_path: Path) -> dict[str, str]: + """Fake gh/sleep on PATH, plus the Actions env the reporter reads.""" + bindir = tmp_path / "fakebin" + bindir.mkdir() + for name, body in (("gh", FAKE_GH_REPORT_FAILURE), ("sleep", FAKE_SLEEP)): + path = bindir / name + path.write_text(body) + path.chmod(0o755) + + jq = shutil.which("jq") + assert jq, "jq is required for these tests" + + event = tmp_path / "event.json" + event.write_text(json.dumps({"pull_request": {"number": 851}})) + (tmp_path / "open-issues.json").write_text( + json.dumps([{"number": 42, "title": OUTAGE_TITLE}]) + ) + (tmp_path / "issue-comments.json").write_text("[]") + (tmp_path / "keeper.json").write_text('{"body": "", "comments": []}') + (tmp_path / "comment-bodies.txt").write_text("") + + return { + "PATH": f"{bindir}:{Path(jq).parent}:/usr/bin:/bin", + "GH_CALLS": str(tmp_path / "gh-calls.log"), + "OPEN_ISSUES_JSON": str(tmp_path / "open-issues.json"), + "ISSUE_COMMENTS_JSON": str(tmp_path / "issue-comments.json"), + "KEEPER_JSON": str(tmp_path / "keeper.json"), + "COMMENT_BODIES": str(tmp_path / "comment-bodies.txt"), + "POSTED_AT": POSTED_AT, + "FAKE_NEW_ISSUE": "42", + "GITHUB_REPOSITORY": "owner/repo", + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_RUN_ID": "12345", + "GITHUB_EVENT_NAME": "pull_request_target", + "GITHUB_EVENT_PATH": str(event), + } + + +def _run_report_failure(env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["bash", str(REPORT_FAILURE)], env=env, capture_output=True, text=True + ) + + +def _comments(env: dict[str, str]) -> str: + return Path(env["COMMENT_BODIES"]).read_text() + + +def _deleted(env: dict[str, str]) -> list[str]: + """Comment ids the reconcile deleted.""" + return [c.rsplit("/", 1)[-1] for c in _calls(env) if "-X DELETE" in c] + + +def _issue_comments(env: dict[str, str], *comments: dict) -> None: + """Seed the comment list the reconcile reads, oldest-first as the API serves it.""" + Path(env["ISSUE_COMMENTS_JSON"]).write_text(json.dumps(list(comments))) + + +def _comment(number: int, body: str, at: str) -> dict: + return {"id": number, "created_at": at, "body": body} + + +def _filler(count: int, *, first_id: int = 1) -> list[dict]: + """Unrelated comments, none carrying this run's anchor.""" + return [ + _comment(first_id + i, f"nightly enrichment {i}", f"2026-01-01T00:{i:02d}:00Z") + for i in range(count) + ] + + +def _seen_by_the_guard(env: dict[str, str], *bodies: str) -> None: + """What `gh issue view --json body,comments` returns for the tracker.""" + Path(env["KEEPER_JSON"]).write_text( + json.dumps({"body": "", "comments": [{"body": b} for b in bodies]}) + ) + + +def test_report_failure_skips_a_run_already_recorded( + report_failure_env: dict[str, str], +) -> None: + """A leg whose sibling already recorded this run posts nothing. + + This is the guard that collapses the flood: a matrix workflow calls the + script once per leg, every leg sharing one GITHUB_RUN_ID, so without it a + 5-leg matrix leaves 5 comments all citing the same run. + """ + _seen_by_the_guard(report_failure_env, f"| when | {RUN_LINK} | #851 |") + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, result.stderr + assert not _comments(report_failure_env), ( + f"appended a second row for a run already recorded: " + f"{_comments(report_failure_env)!r}" + ) + + +def test_report_failure_appends_a_row_for_an_unrecorded_run( + report_failure_env: dict[str, str], +) -> None: + """The happy path: a run the tracker has not seen still gets its row.""" + _seen_by_the_guard(report_failure_env, "some other run's row") + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, result.stderr + assert RUN_LINK in _comments(report_failure_env) + + +def test_report_failure_reconciles_a_racing_leg( + report_failure_env: dict[str, str], +) -> None: + """Two legs that both read the tracker before either posted converge to one row. + + The guard is a check-then-act, so jittered legs can both miss. Every leg + sorts the same list the same way, so each computes the same keeper — the + earliest — and deletes the rest. + """ + _seen_by_the_guard(report_failure_env, "nothing recorded yet") + _issue_comments( + report_failure_env, + _comment(1, f"| when | {RUN_LINK} | #851 |", "2026-01-02T11:59:00Z"), + ) + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, result.stderr + assert _deleted(report_failure_env) == ["2"], ( + f"expected the later of the two rows deleted, got " + f"{_deleted(report_failure_env)}" + ) + + +def test_report_failure_reconciles_past_the_first_page( + report_failure_env: dict[str, str], +) -> None: + """The flood the reconcile exists for is exactly where it must paginate. + + Issue comments come back oldest-first, so on a tracker past 100 comments an + unpaginated read returns only the oldest page — the rows this run and its + racing sibling just posted are not in it, and the reconcile no-ops on the + one issue that needed it. + """ + _seen_by_the_guard(report_failure_env, "nothing recorded yet") + _issue_comments( + report_failure_env, + *_filler(138), + _comment(139, f"| when | {RUN_LINK} | #851 |", "2026-01-02T11:59:00Z"), + ) + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, result.stderr + assert _deleted(report_failure_env) == ["140"], ( + f"the reconcile did not reach past the first page of comments; deleted " + f"{_deleted(report_failure_env)}" + ) + + +def test_report_failure_leaves_a_human_comment_naming_the_run( + report_failure_env: dict[str, str], +) -> None: + """Only the bot's own generated rows are eligible for deletion. + + The reconcile deletes, so its predicate is the whole protection. Selecting + on the bare run URL would make a person linking the run in discussion — the + normal way an outage gets diagnosed — a duplicate to be removed. + """ + _seen_by_the_guard(report_failure_env, "nothing recorded yet") + _issue_comments( + report_failure_env, + _comment( + 1, + "https://github.com/owner/repo/actions/runs/12345 is the one that failed", + "2026-01-02T11:00:00Z", + ), + ) + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, result.stderr + assert not _deleted(report_failure_env), ( + f"deleted a human comment that merely named the run: " + f"{_deleted(report_failure_env)}" + ) diff --git a/shared/steps/report-failure.sh b/shared/steps/report-failure.sh index 0b781ed3..eec08297 100755 --- a/shared/steps/report-failure.sh +++ b/shared/steps/report-failure.sh @@ -68,9 +68,19 @@ if [ -n "$EXISTING" ]; then # every racing leg sorts the same way and computes the same keeper, so # deleting an already-deleted comment is a harmless 404. Selecting on the # anchor keeps only the bot's own generated rows eligible for deletion. + # Paginated, because this endpoint returns comments oldest-first and the + # issues that need reconciling are exactly the flooded ones: past 100 + # comments the rows just posted fall off the first page, and an unpaginated + # read would find nothing to reconcile on the only issues where it matters. + # `--paginate` alone won't do — `gh` applies `--jq` per page, so each page + # would keep its own earliest comment and `sort_by | .[1:]` would delete the + # keeper. `--slurp` refuses `--jq`, hence the filter moved downstream, with + # `add` flattening the array of pages. On an issue with no comments `--slurp` + # yields `[[]]`, so `add` gives `[]` and the filter emits nothing. sleep 5 - gh api "repos/${GITHUB_REPOSITORY}/issues/${EXISTING}/comments?per_page=100" \ - --jq "[.[] | select(.body | contains(\"${ANCHOR}\"))] | sort_by(.created_at) | .[1:] | .[].id" \ + gh api --paginate --slurp \ + "repos/${GITHUB_REPOSITORY}/issues/${EXISTING}/comments?per_page=100" \ + | jq -r "add | [.[] | select(.body | contains(\"${ANCHOR}\"))] | sort_by(.created_at) | .[1:] | .[].id" \ | while read -r DUP_ID; do [ -z "$DUP_ID" ] && continue gh api -X DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${DUP_ID}" 2>/dev/null || true From bb0a92d39bc83e46acd0c7db8f9262a622f51d47 Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Fri, 7 Aug 2026 07:37:48 +0000 Subject: [PATCH 4/6] test(report-failure): model gh's --slurp/--jq refusal, cover the body guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps in the suite added a commit ago, both found by mutating the script: The fake applied `--jq` whatever flags came with it, so `--slurp --jq` was accepted in the test and rejected in production. That combination is the trap the reconcile's comment warns about — folding the filter back into `gh --jq` is the obvious simplification, and it left all five tests green while the script died under pipefail right after posting its row, never reconciling. The fake now refuses it the way `gh` does. The guard matches `.body + "\n" + comments`, but every case reached it through the comments list, so dropping the body half from the script's `--jq` left the suite green. That half carries the first failed run of an outage: one leg takes the `else` branch and seeds the issue with its row in the body, and its siblings then hit the `EXISTING` branch against a tracker with no comments at all. Parametrized, at the cost of one extra case. Confirmed by re-running each mutation: the first now fails 4 tests, the second fails `skips_a_run_already_recorded[in-the-issue-body]`. --- generator/tests/test_shared_steps.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/generator/tests/test_shared_steps.py b/generator/tests/test_shared_steps.py index 1cb4275c..3896a2be 100644 --- a/generator/tests/test_shared_steps.py +++ b/generator/tests/test_shared_steps.py @@ -604,6 +604,14 @@ def test_rate_limit_reopens_rather_than_refiling( prev="$arg" done +# Real `gh` refuses the combination outright and exits 1, and the reconcile's +# shape rests on that: fold the filter back into `--jq` and the script dies +# under pipefail just after posting its row, never reconciling. +if [ -n "$slurp" ] && [ -n "$jq_expr" ]; then + echo "the --slurp option is not supported with --jq or --template" >&2 + exit 1 +fi + emit() { if [ -n "$jq_expr" ]; then printf '%s' "$1" | jq -r "$jq_expr" @@ -720,23 +728,32 @@ def _filler(count: int, *, first_id: int = 1) -> list[dict]: ] -def _seen_by_the_guard(env: dict[str, str], *bodies: str) -> None: +def _seen_by_the_guard(env: dict[str, str], *bodies: str, body: str = "") -> None: """What `gh issue view --json body,comments` returns for the tracker.""" Path(env["KEEPER_JSON"]).write_text( - json.dumps({"body": "", "comments": [{"body": b} for b in bodies]}) + json.dumps({"body": body, "comments": [{"body": b} for b in bodies]}) ) +@pytest.mark.parametrize( + ("body", "comments"), + [ + pytest.param("", (f"| when | {RUN_LINK} | #851 |",), id="in-a-comment"), + pytest.param(f"| when | {RUN_LINK} | #851 |", (), id="in-the-issue-body"), + ], +) def test_report_failure_skips_a_run_already_recorded( - report_failure_env: dict[str, str], + report_failure_env: dict[str, str], body: str, comments: tuple[str, ...] ) -> None: """A leg whose sibling already recorded this run posts nothing. This is the guard that collapses the flood: a matrix workflow calls the script once per leg, every leg sharing one GITHUB_RUN_ID, so without it a - 5-leg matrix leaves 5 comments all citing the same run. + 5-leg matrix leaves 5 comments all citing the same run. The body case is + the first run of an outage: one leg seeds the issue with its row, and the + siblings that follow have no comment to match — only the body. """ - _seen_by_the_guard(report_failure_env, f"| when | {RUN_LINK} | #851 |") + _seen_by_the_guard(report_failure_env, *comments, body=body) result = _run_report_failure(report_failure_env) From 55bb271f54bd887bc71cc7d5e8389a9f98d27a54 Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Sun, 9 Aug 2026 07:14:23 +0000 Subject: [PATCH 5/6] refactor(run-issue): fold the create path's anchor into run_issue_anchor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging main brought #836's create-path dedup alongside this branch's run_issue_anchor helper, leaving the anchor built twice — once by the helper the row is written from, once by hand in run_issue_create_and_reconcile. Change the row's link text and the create path's guard stops matching it, silently. Route it through the helper, as this PR's coordination note said the second lander should. --- shared/steps/lib/run-issue.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/shared/steps/lib/run-issue.sh b/shared/steps/lib/run-issue.sh index 7d46b0a8..97563a3b 100644 --- a/shared/steps/lib/run-issue.sh +++ b/shared/steps/lib/run-issue.sh @@ -209,14 +209,15 @@ run_issue_create_and_reconcile() { # row would just duplicate it, differing only in its timestamp. The # cross-workflow race has distinct run ids, so it still carries over. # Anchor on the generated row's run link rather than the bare id, so a human - # comment mentioning the run cannot suppress the row. Read into a variable - # rather than piped to grep, which would close the pipe under `pipefail` and - # could report a match as a read failure. - local seen run_url - run_url="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + # comment mentioning the run cannot suppress the row — through + # `run_issue_anchor`, the one definition `run_issue_row` writes, so the row + # and the matcher cannot drift apart. Read into a variable rather than piped + # to grep, which would close the pipe under `pipefail` and could report a + # match as a read failure. + local seen seen=$(gh issue view "$keep" --json body,comments \ --jq '.body + "\n" + ([.comments[].body] | join("\n"))' 2>/dev/null || true) - if ! grep -qF "[workflow run](${run_url})" <<< "$seen"; then + if ! grep -qF "$(run_issue_anchor)" <<< "$seen"; then printf '%s\n' "$row" | gh issue comment "$keep" -F - >&2 || true fi From 32891130d4905b2b005a631cc8c79b6ed50abfc2 Mon Sep 17 00:00:00 2001 From: tend-agent <270458913+tend-agent@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:00:55 +0000 Subject: [PATCH 6/6] fix(report-failure): warn rather than abort when the reconcile read fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment reconcile is the last statement in the append branch, so under `set -eo pipefail` a 5xx on `gh api --paginate --slurp` took the script's exit status with it — after the row had already been posted. That reddens the `Report failure` step with no annotation naming why, on precisely the job someone is about to diagnose. Guard it the way the append immediately above and the tracker read at the top are guarded: warn and continue. A failed reconcile leaves duplicate rows on the tracker, which is the cheaper loss than a red step over a completed write. `||` binds to the whole pipeline, so `done ||` covers the read, the filter, and the delete loop. --- generator/tests/test_shared_steps.py | 33 ++++++++++++++++++++++++++++ shared/steps/report-failure.sh | 8 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/generator/tests/test_shared_steps.py b/generator/tests/test_shared_steps.py index dbaadf4d..dc12a603 100644 --- a/generator/tests/test_shared_steps.py +++ b/generator/tests/test_shared_steps.py @@ -1767,6 +1767,10 @@ def test_notifications_check_counts_from_the_snapshot_when_the_recount_fails( # (`--paginate`, `-X DELETE`) where the path would otherwise sit. case "$*" in *"/comments?per_page=100"*) + if [ -n "${FAIL_COMMENT_LIST:-}" ]; then + echo "gh: 502 server error" >&2 + exit 1 + fi # Paged the way the endpoint pages, whether or not the caller asked # for every page: `--slurp` gets the array of pages, a plain read gets # the oldest 100 alone. Both go through `emit`, so a caller passing @@ -2205,6 +2209,35 @@ def test_report_failure_reconciles_past_the_first_page( ) +def test_report_failure_survives_a_failed_reconcile_read( + report_failure_env: dict[str, str], +) -> None: + """A 5xx on the reconcile's read must not redden a step whose row landed. + + The reconcile is best-effort cleanup and the last statement in the append + branch, so left bare under `set -eo pipefail` a failed read takes the whole + script's status with it — *after* the write succeeded. That reddens the + `Report failure` step with no annotation naming why, on precisely the job + someone is about to diagnose. Duplicate rows on the tracker are the better + failure: the append immediately above warns and continues for the same + reason. + """ + _open_tracker(report_failure_env) + _seen_by_the_guard(report_failure_env, "nothing recorded yet") + report_failure_env["FAIL_COMMENT_LIST"] = "1" + + result = _run_report_failure(report_failure_env) + + assert result.returncode == 0, ( + f"a failed reconcile read reddened the step; stdout:\n{result.stdout}" + ) + assert "::warning::" in result.stdout, result.stdout + assert RUN_LINK in _comments(report_failure_env), ( + f"lost the row the reconcile was cleaning up after: " + f"{_comments(report_failure_env)!r}" + ) + + def test_report_failure_leaves_a_human_comment_naming_the_run( report_failure_env: dict[str, str], ) -> None: diff --git a/shared/steps/report-failure.sh b/shared/steps/report-failure.sh index 128f4a86..dfb87512 100755 --- a/shared/steps/report-failure.sh +++ b/shared/steps/report-failure.sh @@ -103,6 +103,12 @@ if [ -n "$EXISTING" ]; then # keeper. `--slurp` refuses `--jq`, hence the filter moved downstream, with # `add` flattening the array of pages. On an issue with no comments `--slurp` # yields `[[]]`, so `add` gives `[]` and the filter emits nothing. + # + # Guarded like the append above, and for the same reason: this is the last + # statement in the branch, so a 5xx on the read would take the script's exit + # status with it *after* the row landed — a red step carrying no annotation + # to say why, on the job someone is about to diagnose. Best-effort cleanup + # failing leaves duplicate rows on the tracker, which is the cheaper loss. sleep 5 gh api --paginate --slurp \ "repos/${GITHUB_REPOSITORY}/issues/${EXISTING}/comments?per_page=100" \ @@ -110,7 +116,7 @@ if [ -n "$EXISTING" ]; then | while read -r DUP_ID; do [ -z "$DUP_ID" ] && continue gh api -X DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${DUP_ID}" 2>/dev/null || true - done + done || echo "::warning::Could not reconcile duplicate rows on #${EXISTING}; this run's row is recorded." else printf '%s\n\n%s\n\n%s\n' \ "The bot failed to process a request. This issue tracks failures until the underlying cause is resolved." \