Hmbown / Hmbown/Codewhale

GET /v1/commands: two contracts collided — fold the composer/argument-shape fields into the shipped one and settle the localization policy

Open
#6,230 0 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.