diff --git a/apps/daemon/internal/cli/connect_environment_diagnostics_test.go b/apps/daemon/internal/cli/connect_environment_diagnostics_test.go index aabf66dfa..f63c4e13e 100644 --- a/apps/daemon/internal/cli/connect_environment_diagnostics_test.go +++ b/apps/daemon/internal/cli/connect_environment_diagnostics_test.go @@ -63,13 +63,24 @@ func TestInstalledDaemonIgnoresRuntimeHomeEnvironment(t *testing.T) { } } -// A lost, moved or incompatible installation gets the working recovery, not a -// dead end. -func TestInstallationConflictNamesRecovery(t *testing.T) { +// A claim refused as already taken names the working recovery and leaves no +// credential file behind in the directory. +func TestClaimConflictNamesRecoveryAndRemovesCredential(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusConflict) })) defer server.Close() - err := installationRequest(t.Context(), server.URL+"/api/v1/agent-daemon/installation/claim", "grant", map[string]string{"executor_token": "x"}, nil) - if err == nil || !strings.Contains(err.Error(), "already claimed") || !strings.Contains(err.Error(), "Reconnect an existing Session") { + directory := t.TempDir() + held, err := os.OpenRoot(directory) + if err != nil { + t.Fatal(err) + } + defer held.Close() + o := &nativeInstallOptions{OnboardURL: server.URL + "/api/v1/agent-daemon/installation", Authorization: "grant"} + o.Directory, o.Environment = directory, "11111111-1111-4111-8111-111111111111" + err = prepareOnboardingCredential(t.Context(), o, held) + if err == nil || !strings.Contains(err.Error(), "already claimed") || !strings.Contains(err.Error(), "Rotate") || !strings.Contains(err.Error(), "Reconnect an existing Session") { t.Fatalf("conflict message: %v", err) } + if _, err := held.Stat("executor-credential.json"); !os.IsNotExist(err) { + t.Fatal("unclaimed credential file was kept") + } } diff --git a/apps/daemon/internal/cli/native_onboarding.go b/apps/daemon/internal/cli/native_onboarding.go index aa251339c..f5e517416 100644 --- a/apps/daemon/internal/cli/native_onboarding.go +++ b/apps/daemon/internal/cli/native_onboarding.go @@ -23,6 +23,8 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/runtimefs" ) +var errCredentialClaimed = errors.New("install: this Session's machine credential is already claimed by another installation, so its install command cannot be used again; to reconnect it, choose Rotate in Web (Session > Executor credentials) and install from an extracted distribution (see Self-hosted executors > Reconnect an existing Session), or create a new Session") + func installationRequest(ctx context.Context, endpoint, authorization string, input any, output any) error { body, err := json.Marshal(input) if err != nil { @@ -44,7 +46,7 @@ func installationRequest(ctx context.Context, endpoint, authorization string, in case http.StatusUnauthorized: return errors.New("install: authorization rejected or expired; copy a fresh command from the Session") case http.StatusConflict: - return errors.New("install: this Session's machine credential is already claimed by another installation; rerun the command in the original installation directory, or rotate the credential in Web and install from an extracted distribution (see Self-hosted executors > Reconnect an existing Session), or create a new Session") + return errCredentialClaimed default: return fmt.Errorf("install: Core rejected installation (HTTP %d); check the Session and obtain a fresh command", response.StatusCode) } @@ -143,7 +145,12 @@ func prepareOnboardingCredential(ctx context.Context, o *nativeInstallOptions, h } else { return err } - return installationRequest(ctx, o.OnboardURL+"/claim", o.Authorization, map[string]string{"executor_token": secret}, nil) + err := installationRequest(ctx, o.OnboardURL+"/claim", o.Authorization, map[string]string{"executor_token": secret}, nil) + if errors.Is(err, errCredentialClaimed) { + // This directory holds no installation, and Core never accepts this secret. + _ = held.Remove("executor-credential.json") + } + return err } func finishOnboarding(ctx context.Context, rc *runContext, o nativeInstallOptions) error { diff --git a/services/core/internal/nativeinstaller/assets/bootstrap.sh b/services/core/internal/nativeinstaller/assets/bootstrap.sh index f36176614..8436104c1 100644 --- a/services/core/internal/nativeinstaller/assets/bootstrap.sh +++ b/services/core/internal/nativeinstaller/assets/bootstrap.sh @@ -35,6 +35,7 @@ cleanup() { rm -rf "$work"; } interrupt() { trap '' INT TERM HUP if [ -n "$active" ]; then kill -TERM "$active" 2>/dev/null || :; wait "$active" 2>/dev/null || :; fi + printf 'Interrupted; rerun the same command to continue.\n' >&2 exit 130 } trap cleanup EXIT @@ -105,6 +106,8 @@ if [ -f "$archive" ]; then printf 'Verifying the installer archive from an earlier run...\n' verified "$archive" || { rm -f "$archive"; printf 'The earlier archive failed verification and was discarded.\n' >&2; } fi +# An interrupt between the last byte and the rename leaves a complete part file; accept it without asking the server for more. +if [ ! -f "$archive" ] && [ -s "$archive.part" ] && verified "$archive.part"; then mv "$archive.part" "$archive"; fi if [ ! -f "$archive" ]; then [ ! -s "$archive.part" ] || printf 'Resuming the earlier download...\n' available=$(df -Pk "$work" | awk 'NR==2 {print $4}') diff --git a/services/core/internal/nativeinstaller/bootstrap_interrupt_unix_test.go b/services/core/internal/nativeinstaller/bootstrap_interrupt_unix_test.go index f66121445..7acfc6f63 100644 --- a/services/core/internal/nativeinstaller/bootstrap_interrupt_unix_test.go +++ b/services/core/internal/nativeinstaller/bootstrap_interrupt_unix_test.go @@ -112,3 +112,45 @@ func TestBootstrapCommandDiscardsTimedOutResponse(t *testing.T) { t.Fatalf("partial bootstrap executed: %v requests=%d %s", err, requests.Load(), output) } } + +func TestBootstrapInterruptSaysHowToContinue(t *testing.T) { + archive := bootstrapFixtureArchive(t) + started := make(chan struct{}) + release := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/installation"): + w.WriteHeader(404) + case strings.HasSuffix(r.URL.Path, ".sha256"): + fmt.Fprintf(w, "%x\n", sha256.Sum256(archive)) + default: + w.Header().Set("Content-Length", fmt.Sprint(len(archive))) + w.Write(archive[:1]) + w.(http.Flusher).Flush() + close(started) + <-release + } + })) + defer server.Close() + defer close(release) + command := bootstrapCommand(t, server.URL, t.TempDir()) + command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + var output bytes.Buffer + command.Stdout = &output + command.Stderr = &output + if err := command.Start(); err != nil { + t.Fatal(err) + } + defer syscall.Kill(-command.Process.Pid, syscall.SIGKILL) + select { + case <-started: + case <-time.After(10 * time.Second): + t.Fatal("download did not start") + } + if err := command.Process.Signal(syscall.SIGINT); err != nil { + t.Fatal(err) + } + if err := command.Wait(); err == nil || !strings.Contains(output.String(), "Interrupted; rerun the same command to continue.") { + t.Fatalf("interrupt message missing: %v %s", err, output.String()) + } +} diff --git a/services/core/internal/nativeinstaller/bootstrap_resume_unix_test.go b/services/core/internal/nativeinstaller/bootstrap_resume_unix_test.go index d529cbcb1..63163b051 100644 --- a/services/core/internal/nativeinstaller/bootstrap_resume_unix_test.go +++ b/services/core/internal/nativeinstaller/bootstrap_resume_unix_test.go @@ -131,7 +131,8 @@ func TestBootstrapAcceptsCompletePartialDownload(t *testing.T) { if err := os.WriteFile(part, archive, 0600); err != nil { t.Fatal(err) } - if output, err := server.command(t, home).CombinedOutput(); err != nil { + output, err := server.command(t, home).CombinedOutput() + if err != nil || strings.Contains(string(output), "416") { t.Fatalf("%v: %s", err, output) } } diff --git a/services/core/internal/nativeinstaller/catalog.go b/services/core/internal/nativeinstaller/catalog.go index 40f8d1a88..cba289bb0 100644 --- a/services/core/internal/nativeinstaller/catalog.go +++ b/services/core/internal/nativeinstaller/catalog.go @@ -16,6 +16,7 @@ import ( "path/filepath" "regexp" "strings" + "time" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" @@ -119,6 +120,8 @@ func (c *Catalog) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } if c.local[platform] { + // An archive outlives the server-wide write timeout on a slow link. + _ = http.NewResponseController(w).SetWriteDeadline(time.Time{}) http.ServeFile(w, r, filepath.Join(c.directory, name)) return } diff --git a/services/core/internal/nativeinstaller/catalog_test.go b/services/core/internal/nativeinstaller/catalog_test.go index ce6fd24f3..2737780a7 100644 --- a/services/core/internal/nativeinstaller/catalog_test.go +++ b/services/core/internal/nativeinstaller/catalog_test.go @@ -1,13 +1,17 @@ package nativeinstaller import ( + "bytes" "crypto/sha256" "encoding/hex" "encoding/json" + "io" + "net/http" "net/http/httptest" "os" "path/filepath" "testing" + "time" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" ) @@ -114,3 +118,35 @@ func TestMissingCatalogServesNoInstallers(t *testing.T) { t.Fatal(catalog, err) } } + +func TestLocalArchiveOutlivesServerWriteTimeout(t *testing.T) { + dir := t.TempDir() + data := bytes.Repeat([]byte("x"), 64<<20) + hash := sha256.Sum256(data) + raw, _ := json.Marshal(Catalog{Version: "build", ProtocolVersion: proto.Version, Artifacts: map[string]Artifact{"linux-amd64": {SHA256: hex.EncodeToString(hash[:])}}}) + if err := os.WriteFile(filepath.Join(dir, "catalog.json"), raw, 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "linux-amd64.tar.gz"), data, 0600); err != nil { + t.Fatal(err) + } + catalog, err := Load(dir, "build") + if err != nil { + t.Fatal(err) + } + server := httptest.NewUnstartedServer(catalog) + server.Config.WriteTimeout = 100 * time.Millisecond + server.Start() + defer server.Close() + response, err := http.Get(server.URL + "/api/v1/agent-daemon/install/build/linux-amd64.tar.gz") + if err != nil { + t.Fatal(err) + } + defer response.Body.Close() + // Read past the write timeout, as a slow link does. + time.Sleep(300 * time.Millisecond) + body, err := io.ReadAll(response.Body) + if err != nil || !bytes.Equal(body, data) { + t.Fatalf("download was cut: %d of %d bytes, %v", len(body), len(data), err) + } +}