diff --git a/.trajectories/completed/2026-08/traj_1pmb0dufncg0.json b/.trajectories/completed/2026-08/traj_1pmb0dufncg0.json new file mode 100644 index 00000000..46d7636b --- /dev/null +++ b/.trajectories/completed/2026-08/traj_1pmb0dufncg0.json @@ -0,0 +1,57 @@ +{ + "id": "traj_1pmb0dufncg0", + "version": 1, + "task": { + "title": "Replace delegated credential shell-out with direct SDK mint", + "source": { + "system": "plain", + "id": "relayfile-sdk-remint-0815" + } + }, + "status": "completed", + "startedAt": "2026-08-15T21:38:24.295Z", + "completedAt": "2026-08-15T21:42:38.104Z", + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-08-15T21:38:31.621Z" + } + ], + "chapters": [ + { + "id": "chap_adte9hja8loe", + "title": "Work", + "agentName": "default", + "startedAt": "2026-08-15T21:38:31.621Z", + "endedAt": "2026-08-15T21:42:38.104Z", + "events": [ + { + "ts": 1786829911623, + "type": "decision", + "content": "Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable: Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable", + "raw": { + "question": "Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable", + "chosen": "Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable", + "alternatives": [], + "reasoning": "Commit 92c495e already removed the delegated re-mint shell-out. The remaining measurable gap was that the refresh-to-remint regression did not poison AGENT_RELAY_BIN, and delegated-expiry text still prescribed cloud login even when the session was healthy." + }, + "significance": "high" + } + ] + } + ], + "retrospective": { + "summary": "Pinned delegated credential recovery to direct Cloud behavior with broker-shaped AGENT_RELAY_BIN regression arms, corrected misleading delegated-expiry guidance, and verified live mint plus daemon status.", + "approach": "Standard approach", + "confidence": 0.93 + }, + "commits": [], + "filesChanged": [], + "projectId": "AgentWorkforce/relayfile", + "tags": [], + "_trace": { + "startRef": "6af7ca2a8a9b3e7148c37cc60e3756bfe88aba31", + "endRef": "6af7ca2a8a9b3e7148c37cc60e3756bfe88aba31" + } +} \ No newline at end of file diff --git a/.trajectories/completed/2026-08/traj_1pmb0dufncg0.md b/.trajectories/completed/2026-08/traj_1pmb0dufncg0.md new file mode 100644 index 00000000..afe7f5cc --- /dev/null +++ b/.trajectories/completed/2026-08/traj_1pmb0dufncg0.md @@ -0,0 +1,32 @@ +# Trajectory: Replace delegated credential shell-out with direct SDK mint + +> **Status:** ✅ Completed +> **Task:** relayfile-sdk-remint-0815 +> **Confidence:** 93% +> **Started:** August 15, 2026 at 11:38 PM +> **Completed:** August 15, 2026 at 11:42 PM + +--- + +## Summary + +Pinned delegated credential recovery to direct Cloud behavior with broker-shaped AGENT_RELAY_BIN regression arms, corrected misleading delegated-expiry guidance, and verified live mint plus daemon status. + +**Approach:** Standard approach + +--- + +## Key Decisions + +### Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable +- **Chose:** Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable +- **Reasoning:** Commit 92c495e already removed the delegated re-mint shell-out. The remaining measurable gap was that the refresh-to-remint regression did not poison AGENT_RELAY_BIN, and delegated-expiry text still prescribed cloud login even when the session was healthy. + +--- + +## Chapters + +### 1. Work +*Agent: default* + +- Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable: Kept origin/main's direct canonical-session and Cloud API mint path; added boundary-level two-arm regression coverage instead of renaming any binary variable diff --git a/.trajectories/index.json b/.trajectories/index.json index 0c47a1be..e0d0bf45 100644 --- a/.trajectories/index.json +++ b/.trajectories/index.json @@ -1,6 +1,6 @@ { "version": 1, - "lastUpdated": "2026-07-30T12:51:42.275Z", + "lastUpdated": "2026-08-15T21:42:38.397Z", "trajectories": { "traj_4pvrlmqfnzng": { "title": "Review PR #278 in AgentWorkforce/relayfile", @@ -183,6 +183,13 @@ "startedAt": "2026-07-30T12:46:12.964Z", "completedAt": "2026-07-30T12:51:42.114Z", "path": "/Users/will/Projects/AgentWorkforce/relayfile-wt-379-b/.trajectories/completed/2026-07/traj_fksma6kiqy93.json" + }, + "traj_1pmb0dufncg0": { + "title": "Replace delegated credential shell-out with direct SDK mint", + "status": "completed", + "startedAt": "2026-08-15T21:38:24.295Z", + "completedAt": "2026-08-15T21:42:38.104Z", + "path": ".trajectories/completed/2026-08/traj_1pmb0dufncg0.json" } } -} \ No newline at end of file +} diff --git a/cmd/relayfile-cli/cloudauth.go b/cmd/relayfile-cli/cloudauth.go index de4a170f..bccd7009 100644 --- a/cmd/relayfile-cli/cloudauth.go +++ b/cmd/relayfile-cli/cloudauth.go @@ -275,10 +275,23 @@ func refreshAgentRelayStoredAuth(ctx context.Context, auth agentRelayStoredAuth) var payload agentRelayStoredAuth decodeErr := json.NewDecoder(resp.Body).Decode(&payload) if resp.StatusCode != http.StatusOK { - return agentRelayStoredAuth{}, fmt.Errorf( - "the stored Agent Relay cloud login has expired (%s returned HTTP %d); run `agent-relay cloud login`", - endpoint, resp.StatusCode, - ) + refreshErr := fmt.Errorf("refresh the stored Agent Relay cloud login at %s: HTTP %d", endpoint, resp.StatusCode) + switch resp.StatusCode { + case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden: + // ErrCloudRefreshExpired's own text prescribes `agent-relay cloud + // login`, which is unreachable for a session supplied through the + // CLOUD_API_* environment. This function cannot tell whether `auth` + // came from the stored file or from the environment, so name both + // recovery paths rather than misdirect a non-interactive caller to + // an interactive one — the same wording the missing-session branch + // below already uses. + return agentRelayStoredAuth{}, fmt.Errorf( + "%w (the CLOUD_API_* environment is the non-interactive alternative): %v", + ErrCloudRefreshExpired, refreshErr, + ) + default: + return agentRelayStoredAuth{}, refreshErr + } } if decodeErr != nil { return agentRelayStoredAuth{}, fmt.Errorf("parse the refresh response from %s: %w", endpoint, decodeErr) @@ -286,10 +299,7 @@ func refreshAgentRelayStoredAuth(ctx context.Context, auth agentRelayStoredAuth) if strings.TrimSpace(payload.AccessToken) == "" || strings.TrimSpace(payload.RefreshToken) == "" || strings.TrimSpace(payload.AccessTokenExpiresAt) == "" { - return agentRelayStoredAuth{}, fmt.Errorf( - "the refresh response from %s did not include a complete token set; run `agent-relay cloud login`", - endpoint, - ) + return agentRelayStoredAuth{}, fmt.Errorf("the refresh response from %s did not include a complete token set", endpoint) } next := agentRelayStoredAuth{ APIURL: firstNonEmpty(strings.TrimSpace(payload.APIURL), apiURL), @@ -336,16 +346,16 @@ func ensureAgentRelayCloudSession(ctx context.Context) (agentRelayStoredAuth, ag if err != nil { if errors.Is(err, os.ErrNotExist) { return agentRelayStoredAuth{}, agentRelayCloudSessionFromFile, fmt.Errorf( - "no Agent Relay cloud session: %s does not exist. Run `agent-relay cloud login`, or set CLOUD_API_URL, CLOUD_API_ACCESS_TOKEN, CLOUD_API_REFRESH_TOKEN and CLOUD_API_ACCESS_TOKEN_EXPIRES_AT", - path, + "%w: no Agent Relay cloud session; %s does not exist (the CLOUD_API_* environment is the non-interactive alternative)", + ErrCloudRefreshExpired, path, ) } return agentRelayStoredAuth{}, agentRelayCloudSessionFromFile, err } if !auth.valid() { return agentRelayStoredAuth{}, agentRelayCloudSessionFromFile, fmt.Errorf( - "the Agent Relay cloud session at %s is incomplete (needs apiUrl, accessToken, refreshToken and an RFC3339 accessTokenExpiresAt); run `agent-relay cloud login`", - path, + "%w: the Agent Relay cloud session at %s is incomplete (needs apiUrl, accessToken, refreshToken and an RFC3339 accessTokenExpiresAt)", + ErrCloudRefreshExpired, path, ) } if !auth.needsRefresh(time.Now()) { diff --git a/cmd/relayfile-cli/cloudauth_test.go b/cmd/relayfile-cli/cloudauth_test.go index ac2e797a..631cd234 100644 --- a/cmd/relayfile-cli/cloudauth_test.go +++ b/cmd/relayfile-cli/cloudauth_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "os" @@ -88,6 +89,9 @@ func TestCloudCredentialsStillFailWithoutACanonicalSession(t *testing.T) { if err == nil { t.Fatal("expected an error when no cloud session exists") } + if !errors.Is(err, ErrCloudRefreshExpired) { + t.Fatalf("missing canonical session must require human action, got: %v", err) + } if !strings.Contains(err.Error(), "cloud-auth.json") { t.Fatalf("error must name the credential file it looked for, got: %v", err) } @@ -118,6 +122,9 @@ func TestCloudCredentialsRejectIncompleteCanonicalSession(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "incomplete") { t.Fatalf("expected an incomplete-session error, got: %v", err) } + if !errors.Is(err, ErrCloudRefreshExpired) { + t.Fatalf("incomplete canonical session must require human action, got: %v", err) + } } // The CLOUD_API_* environment is the documented non-interactive escape hatch @@ -229,9 +236,51 @@ func TestCloudCredentialsReportExpiredLoginWhenRefreshRejected(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "agent-relay cloud login") { t.Fatalf("expected an expired-login error naming the recovery command, got: %v", err) } + if !errors.Is(err, ErrCloudRefreshExpired) { + t.Fatalf("rejected Cloud refresh must preserve the needs-human sentinel, got: %v", err) + } + reason := degradedStallReasonFor(err) + if !strings.Contains(reason, "requires human action") || !strings.Contains(reason, "agent-relay cloud login") { + t.Fatalf("expired Cloud session must name the required sign-in action, got: %s", reason) + } + if strings.Contains(reason, "relayfile will retry") { + t.Fatalf("expired Cloud session must not claim automatic recovery, got: %s", reason) + } if strings.Contains(err.Error(), minAgentRelayCLIVersion) { t.Fatalf("an expired login must not be reported as a CLI version problem, got: %v", err) } + // A session supplied through CLOUD_API_* cannot run an interactive login, + // and this branch cannot tell which source `auth` came from. Naming only + // the interactive command misdirects non-interactive callers — the exact + // class of defect this PR exists to remove. The missing-session branch + // already names both paths; this one must too. + if !strings.Contains(err.Error(), "CLOUD_API_*") { + t.Fatalf("a rejected refresh must also name the non-interactive CLOUD_API_* alternative, got: %v", err) + } +} + +func TestCloudCredentialsTreatTransientRefreshFailureAsRetryable(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + installBrokerShapedAgentRelayBin(t) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + writeAgentRelayCloudAuthExpiringForTest(t, server.URL, "cld_at_expired", time.Now().Add(-time.Minute)) + + _, err := cloudCredentialsFromAgentRelay() + if err == nil { + t.Fatal("expected transient Cloud refresh failure") + } + if errors.Is(err, ErrCloudRefreshExpired) { + t.Fatalf("HTTP 503 must not be classified as requiring a new sign-in: %v", err) + } + reason := degradedStallReasonFor(err) + if !strings.Contains(reason, "is retryable") || strings.Contains(reason, "requires human action") { + t.Fatalf("HTTP 503 must remain retryable, got: %s", reason) + } } // MUST FIRE. diff --git a/cmd/relayfile-cli/delegated_token_error_test.go b/cmd/relayfile-cli/delegated_token_error_test.go index 7b0968ed..497830e0 100644 --- a/cmd/relayfile-cli/delegated_token_error_test.go +++ b/cmd/relayfile-cli/delegated_token_error_test.go @@ -40,7 +40,7 @@ func TestMapDelegatedTokenCloudError(t *testing.T) { { name: "needs_reauth is permanent", err: &apiError{StatusCode: 401, Code: "needs_reauth", Message: "session expired"}, - wantSentinel: ErrDelegatedRelayfileCredentialsExpired, + wantSentinel: ErrCloudRefreshExpired, wantCredExpiry: true, }, { diff --git a/cmd/relayfile-cli/main.go b/cmd/relayfile-cli/main.go index 0e04cd6a..8dcbdfa0 100644 --- a/cmd/relayfile-cli/main.go +++ b/cmd/relayfile-cli/main.go @@ -1598,13 +1598,13 @@ func loadLegacyCloudCredentials() (cloudCredentials, error) { return creds, nil } -// ErrCloudRefreshExpired indicates the cloud refresh token cannot mint new -// access tokens. The mount loop uses this to enter the read-only degraded -// state described in the productized cloud-mount contract acceptance test -// A9 ("Cloud refresh token expired"). -var ErrCloudRefreshExpired = errors.New("cloud session expired. Run 'agent-relay cloud login' to sign in again.") +// ErrCloudRefreshExpired indicates that the canonical Cloud session is absent, +// incomplete, or can no longer refresh, so automatic delegated-credential +// recovery requires a new sign-in. The historical name is retained because +// callers already use it as the needs-human sentinel for this condition. +var ErrCloudRefreshExpired = errors.New("Agent Relay cloud session requires a new sign-in; run 'agent-relay cloud login'") -var ErrDelegatedRelayfileCredentialsExpired = errors.New("delegated relayfile credentials expired or revoked. Re-bootstrap relayfile credentials with agent-relay cloud login.") +var ErrDelegatedRelayfileCredentialsExpired = errors.New("delegated relayfile credentials expired or revoked; automatic Cloud re-mint did not succeed") // ErrDelegatedScopeInsufficient is returned when the cloud delegated-token // mint rejects the requested scopes. This requires human/admin intervention @@ -1619,6 +1619,68 @@ var ErrDelegatedScopeInsufficient = errors.New("delegated relayfile credentials // rather than drive an endless retry loop. var ErrDelegatedScopeInvalid = errors.New("delegated relayfile credentials requested invalid scopes — scopes must be valid relayfile path scopes; retrying will not succeed, re-mint with corrected scopes") +type degradedStallClass uint8 + +const ( + degradedStallUnknown degradedStallClass = iota + degradedStallRetryable + degradedStallRequiresSignIn + degradedStallRequiresScopeCorrection +) + +// degradedStallClassFor returns the stable recovery class used both for the +// operator-facing message and for degraded-notice throttling. Error text may +// contain a different request ID or retry detail on every attempt; those +// details must not make an unchanged recovery class look like a state change. +func degradedStallClassFor(err error) degradedStallClass { + if err == nil { + return degradedStallUnknown + } + switch { + case errors.Is(err, ErrCloudRefreshExpired): + return degradedStallRequiresSignIn + case errors.Is(err, ErrDelegatedScopeInsufficient), errors.Is(err, ErrDelegatedScopeInvalid): + return degradedStallRequiresScopeCorrection + default: + return degradedStallRetryable + } +} + +// degradedStallReasonFor renders the read-only mount's stall reason from the +// error that caused it, and is deliberately not a single fixed sentence. +// +// Both blanket forms are wrong in opposite directions. "Re-authenticate" +// blamed a healthy Cloud session for what is usually a transient re-mint +// failure, and sent operators to run `agent-relay cloud login` for nothing. A +// blanket "relayfile will retry" is the mirror image: for a fully expired +// Cloud session or scopes Cloud refuses to mint, no amount of retrying can +// restore the credential, so promising self-healing leaves the mount read-only +// indefinitely while `relayfile status` claims it is recovering. Name the +// actual cause, and say only of the recoverable case that it will be retried. +func degradedStallReasonFor(err error) string { + switch degradedStallClassFor(err) { + case degradedStallUnknown: + return "delegated relayfile credentials unusable; cause not recorded" + case degradedStallRequiresSignIn: + return fmt.Sprintf("delegated relayfile credential recovery requires human action: %v — automatic Cloud re-mint cannot recover this without a new sign-in", err) + case degradedStallRequiresScopeCorrection: + return fmt.Sprintf("delegated relayfile credential recovery requires human action: %v — automatic Cloud re-mint cannot recover this; the scopes must be corrected before re-minting", err) + case degradedStallRetryable: + return fmt.Sprintf("delegated relayfile credential recovery is retryable: %v — relayfile will retry", err) + default: + panic("unknown degraded credential recovery class") + } +} + +// degradedStallUpdateFor returns the newly rendered detail together with the +// stable class transition that controls whether the notice throttle resets. +// The rendered reason is always refreshed, but volatile detail alone is not a +// class transition and must not trigger another immediate log line. +func degradedStallUpdateFor(previousClass degradedStallClass, err error) (reason string, nextClass degradedStallClass, resetNotice bool) { + nextClass = degradedStallClassFor(err) + return degradedStallReasonFor(err), nextClass, previousClass != nextClass +} + func isMountCredentialExpired(err error) bool { return errors.Is(err, ErrCloudRefreshExpired) || errors.Is(err, ErrDelegatedRelayfileCredentialsExpired) || @@ -1640,7 +1702,7 @@ func mapDelegatedTokenCloudError(err error) error { } switch ae.Code { case "needs_reauth": - return fmt.Errorf("%w: %s", ErrDelegatedRelayfileCredentialsExpired, ae.Message) + return fmt.Errorf("%w: %s", ErrCloudRefreshExpired, ae.Message) case "scope_insufficient": return fmt.Errorf("%w: %s", ErrDelegatedScopeInsufficient, ae.Message) case "invalid_scope": @@ -10668,9 +10730,9 @@ func refreshDelegatedCredentials(path string, bundle delegatedauth.Bundle, force return nil } if errors.Is(err, delegatedauth.ErrRefreshRejected) { - return fmt.Errorf("%w; cloud re-mint fallback failed: %v", ErrDelegatedRelayfileCredentialsExpired, remintErr) + return fmt.Errorf("%w; cloud re-mint fallback failed: %w", ErrDelegatedRelayfileCredentialsExpired, remintErr) } - return fmt.Errorf("%w; cloud re-mint fallback failed: %v", err, remintErr) + return fmt.Errorf("%w; cloud re-mint fallback failed: %w", err, remintErr) } if !changed { renewed = bundle @@ -13660,29 +13722,53 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, degradedAttempts := 0 var nextDegradedAttempt time.Time var statusMu sync.Mutex - const degradedStallReason = "delegated relayfile credentials expired or revoked — re-bootstrap relayfile credentials with agent-relay cloud login" + degradedStallReason := degradedStallReasonFor(nil) + degradedRecoveryClass := degradedStallClassFor(nil) - enterDegraded := func() { + enterDegraded := func(cause error) { + reason := degradedStallReasonFor(cause) + class := degradedStallClassFor(cause) statusMu.Lock() changed := false if !degraded { degraded = true - stallReason = degradedStallReason + degradedStallReason = reason + degradedRecoveryClass = class + stallReason = reason lastDegradedNotice = time.Time{} nextDegradedAttempt = time.Time{} changed = true } statusMu.Unlock() if changed { - log.Printf("mount entering read-only degraded state: %s", degradedStallReason) + log.Printf("mount entering read-only degraded state: %s", reason) } } + // Refresh the recorded cause while degraded: a stall that began as a + // transient re-mint failure can later become one only a human can clear + // (the Cloud session fully expires, or Cloud starts refusing the scopes), + // and the reported reason has to follow it rather than keep promising a + // retry that can no longer succeed. + updateDegradedCause := func(cause error) { + statusMu.Lock() + if degraded { + reason, class, classChanged := degradedStallUpdateFor(degradedRecoveryClass, cause) + degradedStallReason = reason + degradedRecoveryClass = class + stallReason = reason + if classChanged { + lastDegradedNotice = time.Time{} + } + } + statusMu.Unlock() + } exitDegraded := func() { statusMu.Lock() changed := false if degraded { degraded = false stallReason = "" + degradedRecoveryClass = degradedStallUnknown lastDegradedNotice = time.Time{} degradedAttempts = 0 nextDegradedAttempt = time.Time{} @@ -13704,8 +13790,9 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, return } lastDegradedNotice = time.Now() + reason := degradedStallReason statusMu.Unlock() - log.Printf("mount degraded: %s", degradedStallReason) + log.Printf("mount degraded: %s", reason) } isDegraded := func() bool { statusMu.Lock() @@ -13843,6 +13930,7 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, statusMu.Lock() currentlyDegraded := degraded degradedRetryAt := nextDegradedAttempt + currentDegradedReason := degradedStallReason statusMu.Unlock() if currentlyDegraded { // Exponential backoff: skip recovery attempts until nextDegradedAttempt. @@ -13850,7 +13938,7 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, if !degradedRetryAt.IsZero() && time.Now().Before(degradedRetryAt) { maybePrintRecovery() writeSnapshot() - return errors.New(degradedStallReason) + return errors.New(currentDegradedReason) } // Try to recover by re-running auth refresh. if err := refreshMountAuth(true); err != nil { @@ -13870,6 +13958,7 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, nextDegradedAttempt = time.Now().Add(backoff) statusMu.Unlock() if isMountCredentialExpired(err) { + updateDegradedCause(err) maybePrintRecovery() writeSnapshot() return err @@ -13888,7 +13977,7 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, }) if err != nil { if isMountCredentialExpired(err) { - enterDegraded() + enterDegraded(err) maybePrintRecovery() writeSnapshot() return err @@ -13949,7 +14038,7 @@ func runMountLoopWithAuthLock(rootCtx context.Context, syncer *mountsync.Syncer, return syncer.HandleLocalChange(ctx, relativePath, op) }); err != nil { if isMountCredentialExpired(err) { - enterDegraded() + enterDegraded(err) maybePrintRecovery() writeSnapshot() return diff --git a/cmd/relayfile-cli/main_test.go b/cmd/relayfile-cli/main_test.go index 9056d6ff..eacfef62 100644 --- a/cmd/relayfile-cli/main_test.go +++ b/cmd/relayfile-cli/main_test.go @@ -7450,9 +7450,15 @@ func TestLoadDelegatedCredentialsForRequestIgnoresInsufficientLegacyBundle(t *te } } -func TestRefreshDelegatedCredentialsFallsBackToCloudRemint(t *testing.T) { +// MUST FIRE: a rejected delegated refresh must re-mint through Cloud even when +// AGENT_RELAY_BIN points at relay's Rust broker and PATH contains no Node CLI. +// Before the direct cloud-session path this exact environment made relayfile +// exec the broker's nonexistent `cloud` subcommand instead of reaching the +// delegated-token endpoint. +func TestRefreshDelegatedCredentialsCloudRemintIgnoresBrokerShapedAgentRelayBin(t *testing.T) { t.Setenv("HOME", t.TempDir()) clearRelayfileEnv(t) + installBrokerShapedAgentRelayBin(t) expired := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(-time.Minute)) reminted := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(time.Hour)) @@ -7479,7 +7485,7 @@ func TestRefreshDelegatedCredentialsFallsBackToCloudRemint(t *testing.T) { } })) defer server.Close() - installFakeAgentRelaySession(t, server.URL, "cld_access", "demo", "ws_cloud", "ws_relay") + writeAgentRelayCloudAuthForTest(t, server.URL, "cld_access") path := delegatedCredentialsPathForRequest("ws_cloud", defaultInspectScopes) if err := delegatedauth.SaveAtomic(path, delegatedauth.Bundle{ @@ -7521,6 +7527,149 @@ func TestRefreshDelegatedCredentialsFallsBackToCloudRemint(t *testing.T) { } } +// Manual acceptance arm for a relay-spawned process. Unlike the hermetic +// regression above, this test must inherit the real AGENT_RELAY_BIN unchanged. +// Run with RELAYFILE_LIVE_BROKER_COLLISION=1; the test first proves that the +// inherited binary rejects `cloud`, then proves delegated re-mint still reaches +// Cloud and succeeds. +func TestLiveInheritedBrokerDelegatedRemint(t *testing.T) { + if os.Getenv("RELAYFILE_LIVE_BROKER_COLLISION") != "1" { + t.Skip("set RELAYFILE_LIVE_BROKER_COLLISION=1 inside a relay-spawned process") + } + broker := strings.TrimSpace(os.Getenv("AGENT_RELAY_BIN")) + if broker == "" { + t.Fatal("AGENT_RELAY_BIN must be inherited from the relay broker") + } + output, probeErr := exec.Command(broker, "cloud").CombinedOutput() + if probeErr == nil || !strings.Contains(strings.ToLower(string(output)), "unrecognized subcommand") { + t.Fatalf("AGENT_RELAY_BIN treatment was not administered: %q cloud returned err=%v output=%q", broker, probeErr, output) + } + + t.Setenv("HOME", t.TempDir()) + for _, name := range []string{ + "RELAYFILE_AGENT_RELAY_BIN", + "RELAYFILE_SERVER", + "RELAYFILE_BASE_URL", + "RELAYFILE_TOKEN", + "RELAYFILE_WORKSPACE", + "RELAYFILE_CLOUD_API_URL", + "RELAYFILE_CLOUD_TOKEN", + "RELAYFILE_MOUNT_CREDS_FILE", + "RELAYFILE_DELEGATED_CREDENTIALS_FILE", + "CLOUD_API_URL", + "CLOUD_API_ACCESS_TOKEN", + "CLOUD_API_REFRESH_TOKEN", + "CLOUD_API_ACCESS_TOKEN_EXPIRES_AT", + "CLOUD_API_REFRESH_TOKEN_EXPIRES_AT", + } { + t.Setenv(name, "") + } + + expired := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(-time.Minute)) + reminted := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(time.Hour)) + var sawRemint bool + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path != "/api/v1/workspaces/ws_cloud/relayfile/delegated-token" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + sawRemint = true + writeDelegatedBundleResponse(t, w, server.URL, "ws_relay", reminted, "refresh_new") + })) + defer server.Close() + writeAgentRelayCloudAuthForTest(t, server.URL, "cld_access") + + path := delegatedCredentialsPathForRequest("ws_cloud", defaultInspectScopes) + bundle := delegatedauth.Bundle{ + RelayfileURL: server.URL, + RelayfileWorkspaceID: "ws_relay", + WorkspaceID: "ws_cloud", + AccessToken: expired, + RefreshToken: "refresh_old", + AgentName: "relayfile-cli", + Scopes: append([]string(nil), defaultInspectScopes...), + } + if err := delegatedauth.SaveAtomic(path, bundle); err != nil { + t.Fatalf("save delegated credentials failed: %v", err) + } + renewed, err := refreshDelegatedCredentials(path, bundle, false) + if err != nil { + t.Fatalf("delegated credential re-mint failed with inherited AGENT_RELAY_BIN=%q: %v", broker, err) + } + if !sawRemint || renewed.BearerToken() != reminted { + t.Fatalf("delegated credential re-mint did not return the minted token: sawRemint=%v bundle=%#v", sawRemint, renewed) + } + if got := os.Getenv("AGENT_RELAY_BIN"); got != broker { + t.Fatalf("test changed AGENT_RELAY_BIN: got %q want inherited %q", got, broker) + } + t.Logf("delegated credential re-mint succeeded with inherited AGENT_RELAY_BIN=%s", broker) +} + +// MUST NOT FIRE: the normal delegated-token refresh remains the preferred +// happy path. A usable Cloud session is present so a regression that +// needlessly falls back to re-mint would be observable at the endpoint. +func TestRefreshDelegatedCredentialsDoesNotRemintWhenRefreshSucceeds(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + installBrokerShapedAgentRelayBin(t) + + expired := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(-time.Minute)) + refreshed := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(time.Hour)) + var refreshCalls int + var remintCalls int + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/v1/tokens/refresh": + refreshCalls++ + _ = json.NewEncoder(w).Encode(delegatedauth.TokenPair{ + AccessToken: refreshed, + RefreshToken: "refresh_new", + AccessTokenExpiresAt: time.Now().Add(time.Hour).UTC().Format(time.RFC3339), + RefreshTokenExpiresAt: time.Now().Add(24 * time.Hour).UTC().Format(time.RFC3339), + }) + case "/api/v1/workspaces/ws_cloud/relayfile/delegated-token": + remintCalls++ + writeDelegatedBundleResponse(t, w, server.URL, "ws_relay", "unexpected_remint", "unexpected_refresh") + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + writeAgentRelayCloudAuthForTest(t, server.URL, "cld_access") + + path := delegatedCredentialsPathForRequest("ws_cloud", defaultInspectScopes) + bundle := delegatedauth.Bundle{ + RelayfileURL: server.URL, + RelayauthURL: server.URL, + RelayfileWorkspaceID: "ws_relay", + WorkspaceID: "ws_cloud", + AccessToken: expired, + RefreshToken: "refresh_old", + AgentName: "relayfile-cli", + Scopes: append([]string(nil), defaultInspectScopes...), + } + if err := delegatedauth.SaveAtomic(path, bundle); err != nil { + t.Fatalf("save delegated credentials failed: %v", err) + } + + renewed, err := refreshDelegatedCredentials(path, bundle, false) + if err != nil { + t.Fatalf("refreshDelegatedCredentials failed: %v", err) + } + if refreshCalls != 1 { + t.Fatalf("delegated refresh calls = %d, want 1", refreshCalls) + } + if remintCalls != 0 { + t.Fatalf("cloud re-mint calls = %d, want 0", remintCalls) + } + if renewed.BearerToken() != refreshed || renewed.RotationToken() != "refresh_new" { + t.Fatalf("unexpected renewed bundle: %#v", renewed) + } +} + func TestRefreshDelegatedCredentialsSurfacesRemintFailure(t *testing.T) { t.Setenv("HOME", t.TempDir()) clearRelayfileEnv(t) @@ -7560,9 +7709,235 @@ func TestRefreshDelegatedCredentialsSurfacesRemintFailure(t *testing.T) { if !errors.Is(err, ErrDelegatedRelayfileCredentialsExpired) { t.Fatalf("expected delegated expiry sentinel, got %v", err) } + if !errors.Is(err, ErrCloudRefreshExpired) { + t.Fatalf("missing Cloud session must preserve the needs-human sentinel, got %v", err) + } if !strings.Contains(err.Error(), "no Agent Relay cloud session") || !strings.Contains(err.Error(), "cloud-auth.json") { t.Fatalf("expected remint failure detail naming the missing cloud session, got %v", err) } + reason := degradedStallReasonFor(err) + for _, want := range []string{"requires human action", "agent-relay cloud login"} { + if !strings.Contains(reason, want) { + t.Fatalf("missing-session stall reason %q does not contain %q", reason, want) + } + } + if strings.Contains(reason, "relayfile will retry") { + t.Fatalf("missing-session stall reason must not claim automatic recovery: %q", reason) + } +} + +// The degraded mount's stall reason has to track the actual cause. Two blanket +// messages are each wrong in one direction: "re-authenticate" blames a healthy +// Cloud session for a transient re-mint failure, and "relayfile will retry" +// promises self-healing for a fully expired session or refused scopes, where +// retrying can never succeed and the mount would sit read-only indefinitely. +func TestDegradedStallReasonNamesCauseAndActionability(t *testing.T) { + transient := fmt.Errorf("%w; cloud re-mint fallback failed: %v", + ErrDelegatedRelayfileCredentialsExpired, + errors.New("mint delegated relayfile credentials: relayauth_unavailable")) + + for _, tc := range []struct { + name string + err error + // want appears in the reason; notWant must not. + want []string + notWant []string + }{ + { + name: "transient remint failure promises a retry", + err: transient, + want: []string{"is retryable", "relayauth_unavailable", "relayfile will retry"}, + notWant: []string{"requires human action", "cannot recover"}, + }, + { + name: "expired cloud session needs a human, not a retry", + err: fmt.Errorf("refresh delegated credentials: %w", ErrCloudRefreshExpired), + want: []string{"requires human action", "cannot recover", "sign-in"}, + notWant: []string{"relayfile will retry"}, + }, + { + name: "insufficient scope needs a human, not a retry", + err: fmt.Errorf("%w: workspace denies fs:write", ErrDelegatedScopeInsufficient), + want: []string{"requires human action", "cannot recover", "scopes must be corrected"}, + notWant: []string{"relayfile will retry"}, + }, + { + name: "invalid scope needs a human, not a retry", + err: fmt.Errorf("%w: malformed", ErrDelegatedScopeInvalid), + want: []string{"requires human action", "cannot recover", "scopes must be corrected"}, + notWant: []string{"relayfile will retry"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + reason := degradedStallReasonFor(tc.err) + for _, want := range tc.want { + if !strings.Contains(reason, want) { + t.Fatalf("stall reason %q does not contain %q", reason, want) + } + } + for _, notWant := range tc.notWant { + if strings.Contains(reason, notWant) { + t.Fatalf("stall reason %q must not contain %q", reason, notWant) + } + } + // Whatever the class, the reason must never send an operator to + // re-authenticate a session that is not the failure. + if strings.Contains(reason, "Re-bootstrap relayfile credentials") { + t.Fatalf("stall reason %q reinstates blanket re-bootstrap advice", reason) + } + }) + } +} + +func TestDegradedStallClassIgnoresVolatileErrorDetail(t *testing.T) { + firstErr := fmt.Errorf("%w; cloud re-mint fallback failed: request_id=req-1 retry=1", + ErrDelegatedRelayfileCredentialsExpired) + secondErr := fmt.Errorf("%w; cloud re-mint fallback failed: request_id=req-2 retry=2", + ErrDelegatedRelayfileCredentialsExpired) + + firstReason, firstClass, _ := degradedStallUpdateFor(degradedStallUnknown, firstErr) + secondReason, secondClass, resetNotice := degradedStallUpdateFor(firstClass, secondErr) + if firstReason == secondReason { + t.Fatal("volatile-detail treatment was not administered: rendered reasons must differ") + } + if firstClass != degradedStallRetryable || secondClass != degradedStallRetryable { + t.Fatalf("transient failures classified as %v and %v, want retryable", firstClass, secondClass) + } + if firstClass != secondClass { + t.Fatalf("volatile details changed recovery class from %v to %v", firstClass, secondClass) + } + if resetNotice { + t.Fatal("volatile detail within the retryable class reset the degraded-notice throttle") + } + _, humanClass, resetNotice := degradedStallUpdateFor(secondClass, fmt.Errorf("%w: denied", ErrDelegatedScopeInsufficient)) + if humanClass == firstClass { + t.Fatalf("scope refusal class %v must differ from retryable class %v", humanClass, firstClass) + } + if !resetNotice { + t.Fatal("retryable-to-needs-human class transition did not reset the degraded-notice throttle") + } +} + +func TestRefreshDelegatedCredentialsReportsActualCloudRemintFailure(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + installBrokerShapedAgentRelayBin(t) + + expired := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(-time.Minute)) + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/v1/tokens/refresh": + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"code":"delegation_expired"}`)) + case "/api/v1/workspaces/ws_cloud/relayfile/delegated-token": + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"code":"relayauth_unavailable","message":"delegated credential service unavailable"}`)) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + writeAgentRelayCloudAuthForTest(t, server.URL, "cld_access") + + path := delegatedCredentialsPathForRequest("ws_cloud", defaultInspectScopes) + bundle := delegatedauth.Bundle{ + RelayfileURL: server.URL, + RelayauthURL: server.URL, + RelayfileWorkspaceID: "ws_relay", + WorkspaceID: "ws_cloud", + AccessToken: expired, + RefreshToken: "refresh_old", + AgentName: "relayfile-cli", + Scopes: append([]string(nil), defaultInspectScopes...), + } + if err := delegatedauth.SaveAtomic(path, bundle); err != nil { + t.Fatalf("save delegated credentials failed: %v", err) + } + + _, err := refreshDelegatedCredentials(path, bundle, false) + if err == nil { + t.Fatal("expected refresh failure") + } + for _, want := range []string{"cloud re-mint fallback failed", "http 503", "relayauth_unavailable"} { + if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(want)) { + t.Fatalf("error must name actual re-mint failure %q, got: %v", want, err) + } + } + for _, harmful := range []string{"missing fs:write", "re-authenticate", "agent-relay cloud login"} { + if strings.Contains(strings.ToLower(err.Error()), strings.ToLower(harmful)) { + t.Fatalf("healthy cloud session failure must not recommend %q, got: %v", harmful, err) + } + } + reason := degradedStallReasonFor(err) + if !strings.Contains(reason, "is retryable") || !strings.Contains(reason, "relayfile will retry") { + t.Fatalf("transient re-mint failure must be reported as retryable, got: %s", reason) + } + if strings.Contains(reason, "requires human action") { + t.Fatalf("transient re-mint failure must not require human action, got: %s", reason) + } + t.Logf("RETRYABLE: %s", reason) +} + +func TestRefreshDelegatedCredentialsPreservesScopeRefusalForClassification(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + installBrokerShapedAgentRelayBin(t) + + expired := testJWTWithWorkspaceAgentAndExpiry("ws_relay", "relayfile-cli", time.Now().Add(-time.Minute)) + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/v1/tokens/refresh": + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"code":"delegation_expired"}`)) + case "/api/v1/workspaces/ws_cloud/relayfile/delegated-token": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"code":"scope_insufficient","message":"workspace denies fs:write"}`)) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + writeAgentRelayCloudAuthForTest(t, server.URL, "cld_access") + + path := delegatedCredentialsPathForRequest("ws_cloud", defaultInspectScopes) + bundle := delegatedauth.Bundle{ + RelayfileURL: server.URL, + RelayauthURL: server.URL, + RelayfileWorkspaceID: "ws_relay", + WorkspaceID: "ws_cloud", + AccessToken: expired, + RefreshToken: "refresh_old", + AgentName: "relayfile-cli", + Scopes: append([]string(nil), defaultInspectScopes...), + } + if err := delegatedauth.SaveAtomic(path, bundle); err != nil { + t.Fatalf("save delegated credentials failed: %v", err) + } + + _, err := refreshDelegatedCredentials(path, bundle, false) + if err == nil { + t.Fatal("expected scope refusal") + } + if !errors.Is(err, ErrDelegatedRelayfileCredentialsExpired) { + t.Fatalf("expected rejected-refresh sentinel, got: %v", err) + } + if !errors.Is(err, ErrDelegatedScopeInsufficient) { + t.Fatalf("scope sentinel was lost during rejected-refresh wrapping: %v", err) + } + reason := degradedStallReasonFor(err) + for _, want := range []string{"requires human action", "workspace denies fs:write", "scopes must be corrected"} { + if !strings.Contains(reason, want) { + t.Fatalf("scope-refusal stall reason %q does not contain %q", reason, want) + } + } + if strings.Contains(reason, "relayfile will retry") { + t.Fatalf("scope-refusal stall reason must not claim automatic recovery: %q", reason) + } + t.Logf("REQUIRES-HUMAN-ACTION: %s", reason) } func TestWorkspaceCommandRefreshPreservesCatalogDefaultScopes(t *testing.T) { diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index c1c2f26d..f96328ec 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Cloud session auth no longer shells out to `agent-relay cloud session`. The CLI reads the canonical credential file `agent-relay cloud login` writes (`~/.agentworkforce/relay/cloud-auth.json`), or the `CLOUD_API_*` environment, and refreshes preemptively — an access token within five minutes of expiry, or a refresh token within 24 hours of its own expiry — through Cloud's `/api/v1/auth/token/refresh` endpoint, writing the rotated pair back under the same lock `agent-relay` uses. This restores auto-recovery from a routine token expiry. A session supplied through `CLOUD_API_*` is refreshed in memory only and never written to disk; an access token supplied with no refresh token is used as-is and never refreshed. - Relayfile no longer reads `AGENT_RELAY_BIN` to locate the `agent-relay` CLI. Agent Relay uses that variable for the *broker* binary, so every relay-spawned agent pointed Relayfile at `agent-relay-broker` — which has no `cloud` or `workspace` subcommand — and a routine expiry surfaced as `agent-relay CLI >= 8.7.0 required`. Use `RELAYFILE_AGENT_RELAY_BIN` to override the CLI path; otherwise `agent-relay` is resolved from `PATH`. - The `agent-relay` CLI compatibility probe now names the exact argv it ran, the binary it ran it with, and how that binary was resolved. It no longer probes `cloud session`, since Relayfile does not use it. +- Delegated-credential recovery errors no longer blame a healthy Agent Relay cloud session or direct users to log in again. They report that automatic Cloud re-mint failed and preserve the underlying re-mint error; regression coverage now exercises both the re-mint and ordinary refresh arms while `AGENT_RELAY_BIN` points at a broker with no `cloud` subcommand. +- The read-only degraded mount now reports the cause that produced it instead of one fixed sentence, and explicitly classifies recovery as retryable or requiring human action. A transient re-mint failure names the underlying error and states that Relayfile will retry; a missing or fully expired Cloud session, or scopes Cloud refuses to mint, are reported as needing a new sign-in or corrected scopes, because no amount of retrying restores those. The recorded cause is also updated while the mount stays degraded, so a stall that begins transient and later becomes unrecoverable stops advertising a retry that can no longer succeed, while changing request IDs within the same class no longer defeat the degraded-notice throttle. ### Changed