fix: make fduty resolvable by bare name in BYOC sandboxes (single writable tools dir + hard-fail self-check + bundling)

.gitignore

@@ -30,6 +30,9 @@ nohup.out
 
 dist/
 
+# fduty CLI bundles fetched by scripts/bundle-fduty.sh during a release build
+.fduty-bundle/
+
 # Local agent / assistant notes — not for public repo
 CLAUDE.md
 .claude/

.goreleaser.yaml

@@ -5,6 +5,10 @@ before:
   hooks:
     - go mod tidy
     - go generate ./...
+    # Fetch the matched-os/arch fduty CLI into .fduty-bundle/<os>_<arch>/fduty
+    # so each runner archive ships it (runner stages it from disk at first run
+    # instead of curl|sh'ing the CDN installer). Best-effort per os/arch.
+    - sh scripts/bundle-fduty.sh
 
 builds:
   - env:
@@ -32,6 +36,15 @@ archives:
     format_overrides:
       - goos: windows
         formats: zip
+    # Bundle the matched-os/arch fduty next to the runner binary in the archive
+    # root. A glob (not a literal path) so an os/arch the bundler skipped just
+    # contributes nothing rather than failing the release; the runner falls back
+    # to its CDN install path there. install.sh extracts it into the writable
+    # tools dir at install time.
+    files:
+      - src: ".fduty-bundle/{{ .Os }}_{{ .Arch }}/fduty*"
+        dst: .
+        strip_parent: true
 
 checksum:
   name_template: 'checksums.txt'

cmd/main.go

@@ -203,9 +203,20 @@ func runRunner() error {
 		"max_attempts", cfg.MaxAttempts,
 	)
 
-	// Ensure the `fduty` CLI is in the bundled-tools dir (already on bash PATH);
-	// no-op when the cloud image / install.sh already placed it next to the runner.
-	ensureFdutyCLI()
+	// Pin the runner home into the environment so environment.BundledToolsDir
+	// (which reads FLASHDUTY_RUNNER_HOME) resolves the SAME <home>/bin even when
+	// the home came from the --workspace flag rather than the env. Keeps the
+	// install dir and the bash PATH dir from ever diverging.
+	if err = os.Setenv("FLASHDUTY_RUNNER_HOME", cfg.WorkspaceRoot); err != nil {
+		return fmt.Errorf("failed to pin runner home: %w", err)
+	}
+
+	// Ensure the `fduty` CLI is in the bundled-tools dir AND resolves on the bash
+	// PATH. Hard-fails startup rather than serving a runner that 127s every fduty
+	// call; no-op staging when the cloud image / install.sh already placed it.
+	if err = ensureFdutyCLI(); err != nil {
+		return fmt.Errorf("fduty CLI not ready: %w", err)
+	}
 
 	checker := permission.NewChecker(map[string]string{"*": "allow"})
 

cmd/provision.go

