From f19ab1876e701de5c68227f3b3ab1d57f3054141 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Mon, 21 Sep 2026 15:57:13 +0200 Subject: [PATCH 1/7] feat(serverless): report the endpoint-set change a deploy makes A code app's endpoint path is its handler's method name with underscores turned into hyphens, so renaming a method moves a public endpoint and 404s its callers with nothing on the path saying so. An update run with --wait now reads the endpoint set before the deploy and again once the rollout lands, and reports what moved. Only on that combination: a create has no previous set to compare against, and without --wait the command returns before the build that decides the new one. The report goes to stderr so --format json stays machine-readable on stdout, and a failed read costs the warning rather than the deploy. RUNSERV-763 Co-Authored-By: Claude Opus 5 (1M context) --- docs/runware_serverless_deploy.md | 6 + internal/cmd/serverless/deploy.go | 29 +++- internal/cmd/serverless/deploy_endpoints.go | 113 ++++++++++++++ .../cmd/serverless/deploy_endpoints_test.go | 144 ++++++++++++++++++ internal/cmd/serverless/display_test.go | 6 +- 5 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 internal/cmd/serverless/deploy_endpoints.go create mode 100644 internal/cmd/serverless/deploy_endpoints_test.go diff --git a/docs/runware_serverless_deploy.md b/docs/runware_serverless_deploy.md index c940003..3cc25f6 100644 --- a/docs/runware_serverless_deploy.md +++ b/docs/runware_serverless_deploy.md @@ -55,6 +55,12 @@ A volume keeps it out of both. Worker settings are supplied via flags on create. Endpoints are derived server-side from the SDK (code) or from container.yaml (container). +A code app's endpoint path is its handler's method name with underscores turned +into hyphens, so renaming a method moves a public endpoint and 404s its callers. +Updating an existing application with --wait reports what the deploy did to the +endpoint set once the rollout lands. It is a report, not a gate: renaming an +endpoint on purpose is allowed. + ``` runware serverless deploy [file] [flags] ``` diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index 0298501..1360da3 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -183,7 +183,13 @@ download is copied into every checkpoint and fetched again on every cold start. A volume keeps it out of both. Worker settings are supplied via flags on create. Endpoints are derived -server-side from the SDK (code) or from container.yaml (container).`, +server-side from the SDK (code) or from container.yaml (container). + +A code app's endpoint path is its handler's method name with underscores turned +into hyphens, so renaming a method moves a public endpoint and 404s its callers. +Updating an existing application with --wait reports what the deploy did to the +endpoint set once the rollout lands. It is a report, not a gate: renaming an +endpoint on purpose is allowed.`, Example: ` # deploy the current directory, with app.py as the entry point runware serverless deploy ./app.py --id my-app --gpu-type h100 @@ -242,6 +248,18 @@ server-side from the SDK (code) or from container.yaml (container).`, return err } + // The endpoint set the app is serving now, to compare against the one + // the new version publishes. Only on an update we are going to wait + // for: a create has no previous set, and without --wait this command + // returns before the build that decides the new one. A read failure + // is not fatal — it costs the warning, not the deploy. + var endpointsBefore []string + if update && wait { + if paths, err := deployEndpointPaths(cmd.Context(), client, id); err == nil { + endpointsBefore = paths + } + } + var ( appVolumes *[]serverlessapi.AppVolume appEnv *map[string]string @@ -316,6 +334,15 @@ server-side from the SDK (code) or from container.yaml (container).`, } spin.Stop() + // Only once the app is active are the endpoint rows the new version's: + // the set is derived server-side by the build, and the rows are replaced + // when the rollout activates. A deploy that failed moved nothing. + if update && wait && app.Status == serverlessapi.AppStatusActive { + if paths, err := deployEndpointPaths(cmd.Context(), client, app.AppId); err == nil { + reportEndpointSetChange(cmd.ErrOrStderr(), compareEndpointSets(endpointsBefore, paths)) + } + } + if err := output.Print(cmdutil.FormatFor(cmd), appResult(*app)); err != nil { return err } diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go new file mode 100644 index 0000000..56d4ddd --- /dev/null +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -0,0 +1,113 @@ +package serverless + +import ( + "context" + "fmt" + "io" + "slices" + + serverlessapi "github.com/runware/runware-cli/internal/api/serverless" +) + +// endpointSetChange is what a deploy did to the app's public endpoint set: +// the paths it published, and the paths it retired. Both sorted. +// +// A code app's endpoint path is its handler's method name with underscores +// turned into hyphens, so renaming a method is an ordinary local refactor that +// moves a public URL and 404s the customer's callers. Reporting the change is +// the signal that says so; nothing here refuses the deploy. +type endpointSetChange struct { + added []string + removed []string +} + +func (c endpointSetChange) empty() bool { + return len(c.added) == 0 && len(c.removed) == 0 +} + +// deployEndpointPaths reads the app's live endpoint paths, sorted. +// +// One page is the whole set: an app may declare at most 20 endpoints and the +// list route pages at 100, so there is no cursor to follow. A miss is not an +// error to the caller — an app that does not exist yet, or one whose first +// version has not deployed, simply has no endpoints to compare against. +func deployEndpointPaths(ctx context.Context, client *serverlessapi.Client, appID string) ([]string, error) { + page, err := client.ListEndpoints(ctx, appID, nil) + if err != nil { + return nil, err + } + paths := make([]string, 0, len(page.Data)) + for i := range page.Data { + paths = append(paths, page.Data[i].Path) + } + slices.Sort(paths) + return paths, nil +} + +// compareEndpointSets reports what moved between two readings of the set. +func compareEndpointSets(before, after []string) endpointSetChange { + beforeSet := make(map[string]struct{}, len(before)) + for _, path := range before { + beforeSet[path] = struct{}{} + } + afterSet := make(map[string]struct{}, len(after)) + for _, path := range after { + afterSet[path] = struct{}{} + } + + var change endpointSetChange + for _, path := range after { + if _, ok := beforeSet[path]; !ok { + change.added = append(change.added, path) + } + } + for _, path := range before { + if _, ok := afterSet[path]; !ok { + change.removed = append(change.removed, path) + } + } + slices.Sort(change.added) + slices.Sort(change.removed) + return change +} + +// reportEndpointSetChange writes the change to w, and nothing at all when the +// deploy left the set alone — most deploys change code behind an unchanged set, +// and a line on every one of those would bury the deploy that moves a URL. +// +// To stderr, never stdout: stdout carries the machine-readable app record that +// --format json promises, and a warning there would corrupt it. +func reportEndpointSetChange(w io.Writer, change endpointSetChange) { + if change.empty() { + return + } + var clauses []string + if len(change.removed) > 0 { + clauses = append(clauses, "removes "+quotedPaths(change.removed)) + } + if len(change.added) > 0 { + clauses = append(clauses, "adds "+quotedPaths(change.added)) + } + + message := "This deploy " + clauses[0] + for _, clause := range clauses[1:] { + message += " and " + clause + } + _, _ = fmt.Fprintf(w, "Warning: %s.\n", message) + if len(change.removed) > 0 { + _, _ = fmt.Fprintf(w, + "Callers of %s will receive 404s. Rename the handler back if this was not intended.\n", + quotedPaths(change.removed)) + } +} + +func quotedPaths(paths []string) string { + out := "" + for i, path := range paths { + if i > 0 { + out += ", " + } + out += "'" + path + "'" + } + return out +} diff --git a/internal/cmd/serverless/deploy_endpoints_test.go b/internal/cmd/serverless/deploy_endpoints_test.go new file mode 100644 index 0000000..2f88df3 --- /dev/null +++ b/internal/cmd/serverless/deploy_endpoints_test.go @@ -0,0 +1,144 @@ +package serverless + +import ( + "bytes" + "context" + "log/slog" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + serverlessapi "github.com/runware/runware-cli/internal/api/serverless" +) + +func TestCompareEndpointSets(t *testing.T) { + cases := []struct { + name string + before []string + after []string + wantAdded []string + wantRemoved []string + }{ + { + name: "an unchanged set moves nothing", + before: []string{testOtherEndpointPath, testEndpointPath}, + after: []string{testOtherEndpointPath, testEndpointPath}, + }, + { + // The rename this whole report exists for: run_upscale became + // upscale_image, so the public path moved with it. + name: "a rename is a removal and an addition", + before: []string{testOtherEndpointPath, "run-upscale"}, + after: []string{testOtherEndpointPath, "upscale-image"}, + wantAdded: []string{"upscale-image"}, + wantRemoved: []string{"run-upscale"}, + }, + { + name: "a first deploy adds everything", + after: []string{testEndpointPath, testOtherEndpointPath}, + wantAdded: []string{testOtherEndpointPath, testEndpointPath}, + }, + { + name: "a handler deleted outright", + before: []string{testOtherEndpointPath, testEndpointPath}, + after: []string{testOtherEndpointPath}, + wantRemoved: []string{testEndpointPath}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + change := compareEndpointSets(tc.before, tc.after) + if !slices.Equal(change.added, tc.wantAdded) { + t.Errorf("added = %v, want %v", change.added, tc.wantAdded) + } + if !slices.Equal(change.removed, tc.wantRemoved) { + t.Errorf("removed = %v, want %v", change.removed, tc.wantRemoved) + } + if got := change.empty(); got != (len(tc.wantAdded) == 0 && len(tc.wantRemoved) == 0) { + t.Errorf("empty() = %v, disagrees with the reported change", got) + } + }) + } +} + +// TestReportEndpointSetChangeNamesTheBreak: the removal is the half that 404s +// the customer's callers, so it leads and it is called out on its own line. +func TestReportEndpointSetChangeNamesTheBreak(t *testing.T) { + var out bytes.Buffer + reportEndpointSetChange(&out, endpointSetChange{ + added: []string{"upscale-image"}, + removed: []string{"run-upscale"}, + }) + + got := out.String() + for _, want := range []string{ + "This deploy removes 'run-upscale' and adds 'upscale-image'.", + "Callers of 'run-upscale' will receive 404s", + } { + if !strings.Contains(got, want) { + t.Errorf("report = %q, want it to contain %q", got, want) + } + } +} + +// TestReportEndpointSetChangeAdditionOnly: publishing a new endpoint breaks +// nobody, so it is reported without the 404 warning. +func TestReportEndpointSetChangeAdditionOnly(t *testing.T) { + var out bytes.Buffer + reportEndpointSetChange(&out, endpointSetChange{added: []string{testOtherEndpointPath}}) + + got := out.String() + if !strings.Contains(got, "This deploy adds 'embed'.") { + t.Errorf("report = %q, want the addition", got) + } + if strings.Contains(got, "404") { + t.Errorf("report = %q, warns about callers when nothing was removed", got) + } +} + +// TestReportEndpointSetChangeSaysNothingWhenNothingMoved keeps the ordinary +// deploy quiet: most deploys change code behind an unchanged endpoint set. +func TestReportEndpointSetChangeSaysNothingWhenNothingMoved(t *testing.T) { + var out bytes.Buffer + reportEndpointSetChange(&out, endpointSetChange{}) + if out.Len() != 0 { + t.Errorf("report = %q, want nothing for an unchanged set", out.String()) + } +} + +func TestDeployEndpointPaths(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Returned out of order, to prove the reading is sorted before it is diffed. + _, _ = w.Write([]byte(`{"data":[ + {"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","path":"generate"}, + {"id":"019c7654-8b21-7abc-9123-abcdef123457","appId":"my-app","path":"embed"} + ]}`)) + })) + defer server.Close() + + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + paths, err := deployEndpointPaths(context.Background(), client, testAppID) + if err != nil { + t.Fatalf("deployEndpointPaths: %v", err) + } + if !slices.Equal(paths, []string{testOtherEndpointPath, testEndpointPath}) { + t.Errorf("paths = %v, want [embed generate]", paths) + } +} + +// TestDeployEndpointPathsSurfacesTheError: the caller decides a failed read +// costs the warning rather than the deploy, so this must not swallow it here. +func TestDeployEndpointPathsSurfacesTheError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + if _, err := deployEndpointPaths(context.Background(), client, testAppID); err == nil { + t.Fatal("expected an error for 500") + } +} diff --git a/internal/cmd/serverless/display_test.go b/internal/cmd/serverless/display_test.go index 62056c4..1d2c57e 100644 --- a/internal/cmd/serverless/display_test.go +++ b/internal/cmd/serverless/display_test.go @@ -18,6 +18,10 @@ const ( testEnvValue = "hello" testGPUType = "h100" testEventType = "error" + // testEndpointPath and testOtherEndpointPath are two endpoint paths on one + // app, so a set is never a single element. + testEndpointPath = "generate" + testOtherEndpointPath = "embed" ) func TestListPageParams(t *testing.T) { @@ -550,7 +554,7 @@ func TestEndpointResult(t *testing.T) { rows := (endpointResult{ Id: uuid.MustParse("11111111-1111-1111-1111-111111111111"), AppId: testAppID, - Path: "generate", + Path: testEndpointPath, CreatedAt: &created, }).Rows() if len(rows) != 5 { From 70a35038239497433d42f1469859b5934970cad4 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Mon, 21 Sep 2026 16:03:26 +0200 Subject: [PATCH 2/7] fix(serverless): do not warn about endpoints when the before-read failed Reading the app's endpoint set before the deploy can fail, and carrying on with an empty reading made every endpoint the app already served look newly added. A deploy that changed nothing would then print a warning naming the whole set. A warning that cries wolf on an unchanged deploy is worse than no warning: the one it exists to raise is a real rename. So a failed before-read now suppresses the report entirely rather than reporting against nothing. RUNSERV-763 Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/serverless/deploy.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index 1360da3..b9d844c 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -251,12 +251,20 @@ endpoint on purpose is allowed.`, // The endpoint set the app is serving now, to compare against the one // the new version publishes. Only on an update we are going to wait // for: a create has no previous set, and without --wait this command - // returns before the build that decides the new one. A read failure - // is not fatal — it costs the warning, not the deploy. - var endpointsBefore []string + // returns before the build that decides the new one. + // + // A read failure costs the warning, not the deploy — but it has to + // cost the whole warning. Carrying on with an empty "before" would + // make every endpoint the app already served look newly added, and a + // warning that cries wolf on an unchanged deploy is worse than none: + // the one it needs to raise is a real rename. + var ( + endpointsBefore []string + canCompare bool + ) if update && wait { if paths, err := deployEndpointPaths(cmd.Context(), client, id); err == nil { - endpointsBefore = paths + endpointsBefore, canCompare = paths, true } } @@ -337,7 +345,7 @@ endpoint on purpose is allowed.`, // Only once the app is active are the endpoint rows the new version's: // the set is derived server-side by the build, and the rows are replaced // when the rollout activates. A deploy that failed moved nothing. - if update && wait && app.Status == serverlessapi.AppStatusActive { + if canCompare && app.Status == serverlessapi.AppStatusActive { if paths, err := deployEndpointPaths(cmd.Context(), client, app.AppId); err == nil { reportEndpointSetChange(cmd.ErrOrStderr(), compareEndpointSets(endpointsBefore, paths)) } From ccba29feccad106f51cd3c56d551b78b17603405 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Mon, 21 Sep 2026 16:04:28 +0200 Subject: [PATCH 3/7] refactor(serverless): name the two ways the endpoint report can lie The guard was two conditions inline with the reasoning in a comment. Naming it puts the reasoning on the predicate and makes both halves testable: with no reading from before the deploy the report would call the app's whole existing set new, and before the rollout activates it would compare the old set against itself. RUNSERV-763 Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/serverless/deploy.go | 12 ++---- internal/cmd/serverless/deploy_endpoints.go | 15 +++++++ .../cmd/serverless/deploy_endpoints_test.go | 42 +++++++++++++++++++ 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index b9d844c..95624d3 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -253,11 +253,8 @@ endpoint on purpose is allowed.`, // for: a create has no previous set, and without --wait this command // returns before the build that decides the new one. // - // A read failure costs the warning, not the deploy — but it has to - // cost the whole warning. Carrying on with an empty "before" would - // make every endpoint the app already served look newly added, and a - // warning that cries wolf on an unchanged deploy is worse than none: - // the one it needs to raise is a real rename. + // A read failure costs the warning, not the deploy — see + // shouldReportEndpointChange for why it has to cost the whole warning. var ( endpointsBefore []string canCompare bool @@ -342,10 +339,7 @@ endpoint on purpose is allowed.`, } spin.Stop() - // Only once the app is active are the endpoint rows the new version's: - // the set is derived server-side by the build, and the rows are replaced - // when the rollout activates. A deploy that failed moved nothing. - if canCompare && app.Status == serverlessapi.AppStatusActive { + if shouldReportEndpointChange(canCompare, app.Status) { if paths, err := deployEndpointPaths(cmd.Context(), client, app.AppId); err == nil { reportEndpointSetChange(cmd.ErrOrStderr(), compareEndpointSets(endpointsBefore, paths)) } diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go index 56d4ddd..7838440 100644 --- a/internal/cmd/serverless/deploy_endpoints.go +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -25,6 +25,21 @@ func (c endpointSetChange) empty() bool { return len(c.added) == 0 && len(c.removed) == 0 } +// shouldReportEndpointChange decides whether the two readings are worth +// comparing. Both conditions are load-bearing. +// +// readBefore, because carrying on without a reading from before the deploy +// would make every endpoint the app already served look newly added, and a +// warning that cries wolf on an unchanged deploy is worse than none: the one it +// exists to raise is a real rename. +// +// active, because the endpoint rows only become the new version's when the +// rollout activates. A deploy that failed, or one still rolling, moved nothing +// yet, and reading the set then reports the old one against itself. +func shouldReportEndpointChange(readBefore bool, status serverlessapi.AppStatus) bool { + return readBefore && status == serverlessapi.AppStatusActive +} + // deployEndpointPaths reads the app's live endpoint paths, sorted. // // One page is the whole set: an app may declare at most 20 endpoints and the diff --git a/internal/cmd/serverless/deploy_endpoints_test.go b/internal/cmd/serverless/deploy_endpoints_test.go index 2f88df3..5cc9079 100644 --- a/internal/cmd/serverless/deploy_endpoints_test.go +++ b/internal/cmd/serverless/deploy_endpoints_test.go @@ -108,6 +108,48 @@ func TestReportEndpointSetChangeSaysNothingWhenNothingMoved(t *testing.T) { } } +// TestShouldReportEndpointChange guards the two ways this report can lie: with +// no reading from before the deploy it would call the app's whole existing set +// new, and before the rollout activates it would compare the old set to itself. +func TestShouldReportEndpointChange(t *testing.T) { + cases := []struct { + name string + readBefore bool + status serverlessapi.AppStatus + want bool + }{ + { + name: "read before and rolled out", + readBefore: true, + status: serverlessapi.AppStatusActive, + want: true, + }, + { + name: "the before-read failed, so the whole set would look new", + readBefore: false, + status: serverlessapi.AppStatusActive, + }, + { + name: "the deploy failed, so nothing moved", + readBefore: true, + status: serverlessapi.AppStatusFailed, + }, + { + name: "still rolling out, so the rows are still the old version's", + readBefore: true, + status: serverlessapi.AppStatusInitializing, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := shouldReportEndpointChange(tc.readBefore, tc.status); got != tc.want { + t.Errorf("shouldReportEndpointChange(%v, %q) = %v, want %v", + tc.readBefore, tc.status, got, tc.want) + } + }) + } +} + func TestDeployEndpointPaths(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") From 19e7c51e73b6e08932fbd1db861688351c3ce9b6 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Mon, 21 Sep 2026 18:01:51 +0200 Subject: [PATCH 4/7] docs(serverless): trim the endpoint-report comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file explained itself three times over: what an endpoint path is, which the deploy help already says, and a paragraph per helper restating its own name. Kept only what the code cannot say — why the report goes to stderr, why both guard conditions exist, and why one page is the whole set. Comment-only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/serverless/deploy.go | 11 ++---- internal/cmd/serverless/deploy_endpoints.go | 42 ++++++--------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index 95624d3..c3df050 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -248,13 +248,10 @@ endpoint on purpose is allowed.`, return err } - // The endpoint set the app is serving now, to compare against the one - // the new version publishes. Only on an update we are going to wait - // for: a create has no previous set, and without --wait this command - // returns before the build that decides the new one. - // - // A read failure costs the warning, not the deploy — see - // shouldReportEndpointChange for why it has to cost the whole warning. + // The set the app serves now, to compare against what the new version + // publishes. Only on an update we will wait for: a create has no + // previous set, and without --wait this returns before the build that + // decides the new one. A read failure costs the warning, not the deploy. var ( endpointsBefore []string canCompare bool diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go index 7838440..3f3f7cf 100644 --- a/internal/cmd/serverless/deploy_endpoints.go +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -9,13 +9,8 @@ import ( serverlessapi "github.com/runware/runware-cli/internal/api/serverless" ) -// endpointSetChange is what a deploy did to the app's public endpoint set: -// the paths it published, and the paths it retired. Both sorted. -// -// A code app's endpoint path is its handler's method name with underscores -// turned into hyphens, so renaming a method is an ordinary local refactor that -// moves a public URL and 404s the customer's callers. Reporting the change is -// the signal that says so; nothing here refuses the deploy. +// endpointSetChange is what a deploy did to the app's public endpoint set: the +// paths it published and the ones it retired, each sorted. type endpointSetChange struct { added []string removed []string @@ -25,27 +20,16 @@ func (c endpointSetChange) empty() bool { return len(c.added) == 0 && len(c.removed) == 0 } -// shouldReportEndpointChange decides whether the two readings are worth -// comparing. Both conditions are load-bearing. -// -// readBefore, because carrying on without a reading from before the deploy -// would make every endpoint the app already served look newly added, and a -// warning that cries wolf on an unchanged deploy is worse than none: the one it -// exists to raise is a real rename. -// -// active, because the endpoint rows only become the new version's when the -// rollout activates. A deploy that failed, or one still rolling, moved nothing -// yet, and reading the set then reports the old one against itself. +// shouldReportEndpointChange guards the two ways the comparison would lie: +// without a reading from before the deploy the app's whole existing set looks +// new, and before the rollout activates the rows are still the old version's. func shouldReportEndpointChange(readBefore bool, status serverlessapi.AppStatus) bool { return readBefore && status == serverlessapi.AppStatusActive } -// deployEndpointPaths reads the app's live endpoint paths, sorted. -// -// One page is the whole set: an app may declare at most 20 endpoints and the -// list route pages at 100, so there is no cursor to follow. A miss is not an -// error to the caller — an app that does not exist yet, or one whose first -// version has not deployed, simply has no endpoints to compare against. +// deployEndpointPaths reads the app's live endpoint paths, sorted. One page is +// the whole set: at most 20 endpoints per app against a list route that pages at +// 100, so there is no cursor to follow. func deployEndpointPaths(ctx context.Context, client *serverlessapi.Client, appID string) ([]string, error) { page, err := client.ListEndpoints(ctx, appID, nil) if err != nil { @@ -59,7 +43,6 @@ func deployEndpointPaths(ctx context.Context, client *serverlessapi.Client, appI return paths, nil } -// compareEndpointSets reports what moved between two readings of the set. func compareEndpointSets(before, after []string) endpointSetChange { beforeSet := make(map[string]struct{}, len(before)) for _, path := range before { @@ -86,12 +69,9 @@ func compareEndpointSets(before, after []string) endpointSetChange { return change } -// reportEndpointSetChange writes the change to w, and nothing at all when the -// deploy left the set alone — most deploys change code behind an unchanged set, -// and a line on every one of those would bury the deploy that moves a URL. -// -// To stderr, never stdout: stdout carries the machine-readable app record that -// --format json promises, and a warning there would corrupt it. +// reportEndpointSetChange writes the change, and nothing when the deploy left the +// set alone. w must be stderr: stdout carries the app record --format json +// promises, and a warning there would corrupt it. func reportEndpointSetChange(w io.Writer, change endpointSetChange) { if change.empty() { return From 93b4e25f64fdf6dacb893659046d875b973da368 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Mon, 21 Sep 2026 18:10:15 +0200 Subject: [PATCH 5/7] fix(serverless): page through the endpoint list before comparing The reader took one page and my comment justified it with a page size of 100. The shared limit parameter defaults to 20, and an app may declare 20 endpoints, so a full app already sits exactly on the page boundary. A short read is not a missing warning but a false one: the endpoints it never saw look removed, and the report tells the customer their callers are about to 404 on paths that never moved. It now follows the cursor, asking for the contract maximum so the common app still takes one round trip. Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/serverless/deploy_endpoints.go | 39 ++++++++++++++----- .../cmd/serverless/deploy_endpoints_test.go | 33 ++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go index 3f3f7cf..cf96ad4 100644 --- a/internal/cmd/serverless/deploy_endpoints.go +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -9,6 +9,10 @@ import ( serverlessapi "github.com/runware/runware-cli/internal/api/serverless" ) +// endpointPageLimit is the per-page size deployEndpointPaths asks for: the +// contract's maximum, so the common app takes one round trip. +const endpointPageLimit = 100 + // endpointSetChange is what a deploy did to the app's public endpoint set: the // paths it published and the ones it retired, each sorted. type endpointSetChange struct { @@ -27,17 +31,32 @@ func shouldReportEndpointChange(readBefore bool, status serverlessapi.AppStatus) return readBefore && status == serverlessapi.AppStatusActive } -// deployEndpointPaths reads the app's live endpoint paths, sorted. One page is -// the whole set: at most 20 endpoints per app against a list route that pages at -// 100, so there is no cursor to follow. +// deployEndpointPaths reads every live endpoint path on the app, sorted. +// +// It follows the cursor rather than trusting one page. The limit defaults to 20 +// and an app may declare 20 endpoints, so a full app already sits exactly on the +// page boundary; a short read would report the endpoints it did not see as +// removed, and tell the customer their callers are about to 404 on paths that +// never moved. func deployEndpointPaths(ctx context.Context, client *serverlessapi.Client, appID string) ([]string, error) { - page, err := client.ListEndpoints(ctx, appID, nil) - if err != nil { - return nil, err - } - paths := make([]string, 0, len(page.Data)) - for i := range page.Data { - paths = append(paths, page.Data[i].Path) + var ( + paths []string + cursor string + ) + for { + params := &serverlessapi.ListEndpointsParams{} + params.Limit, params.Cursor = listPageParams(endpointPageLimit, cursor) + page, err := client.ListEndpoints(ctx, appID, params) + if err != nil { + return nil, err + } + for i := range page.Data { + paths = append(paths, page.Data[i].Path) + } + if page.NextCursor == nil || *page.NextCursor == "" { + break + } + cursor = *page.NextCursor } slices.Sort(paths) return paths, nil diff --git a/internal/cmd/serverless/deploy_endpoints_test.go b/internal/cmd/serverless/deploy_endpoints_test.go index 5cc9079..2cd17e9 100644 --- a/internal/cmd/serverless/deploy_endpoints_test.go +++ b/internal/cmd/serverless/deploy_endpoints_test.go @@ -171,6 +171,39 @@ func TestDeployEndpointPaths(t *testing.T) { } } +// TestDeployEndpointPathsFollowsTheCursor: an app may declare as many endpoints +// as a default page holds, so a reader that stops at the first page would call +// the endpoints it never saw removed and warn about 404s that are not coming. +func TestDeployEndpointPathsFollowsTheCursor(t *testing.T) { + var gotCursors []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotCursors = append(gotCursors, r.URL.Query().Get("cursor")) + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("cursor") == "" { + _, _ = w.Write([]byte(`{"data":[ + {"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","path":"generate"} + ],"nextCursor":"page2"}`)) + return + } + _, _ = w.Write([]byte(`{"data":[ + {"id":"019c7654-8b21-7abc-9123-abcdef123457","appId":"my-app","path":"embed"} + ]}`)) + })) + defer server.Close() + + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + paths, err := deployEndpointPaths(context.Background(), client, testAppID) + if err != nil { + t.Fatalf("deployEndpointPaths: %v", err) + } + if !slices.Equal(paths, []string{testOtherEndpointPath, testEndpointPath}) { + t.Errorf("paths = %v, want both pages", paths) + } + if !slices.Equal(gotCursors, []string{"", "page2"}) { + t.Errorf("cursors requested = %v, want the second page to be fetched once", gotCursors) + } +} + // TestDeployEndpointPathsSurfacesTheError: the caller decides a failed read // costs the warning rather than the deploy, so this must not swallow it here. func TestDeployEndpointPathsSurfacesTheError(t *testing.T) { From 088f33a8262ebf789d83456497b93d5f9b221937 Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Tue, 22 Sep 2026 12:16:55 +0200 Subject: [PATCH 6/7] fix(serverless): wait for the submitted version before comparing endpoints A source update on an app that is already active answers `active` while its new build runs, so AppDeployTerminal was true immediately, --wait skipped polling, and the endpoint rows read back were still the outgoing version's. Every rename on an existing app therefore compared a set against itself and reported nothing -- the one case this feature is for. The pin is what moves when the submitted version activates, so the comparison now waits on activeVersionId rather than on status. It gives up when no activation can arrive: the app left the states a roll lands in, or the build failed, which on a live app leaves the status active and the pin where it was and would otherwise poll forever. shouldReportEndpointChange goes with it. The pin move proves what its status check approximated, and proves it for the case the status check got wrong. Also make the 404 line source-neutral: container apps take their paths from container.yaml, not from a handler name. Co-Authored-By: Claude Opus 5 (1M context) --- internal/api/serverless/client.go | 7 + internal/cmd/serverless/deploy.go | 18 +- internal/cmd/serverless/deploy_endpoints.go | 81 ++++++++- .../cmd/serverless/deploy_endpoints_test.go | 157 +++++++++++++----- 4 files changed, 210 insertions(+), 53 deletions(-) diff --git a/internal/api/serverless/client.go b/internal/api/serverless/client.go index 4316152..a4c88f0 100644 --- a/internal/api/serverless/client.go +++ b/internal/api/serverless/client.go @@ -94,6 +94,13 @@ type Build = gen.Build // BuildStatus is a build lifecycle status. type BuildStatus = gen.BuildStatus +const ( + BuildStatusBuilding BuildStatus = gen.BuildStatusBuilding + BuildStatusFailed BuildStatus = gen.BuildStatusFailed + BuildStatusQueued BuildStatus = gen.BuildStatusQueued + BuildStatusReady BuildStatus = gen.BuildStatusReady +) + // ListWorkersParams are optional filters for ListWorkers. type ListWorkersParams = gen.ListWorkersParams diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index c3df050..e694664 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -254,12 +254,16 @@ endpoint on purpose is allowed.`, // decides the new one. A read failure costs the warning, not the deploy. var ( endpointsBefore []string + versionBefore *uuid.UUID canCompare bool ) if update && wait { if paths, err := deployEndpointPaths(cmd.Context(), client, id); err == nil { endpointsBefore, canCompare = paths, true } + if live, err := client.GetApp(cmd.Context(), id); err == nil { + versionBefore = live.ActiveVersionId + } } var ( @@ -336,9 +340,17 @@ endpoint on purpose is allowed.`, } spin.Stop() - if shouldReportEndpointChange(canCompare, app.Status) { - if paths, err := deployEndpointPaths(cmd.Context(), client, app.AppId); err == nil { - reportEndpointSetChange(cmd.ErrOrStderr(), compareEndpointSets(endpointsBefore, paths)) + // The endpoint rows are the outgoing version's until the submitted one + // activates, and a source update on an already-active app answers + // `active` throughout its build — so the wait above returns at once and + // says nothing about whether this deploy landed. The pin is what moves + // when it does. + if canCompare { + settled, err := waitForSubmittedVersion(cmd.Context(), client, app.AppId, versionBefore, pollInterval) + if err == nil && activationMoved(versionBefore, settled.ActiveVersionId) { + if paths, err := deployEndpointPaths(cmd.Context(), client, app.AppId); err == nil { + reportEndpointSetChange(cmd.ErrOrStderr(), compareEndpointSets(endpointsBefore, paths)) + } } } diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go index cf96ad4..c9b3cf9 100644 --- a/internal/cmd/serverless/deploy_endpoints.go +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -5,7 +5,9 @@ import ( "fmt" "io" "slices" + "time" + "github.com/google/uuid" serverlessapi "github.com/runware/runware-cli/internal/api/serverless" ) @@ -13,6 +15,74 @@ import ( // contract's maximum, so the common app takes one round trip. const endpointPageLimit = 100 +// waitForSubmittedVersion polls until the app pins a version other than previous, +// and returns the app it saw last. +// +// The app's status cannot answer this on its own. A source update on an app that +// is already active leaves it active while the new build runs, so `--wait` sees a +// terminal status immediately and the endpoint rows it would read are still the +// outgoing version's. The pin is what moves when the submitted version activates. +// +// It gives up when no activation can still arrive: the app left the states a roll +// can land in, or the roll failed, which on a live app leaves the status active +// and the pin where it was — the case that would otherwise poll forever. +func waitForSubmittedVersion( + ctx context.Context, + client *serverlessapi.Client, + appID string, + previous *uuid.UUID, + interval time.Duration, +) (*serverlessapi.App, error) { + if interval <= 0 { + interval = 2 * time.Second + } + for { + app, err := client.GetApp(ctx, appID) + if err != nil { + return nil, err + } + if activationMoved(previous, app.ActiveVersionId) { + return app, nil + } + switch app.Status { + case serverlessapi.AppStatusActive, serverlessapi.AppStatusInitializing: + if buildFailed(ctx, client, appID) { + return app, nil + } + default: + return app, nil + } + + timer := time.NewTimer(interval) + select { + case <-ctx.Done(): + timer.Stop() + return nil, ctx.Err() + case <-timer.C: + } + } +} + +// activationMoved reports that the app pins a different version than it did. +// A first deploy moves from no pin at all, which counts. +func activationMoved(previous, current *uuid.UUID) bool { + if current == nil { + return false + } + return previous == nil || *previous != *current +} + +// buildFailed reports that the app's newest build gave up, so no activation is +// coming. Newest first, as listBuilds returns them; an unreadable list is not a +// failure, and the poll simply continues. +func buildFailed(ctx context.Context, client *serverlessapi.Client, appID string) bool { + page, err := client.ListBuilds(ctx, appID, nil) + if err != nil || len(page.Data) == 0 { + return false + } + return page.Data[0].Status == serverlessapi.BuildStatusFailed +} + // endpointSetChange is what a deploy did to the app's public endpoint set: the // paths it published and the ones it retired, each sorted. type endpointSetChange struct { @@ -24,13 +94,6 @@ func (c endpointSetChange) empty() bool { return len(c.added) == 0 && len(c.removed) == 0 } -// shouldReportEndpointChange guards the two ways the comparison would lie: -// without a reading from before the deploy the app's whole existing set looks -// new, and before the rollout activates the rows are still the old version's. -func shouldReportEndpointChange(readBefore bool, status serverlessapi.AppStatus) bool { - return readBefore && status == serverlessapi.AppStatusActive -} - // deployEndpointPaths reads every live endpoint path on the app, sorted. // // It follows the cursor rather than trusting one page. The limit defaults to 20 @@ -109,8 +172,10 @@ func reportEndpointSetChange(w io.Writer, change endpointSetChange) { } _, _ = fmt.Fprintf(w, "Warning: %s.\n", message) if len(change.removed) > 0 { + // Source-neutral: a code app's paths come from its handler names and a + // container app's from container.yaml, and this report covers both. _, _ = fmt.Fprintf(w, - "Callers of %s will receive 404s. Rename the handler back if this was not intended.\n", + "Callers of %s will receive 404s. Restore those paths in the source if this was not intended.\n", quotedPaths(change.removed)) } } diff --git a/internal/cmd/serverless/deploy_endpoints_test.go b/internal/cmd/serverless/deploy_endpoints_test.go index 2cd17e9..0d89542 100644 --- a/internal/cmd/serverless/deploy_endpoints_test.go +++ b/internal/cmd/serverless/deploy_endpoints_test.go @@ -3,16 +3,131 @@ package serverless import ( "bytes" "context" + "fmt" "log/slog" "net/http" "net/http/httptest" "slices" "strings" "testing" + "time" + "github.com/google/uuid" serverlessapi "github.com/runware/runware-cli/internal/api/serverless" ) +// appBody renders a getApp response pinning activeVersionId, or none when empty. +func appBody(status, activeVersionID string) string { + pin := "null" + if activeVersionID != "" { + pin = fmt.Sprintf("%q", activeVersionID) + } + return fmt.Sprintf(`{ + "appId":"my-app","appName":"My App","status":%q,"activeVersionId":%s, + "configuration":{"maxWorkers":1,"idleTtlSecs":60,"scalingDelaySecs":10,"minWorkers":0,"gpusPerWorker":1,"concurrency":1,"gracefulStopTtlSecs":120,"computeType":"gpu"}, + "environmentVariables":[],"secrets":[], + "createdAt":"2026-07-30T12:00:00Z","updatedAt":"2026-07-30T12:00:00Z" + }`, status, pin) +} + +const ( + oldVersionID = "019c7654-8b21-7abc-9123-aaaaaaaaaaaa" + newVersionID = "019c7654-8b21-7abc-9123-bbbbbbbbbbbb" +) + +// TestWaitForSubmittedVersionWaitsThroughAnActiveBuild is the defect this guard +// exists for: a source update on an app that is already active answers `active` +// while its new build runs, so a reader keyed on status alone compares the +// outgoing version's endpoint rows and reports that nothing moved. +func TestWaitForSubmittedVersionWaitsThroughAnActiveBuild(t *testing.T) { + var calls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.Contains(r.URL.Path, "/builds") { + _, _ = w.Write([]byte(`{"data":[{"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","status":"building","phases":[]}]}`)) + return + } + calls++ + // Still rolling the first two times, pinned the third. + if calls < 3 { + _, _ = w.Write([]byte(appBody("active", oldVersionID))) + return + } + _, _ = w.Write([]byte(appBody("active", newVersionID))) + })) + defer server.Close() + + previous := uuid.MustParse(oldVersionID) + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + app, err := waitForSubmittedVersion(context.Background(), client, testAppID, &previous, time.Millisecond) + if err != nil { + t.Fatalf("waitForSubmittedVersion: %v", err) + } + if app.ActiveVersionId == nil || app.ActiveVersionId.String() != newVersionID { + t.Errorf("pinned version = %v, want the submitted one", app.ActiveVersionId) + } + if calls < 3 { + t.Errorf("getApp calls = %d, want it to have kept polling past the unchanged pin", calls) + } +} + +// TestWaitForSubmittedVersionGivesUpOnAFailedBuild: a roll that fails on a live +// app leaves it active on the version that kept serving, so the pin never moves +// and nothing but the build says the wait is over. +func TestWaitForSubmittedVersionGivesUpOnAFailedBuild(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.Contains(r.URL.Path, "/builds") { + _, _ = w.Write([]byte(`{"data":[{"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","status":"failed","phases":[]}]}`)) + return + } + _, _ = w.Write([]byte(appBody("active", oldVersionID))) + })) + defer server.Close() + + previous := uuid.MustParse(oldVersionID) + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + done := make(chan struct{}) + go func() { + defer close(done) + app, err := waitForSubmittedVersion(context.Background(), client, testAppID, &previous, time.Millisecond) + if err != nil { + t.Errorf("waitForSubmittedVersion: %v", err) + return + } + if activationMoved(&previous, app.ActiveVersionId) { + t.Errorf("reported an activation for a failed build") + } + }() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("waitForSubmittedVersion did not give up on a failed build") + } +} + +func TestActivationMoved(t *testing.T) { + oldID := uuid.MustParse(oldVersionID) + newID := uuid.MustParse(newVersionID) + cases := []struct { + name string + previous, current *uuid.UUID + want bool + }{ + {name: "pin unchanged", previous: &oldID, current: &oldID}, + {name: "pin moved", previous: &oldID, current: &newID, want: true}, + {name: "first ever activation", current: &newID, want: true}, + {name: "nothing pinned yet", previous: &oldID}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := activationMoved(tc.previous, tc.current); got != tc.want { + t.Errorf("activationMoved() = %v, want %v", got, tc.want) + } + }) + } +} + func TestCompareEndpointSets(t *testing.T) { cases := []struct { name string @@ -108,48 +223,6 @@ func TestReportEndpointSetChangeSaysNothingWhenNothingMoved(t *testing.T) { } } -// TestShouldReportEndpointChange guards the two ways this report can lie: with -// no reading from before the deploy it would call the app's whole existing set -// new, and before the rollout activates it would compare the old set to itself. -func TestShouldReportEndpointChange(t *testing.T) { - cases := []struct { - name string - readBefore bool - status serverlessapi.AppStatus - want bool - }{ - { - name: "read before and rolled out", - readBefore: true, - status: serverlessapi.AppStatusActive, - want: true, - }, - { - name: "the before-read failed, so the whole set would look new", - readBefore: false, - status: serverlessapi.AppStatusActive, - }, - { - name: "the deploy failed, so nothing moved", - readBefore: true, - status: serverlessapi.AppStatusFailed, - }, - { - name: "still rolling out, so the rows are still the old version's", - readBefore: true, - status: serverlessapi.AppStatusInitializing, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if got := shouldReportEndpointChange(tc.readBefore, tc.status); got != tc.want { - t.Errorf("shouldReportEndpointChange(%v, %q) = %v, want %v", - tc.readBefore, tc.status, got, tc.want) - } - }) - } -} - func TestDeployEndpointPaths(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") From a19a9ea8d9ba0a082bb4698467c23d899128f0cb Mon Sep 17 00:00:00 2001 From: Emilio Del Tessandoro Date: Tue, 22 Sep 2026 12:26:57 +0200 Subject: [PATCH 7/7] fix(serverless): capture the whole comparison base or none of it The endpoint set and the active version were read independently, so a failed app read left the pin nil while the comparison still ran. A nil pin is indistinguishable from an app that has never activated a version, which activationMoved counts as a move -- so the wait returned on its first poll, against the outgoing endpoint set, reintroducing exactly the silent miss the wait was added to fix. Both reads now succeed together or the comparison is skipped, which is the same rule the before-read already followed: a failed read costs the report, never turns it into a false one. Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/serverless/deploy.go | 7 +- internal/cmd/serverless/deploy_endpoints.go | 24 +++++++ .../cmd/serverless/deploy_endpoints_test.go | 68 +++++++++++++++++-- 3 files changed, 86 insertions(+), 13 deletions(-) diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index e694664..b87ce33 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -258,12 +258,7 @@ endpoint on purpose is allowed.`, canCompare bool ) if update && wait { - if paths, err := deployEndpointPaths(cmd.Context(), client, id); err == nil { - endpointsBefore, canCompare = paths, true - } - if live, err := client.GetApp(cmd.Context(), id); err == nil { - versionBefore = live.ActiveVersionId - } + endpointsBefore, versionBefore, canCompare = endpointComparisonBase(cmd.Context(), client, id) } var ( diff --git a/internal/cmd/serverless/deploy_endpoints.go b/internal/cmd/serverless/deploy_endpoints.go index c9b3cf9..5b19775 100644 --- a/internal/cmd/serverless/deploy_endpoints.go +++ b/internal/cmd/serverless/deploy_endpoints.go @@ -15,6 +15,30 @@ import ( // contract's maximum, so the common app takes one round trip. const endpointPageLimit = 100 +// endpointComparisonBase reads what a comparison after the deploy needs: the set +// the app serves now, and the version it serves it from. +// +// Both or neither. A nil pin from a failed read is indistinguishable from an app +// that has never activated a version, and activationMoved counts the second as a +// move — so a half-captured base would make waitForSubmittedVersion return on its +// first poll, against the outgoing endpoint set, which is the silent miss this +// whole path exists to avoid. +func endpointComparisonBase( + ctx context.Context, + client *serverlessapi.Client, + appID string, +) (paths []string, pin *uuid.UUID, ok bool) { + paths, err := deployEndpointPaths(ctx, client, appID) + if err != nil { + return nil, nil, false + } + app, err := client.GetApp(ctx, appID) + if err != nil { + return nil, nil, false + } + return paths, app.ActiveVersionId, true +} + // waitForSubmittedVersion polls until the app pins a version other than previous, // and returns the app it saw last. // diff --git a/internal/cmd/serverless/deploy_endpoints_test.go b/internal/cmd/serverless/deploy_endpoints_test.go index 0d89542..fe32a13 100644 --- a/internal/cmd/serverless/deploy_endpoints_test.go +++ b/internal/cmd/serverless/deploy_endpoints_test.go @@ -16,18 +16,19 @@ import ( serverlessapi "github.com/runware/runware-cli/internal/api/serverless" ) -// appBody renders a getApp response pinning activeVersionId, or none when empty. -func appBody(status, activeVersionID string) string { +// activeAppBody renders a getApp response for an active app pinning +// activeVersionId, or pinning none when it is empty. +func activeAppBody(activeVersionID string) string { pin := "null" if activeVersionID != "" { pin = fmt.Sprintf("%q", activeVersionID) } return fmt.Sprintf(`{ - "appId":"my-app","appName":"My App","status":%q,"activeVersionId":%s, + "appId":"my-app","appName":"My App","status":"active","activeVersionId":%s, "configuration":{"maxWorkers":1,"idleTtlSecs":60,"scalingDelaySecs":10,"minWorkers":0,"gpusPerWorker":1,"concurrency":1,"gracefulStopTtlSecs":120,"computeType":"gpu"}, "environmentVariables":[],"secrets":[], "createdAt":"2026-07-30T12:00:00Z","updatedAt":"2026-07-30T12:00:00Z" - }`, status, pin) + }`, pin) } const ( @@ -35,6 +36,59 @@ const ( newVersionID = "019c7654-8b21-7abc-9123-bbbbbbbbbbbb" ) +// TestEndpointComparisonBaseNeedsBothReads: a nil pin from a failed read looks +// exactly like an app that has never activated a version, and activationMoved +// counts that as a move — so half a base would send the comparison straight at +// the outgoing endpoint set rather than waiting for the new one. +func TestEndpointComparisonBaseNeedsBothReads(t *testing.T) { + cases := []struct { + name string + appFail bool + epFail bool + }{ + {name: "both succeed"}, + {name: "the app read fails", appFail: true}, + {name: "the endpoint read fails", epFail: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + endpoints := strings.Contains(r.URL.Path, "/endpoints") + if (endpoints && tc.epFail) || (!endpoints && tc.appFail) { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + if endpoints { + _, _ = w.Write([]byte(`{"data":[{"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","path":"generate"}]}`)) + return + } + _, _ = w.Write([]byte(activeAppBody(oldVersionID))) + })) + defer server.Close() + + client := serverlessapi.NewClient("test-key", server.URL, slog.Default()) + paths, pin, ok := endpointComparisonBase(context.Background(), client, testAppID) + wantOK := !tc.appFail && !tc.epFail + if ok != wantOK { + t.Fatalf("ok = %v, want %v", ok, wantOK) + } + if !ok { + if paths != nil || pin != nil { + t.Errorf("half a base escaped: paths=%v pin=%v", paths, pin) + } + return + } + if pin == nil || pin.String() != oldVersionID { + t.Errorf("pin = %v, want the version the app serves now", pin) + } + if !slices.Equal(paths, []string{testEndpointPath}) { + t.Errorf("paths = %v, want the live set", paths) + } + }) + } +} + // TestWaitForSubmittedVersionWaitsThroughAnActiveBuild is the defect this guard // exists for: a source update on an app that is already active answers `active` // while its new build runs, so a reader keyed on status alone compares the @@ -50,10 +104,10 @@ func TestWaitForSubmittedVersionWaitsThroughAnActiveBuild(t *testing.T) { calls++ // Still rolling the first two times, pinned the third. if calls < 3 { - _, _ = w.Write([]byte(appBody("active", oldVersionID))) + _, _ = w.Write([]byte(activeAppBody(oldVersionID))) return } - _, _ = w.Write([]byte(appBody("active", newVersionID))) + _, _ = w.Write([]byte(activeAppBody(newVersionID))) })) defer server.Close() @@ -81,7 +135,7 @@ func TestWaitForSubmittedVersionGivesUpOnAFailedBuild(t *testing.T) { _, _ = w.Write([]byte(`{"data":[{"id":"019c7654-8b21-7abc-9123-abcdef123456","appId":"my-app","status":"failed","phases":[]}]}`)) return } - _, _ = w.Write([]byte(appBody("active", oldVersionID))) + _, _ = w.Write([]byte(activeAppBody(oldVersionID))) })) defer server.Close()