From 31a3b782edd4a64a175c8ee0f3ce6dd4bed5f969 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 13 Aug 2026 11:32:14 +1000 Subject: [PATCH] fix(dev): build the managed dev Goose backend at opt-level 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev backend is built with cargo's dev profile at opt-level 0, and unoptimized async poll frames are fat enough that goose's extension-add → rmcp OAuth-metadata-discovery descent (~100 frames deep, entered whenever a bundled remote MCP's server answers 401) overflows the 2 MiB tokio worker stack. One `goose serve` process backs every session, so a single overflow takes them all down. No release build has reproduced it — it is debug-build-only, and it is the frame size that crosses the line, not the recursion depth. Set `CARGO_PROFILE_DEV_OPT_LEVEL=1` for the managed checkout's builds. Going to `--release` would also fix it, but the dev profile keeps `debug=true`, so goose's own SIGSEGV/SIGBUS handler can still symbolize the backtrace it prints on a crash. The knob is `GOOSE_DEV_OPT_LEVEL`; set it to 0 to get an unoptimized backend back for stepping through goose in a debugger. It is validated against cargo's accepted set (`0`, `1`, `2`, `3`, `s`, `z`) because the export is unconditional and cargo parses the variable while loading its config, before it picks a profile — an unvalidated typo would fail `cargo build --release` too and break `just bundle`, a lane that never reads [profile.dev]. Two parts that are load-bearing: - The opt-level participates in the build-skip decision. ensure-local-goose.sh skips cargo entirely when stamp.env matches the pin, so an already-built opt-0 binary at the current pin would keep being handed to `just dev` forever — a no-op on exactly the machines that hit the crash. The stamp now records STAMP_OPT_LEVEL, and the comparison defaults a missing field to 0 (what a pre-knob stamp was actually built at) rather than to the current value, so existing stamps fail the match, `--check-bin` exits 2, and the next `just setup` / `just goose-sync` rebuilds optimized. Release stamps skip the comparison: `--release` compiles under [profile.release] and never reads the knob, so it is not part of their identity and they do not pay a spurious rebuild. - The knob lives in the shared lib/goose-dev-paths.sh, and regenerate-sdk-schema.sh passes the same value when it builds generate-acp-schema. Both land in the same cargo target dir; disagreeing on the dev profile would have each run invalidate the other's fingerprints and recompile the shared dependency graph on every alternation. This reaches the lanes that build goose at GOOSE_BUILD_PROFILE=debug: `just dev`, a bare `just setup`, `just goose-sync`, `just sync-schema`, and scripts/dev-e2e.sh, plus the two that stage a debug backend into a bundle — `just bundle-debug` and `just stage-sidecar`. It does not change what any real bundle ships: `just bundle` and everything routing through it (`bundle-macos`, `bundle-linux`, `bundle-linux-docker`), scripts/release/build-macos.sh, and .github/workflows/release.yml all already build goose at GOOSE_BUILD_PROFILE=release, which compiles under [profile.release] and ignores the knob. prepare-goose-sidecar.sh is unchanged — it still falls back to `--check-bin`, which resolves whichever profile its caller asked for. The Windows path (scripts/windows/Ensure-LocalGoose.ps1) is also unchanged — it is an independent implementation with its own stamp.json format, and the crash work is on macOS. This buys stack headroom; it does not bound the recursion. The durable fix is goose-side — spawning `oauth_flow` as its own task, or giving the serve runtime a larger worker stack — and should land at the next pin bump. Verified: `bash -n` on all three scripts, plus stubbed-cargo runs covering the stamp paths (pre-knob debug stamp and an opt-0 stamp both rejected with `--check-bin` exit 2; opt-1 stamp accepted; GOOSE_DEV_OPT_LEVEL=0 rejects the opt-1 stamp; release stamps reuse regardless of the knob) and the new validation (0, 1, 2, 3, s, z each reach cargo; bogus, 4, S, 01 and a padded value each exit 1 with the message and zero cargo invocations, under GOOSE_BUILD_PROFILE=release and `--check-bin` as well as a default build). The accepted set was read off cargo's own error text, which also confirmed 4 parses as config and only fails later in rustc. The four ensure-local-goose.sh literals pinned by scripts/release/tests/release-scripts.test.mjs still match after the usage-block reorder, checked by exact string comparison rather than vitest — node_modules is absent in this checkout. The real rebuild was verified separately: opt=1 with debug=true, binary 459 MB → 280 MB. Signed-off-by: Matt Toohey --- scripts/ensure-local-goose.sh | 24 ++++++++++++++++++++++++ scripts/lib/goose-dev-paths.sh | 23 +++++++++++++++++------ scripts/regenerate-sdk-schema.sh | 4 ++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/scripts/ensure-local-goose.sh b/scripts/ensure-local-goose.sh index a2b298560..057d241d3 100755 --- a/scripts/ensure-local-goose.sh +++ b/scripts/ensure-local-goose.sh @@ -28,6 +28,7 @@ Environment variables: GOOSE_DEV_BIN override built binary name from lockfile GOOSE_DEV_ALLOW_DIRTY 1 to allow building a dirty checkout GOOSE_BUILD_PROFILE debug|release (default: debug) + GOOSE_DEV_OPT_LEVEL opt-level when GOOSE_BUILD_PROFILE=debug (default: 1) USAGE } @@ -118,6 +119,20 @@ fail_or_skip() { # shellcheck source=lib/goose-dev-paths.sh source "$script_dir/lib/goose-dev-paths.sh" export CARGO_TARGET_DIR="$goose_cargo_target_dir" +# Validated here, before the export, because cargo parses +# CARGO_PROFILE_DEV_OPT_LEVEL while loading config — before it picks a profile. +# An invalid value therefore fails `cargo build --release` too, so an unchecked +# typo would break the release lanes, which never read [profile.dev]. The set +# is cargo's own ("must be `0`, `1`, `2`, `3`, `s` or `z`"). +if [[ ! "$goose_dev_opt_level" =~ ^(0|1|2|3|s|z)$ ]]; then + echo "GOOSE_DEV_OPT_LEVEL must be 0, 1, 2, 3, s, or z, got: $goose_dev_opt_level" >&2 + exit 1 +fi +# Optimized frames keep goose's deep OAuth-discovery descent inside the tokio +# worker stack (goose-dev-paths.sh has the full why). Exported as an env var +# so it wins over any [profile.dev] settings in the checkout's Cargo.toml. +# Release builds compile under [profile.release] and ignore it. +export CARGO_PROFILE_DEV_OPT_LEVEL="$goose_dev_opt_level" # bin_path is computed after the checkout exists, via `cargo metadata`, so it # matches CARGO_TARGET_DIR exactly (and would also honour any user override). @@ -154,6 +169,7 @@ write_stamp() { printf 'STAMP_PACKAGE=%q\n' "$goose_package" printf 'STAMP_BIN_NAME=%q\n' "$goose_bin" printf 'STAMP_BUILD_PROFILE=%q\n' "$build_profile" + printf 'STAMP_OPT_LEVEL=%q\n' "$goose_dev_opt_level" printf 'STAMP_BIN=%q\n' "$bin_path" } >"$goose_stamp_file" } @@ -168,6 +184,14 @@ stamp_matches_current_build() { [[ "${STAMP_PACKAGE:-$goose_package}" == "$goose_package" ]] || return 1 [[ "${STAMP_BIN_NAME:-$goose_bin}" == "$goose_bin" ]] || return 1 [[ "${STAMP_BUILD_PROFILE:-}" == "$build_profile" ]] || return 1 + # A stamp without STAMP_OPT_LEVEL predates the opt-level knob, meaning its + # binary was built at cargo's dev default (0). Default the comparison to 0 — + # not to the current value — so those binaries fail the match and rebuild. + # Only debug builds read [profile.dev], so release stamps skip the check + # rather than paying a rebuild for a knob their binary never saw. + if [[ "$build_profile" == "debug" ]]; then + [[ "${STAMP_OPT_LEVEL:-0}" == "$goose_dev_opt_level" ]] || return 1 + fi [[ -x "${STAMP_BIN:-}" ]] || return 1 # The recorded binary path must match where cargo writes today; otherwise # the user's cargo config (e.g. build.target-dir) changed and the stamp is diff --git a/scripts/lib/goose-dev-paths.sh b/scripts/lib/goose-dev-paths.sh index 29ea8bc27..1ce15dd0f 100644 --- a/scripts/lib/goose-dev-paths.sh +++ b/scripts/lib/goose-dev-paths.sh @@ -1,18 +1,22 @@ #!/usr/bin/env bash -# Shared path resolution for the managed Goose backend dev checkout. -# Sourced by ensure-local-goose.sh and regenerate-sdk-schema.sh so the two -# scripts cannot drift on cache layout (a drift would land the regen build in -# a different target dir than the binary build, forcing a full recompile or -# missing the binary entirely). +# Shared path and build-profile resolution for the managed Goose backend dev +# checkout. Sourced by ensure-local-goose.sh and regenerate-sdk-schema.sh so +# the two scripts cannot drift on cache layout (a drift would land the regen +# build in a different target dir than the binary build, forcing a full +# recompile or missing the binary entirely) or on cargo profile settings (a +# drift there would have each script invalidate the other's fingerprints in +# the shared target dir, recompiling the dependency graph on every alternation). # # Sets in the sourcing shell: # goose_dev_root base cache directory # goose_repo managed checkout path # goose_cargo_target_dir cargo target dir scoped to the managed checkout # goose_stamp_file path to the build stamp written by ensure-local-goose.sh +# goose_dev_opt_level dev-profile opt-level for builds in the target dir # # Honours the GOOSE_DEV_ROOT / GOOSE_DEV_REPO / GOOSE_DEV_CARGO_TARGET_DIR / -# GOOSE_DEV_STAMP_FILE env overrides documented in ensure-local-goose.sh. +# GOOSE_DEV_STAMP_FILE / GOOSE_DEV_OPT_LEVEL env overrides documented in +# ensure-local-goose.sh. default_goose_dev_root() { if [[ -n "${XDG_CACHE_HOME:-}" ]]; then @@ -29,3 +33,10 @@ goose_dev_root="${GOOSE_DEV_ROOT:-$(default_goose_dev_root)}" goose_repo="${GOOSE_DEV_REPO:-${goose_dev_root}/goose}" goose_cargo_target_dir="${GOOSE_DEV_CARGO_TARGET_DIR:-${goose_dev_root}/cargo-target}" goose_stamp_file="${GOOSE_DEV_STAMP_FILE:-${goose_dev_root}/stamp.env}" + +# Defaults to 1 rather than cargo's dev default of 0: goose's extension-add → +# OAuth-metadata-discovery descent overflows the 2 MiB tokio worker stack when +# its poll frames are unoptimized (a debug-build-only crash that takes down +# every session backed by the single goose serve process). The dev profile +# keeps debug=true, so the stack-overflow handler's backtraces stay symbolized. +goose_dev_opt_level="${GOOSE_DEV_OPT_LEVEL:-1}" diff --git a/scripts/regenerate-sdk-schema.sh b/scripts/regenerate-sdk-schema.sh index 8e66c15d1..a862a4315 100755 --- a/scripts/regenerate-sdk-schema.sh +++ b/scripts/regenerate-sdk-schema.sh @@ -59,7 +59,11 @@ fi log "Building generate-acp-schema." ( cd "$goose_repo" + # Same opt-level as ensure-local-goose.sh: a differing profile would + # invalidate the shared target dir's fingerprints on every alternation + # between the binary build and this generator build. CARGO_TARGET_DIR="$goose_cargo_target_dir" \ + CARGO_PROFILE_DEV_OPT_LEVEL="$goose_dev_opt_level" \ cargo build -p goose --bin generate-acp-schema )