ci(review): keep the review run log so failures are diagnosable - #123
Closed
adnaan wants to merge 2 commits into
Closed
ci(review): keep the review run log so failures are diagnosable#123adnaan wants to merge 2 commits into
adnaan wants to merge 2 commits into
Conversation
When the review job fails, the job log reports only the aggregate result — is_error: true alongside a permission_denials_count — and never names the tool that was denied. That is the one fact needed to fix it, and it lives in the execution output file, which is written to the runner temp dir and discarded with the runner. Diagnosing the current failures ruled out three plausible causes and confirmed none: auth is fine (the run initializes and bills), quota is fine (it spent real budget across 4 turns), and the action version is unchanged (the v1 tag has not moved since before the last green run). The denial itself is all that is left, and it is the part not recorded. Upload it unconditionally. Re-triggering a run is cheap but reproducing one is not: which tool the model reaches for varies per PR, so a failure not captured when it happens may not recur on demand. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG
The upload added in the parent commit produced no artifact on the very
first failing run it was meant to capture. The job log showed the step
running with an empty path:
with:
name: claude-review-execution-log
if-no-files-found: warn
A step that fails never sets its outputs, so steps.claude-review.outputs
.execution_file is empty in exactly the case this upload exists for — it
would only ever have captured the runs that did not need capturing.
Fall back to the action documented temp path, which is where it writes
the file regardless of outcome.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG
Contributor
Author
|
Closing — same reason as livetemplate/client#153: the execution log isn't on disk when a later step looks for it, so the capture step could only ever warn "no files found". Superseded by livetemplate/livetemplate#510. This repo's bot recovered on its own and sets no allowlist at all, so it was never affected. |
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
1 participant
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.
Adds a step capturing the review run's execution log as an artifact, so the next failure can be diagnosed instead of guessed at.
Why
When the review job fails, the job log gives only the aggregate:
It never names the tool that was denied — and that is the one fact needed to fix it. The detail lives in the execution output file, written to the runner temp dir and thrown away with the runner.
What this rules out
Diagnosing the current failures eliminated three plausible causes, none of which held:
"model": "claude-sonnet-5") and bills real budget. (The emptyANTHROPIC_API_KEYin the env dump is a red herring — these workflows authenticate viaclaude_code_oauth_token, so that variable is expected to be blank.)@v1tag, but that tag last moved at2026-07-19T02:57:33Z— hours before the last green run. Same version succeeded and failed.What remains is the permission denial itself, which is precisely the part not recorded anywhere durable.
Note on why capture beats re-running
Re-triggering a run is cheap; reproducing a specific failure is not. Which tool the model reaches for varies per PR, so a denial not captured when it happens may simply not recur on demand. Hence
if: always()rather than only-on-failure — a green run's log is also useful as the baseline to diff against.Companion PRs apply the same step to the other two repos, since all three fail the same way and each has a differently-scoped allowlist worth comparing.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG