GET /v1/commands: two contracts collided — fold the composer/argument-shape fields into the shipped one and settle the localization policy
- Dominant language
- Rust
- Stars
- 41k
- Forks
- 3.6k
- Avg merge
- 13h 59m
- Merged PRs (30d)
- 299
Description
While merging #6229 into current `main`, both sides turned out to have independently implemented `GET /v1/commands`, which the router rejected at runtime:
```
Overlapping method route. Handler for `GET /v1/commands` already exists
```
They are not duplicates. They are two different contracts, and the one that shipped is the thinner of the two.
## What shipped (main, `70cda6e43`, #6178)
`CommandCatalogEntry` in `runtime_api.rs` carries `name`, `aliases`, `summary`, `usage`, `subcommands`, `takes_arguments`, `kind`, `binding`, `discovery`, `hidden`, `shadowed_by`, `shadowed_aliases`. It includes **user-registered commands** and shadowing, which the other does not.
## What the branch had, and main does not
`runtime_api/commands.rs` (removed in the merge, recoverable at `060711b2f1`) projected a richer composer/palette contract. Its test asserted every row carries:
| Field | Main's equivalent |
| --- | --- |
| `description` (localized, with a `locale` on the response naming the resolved pack) | `summary` — English only, by design |
| `requires_argument` | — |
| `requires_required_argument` | `takes_arguments` (boolean, does not distinguish optional from mandatory) |
| `composer_wants_trailing_space` | — |
| `palette_runs_directly` | — |
| `show_in_empty_discovery` | — |
| `unlisted` | `hidden` |
Its stated purpose was to let a native composer reproduce slash completion **without re-parsing `usage`** — e.g. `/profile` sets `requires_required_argument: true` and `palette_runs_directly: false`, a distinction `takes_arguments` flattens.
## The decision that is open
Main's `summary` is deliberately English: *"English source text; localizing is the client's surface."* The branch's `description` is localized server-side with a resolved-locale marker. **Those are incompatible policies for the same field and someone has to choose.** That is a public API contract decision, not a merge mechanic, which is why I did not fold them together while resolving conflicts.
What I did instead: kept main's handler (shipped, and #6178 is closed against it), removed the branch's duplicate route, module and test, and wrote this down so the capability is not silently lost. Everything is recoverable from `060711b2f1`.
## Suggested resolution
Add the argument-shape and composer fields to main's `CommandCatalogEntry` — they are additive and nothing depends on their absence — and decide the localization policy separately, since that one is a real fork:
- server-localized `description` + `locale`, or
- English `summary` and the client owns localization.
The second is what main documents today; the first is what a native composer would rather consume. Worth deciding once rather than per-client.
Contributor guide
Research direction
Start with CommandCatalogEntry in runtime_api.rs and compare the removed runtime_api/commands.rs and its test recovered from 060711b2f1. First settle whether localization belongs on the server or client, then define the additive argument-shape and composer fields for GET /v1/commands. Done means the chosen contract is documented, tested, and only one route remains.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend-api-design, cli
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 40/100