@@ -3,6 +3,8 @@ package main
 import (
 	"bytes"
 	"context"
+	"fmt"
+	"io"
 	"log/slog"
 	"os"
 	"os/exec"
@@ -22,50 +24,128 @@ import (
 // local testing).
 var cliInstallURL = ""
 
-// ensureFdutyCLI makes sure the `fduty` CLI sits in the runner's bundled-tools
-// dir — the directory environment.withBundledToolsPath already prepends to every
-// bash subprocess's PATH. Cloud images and the runner install.sh place fduty
-// next to the runner binary, so this is a no-op there; on local/BYOC hosts that
-// lack it, install it into that same dir from the CDN.
+// ensureFdutyCLI guarantees the `fduty` CLI resolves by bare name through the
+// EXACT environment the bash tool runs with, then HARD-FAILS startup if it does
+// not. A runner that boots "healthy" but 127s every fduty call is worse than one
+// that refuses to start, so any unrecoverable problem here returns an error that
+// aborts serve/run.
 //
-// Installing into the (writable) bundled-tools dir — never /usr/local/bin —
-// keeps install.sh out of its sudo branch, which would otherwise hang a
-// non-interactive `curl | sh` on an unanswerable password prompt. Failures are
-// logged, never fatal: the runner still serves every other tool.
-func ensureFdutyCLI() {
+// Resolution order for getting fduty into the bundled-tools dir (the dir
+// environment.BundledToolsDir prepends to every bash PATH — writable by
+// construction, see its doc comment):
+//  1. already present there (cloud image bakes it; install.sh stages the bundled
+//     copy) → nothing to do;
+//  2. a bundled fduty shipped next to the runner executable → copy it in (no
+//     network);
+//  3. CDN install.sh fallback, when a URL is configured.
+//
+// Regardless of which branch ran — including the no-op "already provisioned"
+// path — the functional self-check below runs and gates startup.
+func ensureFdutyCLI() error {
 	dir := environment.BundledToolsDir()
 	if dir == "" {
-		slog.Warn("fduty CLI provisioning skipped: cannot resolve bundled-tools dir")
-		return
+		return fmt.Errorf("fduty CLI provisioning failed: cannot resolve bundled-tools dir (set FLASHDUTY_RUNNER_HOME or FLASHDUTY_RUNNER_BIN_DIR)")
 	}
 
 	target := filepath.Join(dir, "fduty")
+	if err := provisionFduty(dir, target); err != nil {
+		return err
+	}
+
+	return verifyFdutyOnPath()
+}
+
+// provisionFduty places an fduty binary at target (inside the bundled-tools
+// dir) if one is not already there, trying the bundled-next-to-runner copy
+// before the CDN installer.
+func provisionFduty(dir, target string) error {
 	if fi, err := os.Stat(target); err == nil && !fi.IsDir() {
-		return // our bundled fduty is already in place
+		return nil // already in place (cloud image, install.sh, or a prior run)
+	}
+
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		return fmt.Errorf("fduty CLI provisioning failed: cannot create bundled-tools dir %q: %w", dir, err)
+	}
+
+	// Prefer the fduty that shipped in the release tarball, sitting next to the
+	// runner executable. Copying it in avoids the boot-time network + installer
+	// fragility entirely.
+	if bundled := bundledFdutyNextToExe(); bundled != "" {
+		if err := copyExecutable(bundled, target); err != nil {
+			slog.Warn("failed to stage bundled fduty; falling back to installer", "src", bundled, "error", err)
+		} else {
+			slog.Info("staged bundled fduty CLI", "path", target)
+			return nil
+		}
+	}
+
+	return installFdutyFromCDN(dir, target)
+}
+
+// bundledFdutyNextToExe returns the path to an fduty binary shipped alongside
+// the runner executable, or "" if none is there.
+func bundledFdutyNextToExe() string {
+	exe, err := os.Executable()
+	if err != nil {
+		return ""
+	}
+	cand := filepath.Join(filepath.Dir(exe), "fduty")
+	if fi, err := os.Stat(cand); err == nil && !fi.IsDir() {
+		return cand
+	}
+	return ""
+}
+
+// copyExecutable copies src to dst with 0755 perms, via a temp file + rename so
+// a concurrent reader never sees a half-written binary.
+func copyExecutable(src, dst string) error {
+	in, err := os.Open(src) //nolint:gosec // G304: src is the runner's own bundled binary path
+	if err != nil {
+		return err
+	}
+	defer func() { _ = in.Close() }()
+
+	tmp, err := os.CreateTemp(filepath.Dir(dst), ".fduty-*")
+	if err != nil {
+		return err
 	}
+	tmpName := tmp.Name()
+	defer func() { _ = os.Remove(tmpName) }() // no-op once renamed
 
+	if _, err := io.Copy(tmp, in); err != nil {
+		_ = tmp.Close()
+		return err
+	}
+	if err := tmp.Chmod(0o755); err != nil {
+		_ = tmp.Close()
+		return err
+	}
+	if err := tmp.Close(); err != nil {
+		return err
+	}
+	return os.Rename(tmpName, dst)
+}
+
+// installFdutyFromCDN is the network fallback: it runs flashduty-cli's install.sh
+// targeting the writable bundled-tools dir, so install.sh never relocates to
+// ~/.local/bin (unwritable-target branch) or takes its sudo branch.
+func installFdutyFromCDN(dir, target string) error {
 	installURL := os.Getenv("FLASHDUTY_CLI_INSTALL_URL")
 	if installURL == "" {
 		installURL = cliInstallURL
 	}
 	if installURL == "" {
-		slog.Warn("fduty CLI not found and no install URL configured; agent CLI calls may fail until it is installed",
-			"hint", "set FLASHDUTY_CLI_INSTALL_URL or build with -X main.cliInstallURL=...")
-		return
-	}
-	if err := os.MkdirAll(dir, 0o755); err != nil {
-		slog.Warn("fduty CLI install skipped: cannot create bundled-tools dir", "dir", dir, "error", err)
-		return
+		return fmt.Errorf("fduty CLI not found in %q, no bundled copy next to the runner, and no install URL configured "+
+			"(set FLASHDUTY_CLI_INSTALL_URL or build with -X main.cliInstallURL=...)", dir)
 	}
 
-	slog.Info("installing fduty CLI", "url", installURL, "dir", dir)
+	slog.Info("installing fduty CLI from CDN", "url", installURL, "dir", dir)
 	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
 	defer cancel()
 
-	// Install into the writable bundled-tools dir so install.sh never takes its
-	// sudo branch. The URL is passed through the environment, never interpolated
-	// into the script, so a `$(...)` or backtick in it can't be evaluated by the
-	// shell. INSTALLED_NAME=fduty matches the cloud image / install.sh convention.
+	// The URL is passed through the environment, never interpolated into the
+	// script, so a `$(...)` or backtick in it can't be evaluated by the shell.
+	// INSTALLED_NAME=fduty matches the cloud image / install.sh convention.
 	const script = `set -e; curl -fsSL --connect-timeout 10 --max-time 80 "${_INSTALL_URL}" | sh`
 	cmd := exec.CommandContext(ctx, "sh", "-c", script)
 	cmd.WaitDelay = 10 * time.Second // bound the wait so a stuck child can't block startup past ctx
@@ -78,13 +158,47 @@ func ensureFdutyCLI() {
 	cmd.Stdout = &out
 	cmd.Stderr = &out
 	if err := cmd.Run(); err != nil {
-		slog.Warn("fduty CLI install failed; agent CLI calls may fail", "error", err, "output", out.String())
-		return
+		return fmt.Errorf("fduty CLI install failed: %w; output: %s", err, out.String())
 	}
 
-	if fi, err := os.Stat(target); err == nil && !fi.IsDir() {
-		slog.Info("fduty CLI installed", "path", target)
-	} else {
-		slog.Warn("fduty CLI install ran but binary is not in the bundled-tools dir", "dir", dir, "output", out.String())
+	if fi, err := os.Stat(target); err != nil || fi.IsDir() {
+		return fmt.Errorf("fduty CLI installer ran but no binary landed in %q (install.sh may have relocated it); output: %s", dir, out.String())
+	}
+	slog.Info("fduty CLI installed", "path", target)
+	return nil
+}
+
+// verifyFdutyOnPath runs `fduty version` through the EXACT environment the bash
+// tool uses (environment.BashToolEnv: secrets scrubbed, bundled-tools dir first
+// on PATH) and asserts exit 0. This is the load-bearing gate: it catches the
+// real failure mode — fduty installed somewhere that is not on the bash PATH, so
+// bare `fduty` 127s at agent call time — which a plain os.Stat would miss.
+func verifyFdutyOnPath() error {
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+
+	// Resolve `fduty` through bash exactly like the bash tool does
+	// (environment.executeBashCommand runs `bash -c <cmd>`). We must NOT run
+	// exec.Command("fduty", ...) directly: Go's LookPath resolves the program
+	// against the PARENT process PATH, ignoring cmd.Env — so it would miss the
+	// bundled-tools dir and not mirror the real call-time resolution.
+	cmd := exec.CommandContext(ctx, "bash", "-c", "fduty version")
+	cmd.Env = environment.BashToolEnv()
+	var out bytes.Buffer
+	cmd.Stdout = &out
+	cmd.Stderr = &out
+	if err := cmd.Run(); err != nil {
+		return fmt.Errorf("fduty CLI self-check failed (`fduty version` did not exit 0 on the bash PATH; "+
+			"agent CLI calls would 127): %w; output: %s", err, out.String())
+	}
+	slog.Info("fduty CLI self-check passed", "output_head", head(out.String(), 200))
+	return nil
+}
+
+// head returns at most n bytes of s, for terse log lines.
+func head(s string, n int) string {
+	if len(s) > n {
+		return s[:n]
 	}
+	return s
 }

cmd/provision_test.go

@@ -0,0 +1,118 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// copyExecutable must produce a runnable copy (right bytes, +x) so the bundled
+// fduty staged from next to the runner binary actually executes.
+func TestCopyExecutable(t *testing.T) {
+	dir := t.TempDir()
+	src := filepath.Join(dir, "src")
+	require.NoError(t, os.WriteFile(src, []byte("#!/bin/sh\necho hi\n"), 0o600))
+
+	dst := filepath.Join(dir, "sub", "dst") // parent must already exist for CreateTemp
+	require.NoError(t, os.MkdirAll(filepath.Dir(dst), 0o755))
+
+	require.NoError(t, copyExecutable(src, dst))
+
+	got, err := os.ReadFile(dst)
+	require.NoError(t, err)
+	assert.Equal(t, "#!/bin/sh\necho hi\n", string(got))
+
+	if runtime.GOOS != "windows" {
+		// Unix perm bits are meaningful here; Windows doesn't model an exec bit.
+		fi, err := os.Stat(dst)
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0o100), fi.Mode().Perm()&0o100, "dst must be owner-executable")
+	}
+}
+
+// bundledFdutyNextToExe returns the sibling fduty path only when one exists.
+// We can't relocate the test binary, so assert the no-sibling case is "".
+func TestBundledFdutyNextToExe_AbsentReturnsEmpty(t *testing.T) {
+	exe, err := os.Executable()
+	require.NoError(t, err)
+	if _, statErr := os.Stat(filepath.Join(filepath.Dir(exe), "fduty")); statErr == nil {
+		t.Skip("a real fduty sits next to the test binary; can't assert the absent case")
+	}
+	assert.Equal(t, "", bundledFdutyNextToExe())
+}
+
+// provisionFduty is a no-op when fduty is already in the tools dir — the cloud
+// image / install.sh "already provisioned" soft path.
+func TestProvisionFduty_AlreadyPresentIsNoop(t *testing.T) {
+	dir := t.TempDir()
+	target := filepath.Join(dir, "fduty")
+	require.NoError(t, os.WriteFile(target, []byte("baked"), 0o755))
+
+	require.NoError(t, provisionFduty(dir, target))
+
+	got, err := os.ReadFile(target)
+	require.NoError(t, err)
+	assert.Equal(t, "baked", string(got), "must not overwrite a present fduty")
+}
+
+// provisionFduty must HARD-FAIL (return error) when fduty is absent, no bundled
+// copy sits next to the runner, and no install URL is configured — not silently
+// succeed, which is the whole point of the redesign.
+func TestProvisionFduty_NoSourceErrors(t *testing.T) {
+	if bundledFdutyNextToExe() != "" {
+		t.Skip("a real fduty sits next to the test binary; the bundled branch would succeed")
+	}
+	t.Setenv("FLASHDUTY_CLI_INSTALL_URL", "")
+	saved := cliInstallURL
+	cliInstallURL = ""
+	t.Cleanup(func() { cliInstallURL = saved })
+
+	dir := t.TempDir()
+	target := filepath.Join(dir, "fduty")
+
+	err := provisionFduty(dir, target)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "no install URL configured")
+}
+
+// verifyFdutyOnPath runs `fduty version` through the bash tool env and gates on
+// exit 0. With a fake fduty placed in the tools dir (FLASHDUTY_RUNNER_BIN_DIR),
+// the bundled-tools dir is first on PATH, so bare `fduty` resolves to ours.
+func TestVerifyFdutyOnPath_PassesWithStubOnPath(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("stub uses a POSIX shebang")
+	}
+	binDir := t.TempDir()
+	writeStubFduty(t, binDir, 0)
+	t.Setenv("FLASHDUTY_RUNNER_BIN_DIR", binDir)
+
+	require.NoError(t, verifyFdutyOnPath())
+}
+
+// verifyFdutyOnPath must return an error when the resolved fduty exits non-zero
+// (a broken binary is as bad as a missing one).
+func TestVerifyFdutyOnPath_FailsWhenStubExitsNonZero(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("stub uses a POSIX shebang")
+	}
+	binDir := t.TempDir()
+	writeStubFduty(t, binDir, 3)
+	t.Setenv("FLASHDUTY_RUNNER_BIN_DIR", binDir)
+
+	err := verifyFdutyOnPath()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "self-check failed")
+}
+
+// writeStubFduty drops an executable `fduty` in dir that prints a banner and
+// exits with the given code, so the dir-first-on-PATH stub shadows any real one.
+func writeStubFduty(t *testing.T, dir string, exit int) {
+	t.Helper()
+	script := "#!/bin/sh\necho 'fduty stub version 0.0.0'\nexit " + strconv.Itoa(exit) + "\n"
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "fduty"), []byte(script), 0o755))
+}