amadan
cli: placement echoed and verified at create; -target is tri-state
Opus review of task 8 (2026-08-06), two Importants:
I1. The create path printed "placed on fleet/pi" without ever verifying
it. claimApp (server.go) now returns a second value — what actually
stuck, "" on a swallowed store-write failure or when nothing was
requested. handleAPICreateApp echoes it as the response's "placement"
field (the VALUE, not a boolean — an older console's readJSON ignores
the unknown request field and its response has no such key at all,
which unmarshals to the same "" zero value a genuine swallowed write
gives, so one comparison catches both cases). runAppsCreate compares
the echo against what -place asked for via the new pure
placementMismatchErr helper; a mismatch (including the absent-field
case) prints a loud warning naming the exact remedy
("carlos apps place -app <a> -target <t>") and exits nonzero.
I2. -target on `apps place` was NOT tri-state: an omitted flag and an
explicit -target "" were indistinguishable, so leaving it off silently
CLEARED placement — a fleet-placed app's routes would retire on the
box's next reconcile. Follows cli.go's own -backing/-hibernate
precedent: fs.Visit records whether -target was actually named;
unnamed is now a usage error. Explicit -target "" still means clear.
Tests: TestClaimAppPlacementWriteFailureDoesNotUndoClaim (drives
claimApp directly against a store with its placement column dropped —
the claim still succeeds, the echo says ""); TestAPICreateAppWithPlace
strengthened to assert the echoed field on both the placed and
plain-create paths; TestPlacementMismatchErr (pure function, all four
cases); TestRunAppsCreatePlacementVerification (subprocess, three
canned console responses: swallowed write, old-console absent field,
happy path); TestFleetsAndPlaceCLIUsageErrors gained the omitted
-target case. Full gate green (18/18 packages), gofmt clean.
7 files changed,
+276
−33
apimode.go+11 −3apps.go+60 −5apps_test.go+27 −0fleets_cli_test.go+90 −0internal/console/apicli.go+10 −2internal/console/apicli_test.go+41 −1internal/console/server.go+37 −22
diff --git a/apimode.go b/apimode.go| index 93b75c420..fb680f47d 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -346,10 +346,18 @@ func (s *apiSession) promote(ctx context.Context, a apiAcct, app, version, chann |
| // apiCreateAppResult carries what `carlos apps create` prints: the console's |
| // canonical account sqid (the segment the caller typed may have been a |
| // name), the normalized app name, and the app's console page. |
| +// Placement is what the console actually stored for the "place" field this |
| +// call's request carried — "" for both "no place was requested" and "a |
| +// place WAS requested but did not stick" (an older console that never |
| +// echoes the field at all leaves this at the same "" zero value |
| +// encoding/json gives an explicit empty string, which is exactly what makes |
| +// it usable as the one signal runAppsCreate needs — see |
| +// placementMismatchErr, apps.go). |
| type apiCreateAppResult struct { |
| - Account string `json:"account"` |
| - App string `json:"app"` |
| - URL string `json:"url"` |
| + Account string `json:"account"` |
| + App string `json:"app"` |
| + URL string `json:"url"` |
| + Placement string `json:"placement"` |
| } |
| // createApp claims one app name through the console — the API door onto the |
diff --git a/apps.go b/apps.go| index 9635537ce..1c832cce4 100644 |
| --- a/apps.go |
| +++ b/apps.go |
| @@ -57,7 +57,10 @@ const appsUsage = `usage: carlos apps <create|place|delete|restore> [flags] <app |
| or "" to clear back to platform-served. Every NEW instance |
| created afterwards inherits this target automatically; |
| every instance that already exists is repointed by this |
| - call itself. |
| + call itself. -target is MANDATORY, not optional despite the |
| + "" spelling — leaving it off is a usage error, never a |
| + silent clear (a fleet box retires an app's routes on its |
| + next reconcile the moment placement reads empty). |
| delete [flags] <app> move an app to the Trash: every route drops |
| (custom domains included), its name stays reserved, and it |
| @@ -420,9 +423,42 @@ func runAppsCreate(args []string) { |
| fmt.Fprintf(stdout, "created app %s in account %s — it's yours\n", res.App, res.Account) |
| fmt.Fprintf(stdout, " open: %s\n", res.URL) |
| fmt.Fprintf(stdout, " ship: carlos ship -account %s -app %s ...\n", res.Account, res.App) |
| - if place2 != "" { |
| - fmt.Fprintf(stdout, " placed on %s\n", place2) |
| + if place2 == "" { |
| + return |
| + } |
| + // The app WAS created (everything above already printed and is true) — |
| + // what's still unverified is whether -place actually stuck. The server |
| + // echoes back what it really stored (handleAPICreateApp's "placement" |
| + // field), and this is the one place that gets checked: silently |
| + // trusting the request would let a stolen best-effort store write |
| + // (claimApp's, internal/console/server.go) OR an older console version |
| + // (whose readJSON ignores an unknown "place" key and returns 200 with |
| + // no such field at all — which unmarshals to the same "" zero value) |
| + // report success for an app that is NOT actually placed. |
| + if err := placementMismatchErr(res.App, place2, res.Placement); err != nil { |
| + log.Fatalf("apps create: %v", err) |
| } |
| + fmt.Fprintf(stdout, " placed on %s\n", place2) |
| +} |
| + |
| +// placementMismatchErr compares what -place asked for (requested) against |
| +// what the console's create response actually echoed back (echoed), and |
| +// builds the loud, remedy-naming error runAppsCreate exits on when they |
| +// disagree. Pulled out as its own pure function — no log.Fatal, no I/O — so |
| +// both the ordinary "the write silently failed" case and the "an OLDER |
| +// console never echoes placement at all" case (readJSON there ignores the |
| +// unknown request field and its response JSON simply has no "placement" |
| +// key, which encoding/json leaves at the same "" zero value an explicit |
| +// empty echo would) are covered by ONE string comparison and are directly |
| +// testable without a subprocess. |
| +func placementMismatchErr(app, requested, echoed string) error { |
| + if requested == echoed { |
| + return nil |
| + } |
| + return fmt.Errorf("the app was created, but its placement did NOT stick (the console echoed placement=%q, "+ |
| + "want %q — either the store write silently failed, or this console version is too old to echo "+ |
| + "placement at all). Run this to place it now:\n\n carlos apps place -app %s -target %s\n", |
| + echoed, requested, app, requested) |
| } |
| // runAppsPlace repoints where app's routes and already-provisioned |
| @@ -441,9 +477,28 @@ func runAppsPlace(args []string) { |
| acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account; this is the flag an ambiguity error tells you to use") |
| consoleFlag := fs.String("console", "", "console to place through (default: the only logged-in one)") |
| fs.Parse(args) |
| + const placeUsage = `usage: carlos apps place -app <name> -target fleet/<f>|"" [-account <sqid|name>] ` + |
| + "[-account-id <sqid>] [-console <origin>]" |
| if strings.TrimSpace(*app) == "" { |
| - log.Fatal(`usage: carlos apps place -app <name> -target fleet/<f>|"" [-account <sqid|name>] ` + |
| - "[-account-id <sqid>] [-console <origin>]") |
| + log.Fatal(placeUsage) |
| + } |
| + // -target is TRI-state, the same reason route's -backing is (cli.go's |
| + // -hibernate/-backing precedent, ~line 355): "" is a LEGAL value here — |
| + // it CLEARS placement — so a flag left unmentioned and a flag explicitly |
| + // set to "" cannot be told apart by the value alone. They must not be |
| + // treated the same: an omitted -target silently reading as "clear" would |
| + // retire every route of a fleet-placed app on that fleet box's very next |
| + // reconcile pass. fs.Visit reports only flags actually present on the |
| + // command line, so this is the one reliable way to tell "not named" from |
| + // "named, empty". |
| + targetSet := false |
| + fs.Visit(func(f *flag.Flag) { |
| + if f.Name == "target" { |
| + targetSet = true |
| + } |
| + }) |
| + if !targetSet { |
| + log.Fatal(placeUsage) |
| } |
| if err := validAppName(strings.TrimSpace(*app)); err != nil { |
| log.Fatalf("apps place: %v", err) |
diff --git a/apps_test.go b/apps_test.go| index 16dabce61..8f1f95509 100644 |
| --- a/apps_test.go |
| +++ b/apps_test.go |
| @@ -241,6 +241,33 @@ func TestValidAppNameRejectsWhatReleaseWouldAccidentallyMint(t *testing.T) { |
| } |
| } |
| +// TestPlacementMismatchErr pins the pure comparison runAppsCreate exits on |
| +// (2026-08-06 review: the create path was printing a guarantee it never |
| +// verified). requested==echoed is the only nil case — everything else, |
| +// including echoed=="" (both the "store write silently failed" case AND an |
| +// OLDER console that never echoes "placement" at all, which unmarshals to |
| +// the same "" zero value a Go json.Decoder gives an absent field), must |
| +// error and the error must name the exact remedy command. |
| +func TestPlacementMismatchErr(t *testing.T) { |
| + if err := placementMismatchErr("titogo", "fleet/pi", "fleet/pi"); err != nil { |
| + t.Fatalf("matching echo = %v, want nil", err) |
| + } |
| + if err := placementMismatchErr("titogo", "fleet/pi", ""); err == nil { |
| + t.Fatal("empty echo against a real request = nil, want an error (swallowed write / old console)") |
| + } else if !strings.Contains(err.Error(), "carlos apps place -app titogo -target fleet/pi") { |
| + t.Fatalf("error %q does not name the exact remedy command", err) |
| + } |
| + if err := placementMismatchErr("titogo", "fleet/pi", "fleet/other"); err == nil { |
| + t.Fatal("disagreeing echo = nil, want an error") |
| + } |
| + // requested=="" is never called this way by runAppsCreate (guarded by |
| + // `if place2 == "" { return }` before this is ever reached), but the |
| + // function itself is still symmetric: "" == "" is fine. |
| + if err := placementMismatchErr("titogo", "", ""); err != nil { |
| + t.Fatalf("both empty = %v, want nil", err) |
| + } |
| +} |
| + |
| // newHeadlessTestStore returns a fresh DirStore plus a parsed keypair |
| // (release.GenerateKey base64-encoded, parsed the same way the CLI parses |
| // CARLOS_RELEASE_PUBKEY/CARLOS_RELEASE_KEY) for headlessTrash's direct tests. |
diff --git a/fleets_cli_test.go b/fleets_cli_test.go| index e75db593f..b0764f578 100644 |
| --- a/fleets_cli_test.go |
| +++ b/fleets_cli_test.go |
| @@ -348,8 +348,14 @@ func newFakeFleetsConsole(t *testing.T) *fakeFleetsConsole { |
| var req struct{ Name, Place string } |
| json.NewDecoder(r.Body).Decode(&req) |
| fc.lastCreatePlace.Store(req.Place) |
| + // Echoes place back as "placement" — this fake stands in for a |
| + // CURRENT console, which always echoes what it actually stored |
| + // (handleAPICreateApp, internal/console/apicli.go); the mismatch/ |
| + // absent-field cases that echo is FOR are exercised separately by |
| + // TestRunAppsCreatePlacementVerification's own canned-response fakes. |
| json.NewEncoder(w).Encode(map[string]any{ |
| "account": "bab", "app": req.Name, "url": fc.ts.URL + "/a/bab/apps/" + req.Name, |
| + "placement": req.Place, |
| }) |
| }) |
| mux.HandleFunc("POST /api/cli/apps/{account}/{app}/place", func(w http.ResponseWriter, r *http.Request) { |
| @@ -592,6 +598,12 @@ func TestFleetsAndPlaceCLIUsageErrors(t *testing.T) { |
| {"apps place missing app", []string{"apps", "place", "-target", "fleet/pi"}, "usage: carlos apps place"}, |
| {"apps place bad target shape", []string{"apps", "place", "-app", "titogo", "-target", "eu"}, "not a usable placement target"}, |
| {"apps create bad place shape", []string{"apps", "create", "-app", "titogo", "-place", "eu"}, "not a usable placement target"}, |
| + // I2 (2026-08-06 review): -target is tri-state — omitting it must |
| + // NEVER be read as "clear placement" (that would retire a fleet- |
| + // placed app's routes on the fleet box's next reconcile), so an |
| + // invocation with -app but no -target at all is a usage error, the |
| + // same as omitting -app. |
| + {"apps place omitted target", []string{"apps", "place", "-app", "titogo"}, "usage: carlos apps place"}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| out, err := run(tc.args...) |
| @@ -604,3 +616,81 @@ func TestFleetsAndPlaceCLIUsageErrors(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// ---- I1: placement is verified, not just requested (2026-08-06 review) --- |
| + |
| +// fakeCreateEcho spins a minimal httptest console that answers ONLY POST |
| +// /api/cli/apps/{account}/new, with a fixed canned body — enough to drive |
| +// runAppsCreate's create-with-place path without any of the real claim |
| +// machinery, since these tests are entirely about what the CLI does with |
| +// three different possible echoes. |
| +func fakeCreateEcho(t *testing.T, body string) *httptest.Server { |
| + t.Helper() |
| + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("Content-Type", "application/json") |
| + w.Write([]byte(body)) |
| + })) |
| + t.Cleanup(ts.Close) |
| + return ts |
| +} |
| + |
| +// TestRunAppsCreatePlacementVerification drives `carlos apps create -place |
| +// fleet/pi` as a subprocess against three canned console responses — the |
| +// three cases I1's review comment named by name. Subprocess because the |
| +// mismatch path calls log.Fatal (nonzero exit is the very thing under |
| +// test), which would kill an in-process test binary outright. |
| +func TestRunAppsCreatePlacementVerification(t *testing.T) { |
| + bin := buildCarlos(t) |
| + |
| + run := func(t *testing.T, consoleURL string) (string, error) { |
| + t.Helper() |
| + home := t.TempDir() |
| + creds := filepath.Join(home, "credentials") |
| + writeTestCredential(t, creds, consoleURL) |
| + cmd := exec.Command(bin, "apps", "create", "-app", "titogo", "-place", "fleet/pi", "-account", "bab") |
| + cmd.Env = append(os.Environ(), |
| + "CARLOS_CREDENTIALS="+creds, |
| + "HOME="+home, |
| + "CARLOS_CONSOLE=", |
| + ) |
| + cmd.Dir = home |
| + out, err := cmd.CombinedOutput() |
| + return string(out), err |
| + } |
| + |
| + t.Run("swallowed write: placement echoed empty", func(t *testing.T) { |
| + ts := fakeCreateEcho(t, `{"account":"bab","app":"titogo","url":"http://x/a/bab/apps/titogo","placement":""}`) |
| + out, err := run(t, ts.URL) |
| + if err == nil { |
| + t.Fatalf("exited 0, want nonzero (placement did not stick):\n%s", out) |
| + } |
| + if !strings.Contains(out, "did NOT stick") { |
| + t.Fatalf("output missing the did-NOT-stick warning:\n%s", out) |
| + } |
| + if !strings.Contains(out, "carlos apps place -app titogo -target fleet/pi") { |
| + t.Fatalf("output missing the exact remedy command:\n%s", out) |
| + } |
| + }) |
| + |
| + t.Run("old console: placement key absent entirely", func(t *testing.T) { |
| + ts := fakeCreateEcho(t, `{"account":"bab","app":"titogo","url":"http://x/a/bab/apps/titogo"}`) |
| + out, err := run(t, ts.URL) |
| + if err == nil { |
| + t.Fatalf("exited 0, want nonzero (no placement key at all — an old console's response shape):\n%s", out) |
| + } |
| + if !strings.Contains(out, "did NOT stick") || !strings.Contains(out, "carlos apps place -app titogo -target fleet/pi") { |
| + t.Fatalf("output missing the warning/remedy:\n%s", out) |
| + } |
| + }) |
| + |
| + t.Run("happy path: echo matches", func(t *testing.T) { |
| + ts := fakeCreateEcho(t, `{"account":"bab","app":"titogo","url":"http://x/a/bab/apps/titogo","placement":"fleet/pi"}`) |
| + out, err := run(t, ts.URL) |
| + if err != nil { |
| + t.Fatalf("exited nonzero, want 0:\n%s", out) |
| + } |
| + if !strings.Contains(out, "placed on fleet/pi") { |
| + t.Fatalf("output missing the placed confirmation:\n%s", out) |
| + } |
| + }) |
| +} |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 13529e3c7..aec5659c0 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -205,7 +205,8 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| } |
| - switch s.claimApp(r.Context(), acct, name, addr, place) { |
| + outcome, placed := s.claimApp(r.Context(), acct, name, addr, place) |
| + switch outcome { |
| case claimInvalid: |
| jsonError(w, http.StatusBadRequest, |
| "invalid app name — lowercase letters, digits, and dashes, starting with a letter") |
| @@ -219,9 +220,16 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusBadGateway, "could not check the release store — try again") |
| case claimOK: |
| s.audit(acct, name, addr, "create", "created app "+name+" (cli)") |
| + // "placement" echoes what actually stuck (claimApp's second |
| + // return) — NOT the request's own place value — so a caller (CLI's |
| + // runAppsCreate, apps.go) can tell "placed" apart from "claim |
| + // succeeded, placement silently didn't" (a swallowed store-write |
| + // error) by comparing this field to what it asked for, rather than |
| + // trusting a 200 to mean everything it asked for happened. |
| writeJSON(w, http.StatusOK, map[string]any{ |
| "account": acct.Sqid, "app": name, |
| - "url": s.cfg.Origin + "/a/" + url.PathEscape(acct.Sqid) + "/apps/" + url.PathEscape(name), |
| + "url": s.cfg.Origin + "/a/" + url.PathEscape(acct.Sqid) + "/apps/" + url.PathEscape(name), |
| + "placement": placed, |
| }) |
| } |
| } |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 8564fee0b..6d3005c11 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -2031,22 +2031,62 @@ func TestAPICreateAppWithPlace(t *testing.T) { |
| if status != http.StatusOK { |
| t.Fatalf("create with place = %d %v, want 200", status, out) |
| } |
| + // The response ECHOES what actually stuck (the 2026-08-06 review's |
| + // observability fix) — the CLI (runAppsCreate) compares this field |
| + // against what it asked for, so it must carry the real value, not just |
| + // a boolean. |
| + if got := str(t, out, "placement"); got != "fleet/pi" { |
| + t.Fatalf("response placement = %q, want the echoed fleet/pi", got) |
| + } |
| p, err := fx.st.AppPlacement(OpsAccountID, "titogo") |
| if err != nil || p != "fleet/pi" { |
| t.Fatalf("AppPlacement after create-with-place = %q %v, want fleet/pi", p, err) |
| } |
| // Clearing side: a create with NO place at all still leaves the app |
| - // unplaced, exactly as before this field existed. |
| + // unplaced, exactly as before this field existed — and the echo agrees. |
| status, out = postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", map[string]string{"name": "unplaced"}, token) |
| if status != http.StatusOK { |
| t.Fatalf("plain create = %d %v, want 200", status, out) |
| } |
| + if got := str(t, out, "placement"); got != "" { |
| + t.Fatalf("response placement = %q, want \"\" for a plain create", got) |
| + } |
| if p, err := fx.st.AppPlacement(OpsAccountID, "unplaced"); err != nil || p != "" { |
| t.Fatalf("AppPlacement of a plain create = %q %v, want \"\"", p, err) |
| } |
| } |
| +// TestClaimAppPlacementWriteFailureDoesNotUndoClaim exercises claimApp's |
| +// "the claim succeeded but the placement write did not" branch directly — |
| +// the observability fix a 2026-08-06 review required after finding the |
| +// create path printed a guarantee ("placed on fleet/pi") it never verified. |
| +// Server.store is a concrete *Store (no interface seam to swap in a |
| +// failing double), so the placement column itself is dropped after the |
| +// fixture's normal migration has already created it — CreateAppIn (which |
| +// never touches that column) keeps working, and SetAppPlacement now fails |
| +// on every call, which is exactly the scenario claimApp's `placed` return |
| +// (and handleAPICreateApp's echo, tested at the HTTP layer above) exist to |
| +// make observable instead of silently trusted. |
| +func TestClaimAppPlacementWriteFailureDoesNotUndoClaim(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + if _, err := fx.st.db.Exec(`ALTER TABLE apps DROP COLUMN placement`); err != nil { |
| + t.Fatalf("drop placement column (test setup): %v", err) |
| + } |
| + |
| + acct := &Account{ID: OpsAccountID, Sqid: fx.sqid} |
| + outcome, placed := fx.srv.claimApp(context.Background(), acct, "titogo", "paul@keymail.dev", "fleet/pi") |
| + if outcome != claimOK { |
| + t.Fatalf("claimApp outcome = %v, want claimOK — the claim itself must still succeed", outcome) |
| + } |
| + if placed != "" { |
| + t.Fatalf("claimApp placed = %q, want \"\" — the placement write failed and must not be reported as having stuck", placed) |
| + } |
| + if has, err := fx.st.HasAppIn(OpsAccountID, "titogo"); err != nil || !has { |
| + t.Fatalf("HasAppIn after a failed placement write = %v %v, want true — the claim must not be undone", has, err) |
| + } |
| +} |
| + |
| // TestAPICreateAppWithInvalidPlaceShape400: a place value that doesn't parse |
| // as "fleet/<name>" is a plain 400 — the same grammar check `carlos apps |
| // place` applies to its target — and, critically, the app is NOT created: |
diff --git a/internal/console/server.go b/internal/console/server.go| index 70a60f5b9..485f024c3 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1352,34 +1352,42 @@ func normalizeAppName(name string) string { |
| // the same inline checks handleAPIPlace's target uses, BEFORE calling this, |
| // so an invalid or foreign placement never reaches here at all and never |
| // costs a claim attempt. handleNewApp (the browser door) has no placement |
| -// concept and always passes "". On a successful claim the placement is |
| -// stamped as a SEPARATE store write, after CreateAppIn — not threaded into |
| -// that INSERT — because CreateAppIn has many other, unrelated callers; a |
| -// failure of this second write is logged and does not undo the claim (the |
| -// same best-effort shape ensureAccountManifest below already uses): an app |
| -// that exists but isn't placed is recoverable with `carlos apps place`, |
| -// while failing the whole request after the row is already committed would |
| -// leave the caller unable to tell whether retrying `apps create` means |
| -// "taken" or "try again". |
| -func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string) claimOutcome { |
| +// concept and always passes "". |
| +// |
| +// placed, the second return, is what actually ended up stored: placement |
| +// itself on a successful write, "" on a failed one (or when placement was |
| +// "" to begin with — nothing was ever asked for). On a successful claim the |
| +// placement is stamped as a SEPARATE store write, after CreateAppIn — not |
| +// threaded into that INSERT — because CreateAppIn has many other, unrelated |
| +// callers; a failure of this second write is logged and does NOT undo the |
| +// claim (the same best-effort shape ensureAccountManifest below already |
| +// uses): an app that exists but isn't placed is recoverable with `carlos |
| +// apps place`, while failing the whole request after the row is already |
| +// committed would leave the caller unable to tell whether retrying `apps |
| +// create` means "taken" or "try again". `placed` is what lets the CALLER |
| +// (handleAPICreateApp, echoed to the wire as "placement") make that failure |
| +// OBSERVABLE instead of silently trusting the request — a caller that never |
| +// checks the echo cannot tell "placed" from "claim succeeded, placement |
| +// silently didn't". |
| +func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string) (outcome claimOutcome, placed string) { |
| if !release.ValidApp(name) { |
| - return claimInvalid |
| + return claimInvalid, "" |
| } |
| // Taken means taken IN THIS ACCOUNT — app names are unique per account, |
| // never globally, so another account's "blog" is not a conflict. |
| if has, err := s.store.HasAppIn(acct.ID, name); err != nil { |
| - return claimCheckFailed |
| + return claimCheckFailed, "" |
| } else if has { |
| - return claimTaken |
| + return claimTaken, "" |
| } |
| // The pre-accounts checks still apply to the OPS account, where every |
| // CLI-granted and pre-accounts app lives: a name with grants or bucket |
| // objects there stays claim-proof. |
| if acct.ID == OpsAccountID { |
| if has, err := s.store.HasOwners(name); err != nil { |
| - return claimCheckFailed |
| + return claimCheckFailed, "" |
| } else if has { |
| - return claimTaken |
| + return claimTaken, "" |
| } |
| } |
| // The account's own prefix, plus — for ops only — the legacy unqualified |
| @@ -1402,25 +1410,28 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| for _, prefix := range prefixes { |
| keys, err := s.cfg.Releases.List(ctx, prefix) |
| if err != nil { |
| - return claimStoreFailed |
| + return claimStoreFailed, "" |
| } |
| if len(keys) > 0 { |
| - return claimTaken |
| + return claimTaken, "" |
| } |
| } |
| created, err := s.store.CreateAppIn(acct.ID, name, addr, time.Now()) |
| if err != nil { |
| - return claimCreateFailed |
| + return claimCreateFailed, "" |
| } |
| if !created { // lost a claim race after the checks |
| - return claimTaken |
| + return claimTaken, "" |
| } |
| if placement != "" { |
| if err := s.store.SetAppPlacement(acct.ID, name, placement); err != nil { |
| // Best-effort, like ensureAccountManifest below: the claim itself |
| // already succeeded and must not be undone by this — see this |
| - // function's doc comment for why. |
| + // function's doc comment for why. `placed` stays "" so the |
| + // caller's echo tells the truth about it. |
| log.Printf("console: claim %s/%s: set placement %s: %v", acct.Sqid, name, placement, err) |
| + } else { |
| + placed = placement |
| } |
| } |
| // The app→account linkage needs no manifest of its own any more: it IS the |
| @@ -1428,7 +1439,7 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| // account's own manifest is what the agent materializes, and minting |
| // already wrote it. |
| s.ensureAccountManifest(ctx, acct) |
| - return claimOK |
| + return claimOK, placed |
| } |
| // handleNewApp is the browser door onto claimApp: the account page's New app |
| @@ -1451,7 +1462,11 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, home, http.StatusSeeOther) |
| } |
| name := normalizeAppName(r.FormValue("name")) |
| - switch s.claimApp(r.Context(), acct, name, addr, "") { |
| + // The browser door never places at create time, so the second return |
| + // (what actually stuck) is always "" here and there is nothing to echo |
| + // it to — this form has no placement concept at all. |
| + outcome, _ := s.claimApp(r.Context(), acct, name, addr, "") |
| + switch outcome { |
| case claimInvalid: |
| fail("Invalid app name — lowercase letters, digits, and dashes, starting with a letter.") |
| case claimTaken: |