diff --git a/Makefile b/Makefile index bc373dd4f2..1a84b669a7 100644 --- a/Makefile +++ b/Makefile @@ -46,8 +46,8 @@ embed: manual-pack-law # ── the furrow that rides inside ──────────────────────────────────────────── # -# Unix builds carry furrow, so the build fetches it before producing codeaf. -# Windows embeds the native engine through the same artifact door. +# Every release platform carries furrow, including native Windows, so the build +# fetches it before producing codeaf. # This step downloads the release pinned in # internal/furrowbin/pin.json for whatever platform is being built for, checks # it against the sha256 the pin names, keeps it in a gitignored third_party/ @@ -61,9 +61,10 @@ embed: manual-pack-law # build that produced a codeaf without furrow inside it. # # On a machine with no network, point it at an artifact already on disk — the -# explicit artifact is a developer override; the normal fetch checks the pin: +# explicit artifact is a developer override and announces the pinned version it +# replaces; the normal fetch checks the pin: # -# make furrow FURROW_ARTIFACT=~/.agentfield/bin/furrow +# make furrow FURROW_ARTIFACT=/path/to/furrow-linux-amd64 # # THE TARGET IS READ FIRST AND THEN UNSET. A cross-compiling build sets GOOS and # GOARCH for codeaf, and the fetcher has to know them — but it must not be built @@ -456,9 +457,8 @@ check: vet fmt-check test test-packed-manual size build-cross # make app-sidecar APP_GOOS=darwin APP_GOARCH=arm64 # one triple # make app-sidecars # all four # -# Windows has no pinned furrow, so — as release.yml does — a Windows sidecar is -# built without one and codeaf answers ErrNotEmbedded where furrow would be asked. -# It is also built WITHOUT garble: Microsoft Defender quarantined the garbled +# Windows carries its pinned native furrow, just as release.yml does. +# It is built WITHOUT garble: Microsoft Defender quarantined the garbled # binary on first run as "a virus or potentially unwanted software" (2026-10-19). # release.yml's Windows codeaf is a plain go build too. # It is not in app-sidecars' loop: the desktop app ships for macOS and Linux, and @@ -476,7 +476,7 @@ $(GARBLE): mv $(@D)/garble-$(GARBLE_VERSION).d/garble $@ rmdir $(@D)/garble-$(GARBLE_VERSION).d -app-sidecar: embed $(GARBLE) +app-sidecar: embed $(if $(filter windows,$(APP_GOOS)),,$(GARBLE)) @set -eu; \ case "$(APP_GOOS)/$(APP_GOARCH)" in \ darwin/arm64) triple=aarch64-apple-darwin ;; \ @@ -486,12 +486,10 @@ app-sidecar: embed $(GARBLE) windows/amd64) triple=x86_64-pc-windows-msvc ;; \ *) printf 'app-sidecar: no desktop target for %s/%s\n' "$(APP_GOOS)" "$(APP_GOARCH)" >&2; exit 1 ;; \ esac; \ + env -u GOOS -u GOARCH go run ./internal/furrowbin/cmd/fetch -goos=$(APP_GOOS) -goarch=$(APP_GOARCH); \ ext=""; build="$(GARBLE) -literals -tiny build"; \ if [ "$(APP_GOOS)" = windows ]; then \ - rm -f internal/furrowbin/cache/furrow-*.gz; \ ext=".exe"; build="go build"; \ - else \ - env -u GOOS -u GOARCH go run ./internal/furrowbin/cmd/fetch -goos=$(APP_GOOS) -goarch=$(APP_GOARCH); \ fi; \ mkdir -p $(APP_DIST); \ CGO_ENABLED=0 GOOS=$(APP_GOOS) GOARCH=$(APP_GOARCH) $$build \ diff --git a/docs/changes/unreleased/windows-sidecar-furrow.md b/docs/changes/unreleased/windows-sidecar-furrow.md new file mode 100644 index 0000000000..422939e3b8 --- /dev/null +++ b/docs/changes/unreleased/windows-sidecar-furrow.md @@ -0,0 +1,13 @@ +--- +kind: fixed +title: Windows desktop sidecars carry the pinned native furrow engine +pr: 1987 +surface: [engine] +invalidates: + - "The Windows desktop sidecar cleared staged furrow engines and shipped without one even after native Windows artifacts were pinned. It now fetches and verifies its pinned engine before building, and a failed fetch stops the build." + - "Explicit FURROW_ARTIFACT and FURROW_SOURCE overrides silently bypassed the release pin. The fetcher now announces the staged path, target platform and pinned version without preventing deliberate developer overrides." +--- + +Windows sidecars retain plain Go builds because Microsoft Defender quarantined +the garbled executable. They do not need to install garble. Native Windows +extracts the embedded engine as an `.exe` on first use. diff --git a/docs/testing-anywhere.md b/docs/testing-anywhere.md index 2b115fb0cd..b64c6a3815 100644 --- a/docs/testing-anywhere.md +++ b/docs/testing-anywhere.md @@ -4,10 +4,10 @@ This page says how to test every path a chat takes between two computers, from t fastest check to a walk by hand. Run the sections in order. Each one costs more than the one before it and proves more. -Moving chats is off on every build unless `CODEAF_CELLS=1` is set when codeaf starts -on both computers. Add `export CODEAF_CELLS=1` to each shell profile to keep it on; -unset, `0` or any other value leaves it off. It needs a compatible configured or embedded -furrow engine and Unix; Windows users run codeaf in WSL. +Moving chats needs a compatible configured or embedded furrow engine on both +computers. Normal builds, including the desktop sidecar, carry the pinned engine +for Linux, macOS and native Windows. Native Windows uses Git for Windows for +shell commands. The former `CODEAF_CELLS` switch is no longer needed. Settings. Every script that names a machine, a folder or a relay reads it from an environment variable (through `scripts/measure/rigenv.py`) and stops with a message that @@ -120,8 +120,10 @@ For an unreleased engine, use `make build FURROW_SOURCE=/path/to/furrow`. For a cross-build, build furrow for the destination platform in that external checkout first (for Linux, in a Linux container), then pass its binary as `GOOS=linux GOARCH=arm64 make build FURROW_ARTIFACT=/path/to/furrow-linux-arm64`. -The source and artifact roads deliberately override the pin. Runtime checks the actual -exchange verbs and changed-path sealing flags once; an older engine leaves chat movement +The source and artifact roads deliberately override the pin; the fetcher prints the +staged path, target platform and pinned version without verifying the override against +the pin. Runtime checks the actual exchange verbs and changed-path sealing flags once; +an older engine leaves chat movement absent and says `moving chats needs a newer furrow engine than this build carries`. Export the settings above first; each script's docstring says which of them it reads and the diff --git a/internal/furrowbin/cmd/fetch/main.go b/internal/furrowbin/cmd/fetch/main.go index e89531d1f3..2a1ca0f76e 100644 --- a/internal/furrowbin/cmd/fetch/main.go +++ b/internal/furrowbin/cmd/fetch/main.go @@ -5,14 +5,16 @@ // is the step that either gets it or stops. It never half-succeeds. Either the // normal fetched archive matches internal/furrowbin/pin.json, or the command // exits non-zero with the sentence that says what to run. Explicit developer -// artifacts bypass the pin; Windows deliberately clears staged engines. +// artifacts bypass the pin and announce the override and pinned version. // // go run ./internal/furrowbin/cmd/fetch -// go run ./internal/furrowbin/cmd/fetch -from ~/.agentfield/bin/furrow +// go run ./internal/furrowbin/cmd/fetch -from /path/to/furrow-windows-amd64.exe -goos windows -goarch amd64 // go run ./internal/furrowbin/cmd/fetch -built /path/to/furrow/target/release/furrow // // -built stages an explicitly selected artifact or an engine from an external // furrow checkout. These developer overrides do not consult the release pin. +// -omit clears staged engines for a platform without a native furrow release; +// Windows has pinned native artifacts and must use the normal fetch path. // // A build machine with no network is the case worth spelling out: -from takes // the artifact off local disk and checks it against the same pin, so an air @@ -67,6 +69,7 @@ func main() { fmt.Fprintf(os.Stderr, "stage the built furrow: %v\n", err) os.Exit(1) } + fmt.Fprintf(os.Stderr, "furrow: staged developer override %q for %s without pin verification (pinned version %s)\n", *built, platform, furrowbin.Version()) return } if err := fetch(furrowbin.ReadPin(), platform, *cache, *stage, *from); err != nil { @@ -170,7 +173,7 @@ func stageInto(stage, platform string, binary []byte) error { } // clearStaged prevents another platform's engine from riding in this build. -// An empty keep is the deliberate no-native-engine road used by Windows. +// An empty keep is the explicit -omit path for platforms without a native engine. func clearStaged(stage, keep string) error { leftovers, err := filepath.Glob(filepath.Join(stage, "furrow-*.gz")) if err != nil { diff --git a/internal/furrowbin/cmd/fetch/main_test.go b/internal/furrowbin/cmd/fetch/main_test.go index 23ecc3dd6d..b9cb623124 100644 --- a/internal/furrowbin/cmd/fetch/main_test.go +++ b/internal/furrowbin/cmd/fetch/main_test.go @@ -7,7 +7,9 @@ import ( "net/http" "net/http/httptest" "os" + "os/exec" "path/filepath" + "strconv" "strings" "testing" @@ -269,3 +271,40 @@ func TestAMissingBuiltArtifactIsAnError(t *testing.T) { t.Fatal("staging a file that does not exist succeeded") } } + +// TestW3BuiltArtifactAnnouncesOverride checks W3 at the fetch command: an +// unpinned developer artifact is staged, with one notice naming the override +// and the pinned version it replaces. +func TestW3BuiltArtifactAnnouncesOverride(t *testing.T) { + if os.Getenv("CODEAF_TEST_FETCH_MAIN") == "1" { + for i, arg := range os.Args { + if arg == "--" { + os.Args = append(os.Args[:1], os.Args[i+1:]...) + main() + os.Exit(0) + } + } + t.Fatal("fetch helper needs command arguments") + } + + stage := t.TempDir() + artifact := artifactOn(t, "a deliberately unpinned developer engine") + command := exec.Command(os.Args[0], "-test.run=^TestW3BuiltArtifactAnnouncesOverride$", "--", + "-built", artifact, "-stage", stage, "-goos", "windows", "-goarch", "amd64") + command.Env = append(os.Environ(), "CODEAF_TEST_FETCH_MAIN=1") + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("fetch developer override: %v: %s", err, output) + } + for _, phrase := range []string{strconv.Quote(artifact), "windows-amd64", "without pin verification", "pinned version " + furrowbin.Version()} { + if !strings.Contains(string(output), phrase) { + t.Fatalf("W3 override notice missing %q: %q", phrase, output) + } + } + if bytes.Count(output, []byte("\n")) != 1 { + t.Fatalf("W3 needs one override notice, got %q", output) + } + if got := stagedBody(t, stage, "windows-amd64"); got != "a deliberately unpinned developer engine" { + t.Fatalf("W3 override staged %q", got) + } +} diff --git a/internal/furrowbin/cmd/fetch/platform_test.go b/internal/furrowbin/cmd/fetch/platform_test.go index 4ed53c9f4a..881b0098f8 100644 --- a/internal/furrowbin/cmd/fetch/platform_test.go +++ b/internal/furrowbin/cmd/fetch/platform_test.go @@ -2,13 +2,16 @@ package main import ( "os" + "os/exec" "path/filepath" + "runtime" + "strings" "testing" ) -// TestC11WindowsOmitsStaleEngines proves C11's packaging contract after a -// developer has already staged engines for other platforms in this checkout. -func TestC11WindowsOmitsStaleEngines(t *testing.T) { +// TestOmitClearsStaleEngines checks the explicit -omit path for platforms +// without a native engine, retaining the README needed by go:embed. +func TestOmitClearsStaleEngines(t *testing.T) { dir := t.TempDir() for _, name := range []string{"furrow-linux-amd64.gz", "furrow-darwin-arm64.gz", "README.md"} { if err := os.WriteFile(filepath.Join(dir, name), []byte("fixture"), 0o600); err != nil { @@ -23,6 +26,72 @@ func TestC11WindowsOmitsStaleEngines(t *testing.T) { t.Fatal(err) } if len(files) != 1 || files[0].Name() != "README.md" { - t.Fatalf("Windows packaging retained: %v", files) + t.Fatalf("omit retained staged engines: %v", files) + } +} + +// TestW1WindowsSidecarFetchesBeforeBuilding checks W1 at the make target: the +// Windows sidecar fetches its own engine, and a fetch refusal stops the build +// with that message. A fake toolchain keeps this packaging check offline. +func TestW1WindowsSidecarFetchesBeforeBuilding(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the cross-build recipe runs on a Unix build host") + } + makePath, err := exec.LookPath("make") + if err != nil { + t.Skip("the sidecar packaging check needs make") + } + makefile, err := filepath.Abs(filepath.Join("..", "..", "..", "..", "Makefile")) + if err != nil { + t.Fatal(err) + } + for _, failFetch := range []bool{false, true} { + t.Run(map[bool]string{false: "pinned-fetch", true: "fetch-refusal"}[failFetch], func(t *testing.T) { + root := t.TempDir() + log := filepath.Join(root, "calls") + tool := filepath.Join(root, "go") + script := `#!/bin/sh +if [ "$1" = env ]; then + if [ "$2" = GOOS ]; then printf '%s\n' linux; else printf '%s\n' amd64; fi + exit 0 +fi +printf '%s %s/%s\n' "$*" "$GOOS" "$GOARCH" >> "$CODEAF_TEST_SIDECAR_LOG" +if [ "$1" = run ] && [ "$CODEAF_TEST_FETCH_FAIL" = 1 ]; then + printf '%s\n' 'fixture: pinned Windows furrow download failed' >&2 + exit 23 +fi +` + if err := os.WriteFile(tool, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + command := exec.Command(makePath, "--no-print-directory", "-f", makefile, + "-o", "embed", "-o", tool, "app-sidecar", "APP_GOOS=windows", "APP_GOARCH=amd64", + "APP_DIST="+filepath.Join(root, "dist"), "GARBLE="+tool, + "FURROW_GOOS=linux", "FURROW_GOARCH=amd64", "BUILD_REV=fixture", "BUILD_DIRTY=false", "BUILD_AT=fixture") + command.Dir = root + failed := "0" + if failFetch { + failed = "1" + } + command.Env = append(os.Environ(), "PATH="+root+string(os.PathListSeparator)+os.Getenv("PATH"), + "CODEAF_TEST_SIDECAR_LOG="+log, "CODEAF_TEST_FETCH_FAIL="+failed, + "MAKEFLAGS=", "GOOS=", "GOARCH=") + output, buildErr := command.CombinedOutput() + calls, err := os.ReadFile(log) + if err != nil { + t.Fatalf("W1 no toolchain calls: %v: %s", err, output) + } + lines := strings.Split(strings.TrimSpace(string(calls)), "\n") + if !strings.HasPrefix(lines[0], "run ./internal/furrowbin/cmd/fetch -goos=windows -goarch=amd64 ") { + t.Fatalf("W1 must fetch the pinned Windows engine first, got:\n%s", calls) + } + if failFetch { + if buildErr == nil || len(lines) != 1 || !strings.Contains(string(output), "fixture: pinned Windows furrow download failed") { + t.Fatalf("W1 fetch refusal must stop before build: %v; calls %s; output %s", buildErr, calls, output) + } + } else if buildErr != nil || len(lines) != 2 || !strings.HasPrefix(lines[1], "build ") || !strings.HasSuffix(lines[1], " windows/amd64") { + t.Fatalf("W1 must build Windows after fetching: %v; calls %s; output %s", buildErr, calls, output) + } + }) } }