amadan
carlos billing: the fourth review's P3s (a deleted account's mark on the wire and in the listing, the console's own 5xx on a write, the moved console after the offer, the help's two facts)
Opus's second and Astra's fourth reading of 4e9cac43: both accept with fixes, all P3. Answered here: - account_deleted is always on the wire (the console no longer omits it when false), in the contract file, decoded by the client and printed on the row; the guide says what such a hold is. - The console's own 5xx on a write (a resolution recorded, then the retirement not read back) keeps its words and adds the advice that the change may stand. - After the interactive offer the console-moved error is worded for someone who has just logged in and still unwraps to the move (consoleMovedAfterOffer, every verb); the offer's test asserts both. - The help names the two consequential effects of resolving. - Tests: billingSession's three remedies through the environment; a house acknowledgement with no marker; rejected --json through the command. Not done: main.go's dispatch line and the command's exit codes are not tested; no seeded resolution through package main.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7 files changed,
+101
−23
apimode.go+15 −2apimode_test.go+9 −0billing_cmd.go+26 −7billing_cmd_test.go+47 −10implementation/carloku-paid-launch/billing-operator.md+2 −2internal/console/billingoperator.go+1 −1internal/console/testdata/billing-operator-wire.json+1 −1
CI passed — run details
diff --git a/apimode.go b/apimode.go| index 85086aec5..86d20006b 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -209,7 +209,7 @@ func offerConsoleMoved(e *consoleMovedError) error { |
| // The move is still the fact a caller may need to recognise (a |
| // write that was never carried out); the login's failure is why |
| // nothing more happened. |
| - return fmt.Errorf("%w; logging in to %s failed: %v", e, e.To, err) |
| + return &consoleMovedAfterOffer{moved: e, msg: fmt.Sprintf("this console has moved: %s -> %s; logging in to %s failed: %v", e.From, e.To, e.To, err)} |
| } |
| if cfgConsole, src, err := configDefault(); err == nil && cfgConsole == e.From && src != "" { |
| // The destination is named in the prompt, not only the file: a y |
| @@ -223,9 +223,22 @@ func offerConsoleMoved(e *consoleMovedError) error { |
| } |
| } |
| } |
| - return fmt.Errorf("%w; logged in to %s — re-run your command against it", e, e.To) |
| + return &consoleMovedAfterOffer{moved: e, msg: fmt.Sprintf("logged in to %s — re-run your command against it", e.To)} |
| } |
| +// consoleMovedAfterOffer is the console-moved error once the interactive |
| +// offer has run: worded for someone who has just been through it (no |
| +// second instruction to log in), and still the move for any caller that |
| +// asks (errors.As on *consoleMovedError), since a request answered with |
| +// a move was never carried out. |
| +type consoleMovedAfterOffer struct { |
| + moved *consoleMovedError |
| + msg string |
| +} |
| + |
| +func (e *consoleMovedAfterOffer) Error() string { return e.msg } |
| +func (e *consoleMovedAfterOffer) Unwrap() error { return e.moved } |
| + |
| // errNoDeploymentTarget is resolveMode's "nothing configured at all" |
| // refusal — a var so offerLogin can recognize it by identity rather than |
| // by matching its wording. |
diff --git a/apimode_test.go b/apimode_test.go| index 681094f26..0afdfb175 100644 |
| --- a/apimode_test.go |
| +++ b/apimode_test.go |
| @@ -3435,6 +3435,15 @@ func TestConsoleMovedOfferLogsInAndRepointsConfig(t *testing.T) { |
| if err == nil || !strings.Contains(err.Error(), "re-run your command") { |
| t.Fatalf("offer accepted = %v, want the re-run message", err) |
| } |
| + // Worded for someone who has just logged in (no second instruction to |
| + // log in), and still the move for a caller that asks: a request |
| + // answered with a move was never carried out. |
| + if strings.Contains(err.Error(), "run: carlos auth login") { |
| + t.Fatalf("after the login the message still says to log in: %v", err) |
| + } |
| + if !errors.As(err, new(*consoleMovedError)) { |
| + t.Fatalf("after the offer the error is no longer the move: %T %v", err, err) |
| + } |
| // The device flow really ran against the new host… |
| f, ferr := loadCredentials() |
| if ferr != nil || f.find(fc.ts.URL) == nil { |
diff --git a/billing_cmd.go b/billing_cmd.go| index 9e6221653..ff4c6749e 100644 |
| --- a/billing_cmd.go |
| +++ b/billing_cmd.go |
| @@ -45,10 +45,14 @@ const billingUsage = `usage: carlos billing reviews [--console <url>] [--json] |
| Operator only. reviews lists the open billing review holds; resolve clears |
| one, and only if it has not recurred since it was listed (--occurrences is |
| -the count the listing showed). For a checkout refusal, --outcome is retire |
| -(the default) or keep, and the console answers whether its recorded state |
| -allows it. rejected lists the webhook events the console refused, newest |
| -first. house lists the house accounts, or sets or clears the marker on one. |
| +the count the listing showed). Resolving is an act: a hold on an unsettled |
| +seat change, once resolved, lets that change be retired. For a checkout |
| +refusal, --outcome is retire (the default) or keep; the console first works |
| +the checkout toward a proven outcome, which can expire a session still open |
| +at the provider even when it then refuses, and answers whether what it has |
| +recorded allows the outcome. rejected lists the webhook events the console |
| +refused, newest first. house lists the house accounts, or sets or clears |
| +the marker on one. |
| The console is --console, else CARLOS_CONSOLE, else the one your |
| configuration names (the file CARLOS_CONFIG names; else a .carlos/config |
| @@ -79,7 +83,11 @@ type apiBillingReview struct { |
| CreatedAt int64 `json:"created_at"` |
| LastSeenAt int64 `json:"last_seen_at"` |
| Reference string `json:"reference,omitempty"` |
| - Disposition *struct { |
| + // AccountDeleted: the account was deleted after the hold opened, or |
| + // the hold is evidence that landed after its retirement was |
| + // committed. Still a person's to see. |
| + AccountDeleted bool `json:"account_deleted"` |
| + Disposition *struct { |
| Fate string `json:"fate,omitempty"` |
| Disputed bool `json:"disputed,omitempty"` |
| Obligations []string `json:"obligations,omitempty"` |
| @@ -228,14 +236,22 @@ func (s *apiSession) billingHouseSet(ctx context.Context, account string, house |
| } |
| // billingWriteError words a write the console, or something in front of |
| -// it, refused with a status. The console's own refusal carries a JSON |
| -// error and is definite: it is passed on as it is. A plain 404 is the |
| +// it, refused with a status. The console's own refusal below 500 carries |
| +// a JSON error and is definite: it is passed on as it is; its own 5xx is |
| +// its words, and may follow a committed transaction. A plain 404 is the |
| // doors' answer to anyone who is not an operator, and is definite too. |
| // Any other answer with no JSON error (a 502 or 504 from the edge, a |
| // page from a proxy) came from something other than the door, after the |
| // request went: the console may have carried the write out, and the |
| // operator is told so. |
| func (s *apiSession) billingWriteError(status int, body []byte, next string) error { |
| + if hasJSONError(body) && status >= http.StatusInternalServerError { |
| + // The console's own diagnosis of a failure that may have come |
| + // after its transaction committed (a resolution recorded, then |
| + // the retirement not read back): the words are its, and the |
| + // change may stand. |
| + return fmt.Errorf("%w. the console may have made the change before it failed: %s", s.billingError(status, body), next) |
| + } |
| if hasJSONError(body) || status == http.StatusNotFound { |
| return s.billingError(status, body) |
| } |
| @@ -601,6 +617,9 @@ func printBillingReviews(w io.Writer, reviews []apiBillingReview) { |
| } |
| for _, r := range reviews { |
| fmt.Fprintf(w, "%s %s %s occurrences %d last seen %s\n", dashIfEmpty(r.Account), r.Subscription, r.Reason, r.Occurrences, billingTime(r.LastSeenAt)) |
| + if r.AccountDeleted { |
| + fmt.Fprintf(w, " account deleted: this hold is evidence about a purchase of an account that no longer exists\n") |
| + } |
| if r.Reference != "" { |
| fmt.Fprintf(w, " session %s\n", r.Reference) |
| } |
diff --git a/billing_cmd_test.go b/billing_cmd_test.go| index 459b36279..8c5ddc3a6 100644 |
| --- a/billing_cmd_test.go |
| +++ b/billing_cmd_test.go |
| @@ -88,9 +88,9 @@ func (c *billingConsole) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| answer := func(v any) { _ = json.NewEncoder(w).Encode(v) } |
| switch r.Method + " " + r.URL.Path { |
| case "GET /api/billing/reviews": |
| - refusal := c.named("review_row", map[string]any{"account": "bac", "subscription": "ci_1", "reason": "billing_checkout_currency_refused", "occurrences": 2, "created_at": 1789990000, "last_seen_at": 1790003600, "reference": "cs_test_1", |
| + refusal := c.named("review_row", map[string]any{"account": "bac", "account_deleted": false, "subscription": "ci_1", "reason": "billing_checkout_currency_refused", "occurrences": 2, "created_at": 1789990000, "last_seen_at": 1790003600, "reference": "cs_test_1", |
| "disposition": c.named("disposition", map[string]any{"fate": "expired", "disputed": false, "obligations": []string{"sub_hand"}, "retire_held": "a subscription is attached to the purchase (sub_hand), and canceling a subscription is not settling it", "keep_held": "its session is not proven complete"})}) |
| - gone := c.named("review_row", map[string]any{"account": "bae", "subscription": "ci_2", "reason": "billing_checkout_currency_refused", "occurrences": 1, "created_at": 1790000000, "last_seen_at": 1790000000, "reference": "cs_test_2", |
| + gone := c.named("review_row", map[string]any{"account": "bae", "account_deleted": true, "subscription": "ci_2", "reason": "billing_checkout_currency_refused", "occurrences": 1, "created_at": 1790000000, "last_seen_at": 1790000000, "reference": "cs_test_2", |
| "disposition": c.named("disposition", map[string]any{"fate": "", "disputed": true, "obligations": []string{}, "retire_held": "", "keep_held": c.wire["no_intent_stands"][0]})}) |
| unread := map[string]any{"account": "baf", "subscription": "ci_3", "reason": "billing_checkout_currency_refused", "occurrences": 1, "created_at": 1790000000, "last_seen_at": 1790000000} |
| other := map[string]any{"account": "bad", "subscription": "sub_9", "reason": "billing_location_changed", "occurrences": 1, "created_at": 1790000000, "last_seen_at": 1790000000} |
| @@ -161,8 +161,8 @@ func TestBillingReviewsListing(t *testing.T) { |
| first.Disposition == nil || first.Disposition.Fate != "expired" || len(first.Disposition.Obligations) != 1 || first.Disposition.RetireHeld == "" || first.Disposition.KeepHeld == "" { |
| t.Fatalf("the first review as decoded: %+v %+v", first, first.Disposition) |
| } |
| - if second := reviews[1]; second.Disposition == nil || !second.Disposition.Disputed { |
| - t.Fatalf("the disputed review as decoded: %+v", second.Disposition) |
| + if second := reviews[1]; second.Disposition == nil || !second.Disposition.Disputed || !second.AccountDeleted || first.AccountDeleted { |
| + t.Fatalf("the disputed, deleted account's review as decoded: %+v %+v", second, second.Disposition) |
| } |
| var buf bytes.Buffer |
| printBillingReviews(&buf, reviews) |
| @@ -171,7 +171,7 @@ func TestBillingReviewsListing(t *testing.T) { |
| "bac ci_1 billing_checkout_currency_refused occurrences 2 last seen " + time.Unix(1790003600, 0).UTC().Format("2006-01-02 15:04"), |
| "session cs_test_1", "session fate: expired", "attached: sub_hand", |
| "retire: held: a subscription is attached to the purchase (sub_hand)", "keep: held: its session is not proven complete", |
| - "session fate: no intent stands, disputed", "retire: not held by what is recorded (the resolution may still read the provider, and refuse)", "keep: held: no such intent stands", |
| + "session fate: no intent stands, disputed", "account deleted: this hold is evidence about a purchase of an account that no longer exists", "retire: not held by what is recorded (the resolution may still read the provider, and refuse)", "keep: held: no such intent stands", |
| "baf ci_3 billing_checkout_currency_refused", "disposition unavailable", |
| "bad sub_9 billing_location_changed occurrences 1", |
| } { |
| @@ -183,7 +183,7 @@ func TestBillingReviewsListing(t *testing.T) { |
| t.Errorf("the listing calls an outcome available, which only the resolution can say:\n%s", out) |
| } |
| // A hold that is not a checkout refusal has no disposition lines. |
| - if strings.Count(out, "session fate:") != 2 || strings.Count(out, "disposition unavailable") != 1 { |
| + if strings.Count(out, "session fate:") != 2 || strings.Count(out, "disposition unavailable") != 1 || strings.Count(out, "account deleted") != 1 { |
| t.Errorf("disposition lines:\n%s", out) |
| } |
| buf.Reset() |
| @@ -366,6 +366,9 @@ func TestBillingRunSendsWhatWasParsedAndPrintsTheAnswer(t *testing.T) { |
| if out, err = run("house", "--json"); err != nil || !strings.Contains(out, `"house"`) { |
| t.Fatalf("house --json: %q %v", out, err) |
| } |
| + if out, err = run("rejected", "--json"); err != nil || json.Unmarshal([]byte(out), &listing) != nil || listing["rejected"] == nil { |
| + t.Fatalf("rejected --json: %q %v", out, err) |
| + } |
| if len(c.bodies) != writes { |
| t.Fatalf("a listing made %d writes", len(c.bodies)-writes) |
| } |
| @@ -498,11 +501,20 @@ func TestBillingWriteErrorsSayWhatIsKnown(t *testing.T) { |
| } |
| } |
| c0.write = func(w http.ResponseWriter) { |
| - w.WriteHeader(http.StatusServiceUnavailable) |
| - _, _ = io.WriteString(w, `{"error":"the intent's session could not be read. try again"}`) |
| + w.WriteHeader(http.StatusConflict) |
| + _, _ = io.WriteString(w, `{"error":"the account has a subscription. the house marker can change only when it has none"}`) |
| + } |
| + if err := sess0.billingHouseSet(context.Background(), "bac", true, "n"); err == nil || err.Error() != "the account has a subscription. the house marker can change only when it has none" { |
| + t.Errorf("the console's own 409: %v; want it as it is", err) |
| + } |
| + // The console's own 5xx is its words, and may follow a committed |
| + // transaction: the words, and the advice. |
| + c0.write = func(w http.ResponseWriter) { |
| + w.WriteHeader(http.StatusInternalServerError) |
| + _, _ = io.WriteString(w, `{"error":"the retirement could not be read back"}`) |
| } |
| - if err := sess0.billingHouseSet(context.Background(), "bac", true, "n"); err == nil || err.Error() != "the intent's session could not be read. try again" { |
| - t.Errorf("the console's own 503: %v; want it as it is", err) |
| + if _, _, err := sess0.billingResolve(context.Background(), apiBillingResolution{Account: "bac", Subject: "ci_1", Reason: "r", Occurrences: 1, Note: "n"}); err == nil || !strings.Contains(err.Error(), "the retirement could not be read back") || !strings.Contains(err.Error(), "may have made the change before it failed: list the reviews again") { |
| + t.Errorf("the console's own 500 on a resolve: %v; want its words and the advice", err) |
| } |
| // Through the session, against a console that is not there. |
| gone := httptest.NewServer(http.NotFoundHandler()) |
| @@ -672,6 +684,7 @@ func TestBillingRejectedAndHouse(t *testing.T) { |
| for name, answer := range map[string]string{ |
| "an empty object": `{}`, |
| "the other marker": `{"account":"bac","house":true}`, |
| + "no marker at all": `{"account":"bac"}`, |
| "another account": `{"account":"zzz","house":false}`, |
| "a page from something else": `<html><body>welcome</body></html>`, |
| } { |
| @@ -682,6 +695,30 @@ func TestBillingRejectedAndHouse(t *testing.T) { |
| } |
| } |
| +// The session: billing's one route is a console, and the refusals say so. |
| +func TestBillingSessionNamesItsOneRoute(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + // No configuration of this machine's reaches the test: not the home |
| + // directory's, not one above the working directory. |
| + empty := filepath.Join(t.TempDir(), "config") |
| + if err := os.WriteFile(empty, []byte("{}"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Setenv("CARLOS_CONFIG", empty) |
| + t.Setenv("CARLOS_DEPLOYMENT_DIR", t.TempDir()) |
| + if _, err := billingSession(""); err == nil || !strings.Contains(err.Error(), "unset it to use the console you are logged in to") { |
| + t.Fatalf("direct mode: %v", err) |
| + } |
| + if _, err := billingSession("https://c.example"); err == nil || !strings.Contains(err.Error(), "works only through a console") || !strings.Contains(err.Error(), "unset CARLOS_DEPLOYMENT_BUCKET and CARLOS_DEPLOYMENT_DIR") { |
| + t.Fatalf("--console with direct mode set: %v", err) |
| + } |
| + clearDeploymentEnv(t) |
| + if _, err := billingSession(""); err == nil || !strings.Contains(err.Error(), "works only through a console") || strings.Contains(err.Error(), "direct bucket access") { |
| + t.Fatalf("nothing configured: %v; want billing's own route, not direct-mode advice", err) |
| + } |
| +} |
| + |
| // A plain 404 is the doors' answer to anyone who is not an operator, and |
| // a console's from before the doors: it is said as that, with the console |
| // and the address, not as advice about --account and --app. The |
diff --git a/implementation/carloku-paid-launch/billing-operator.md b/implementation/carloku-paid-launch/billing-operator.md| index 9fcc46783..724446b2f 100644 |
| --- a/implementation/carloku-paid-launch/billing-operator.md |
| +++ b/implementation/carloku-paid-launch/billing-operator.md |
| @@ -26,7 +26,7 @@ One block per open hold. The first line carries what `resolve` needs: the accoun |
| What a hold does: while any hold is open on an account, the console authorises no new invoice work for it (no month is frozen, no invoice advanced); it still records the outcome of what the provider has already done. A checkout refusal on the account's standing checkout also stops the account starting any purchase (an older refusal whose checkout is gone holds invoices, not new checkouts). Resolving one hold releases nothing while another is open on the same account, and repairs nothing about the others. |
| -For a checkout refusal (`billing_checkout_currency_refused`) the block also shows what the console has recorded: the session, its fate (`expired`, `complete`, not proven, or that no intent stands), any subscription attached to the purchase, and for each outcome whether it is held and why, in the console's words. "Not held by what is recorded" is not "will succeed": a keep reads the session again and can refuse; a retire of a session already recorded expired reads nothing more. A refusal listed with "disposition unavailable" is one the console could not read its record of; list again. |
| +For a checkout refusal (`billing_checkout_currency_refused`) the block also shows what the console has recorded: the session, its fate (`expired`, `complete`, not proven, or that no intent stands), any subscription attached to the purchase, and for each outcome whether it is held and why, in the console's words. "Not held by what is recorded" is not "will succeed": a keep reads the session again and can refuse; a retire of a session already recorded expired reads nothing more. A refusal listed with "disposition unavailable" is one the console could not read its record of; list again. A hold marked "account deleted" is evidence about a purchase of an account that no longer exists (it landed after the account's retirement was committed, or the account went since): it holds nothing a person can release in stage 1 and is kept for the record. |
| ## resolve |
| @@ -47,7 +47,7 @@ The confirmation says what the console acknowledged in terms of what you asked: |
| Repeating a resolution exactly (the same operator, the same note, the same outcome) answers as the first time, while the occurrence it resolved is still the current one. If the hold has recurred since (a keep whose purchase was refused again), the repeat is refused as recurred, and the new occurrence needs its own resolution. Anything else for an occurrence already resolved is refused, another operator's exact repeat included. |
| -If the answer never arrives (a timeout: the console can make up to four provider calls before it answers a keep), or something in front of the console answers with a status and none of the console's own words (a 502 from the edge), the change may have been made. For a resolution: list again, or repeat the command exactly, on the same console, as the same operator. A reworded note is not an exact repeat. For the house marker: list the house accounts first. A repeat sets the marker to what the command says whatever anyone has done since. A console that says it has moved, or that could not be reached at all, changed nothing, and the error says that instead. |
| +If the answer never arrives (a timeout: the console can make up to four provider calls before it answers a keep), or something in front of the console answers with a status and none of the console's own words (a 502 from the edge), or the console's own answer is a 5xx in its own words (a resolution recorded, then the retirement not read back), the change may have been made. For a resolution: list again, or repeat the command exactly, on the same console, as the same operator. A reworded note is not an exact repeat. For the house marker: list the house accounts first. A repeat sets the marker to what the command says whatever anyone has done since. A console that says it has moved, or that could not be reached at all, changed nothing, and the error says that instead. |
| ## rejected |
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go| index 58c509561..32c3fd741 100644 |
| --- a/internal/console/billingoperator.go |
| +++ b/internal/console/billingoperator.go |
| @@ -184,7 +184,7 @@ type billingReviewRow struct { |
| Reference string `json:"reference,omitempty"` |
| // AccountDeleted: the review is a deleted account's (evidence that |
| // landed after its retirement was committed). Still a person's to see. |
| - AccountDeleted bool `json:"account_deleted,omitempty"` |
| + AccountDeleted bool `json:"account_deleted"` |
| // Disposition, for a checkout refusal: the session's recorded fate, |
| // what is attached, and why retire or keep is held (empty: available). |
| Disposition *refusalDisposition `json:"disposition,omitempty"` |
diff --git a/internal/console/testdata/billing-operator-wire.json b/internal/console/testdata/billing-operator-wire.json| index 2bc7ae1e1..43a982aa3 100644 |
| --- a/internal/console/testdata/billing-operator-wire.json |
| +++ b/internal/console/testdata/billing-operator-wire.json |
| @@ -1,6 +1,6 @@ |
| { |
| "envelopes": ["house", "rejected", "reviews"], |
| - "review_row": ["account", "created_at", "disposition", "last_seen_at", "occurrences", "reason", "reference", "subscription"], |
| + "review_row": ["account", "account_deleted", "created_at", "disposition", "last_seen_at", "occurrences", "reason", "reference", "subscription"], |
| "disposition": ["disputed", "fate", "keep_held", "obligations", "retire_held"], |
| "rejected_row": ["account", "id", "object", "reason", "received_at", "subscription", "type"], |
| "resolve_request": ["account", "note", "occurrences", "outcome", "reason", "subscription"], |