Repository navigation
feat(comment-triggered-check): reply to each command with its runs - #360
Merged
Merged
Conversation
loft-bot
reviewed
Nov 6, 2026
loft-bot
left a comment
There was a problem hiding this comment.
Panel review: 1 blocking, 4 quality notes, 8 lanes.
Blocking concerns
.github/actions/comment-triggered-check/src/reply.sh:58-81: parallel writers can lose a run link for good. After that, a failed run shows as "did not start".
What was checked
Checked: correctness, security, test-quality, operability, reuse, architecture, typos, PR metadata. Skipped: e2e rubric, gap analysis, infra, entry-point fidelity, dead code, layout (no e2e or infra files, no Go unit tests, no linked customer bug, no deletions or moves). The cross-PR check against #358 found no deleted symbols.
Quality notes (non-blocking)
- consider:
src/start.sh:330: Could we look for an existing reply again right before posting one? Twostartcalls for the same command, in parallel jobs or separate workflows, can both find none and post two replies. This becomes blocking if callers run per-targetstartsteps in parallel. - consider:
README.md:255: Could "Reporting failures to the author" (line 466) namereplyas the opt-in exception? It still says the action posts no comments and points tosticky-pr-comment. - consider:
test/reply.bats:172: Could the "cannot be edited" test also check that PATCH was called twice? Right now it passes even if the retry loop stops after the first attempt. This becomes blocking if a retry is the only thing that keeps a reply current after a brief API error. - nit:
src/lib.sh:585: Could the code span use a fence longer than the longest run of backticks in the command? A command containing two backticks in a row closes the span early, so the rest renders as Markdown, @-mentions included, in the bot's reply.
…l jobs cannot lose them
vcauesantos
marked this pull request as ready for review
November 6, 2026 18:27
This was referenced Nov 6, 2026
Piotr1215
reviewed
Nov 6, 2026
Piotr1215
left a comment
Contributor
There was a problem hiding this comment.
LGTM overall. I left a few comments.
Piotr1215
approved these changes
Nov 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happened
/test-e2estarts runs, but nothing on the pull request links to them. When a command is repeated on the same commit, the newer check replaces the older one in the checks list. The older run is then only reachable through the Actions tab.We saw this on a real pull request this week. Five commands ran over two days, and two of them used the same filter on the same commit. GitHub now lists only one of those two checks.
vcluster-pro posts one "started" comment per pull request, and every new command overwrites it. It has no links. loft-enterprise posts nothing when a command starts.
What this PR changes
Every command can get its own reply. With
reply: "true"andcomment-id, start mode reacts with 👀 to the command. It then posts one reply that says the command is starting, and fills in a row per run, such asPro: running, watch the run. The first version never shows a failure, because it is the one notification emails carry. Each new command gets a new reply, so older runs stay one click away.The reply updates in place. A new
linkmode stores the run URL on the check after dispatch. Finish mode redraws the reply with the outcome: passed, failed, timed out, cancelled, nothing to run, or did not start. Only the first reply is a new comment, so people following the pull request get one notification per command. Every later update is an edit, which notifies nobody.The checks are the source of truth. Each check's
external_idis a small JSON object: the reply id, the workflow attempt, the row label and, once linked, the run URL. Every redraw lists the checks on the commit, keeps this reply's checks from the newest attempt, and draws the rows, labels, status and links from them. The reply's own hidden state holds only the command and the commit. Link and finish find the reply from the check alone, so the suite workflows need no new inputs.GitHub has no conditional edit for comments, so writers can overwrite each other. That is safe here, because nothing a writer puts back is a source of truth. Every writer reads the reply back and redraws if it differs, so a stale write is corrected by the next pass of any job. That covers parallel
--runsjobs linking at once, and a re-run whose cancelled first attempt redraws while the new attempt opens its checks.Only bot comments count. An existing reply must be a comment by a bot account, so a person cannot plant the marker. A run link is drawn only when it is a plain https URL. The command is shown in a code span fenced longer than any backticks it contains, so nothing in it renders as markdown or pings anyone.
The reply never breaks a command. A reply that cannot be posted or edited only warns. Start and finish otherwise behave as before, and link never fails its job.
It is opt-in. Callers that do not set
replyget the same checks and no comments. The only difference is that finish reads the check once more to look for a reply. Socomment-triggered-check/v1can advance after merge. Callers that opt in needpull-requests: writein the jobs that run start, link and finish.linkedits the check, so it needs the same app inputs as start when the app opened the check. Thestartcalls for one command must run one after another.What it looks like
Made-up data, rendered by GitHub exactly as the bot posts it. A command with two runs, while one is still going:
The same reply once both finish:
--runs 3with one failure and one run that never started:How it was tested
bats .github/actions/comment-triggered-check/test/*.bats: 264 tests pass, 44 of them new, in CI and under bash 5 locally. They cover rendering, the start reply, re-runs, a second target in the same run, link mode, and finish redraws. They also cover the failures: listing the comments or the checks, posting or editing the reply, and editing the check. The review's races are replayed: a job that overwrites the reply with an older view, a stale write that drops a row, and a re-run on a newer commit.make lint) are clean, andmake check-docspasses.vClusterLabs-Experiments/github-actions-test, with the callers from test(e2e-command): reply to each command with its runs vClusterLabs-Experiments/github-actions-test#76. The current design ran on test PR test: probe the command reply after review fixes vClusterLabs-Experiments/github-actions-test#78:/test-e2e okwhile its first attempt was still running: both old runs were cancelled, their finish jobs redrew after the new checks opened, and the reply settled on the second attempt's runs, green./test-e2e pro-failand/test-platform-e2e ok --runs 3ended correctly, with every link in place.external_idwith the run URL in it, and no run logged a reply warning./test-e2e ok: both runs passed, and the reply ended with a check mark./test-e2e pro-fail: pro failed and oss passed, and the reply ended with a cross./test-e2e nothing: both rows said nothing to run./test-platform-e2e ok --runs 3: three parallel jobs linked the same reply, and all three links point at different runs./test-e2e okworkflow reused the same reply, started its rows over with the new checks, and finished green.external_id. The bot reacted, posted and edited every reply, and none of the 21 runs logged a reply warning.The timeout work in #358 changes the same action:
start.sh,lib.sh,action.yml, the README,lib.batsandstart.bats. Whichever PR merges second needs a rebase.Related to DEVOPS-1843