amadan
carlos billing: Astra's second review answered (ambiguous flag values, the keep wording, strict listings, accurate advice, tests through the run function)
Astra's second review of 51ef27ac: accept with fixes, three P2s and six P3s. Answered here: - A flag given twice, a flag given with an empty value, and a value that looks like a flag are refused before anything is sent: "--set --note --clear" set the marker with the note "--clear", "--outcome keep --outcome retire" retired, and --console "$UNSET" fell through to the configured console. - A keep is reported as kept, not as "the intent stands": an exact repeat answers the same after the purchase recorded and the intent went. - A 200 that is not the door's listing (no envelope key, or not an array) is refused, not read as an empty listing. - A write that errors: a console that moved and a connection never made pass as they are; anything else may have been a change, with advice that fits the write (a house write is not replay-safe). - The note is checked with the console's own predicate. - Console selection is described with every source that can choose it. - billingRun is the part of the command between the parsed line and the doors, and is tested: clear sends false, keep sends the outcome, --json prints the answer, a refusal prints nothing as done. - The tests assert every field the client decodes against a value that is not its zero (disputed, created_at, received_at were not). Not done: a seeded resolution through package main against the real handlers (the console's contract test does it against the handlers); main.go's dispatch line is not tested. The public reference section is still unreviewed copy.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+378
−76
billing_cmd.go+159 −60billing_cmd_test.go+206 −6docs/site/cli.md+6 −3implementation/carloku-paid-launch/billing-operator.md+7 −7
diff --git a/billing_cmd.go b/billing_cmd.go| index 0a0528467..da50ed0c8 100644 |
| --- a/billing_cmd.go |
| +++ b/billing_cmd.go |
| @@ -28,11 +28,11 @@ import ( |
| "fmt" |
| "io" |
| "log" |
| + "net" |
| "net/http" |
| "os" |
| "strings" |
| "time" |
| - "unicode" |
| "unicode/utf8" |
| ) |
| @@ -49,9 +49,12 @@ the count the listing showed). For a checkout refusal, --outcome is retire |
| 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 console is --console, else CARLOS_CONSOLE, else the one a .carlos/config |
| -above the current directory names, else the only one logged in. A write |
| -says which console it acted on. Quote a note of more than one word. |
| +The console is --console, else CARLOS_CONSOLE, else the one your |
| +configuration names (the file CARLOS_CONFIG names; else a .carlos/config |
| +above the current directory; else ~/.carlos/config), else the only one |
| +logged in. A write says which console it acted on, after it has acted: |
| +pass --console for a write. Quote a note of more than one word, and give |
| +no flag twice. |
| ` |
| // billingCheckoutRefused is the one reason whose resolution has an |
| @@ -106,7 +109,16 @@ func (s *apiSession) billingError(status int, body []byte) error { |
| return apiErrorFromBody(status, body) |
| } |
| -func (s *apiSession) billingGet(ctx context.Context, path string, out any) ([]byte, error) { |
| +// errBillingNotAListing is a 200 that is not the door's listing: something |
| +// between here and the console answered, or the console is not the build |
| +// this client knows. It is not an empty listing. |
| +var errBillingNotAListing = errors.New("the console's answer is not the listing this command reads. nothing is known from it") |
| + |
| +// billingList reads one listing door: a JSON object whose key holds an |
| +// array. An object without the key, or with anything but an array under |
| +// it, is refused, not read as "none": an operator told there are no open |
| +// reviews must be able to rely on it. |
| +func (s *apiSession) billingList(ctx context.Context, path, key string, out any) ([]byte, error) { |
| status, body, err := s.getJSON(ctx, path) |
| if err != nil { |
| return nil, err |
| @@ -114,31 +126,36 @@ func (s *apiSession) billingGet(ctx context.Context, path string, out any) ([]by |
| if status != http.StatusOK { |
| return nil, s.billingError(status, body) |
| } |
| - return body, json.Unmarshal(body, out) |
| + var envelope map[string]json.RawMessage |
| + if err := json.Unmarshal(body, &envelope); err != nil { |
| + return nil, errBillingNotAListing |
| + } |
| + rows, ok := envelope[key] |
| + if !ok || len(rows) == 0 || rows[0] != '[' { |
| + return nil, errBillingNotAListing |
| + } |
| + if err := json.Unmarshal(rows, out); err != nil { |
| + return nil, errBillingNotAListing |
| + } |
| + return body, nil |
| } |
| func (s *apiSession) billingReviews(ctx context.Context) ([]apiBillingReview, []byte, error) { |
| - var res struct { |
| - Reviews []apiBillingReview `json:"reviews"` |
| - } |
| - raw, err := s.billingGet(ctx, "/api/billing/reviews", &res) |
| - return res.Reviews, raw, err |
| + var rows []apiBillingReview |
| + raw, err := s.billingList(ctx, "/api/billing/reviews", "reviews", &rows) |
| + return rows, raw, err |
| } |
| func (s *apiSession) billingRejected(ctx context.Context) ([]apiBillingRejected, []byte, error) { |
| - var res struct { |
| - Rejected []apiBillingRejected `json:"rejected"` |
| - } |
| - raw, err := s.billingGet(ctx, "/api/billing/rejected", &res) |
| - return res.Rejected, raw, err |
| + var rows []apiBillingRejected |
| + raw, err := s.billingList(ctx, "/api/billing/rejected", "rejected", &rows) |
| + return rows, raw, err |
| } |
| func (s *apiSession) billingHouse(ctx context.Context) ([]string, []byte, error) { |
| - var res struct { |
| - House []string `json:"house"` |
| - } |
| - raw, err := s.billingGet(ctx, "/api/billing/house", &res) |
| - return res.House, raw, err |
| + var rows []string |
| + raw, err := s.billingList(ctx, "/api/billing/house", "house", &rows) |
| + return rows, raw, err |
| } |
| // apiBillingResolution is what the operator asked for; outcome is sent |
| @@ -165,7 +182,7 @@ var errBillingUnacknowledged = errors.New("the console answered without acknowle |
| func (s *apiSession) billingResolve(ctx context.Context, r apiBillingResolution) (retired, known bool, err error) { |
| status, body, err := s.postJSON(ctx, "/api/billing/reviews/resolve", r) |
| if err != nil { |
| - return false, false, billingWriteUnknown(err) |
| + return false, false, billingWriteUnknown(err, "list the reviews again, or repeat this exact command on the same console (an exact repeat by the same operator answers as the first did; a changed note is not one)") |
| } |
| if status != http.StatusOK { |
| return false, false, s.billingError(status, body) |
| @@ -192,7 +209,7 @@ func (s *apiSession) billingResolve(ctx context.Context, r apiBillingResolution) |
| func (s *apiSession) billingHouseSet(ctx context.Context, account string, house bool, note string) error { |
| status, body, err := s.postJSON(ctx, "/api/billing/house", map[string]any{"account": account, "house": house, "note": note}) |
| if err != nil { |
| - return billingWriteUnknown(err) |
| + return billingWriteUnknown(err, "list the house accounts before deciding whether to repeat it (a repeat sets the marker to what this command says, whatever anyone has done since)") |
| } |
| if status != http.StatusOK { |
| return s.billingError(status, body) |
| @@ -207,12 +224,23 @@ func (s *apiSession) billingHouseSet(ctx context.Context, account string, house |
| return nil |
| } |
| -// billingWriteUnknown is a write whose answer never arrived (a timeout, a |
| -// dropped connection): the console may have made the change. Repeating it |
| -// exactly is safe (an exact repeat of a resolution answers as the first |
| -// did), and a changed note is not an exact repeat. |
| -func billingWriteUnknown(err error) error { |
| - return fmt.Errorf("%w. the console may have made the change: list again, or repeat the command exactly", err) |
| +// billingWriteUnknown words a write that returned an error instead of an |
| +// answer. Two kinds of error are known not to have changed anything and |
| +// pass as they are: the console saying it has moved (a definite answer, |
| +// with its own remedy), and a connection that was never made. Anything |
| +// else (a timeout, a connection lost after the request went) may have |
| +// been a change, and the operator is told so, with what to do for this |
| +// kind of write. |
| +func billingWriteUnknown(err error, next string) error { |
| + var moved *consoleMovedError |
| + if errors.As(err, &moved) { |
| + return err |
| + } |
| + var op *net.OpError |
| + if errors.As(err, &op) && op.Op == "dial" { |
| + return err |
| + } |
| + return fmt.Errorf("%w. the console may have made the change: %s", err, next) |
| } |
| func billingSession(consoleFlag string) *apiSession { |
| @@ -253,7 +281,7 @@ var billingFlagsOf = map[string]map[string]bool{ |
| } |
| // billingNote checks a note as the console will: something, in at most |
| -// billingNoteMax bytes of valid text with no control characters. |
| +// billingNoteMax bytes of valid text with no ASCII control character. |
| func billingNote(note string) (string, error) { |
| note = strings.TrimSpace(note) |
| switch { |
| @@ -261,8 +289,17 @@ func billingNote(note string) (string, error) { |
| return "", errors.New("--note is required") |
| case len(note) > billingNoteMax: |
| return "", fmt.Errorf("--note is %d bytes; the console keeps at most %d", len(note), billingNoteMax) |
| - case !utf8.ValidString(note) || strings.IndexFunc(note, unicode.IsControl) >= 0: |
| - return "", errors.New("--note must be plain text on one line") |
| + case !utf8.ValidString(note): |
| + return "", errors.New("--note is not valid text") |
| + } |
| + // The console's own rule (internal/console: validReviewText): no |
| + // control character below U+0020, and not U+007F. That excludes a |
| + // tab and a line feed; it is not a promise that the note is one line |
| + // in every sense. |
| + for _, r := range note { |
| + if r < 0x20 || r == 0x7f { |
| + return "", errors.New("--note contains a control character (a tab or a line break, perhaps); the console refuses those") |
| + } |
| } |
| return note, nil |
| } |
| @@ -278,6 +315,9 @@ func parseBilling(args []string) (billingInvocation, error) { |
| if _, known := billingFlagsOf[inv.Verb]; !known || inv.Verb == "house write" { |
| return inv, fmt.Errorf("no such subcommand: %s", inv.Verb) |
| } |
| + if err := billingArgsAreUnambiguous(args[1:]); err != nil { |
| + return inv, err |
| + } |
| fs := flag.NewFlagSet("billing "+inv.Verb, flag.ContinueOnError) |
| fs.SetOutput(io.Discard) |
| consoleFlag := fs.String("console", "", "console to use") |
| @@ -301,6 +341,15 @@ func parseBilling(args []string) (billingInvocation, error) { |
| } |
| given := map[string]bool{} |
| fs.Visit(func(f *flag.Flag) { given[f.Name] = true }) |
| + // A flag given with nothing in it is not the flag left out: an unset |
| + // shell variable in --console "$TARGET" would otherwise fall through |
| + // to whichever console is configured, and --outcome "" to the |
| + // console's default, which is retire. |
| + for name, value := range map[string]string{"console": *consoleFlag, "outcome": *outcome, "account": *account, "subject": *subject, "reason": *reason} { |
| + if given[name] && strings.TrimSpace(value) == "" { |
| + return inv, fmt.Errorf("--%s was given with no value", name) |
| + } |
| + } |
| form := inv.Verb |
| if inv.Verb == "house" && (given["set"] || given["clear"] || given["account"] || given["note"]) { |
| form = "house write" |
| @@ -342,6 +391,41 @@ func parseBilling(args []string) (billingInvocation, error) { |
| return inv, nil |
| } |
| +// billingBoolFlags are the flags that take no value. |
| +var billingBoolFlags = map[string]bool{"json": true, "set": true, "clear": true, "help": true, "h": true} |
| + |
| +// billingArgsAreUnambiguous refuses two things the flag package accepts |
| +// and that change what is sent. A flag given twice: the second silently |
| +// wins, so a line that ends "--outcome keep --outcome retire" retires. |
| +// And a value that looks like a flag, taken as the value of the flag |
| +// before it: "--set --note --clear" sets the marker with the note |
| +// "--clear", and "--note --help" writes instead of helping. A value that |
| +// really does begin with a dash is written --note=<value>. |
| +func billingArgsAreUnambiguous(args []string) error { |
| + seen := map[string]bool{} |
| + for i := 0; i < len(args); i++ { |
| + arg := args[i] |
| + if arg == "--" || !strings.HasPrefix(arg, "-") || arg == "-" { |
| + // The first argument that is not a flag ends the flags; what |
| + // follows is refused by the caller as unexpected. |
| + return nil |
| + } |
| + name, _, inline := strings.Cut(strings.TrimLeft(arg, "-"), "=") |
| + if seen[name] { |
| + return fmt.Errorf("--%s was given twice", name) |
| + } |
| + seen[name] = true |
| + if inline || billingBoolFlags[name] { |
| + continue |
| + } |
| + if i+1 < len(args) && strings.HasPrefix(args[i+1], "-") && args[i+1] != "-" { |
| + return fmt.Errorf("the value of --%s is %q, which looks like a flag; if it is the value, write --%s=%s", name, args[i+1], name, args[i+1]) |
| + } |
| + i++ |
| + } |
| + return nil |
| +} |
| + |
| func runBilling(args []string) { |
| inv, err := parseBilling(args) |
| if err != nil { |
| @@ -351,68 +435,80 @@ func runBilling(args []string) { |
| } |
| log.Fatalf("billing: %v\n%s", err, billingUsage) |
| } |
| - ctx := context.Background() |
| - sess := billingSession(inv.Console) |
| + if err := billingRun(context.Background(), billingSession(inv.Console), inv, stdout); err != nil { |
| + log.Fatal(err) |
| + } |
| +} |
| + |
| +// billingRun carries out one understood invocation against one console |
| +// and prints what the console answered. Everything between the parsed |
| +// command line and the doors is here, so a test can drive it. |
| +func billingRun(ctx context.Context, sess *apiSession, inv billingInvocation, w io.Writer) error { |
| switch inv.Verb { |
| case "reviews": |
| reviews, raw, err := sess.billingReviews(ctx) |
| if err != nil { |
| - log.Fatalf("billing reviews: %v", err) |
| + return fmt.Errorf("billing reviews on %s: %w", sess.console, err) |
| } |
| if inv.JSON { |
| - fmt.Fprintln(stdout, string(raw)) |
| - return |
| + fmt.Fprintln(w, string(raw)) |
| + return nil |
| } |
| - printBillingReviews(stdout, reviews) |
| + printBillingReviews(w, reviews) |
| case "rejected": |
| rejected, raw, err := sess.billingRejected(ctx) |
| if err != nil { |
| - log.Fatalf("billing rejected: %v", err) |
| + return fmt.Errorf("billing rejected on %s: %w", sess.console, err) |
| } |
| if inv.JSON { |
| - fmt.Fprintln(stdout, string(raw)) |
| - return |
| + fmt.Fprintln(w, string(raw)) |
| + return nil |
| } |
| - printBillingRejected(stdout, rejected) |
| + printBillingRejected(w, rejected) |
| case "resolve": |
| retired, known, err := sess.billingResolve(ctx, inv.Resolve) |
| if err != nil { |
| - log.Fatalf("billing resolve on %s: %v", sess.console, err) |
| + return fmt.Errorf("billing resolve on %s: %w", sess.console, err) |
| } |
| line, err := billingResolvedLine(sess.console, inv.Resolve, retired, known) |
| if err != nil { |
| - log.Fatalf("billing resolve on %s: %v", sess.console, err) |
| + return fmt.Errorf("billing resolve on %s: %w", sess.console, err) |
| } |
| - fmt.Fprintln(stdout, line) |
| + fmt.Fprintln(w, line) |
| case "house": |
| if inv.HouseWrite { |
| if err := sess.billingHouseSet(ctx, inv.HouseAccount, inv.House, inv.HouseNote); err != nil { |
| - log.Fatalf("billing house on %s: %v", sess.console, err) |
| + return fmt.Errorf("billing house on %s: %w", sess.console, err) |
| } |
| - fmt.Fprintln(stdout, billingHouseLine(sess.console, inv.HouseAccount, inv.House)) |
| - return |
| + fmt.Fprintln(w, billingHouseLine(sess.console, inv.HouseAccount, inv.House)) |
| + return nil |
| } |
| house, raw, err := sess.billingHouse(ctx) |
| if err != nil { |
| - log.Fatalf("billing house: %v", err) |
| + return fmt.Errorf("billing house on %s: %w", sess.console, err) |
| } |
| if inv.JSON { |
| - fmt.Fprintln(stdout, string(raw)) |
| - return |
| + fmt.Fprintln(w, string(raw)) |
| + return nil |
| } |
| for _, sqid := range house { |
| - fmt.Fprintln(stdout, sqid) |
| + fmt.Fprintln(w, sqid) |
| } |
| if len(house) == 0 { |
| - mutedf(stdout, "no house accounts") |
| + mutedf(w, "no house accounts") |
| } |
| + default: |
| + return fmt.Errorf("billing: no such subcommand: %s", inv.Verb) |
| } |
| + return nil |
| } |
| // billingResolvedLine says what the console acknowledged, on which |
| // console, in terms of the outcome that was asked for: the console's |
| -// retired answer says only whether an intent was retired, and a retire |
| -// that found no intent standing retired nothing, which is not a keep. |
| +// retired answer says only whether an intent was retired: a retire that |
| +// found no intent standing retired nothing, which is not a keep; and a |
| +// keep says what was decided, not that the intent still stands (the |
| +// answer to an exact repeat is the same after its purchase recorded). |
| func billingResolvedLine(console string, r apiBillingResolution, retired, known bool) (string, error) { |
| line := fmt.Sprintf("on %s: resolved %s %s (%s)", console, r.Account, r.Subject, r.Reason) |
| if !known { |
| @@ -422,7 +518,9 @@ func billingResolvedLine(console string, r apiBillingResolution, retired, known |
| case r.Outcome == "keep" && retired: |
| return "", errors.New("the console answered that a keep retired the intent. list the reviews and read what it recorded") |
| case r.Outcome == "keep": |
| - return line + ": the intent stands, for its purchase to record", nil |
| + // Not "the intent stands": an exact repeat of a keep answers the |
| + // same after the purchase has recorded and the intent has gone. |
| + return line + ": kept (nothing was retired)", nil |
| case retired: |
| return line + ": the intent was retired", nil |
| } |
| @@ -485,12 +583,13 @@ func printBillingReviews(w io.Writer, reviews []apiBillingReview) { |
| } |
| // billingAvailability words one outcome's state. Not held means not held |
| -// by what the console has recorded: the resolution still reads the |
| -// provider (keep requires the session, read then, to be complete, paid |
| -// and the intent's own purchase) and can still be refused. |
| +// by what the console has recorded. A keep reads the session then and |
| +// requires it complete, paid and the intent's own purchase; a retire of a |
| +// session already recorded expired reads nothing more. Either can still |
| +// be refused. |
| func billingAvailability(held string) string { |
| if held == "" { |
| - return "not held by what is recorded (the console still reads the provider when asked)" |
| + return "not held by what is recorded (the resolution may still read the provider, and refuse)" |
| } |
| return "held: " + held |
| } |
diff --git a/billing_cmd_test.go b/billing_cmd_test.go| index f49b6f1f5..f019ddc0c 100644 |
| --- a/billing_cmd_test.go |
| +++ b/billing_cmd_test.go |
| @@ -7,6 +7,7 @@ import ( |
| "errors" |
| "flag" |
| "io" |
| + "net" |
| "net/http" |
| "net/http/httptest" |
| "os" |
| @@ -84,10 +85,10 @@ 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": 1790000000, "last_seen_at": 1790003600, "reference": "cs_test_1", |
| + 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", |
| "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", |
| - "disposition": c.named("disposition", map[string]any{"fate": "", "disputed": false, "obligations": []string{}, "retire_held": "", "keep_held": c.wire["no_intent_stands"][0]})}) |
| + "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} |
| answer(map[string]any{c.wire["envelopes"][2]: []any{refusal, gone, unread, other}}) |
| @@ -150,6 +151,16 @@ func TestBillingReviewsListing(t *testing.T) { |
| if c.bearer != "Bearer tok_operator" { |
| t.Fatalf("authorization %q", c.bearer) |
| } |
| + // Every field the client decodes, against a value that is not its |
| + // zero: a tag renamed on the client alone reads as the zero value. |
| + first := reviews[0] |
| + if first.Account != "bac" || first.Subscription != "ci_1" || first.Reason != billingCheckoutRefused || first.Occurrences != 2 || first.CreatedAt != 1789990000 || first.LastSeenAt != 1790003600 || first.Reference != "cs_test_1" || |
| + 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) |
| + } |
| var buf bytes.Buffer |
| printBillingReviews(&buf, reviews) |
| out := buf.String() |
| @@ -157,7 +168,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", "retire: not held by what is recorded", "keep: held: no such intent stands", |
| + "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", |
| "baf ci_3 billing_checkout_currency_refused", "disposition unavailable", |
| "bad sub_9 billing_location_changed occurrences 1", |
| } { |
| @@ -200,7 +211,19 @@ func TestBillingArgumentsAreOneRequestOrRefused(t *testing.T) { |
| {"no occurrences", []string{"resolve", "--account", "bac", "--subject", "ci_1", "--reason", "r", "--note", "n"}, "are required"}, |
| {"a blank note", []string{"resolve", "--account", "bac", "--subject", "ci_1", "--reason", "r", "--occurrences", "1", "--note", " "}, "--note is required"}, |
| {"a note too long", []string{"resolve", "--account", "bac", "--subject", "ci_1", "--reason", "r", "--occurrences", "1", "--note", long}, "at most 256"}, |
| - {"a note of two lines", []string{"house", "--account", "bac", "--set", "--note", "one\ntwo"}, "plain text on one line"}, |
| + {"a note of two lines", []string{"house", "--account", "bac", "--set", "--note", "one\ntwo"}, "control character"}, |
| + {"a flag taken as the note", []string{"house", "--account", "bac", "--set", "--note", "--clear"}, `the value of --note is "--clear"`}, |
| + {"help taken as the note", []string{"house", "--account", "bac", "--set", "--note", "--help"}, `the value of --note is "--help"`}, |
| + {"a flag taken as the account", []string{"resolve", "--account", "--subject", "ci_1", "--reason", "r", "--occurrences", "1", "--note", "n"}, `the value of --account is "--subject"`}, |
| + {"the outcome given twice", []string{"resolve", "--account", "bac", "--subject", "ci_1", "--reason", billingCheckoutRefused, "--occurrences", "1", "--note", "n", "--outcome", "keep", "--outcome", "retire"}, "--outcome was given twice"}, |
| + {"the account given twice", []string{"house", "--account", "bac", "--account", "bad", "--set", "--note", "n"}, "--account was given twice"}, |
| + {"the console given twice, once inline", []string{"reviews", "--console=https://a.example", "--console", "https://b.example"}, "--console was given twice"}, |
| + {"an empty console", []string{"reviews", "--console", ""}, "--console was given with no value"}, |
| + {"an empty console on a write", []string{"house", "--account", "bac", "--set", "--note", "n", "--console", ""}, "--console was given with no value"}, |
| + {"an empty outcome", []string{"resolve", "--account", "bac", "--subject", "ci_1", "--reason", billingCheckoutRefused, "--occurrences", "1", "--note", "n", "--outcome", ""}, "--outcome was given with no value"}, |
| + {"an empty outcome for another reason", []string{"resolve", "--account", "bac", "--subject", "sub_9", "--reason", "billing_location_changed", "--occurrences", "1", "--note", "n", "--outcome="}, "--outcome was given with no value"}, |
| + {"a note with a tab", []string{"house", "--account", "bac", "--set", "--note", "one\ttwo"}, "control character"}, |
| + {"a note that is not text", []string{"house", "--account", "bac", "--set", "--note", "\xff\xfe"}, "not valid text"}, |
| {"no subcommand", nil, "a subcommand is required"}, |
| {"an unknown subcommand", []string{"settle"}, "no such subcommand"}, |
| {"the internal form's name", []string{"house write"}, "no such subcommand"}, |
| @@ -235,6 +258,182 @@ func TestBillingArgumentsAreOneRequestOrRefused(t *testing.T) { |
| if _, err := parseBilling([]string{"reviews", "--help"}); !errors.Is(err, flag.ErrHelp) { |
| t.Fatalf("--help: %v", err) |
| } |
| + // A value that really begins with a dash is given inline. |
| + inv, err = parseBilling([]string{"house", "--account", "bac", "--set", "--note=--clear the old marker"}) |
| + if err != nil || inv.HouseNote != "--clear the old marker" || !inv.House { |
| + t.Fatalf("an inline note beginning with a dash: %+v %v", inv, err) |
| + } |
| + // The note's limit is the console's: bytes, at most 256, and its own |
| + // excluded characters (below U+0020, and U+007F), no others. |
| + exact := strings.Repeat("é", billingNoteMax/2) |
| + if inv, err = parseBilling([]string{"house", "--account", "bac", "--set", "--note", exact}); err != nil || inv.HouseNote != exact { |
| + t.Fatalf("a note of exactly the limit in bytes: %v", err) |
| + } |
| + if _, err = parseBilling([]string{"house", "--account", "bac", "--set", "--note", exact + "é"}); err == nil || !strings.Contains(err.Error(), "at most 256") { |
| + t.Fatalf("a note two bytes over the limit: %v", err) |
| + } |
| + for name, note := range map[string]string{"a C1 control": "one\u0085two", "a line separator": "one\u2028two"} { |
| + if _, err := parseBilling([]string{"house", "--account", "bac", "--set", "--note", note}); err != nil { |
| + t.Errorf("a note with %s, which the console accepts: %v", name, err) |
| + } |
| + } |
| + if _, err = parseBilling([]string{"house", "--account", "bac", "--set", "--note", "one\x7ftwo"}); err == nil { |
| + t.Fatal("a note with U+007F was accepted") |
| + } |
| +} |
| + |
| +// billingRun connects a parsed command line to the doors: what the parser |
| +// understood is what is sent, on the session it was given, and what is |
| +// printed is the console's answer. |
| +func TestBillingRunSendsWhatWasParsedAndPrintsTheAnswer(t *testing.T) { |
| + c := &billingConsole{t: t} |
| + sess := billingTestSession(t, c) |
| + run := func(args ...string) (string, error) { |
| + t.Helper() |
| + inv, err := parseBilling(args) |
| + if err != nil { |
| + t.Fatalf("parse %v: %v", args, err) |
| + } |
| + var buf bytes.Buffer |
| + err = billingRun(context.Background(), sess, inv, &buf) |
| + return buf.String(), err |
| + } |
| + out, err := run("house", "--account", "bac", "--clear", "--note", "left the team") |
| + if err != nil || out != "on "+sess.console+": bac is not a house account\n" { |
| + t.Fatalf("house clear: %q %v", out, err) |
| + } |
| + if body := c.bodies[len(c.bodies)-1]; body["house"] != false || body["account"] != "bac" || body["note"] != "left the team" { |
| + t.Fatalf("house clear sent %v", body) |
| + } |
| + out, err = run("house", "--account", "bac", "--set", "--note", "ours") |
| + if err != nil || out != "on "+sess.console+": bac is a house account\n" || c.bodies[len(c.bodies)-1]["house"] != true { |
| + t.Fatalf("house set: %q %v %v", out, err, c.bodies[len(c.bodies)-1]) |
| + } |
| + out, err = run("resolve", "--account", "bac", "--subject", "ci_1", "--reason", billingCheckoutRefused, "--occurrences", "2", "--note", "paid", "--outcome", "keep") |
| + if err != nil || out != "on "+sess.console+": resolved bac ci_1 ("+billingCheckoutRefused+"): kept (nothing was retired)\n" { |
| + t.Fatalf("keep: %q %v", out, err) |
| + } |
| + if body := c.bodies[len(c.bodies)-1]; body["outcome"] != "keep" || body["subscription"] != "ci_1" || body["occurrences"] != float64(2) { |
| + t.Fatalf("keep sent %v", body) |
| + } |
| + c.retired = true |
| + out, err = run("resolve", "--account", "bac", "--subject", "ci_1", "--reason", billingCheckoutRefused, "--occurrences", "2", "--note", "expired") |
| + if err != nil || !strings.HasSuffix(out, ": the intent was retired\n") { |
| + t.Fatalf("retire: %q %v", out, err) |
| + } |
| + if _, sent := c.bodies[len(c.bodies)-1]["outcome"]; sent { |
| + t.Fatalf("retire with no --outcome sent one: %v", c.bodies[len(c.bodies)-1]) |
| + } |
| + writes := len(c.bodies) |
| + out, err = run("reviews", "--json") |
| + var listing map[string]any |
| + if err != nil || json.Unmarshal([]byte(out), &listing) != nil || listing["reviews"] == nil { |
| + t.Fatalf("reviews --json: %q %v", out, err) |
| + } |
| + if out, err = run("reviews"); err != nil || !strings.Contains(out, "bac ci_1") || strings.HasPrefix(strings.TrimSpace(out), "{") { |
| + t.Fatalf("reviews: %q %v", out, err) |
| + } |
| + if out, err = run("rejected"); err != nil || !strings.Contains(out, time.Unix(1790003600, 0).UTC().Format("2006-01-02 15:04")+" evt_2") { |
| + t.Fatalf("rejected: %q %v; want the row with the time it was received", out, err) |
| + } |
| + if out, err = run("house"); err != nil || out != "ops\n" { |
| + t.Fatalf("house: %q %v", out, err) |
| + } |
| + if out, err = run("house", "--json"); err != nil || !strings.Contains(out, `"house"`) { |
| + t.Fatalf("house --json: %q %v", out, err) |
| + } |
| + if len(c.bodies) != writes { |
| + t.Fatalf("a listing made %d writes", len(c.bodies)-writes) |
| + } |
| + // A refusal by the console is this command's failure, named with the |
| + // console, and prints nothing as done. |
| + c.write = func(w http.ResponseWriter) { |
| + w.WriteHeader(http.StatusConflict) |
| + _, _ = io.WriteString(w, `{"error":"held: its session is not yet proven expired. the review stays open"}`) |
| + } |
| + out, err = run("resolve", "--account", "bac", "--subject", "ci_1", "--reason", billingCheckoutRefused, "--occurrences", "2", "--note", "x") |
| + if err == nil || out != "" || err.Error() != "billing resolve on "+sess.console+": held: its session is not yet proven expired. the review stays open" { |
| + t.Fatalf("a held resolution: %q %v", out, err) |
| + } |
| +} |
| + |
| +// A 200 that is not the door's listing is not an empty listing. |
| +func TestBillingListingsRequireTheirEnvelope(t *testing.T) { |
| + for name, body := range map[string]string{ |
| + "an empty object": `{}`, |
| + "null": `null`, |
| + "an error under a 200": `{"error":"unavailable"}`, |
| + "the key with null": `{"reviews":null,"rejected":null,"house":null}`, |
| + "the key with an object": `{"reviews":{},"rejected":{},"house":{}}`, |
| + "a page from something else": `<html><body>welcome</body></html>`, |
| + } { |
| + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, body) })) |
| + sess := &apiSession{c: newConsoleClient(srv.URL, "tok"), console: srv.URL, address: "ops@keymail.dev"} |
| + if rows, _, err := sess.billingReviews(context.Background()); !errors.Is(err, errBillingNotAListing) { |
| + t.Errorf("%s: reviews %v %v; want it refused as not a listing", name, rows, err) |
| + } |
| + if rows, _, err := sess.billingRejected(context.Background()); !errors.Is(err, errBillingNotAListing) { |
| + t.Errorf("%s: rejected %v %v", name, rows, err) |
| + } |
| + if rows, _, err := sess.billingHouse(context.Background()); !errors.Is(err, errBillingNotAListing) { |
| + t.Errorf("%s: house %v %v", name, rows, err) |
| + } |
| + srv.Close() |
| + } |
| + // The doors' own empty answers are empty listings. |
| + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + _, _ = io.WriteString(w, `{"reviews":[],"rejected":[],"house":[]}`) |
| + })) |
| + defer srv.Close() |
| + sess := &apiSession{c: newConsoleClient(srv.URL, "tok"), console: srv.URL, address: "ops@keymail.dev"} |
| + if rows, _, err := sess.billingReviews(context.Background()); err != nil || len(rows) != 0 { |
| + t.Fatalf("an empty listing: %v %v", rows, err) |
| + } |
| + if rows, _, err := sess.billingHouse(context.Background()); err != nil || len(rows) != 0 { |
| + t.Fatalf("an empty house listing: %v %v", rows, err) |
| + } |
| +} |
| + |
| +// A write that errors instead of answering: a console that says it has |
| +// moved, and a connection never made, changed nothing and say so as they |
| +// are; anything else may have been a change, with advice that fits the |
| +// write. |
| +func TestBillingWriteErrorsSayWhatIsKnown(t *testing.T) { |
| + moved := &consoleMovedError{From: "https://old.example", To: "https://new.example"} |
| + if err := billingWriteUnknown(moved, "x"); err != error(moved) { |
| + t.Fatalf("a console that moved: %v; want it as it is", err) |
| + } |
| + dial := &net.OpError{Op: "dial", Err: errors.New("connection refused")} |
| + if err := billingWriteUnknown(dial, "x"); err != error(dial) { |
| + t.Fatalf("a connection never made: %v; want it as it is", err) |
| + } |
| + lost := &net.OpError{Op: "read", Err: errors.New("connection reset")} |
| + if err := billingWriteUnknown(lost, "list the house accounts"); err == nil || !strings.Contains(err.Error(), "may have made the change: list the house accounts") || !errors.Is(err, lost) { |
| + t.Fatalf("an answer lost: %v", err) |
| + } |
| + // Through the session, against a console that is not there. |
| + gone := httptest.NewServer(http.NotFoundHandler()) |
| + url := gone.URL |
| + gone.Close() |
| + sess := &apiSession{c: newConsoleClient(url, "tok"), console: url, address: "ops@keymail.dev"} |
| + if err := sess.billingHouseSet(context.Background(), "bac", true, "n"); err == nil || strings.Contains(err.Error(), "may have made the change") { |
| + t.Fatalf("a house write to a console that is not listening: %v; want the failure with no suggestion that it changed anything", err) |
| + } |
| + // And against one that drops the connection after the request: the |
| + // two writes give their own advice. |
| + c := &billingConsole{t: t} |
| + sess = billingTestSession(t, c) |
| + c.write = func(w http.ResponseWriter) { |
| + hj, _ := w.(http.Hijacker) |
| + conn, _, _ := hj.Hijack() |
| + conn.Close() |
| + } |
| + if err := sess.billingHouseSet(context.Background(), "bac", true, "n"); err == nil || !strings.Contains(err.Error(), "list the house accounts before deciding") { |
| + t.Fatalf("a house write whose answer was lost: %v", err) |
| + } |
| + if _, _, err := sess.billingResolve(context.Background(), apiBillingResolution{Account: "bac", Subject: "ci_1", Reason: "r", Occurrences: 1, Note: "n"}); err == nil || !strings.Contains(err.Error(), "repeat this exact command on the same console") { |
| + t.Fatalf("a resolution whose answer was lost: %v", err) |
| + } |
| } |
| // resolve sends exactly the contract's request, reads the console's |
| @@ -279,6 +478,7 @@ func TestBillingResolveSendsTheContractAndReadsTheAcknowledgement(t *testing.T) |
| "resolved false": `{"account":"bac","subscription":"ci_1","reason":"billing_checkout_currency_refused","resolved":false,"retired":false}`, |
| "another account": `{"account":"zzz","subscription":"ci_1","reason":"billing_checkout_currency_refused","resolved":true,"retired":true}`, |
| "another subject": `{"account":"bac","subscription":"ci_9","reason":"billing_checkout_currency_refused","resolved":true,"retired":true}`, |
| + "another reason": `{"account":"bac","subscription":"ci_1","reason":"billing_location_changed","resolved":true,"retired":true}`, |
| "no retired for a refusal": `{"account":"bac","subscription":"ci_1","reason":"billing_checkout_currency_refused","resolved":true}`, |
| "a page from something else": `<html><body>welcome</body></html>`, |
| } { |
| @@ -321,11 +521,11 @@ func TestBillingResolvedLine(t *testing.T) { |
| {refusal, true, true, "on https://c.example: resolved bac ci_1 (billing_checkout_currency_refused): the intent was retired"}, |
| {explicit, true, true, "the intent was retired"}, |
| {refusal, false, true, "no intent stood, so nothing was retired"}, |
| - {keep, false, true, "the intent stands, for its purchase to record"}, |
| + {keep, false, true, "): kept (nothing was retired)"}, |
| {apiBillingResolution{Account: "bad", Subject: "sub_9", Reason: "billing_location_changed"}, false, false, "on https://c.example: resolved bad sub_9 (billing_location_changed)"}, |
| } { |
| got, err := billingResolvedLine("https://c.example", tc.r, tc.retired, tc.known) |
| - if err != nil || !strings.Contains(got, tc.want) || strings.Contains(got, "kept") { |
| + if err != nil || !strings.Contains(got, tc.want) || strings.Contains(got, "stands") || (tc.r.Outcome != "keep" && strings.Contains(got, "kept")) { |
| t.Errorf("%+v retired %v known %v: %q %v; want %q", tc.r, tc.retired, tc.known, got, err, tc.want) |
| } |
| if !tc.known && strings.Contains(got, "intent") { |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index a86dd1232..ce736fbfa 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -1279,9 +1279,12 @@ allows it. A refusal it holds comes back in its own words and stays open. |
| `--set` or `--clear`. |
| Every write names the console it acted on. The console is `--console`, else |
| -`CARLOS_CONSOLE`, else the one your project's `.carlos/config` names, else |
| -the only one you are logged in to. Quote a note of more than one word: an |
| -argument the command would otherwise drop is refused before anything is sent. |
| +`CARLOS_CONSOLE`, else the one your configuration names (the file |
| +`CARLOS_CONFIG` names, else a `.carlos/config` above the current directory, |
| +else `~/.carlos/config`), else the only one you are logged in to. For a |
| +write, pass `--console`. Quote a note of more than one word and give no flag |
| +twice: a command line that could mean two things is refused before anything |
| +is sent. |
| Operator only, and the console answers everyone else with the same bare 404 |
| it gives an unknown route. |
diff --git a/implementation/carloku-paid-launch/billing-operator.md b/implementation/carloku-paid-launch/billing-operator.md| index c678f390c..d7ec51365 100644 |
| --- a/implementation/carloku-paid-launch/billing-operator.md |
| +++ b/implementation/carloku-paid-launch/billing-operator.md |
| @@ -1,10 +1,10 @@ |
| # The operator's billing doors, from a terminal |
| -Written 2026-10-02 on `feat/carloku-billing-operator-cli-20261002` (off the multi-currency branch); revised the same day after its first review. Internal. The public reference has a short `carlos billing` section (`docs/site/cli.md`), which is new copy and waits for Paul's review before this branch merges. The verb is not in `carlos`'s top-level usage. Billing is OFF everywhere; nothing here has run against a live console. |
| +Written 2026-10-02 on `feat/carloku-billing-operator-cli-20261002` (off the multi-currency branch); revised the same day after its first and second reviews. Internal. The public reference has a short `carlos billing` section (`docs/site/cli.md`), which is new copy and waits for Paul's review before this branch merges. The verb is not in `carlos`'s top-level usage. Billing is OFF everywhere; nothing here has run against a live console. |
| `carlos billing` is a client for four console API doors. They answer only a platform operator (`users.operator`, set on the box with the console's own tooling). Any other signed-in address gets a plain 404; no credential, or an expired or revoked one, gets a 401 that says to log in. |
| -The client decides nothing about billing. It does refuse, before sending anything, a command line that is not one unambiguous request: an argument it would otherwise drop (a note typed without quotes loses its second word and every flag after it), a flag the subcommand does not use, a note the console would refuse (empty, over 256 bytes, or more than one line). It reports a write as done only when the console's answer acknowledges it, and says which console it acted on. |
| +The client decides nothing about billing. It does refuse, before sending anything, a command line that is not one unambiguous request: an argument it would otherwise drop (a note typed without quotes loses its second word and every flag after it); a flag the subcommand does not use; a flag given twice; a flag given with an empty value (an unset shell variable in `--console "$TARGET"`); a value that looks like a flag (write `--note=--like-this` if it really is the value); a note the console would refuse (empty, over 256 bytes, or containing a control character such as a tab or a line break). It reports a write as done only when the console's answer acknowledges it, and says which console it acted on. |
| ``` |
| carlos billing reviews [--console <url>] [--json] |
| @@ -16,7 +16,7 @@ carlos billing house --account <sqid> (--set | --clear) --note <text> [--cons |
| ## Which console |
| -`--console`, else `CARLOS_CONSOLE`, else the console a `.carlos/config` above the current directory names, else the only one you are logged in to. With two logins and a project directory that names one of them, a write with no `--console` goes to that one. Sqids are per console: the same sqid can name a different account on another. Every write's confirmation starts `on <console>:`. Read it. For a write, pass `--console`. |
| +`--console`, else `CARLOS_CONSOLE`, else the console your configuration names (the file `CARLOS_CONFIG` names, and nothing else if that is set; else a `.carlos/config` above the current directory; else `~/.carlos/config`), else the only one you are logged in to. With two logins and a configuration that names one of them, from anywhere, a write with no `--console` goes to that one. Sqids are per console: the same sqid can name a different account on another. Every write's confirmation starts `on <console>:`, which is after the write. For a write, pass `--console`. |
| With `CARLOS_DEPLOYMENT_BUCKET` or `CARLOS_DEPLOYMENT_DIR` set the verb refuses to run: unset it. Logging in does not change that. |
| @@ -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 also stops the account starting any purchase. 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": the resolution reads the provider again and can still refuse. 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. |
| ## resolve |
| @@ -39,15 +39,15 @@ Resolving is an act, not an acknowledgement. Know what the hold is about first. |
| For a checkout refusal the outcome is `retire` (the default) or `keep`. `--outcome` is refused for any other reason. |
| - **Either outcome can expire the session.** Before it judges the outcome, the console works the refusal toward a proven fate: if the bound session is still open at the provider, it requests its expiry and reads it again. Asking to `keep` an open session expires it and then answers that keep is held. |
| -- `retire` is accepted for a session the provider shows expired and unpaid, with nothing attached; for an intent that never had a session at all; and for a review whose intent no longer stands (nothing is retired then, and the confirmation says so). While the provider cannot be read the answer is "try again"; while the expiry is not proven it is "held". |
| +- `retire` is accepted for a session the provider shows expired and unpaid, with nothing attached; for an intent that never had a session at all; and for a review whose intent no longer stands (nothing is retired then, and the confirmation says so). Where the console has to read the provider and cannot, the answer is "try again"; where the expiry is already recorded it reads nothing and the retire goes through; while the expiry is not proven it is "held". |
| - `keep` is accepted for a completed session, paid or with no payment required, that is the intent's own purchase, with no subscription attached but its own. It leaves the intent standing for the next reconciliation to record the purchase. It does not record it and does not guarantee it: a subscription that is still not the purchase is refused again and the review comes back as a new occurrence. |
| - Anything else is **held** and stays under review: a paid session that is not kept, a case a subscription made outside checkout is attached to, a case the provider contradicted itself about, and a review opened before the retirement protocol. The answer says which. There is no outcome for these yet; what disposes of them is decisions 1 to 5 in `DECISIONS-FOR-PAUL.md`. |
| -The confirmation says what the console acknowledged in terms of what you asked: the intent was retired; no intent stood, so nothing was retired; or the intent stands for its purchase to record. |
| +The confirmation says what the console acknowledged in terms of what you asked: the intent was retired; no intent stood, so nothing was retired; or kept (nothing was retired). "Kept" is the decision, not the present state: an exact repeat of a keep answers the same after the purchase has recorded and the intent has gone. |
| Repeating a resolution exactly (the same operator, the same note, the same outcome) answers as the first time. 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 three provider calls before it answers), the change may have been made. List again, or repeat the command exactly. A reworded note is not an exact repeat. |
| +If the answer never arrives (a timeout: the console can make three provider calls before it answers), 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 |