[CI] Debug claude-review-fvm failures: full output + artifact upload - #8630
[CI] Debug claude-review-fvm failures: full output + artifact upload#8630janezpodhostnik wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe workflow now exposes complete Claude action output in GitHub Actions logs and always attempts to upload the Claude execution JSON artifact, warning when the artifact is missing. ChangesClaude review diagnostics
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency ReviewThe following issues were found:
License Issues.github/workflows/claude-review-fvm.yml
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/claude-review-fvm.yml (1)
96-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the action’s
execution_fileoutput instead of a hardcoded runner path.The Claude action exposes the generated execution-log path as
execution_file; use a stepidand reference that output, optionally retaining${{ runner.temp }}as a fallback. This avoids silently missing the artifact if the runner layout changes. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/claude-review-fvm.yml around lines 96 - 101, Update the Claude action step to expose an id, then change the “Upload Claude execution output” step to use that step’s execution_file output as the artifact path, optionally falling back to runner.temp. Remove the hardcoded /home/runner/work/_temp path while preserving the existing artifact name and always condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/claude-review-fvm.yml:
- Around line 60-66: Protect both raw Claude diagnostic outputs from public
exposure: gate or sanitize show_full_output in the workflow debug configuration
at .github/workflows/claude-review-fvm.yml lines 60-66, and apply equivalent
protection to the uploaded JSON artifact at lines 91-102, including shortened
retention or controlled access where applicable.
---
Nitpick comments:
In @.github/workflows/claude-review-fvm.yml:
- Around line 96-101: Update the Claude action step to expose an id, then change
the “Upload Claude execution output” step to use that step’s execution_file
output as the artifact path, optionally falling back to runner.temp. Remove the
hardcoded /home/runner/work/_temp path while preserving the existing artifact
name and always condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73c6daab-7059-497f-af54-7dce91ec28ab
📒 Files selected for processing (1)
.github/workflows/claude-review-fvm.yml
| # TEMPORARY DEBUG: surface Claude's full output to diagnose the | ||
| # recurring `is_error:true` / num_turns:1 / $0 failures. The | ||
| # action's own warning applies: this logs ALL Claude messages | ||
| # (including tool results), publicly visible in the Actions log. | ||
| # Remove once the root cause is identified. | ||
| show_full_output: true | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Guard all raw Claude diagnostics against sensitive-data exposure.
Both the public log and uploaded artifact expose unsanitized Claude messages and tool results. Restrict this debugging mode to controlled access, shorten artifact retention, or sanitize the output before publishing.
.github/workflows/claude-review-fvm.yml#L60-L66: gate or redactshow_full_output..github/workflows/claude-review-fvm.yml#L91-L102: apply the same protection to the uploaded JSON artifact.
📍 Affects 1 file
.github/workflows/claude-review-fvm.yml#L60-L66(this comment).github/workflows/claude-review-fvm.yml#L91-L102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/claude-review-fvm.yml around lines 60 - 66, Protect both
raw Claude diagnostic outputs from public exposure: gate or sanitize
show_full_output in the workflow debug configuration at
.github/workflows/claude-review-fvm.yml lines 60-66, and apply equivalent
protection to the uploaded JSON artifact at lines 91-102, including shortened
retention or controlled access where applicable.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Problem
claude-review-fvmhas failed on every run since it started triggering (5/5, across multiple FVM PRs). Every failure has the identical signature:The actual error message is invisible: the action hides Claude's output ("full output hidden for security"), and the raw output written to
/home/runner/work/_temp/claude-execution-output.jsonis lost when the runner exits. Re-running with Actions debug logging does not help — it only adds runner-internal##[debug]lines.Changes
Two temporary debugging additions to
.github/workflows/claude-review-fvm.yml:show_full_output: trueon the claude-code-action step — prints Claude's messages to the log so the real error is visible (the action warns this echoes all messages incl. tool results; acceptable here for a public repo, to be removed after diagnosis).actions/upload-artifact@v4step (if: always()) that uploadsclaude-execution-output.jsonas theclaude-execution-outputartifact, so the raw error survives the runner even on failure.The model is intentionally not pinned — first gather the actual error, then decide.
Both additions are marked
TEMPORARY DEBUGand should be reverted once the root cause is identified.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit