fix(mount): treat 429 backpressure as a yield, not a terminal cycle failure

cmd/relayfile-cli/program_name_test.go

@@ -0,0 +1,94 @@
+package main
+
+import (
+	"bytes"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// commandMention matches guidance naming the relayfile binary as a command to
+// run — "relayfile mount", "relayfile supervisor install". It deliberately
+// requires a space, so the systemd unit name (relayfile-listen.service) and the
+// launchd label (com.relayfile.listen) do not match: those are real filenames
+// and must stay literal whoever is invoking us.
+//
+// A bare `relayfile` on its own in a usage block counts too: one such line
+// survived a pass that only looked for `relayfile <verb>`.
+var commandMention = regexp.MustCompile(`\brelayfile(?: [a-z]|\s*$|\s{2,})`)
+
+// Nouns, not instructions. "delegated relayfile credentials" describes what is
+// missing; it tells nobody to run anything.
+var allowedNouns = []string{"relayfile credentials", "relayfile workspace id"}
+
+func withoutAllowedNouns(text string) string {
+	for _, noun := range allowedNouns {
+		text = strings.ReplaceAll(text, noun, "")
+	}
+	return text
+}
+
+// TestUsageNamesTheInvokingProgram pins the contract behind relayfile#509:
+// mounted as `agent-relay file`, nothing may tell the user to run `relayfile`,
+// because that binary is not installed for them.
+//
+// Checked over the usage printers rather than one message, since the leak was
+// never in one place: an earlier fix corrected five call sites found by
+// grepping for "run relayfile", and review found dozens more phrased
+// differently. A pattern is the only thing that catches the next one.
+func TestUsageNamesTheInvokingProgram(t *testing.T) {
+	t.Setenv(programNameEnv, "agent-relay file")
+
+	printers := map[string]func(*bytes.Buffer){
+		"usage":      func(b *bytes.Buffer) { printUsage(b) },
+		"listen":     func(b *bytes.Buffer) { printListenUsage(b) },
+		"supervisor": func(b *bytes.Buffer) { printSupervisorUsage(b) },
+		"workspace":  func(b *bytes.Buffer) { printWorkspaceUsage(b, "") },
+		"integration": func(b *bytes.Buffer) {
+			printIntegrationUsage(b, "")
+		},
+		"ops":       func(b *bytes.Buffer) { printOpsUsage(b, "") },
+		"writeback": func(b *bytes.Buffer) { printWritebackUsage(b, "") },
+		"digest":    func(b *bytes.Buffer) { printDigestUsage(b, "") },
+	}
+
+	for name, print := range printers {
+		t.Run(name, func(t *testing.T) {
+			var out bytes.Buffer
+			print(&out)
+			if found := commandMention.FindString(withoutAllowedNouns(out.String())); found != "" {
+				t.Errorf("%s usage tells a mounted user to run %q; use programName()", name, found)
+			}
+			if !strings.Contains(out.String(), "agent-relay file") {
+				t.Errorf("%s usage never names the invoking program", name)
+			}
+		})
+	}
+}
+
+// TestUsageKeepsServiceFileNames guards the other direction: the systemd unit
+// and launchd label are filenames on disk, identical for every caller, and a
+// blanket rename would have broken them.
+func TestUsageKeepsServiceFileNames(t *testing.T) {
+	t.Setenv(programNameEnv, "agent-relay file")
+	var out bytes.Buffer
+	printSupervisorUsage(&out)
+	for _, literal := range []string{"relayfile-listen.service", "com.relayfile.listen.plist"} {
+		if !strings.Contains(out.String(), literal) {
+			t.Errorf("supervisor usage no longer names %s; that is a real path, not a command", literal)
+		}
+	}
+}
+
+// TestUsageDefaultsToRelayfile keeps direct users seeing the name they typed.
+func TestUsageDefaultsToRelayfile(t *testing.T) {
+	t.Setenv(programNameEnv, "")
+	var out bytes.Buffer
+	printUsage(&out)
+	if !strings.Contains(out.String(), "relayfile ") {
+		t.Error("unmounted usage should name relayfile")
+	}
+	if strings.Contains(out.String(), "agent-relay file") {
+		t.Error("unmounted usage must not name the host")
+	}
+}

cmd/relayfile-mount/backpressure_yield_test.go

@@ -0,0 +1,121 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/agentworkforce/relayfile/internal/mountsync"
+)
+
+// A busy workspace answers 429 workspace_busy with an advertised Retry-After.
+// Classifying that as a cycle FAILURE made the first cycle of an initial
+// bootstrap fatal, so the run died roughly 35s into a 210s budget having synced
+// zero files — surfacing to Cloud as BootstrapFailedError. That accounted for 50
+// of 62 proactive mount-bootstrap failures over three days in production. The
+// retry budget already existed; the cycle only had to yield so the ticker could
+// use it.
+func TestBackpressureIsAYieldNotACycleFailure(t *testing.T) {
+	busy := &mountsync.HTTPError{
+		StatusCode: http.StatusTooManyRequests,
+		Code:       "workspace_busy",
+		Message:    "workspace durable object is busy; retry after the advertised delay",
+	}
+
+	if !isBackpressureError(busy) {
+		t.Fatalf("429 workspace_busy must be recognised as backpressure")
+	}
+	if !cycleYielded(&cycleOutcomeError{cause: busy, yielded: true}) {
+		t.Fatalf("a backpressure cycle must report as yielded so the bootstrap resumes")
+	}
+	// Wrapped the way the syncer actually returns it.
+	if !isBackpressureError(fmt.Errorf("mount sync cycle: %w", busy)) {
+		t.Fatalf("backpressure must be detected through error wrapping")
+	}
+}
+
+// Deliberately narrow. A 5xx is the server being BROKEN, not busy, and a
+// context deadline is handled by its own pre-existing branch. Treating either
+// as backpressure would let a genuinely unhealthy backend look like a queue and
+// spin the bootstrap for its whole budget instead of failing loudly.
+func TestOnlyTooManyRequestsCountsAsBackpressure(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		err  error
+	}{
+		{"500", &mountsync.HTTPError{StatusCode: http.StatusInternalServerError, Message: "boom"}},
+		{"503", &mountsync.HTTPError{StatusCode: http.StatusServiceUnavailable, Message: "down"}},
+		{"403", &mountsync.HTTPError{StatusCode: http.StatusForbidden, Message: "nope"}},
+		{"deadline", context.DeadlineExceeded},
+		{"plain", errors.New("something else")},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if isBackpressureError(tc.err) {
+				t.Fatalf("%v must not be treated as server backpressure", tc.err)
+			}
+		})
+	}
+}
+
+// Devin and Cursor both flagged this independently on PR #511, and they were
+// right: marking a 429 `yielded` is only half the story. finishInitialBootstrap
+// treats a yielded first cycle with nothing in progress as a COMPLETED
+// bootstrap, because the only other thing that yields — a per-cycle deadline —
+// cannot reach that state without a persisted checkpoint. A 429 can: it may
+// arrive before the very first saveState. Falling through would exit 0 and hand
+// Cloud an empty mirror it believes is fully synced, which is worse than the
+// hard failure this PR set out to fix, because it fails silently.
+func TestColdBackpressureIsNotReportedAsACompletedBootstrap(t *testing.T) {
+	busy := &cycleOutcomeError{
+		cause: &mountsync.HTTPError{
+			StatusCode: http.StatusTooManyRequests,
+			Code:       "workspace_busy",
+			Message:    "workspace durable object is busy; retry after the advertised delay",
+		},
+		yielded:      true,
+		backpressure: true,
+	}
+
+	if !cycleYielded(busy) {
+		t.Fatalf("a 429 must still yield, so it is not a terminal cycle failure")
+	}
+	if !cycleBackpressure(busy) {
+		t.Fatalf("a 429 yield must be distinguishable from a deadline yield")
+	}
+
+	// A deadline yield must NOT be mistaken for backpressure: it reaches
+	// finishInitialBootstrap only with a checkpoint on disk, where completing is
+	// the correct outcome.
+	deadline := &cycleOutcomeError{cause: context.DeadlineExceeded, yielded: true}
+	if cycleBackpressure(deadline) {
+		t.Fatalf("a deadline yield must not be treated as server backpressure")
+	}
+	if !cycleYielded(deadline) {
+		t.Fatalf("a deadline yield must still count as yielded")
+	}
+}
+
+// The resumable outcome is what makes the cold case safe: it exits
+// initialBootstrapIncompleteExitCode so the caller reruns us, rather than
+// exit 1 (fatal, the original bug) or exit 0 (silently empty, the regression).
+func TestResumableIncompleteExitsRetryableUnderOnce(t *testing.T) {
+	resumable := newResumableInitialBootstrapIncompleteError(
+		bootstrapResumeState{},
+		"initial cycle yielded to server backpressure before any bootstrap progress",
+		errors.New("http 429 workspace_busy"),
+	)
+
+	if got := mountProcessExitCode(mountConfig{once: true}, resumable); got != initialBootstrapIncompleteExitCode {
+		t.Fatalf("cold backpressure must exit retryable, got %d", got)
+	}
+	terminal := newInitialBootstrapIncompleteError(
+		bootstrapResumeState{},
+		"initial cycle failed",
+		errors.New("http 500"),
+	)
+	if got := mountProcessExitCode(mountConfig{once: true}, terminal); got != 1 {
+		t.Fatalf("a real failure must stay fatal, got %d", got)
+	}
+}

