From 3ab98ce525abbf203c0a08b4c99ace1fc62a4efd Mon Sep 17 00:00:00 2001 From: Ed Snible Date: Fri, 21 Aug 2026 14:10:17 -0400 Subject: [PATCH] Route the context operations in the in-process server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /contexts paths were absent from the route table, so every `rossoctl contexts` command against a cortex context fell through to the mux and 404'd. They are placeholders like most of the table, but they have to be present: before: GET .../contexts/team1/research -> 404 page not found after: GET .../contexts/team1/research -> 500 {"detail":"UNIMPLEMENTED"} The 404 was not merely the wrong code. contextListError maps a 404 to "this Rosso server does not support context infrastructure", naming the API version that introduced the resource — a true statement about an old server and a misleading one about a local cortex, which sent the reader to check their server's version rather than their current context. UNIMPLEMENTED is what every other unimplemented cortex endpoint reports. Covered by a new test in wire_test.go that drives the real apiclient methods rather than requesting hand-written paths. That distinction is the point: a test asking for "/contexts/ns/name" directly would keep passing if the client changed the path it requests, which is how the table fell behind to begin with. Verified to fail, naming the cause, with the four routes removed. TestRouteTableMatchesOpenAPI's count goes from 44 to 48. Its comment warns that raising the number is only correct alongside evidence the OpenAPI document grew, so the exception is recorded rather than waved through: these four are the context resource API from rossoctl/rossoctl#2392, which postdates that document, and they are transcribed from the paths internal/apiclient requests. The new test is what holds them to that. Assisted by Claude. Signed-off-by: Ed Snible --- internal/serve/serve.go | 21 ++++++++++- internal/serve/serve_test.go | 16 +++++++-- internal/serve/wire_test.go | 70 ++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/internal/serve/serve.go b/internal/serve/serve.go index 4752b11..4780434 100644 --- a/internal/serve/serve.go +++ b/internal/serve/serve.go @@ -3,7 +3,9 @@ // The API surface mirrors the OpenAPI document the real backend publishes at // /api/openapi.json: every operation in that document is routed here, so a UI // pointed at this server sees the same set of endpoints rather than a wall of -// 404s. Most are placeholders that answer 500 UNIMPLEMENTED. Ten are real: +// 404s. The /contexts operations are routed too, though they postdate that +// document; see the note on them in apiRoutes. Most are placeholders that answer +// 500 UNIMPLEMENTED. Ten are real: // // - GET /auth/config reports authentication as disabled, so a UI can finish // initializing without a Keycloak realm behind it. @@ -151,6 +153,23 @@ var apiRoutes = []Route{ {http.MethodGet, "/config/mcp-gateway-status", unimplemented}, {http.MethodGet, "/config/platform-status", unimplemented}, + // contexts + // + // The context resource API (rossoctl/rossoctl#2392) is newer than the OpenAPI + // document the rest of this table was transcribed from, so these are listed + // from the paths internal/apiclient requests rather than from that document. + // They are placeholders like most of the table, but they have to be *present*: + // an unlisted path falls through to the mux and 404s, and a 404 from a cortex + // context is the same answer a server that predates the context API gives — + // which is what `contexts list` reads as "this server does not support context + // infrastructure". A local cortex does not support it either, but for an + // unrelated reason, and reporting the two identically sends the user looking at + // their server's version instead of at their current context. + {http.MethodPost, "/contexts", unimplemented}, + {http.MethodGet, "/contexts/{namespace}", unimplemented}, + {http.MethodDelete, "/contexts/{namespace}/{name}", unimplemented}, + {http.MethodGet, "/contexts/{namespace}/{name}", unimplemented}, + // namespaces {http.MethodGet, "/namespaces", namespacesRoute}, diff --git a/internal/serve/serve_test.go b/internal/serve/serve_test.go index 81f2a84..aa8ed19 100644 --- a/internal/serve/serve_test.go +++ b/internal/serve/serve_test.go @@ -325,16 +325,26 @@ func TestListenPortInUse(t *testing.T) { } // TestRouteTableMatchesOpenAPI guards the operation count against accidental -// edits to the route table. The backend's OpenAPI document lists 44 operations -// under /api/v1 plus /health and /ready at the root. +// edits to the route table. The count is 48: 44 operations from the backend's +// OpenAPI document under /api/v1, plus the 4 context operations described below, +// with /health and /ready at the root counted separately. // // It was 43 until PUT /agents/{namespace}/{name}/identity-config was added: the // backend has always published it (agents.py declares it beside the GET), and the // transcription had simply missed it. Raising this number is therefore only // correct alongside evidence that the document grew — otherwise a route invented // here would be waved through. +// +// The 4 context routes (POST /contexts, GET /contexts/{namespace}, and GET and +// DELETE /contexts/{namespace}/{name}) are the evidence-backed exception to +// "transcribed from the document": they are the context resource API from +// rossoctl/rossoctl#2392, which postdates the document this table was built from, +// and they are listed from the paths internal/apiclient actually requests. +// TestContextRoutesAreReachedByTheClient in wire_test.go is what holds them to +// that — it drives the real client, so a path here that the client does not ask +// for, or vice versa, fails. func TestRouteTableMatchesOpenAPI(t *testing.T) { - if got, want := len(APIRoutes()), 44; got != want { + if got, want := len(APIRoutes()), 48; got != want { t.Errorf("API route count = %d, want %d", got, want) } if got, want := len(HealthRoutes()), 2; got != want { diff --git a/internal/serve/wire_test.go b/internal/serve/wire_test.go index 411f567..d87b017 100644 --- a/internal/serve/wire_test.go +++ b/internal/serve/wire_test.go @@ -1,9 +1,12 @@ package serve import ( + "context" "encoding/json" + "errors" "net/http" "reflect" + "strings" "testing" "github.com/rossoctl/rossoctl-cli/internal/agentapi" @@ -319,3 +322,70 @@ func TestAgentCardNullDescriptionDecodes(t *testing.T) { t.Error("decode was vacuous; nothing arrived") } } + +// TestContextRoutesAreReachedByTheClient verifies each context operation the CLI +// can issue lands on a registered route, by driving the real client against this +// server rather than by requesting a hand-written path. +// +// This is the regression test for the bug it was written for: the /contexts paths +// were absent from the route table, so every context command 404'd. A 404 is not a +// neutral failure here — `contexts list` reads it as "this Rosso server does not +// support context infrastructure" (see contextListError), which is a true statement +// about an old *server* and a misleading one about a local cortex, sending the user +// to check their server's version instead of their current context. +// +// Asserting through apiclient is what makes this catch drift: a test requesting +// "/contexts/ns/name" by hand would keep passing if the client changed the path it +// asks for, which is exactly how the table fell behind in the first place. +func TestContextRoutesAreReachedByTheClient(t *testing.T) { + stubGetter(t, mixedInstances()) + ts := newTestServer(t, "/api/v1") + client := &apiclient.Client{BaseURL: ts.URL + "/api/v1/"} + + // requireUnimplemented asserts the call reached a placeholder route: a 500 + // carrying the UNIMPLEMENTED detail, never a 404 from the mux. + requireUnimplemented := func(t *testing.T, op string, err error) { + t.Helper() + if err == nil { + t.Fatalf("%s: expected an error from a placeholder route", op) + } + var statusErr *apiclient.StatusError + if !errors.As(err, &statusErr) { + t.Fatalf("%s: error is not a StatusError: %v", op, err) + } + if statusErr.StatusCode == http.StatusNotFound { + t.Fatalf("%s: got 404 — the path the client requests is not in the route table: %v", + op, err) + } + if statusErr.StatusCode != http.StatusInternalServerError { + t.Errorf("%s: status = %d, want 500", op, statusErr.StatusCode) + } + if !strings.Contains(statusErr.Body, unimplementedMessage) { + t.Errorf("%s: body = %q, want it to carry %q", op, statusErr.Body, unimplementedMessage) + } + } + + ctx := context.Background() + + t.Run("CreateContext", func(t *testing.T) { + _, err := client.CreateContext(ctx, &apiclient.CreateContextRequest{ + Name: "research", Namespace: "nsA", Type: "workspace", + }) + requireUnimplemented(t, "CreateContext", err) + }) + + t.Run("ListContexts", func(t *testing.T) { + _, err := client.ListContexts(ctx, "nsA") + requireUnimplemented(t, "ListContexts", err) + }) + + t.Run("GetContext", func(t *testing.T) { + _, err := client.GetContext(ctx, "nsA", "research") + requireUnimplemented(t, "GetContext", err) + }) + + t.Run("DeleteContext", func(t *testing.T) { + err := client.DeleteContext(ctx, "nsA", "research") + requireUnimplemented(t, "DeleteContext", err) + }) +}