From bcf00918e5acad91201994294e51059f4ec2f7b8 Mon Sep 17 00:00:00 2001 From: E Ismail Date: Sat, 10 Oct 2026 18:35:32 +0300 Subject: [PATCH] flow: an answer with no code says whether waiting will help A refusal whose body is not the wire envelope carries no code. The service's own handlers always write the envelope, so such an answer came from something else: a proxy in front of the service while it restarts, a router answering for a route it does not serve, a captive portal, or an address that points elsewhere. It was answered as a code this build does not know: it gave up, said updating curious may help, and quoted the client's own "could not parse" sentence as the server's words. Each of the six handler switches now meets the empty code in a case of its own, and the HTTP status decides between two new answers: - server-error-unexplained (Wait) for any 5xx, 408, 425 and 429, which pass on their own; - server-request-unserved (GiveUp) for every other status, including a redirect, which curious never follows. It says what to check. Neither quotes anything or claims the build is older or newer than the server. Each site says what it can and cannot tell about what had already happened: a publish or a start may have been acted on, a login code may be spent, and a build log answered this way may mean the build ended with the service. The build log also gains a case for internal, the server's own fault while reading the deploy: server-fault, Wait, the message quoted. The dispatch guard now requires every handler switch to answer the empty code with the two new answers, and the shared answer's own empty-code branch and its exception are gone. The catalog and the README move with the ids. The test servers gain a refusal with no envelope, honoured everywhere a scripted answer is written or tested, and the build-log server can refuse before it streams. --- README.md | 64 +++++ catalog.json | 46 ++++ internal/flow/capacity.go | 12 + internal/flow/codeless.go | 96 +++++++ internal/flow/codeless_test.go | 347 +++++++++++++++++++++++++ internal/flow/create.go | 12 + internal/flow/deploy_test.go | 21 +- internal/flow/login.go | 21 ++ internal/flow/login_test.go | 39 ++- internal/flow/publish.go | 14 + internal/flow/stream.go | 53 +++- internal/flow/unrecognised.go | 11 +- internal/flow/unrecognised_test.go | 19 +- internal/flow/waitlist_test.go | 3 + internal/guard/codedispatch_test.go | 111 +++++++- internal/guard/failurecensus_test.go | 5 + internal/guard/failurecontract_test.go | 3 +- internal/mcp/codeless_test.go | 37 +++ internal/mcp/tools_test.go | 7 + internal/ui/failureid.go | 8 +- 20 files changed, 900 insertions(+), 29 deletions(-) create mode 100644 internal/flow/codeless.go create mode 100644 internal/flow/codeless_test.go create mode 100644 internal/mcp/codeless_test.go diff --git a/README.md b/README.md index e5ad599..ba36be8 100644 --- a/README.md +++ b/README.md @@ -685,12 +685,46 @@ inside it. > curious couldn't finish logging you in. +### server-error-unexplained + +**addresses.** A person is waiting for a finished deploy to be given its public address. + +> The server wouldn't give this deploy an address. + +**build log.** A person is watching the build log stream in while the build runs. + +> The server wouldn't send the build log. + +**building.** A person is waiting while the server builds the project and decides whether to take the result. + +> The server wouldn't start the build. + +**deploys.** A person is starting a deploy: curious is asking whether there is room today and for somewhere to upload the project. + +> curious couldn't start the deploy. + +**logins.** A person is logging in, or curious is saving the login they just completed. + +> curious couldn't finish logging you in. + +The answer carried no error code, so something in front of the service +wrote it rather than the service itself, and curious cannot tell what is +behind it: that is what "unexplained" means here. The message shows the +HTTP status, which is a 5xx, a 408, a 425 or a 429, and answers like these +usually pass on their own, for example while the service restarts. Try +again in a moment. Where the step may already have happened, the message +says what curious cannot tell. + ### server-fault **addresses.** A person is waiting for a finished deploy to be given its public address. > The server hit a problem finishing the deploy. +**build log.** A person is watching the build log stream in while the build runs. + +> The server hit a problem sending the build log. + **building.** A person is waiting while the server builds the project and decides whether to take the result. > The server hit a problem starting the build. @@ -699,6 +733,36 @@ inside it. > The server hit a problem starting the deploy. +### server-request-unserved + +**addresses.** A person is waiting for a finished deploy to be given its public address. + +> The server wouldn't give this deploy an address. + +**build log.** A person is watching the build log stream in while the build runs. + +> The server wouldn't send the build log. + +**building.** A person is waiting while the server builds the project and decides whether to take the result. + +> The server wouldn't start the build. + +**deploys.** A person is starting a deploy: curious is asking whether there is room today and for somewhere to upload the project. + +> curious couldn't start the deploy. + +**logins.** A person is logging in, or curious is saving the login they just completed. + +> curious couldn't finish logging you in. + +The answer carried no error code, and its HTTP status is one that does +not change by waiting: a 404 or a 405 for a request the address does not +serve, or a redirect, which curious never follows. Something other than +the service may be answering for the address. Check `CURIOUS_API_URL`, or +unset it to use the default, and anything between this machine and the +service, such as a proxy. If both are right, please report it with the +output of `curious version`. + ### server-unanswered **addresses.** A person is waiting for a finished deploy to be given its public address. diff --git a/catalog.json b/catalog.json index d1901e4..12cc4da 100644 --- a/catalog.json +++ b/catalog.json @@ -697,6 +697,28 @@ "logins" ] }, + { + "action": [ + "Wait" + ], + "family": null, + "headline": { + "addresses": "The server wouldn't give this deploy an address.", + "build log": "The server wouldn't send the build log.", + "building": "The server wouldn't start the build.", + "deploys": "curious couldn't start the deploy.", + "logins": "curious couldn't finish logging you in." + }, + "headline_reason": {}, + "id": "server-error-unexplained", + "stages": [ + "addresses", + "build log", + "building", + "deploys", + "logins" + ] + }, { "action": [ "Wait" @@ -704,6 +726,7 @@ "family": null, "headline": { "addresses": "The server hit a problem finishing the deploy.", + "build log": "The server hit a problem sending the build log.", "building": "The server hit a problem starting the build.", "deploys": "The server hit a problem starting the deploy." }, @@ -711,10 +734,33 @@ "id": "server-fault", "stages": [ "addresses", + "build log", "building", "deploys" ] }, + { + "action": [ + "GiveUp" + ], + "family": null, + "headline": { + "addresses": "The server wouldn't give this deploy an address.", + "build log": "The server wouldn't send the build log.", + "building": "The server wouldn't start the build.", + "deploys": "curious couldn't start the deploy.", + "logins": "curious couldn't finish logging you in." + }, + "headline_reason": {}, + "id": "server-request-unserved", + "stages": [ + "addresses", + "build log", + "building", + "deploys", + "logins" + ] + }, { "action": [ "Wait" diff --git a/internal/flow/capacity.go b/internal/flow/capacity.go index 92f604c..7188ee8 100644 --- a/internal/flow/capacity.go +++ b/internal/flow/capacity.go @@ -175,6 +175,18 @@ func capacityCheckFailure(err error) error { apiErr.Message, ui.NextWait, "Try again in a moment. "+uploadedNothing) + case "": + // No code: something in front of the service answered, after the + // client had already asked three times if the status was a 5xx. + // The check is a read, so nothing has happened either way. The + // headline is the create's, for the reason given below. + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageDeploys, "curious couldn't start the deploy.", + apiErr, uploadedNothing) + } + return requestUnserved(ui.StageDeploys, "curious couldn't start the deploy.", + apiErr, uploadedNothing) + default: // The endpoint answers with the two codes above and no other, so // anything else is a code this step has no copy for. Its headline diff --git a/internal/flow/codeless.go b/internal/flow/codeless.go new file mode 100644 index 0000000..50810a8 --- /dev/null +++ b/internal/flow/codeless.go @@ -0,0 +1,96 @@ +package flow + +import ( + "fmt" + "net/http" + + "github.com/curiouspub/cli/internal/api" + "github.com/curiouspub/cli/internal/ui" +) + +// A REFUSAL WITH NO CODE is a body that was not the wire envelope at all, +// or an envelope with an empty code. The service's own handlers always +// write the envelope, so an answer without one came from somewhere else: a +// proxy in front of the service while the service restarts or is down, a +// router answering for a route or a method it does not serve, a captive +// portal, or a CURIOUS_API_URL that points somewhere else. +// +// It is not a code this build has no copy for, so it does not get that +// answer. Updating curious is the remedy for few of the causes above, and +// nothing in the answer says which one it was. The client's own sentence +// about an answer it could not read is not the server's, so nothing is +// quoted. +// +// WHAT CAN BE TOLD IS THE STATUS, and it decides the remedy. An answer that +// passes on its own says to wait. One that does not says what to check, +// and claims neither that this build is older than the server nor newer. +// Each handler meets the empty code in a `case "":` of its own switch, +// passes what it can honestly say about what had already happened, and a +// guard holds every handler to it. + +// clearsOnItsOwn reports whether a code-less answer is one that passes +// without anything outside the run changing: any 5xx, and the three 4xx +// statuses that say "not now" rather than "not this". +// +// Everything else does not pass by waiting. A 404 or a 405 is a router +// that does not serve the request, and a 1xx or 3xx arrives here only +// because curious never follows a redirect: something else is answering +// for the address. +func clearsOnItsOwn(apiErr *api.APIError) bool { + switch apiErr.Status { + case http.StatusRequestTimeout, http.StatusTooEarly, http.StatusTooManyRequests: + return true + } + return apiErr.Status >= http.StatusInternalServerError +} + +const ( + // whyUnexplained and whyUnserved are the fixed half of each answer's + // Why, with the status in it. They are constants at the construction + // so that a setting either one names is visible there. + whyUnexplained = "The server answered HTTP %d, in a form curious could not read as an\n" + + "answer. Answers like this usually pass on their own." + whyUnserved = "The server answered HTTP %d, in a form curious could not read as an\n" + + "answer, and sending the same request again will not change that. The\n" + + "address may not be the curious.pub service, something in between may be\n" + + "answering for it, or the service may not serve this request. The address\n" + + "comes from CURIOUS_API_URL." + + tryAgainUnexplained = "Try again in a moment. If it keeps happening, check anything between\n" + + "this machine and the service, such as a proxy." + checkTheAddress = "Check CURIOUS_API_URL, or unset it to use the default, and anything\n" + + "between this machine and the service, such as a proxy. If both are\n" + + "right, please report this with the output of `curious version`." +) + +// errorUnexplained is the answer to a code-less refusal that passes on +// its own. The caller says where the person was, what did not happen, and +// what it can and cannot tell about what had already happened. +func errorUnexplained(stage ui.Stage, what string, apiErr *api.APIError, situation string) *ui.Failure { + return ui.NewFailure( + ui.IDServerErrorUnexplained, + stage, + what, + fmt.Sprintf(whyUnexplained, apiErr.Status)+thenSituation(situation), ui.NextWait, + tryAgainUnexplained) +} + +// requestUnserved is the answer to a code-less refusal that does not pass +// by waiting. It names what can be checked from this machine. +func requestUnserved(stage ui.Stage, what string, apiErr *api.APIError, situation string) *ui.Failure { + return ui.NewFailure( + ui.IDServerRequestUnserved, + stage, + what, + fmt.Sprintf(whyUnserved, apiErr.Status)+thenSituation(situation), ui.NextGiveUp, + checkTheAddress) +} + +// thenSituation is the caller's paragraph about what had already +// happened, or nothing when it has none. +func thenSituation(situation string) string { + if situation == "" { + return "" + } + return "\n\n" + situation +} diff --git a/internal/flow/codeless_test.go b/internal/flow/codeless_test.go new file mode 100644 index 0000000..ab2b60e --- /dev/null +++ b/internal/flow/codeless_test.go @@ -0,0 +1,347 @@ +package flow + +import ( + "context" + "errors" + "net/http" + "strconv" + "strings" + "testing" + "time" + + "github.com/curiouspub/cli/internal/api" + "github.com/curiouspub/cli/internal/ui" + "github.com/curiouspub/cli/pkg/wire" +) + +// A REFUSAL WITH NO CODE is a body that was not the wire envelope at all. +// The service always writes the envelope, so one without it was written by +// something else: a proxy in front of the service while it restarts, or a +// router answering for a route it does not serve. Which of those it was is +// in the status, and that decides whether waiting can help. +// +// These rows used to be answered as a code this build does not know, which +// said "Updating curious may help" and gave up. A proxy's 502 passes on its +// own, and updating changes nothing about it. + +const ( + idErrorUnexplained = ui.FailureID("server-error-unexplained") + idRequestUnserved = ui.FailureID("server-request-unserved") + + // codelessMessage is what the client itself puts in an answer it could + // not read. It is not the server's, so it must never be quoted as if it + // were. + codelessMessage = "the server returned an error this client could not parse" + + unexplainedNext = "Try again in a moment. If it keeps happening, check anything between\n" + + "this machine and the service, such as a proxy." + unservedNext = "Check CURIOUS_API_URL, or unset it to use the default, and anything\n" + + "between this machine and the service, such as a proxy. If both are\n" + + "right, please report this with the output of `curious version`." + + wantLoginMayHaveActed = "The server may have acted before this answer arrived, so a code already\n" + + "entered may be spent. Start the login again for a new code; sending this\n" + + "code again may count as a wrong attempt." + wantBuildLogCannotTell = "curious cannot tell from this answer whether the build is still running." + wantBuildNotAffected = "The build itself is not affected: it runs on the server, and this was\n" + + "only the window onto it." + wantPublishCannotTell = "The deploy may or may not have got an address; curious cannot tell\n" + + "from this answer.\n\nThe deploy is dpl-abc." +) + +func codeless(status int) *api.APIError { + return &api.APIError{Status: status, Message: codelessMessage} +} + +func unexplainedWhy(status string) string { + return "The server answered HTTP " + status + ", in a form curious could not read as an\n" + + "answer. Answers like this usually pass on their own." +} + +func unservedWhy(status string) string { + return "The server answered HTTP " + status + ", in a form curious could not read as an\n" + + "answer, and sending the same request again will not change that. The\n" + + "address may not be the curious.pub service, something in between may be\n" + + "answering for it, or the service may not serve this request. The address\n" + + "comes from CURIOUS_API_URL." +} + +// codelessSite is one of the six handlers, met with a given answer. +type codelessSite struct { + name string + stop func(*api.APIError) error + stage ui.Stage + wantWhat string +} + +func codelessSites() []codelessSite { + now := time.Date(2026, 9, 12, 12, 0, 0, 0, time.UTC) + return []codelessSite{ + {"at the capacity check", func(e *api.APIError) error { return capacityCheckFailure(e) }, + ui.StageDeploys, "curious couldn't start the deploy."}, + {"at the create", func(e *api.APIError) error { return createStopFailure(e, now) }, + ui.StageDeploys, "curious couldn't start the deploy."}, + {"at the login", func(e *api.APIError) error { + return stopFailure(context.Background(), e, LoginDeps{Endpoint: "https://api.example"}, + "someone@example.com", now) + }, ui.StageLogins, "curious couldn't finish logging you in."}, + {"at the publish", func(e *api.APIError) error { return publishStopFailure(e, "dpl-abc", now) }, + ui.StageAddresses, "The server wouldn't give this deploy an address."}, + {"at the start", func(e *api.APIError) error { return startFailure(e, now) }, + ui.StageBuilding, "The server wouldn't start the build."}, + {"at the build log", func(e *api.APIError) error { return streamRefusedFailure(e) }, + ui.StageBuildLog, "The server wouldn't send the build log."}, + } +} + +// assertCodeless holds every part of a code-less answer that does not +// depend on the site. +func assertCodeless(t *testing.T, err error, id ui.FailureID, next ui.NextAction, nextText string) *ui.Failure { + t.Helper() + var f *ui.Failure + if !errors.As(err, &f) { + t.Fatalf("the stop is a %T", err) + } + if f.ID != id { + t.Errorf("id = %q, want %q", f.ID, id) + } + if f.Next != next { + t.Errorf("action = %q, want %q", f.Next, next) + } + if f.NextText != nextText { + t.Errorf("next-step copy = %q, want %q", f.NextText, nextText) + } + // NOTHING IS QUOTED. The only sentence an answer like this carries is + // the client's own, and quoting it would put it in the server's mouth. + if f.Detail != "" { + t.Errorf("the failure quotes %q as the server's words; the server sent none", f.Detail) + } + out := rendered(err) + for _, never := range []string{"older than the server", `the code "`, codelessMessage} { + if strings.Contains(out, never) { + t.Errorf("a refusal with no code says %q:\n%s", never, out) + } + } + if code := exitCodeFor(t, err); code != 1 { + t.Errorf("exit code %d, want 1: this is a failure of the run, not a closed door", code) + } + return f +} + +// TestACodelessAnswerThatCanClearSaysWait. A 502 is what the proxy in front +// of the service sends while the service restarts, and it passes. Each site +// says what it can and cannot tell about what happened before it. +// +// REQUIRED MUTATIONS, run: +// - delete the six `case "":` clauses, so a code-less answer falls to the +// shared answer for a code this build has no copy for. Every site reds. +// - make the status predicate never answer "clears". Every site reds. +func TestACodelessAnswerThatCanClearSaysWait(t *testing.T) { + situation := map[string]string{ + "at the capacity check": uploadedNothing, + "at the create": uploadedNothing, + "at the login": wantLoginMayHaveActed, + "at the publish": wantPublishCannotTell, + "at the start": "The build may be running anyway — an answer like this can arrive after\n" + + "the server acted.", + "at the build log": wantBuildLogCannotTell, + } + for _, site := range codelessSites() { + t.Run(site.name, func(t *testing.T) { + err := site.stop(codeless(http.StatusBadGateway)) + f := assertCodeless(t, err, idErrorUnexplained, ui.NextWait, unexplainedNext) + if f.Stage != site.stage || f.What != site.wantWhat { + t.Errorf("stage and headline = %q, %q; want %q, %q", f.Stage, f.What, site.stage, site.wantWhat) + } + if want := unexplainedWhy("502") + "\n\n" + situation[site.name]; f.Why != want { + t.Errorf("why = %q,\nwant %q", f.Why, want) + } + }) + } +} + +// TestACodelessAnswerThatCannotClearGivesUp. A 404 or a 405 is a router +// answering for a route or a method it does not serve, and a redirect is +// something else answering for the address. Neither passes by waiting. +// +// REQUIRED MUTATION, run: make the status predicate always answer "clears". +// Every site reds. +func TestACodelessAnswerThatCannotClearGivesUp(t *testing.T) { + situation := map[string]string{ + "at the capacity check": uploadedNothing, + "at the create": uploadedNothing, + "at the publish": wantPublishCannotTell, + "at the build log": wantBuildNotAffected, + } + for _, site := range codelessSites() { + t.Run(site.name, func(t *testing.T) { + err := site.stop(codeless(http.StatusNotFound)) + f := assertCodeless(t, err, idRequestUnserved, ui.NextGiveUp, unservedNext) + if f.Stage != site.stage || f.What != site.wantWhat { + t.Errorf("stage and headline = %q, %q; want %q, %q", f.Stage, f.What, site.stage, site.wantWhat) + } + want := unservedWhy("404") + if s := situation[site.name]; s != "" { + want += "\n\n" + s + } + if f.Why != want { + t.Errorf("why = %q,\nwant %q", f.Why, want) + } + }) + } + // A REDIRECT. curious never follows one, so a captive portal's 307 + // arrives here with no code. + t.Run("a redirect, at the capacity check", func(t *testing.T) { + err := capacityCheckFailure(codeless(http.StatusTemporaryRedirect)) + f := assertCodeless(t, err, idRequestUnserved, ui.NextGiveUp, unservedNext) + if want := unservedWhy("307") + "\n\n" + uploadedNothing; f.Why != want { + t.Errorf("why = %q,\nwant %q", f.Why, want) + } + }) +} + +// TestTheStatusDecidesWhetherACodelessAnswerCanClear pins the edges of the +// split at one site, the create. A 408, a 425 and a 429 pass on their own +// like any 5xx; every other status does not. +// +// REQUIRED MUTATIONS, run: make the status predicate always answer +// "clears" (the GiveUp rows red), and never (the Wait rows red). +func TestTheStatusDecidesWhetherACodelessAnswerCanClear(t *testing.T) { + now := time.Date(2026, 9, 12, 12, 0, 0, 0, time.UTC) + for _, tc := range []struct { + status int + want ui.FailureID + }{ + {http.StatusRequestTimeout, idErrorUnexplained}, + {http.StatusTooEarly, idErrorUnexplained}, + {http.StatusTooManyRequests, idErrorUnexplained}, + {http.StatusInternalServerError, idErrorUnexplained}, + {http.StatusFound, idRequestUnserved}, + {http.StatusMethodNotAllowed, idRequestUnserved}, + {http.StatusConflict, idRequestUnserved}, + {499, idRequestUnserved}, + } { + t.Run(http.StatusText(tc.status)+" "+strconv.Itoa(tc.status), func(t *testing.T) { + var f *ui.Failure + if err := createStopFailure(codeless(tc.status), now); !errors.As(err, &f) { + t.Fatalf("the stop is a %T", err) + } + if f.ID != tc.want { + t.Errorf("HTTP %d at the create: id = %q, want %q", tc.status, f.ID, tc.want) + } + }) + } +} + +// TestACodelessAnswerAtTheCapacityCheckIsRetriedThenSaysWait drives the +// real client, so its retry is in the row: the capacity check is the one +// call of the six that retries, and it retries a 5xx and nothing else. +func TestACodelessAnswerAtTheCapacityCheckIsRetriedThenSaysWait(t *testing.T) { + t.Run("an empty 502", func(t *testing.T) { + run := newGateRun(t) + run.script.capacityOutcome = bare(http.StatusBadGateway, "", "") + err := run.run(t) + assertCodeless(t, err, idErrorUnexplained, ui.NextWait, unexplainedNext) + if got := run.script.checks(); got != 3 { + t.Errorf("the check was made %d times, want 3: a 5xx is retried twice", got) + } + }) + t.Run("a text/plain 405", func(t *testing.T) { + run := newGateRun(t) + run.script.capacityOutcome = bare(http.StatusMethodNotAllowed, + "text/plain; charset=utf-8", "Method Not Allowed\n") + err := run.run(t) + assertCodeless(t, err, idRequestUnserved, ui.NextGiveUp, unservedNext) + if got := run.script.checks(); got != 1 { + t.Errorf("the check was made %d times, want 1: a 4xx is not retried", got) + } + }) +} + +// TestACodelessAnswerAtTheLoginStopsAndSaysWait drives the two login calls +// through the real client and the routing they share, which the six-site +// row bypasses. Neither call is repeated by the client, and a verify the +// server acted on has spent its code, so the copy says to start again. +func TestACodelessAnswerAtTheLoginStopsAndSaysWait(t *testing.T) { + for _, tc := range []struct { + name string + call func(run *loginRun) *LoginRefusal + }{ + {"asking for a code", func(run *loginRun) *LoginRefusal { + run.script.startOutcomes = []outcome{bare(http.StatusBadGateway, "", "")} + return LoginStart(t.Context(), run.deps, "someone@example.com") + }}, + {"sending the code", func(run *loginRun) *LoginRefusal { + run.script.verifyOutcomes = []outcome{bare(http.StatusBadGateway, "", "")} + return LoginVerify(t.Context(), run.deps, VerifyRequest{ + Email: "someone@example.com", Code: "123456", + }) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + run := newLoginRun(t) + run.deps.Prompt = nil + run.deps.Offer = nil + refusal := tc.call(run) + if refusal == nil { + t.Fatal("the call succeeded against a server that refused it") + } + if refusal.Recoverable { + t.Errorf("a refusal with no code was offered as recoverable, said %q", refusal.Said) + } + f := assertCodeless(t, refusal.Failure, idErrorUnexplained, ui.NextWait, unexplainedNext) + if !strings.Contains(f.Why, wantLoginMayHaveActed) { + t.Errorf("the login's failure does not say a code may be spent:\n%s", f.Why) + } + }) + } +} + +// TestTheBuildLogsInternalSaysWait. The events route answers `internal` +// when the server fails while reading the deploy, and its own message says +// to try again. It is the start's answer to the same code, at the build log. +// +// REQUIRED MUTATION, run: delete streamRefusedFailure's internal case. The +// refusal falls to the shared answer and reds. +func TestTheBuildLogsInternalSaysWait(t *testing.T) { + const serverSaid = "internal error" + run := newDeployRun(t, fixtureProject(t, "valid")).scriptedLogin() + run.prompt.confirms = []answer{no()} + run.script.eventScripts = []eventScript{{ + refuse: fails(http.StatusInternalServerError, wire.CodeInternal, serverSaid), + }} + + handoff, err := run.run() + defer handoff.Release() + var f *ui.Failure + if !errors.As(err, &f) { + t.Fatalf("the stop is a %T: %v", err, err) + } + if f.ID != ui.IDServerFault || f.Next != ui.NextWait { + t.Errorf("id and action = %q, %q; want %q, %q", f.ID, f.Next, ui.IDServerFault, ui.NextWait) + } + if f.Stage != ui.StageBuildLog || f.What != "The server hit a problem sending the build log." { + t.Errorf("stage and headline = %q, %q", f.Stage, f.What) + } + if f.Detail != serverSaid { + t.Errorf("the server's message is %q here, want it verbatim: %q", f.Detail, serverSaid) + } + if want := "Try again in a moment: run `curious deploy` again.\n" + wantBuildNotAffected; f.NextText != want { + t.Errorf("next-step copy = %q,\nwant %q", f.NextText, want) + } +} + +// TestTheBuildLogsRateLimitStaysWithTheSharedAnswer is a control: the +// events route is not rate limited, so a rate limit there is a code this +// step has no copy for, and the build log's new case is for internal only. +func TestTheBuildLogsRateLimitStaysWithTheSharedAnswer(t *testing.T) { + var f *ui.Failure + err := streamRefusedFailure(&api.APIError{Status: http.StatusTooManyRequests, + Code: wire.CodeRateLimited, Message: "slow down"}) + if !errors.As(err, &f) { + t.Fatalf("the stop is a %T", err) + } + if f.ID != ui.IDServerAnswerUnrecognised { + t.Errorf("id = %q, want %q", f.ID, ui.IDServerAnswerUnrecognised) + } +} diff --git a/internal/flow/create.go b/internal/flow/create.go index 28b72fa..07a26dd 100644 --- a/internal/flow/create.go +++ b/internal/flow/create.go @@ -273,6 +273,18 @@ func createStopFailure(apiErr *api.APIError, now time.Time) error { "which one — and please report this if it keeps happening. "+ uploadedNothing).Quoting(apiErr.Message) + case "": + // No code: something in front of the service answered. The server + // may have created a deploy before it did, but the upload starts + // only after a create succeeds, so nothing was uploaded either way, + // and an unused deploy is discarded on its own. + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageDeploys, "curious couldn't start the deploy.", + apiErr, uploadedNothing) + } + return requestUnserved(ui.StageDeploys, "curious couldn't start the deploy.", + apiErr, uploadedNothing) + default: // Routed to a stop with no copy of its own, or a code this build // predates. The contract is additive-only, so the server is diff --git a/internal/flow/deploy_test.go b/internal/flow/deploy_test.go index eaca732..f334226 100644 --- a/internal/flow/deploy_test.go +++ b/internal/flow/deploy_test.go @@ -352,6 +352,12 @@ type eventScript struct { // the test ends or the client goes away: the server the transport's // bound on answering exists for. neverAnswer bool + + // refuse answers this connection with a refusal instead of the + // stream: the status and body are written before any event-stream + // header, which is the only point at which the route can still say + // no with a status. + refuse outcome } // eventsPathSuffix and startPathSuffix name the two per-deploy endpoints @@ -435,6 +441,12 @@ func (s *deployScript) serveEvents(w http.ResponseWriter, r *http.Request, scrip } } + if script.refuse.refuses() { + s.reply(w, script.refuse, nil) + mark("refused") + return + } + w.Header().Set("Content-Type", "text/event-stream") w.WriteHeader(http.StatusOK) flusher, canFlush := w.(http.Flusher) @@ -693,7 +705,7 @@ func (s *deployScript) servePublish(w http.ResponseWriter, r *http.Request) { `this deploy is "building" and cannot be given an address yet`), nil) return } - if s.publishOutcome.code != "" { + if s.publishOutcome.refuses() { s.reply(w, s.publishOutcome, nil) return } @@ -761,7 +773,7 @@ func (s *deployScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { panic("this server cannot drop a connection, so the row that needs one " + "would silently be measuring something else") } - if s.startOutcome.code != "" { + if s.startOutcome.refuses() { s.reply(w, s.startOutcome, nil) return } @@ -800,7 +812,7 @@ func (s *deployScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { "this request was not authenticated"), nil) return } - if s.deployOutcome.code != "" { + if s.deployOutcome.refuses() { s.reply(w, s.deployOutcome, nil) return } @@ -815,6 +827,9 @@ func (s *deployScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func (s *deployScript) reply(w http.ResponseWriter, o outcome, success any) { + if writeBare(w, o) { + return + } if o.code == "" { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) diff --git a/internal/flow/login.go b/internal/flow/login.go index da4903b..5f5b259 100644 --- a/internal/flow/login.go +++ b/internal/flow/login.go @@ -688,6 +688,13 @@ func classify(ctx context.Context, err error, deps LoginDeps, email string, now return routeStop, "", stopFailure(ctx, apiErr, deps, email, now) } +// loginMayHaveActed is what a login answered without a code can and cannot +// tell. It names no command, because the fresh login is a fresh run at a +// terminal and a call to ask for a code elsewhere. +const loginMayHaveActed = "The server may have acted before this answer arrived, so a code already\n" + + "entered may be spent. Start the login again for a new code; sending this\n" + + "code again may count as a wrong attempt." + // stopFailure is the copy for each code that ends the run. func stopFailure(ctx context.Context, apiErr *api.APIError, deps LoginDeps, email string, now time.Time) error { switch apiErr.Code { @@ -767,6 +774,20 @@ func stopFailure(ctx context.Context, apiErr *api.APIError, deps LoginDeps, emai deps.Endpoint), ui.NextGiveUp, "Check CURIOUS_API_URL, or unset it to use the default.") + case "": + // No code: something in front of the service answered. Neither + // login call is repeated, and either may have been acted on before + // the answer came back: a verify that was has spent its code, and + // sending it again counts against the person as a wrong one. So + // the answer that passes says to start again; the one that does + // not means the request was never served, and says nothing of it. + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageLogins, "curious couldn't finish logging you in.", + apiErr, loginMayHaveActed) + } + return requestUnserved(ui.StageLogins, "curious couldn't finish logging you in.", + apiErr, "") + default: // Routed to a stop by the table above without copy of its own, or // a code this binary predates. It is deliberately the same shape diff --git a/internal/flow/login_test.go b/internal/flow/login_test.go index a9a2488..3878c48 100644 --- a/internal/flow/login_test.go +++ b/internal/flow/login_test.go @@ -218,18 +218,52 @@ func (s *scriptedPrompt) asked(question string) int { return n } -// outcome is one scripted HTTP answer. A zero Code means success. +// outcome is one scripted HTTP answer. A zero outcome means success. type outcome struct { status int code wire.ErrorCode message string retryAfter string + + // bareStatus answers with this status and a body that is NOT the wire + // envelope: bareBody under bareType, or nothing at all. It is what a + // proxy in front of the service, or a router answering for a route it + // does not serve, sends. It wins over code everywhere an outcome is + // rendered or tested, because a refusal with no code would otherwise + // read as success and answer 200. + bareStatus int + bareType string + bareBody string } func fails(status int, code wire.ErrorCode, message string) outcome { return outcome{status: status, code: code, message: message} } +// bare is a refusal that carries no envelope and so no code. +func bare(status int, contentType, body string) outcome { + return outcome{bareStatus: status, bareType: contentType, bareBody: body} +} + +// refuses reports whether the outcome is a refusal of either kind. +func (o outcome) refuses() bool { + return o.bareStatus != 0 || o.code != "" +} + +// writeBare writes a bare outcome and reports whether it did. Every +// renderer of an outcome calls it first. +func writeBare(w http.ResponseWriter, o outcome) bool { + if o.bareStatus == 0 { + return false + } + if o.bareType != "" { + w.Header().Set("Content-Type", o.bareType) + } + w.WriteHeader(o.bareStatus) + _, _ = w.Write([]byte(o.bareBody)) + return true +} + func (o outcome) after(retryAfter string) outcome { o.retryAfter = retryAfter return o @@ -277,6 +311,9 @@ func (s *apiScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func (s *apiScript) reply(w http.ResponseWriter, o outcome, success any) { + if writeBare(w, o) { + return + } if o.code == "" { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) diff --git a/internal/flow/publish.go b/internal/flow/publish.go index b31cfbe..55cdfeb 100644 --- a/internal/flow/publish.go +++ b/internal/flow/publish.go @@ -595,6 +595,20 @@ func publishStopFailure(apiErr *api.APIError, deployID string, now time.Time) er ui.Written(nothingDeployed+"\n\nThe deploy is %s.", deployID), ui.NextWait, "Try again in a moment: run `curious deploy` again.").Quoting(apiErr.Message) + case "": + // No code: something in front of the service answered, and it + // says nothing about whether the service acted first. The publish + // is not repeated, so this claims neither outcome, whichever the + // status. + cannotTell := ui.Written("The deploy may or may not have got an address; curious cannot tell\n"+ + "from this answer.\n\nThe deploy is %s.", deployID) + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageAddresses, "The server wouldn't give this deploy an address.", + apiErr, cannotTell) + } + return requestUnserved(ui.StageAddresses, "The server wouldn't give this deploy an address.", + apiErr, cannotTell) + default: // Routed to a stop with no copy of its own, or a code this build // predates. The contract is additive-only, so the server is diff --git a/internal/flow/stream.go b/internal/flow/stream.go index d7dca88..162917d 100644 --- a/internal/flow/stream.go +++ b/internal/flow/stream.go @@ -141,6 +141,12 @@ const ( // answered. A failure after the request left this process is // indistinguishable from one before it. buildMayBeRunning = "The build may be running anyway" + + // buildNotAffected is said only where the service itself answered the + // build log and refused it, so the process running the build is known + // to be up. + buildNotAffected = "The build itself is not affected: it runs on the server, and this was\n" + + "only the window onto it." ) // errStreamStalled is the cause the watchdog cancels with, so the branch @@ -334,9 +340,10 @@ func streamBuild(ctx context.Context, deps streamDeps) (wire.DoneEvent, error) { lastErr = err // A REFUSAL THE SERVER ANSWERED IS NOT A DROPPED CONNECTION. - // It arrived, it was decoded, and it will say the same thing next - // time — so it stops the run rather than spending the budget a - // broken connection needs. + // It arrived and it was decoded, so it stops the run rather than + // spending the budget a broken connection needs. Whether a + // refusal with no code, which something in front of the service + // sent, should reconnect instead is a separate decision. var apiErr *api.APIError if errors.As(err, &apiErr) { return wire.DoneEvent{}, streamRefusedFailure(apiErr) @@ -795,6 +802,16 @@ func startFailure(err error, now time.Time) error { "Try again in a moment. The archive was uploaded, and nothing has\n"+ "been built or deployed.") + case "": + // No code: something in front of the service answered. One that + // passes may have come after the service started the build; one + // that does not means the request was never served. + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageBuilding, "The server wouldn't start the build.", apiErr, + buildMayBeRunning+" — an answer like this can arrive after\nthe server acted.") + } + return requestUnserved(ui.StageBuilding, "The server wouldn't start the build.", apiErr, "") + default: // EVERY OTHER CODE ENDS THE RUN THE SAME WAY, so there is no // routing table here: one would have a single column. What the @@ -806,8 +823,9 @@ func startFailure(err error, now time.Time) error { } // streamRefusedFailure is what a stream the server REFUSED ends the run -// as. It is deliberately not a reconnection: the answer arrived, it was -// decoded, and it will say the same thing next time. +// as. A refusal stops the run rather than reconnecting: the answer +// arrived and it was decoded. Whether a refusal with no code should +// reconnect instead is a separate decision. func streamRefusedFailure(apiErr *api.APIError) error { switch apiErr.Code { case wire.CodeMaintenance: @@ -818,6 +836,31 @@ func streamRefusedFailure(apiErr *api.APIError) error { apiErr.Message, ui.NextWait, "Try again a little later.")) + case wire.CodeInternal: + // The events route answers this when the server fails while + // reading the deploy, and its own message says to try again. The + // service answered, so the build it runs is unaffected. + return ui.Quoted( + ui.IDServerFault, + ui.StageBuildLog, + "The server hit a problem sending the build log.", + apiErr.Message, ui.NextWait, + "Try again in a moment: run `curious deploy` again.\n"+buildNotAffected) + + case "": + // No code: something in front of the service answered. Here that + // usually means the service restarted, and the build may have + // ended with it, so the answer that passes claims nothing about + // the build. The one that does not is a request that was not + // served, by an answerer that is up, and says nothing against the + // build. + if clearsOnItsOwn(apiErr) { + return errorUnexplained(ui.StageBuildLog, "The server wouldn't send the build log.", apiErr, + "curious cannot tell from this answer whether the build is still running.") + } + return requestUnserved(ui.StageBuildLog, "The server wouldn't send the build log.", apiErr, + buildNotAffected) + default: return unrecognisedAnswer(ui.StageBuildLog, "The server wouldn't send the build log.", apiErr, "") } diff --git a/internal/flow/unrecognised.go b/internal/flow/unrecognised.go index 206a55a..ee7bffb 100644 --- a/internal/flow/unrecognised.go +++ b/internal/flow/unrecognised.go @@ -32,16 +32,13 @@ const updatingMayHelp = "Updating curious may help: this build may be older than // // Every switch over a code ends here, and a guard holds them to it. The // caller says where the person was and what did not happen; this says -// everything else, in one place. +// everything else, in one place. An answer with no code at all never +// reaches it: each switch meets that in a case of its own, because it is +// not a code the server sent. func unrecognisedAnswer(stage ui.Stage, what string, apiErr *api.APIError, situation string) *ui.Failure { why := ui.Written("The server answered with the code \"%s\",\n"+ "which this build of curious does not recognise.", string(apiErr.Code)) - if apiErr.Code == "" { - // A body that was not the wire envelope at all — a proxy's own - // page, an empty body — carries no code, and quoting an empty one - // would put words in the server's mouth. - why = "The server's answer carried no code this build could read." - } else if slices.Contains(wire.AllErrorCodes, apiErr.Code) { + if slices.Contains(wire.AllErrorCodes, apiErr.Code) { // A code this build declares. Calling it unrecognised would be // false; what is true is that this step has no copy for it. why = ui.Written("The server answered with the code \"%s\",\n"+ diff --git a/internal/flow/unrecognised_test.go b/internal/flow/unrecognised_test.go index ddd5e68..776e50f 100644 --- a/internal/flow/unrecognised_test.go +++ b/internal/flow/unrecognised_test.go @@ -47,22 +47,27 @@ func TestADefinedCodeAStepHasNoCopyForIsNotCalledUnrecognised(t *testing.T) { // TestAnAnswerWithNoCodeIsNotSaidToHaveOne. A refusal whose body is not the // wire envelope at all — a proxy's own page, an empty body — arrives with // no code. Saying the server answered with the code "" would put a -// quotation in the server's mouth that it never said. +// quotation in the server's mouth that it never said, and so would quoting +// the sentence the client writes for itself when it cannot read an answer. +// The answer such a refusal does get is asserted in full beside it. func TestAnAnswerWithNoCodeIsNotSaidToHaveOne(t *testing.T) { now := time.Date(2026, 9, 12, 12, 0, 0, 0, time.UTC) codeless := &api.APIError{Status: http.StatusBadGateway, Message: "the server returned an error this client could not parse"} var f *ui.Failure - if err := startFailure(codeless, now); !errors.As(err, &f) { + err := startFailure(codeless, now) + if !errors.As(err, &f) { t.Fatalf("the stop is a %T", err) } - const want = "The server's answer carried no code this build could read." - if f.Why != want { - t.Errorf("why = %q, want %q", f.Why, want) + if strings.Contains(f.Why, "code") { + t.Errorf("why = %q: a refusal with no code is described by its code", f.Why) + } + if f.Detail != "" { + t.Errorf("the failure quotes %q as the server's words; the server sent none", f.Detail) } - if f.Detail != codeless.Message { - t.Errorf("the message is %q here, want it verbatim: %q", f.Detail, codeless.Message) + if out := rendered(err); strings.Contains(out, codeless.Message) { + t.Errorf("the client's own sentence reaches the reader as the server's:\n%s", out) } } diff --git a/internal/flow/waitlist_test.go b/internal/flow/waitlist_test.go index 3858f55..751b756 100644 --- a/internal/flow/waitlist_test.go +++ b/internal/flow/waitlist_test.go @@ -78,6 +78,9 @@ func (s *capacityScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { // writeOutcome renders one scripted answer. A zero outcome is success. func writeOutcome(w http.ResponseWriter, o outcome, success any) { + if writeBare(w, o) { + return + } w.Header().Set("Content-Type", "application/json") if o.code == "" { w.WriteHeader(http.StatusOK) diff --git a/internal/guard/codedispatch_test.go b/internal/guard/codedispatch_test.go index 3dd11f6..7570ffd 100644 --- a/internal/guard/codedispatch_test.go +++ b/internal/guard/codedispatch_test.go @@ -74,6 +74,8 @@ import ( const ( wireErrorCodeKey = modulePath + "/pkg/wire.ErrorCode" unrecognisedAnswer = modulePath + "/internal/flow.unrecognisedAnswer" + errorUnexplained = modulePath + "/internal/flow.errorUnexplained" + requestUnserved = modulePath + "/internal/flow.requestUnserved" ) // codeSwitchExceptions are the switches over a code whose default: clause @@ -98,9 +100,6 @@ var codeComparisonExceptions = []struct{ file, fn, against, reason string }{ "counts consecutive refusals of one kind; returns a count and renders nothing"}, {"internal/flow/stream.go", "errorLine", `""`, "an error event with no code renders its message alone; not dispatch on a code"}, - {"internal/flow/unrecognised.go", "unrecognisedAnswer", `""`, - "a refusal whose body was not the wire envelope carries no code, and the shared answer " + - "does not quote one it was not sent; not dispatch on a code"}, } type codeSite struct { @@ -109,6 +108,9 @@ type codeSite struct { fn string against string // comparisons only problem string // switches only; empty when the switch is sound + // emptyProblem is what is wrong with a switch's answer to the empty + // code, or nothing. Switches only. + emptyProblem string } func TestEveryCodeDispatchEndsInTheSharedUnknownAnswer(t *testing.T) { @@ -187,6 +189,50 @@ func TestEveryCodeDispatchEndsInTheSharedUnknownAnswer(t *testing.T) { } } +// A REFUSAL WITH NO CODE GETS ITS OWN ANSWER AT EVERY HANDLER, AND THE +// STATUS DECIDES WHICH. +// +// A body that is not the wire envelope carries no code. Answered by the +// default: clause, it would be described as a code the server sent and +// would quote the client's own sentence as the server's. So every handler +// switch over wire.ErrorCode, outside the keyed switch exceptions, has a +// `case "":` clause whose every return is one of the two code-less +// answers in internal/flow, and which reaches both: errorUnexplained for a +// status that passes on its own, requestUnserved for one that does not. +// +// WHAT THIS CANNOT SEE: whether the condition choosing between the two is +// the status, and whether the situation each site passes is true. The +// rows beside the handlers drive a code-less answer through each one. +// +// REQUIRED MUTATION, made, run and reverted: delete one handler's +// `case "":` clause. Reds: that switch has no case "": clause. +func TestEveryCodeSwitchAnswersAnAnswerWithNoCode(t *testing.T) { + switches, _ := codeDispatchSites(t) + checked := 0 + for _, s := range switches { + excepted := false + for _, e := range codeSwitchExceptions { + excepted = excepted || (s.file == e.file && s.fn == e.fn) + } + if excepted { + continue + } + checked++ + if s.emptyProblem != "" { + t.Errorf("%s: the switch over wire.ErrorCode in %s %s.\n"+ + "A refusal with no code is not a code this build has no copy for: it gets "+ + "`case \"\": if clearsOnItsOwn(apiErr) { return errorUnexplained(...) }; "+ + "return requestUnserved(...)`.", s.where, s.fn, s.emptyProblem) + } + } + if checked == 0 { + t.Fatal("no handler switch over wire.ErrorCode was checked, so this rule checked nothing") + } + if !t.Failed() { + t.Logf("%d handler switches answer an answer with no code", checked) + } +} + // codeDispatchSites is every switch whose tag is a wire.ErrorCode and every // comparison with one, across every platform the release builds. A file // that builds on several platforms is reported once. @@ -229,7 +275,8 @@ func codeDispatchSites(t *testing.T) (switches, comparisons []codeSite) { site := codeSite{ where: fmt.Sprintf("%s:%d:%d", rel, pos.Line, pos.Column), file: rel, fn: enclosingFuncName(p.file, node.Pos()), - problem: defaultProblem(p.info, node), + problem: defaultProblem(p.info, node), + emptyProblem: emptyCodeProblem(p.info, node), } if !seen["switch "+site.where] { seen["switch "+site.where] = true @@ -302,6 +349,62 @@ func defaultProblem(info *types.Info, sw *ast.SwitchStmt) string { return "" } +// emptyCodeProblem says what is wrong with a switch's answer to the empty +// code, or nothing. The clause names the empty code alone, and every return +// in it is a direct call to one of the two code-less answers, both of which +// it reaches: the status decides between them, so a clause that can reach +// only one has dropped the decision. +func emptyCodeProblem(info *types.Info, sw *ast.SwitchStmt) string { + var clause *ast.CaseClause + for _, stmt := range sw.Body.List { + cc, ok := stmt.(*ast.CaseClause) + if !ok { + continue + } + for _, e := range cc.List { + if lit, ok := unparen(e).(*ast.BasicLit); ok && lit.Kind == token.STRING && + (lit.Value == `""` || lit.Value == "``") { + clause = cc + } + } + } + if clause == nil { + return `has no case "": clause` + } + if len(clause.List) != 1 { + return `answers the empty code in a clause shared with other codes` + } + called := map[string]bool{} + problem := "" + ast.Inspect(&ast.BlockStmt{List: clause.Body}, func(n ast.Node) bool { + if _, nested := n.(*ast.FuncLit); nested { + return false + } + ret, ok := n.(*ast.ReturnStmt) + if !ok { + return true + } + callee := "" + if len(ret.Results) > 0 { + if call, ok := unparen(ret.Results[len(ret.Results)-1]).(*ast.CallExpr); ok { + callee = calledObject(info, call.Fun) + } + } + if callee != errorUnexplained && callee != requestUnserved { + problem = `has a case "": clause with a return that is neither code-less answer` + } + called[callee] = true + return true + }) + if problem != "" { + return problem + } + if !called[errorUnexplained] || !called[requestUnserved] { + return `has a case "": clause that does not reach both code-less answers` + } + return "" +} + func calledObject(info *types.Info, fun ast.Expr) string { var obj types.Object switch f := unparen(fun).(type) { diff --git a/internal/guard/failurecensus_test.go b/internal/guard/failurecensus_test.go index 8a23cb4..7ac0835 100644 --- a/internal/guard/failurecensus_test.go +++ b/internal/guard/failurecensus_test.go @@ -73,6 +73,11 @@ func TestTheCheckerVisitsEveryConstruction(t *testing.T) { // own id, action and words, so its callers carry the obligations // and the construction in its body is a definition. modulePath + "/internal/flow.unrecognisedAnswer": true, + // The two answers to a refusal with no code, one for a status that + // passes on its own and one for a status that does not. Each fixes its + // own id, action and words in the same way. + modulePath + "/internal/flow.errorUnexplained": true, + modulePath + "/internal/flow.requestUnserved": true, } for _, p := range files { if p.info == nil { diff --git a/internal/guard/failurecontract_test.go b/internal/guard/failurecontract_test.go index c6a3621..ff08ff2 100644 --- a/internal/guard/failurecontract_test.go +++ b/internal/guard/failurecontract_test.go @@ -200,7 +200,8 @@ func TestTheFailureContractHolds(t *testing.T) { } } } - for _, want := range []string{"NewFailure", "Quoted", "uploadFailed", "unrecognisedAnswer"} { + for _, want := range []string{"NewFailure", "Quoted", "uploadFailed", "unrecognisedAnswer", + "errorUnexplained", "requestUnserved"} { found := false for key := range summaries { found = found || strings.HasSuffix(key, "."+want) diff --git a/internal/mcp/codeless_test.go b/internal/mcp/codeless_test.go new file mode 100644 index 0000000..16dcfe6 --- /dev/null +++ b/internal/mcp/codeless_test.go @@ -0,0 +1,37 @@ +package mcp + +import ( + "net/http" + "strings" + "testing" +) + +// TestALoginRefusalWithNoCodeIsRenderedAsTheFailure. A login call answered +// by a proxy's empty 502 is not one the agent can recover from by asking +// for the code again: the server may have spent that code. So the tool +// hands back the failure, with its id and its next step, and not the +// recoverable answer that invites resending the same code. +func TestALoginRefusalWithNoCodeIsRenderedAsTheFailure(t *testing.T) { + for _, tc := range []struct{ tool, args string }{ + {toolLoginStart, `{"email":"someone@example.com"}`}, + {toolLoginVerify, `{"email":"someone@example.com","code":"123456"}`}, + } { + t.Run(tc.tool, func(t *testing.T) { + run := newToolsRun(t, refusingScript(http.StatusBadGateway, "", "")) + result := run.call(tc.tool, tc.args) + if !result.IsError { + t.Fatalf("a refused login answered as a success: %s", text(t, result)) + } + said := text(t, result) + if strings.Contains(said, "can be recovered from") { + t.Errorf("a refusal with no code was offered as recoverable:\n%s", said) + } + for _, want := range []string{"server-error-unexplained", "Try again in a moment", + "may be spent"} { + if !strings.Contains(said, want) { + t.Errorf("the refusal never says %q:\n%s", want, said) + } + } + }) + } +} diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 086d47c..158c17a 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -98,6 +98,13 @@ func (s *deployScript) ServeHTTP(w http.ResponseWriter, r *http.Request) { // written. case s.refuseAuth != nil && strings.Contains(path, "/auth/"): s.note("auth refused") + if s.refuseAuth.code == "" { + // NO CODE, NO ENVELOPE: the status alone, with an empty body, + // which is what a proxy in front of the service sends while + // the service is down. + w.WriteHeader(s.refuseAuth.status) + return + } w.Header().Set("Content-Type", "application/json") if wire.CarriesRetryAfter(s.refuseAuth.code) { // THE CONTRACT PROMISES A TIME FOR THESE CODES, so the diff --git a/internal/ui/failureid.go b/internal/ui/failureid.go index 1487119..2b55b0c 100644 --- a/internal/ui/failureid.go +++ b/internal/ui/failureid.go @@ -54,6 +54,12 @@ const ( // One family at every stage that meets it: the diagnosis (theirs, and // transient) and the remedy (try again in a moment) are the same. IDServerFault FailureID = "server-fault" + // A refusal that carried no code, so nothing the service wrote: an + // answer from something in front of it. "Unexplained" because curious + // cannot tell what is behind it. The status splits it in two, because + // the remedies differ: one passes on its own, the other does not. + IDServerErrorUnexplained FailureID = "server-error-unexplained" + IDServerRequestUnserved FailureID = "server-request-unserved" // This machine, before anything is sent. IDProjectDirUnknown FailureID = "project-dir-unknown" @@ -155,7 +161,7 @@ var ActiveFailureIDs = []FailureID{ IDServerUnanswered, IDServiceUnavailable, IDCapacityCheckUnreachable, IDCapacityCheckFailed, IDDailyCapacityClosed, IDRateLimited, IDClientRequestRejected, IDFreshLoginRefused, IDServerAnswerUnrecognised, - IDServerFault, + IDServerFault, IDServerErrorUnexplained, IDServerRequestUnserved, IDProjectDirUnknown, IDProjectDirMissing, IDProjectDirUnreadable, IDProjectPathNotADirectory, IDProjectUnreadable, IDConfigLocationUnusable,