Conversation
…n step Crush, like Claude Code, requires post-install scripts to download native binaries. Remove --ignore-scripts from the crush npm install command and add a `crush --version` step to force deferred binary download. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b1b42986-fd30-44bf-b8fd-6902520ce31d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b1b42986-fd30-44bf-b8fd-6902520ce31d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent test quality
Test Classification Details
AnalysisThe modified test in
Both assertions verify observable behavior that users and operators depend on. The use of Test inflation: 26 lines added to test vs. 24 to production — ratio 1.08:1, well within the 2:1 threshold ✅ Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §25249510917
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
There was a problem hiding this comment.
Pull request overview
Removes --ignore-scripts from the Crush CLI npm install so post-install hooks can download required native binaries, and adds a verification step to ensure the crush binary is available after install.
Changes:
- Update Crush engine install step generation to allow npm install scripts and add a
crush --versionverification step. - Strengthen Crush engine installation tests to assert script execution is not disabled and the verification step exists.
- Recompile the locked smoke-test workflow to reflect the new install + verify sequence.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/crush_engine.go |
Enables npm post-install scripts for Crush install and appends a crush --version verification step. |
pkg/workflow/crush_engine_test.go |
Updates installation test assertions to ensure --ignore-scripts is absent and the verification step is present. |
.github/workflows/smoke-crush.lock.yml |
Updates compiled workflow output to match the new install command and added verification step. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
| commandName := "crush" | ||
| if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { | ||
| commandName = workflowData.EngineConfig.Command | ||
| } | ||
| versionStep := GitHubActionStep{ | ||
| " - name: Verify Crush CLI installation", | ||
| " run: " + commandName + " --version", |
There was a problem hiding this comment.
Ug! Me agree with other caveman reviewer. Dead code bad. Simplify good!
📰 BREAKING: Report filed by Smoke Copilot · ● 860K
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
|
|
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test Codex 25249581404: FAIL Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "ab.chatgpt.com"
- "chatgpt.com"See Network Configuration for more information.
|
|
Ooga booga! Smoke test caveman was here again! Me run many test. All pass! Fire good, code strong! 🔥🦴
|
|
Smoke test done! ✅ PASS
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Me caveman reviewer. Fix good: remove --ignore-scripts right call. Version step helpful. But dead commandName code need cleanup. Overall: fire strong!
📰 BREAKING: Report filed by Smoke Copilot · ● 860K
| } | ||
| versionStep := GitHubActionStep{ | ||
| " - name: Verify Crush CLI installation", | ||
| " run: " + commandName + " --version", |
There was a problem hiding this comment.
Ug! Me see commandName logic never reach here. Early return block above take over when EngineConfig.Command not empty. Maybe just use hardcoded crush --version?
| "Install Crush CLI", | ||
| "crush", | ||
| workflowData, | ||
| true, // Include Node.js setup |
There was a problem hiding this comment.
Me like version override! Good cave code. But maybe add log line when version overridden? Help debug later.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Summary
Crush, like Claude Code, requires post-install scripts to download native binaries. With
--ignore-scripts, these scripts are skipped and the binary is never downloaded, causing thecrushcommand to fail at runtime.Changes
pkg/workflow/crush_engine.go: ChangedGetInstallationStepsto callGenerateNpmInstallStepsdirectly withrunInstallScripts=true(removing--ignore-scripts), mirroring how the Claude engine handles its install. Also added acrush --versionstep after installation to force any deferred binary downloads.pkg/workflow/crush_engine_test.go: UpdatedTestCrushEngineInstallation/standard_installationto assert:--ignore-scriptsflag in the install commandcrush --versionstep is present.github/workflows/smoke-crush.lock.yml: Recompiled — now showsnpm install -g @charmland/crush@...(no--ignore-scripts) followed bycrush --version