fix: don't let a late failure erase a review that already posted by Svilen-Stefanov · Pull Request #66 · CodeBoarding/CodeBoarding-action

Svilen-Stefanov · 2026-08-01T06:40:25Z

The bug

The failure and quota comments reuse the review comment's sticky header, so posting
either one replaces the architecture diff instead of adding to it. Any step that fails
after the review has been published therefore deletes the thing the user came for, and
the PR is left showing "Architecture review · failed" for a review that actually worked.

Seen on CodeBoarding-webview#22
(run) — every
step that matters succeeded:

diagram                 outcome=success
body                    outcome=success
upload_review_artifact  outcome=success
Post sticky PR comment  outcome=success   ← the review WAS published
<enrichment step>       outcome=failure   ← then this failed

…and the failure comment then overwrote it. The trigger there was an add-on step on an
unmerged branch, but the defect is on main and any late failure hits it — a flaky
upload-artifact, a transient API error while posting.

The fix

Give the review comment an id, and gate both failure paths on it:

- name: Post sticky PR comment
  id: review_comment
  …

- name: Post failure comment
  if: … && steps.review_comment.outcome != 'success'

A step failing after publication cannot un-publish the review, so it must not erase it.

Genuine failures still report

!= 'success' rather than == 'failure' on purpose — it fails open:

Case	`review_comment.outcome`	Failure comment
Analysis fails before the comment	`skipped` / empty	posts (unchanged)
The comment step itself fails	`failure`	posts (unchanged)
Quota exhausted before the review	`skipped`	quota comment posts (unchanged)
Review posts, later step fails	`success`	suppressed (the fix)
Sync mode	n/a	already gated on `mode == 'review'`

Notes

action.yml only; action.yml is byte-identical between v1 and main, so this
applies cleanly to the shipping code.
actionlint reports nothing for action.yml. It does flag create-github-app-token
inputs in this repo's own workflows, but that is pre-existing, untouched by this PR, and
does not reproduce under the version CI pins (v1.7.7; local was 1.7.12).
Per AGENT.md, merging alone ships nothing — this needs a release to re-point @v1
before consumers pick it up.

🤖 Generated with Claude Code

The failure and quota comments reuse the review comment's sticky header, so posting either one REPLACES the architecture diff rather than adding to it. Any step that fails after the review has been published therefore deletes the thing the user came for, and the PR ends up showing "Architecture review · failed" for a review that succeeded. Seen on CodeBoarding-webview#22: diagram, body, artifact upload and the sticky comment all succeeded, then an enrichment step failed and overwrote the result. Gives the review comment an `id` and gates both failure paths on `steps.review_comment.outcome != 'success'`. A step failing after publication cannot un-publish the review, so it must not erase it either. Every genuine failure path is unaffected: when the review never posts the outcome is `skipped` (or empty, if the job died earlier), both of which are != 'success', so the failure and quota comments still appear as before. Sync mode is untouched — those steps are already gated on `mode == 'review'`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

codeboarding-review · 2026-08-01T06:40:46Z

Architecture review · no architectural changes

graph LR
    n_Analysis_Engine_Adapter["Analysis Engine Adapter"]
    n_Visual_Rendering_Engine["Visual Rendering Engine"]
    n_Structural_Diff_Engine["Structural Diff Engine"]
    n_Telemetry_Feedback_Handler["Telemetry #38; Feedback Handler"]
    n_Interaction_Orchestrator["Interaction Orchestrator"]
    n_Analysis_Engine_Adapter -- "Orchestrates documentation generation" --> n_Visual_Rendering_Engine
    n_Visual_Rendering_Engine -- "Queries for architectural changes" --> n_Structural_Diff_Engine
    n_Structural_Diff_Engine -- "Provides change-set data" --> n_Visual_Rendering_Engine
    n_Telemetry_Feedback_Handler -- "Captures user intent from UI" --> n_Interaction_Orchestrator
    n_Interaction_Orchestrator -- "Consumes issue counts for UI" --> n_Visual_Rendering_Engine
    classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
    classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
    classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;

Colors indicate component changes compared to target branch main: 🟩 Added · 🟨 Modified · 🟥 Removed

Download the PR analysis artifacts from this workflow artifact.

⚠️ 1 architecture issue found — open CodeBoarding to explore them.

Explore this PR’s architecture in your browser or VS Code.

_{codeboarding-action · run 30688190355}

ivanmilevtues

good catch merging.

ivanmilevtues · 2026-08-01T18:23:51Z


    - name: Post quota-exhausted comment
-      if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.quota.outputs.exhausted == 'true'
+      if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.quota.outputs.exhausted == 'true' && steps.review_comment.outcome != 'success'


Yep seems reasonable.

ivanmilevtues · 2026-08-01T18:23:57Z

+    # add-on, an artifact upload) cannot un-publish it, and must not erase it either.
    - name: Post failure comment
-      if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.quota.outputs.exhausted != 'true'
+      if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.quota.outputs.exhausted != 'true' && steps.review_comment.outcome != 'success'


proly should be refactored with a single if failure branch and then have different things for the comments

Svilen-Stefanov self-assigned this Aug 1, 2026

Svilen-Stefanov requested a review from ivanmilevtues August 1, 2026 06:40

ivanmilevtues approved these changes Aug 1, 2026

View reviewed changes

ivanmilevtues merged commit 4606c1a into main Aug 1, 2026
3 checks passed

github-actions Bot mentioned this pull request Aug 1, 2026

chore(main): release 1.9.1 #67

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: don't let a late failure erase a review that already posted - #66

fix: don't let a late failure erase a review that already posted#66
ivanmilevtues merged 1 commit into
mainfrom
fix/failure-comment-clobbers-posted-review

Svilen-Stefanov commented Aug 1, 2026

Uh oh!

codeboarding-review Bot commented Aug 1, 2026 •

edited

Loading

Uh oh!

ivanmilevtues left a comment

Uh oh!

ivanmilevtues Aug 1, 2026

Uh oh!

ivanmilevtues Aug 1, 2026

Uh oh!

ivanmilevtues Aug 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Svilen-Stefanov commented Aug 1, 2026

The bug

The fix

Genuine failures still report

Notes

Uh oh!

codeboarding-review Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Architecture review · no architectural changes

Uh oh!

ivanmilevtues left a comment

Choose a reason for hiding this comment

Uh oh!

ivanmilevtues Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

ivanmilevtues Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

ivanmilevtues Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codeboarding-review Bot commented Aug 1, 2026 •

edited

Loading