amadan
cli: review round — alias-aware enable gate, honest fold, right usage pages
Adversarial review findings, all taken: - instances enable gated its channel/seed/operator flags on a raw fs.Visit name set; -c edge visited "c", read as absent, and the user's explicit channel was silently dropped (signed-in mode) or the explicit-empty refusal became a silent stable default (store mode). All gates now go through the alias-aware flagWasSet. - rollback -h printed the ledger page and release-keygen -h the status page (copy-paste headers); each now prints its own. - the printer folds a short flag into a primary only when it carries the registrar's marker usage — a command registering its own -v would otherwise be hidden behind a lying "--version, -v" row (pinned by test); "0s" joins the zero-default skip list. - economics bill's half-swept synopsis finished, and the hand-kept flag lists in economics/instances/steering usage consts deleted (the generated list was double-printing against them, disagreeing about defaults); the two per-flag nuances worth keeping moved into the registered usage strings. - docs: the vetted app's contract flag reads -socket again (it names the APP's flag, not carlos's); the console settings template's migrate-app suggestion finishes its sweep (--to). - host-side verbs (edge, agent, adopt, bootstrap, ops) wire the shared printer too — their pages were still stock single-dash.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
11 files changed,
+67
−96
cli.go+4 −4docs/site/cli.md+2 −2economics.go+2 −12flags_test.go+20 −0instances_cli.go+11 −71internal/cliflag/cliflag.go+11 −6internal/console/templates/app_settings.html+1 −1internal/edge/edge.go+3 −0internal/host/agent.go+5 −0internal/host/bootstrap.go+3 −0internal/ops/ops.go+5 −0
diff --git a/cli.go b/cli.go| index 3f780fe36..27393f147 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -2459,9 +2459,9 @@ func runRollback(args []string) { |
| app := appFlag(fs, "") |
| acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to roll back through (API mode only; default: the only logged-in one)") |
| - fs.Usage = flagUsage(fs, ledgerUsage) |
| - fs.Parse(args) |
| usage := "usage: carlos rollback --app <a> [--account <sqid-or-name>] [--account-id <sqid>] [--console <origin>] <channel>" |
| + fs.Usage = flagUsage(fs, usage) |
| + fs.Parse(args) |
| if fs.NArg() != 1 { |
| log.Fatal(usage) |
| } |
| @@ -2871,12 +2871,12 @@ func runKeygen(args []string) { |
| scope := fs.String("scope", "", "mint a SCOPED key for these rungs, comma separated (canary,edge,beta) — stable is never delegable") |
| name := fs.String("name", "", "label for the scoped key; it appears in every refusal it causes") |
| expires := fs.Duration("expires", defaultGrantTTL, "how long the scoped key may sign for (max 8760h)") |
| - fs.Usage = flagUsage(fs, statusUsage) |
| + fs.Usage = flagUsage(fs, "") |
| fs.Parse(args) |
| if *scope == "" { |
| var stray []string |
| - fs.Visit(func(f *flag.Flag) { stray = append(stray, "-"+f.Name) }) |
| + fs.Visit(func(f *flag.Flag) { stray = append(stray, "--"+f.Name) }) |
| if len(stray) > 0 { |
| log.Fatalf("%s only apply with --scope; without it this mints the deployment's ROOT key", strings.Join(stray, " and ")) |
| } |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index ed2c49cd3..211af519a 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -440,7 +440,7 @@ have to match the sha256 and size it claims. |
| carlos vet --app hello --version a1b2c3d --boot |
| ``` |
| -`--boot` goes further and runs the binary: it must accept `--socket <path>`, |
| +`--boot` goes further and runs the binary: it must accept `-socket <path>`, |
| serve HTTP on that socket, and answer `GET /healthz` with a 200 within ten |
| seconds. That is the entire app contract, and this is the cheapest place to |
| find out you have broken it. |
| @@ -610,7 +610,7 @@ Records one month's AWS bill so the console's economics dashboard has a real |
| number to divide by. |
| ```sh |
| -carlos economics bill --month 2026-08 -total 412.55 -line AmazonEC2=55.00 |
| +carlos economics bill --month 2026-08 --total 412.55 --line AmazonEC2=55.00 |
| ``` |
| Operator only, and the console answers everyone else with the same bare 404 |
diff --git a/economics.go b/economics.go| index 79053cfb1..e80a62ba5 100644 |
| --- a/economics.go |
| +++ b/economics.go |
| @@ -28,7 +28,7 @@ import ( |
| "time" |
| ) |
| -const economicsUsage = `usage: carlos economics bill --month <YYYY-MM> -total <usd> [-line Service=usd ...] [--console <origin>] |
| +const economicsUsage = `usage: carlos economics bill --month <YYYY-MM> --total <usd> [--line Service=usd ...] [--console <origin>] |
| carlos economics backhaul [--period mtd|last|7d] [--console <origin>] |
| Records one month's AWS bill for the economics dashboard. Operator only: the |
| @@ -36,16 +36,6 @@ console answers a non-operator's token with the same bare 404 it gives an |
| unknown route (it must not be probeable), so "not found" here almost always |
| means your account lacks the fleet-operator bit. |
| -flags: |
| - --month <YYYY-MM> the billing month, e.g. 2026-08 (required) |
| - -total <usd> the invoice total in US dollars (required; must be |
| - a finite, non-negative number) |
| - -line Service=usd one line item, repeatable — e.g. |
| - -line AmazonEC2=55.00 -line AmazonS3=12.30 |
| - (amount must be finite and non-negative) |
| - --console <origin> console to record through (signed-in mode only; |
| - default: the only logged-in one) |
| - |
| A manual entry always overwrites whatever the console already holds for that |
| month — a previous Cost Explorer pull or an earlier manual entry alike. |
| ` |
| @@ -301,7 +291,7 @@ func runEconomicsBill(args []string) { |
| log.Fatalf("economics bill: %v", err) |
| } |
| if err := validAmount(*total); err != nil { |
| - log.Fatalf("economics bill: -total %v", err) |
| + log.Fatalf("economics bill: --total %v", err) |
| } |
| sess, err := resolveMode(*consoleFlag) |
diff --git a/flags_test.go b/flags_test.go| index 57e3766f9..3493f890a 100644 |
| --- a/flags_test.go |
| +++ b/flags_test.go |
| @@ -162,3 +162,23 @@ func TestFlagUsageShowsRealDefaultsOnly(t *testing.T) { |
| t.Errorf("empty repeatable default shown:\n%s", out) |
| } |
| } |
| + |
| +// A command that registers its own single-letter flag colliding with a |
| +// claimed letter — without the registrar — must get honest help: no fold |
| +// (the pair shares no value), and both rows printed. The fold is |
| +// recognised by the alias's marker usage, not by name alone. |
| +func TestFlagUsageDoesNotFoldAnImpostorLetter(t *testing.T) { |
| + fs := quietFlagSet("t") |
| + fs.String("version", "", "version to act on") |
| + fs.Bool("v", false, "chatty output") |
| + out := printedUsage(fs, "") |
| + if !strings.Contains(out, "--version <string>") { |
| + t.Errorf("--version row missing or wrongly folded:\n%s", out) |
| + } |
| + if strings.Contains(out, "--version, -v") { |
| + t.Errorf("folded -v into --version though they share no value:\n%s", out) |
| + } |
| + if !strings.Contains(out, "\n -v ") { |
| + t.Errorf("-v row hidden:\n%s", out) |
| + } |
| +} |
diff --git a/instances_cli.go b/instances_cli.go| index 896ad5ae5..7096f883b 100644 |
| --- a/instances_cli.go |
| +++ b/instances_cli.go |
| @@ -44,44 +44,18 @@ store-mode flags and are refused here. |
| Store mode (CARLOS_DEPLOYMENT_BUCKET/CARLOS_DEPLOYMENT_DIR set): every field |
| is the operator's call, and --domain is required. |
| -flags: |
| - --app <a> app whose instances this enables (required) |
| - --domain <d> every instance host must be exactly one DNS label |
| - + "." + this domain (store mode: required; |
| - signed-in mode: refused — the console pins it) |
| - --pool <p> pool stamped into every record at create (store |
| - mode only; default: "", the shared pool) |
| - --channel <c> release channel stamped into every record at |
| - create (store-mode default "stable"; signed-in |
| - mode: omitted = preserve what the manifest holds) |
| - --regions <r1,r2,...> allow-list for a record's region (store mode |
| - only); empty (default) = any shape-valid region |
| - --seed "args..." argv of the app's first-boot init command, |
| - whitespace-split via strings.Fields — shell-word |
| - semantics (quoting, escaping) are deliberately NOT |
| - supported here; anything richer belongs in the app |
| - --account <a> account (sqid or name) the app belongs to |
| - (default "ops") |
| - --account-id <id> account sqid, matched as a sqid ONLY — supersedes |
| - --account |
| - --console <origin> console to enable through (signed-in mode only; |
| - default: the only logged-in one) |
| - |
| -Writing this manifest is the app's opt-in to instance provisioning: absence |
| -means the feature does not exist for the app, and the API refuses creates |
| -until this is set (spec §3.1). |
| ` |
| func runInstancesEnable(args []string) { |
| fs := flag.NewFlagSet("instances enable", flag.ExitOnError) |
| app := appFlag(fs, "app whose instances this enables") |
| - domain := fs.String("domain", "", "instance host domain suffix (store mode only)") |
| + domain := fs.String("domain", "", "instance host domain suffix (store mode: required; signed-in mode: refused — the console pins it)") |
| pool := fs.String("pool", "", "pool stamped into every record at create (store mode only)") |
| // Default "" so signed-in mode can tell "unset" from "asked for": an |
| // omitted --channel must preserve whatever the manifest already carries, |
| // not silently reset it to stable. Store mode applies the documented |
| // "stable" default below, so its behaviour is unchanged. |
| - channel := channelFlag(fs, "release channel stamped into every record at create") |
| + channel := channelFlag(fs, "release channel stamped into every record at create (store-mode default stable; signed-in mode: omitted preserves the manifest's)") |
| regions := fs.String("regions", "", "comma-separated region allow-list (store mode only); empty = any shape-valid region") |
| seed := fs.String("seed", "", "argv of the app's first-boot init command, whitespace-split (strings.Fields)") |
| consoleFlag := fs.String("console", "", "console to enable through (signed-in mode only; default: the only logged-in one)") |
| @@ -91,9 +65,6 @@ func runInstancesEnable(args []string) { |
| if *app != "" && !release.ValidApp(*app) { |
| log.Fatalf("carlos instances enable: %q is not a valid app name", *app) |
| } |
| - set := map[string]bool{} |
| - fs.Visit(func(f *flag.Flag) { set[f.Name] = true }) |
| - |
| sess, err := resolveMode(*consoleFlag) |
| if err != nil { |
| log.Fatal(err) |
| @@ -103,16 +74,16 @@ func runInstancesEnable(args []string) { |
| // ignored — a flag that silently did nothing would read as policy |
| // applied. |
| for _, f := range []string{"domain", "pool", "regions"} { |
| - if set[f] { |
| + if flagWasSet(fs, f) { |
| log.Fatalf("carlos instances enable: -%s is an operator, store-mode flag — the console pins the manifest to your account's platform subdomain. Set CARLOS_DEPLOYMENT_BUCKET/CARLOS_DEPLOYMENT_DIR to enable in store mode instead", f) |
| } |
| } |
| var channelReq *string |
| - if set["channel"] { |
| + if flagWasSet(fs, "channel") { |
| channelReq = channel |
| } |
| var seedReq *[]string |
| - if set["seed"] { |
| + if flagWasSet(fs, "seed") { |
| // parseSeed("") is an empty (non-nil after this) argv: an explicit |
| // --seed "" clears the manifest's seed, matching --channel's |
| // explicit-vs-absent split. |
| @@ -138,14 +109,17 @@ func runInstancesEnable(args []string) { |
| // the documented "stable" default, while an explicitly empty one stays |
| // the "not a valid channel" refusal it has always been (the flag's |
| // default moved from "stable" to "" for the signed-in mode's |
| - // preserve-vs-replace split; set[] is what keeps the two cases apart). |
| + // preserve-vs-replace split; flagWasSet is what keeps the two cases |
| + // apart, and it must be flagWasSet, not a raw fs.Visit set: Visit |
| + // reports the spelling typed, so -c would read as "absent" and the |
| + // user's explicit channel would be silently dropped). |
| if *app == "" { |
| log.Fatal(instancesEnableUsage) |
| } |
| if *domain == "" { |
| log.Fatal(instancesEnableUsage) |
| } |
| - if *channel == "" && !set["channel"] { |
| + if *channel == "" && !flagWasSet(fs, "channel") { |
| *channel = "stable" |
| } |
| // A domain is dot-joined labels with no suffix of its own to check |
| @@ -215,34 +189,7 @@ const instancesRecordsUsage = `usage: carlos instances create --app <a> --host < |
| Signed-in only (carlos auth login): instance records are console-authorized |
| writes — the console checks the host against the app's manifest and the |
| -box's reconcile pass materializes the route. Common flags: |
| - |
| - --account <a> account (sqid or name; default "ops") |
| - --account-id <id> account sqid, matched as a sqid ONLY |
| - --console <origin> console to act through (default: the only logged-in one) |
| - |
| -create-only flags: |
| - |
| - --kind <k> route class: instance (default) | service | static. |
| - A static record serves a file tree straight off |
| - its channel pointer — no process, no upstream — |
| - which is what "carlos deploy --kind static" mints |
| - for an app's platform host |
| - --addr <a:p,...> comma-separated TCP upstream set, host:port each |
| - (e.g. 10.0.0.1:8090,10.0.0.2:8090); omitted means |
| - the platform's own default upstream |
| - --health-path <p> states the WHOLE probe alongside --health-status |
| - (default "/"); honoured for a single --addr too |
| - — that is exactly how you probe one |
| - --health-status <n> probe's expected response status (default 200); |
| - fields neither flag names take the platform's |
| - own defaults, not "unset" |
| - |
| -list-only flags: |
| - |
| - --match <substr> only hosts containing this substring |
| - --health <slug> only this state: running, serving, setting-up, asleep, |
| - no-readings, not-responding, failed |
| +box's reconcile pass materializes the route. |
| set-channel moves ONE route onto a different release channel. The channel |
| lives in the instance record, which the box converges the route from every |
| @@ -532,13 +479,6 @@ requester at all. |
| edge answers, instead of today's single static answer |
| off clear the opt-in — back to today's static per-host answer |
| -flags: |
| - --app <a> app the instance belongs to |
| - --host <h> instance host to steer |
| - --account <a> account (sqid or name; default "ops") |
| - --account-id <id> account sqid, matched as a sqid ONLY |
| - --console <origin> console to act through (default: the only logged-in one) |
| - |
| v1 constraint: steering requires the shared pool (--pool "" on the app's |
| instances manifest). A pool-scoped route is refused with a 409 naming this — |
| pool-scoped steering is a recorded future, not v1. |
diff --git a/internal/cliflag/cliflag.go b/internal/cliflag/cliflag.go| index 92b2440f1..ccb98279f 100644 |
| --- a/internal/cliflag/cliflag.go |
| +++ b/internal/cliflag/cliflag.go |
| @@ -61,18 +61,23 @@ func Usage(fs *flag.FlagSet, header string) func() { |
| type row struct{ head, usage string } |
| var rows []row |
| width := 0 |
| + isAlias := func(short, primary string) bool { |
| + f := fs.Lookup(short) |
| + return f != nil && f.Usage == "alias for --"+primary |
| + } |
| fs.VisitAll(func(f *flag.Flag) { |
| - // Fold only pairs the table AND the set both know: a |
| - // command registering a plain -a of its own without the |
| - // registrar would neither be hidden nor claim the fold. |
| - if p, ok := aliasOf[f.Name]; ok && fs.Lookup(p) != nil { |
| + // Fold only pairs String itself registered — recognised by |
| + // the alias's marker usage, not by name alone, so a command |
| + // registering a plain -a of its own is neither hidden nor |
| + // folded into a primary it does not share a value with. |
| + if p, ok := aliasOf[f.Name]; ok && isAlias(f.Name, p) && fs.Lookup(p) != nil { |
| return |
| } |
| head := "--" + f.Name |
| if len(f.Name) == 1 { |
| head = "-" + f.Name |
| } |
| - if short := Aliases[f.Name]; short != "" && fs.Lookup(short) != nil { |
| + if short := Aliases[f.Name]; short != "" && isAlias(short, f.Name) { |
| head += ", -" + short |
| } |
| arg, usage := flag.UnquoteUsage(f) |
| @@ -80,7 +85,7 @@ func Usage(fs *flag.FlagSet, header string) func() { |
| head += " <" + arg + ">" |
| } |
| switch f.DefValue { |
| - case "", "false", "0", "[]": |
| + case "", "false", "0", "0s", "[]": |
| default: |
| usage += " (default: " + f.DefValue + ")" |
| } |
diff --git a/internal/console/templates/app_settings.html b/internal/console/templates/app_settings.html| index a157edd8e..6a3a382d6 100644 |
| --- a/internal/console/templates/app_settings.html |
| +++ b/internal/console/templates/app_settings.html |
| @@ -93,7 +93,7 @@ the certificate is minted on the first request and renews on its own.</p> |
| <p class="small">{{if not .IsOwner}}Moving the record is the account owner's |
| to do{{else if not .Settings.TargetKnown}}The routes name an account this |
| console does not know{{else}}You are not a member of that account{{end}} — |
| -the operator path is <code>console migrate-app --app {{.Detail.App}} -to |
| +the operator path is <code>console migrate-app --app {{.Detail.App}} --to |
| {{.Settings.MismatchTarget}}</code> on the console's box.</p> |
| {{end}} |
| {{else}}{{if .IsOwner}} |
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index 5e1b694d0..c08c3f022 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -17,6 +17,8 @@ |
| package edge |
| import ( |
| + "github.com/carlosframework/platform/internal/cliflag" |
| + |
| "bufio" |
| "context" |
| "crypto/ed25519" |
| @@ -2266,6 +2268,7 @@ func RunWithActivatorReady(args []string, act Activator, cache *Cache, ready fun |
| httpAddr := fs.String("http", ":80", "HTTP listen address (ACME HTTP-01 challenge + redirect to https; in --dev mode, the only listener)") |
| fallbackURL := fs.String("fallback", "", "redirect requests for unknown hosts here, e.g. an apex marketing site; empty keeps the plain 404") |
| dev := fs.Bool("dev", false, "plain-HTTP mode: serve routes on --http with no TLS/ACME (local development only)") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| reg, err := registry.Open(*registryPath) |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 7729df454..3b347296c 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -131,6 +131,8 @@ |
| package host |
| import ( |
| + "github.com/carlosframework/platform/internal/cliflag" |
| + |
| "context" |
| "crypto/ecdh" |
| "crypto/ed25519" |
| @@ -449,6 +451,7 @@ func peekRegistryPath(args []string, dd string) string { |
| fs.String("http", "", "") |
| fs.String("fallback", "", "") |
| fs.Bool("dev", false, "") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| return *regPath |
| } |
| @@ -1414,6 +1417,7 @@ func peekAcmeCache(args []string) string { |
| fs.String("http", "", "") |
| fs.String("fallback", "", "") |
| fs.Bool("dev", false, "") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| return *acme |
| } |
| @@ -2528,6 +2532,7 @@ func RunAdopt(args []string) { |
| configOnly := fs.Bool("config-only", false, "sync unit-backed config and restart changed routes only: no release adoption, no canary nag") |
| stageOnly := fs.Bool("stage-only", false, "adopt releases without the canary nag: the agent-requested pass (carlos-stage.service)") |
| restartOnly := fs.Bool("restart-only", false, "cycle the routes of apps whose restart generation moved, and nothing else: the agent-requested pass (carlos-restart.service)") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| reg, err := registry.Open(*regPath) |
| if err != nil { |
diff --git a/internal/host/bootstrap.go b/internal/host/bootstrap.go| index 648b482c6..7611bde29 100644 |
| --- a/internal/host/bootstrap.go |
| +++ b/internal/host/bootstrap.go |
| @@ -10,6 +10,8 @@ |
| package host |
| import ( |
| + "github.com/carlosframework/platform/internal/cliflag" |
| + |
| "embed" |
| "flag" |
| "fmt" |
| @@ -116,6 +118,7 @@ func RunBootstrap(args []string) { |
| offcloud := fs.Bool("offcloud", false, "off-cloud box: require a staged 0600 credentials file instead of an EC2 instance profile") |
| var consumers repeatableFlag |
| fs.Var(&consumers, "socket-consumer", "host-app user to enroll in the "+socketsGroup+" group (repeatable); members may dial every instance socket on the box") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| if *offcloud { |
| if err := bootstrapOffcloud(*root, consumers); err != nil { |
diff --git a/internal/ops/ops.go b/internal/ops/ops.go| index 84fdc0c86..6ff589464 100644 |
| --- a/internal/ops/ops.go |
| +++ b/internal/ops/ops.go |
| @@ -5,6 +5,8 @@ |
| package ops |
| import ( |
| + "github.com/carlosframework/platform/internal/cliflag" |
| + |
| "context" |
| "errors" |
| "flag" |
| @@ -97,6 +99,7 @@ func runLitestream(args []string) { |
| out := fs.String("out", defaultConfigPath, "config file to write") |
| lsBin := fs.String("litestream-bin", "litestream", "litestream binary to check the snapshot policy against (pinned release "+litestreamVersion+")") |
| allowShrink := fs.Bool("allow-shrink", false, "permit a config that replicates FEWER databases than the one it replaces") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| if *bucket == "" || *prefix == "" { |
| @@ -179,6 +182,7 @@ func runRestoreVerify(args []string) { |
| fs := flag.NewFlagSet("ops restore-verify", flag.ExitOnError) |
| cfgPath := fs.String("config", defaultConfigPath, "generated litestream config to read the database set from") |
| lsBin := fs.String("litestream-bin", "litestream", "litestream binary (pinned release "+litestreamVersion+")") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| tmp, err := os.MkdirTemp("", "carlos-restore") |
| @@ -223,6 +227,7 @@ func runHibernateVerify(args []string) { |
| regPath := fs.String("registry", "", "registry database (default <data>/registry.db)") |
| data := fs.String("data", "", "data directory (default $CARLOS_DATA_DIR or "+defaultDataDir+")") |
| max := fs.Int("max", defaultHibernateMax, "most replicas to restore in one run (0 = no cap)") |
| + fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| ctx := context.Background() |