From e4e138bc4f6d6176457dc02673899e8f6561650b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=A4=E5=B7=9D?= Date: Thu, 22 Oct 2026 19:26:20 +0800 Subject: [PATCH] fix(install): fail when requested skills are missing Stop both installers from reporting success when release skills are unusable and local fallback is unavailable. Add isolated regression coverage for the failure branches. Refs #779 --- .changes/779-installer-skills-fail-closed.md | 5 + scripts/install.ps1 | 7 +- scripts/install.sh | 3 +- test/scripts/install_skills_failure_test.go | 136 +++++++++++++++++++ 4 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 .changes/779-installer-skills-fail-closed.md create mode 100644 test/scripts/install_skills_failure_test.go diff --git a/.changes/779-installer-skills-fail-closed.md b/.changes/779-installer-skills-fail-closed.md new file mode 100644 index 0000000000..2926c61fc9 --- /dev/null +++ b/.changes/779-installer-skills-fail-closed.md @@ -0,0 +1,5 @@ +--- +category: Fixed +--- + +- **Installer skill failures** (#779) — fail explicitly without an installation-success banner when a release archive has no usable skills and no local source is available; also fail when the PowerShell local mono skill directory is missing. diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 04375bd408..089a217e0d 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -1362,9 +1362,7 @@ function Install-SkillsLocal { Write-Say "⚠️ multi skill tree not found or empty at $multiSrc; falling back to mono." } if (!(Test-Path $skillSrc)) { - Write-Say "⚠️ Local skills directory not found: $skillSrc" - Write-Say " Skipping skills installation." - return + throw "Local skills directory not found: $skillSrc. Skills were NOT installed." } Write-Say "" @@ -1818,8 +1816,7 @@ function Install-Skills { Install-SkillsLocal -Root $localRoot return } - Write-Say "⚠️ No local source found either. Skipping skills installation." - return + throw "No usable skills found in release asset and no local source checkout found. Skills were NOT installed." } if (!(Install-SkillsToHomes -SkillSrc $skillSrc -Root $HOME)) { throw "mono Skill installation failed" diff --git a/scripts/install.sh b/scripts/install.sh index 0ed178d327..002dd224f2 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2157,8 +2157,7 @@ install_skills() { install_skills_local "$local_root" return else - say "⚠️ No local source checkout found either. Skipping skills installation." - return + err "No usable skills found in release asset and no local source checkout found. Skills were NOT installed." fi fi install_skills_to_homes "$skill_src" diff --git a/test/scripts/install_skills_failure_test.go b/test/scripts/install_skills_failure_test.go new file mode 100644 index 0000000000..028960cd04 --- /dev/null +++ b/test/scripts/install_skills_failure_test.go @@ -0,0 +1,136 @@ +package scripts_test + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// These tests exercise the installer main flow with an authentic, checksummed +// but unusable release archive. Network requests are served by local fixtures. +func TestInstallScriptMissingSkillsFailsWithoutSuccessBanner(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX shell installer") + } + data, err := os.ReadFile(filepath.Join("..", "..", "scripts", "install.sh")) + if err != nil { + t.Fatal(err) + } + for _, mode := range []string{"mono", "multi"} { + t.Run(mode, func(t *testing.T) { + root := t.TempDir() + script := filepath.Join(root, "scripts", "install.sh") + mustWriteFile(t, script, data, 0o755) + release := filepath.Join(root, "release") + files := map[string]string{"README.md": "No usable skills\n"} + if mode == "mono" { + files["multi/dingtalk-chat/SKILL.md"] = "# Multi only\n" + } + writeZip(t, filepath.Join(release, "dws-skills.zip"), files) + writeInstallerFixtureChecksums(t, release) + stubs := filepath.Join(root, "stubs") + writeFakeCurl(t, filepath.Join(stubs, "curl")) + fixture := &installSourceFixture{root: root, stubRoot: stubs, fakeHome: filepath.Join(root, "home")} + cmd := exec.Command("sh", script) + cmd.Env = fixture.envWithSkillMode(mode, + "DWS_VERSION=v0.0.0-test", "DWS_SKILLS_ONLY=1", "DWS_NO_SKILLS=0", + "DWS_NO_FALLBACK=1", "DWS_GITEE_REPO=", "FAKE_ASSET_NAME=unused", + "FAKE_RELEASE_DIR="+release, + ) + output, runErr := cmd.CombinedOutput() + if runErr == nil { + t.Fatalf("unusable skills must fail:\n%s", output) + } + for _, want := range []string{"SHA256 checksum verified", "Skills were NOT installed."} { + if !strings.Contains(string(output), want) { + t.Fatalf("missing %q:\n%s", want, output) + } + } + if strings.Contains(string(output), "Installation complete!") { + t.Fatalf("failed installer printed success:\n%s", output) + } + if _, err := os.Stat(filepath.Join(fixture.fakeHome, ".agents", "skills", "dws")); !os.IsNotExist(err) { + t.Fatalf("failed installer created a Skill directory: %v", err) + } + }) + } +} + +func TestInstallPowerShellMissingSkillsFailsWithoutSuccessBanner(t *testing.T) { + pwsh := lookPowerShellForScripts(t) + data, err := os.ReadFile(filepath.Join("..", "..", "scripts", "install.ps1")) + if err != nil { + t.Fatal(err) + } + cut := strings.LastIndex(string(data), "# ── Main") + if cut < 0 { + t.Fatal("install.ps1 main section not found") + } + // Preserve the actual main flow, redirect HOME references to the fixture, + // and replace source discovery and HTTP transport with deterministic stubs. + prefix := strings.ReplaceAll(string(data[:cut]), "$HOME", "$env:DWS_TEST_HOME") + stubs := ` +function Resolve-SourceRoot { return $null } +function Resolve-Source {} +function Invoke-WebRequest { + [CmdletBinding()] + param([string]$Uri, [string]$OutFile, [switch]$UseBasicParsing) + $name = ([uri]$Uri).Segments[-1] + if ($name -ne 'dws-skills.zip' -and $name -ne 'checksums.txt') { + throw "Unexpected fixture URL: $Uri" + } + Copy-Item -LiteralPath (Join-Path $env:DWS_TEST_RELEASE $name) -Destination $OutFile +} +function Invoke-RestMethod { throw 'Network access is forbidden in this test' } +` + for _, scenario := range []string{"mono", "multi", "local-missing"} { + t.Run(scenario, func(t *testing.T) { + root := t.TempDir() + home := filepath.Join(root, "home") + if err := os.MkdirAll(home, 0o755); err != nil { + t.Fatal(err) + } + release := filepath.Join(root, "release") + files := map[string]string{"README.md": "No usable skills\n"} + if scenario == "mono" { + files["multi/dingtalk-chat/SKILL.md"] = "# Multi only\n" + } + writeZip(t, filepath.Join(release, "dws-skills.zip"), files) + writeInstallerFixtureChecksums(t, release) + mode := scenario + override := "" + if scenario == "local-missing" { + mode = "mono" + override = "\nfunction Install-Skills { Install-SkillsLocal -Root $env:DWS_TEST_SOURCE }\n" + } + harness := filepath.Join(root, "harness.ps1") + // Windows PowerShell 5.1 needs a UTF-8 BOM for the script's emoji. + mustWriteFile(t, harness, []byte("\xef\xbb\xbf"+prefix+stubs+override+string(data[cut:])), 0o644) + cmd := exec.Command(pwsh, "-NoProfile", "-NonInteractive", "-File", harness) + var env []string + for _, kv := range os.Environ() { + if !strings.HasPrefix(kv, "DWS_") { + env = append(env, kv) + } + } + cmd.Env = append(env, "DWS_VERSION=v0.0.0-test", "DWS_SKILLS_ONLY=1", "DWS_SKILL_MODE="+mode, + "DWS_TEST_HOME="+home, "DWS_TEST_RELEASE="+release, "DWS_TEST_SOURCE="+filepath.Join(root, "missing-source")) + output, runErr := cmd.CombinedOutput() + if runErr == nil || !strings.Contains(string(output), "Skills were NOT installed.") { + t.Fatalf("missing skills must fail explicitly: %v\n%s", runErr, output) + } + if scenario != "local-missing" && !strings.Contains(string(output), "SHA256 checksum verified") { + t.Fatalf("fixture did not reach the checked archive path:\n%s", output) + } + if strings.Contains(string(output), "Installation complete!") { + t.Fatalf("failed installer printed success:\n%s", output) + } + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "dws")); !os.IsNotExist(err) { + t.Fatalf("failed installer created a Skill directory: %v", err) + } + }) + } +}