From facc553aad1eaf9e56278ed4b788f63cf4fb1a9c Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sat, 3 Oct 2026 22:35:55 +0000 Subject: [PATCH 1/4] fix(cli): retry transient one-shot reads --- cmd/relayfile-cli/commandspec.go | 13 +- cmd/relayfile-cli/main.go | 59 +++++---- cmd/relayfile-cli/one_shot_retry.go | 104 +++++++++++++++ cmd/relayfile-cli/one_shot_retry_test.go | 161 +++++++++++++++++++++++ docs/cli-design.md | 19 ++- summary.md | 16 +++ 6 files changed, 338 insertions(+), 34 deletions(-) create mode 100644 cmd/relayfile-cli/one_shot_retry.go create mode 100644 cmd/relayfile-cli/one_shot_retry_test.go create mode 100644 summary.md diff --git a/cmd/relayfile-cli/commandspec.go b/cmd/relayfile-cli/commandspec.go index 332c45d6..a44500a9 100644 --- a/cmd/relayfile-cli/commandspec.go +++ b/cmd/relayfile-cli/commandspec.go @@ -680,9 +680,10 @@ func relayfileCommands() []cliCommandSpec { {Flags: "--path ", Description: "remote path to list", DefaultValue: "/"}, {Flags: "--depth ", Description: "tree depth", DefaultValue: 1}, {Flags: "--json", Description: "print the raw JSON response", DefaultValue: false}, + {Flags: "--no-retry", Description: "do not retry transient read failures", DefaultValue: false}, }, dispatch: func(inv cliInvocation) error { - return runTree(inv.args, inv.stdout) + return runTree(inv.args, inv.stdout, inv.stderr) }, }, { @@ -703,9 +704,10 @@ func relayfileCommands() []cliCommandSpec { tokenFlagOption, {Flags: "--output ", Description: "output file path or - for stdout", DefaultValue: "-"}, {Flags: "--json", Description: "print the raw JSON response", DefaultValue: false}, + {Flags: "--no-retry", Description: "do not retry transient read failures", DefaultValue: false}, }, dispatch: func(inv cliInvocation) error { - return runRead(inv.args, inv.stdout) + return runRead(inv.args, inv.stdout, inv.stderr) }, }, { @@ -731,9 +733,10 @@ func relayfileCommands() []cliCommandSpec { tokenFlagOption, {Flags: "--format ", Description: "export format: tar, json, or patch", DefaultValue: "json"}, {Flags: "--output ", Description: "output file path or - for stdout", DefaultValue: "-"}, + {Flags: "--no-retry", Description: "do not retry transient read failures", DefaultValue: false}, }, dispatch: func(inv cliInvocation) error { - return runExport(inv.args, inv.stdout) + return runExport(inv.args, inv.stdout, inv.stderr) }, }, { @@ -741,9 +744,9 @@ func relayfileCommands() []cliCommandSpec { Description: "Show sync status and local mirror state for a workspace", flagSource: "runStatus", Args: []cliArgSpec{workspaceArg}, - Options: []cliOptionSpec{serverFlagOption, tokenFlagOption, jsonFlagOption}, + Options: []cliOptionSpec{serverFlagOption, tokenFlagOption, jsonFlagOption, {Flags: "--no-retry", Description: "do not retry transient read failures", DefaultValue: false}}, dispatch: func(inv cliInvocation) error { - return runStatus(inv.args, inv.stdout) + return runStatus(inv.args, inv.stdout, inv.stderr) }, }, { diff --git a/cmd/relayfile-cli/main.go b/cmd/relayfile-cli/main.go index 2f84f217..4cc84d3a 100644 --- a/cmd/relayfile-cli/main.go +++ b/cmd/relayfile-cli/main.go @@ -9402,7 +9402,7 @@ func plistEscapeXML(s string) string { return s } -func runTree(args []string, stdout io.Writer) error { +func runTree(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("tree", flag.ContinueOnError) fs.SetOutput(io.Discard) server := fs.String("server", "", "relayfile server URL override") @@ -9410,12 +9410,14 @@ func runTree(args []string, stdout io.Writer) error { pathFlag := fs.String("path", "/", "remote path to list") depth := fs.Int("depth", 1, "tree depth") jsonOutput := fs.Bool("json", false, "print the raw JSON response") + noRetry := fs.Bool("no-retry", false, "do not retry transient read failures") if err := fs.Parse(normalizeFlagArgs(args, map[string]bool{ - "server": true, - "token": true, - "path": true, - "depth": true, - "json": false, + "server": true, + "token": true, + "path": true, + "depth": true, + "json": false, + "no-retry": false, })); err != nil { return err } @@ -9452,7 +9454,7 @@ func runTree(args []string, stdout io.Writer) error { query := url.Values{} query.Set("path", remotePath) query.Set("depth", strconv.Itoa(*depth)) - body, _, err := commandClient.getWorkspaceBytes(context.Background(), func(workspaceID string) string { + body, _, err := defaultOneShotGETRetryPolicy(!*noRetry, stderr).getWorkspaceBytes(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/fs/tree?%s", url.PathEscape(workspaceID), query.Encode()) }) if err != nil { @@ -9552,18 +9554,20 @@ func knownRemoteRootSegment(segment string) bool { } } -func runRead(args []string, stdout io.Writer) error { +func runRead(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("read", flag.ContinueOnError) fs.SetOutput(io.Discard) server := fs.String("server", "", "relayfile server URL override") token := fs.String("token", "", "relayfile token override") output := fs.String("output", "-", "output file path or - for stdout") jsonOutput := fs.Bool("json", false, "print the raw JSON response") + noRetry := fs.Bool("no-retry", false, "do not retry transient read failures") if err := fs.Parse(normalizeFlagArgs(args, map[string]bool{ - "server": true, - "token": true, - "output": true, - "json": false, + "server": true, + "token": true, + "output": true, + "json": false, + "no-retry": false, })); err != nil { return err } @@ -9589,7 +9593,7 @@ func runRead(args []string, stdout io.Writer) error { query := url.Values{} query.Set("path", remotePath) - body, _, err := commandClient.getWorkspaceBytes(context.Background(), func(workspaceID string) string { + body, _, err := defaultOneShotGETRetryPolicy(!*noRetry, stderr).getWorkspaceBytes(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/fs/file?%s", url.PathEscape(workspaceID), query.Encode()) }) if err != nil { @@ -9696,18 +9700,20 @@ func runSeed(args []string, stdout io.Writer) error { return nil } -func runExport(args []string, stdout io.Writer) error { +func runExport(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("export", flag.ContinueOnError) fs.SetOutput(io.Discard) server := fs.String("server", "", "relayfile server URL override") token := fs.String("token", "", "relayfile token override") format := fs.String("format", "json", "export format: tar, json, or patch") output := fs.String("output", "-", "output file path or - for stdout") + noRetry := fs.Bool("no-retry", false, "do not retry transient read failures") if err := fs.Parse(normalizeFlagArgs(args, map[string]bool{ - "server": true, - "token": true, - "format": true, - "output": true, + "server": true, + "token": true, + "format": true, + "output": true, + "no-retry": false, })); err != nil { return err } @@ -9724,7 +9730,7 @@ func runExport(args []string, stdout io.Writer) error { return err } exportFormat := url.QueryEscape(strings.ToLower(strings.TrimSpace(*format))) - body, _, err := commandClient.getWorkspaceBytes(context.Background(), func(workspaceID string) string { + body, _, err := defaultOneShotGETRetryPolicy(!*noRetry, stderr).getWorkspaceBytes(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/fs/export?format=%s", url.PathEscape(workspaceID), exportFormat) }) if err != nil { @@ -9743,16 +9749,18 @@ func runExport(args []string, stdout io.Writer) error { return os.WriteFile(*output, body, 0o644) } -func runStatus(args []string, stdout io.Writer) error { +func runStatus(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("status", flag.ContinueOnError) fs.SetOutput(io.Discard) server := fs.String("server", "", "relayfile server URL override") token := fs.String("token", "", "relayfile token override") jsonOutput := fs.Bool("json", false, "emit JSON") + noRetry := fs.Bool("no-retry", false, "do not retry transient read failures") if err := fs.Parse(normalizeFlagArgs(args, map[string]bool{ - "server": true, - "token": true, - "json": false, + "server": true, + "token": true, + "json": false, + "no-retry": false, })); err != nil { return err } @@ -9769,7 +9777,8 @@ func runStatus(args []string, stdout io.Writer) error { return err } var status syncStatusResponse - err = commandClient.getWorkspaceJSON(context.Background(), func(workspaceID string) string { + retryPolicy := defaultOneShotGETRetryPolicy(!*noRetry, stderr) + err = retryPolicy.getWorkspaceJSON(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/sync/status", url.PathEscape(workspaceID)) }, &status) if err != nil { @@ -9781,7 +9790,7 @@ func runStatus(args []string, stdout io.Writer) error { var ingress *syncIngressStatusResponse if statusNeedsIngressDiagnostics(status) { var ingressStatus syncIngressStatusResponse - if err := commandClient.getWorkspaceJSON(context.Background(), func(workspaceID string) string { + if err := retryPolicy.getWorkspaceJSON(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/sync/ingress", url.PathEscape(workspaceID)) }, &ingressStatus); err == nil { ingress = &ingressStatus diff --git a/cmd/relayfile-cli/one_shot_retry.go b/cmd/relayfile-cli/one_shot_retry.go new file mode 100644 index 00000000..8f292766 --- /dev/null +++ b/cmd/relayfile-cli/one_shot_retry.go @@ -0,0 +1,104 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "time" +) + +const ( + oneShotGETMaxAttempts = 3 + oneShotGETMinDelay = time.Second + oneShotGETMaxDelay = 60 * time.Second + oneShotGETJitter = 0.2 +) + +type oneShotGETRetryPolicy struct { + enabled bool + stderr io.Writer + opts politeOpts +} + +func defaultOneShotGETRetryPolicy(enabled bool, stderr io.Writer) oneShotGETRetryPolicy { + return oneShotGETRetryPolicy{ + enabled: enabled, + stderr: stderr, + opts: politeOpts{ + minInterval: oneShotGETMinDelay, + maxInterval: oneShotGETMaxDelay, + jitterFraction: oneShotGETJitter, + hardMaxPerSecond: 1, + }, + } +} + +func (p oneShotGETRetryPolicy) run(ctx context.Context, get func(context.Context) error) error { + if !p.enabled { + return get(ctx) + } + + var lastErr error + attempts := 0 + opts := p.opts + baseSleep := opts.sleep + if baseSleep == nil { + baseSleep = sleepCtx + } + opts.sleep = func(ctx context.Context, delay time.Duration) error { + // politePoll applies jitter after its upper clamp. Keep this command's + // advertised 60-second ceiling absolute. + delay = clampDuration(delay, time.Millisecond, oneShotGETMaxDelay) + if p.stderr != nil { + fmt.Fprintf(p.stderr, "%s, retrying in %s\n", oneShotRetryLabel(lastErr), delay.Round(time.Millisecond)) + } + return baseSleep(ctx, delay) + } + + err := politePoll(ctx, func(ctx context.Context) pollResult { + attempts++ + lastErr = get(ctx) + if lastErr == nil || attempts >= oneShotGETMaxAttempts || !isRetryableOneShotGETError(lastErr) { + return pollResult{done: true} + } + var apiErr *apiError + _ = errors.As(lastErr, &apiErr) + return pollResult{err: lastErr, httpStatus: apiErr.StatusCode, retryAfter: apiErr.RetryAfter} + }, opts) + if err != nil { + return err + } + return lastErr +} + +func isRetryableOneShotGETError(err error) bool { + var apiErr *apiError + return errors.As(err, &apiErr) && (apiErr.StatusCode == http.StatusTooManyRequests || apiErr.StatusCode == http.StatusServiceUnavailable) +} + +func oneShotRetryLabel(err error) string { + var apiErr *apiError + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusTooManyRequests && apiErr.Code == "workspace_busy" { + return "workspace busy" + } + return "service unavailable" +} + +func (p oneShotGETRetryPolicy) getWorkspaceBytes(ctx context.Context, client *workspaceCommandClient, pathForWorkspace func(string) string) ([]byte, string, error) { + var body []byte + var contentType string + err := p.run(ctx, func(ctx context.Context) error { + var err error + body, contentType, err = client.getWorkspaceBytes(ctx, pathForWorkspace) + return err + }) + return body, contentType, err +} + +func (p oneShotGETRetryPolicy) getWorkspaceJSON(ctx context.Context, client *workspaceCommandClient, pathForWorkspace func(string) string, out any) error { + return p.run(ctx, func(ctx context.Context) error { + return client.getWorkspaceJSON(ctx, pathForWorkspace, out) + }) +} diff --git a/cmd/relayfile-cli/one_shot_retry_test.go b/cmd/relayfile-cli/one_shot_retry_test.go new file mode 100644 index 00000000..77655dcd --- /dev/null +++ b/cmd/relayfile-cli/one_shot_retry_test.go @@ -0,0 +1,161 @@ +package main + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +func testOneShotPolicy(clock *fakeClock, stderr *bytes.Buffer) oneShotGETRetryPolicy { + return oneShotGETRetryPolicy{ + enabled: true, + stderr: stderr, + opts: politeOpts{ + minInterval: time.Second, + maxInterval: 60 * time.Second, + jitterFraction: 0, + hardMaxPerSecond: 1, + now: clock.Now, + sleep: clock.Sleep, + }, + } +} + +func TestOneShotGETRetryPolicyRetriesWorkspaceBusyThenSucceeds(t *testing.T) { + clock := &fakeClock{now: time.Unix(0, 0)} + var stderr bytes.Buffer + var attempts int + err := testOneShotPolicy(clock, &stderr).run(context.Background(), func(context.Context) error { + attempts++ + if attempts == 1 { + return &apiError{StatusCode: http.StatusTooManyRequests, Code: "workspace_busy", RetryAfter: 7 * time.Second} + } + return nil + }) + if err != nil { + t.Fatalf("retry returned error: %v", err) + } + if attempts != 2 { + t.Fatalf("attempts = %d, want 2", attempts) + } + if len(clock.sleeps) != 1 || clock.sleeps[0] != 7*time.Second { + t.Fatalf("sleeps = %v, want [7s]", clock.sleeps) + } + if got := stderr.String(); got != "workspace busy, retrying in 7s\n" { + t.Fatalf("stderr = %q", got) + } +} + +func TestOneShotGETRetryPolicyClassificationAndExhaustion(t *testing.T) { + tests := []struct { + name string + err error + wantAttempts int + }{ + {name: "429", err: &apiError{StatusCode: http.StatusTooManyRequests}, wantAttempts: 3}, + {name: "503", err: &apiError{StatusCode: http.StatusServiceUnavailable}, wantAttempts: 3}, + {name: "other 4xx", err: &apiError{StatusCode: http.StatusNotFound}, wantAttempts: 1}, + {name: "transport", err: errors.New("connection reset"), wantAttempts: 1}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + clock := &fakeClock{now: time.Unix(0, 0)} + var attempts int + gotErr := testOneShotPolicy(clock, &bytes.Buffer{}).run(context.Background(), func(context.Context) error { + attempts++ + return tc.err + }) + if gotErr != tc.err { + t.Fatalf("error = %v, want original %v", gotErr, tc.err) + } + if attempts != tc.wantAttempts { + t.Fatalf("attempts = %d, want %d", attempts, tc.wantAttempts) + } + }) + } +} + +func TestOneShotGETRetryPolicyClampsDelayAndCancels(t *testing.T) { + clock := &fakeClock{now: time.Unix(0, 0)} + ctx, cancel := context.WithCancel(context.Background()) + policy := testOneShotPolicy(clock, &bytes.Buffer{}) + var gotDelay time.Duration + policy.opts.sleep = func(_ context.Context, delay time.Duration) error { + gotDelay = delay + cancel() + return context.Canceled + } + err := policy.run(ctx, func(context.Context) error { + return &apiError{StatusCode: http.StatusTooManyRequests, RetryAfter: 24 * time.Hour} + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context canceled", err) + } + if gotDelay != oneShotGETMaxDelay { + t.Fatalf("delay = %s, want clamp %s", gotDelay, oneShotGETMaxDelay) + } +} + +func TestOneShotWorkspaceReadRetriesWithoutMixingOutput(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + w.Header().Set("Retry-After", "2") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"code":"workspace_busy","message":"busy"}`)) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"content":"hello"}`)) + })) + defer server.Close() + + client, err := newAPIClient(server.URL, testJWTWithWorkspace("ws_demo")) + if err != nil { + t.Fatal(err) + } + commandClient := &workspaceCommandClient{workspaceID: "ws_demo", client: client, directToken: true} + clock := &fakeClock{now: time.Unix(0, 0)} + var stderr bytes.Buffer + body, _, err := testOneShotPolicy(clock, &stderr).getWorkspaceBytes(context.Background(), commandClient, func(string) string { return "/file" }) + if err != nil { + t.Fatalf("read failed: %v", err) + } + if got := string(body); got != `{"content":"hello"}` { + t.Fatalf("body = %q", got) + } + if strings.Contains(string(body), "retrying") || !strings.Contains(stderr.String(), "retrying in 2s") { + t.Fatalf("body/stderr mixed: body=%q stderr=%q", body, stderr.String()) + } +} + +func TestReadNoRetryMakesSingleRequest(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"code":"workspace_busy","message":"busy"}`)) + })) + defer server.Close() + + var stdout, stderr bytes.Buffer + err := run([]string{"read", "ws_demo", "/file", "--server", server.URL, "--token", testJWTWithWorkspace("ws_demo"), "--no-retry"}, strings.NewReader(""), &stdout, &stderr) + if err == nil { + t.Fatal("expected read error") + } + if requests.Load() != 1 { + t.Fatalf("requests = %d, want 1", requests.Load()) + } + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } +} diff --git a/docs/cli-design.md b/docs/cli-design.md index 360b8bd5..beb81b6c 100644 --- a/docs/cli-design.md +++ b/docs/cli-design.md @@ -348,7 +348,7 @@ relayfile mount [local-dir] List a remote workspace path without mounting. ``` -relayfile tree [path] [--depth n] [--json] +relayfile tree [path] [--depth n] [--json] [--no-retry] ``` | Argument | Required | Default | Description | @@ -360,12 +360,14 @@ relayfile tree [path] [--depth n] [--json] |------|---------|-------------| | `--depth` | `1` | Maximum tree depth | | `--json` | `false` | Pretty-print the raw API response | +| `--no-retry` | `false` | Fail immediately instead of retrying transient `429`/`503` responses | **Behavior:** 1. Calls `GET /v1/workspaces/{workspaceId}/fs/tree?path=&depth=`. 2. Prints a compact human-readable tree by default. 3. With `--json`, prints the raw response formatted as JSON for scripts. +4. By default, retries transient `429`/`503` responses up to three attempts, honoring `Retry-After` with bounded jitter and reporting waits on stderr. --- @@ -374,7 +376,7 @@ relayfile tree [path] [--depth n] [--json] Read one remote file without mounting. ``` -relayfile read [--output file] [--json] +relayfile read [--output file] [--json] [--no-retry] relayfile cat ``` @@ -387,6 +389,7 @@ relayfile cat |------|---------|-------------| | `--output` | `-` | Output file path or stdout | | `--json` | `false` | Pretty-print the raw API response | +| `--no-retry` | `false` | Fail immediately instead of retrying transient `429`/`503` responses | **Behavior:** @@ -394,6 +397,7 @@ relayfile cat 2. Prints file content to stdout by default. 3. Decodes `encoding: "base64"` content before writing to stdout or `--output`. 4. With `--json`, prints the raw response formatted as JSON. +5. By default, retries transient `429`/`503` responses up to three attempts, honoring `Retry-After`; retry notices go to stderr and never mix with file content. --- @@ -432,7 +436,7 @@ relayfile seed [dir] Download a workspace snapshot. ``` -relayfile export [--format tar|json|patch] [--output file] +relayfile export [--format tar|json|patch] [--output file] [--no-retry] ``` | Flag | Default | Description | @@ -440,6 +444,7 @@ relayfile export [--format tar|json|patch] [--output file] | `--format` | `tar` | Export format | | `--output` | stdout | Output file path (or `-` for stdout) | | `--path` | `/` | Export only files under this path | +| `--no-retry` | `false` | Fail immediately instead of retrying transient `429`/`503` responses | **Behavior:** @@ -447,6 +452,7 @@ relayfile export [--format tar|json|patch] [--output file] 2. `GET /v1/workspaces/{workspaceId}/fs/export?format=&path=`. 3. Stream response to `--output` or stdout. 4. Print to stderr: `[export] downloaded 42 files (1.3 MB) from ws_abc123`. +5. By default, retry transient `429`/`503` responses up to three attempts, honoring `Retry-After`. **Uses the Workspace Export API** defined in `docs/bulk-export-design.md`. @@ -457,17 +463,22 @@ relayfile export [--format tar|json|patch] [--output file] Show workspace sync status including per-provider state. ``` -relayfile status [workspace] [--json] +relayfile status [workspace] [--json] [--no-retry] ``` | Argument | Required | Default | Description | |----------|----------|---------|-------------| | `workspace` | No | default workspace | Workspace name or ID | +| Flag | Default | Description | +|------|---------|-------------| +| `--no-retry` | `false` | Fail immediately instead of retrying transient `429`/`503` responses | + **Behavior:** 1. Resolve workspace (uses default if omitted). 2. Read Cloud `/sync` endpoint and local `.relay/state.json`. + Transient `429`/`503` responses are retried up to three attempts by default, including the optional ingress-diagnostics read. 3. Print one-screen summary: ``` workspace ws_abc (my-project) mode: poll lag: 4s diff --git a/summary.md b/summary.md new file mode 100644 index 00000000..72dd44bf --- /dev/null +++ b/summary.md @@ -0,0 +1,16 @@ +# PR summary + +## What changed + +- Added bounded retries for transient `429` and `503` responses to the one-shot `tree`, `read`, `export`, and `status` GET commands. +- Reused the CLI's existing polite polling implementation for `Retry-After`, exponential fallback, jitter, rate limiting, and context-aware waits, with three attempts and a 60-second maximum delay. +- Added `--no-retry` to each affected command for callers that require immediate failure. +- Routed retry notices to stderr so file, JSON, and export payloads on stdout remain clean. +- Documented the retry behavior and opt-out flag. + +## Tests + +- Added deterministic regression coverage for `429 workspace_busy` recovery, `503`, exhaustion, non-retryable errors, delay clamping, cancellation, stdout/stderr separation, and `read --no-retry` request count. +- `go test ./cmd/relayfile-cli -count=1` +- `scripts/check-contract-surface.sh` +- `git diff --check` From 5673bb8090d2ce05bb973b7c7acd475f5d120f25 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sat, 3 Oct 2026 22:43:17 +0000 Subject: [PATCH 2/4] fix(cli): refresh command spec for retry flags --- .../src/relay-cli/command-spec.json | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/sdk/typescript/src/relay-cli/command-spec.json b/packages/sdk/typescript/src/relay-cli/command-spec.json index 74713484..0367fbb5 100644 --- a/packages/sdk/typescript/src/relay-cli/command-spec.json +++ b/packages/sdk/typescript/src/relay-cli/command-spec.json @@ -1372,6 +1372,11 @@ "flags": "--json", "description": "print the raw JSON response", "defaultValue": false + }, + { + "flags": "--no-retry", + "description": "do not retry transient read failures", + "defaultValue": false } ] }, @@ -1411,6 +1416,11 @@ "flags": "--json", "description": "print the raw JSON response", "defaultValue": false + }, + { + "flags": "--no-retry", + "description": "do not retry transient read failures", + "defaultValue": false } ] }, @@ -1468,6 +1478,11 @@ "flags": "--output ", "description": "output file path or - for stdout", "defaultValue": "-" + }, + { + "flags": "--no-retry", + "description": "do not retry transient read failures", + "defaultValue": false } ] }, @@ -1494,6 +1509,11 @@ "flags": "--json", "description": "emit JSON", "defaultValue": false + }, + { + "flags": "--no-retry", + "description": "do not retry transient read failures", + "defaultValue": false } ] }, From cda02fb88f3c6f8c35d49923315fd77f9f1711d0 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sat, 3 Oct 2026 22:51:04 +0000 Subject: [PATCH 3/4] Keep relayflow working files out of the change --- summary.md | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 summary.md diff --git a/summary.md b/summary.md deleted file mode 100644 index 72dd44bf..00000000 --- a/summary.md +++ /dev/null @@ -1,16 +0,0 @@ -# PR summary - -## What changed - -- Added bounded retries for transient `429` and `503` responses to the one-shot `tree`, `read`, `export`, and `status` GET commands. -- Reused the CLI's existing polite polling implementation for `Retry-After`, exponential fallback, jitter, rate limiting, and context-aware waits, with three attempts and a 60-second maximum delay. -- Added `--no-retry` to each affected command for callers that require immediate failure. -- Routed retry notices to stderr so file, JSON, and export payloads on stdout remain clean. -- Documented the retry behavior and opt-out flag. - -## Tests - -- Added deterministic regression coverage for `429 workspace_busy` recovery, `503`, exhaustion, non-retryable errors, delay clamping, cancellation, stdout/stderr separation, and `read --no-retry` request count. -- `go test ./cmd/relayfile-cli -count=1` -- `scripts/check-contract-surface.sh` -- `git diff --check` From 2b1bd9b4c4203eefc20642bc4872e6dd0714e850 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sat, 3 Oct 2026 23:04:20 +0000 Subject: [PATCH 4/4] fix(cli): honor one-shot retry delays --- cmd/relayfile-cli/commandspec.go | 3 +- cmd/relayfile-cli/main.go | 10 +-- cmd/relayfile-cli/one_shot_retry.go | 59 ++++++++------- cmd/relayfile-cli/one_shot_retry_test.go | 72 +++++++++++++++++++ .../src/relay-cli/command-spec.json | 5 ++ 5 files changed, 118 insertions(+), 31 deletions(-) diff --git a/cmd/relayfile-cli/commandspec.go b/cmd/relayfile-cli/commandspec.go index a44500a9..bddeadb8 100644 --- a/cmd/relayfile-cli/commandspec.go +++ b/cmd/relayfile-cli/commandspec.go @@ -580,9 +580,10 @@ func relayfileCommands() []cliCommandSpec { {Flags: "--reason ", Description: "free-form reason recorded server-side", DefaultValue: "manual"}, serverFlagOption, tokenFlagOption, + {Flags: "--no-retry", Description: "do not retry transient provider-status read failures", DefaultValue: false}, }, dispatch: func(inv cliInvocation) error { - return runPull(inv.args, inv.stdout) + return runPull(inv.args, inv.stdout, inv.stderr) }, }, { diff --git a/cmd/relayfile-cli/main.go b/cmd/relayfile-cli/main.go index 4cc84d3a..ffd7fe1b 100644 --- a/cmd/relayfile-cli/main.go +++ b/cmd/relayfile-cli/main.go @@ -6311,7 +6311,7 @@ func runOpsReplay(args []string, stdin io.Reader, stdout io.Writer) error { return nil } -func runPull(args []string, stdout io.Writer) error { +func runPull(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("pull", flag.ContinueOnError) fs.SetOutput(io.Discard) workspaceName := fs.String("workspace", "", "workspace name or id") @@ -6319,12 +6319,14 @@ func runPull(args []string, stdout io.Writer) error { reason := fs.String("reason", "manual", "free-form reason recorded server-side") server := fs.String("server", "", "relayfile server URL override") tokenOverride := fs.String("token", "", "relayfile token override") + noRetry := fs.Bool("no-retry", false, "do not retry transient provider-status read failures") if err := fs.Parse(normalizeFlagArgs(args, map[string]bool{ "workspace": true, "provider": true, "reason": true, "server": true, "token": true, + "no-retry": false, })); err != nil { return err } @@ -6337,7 +6339,7 @@ func runPull(args []string, stdout io.Writer) error { return err } - providers, err := resolvePullProviders(commandClient, strings.TrimSpace(*provider)) + providers, err := resolvePullProviders(commandClient, strings.TrimSpace(*provider), defaultOneShotGETRetryPolicy(!*noRetry, stderr)) if err != nil { return err } @@ -6370,12 +6372,12 @@ func runPull(args []string, stdout io.Writer) error { return nil } -func resolvePullProviders(commandClient *workspaceCommandClient, requested string) ([]string, error) { +func resolvePullProviders(commandClient *workspaceCommandClient, requested string, retryPolicy oneShotGETRetryPolicy) ([]string, error) { if requested != "" { return []string{normalizeProviderID(requested)}, nil } var status syncStatusResponse - err := commandClient.getWorkspaceJSON(context.Background(), func(workspaceID string) string { + err := retryPolicy.getWorkspaceJSON(context.Background(), commandClient, func(workspaceID string) string { return fmt.Sprintf("/v1/workspaces/%s/sync/status", url.PathEscape(workspaceID)) }, &status) if err != nil { diff --git a/cmd/relayfile-cli/one_shot_retry.go b/cmd/relayfile-cli/one_shot_retry.go index 8f292766..d05dec82 100644 --- a/cmd/relayfile-cli/one_shot_retry.go +++ b/cmd/relayfile-cli/one_shot_retry.go @@ -20,6 +20,7 @@ type oneShotGETRetryPolicy struct { enabled bool stderr io.Writer opts politeOpts + jitter func(time.Duration, float64) time.Duration } func defaultOneShotGETRetryPolicy(enabled bool, stderr io.Writer) oneShotGETRetryPolicy { @@ -40,37 +41,40 @@ func (p oneShotGETRetryPolicy) run(ctx context.Context, get func(context.Context return get(ctx) } - var lastErr error - attempts := 0 - opts := p.opts - baseSleep := opts.sleep - if baseSleep == nil { - baseSleep = sleepCtx + opts := p.opts.withDefaults() + jitter := p.jitter + if jitter == nil { + jitter = applyJitter } - opts.sleep = func(ctx context.Context, delay time.Duration) error { - // politePoll applies jitter after its upper clamp. Keep this command's - // advertised 60-second ceiling absolute. - delay = clampDuration(delay, time.Millisecond, oneShotGETMaxDelay) - if p.stderr != nil { - fmt.Fprintf(p.stderr, "%s, retrying in %s\n", oneShotRetryLabel(lastErr), delay.Round(time.Millisecond)) + for attempt := 1; ; attempt++ { + if err := ctx.Err(); err != nil { + return err } - return baseSleep(ctx, delay) - } - - err := politePoll(ctx, func(ctx context.Context) pollResult { - attempts++ - lastErr = get(ctx) - if lastErr == nil || attempts >= oneShotGETMaxAttempts || !isRetryableOneShotGETError(lastErr) { - return pollResult{done: true} + err := get(ctx) + if err == nil || attempt >= oneShotGETMaxAttempts || !isRetryableOneShotGETError(err) { + return err } + var apiErr *apiError - _ = errors.As(lastErr, &apiErr) - return pollResult{err: lastErr, httpStatus: apiErr.StatusCode, retryAfter: apiErr.RetryAfter} - }, opts) - if err != nil { - return err + _ = errors.As(err, &apiErr) + baseDelay := backoffFor(attempt, opts.minInterval, opts.maxInterval) + if apiErr.RetryAfter > 0 { + baseDelay = clampDuration(apiErr.RetryAfter, opts.minInterval, opts.maxInterval) + } + delay := jitter(baseDelay, opts.jitterFraction) + // Retry-After is a lower bound. Symmetric jitter must never make us + // retry before the server's advertised delay has elapsed. + if apiErr.RetryAfter > 0 && delay < baseDelay { + delay = baseDelay + } + delay = clampDuration(delay, time.Millisecond, opts.maxInterval) + if p.stderr != nil { + fmt.Fprintf(p.stderr, "%s, retrying in %s\n", oneShotRetryLabel(err), delay.Round(time.Millisecond)) + } + if err := opts.sleep(ctx, delay); err != nil { + return err + } } - return lastErr } func isRetryableOneShotGETError(err error) bool { @@ -83,6 +87,9 @@ func oneShotRetryLabel(err error) string { if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusTooManyRequests && apiErr.Code == "workspace_busy" { return "workspace busy" } + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusTooManyRequests { + return "rate limited" + } return "service unavailable" } diff --git a/cmd/relayfile-cli/one_shot_retry_test.go b/cmd/relayfile-cli/one_shot_retry_test.go index 77655dcd..1e319ee5 100644 --- a/cmd/relayfile-cli/one_shot_retry_test.go +++ b/cmd/relayfile-cli/one_shot_retry_test.go @@ -52,6 +52,46 @@ func TestOneShotGETRetryPolicyRetriesWorkspaceBusyThenSucceeds(t *testing.T) { } } +func TestOneShotGETRetryPolicyHonorsRetryAfterFor503(t *testing.T) { + clock := &fakeClock{now: time.Unix(0, 0)} + policy := testOneShotPolicy(clock, &bytes.Buffer{}) + policy.opts.jitterFraction = 0.2 + policy.jitter = func(delay time.Duration, _ float64) time.Duration { return delay - time.Second } + var attempts int + err := policy.run(context.Background(), func(context.Context) error { + attempts++ + if attempts == 1 { + return &apiError{StatusCode: http.StatusServiceUnavailable, RetryAfter: 30 * time.Second} + } + return nil + }) + if err != nil { + t.Fatalf("retry returned error: %v", err) + } + if len(clock.sleeps) != 1 || clock.sleeps[0] != 30*time.Second { + t.Fatalf("sleeps = %v, want [30s]", clock.sleeps) + } +} + +func TestOneShotGETRetryPolicyLabelsGeneric429(t *testing.T) { + clock := &fakeClock{now: time.Unix(0, 0)} + var stderr bytes.Buffer + var attempts int + err := testOneShotPolicy(clock, &stderr).run(context.Background(), func(context.Context) error { + attempts++ + if attempts == 1 { + return &apiError{StatusCode: http.StatusTooManyRequests, Code: "rate_limited"} + } + return nil + }) + if err != nil { + t.Fatalf("retry returned error: %v", err) + } + if got := stderr.String(); got != "rate limited, retrying in 1s\n" { + t.Fatalf("stderr = %q", got) + } +} + func TestOneShotGETRetryPolicyClassificationAndExhaustion(t *testing.T) { tests := []struct { name string @@ -159,3 +199,35 @@ func TestReadNoRetryMakesSingleRequest(t *testing.T) { t.Fatalf("stderr = %q, want empty", stderr.String()) } } + +func TestResolvePullProvidersRetriesStatusRead(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + w.Header().Set("Retry-After", "2") + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"code":"service_unavailable","message":"busy"}`)) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"providers":[{"provider":"slack"}]}`)) + })) + defer server.Close() + + client, err := newAPIClient(server.URL, testJWTWithWorkspace("ws_demo")) + if err != nil { + t.Fatal(err) + } + commandClient := &workspaceCommandClient{workspaceID: "ws_demo", client: client, directToken: true} + clock := &fakeClock{now: time.Unix(0, 0)} + providers, err := resolvePullProviders(commandClient, "", testOneShotPolicy(clock, &bytes.Buffer{})) + if err != nil { + t.Fatalf("resolve providers: %v", err) + } + if len(providers) != 1 || providers[0] != "slack" { + t.Fatalf("providers = %v, want [slack]", providers) + } + if len(clock.sleeps) != 1 || clock.sleeps[0] != 2*time.Second { + t.Fatalf("sleeps = %v, want [2s]", clock.sleeps) + } +} diff --git a/packages/sdk/typescript/src/relay-cli/command-spec.json b/packages/sdk/typescript/src/relay-cli/command-spec.json index 0367fbb5..2ac0b8d6 100644 --- a/packages/sdk/typescript/src/relay-cli/command-spec.json +++ b/packages/sdk/typescript/src/relay-cli/command-spec.json @@ -997,6 +997,11 @@ { "flags": "--token ", "description": "relayfile token override" + }, + { + "flags": "--no-retry", + "description": "do not retry transient provider-status read failures", + "defaultValue": false } ] },