cmd/relayfile-mount/main.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"log"
 	"math/rand"
+	"net/http"
 	"os"
 	"os/signal"
 	"path/filepath"
@@ -697,6 +698,21 @@ func runSinglePollingMount(rootCtx context.Context, cfg mountConfig) error {
 					return nil
 				}
 			}
+			// Server-advertised backpressure is a YIELD, not a cycle
+			// failure. The workspace Durable Object is single-threaded, so a
+			// busy workspace answers 429 workspace_busy with a Retry-After and
+			// means "come back shortly" — the mirror is fine, the server is
+			// saturated. Treating it as terminal made the FIRST cycle of an
+			// initial bootstrap fatal: the run died ~35s into a 210s budget
+			// having synced 0 files, surfacing to Cloud as BootstrapFailedError.
+			// That was 81% of proactive mount-bootstrap failures in production
+			// (50 of 62 over three days). The retry budget is already there;
+			// the cycle just has to let the ticker use it.
+			if isBackpressureError(err) {
+				lastCycleErr = &cycleOutcomeError{cause: err, yielded: true, backpressure: true}
+				log.Printf("mount sync cycle yielded to server backpressure (will retry): %v", err)
+				return nil
+			}
 			lastCycleErr = &cycleOutcomeError{cause: err}
 			log.Printf("mount sync cycle failed: %v", err)
 			return nil
