Fail Claude smoke runs when startup exits before structured log output#16240
Fail Claude smoke runs when startup exits before structured log output#16240
Conversation
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
🧪 Smoke Project is now testing project operations... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
❌ Smoke Copilot SDK failed. Please review the logs for details. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅
|
|
✅ Smoke Temporary ID completed successfully. Temporary ID validation passed. |
|
✅ Smoke Project completed successfully. All project operations validated. |
|
🤖 Beep boop! The smoke test agent just teleported through discussion #16225! 🚀 I left a trail of binary breadcrumbs while testing the Copilot engine. If you hear mysterious clicking sounds, that's just me running more tests in the background! 🎭 Smoke test agent, signing off with style!
|
|
Smoke Test Results: PASS ✅
10/11 tests passed (Serena MCP not available)
|
There was a problem hiding this comment.
Solid implementation of Claude execution validation. The guardrail catches silent failures early.
📰 BREAKING: Report filed by Smoke Copilot for issue #16240
| } finally { | ||
| fs.unlinkSync(logFile); | ||
| fs.rmdirSync(tmpDir); | ||
| } |
There was a problem hiding this comment.
Test coverage for the zero-entries guardrail ensures silent failures are caught.
| // Claude-specific guardrail: if no structured log entries were parsed, treat as execution failure. | ||
| // This catches silent startup failures where Claude exits before producing JSON tool activity. | ||
| if (parserName === "Claude" && (!logEntries || logEntries.length === 0)) { | ||
| core.setFailed("Claude execution failed: no structured log entries were produced. This usually indicates a startup or configuration error before tool execution."); |
There was a problem hiding this comment.
Good safeguard! This prevents silent failures where Claude exits before tool execution.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke test (Codex)
|
Smoke Test: Claude - Run 22081144175Core Tests (1-10): ✅✅✅✅✅✅✅✅✅✅ PR Review Tests (11-17): ✅✅✅ Overall Status: PARTIAL All critical tests passed. 3 tests appropriately skipped due to environmental constraints.
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
The documentation updates look great! Version tracking and quality verification metadata are properly maintained.
💥 [THE END] — Illustrated by Smoke Claude for issue #16240
There was a problem hiding this comment.
Pull request overview
This PR adds a Claude-specific failure guard to catch silent startup failures where Claude exits before producing any structured log entries. Previously, smoke-Claude could report success even when Claude failed during startup because the parser only failed runs for MCP startup failures or max-turns limits.
Changes:
- Added a check in
log_parser_bootstrap.cjsthat fails Claude runs when no structured log entries are produced - Added unit test coverage in
log_parser_bootstrap.test.cjsfor the new failure path - Added integration test in
parse_claude_log.test.cjsto verify failure on unstructured Claude output
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/log_parser_bootstrap.cjs | Added Claude-specific guardrail (lines 212-216) to fail runs when no structured log entries are parsed |
| actions/setup/js/log_parser_bootstrap.test.cjs | Added test case (lines 65-80) to verify Claude runs fail when no structured log entries are produced |
| actions/setup/js/parse_claude_log.test.cjs | Added integration test (lines 441-444) to verify failure on unstructured Claude output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (parserName === "Claude" && (!logEntries || logEntries.length === 0)) { | ||
| core.setFailed("Claude execution failed: no structured log entries were produced. This usually indicates a startup or configuration error before tool execution."); | ||
| } |
There was a problem hiding this comment.
The success message "Claude log parsed successfully" is logged on line 182 before the failure check on line 214. This creates a confusing situation where Claude logs indicate success, then immediately fail the run. The check for empty logEntries should happen earlier, before any success messages are logged, or the success message should be conditional to avoid this contradiction.
Smoke-Claude could report success even when Claude failed during startup, because the parser only failed runs for MCP startup failures or max-turns and did not treat “no structured Claude entries” as an execution failure. This allowed silent startup/configuration regressions (e.g.,
.claude.jsonwrite failures) to evade smoke detection.What changed
logEntries, mark the run failed.Behavioral impact
Targeted test coverage
log_parser_bootstrap.test.cjsfor the Claude + emptylogEntriesfailure path.parse_claude_log.test.cjsto assert failure on unstructured/non-JSON Claude output.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Changeset