From ed7efea83bae8e870efa1e300e8ad0b7804ebb9a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 10:03:13 +0000 Subject: [PATCH 1/2] fix: remove --ignore-scripts for crush install and add crush --version 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> --- .github/workflows/smoke-crush.lock.yml | 8 ++++++-- pkg/workflow/crush_engine.go | 27 +++++++++++++++++++++++--- pkg/workflow/crush_engine_test.go | 14 +++++++++++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/.github/workflows/smoke-crush.lock.yml b/.github/workflows/smoke-crush.lock.yml index 892a6e6afb5..07b8857103a 100644 --- a/.github/workflows/smoke-crush.lock.yml +++ b/.github/workflows/smoke-crush.lock.yml @@ -445,7 +445,9 @@ jobs: - name: Install AWF binary run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.29 - name: Install Crush CLI - run: npm install --ignore-scripts -g @charmland/crush@0.59.0 + run: npm install -g @charmland/crush@0.59.0 + - name: Verify Crush CLI installation + run: crush --version - name: Determine automatic lockdown mode for GitHub MCP Server id: determine-automatic-lockdown uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 @@ -1347,7 +1349,9 @@ jobs: - name: Install AWF binary run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.29 - name: Install Crush CLI - run: npm install --ignore-scripts -g @charmland/crush@0.59.0 + run: npm install -g @charmland/crush@0.59.0 + - name: Verify Crush CLI installation + run: crush --version - name: Write Crush Config if: always() && steps.detection_guard.outputs.run_detection == 'true' continue-on-error: true diff --git a/pkg/workflow/crush_engine.go b/pkg/workflow/crush_engine.go index c72e94cb762..4491386262a 100644 --- a/pkg/workflow/crush_engine.go +++ b/pkg/workflow/crush_engine.go @@ -59,13 +59,34 @@ func (e *CrushEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA return []GitHubActionStep{} } - npmSteps := BuildStandardNpmEngineInstallSteps( + // Use version from engine config if provided, otherwise default to pinned version + version := string(constants.DefaultCrushVersion) + if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { + version = workflowData.EngineConfig.Version + } + + // Crush requires post-install scripts (native binaries) so --ignore-scripts must + // NOT be passed. This is intentionally different from other engine installs. + npmSteps := GenerateNpmInstallSteps( "@charmland/crush", - string(constants.DefaultCrushVersion), + version, "Install Crush CLI", "crush", - workflowData, + true, // Include Node.js setup + true, // Crush requires post-install scripts for native binaries ) + + // Run crush --version to verify the installation and force any deferred binary downloads + commandName := "crush" + if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { + commandName = workflowData.EngineConfig.Command + } + versionStep := GitHubActionStep{ + " - name: Verify Crush CLI installation", + " run: " + commandName + " --version", + } + npmSteps = append(npmSteps, versionStep) + return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) } diff --git a/pkg/workflow/crush_engine_test.go b/pkg/workflow/crush_engine_test.go index ef931cf2ca4..d677615e1b0 100644 --- a/pkg/workflow/crush_engine_test.go +++ b/pkg/workflow/crush_engine_test.go @@ -161,8 +161,18 @@ func TestCrushEngineInstallation(t *testing.T) { steps := engine.GetInstallationSteps(workflowData) require.NotEmpty(t, steps, "Should generate installation steps") - // Should have at least: Node.js setup + Install Crush - assert.GreaterOrEqual(t, len(steps), 2, "Should have at least 2 installation steps") + // Should have at least: Node.js setup + Install Crush + Verify Crush CLI installation + assert.GreaterOrEqual(t, len(steps), 3, "Should have at least 3 installation steps") + + // Verify --ignore-scripts is NOT present (Crush needs post-install scripts for native binaries) + installStep := strings.Join(steps[1], "\n") + assert.NotContains(t, installStep, "--ignore-scripts", "Should not use --ignore-scripts for Crush (requires post-install scripts for native binaries)") + assert.Contains(t, installStep, "@charmland/crush@", "Should install the crush package") + + // Verify crush --version step is present to force binary download + versionStep := strings.Join(steps[2], "\n") + assert.Contains(t, versionStep, "crush --version", "Should run crush --version to force binary download") + assert.Contains(t, versionStep, "Verify Crush CLI installation", "Should have a descriptive step name") }) t.Run("custom command skips installation", func(t *testing.T) { From 1ebdb81964f3679de9b8d555e35a86922dbb4d25 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 10:04:07 +0000 Subject: [PATCH 2/2] test: use search loops instead of hardcoded indices in crush install 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> --- pkg/workflow/crush_engine_test.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/crush_engine_test.go b/pkg/workflow/crush_engine_test.go index d677615e1b0..dde1eab279e 100644 --- a/pkg/workflow/crush_engine_test.go +++ b/pkg/workflow/crush_engine_test.go @@ -164,14 +164,28 @@ func TestCrushEngineInstallation(t *testing.T) { // Should have at least: Node.js setup + Install Crush + Verify Crush CLI installation assert.GreaterOrEqual(t, len(steps), 3, "Should have at least 3 installation steps") - // Verify --ignore-scripts is NOT present (Crush needs post-install scripts for native binaries) - installStep := strings.Join(steps[1], "\n") + // Find install step and verify --ignore-scripts is NOT present (Crush needs post-install scripts for native binaries) + var installStep string + for _, step := range steps { + content := strings.Join(step, "\n") + if strings.Contains(content, "@charmland/crush@") { + installStep = content + break + } + } + require.NotEmpty(t, installStep, "Should find a step installing @charmland/crush") assert.NotContains(t, installStep, "--ignore-scripts", "Should not use --ignore-scripts for Crush (requires post-install scripts for native binaries)") - assert.Contains(t, installStep, "@charmland/crush@", "Should install the crush package") - // Verify crush --version step is present to force binary download - versionStep := strings.Join(steps[2], "\n") - assert.Contains(t, versionStep, "crush --version", "Should run crush --version to force binary download") + // Find crush --version step to confirm binary download is forced + var versionStep string + for _, step := range steps { + content := strings.Join(step, "\n") + if strings.Contains(content, "crush --version") { + versionStep = content + break + } + } + require.NotEmpty(t, versionStep, "Should find crush --version step to force binary download") assert.Contains(t, versionStep, "Verify Crush CLI installation", "Should have a descriptive step name") })