@@ -1043,6 +1059,23 @@ func finishInitialBootstrap(rootCtx context.Context, cfg mountConfig, run func(r
 				if rootErr := rootCtx.Err(); rootErr != nil {
 					return newResumableInitialBootstrapIncompleteError(state, "context cancelled after yielded initial cycle", rootErr)
 				}
+				// A deadline yield reaching here means the bootstrap finished
+				// (that branch only yields once a checkpoint exists), so the
+				// fall-through to success below is right for it. A 429 is NOT
+				// that: it can arrive before the first saveState, leaving
+				// nothing on disk and nothing synced. Falling through would
+				// exit 0 and tell Cloud an EMPTY mirror was bootstrapped —
+				// strictly worse than the hard failure this change set out to
+				// fix, because it fails silently. Report it as resumable
+				// instead, which exits initialBootstrapIncompleteExitCode and
+				// asks the caller to run us again.
+				if cycleBackpressure(err) {
+					return newResumableInitialBootstrapIncompleteError(
+						state,
+						"initial cycle yielded to server backpressure before any bootstrap progress",
+						err,
+					)
+				}
 			} else {
 				return newInitialBootstrapIncompleteError(state, "initial cycle failed", err)
 			}
@@ -1138,6 +1171,14 @@ type bootstrapResumeState struct {
 type cycleOutcomeError struct {
 	cause   error
 	yielded bool
+	// backpressure marks a yield caused by the SERVER asking us to slow down
+	// (HTTP 429) rather than by this process running out of per-cycle time.
+	// The two need different completion handling: a deadline yield only ever
+	// fires once a bootstrap checkpoint exists, so "not in progress" genuinely
+	// means finished, whereas a 429 can arrive before anything at all has been
+	// persisted — and reporting THAT as a completed bootstrap hands Cloud an
+	// empty mirror it believes is fully synced.
+	backpressure bool
 }
 
 func (e *cycleOutcomeError) Error() string {
@@ -1181,6 +1222,22 @@ func newInitialBootstrapIncompleteErrorWithResumable(state bootstrapResumeState,
 	}
 }
 
+// isBackpressureError reports a server telling us to slow down rather than a
+// sync that went wrong. Deliberately narrow: only HTTP 429. A 5xx is the server
+// being broken, not busy, and must stay a real cycle failure so a genuinely
+// unhealthy backend is not mistaken for a queue.
+func isBackpressureError(err error) bool {
+	var httpErr *mountsync.HTTPError
+	return errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusTooManyRequests
+}
+
+// cycleBackpressure reports a yield that came from a server 429 rather than a
+// per-cycle deadline. See the field comment on cycleOutcomeError.
+func cycleBackpressure(err error) bool {
+	var outcome *cycleOutcomeError
+	return errors.As(err, &outcome) && outcome.backpressure
+}
+
 func cycleYielded(err error) bool {
 	var outcome *cycleOutcomeError
 	return errors.As(err, &outcome) && outcome.yielded

packages/sdk/typescript/src/relay-cli/binary-output.test.ts

@@ -126,7 +126,11 @@ describe("stdout is byte-exact", () => {
     const throughSurface = await invoke(realBinDir, argv)
     const direct = spawnSync(
       path.join(realBinDir, process.platform === "win32" ? "relayfile.exe" : "relayfile"),
-      argv
+      argv,
+      // The surface sets this, so the baseline must too: the comparison is
+      // about bytes surviving the stdio path, not about how the binary names
+      // itself, which mounted output deliberately changes (relayfile#509).
+      { env: { ...process.env, RELAYFILE_PROGRAM_NAME: "agent-relay file" } }
     )
 
     expect(throughSurface.code).toBe(direct.status)

packages/sdk/typescript/src/relay-cli/index.ts

@@ -186,6 +186,12 @@ function exitCodeForSignal(signal: NodeJS.Signals): number {
  * @param options - Optional overrides for binary lookup, env, and cwd.
  * @returns A surface satisfying `@agent-relay/cli-surface`'s `RelayCliSurface`.
  */
+/**
+ * How users reach this binary when it is mounted, for messages that instruct
+ * them to run something. Matches the group name the host registers.
+ */
+const MOUNTED_PROGRAM_NAME = "agent-relay file"
+
 export function createRelayCliSurface(
   options: CreateRelayCliSurfaceOptions = {}
 ): RelayCliSurface {
@@ -268,7 +274,12 @@ export function createRelayCliSurface(
         // signal handlers are installed: the host owns them.
         const child = spawn(command, childArgs, {
           cwd,
-          env,
+          // The binary writes messages that tell users to run something. It
+          // has no way to know it was reached through a host, so left alone it
+          // says "run relayfile login", naming a binary someone who installed
+          // `agent-relay` does not have. Telling it how it was invoked keeps
+          // that advice followable; unset, direct users still see `relayfile`.
+          env: { ...env, RELAYFILE_PROGRAM_NAME: MOUNTED_PROGRAM_NAME },
           stdio: ["inherit", "pipe", "pipe"]
         })
 

packages/sdk/typescript/src/relay-cli/mount-routing.test.ts

@@ -43,8 +43,22 @@ interface Capture {
   stderr: string
 }
 
+/**
+ * Env for a direct binary spawn used as the comparison baseline.
+ *
+ * RELAYFILE_PROGRAM_NAME matches what the surface sets, because these tests
+ * assert that argv *routes* identically — not that the binary names itself
+ * identically. Mounted, it deliberately says `agent-relay file` so its advice
+ * points at a binary the user actually has (relayfile#509). Without this the
+ * comparison fails on the one difference the mount is supposed to make.
+ */
 function childEnv(): NodeJS.ProcessEnv {
-  return { ...process.env, HOME: home, USERPROFILE: home }
+  return {
+    ...process.env,
+    HOME: home,
+    USERPROFILE: home,
+    RELAYFILE_PROGRAM_NAME: "agent-relay file"
+  }
 }
 
 async function throughSurface(argv: readonly string[]): Promise<Capture> {