feat(interfaces): scenario_control/v1 gains list, and load answers with what it loaded

[TedSjoblom]

The gap

load takes an id, and nothing in the interface lists the ids. A caller has to already know one, so a control station cannot offer a picker — an operator picks a scenario by reading a directory listing over someone's shoulder.

Slipway is the server implementation of this interface and resolves ids inside a configured library directory, which is exactly the shape a list would describe.

Why this shape

The shape is already settled twice in this repository, and this follows it rather than inventing a third:

	replay_control/v1	scenario_checkpoint/v1	here
request	ListFilesRequest.pattern	ListCheckpointsRequest.pattern	ListRequest.pattern
reply	base_directory, files	base_directory, checkpoints, truncated	base_directory, scenarios, truncated

truncated is carried for the reason ListCheckpointsResponse gives it: a listing that quietly stopped short reads as "that is all of them", and an operator goes looking for a scenario that is in fact there.

The one field worth arguing about

ScenarioInfo carries bool loadable + string reason instead of filtering unloadable rows out. Both alternatives are worse:

• Omitting them hides a file the operator can see in the library, and leaves them asking why one is missing.
• Listing them as loadable is a lie the picker only discovers by trying.

This is not a corner case for a simulator whose scenario language is still growing: a document parses fine and names a construct the build cannot yet elaborate. Slipway has exactly that today — its library holds both YAML and OpenSCENARIO DSL documents, and most of the DSL subset is declared-but-not-built.

Two corrections to load, while this file is open

Both were raised on #215 before it merged and have had nowhere to go since.

1. LoadResponse was empty. StepResponse.tick twelve lines below already makes the argument: a caller should not have to race a 1 Hz status broadcast to learn the result of its own call. It now carries the resolved scenario_id, scenario_hash and the new run_id. It also makes an ambiguous id visible — a caller that asked for harbor and got back a hash it did not expect can say so.

2. The stated failure path was destructive. "on failure … the state returns to NO_SCENARIO", followed literally, means a typo'd scenario id kills a running exercise. Slipway deliberately deviates and implements load transactionally: everything is validated first and the swap happens only on success, so a failed load leaves the running exercise untouched. This makes that the contract.

Scope and compatibility

One file, +89/-4. Nothing existing changes wire format — list is a new procedure and LoadResponse's fields are additions to an empty message, so an old client reading a new reply sees what it always saw. A client calling list against a server that does not serve it gets no responder, which is the same answer it gets for any procedure a connector has not implemented.

Generated SDK code is not committed here; sdks/python/generate_python.sh and the SDK test suite both pass against it locally (164 passed, 1 skipped).

🤖 Generated with Claude Code

[freol35241]

The gap is real and the shape is right — pattern / base_directory / truncated follows list_files and list_checkpoints faithfully, and keeping unloadable rows on the wire with a reason is the correct call. Scope is good: one file, wire-compatible, and no interfaces.yaml change needed since procedures come from the descriptor.

Three things I'd like fixed before merge, plus some polish.

1. bool loadable = 7 has the dangerous default. Proto3 zero is false = "would refuse to load". Any ScenarioInfo built without explicitly setting the field — a simpler responder, a partially-populated row — reports the whole library as unloadable and the picker greys out everything. The safe default has to be the affirmative one. An enum with a countable unknown (LOADABILITY_UNKNOWN = 0) is my preference; optional bool loadable also works.

2. The transactional rewrite contradicts the state machine line above it. The diff keeps load : any -> LOADING -> STOPPED while the prose says a failed load leaves the previous scenario "running untouched". Both can't hold — if it kept running, the state was never LOADING. Dropping | NO_SCENARIO also removed the only hint that a successful load from RUNNING stops the run. Two explicit sentences would settle it: when LOADING is entered (only on commit), and that a successful load does terminate a running exercise.

3. run_id in LoadResponse conflicts with how run_id is minted everywhere else. RestoreResponse.run_id is documented as "the run that just began"; load lands in STOPPED, where no run has begun. And reset returns to tick 0 with the scenario intact — does that re-mint? As written, load -> start -> reset -> start gives two runs from tick 0 sharing one run_id, which undercuts the recording-is-evidence reasoning scenario_checkpoint/v1 leans on. Either drop it from LoadResponse, or document minting at load and re-minting at reset.

Polish:

• step_size_ms is a uint32 where ScenarioStatus.step_size is a google.protobuf.Duration. Same quantity, two encodings — a picker comparing a listed scenario against the running one has to convert.
• Overloaded zeros in ScenarioInfo: duration_ticks = 0 is documented as both "unknown" and "open-ended", and vessel_count / step_size_ms overload zero too. Explicit presence would remove the ambiguity.
• Siblings are list_files and list_checkpoints; this is bare list with generic ListRequest/ListResponse. No collision risk given the per-interface package, so take it or leave it — but it does break the pattern the description cites as precedent.
• Undefined whether reason may be non-empty when loadable is true.

[TedSjoblom]

Pushed as 2733998 — one file, interfaces/ScenarioControl.proto, still wire-compatible in the sense that matters here: nothing is published under these field numbers yet, so all three blocking changes were free to make.

Blocking

1. The dangerous default is gone. bool loadable is now a nested enum with a countable unknown:

enum Loadability {
  LOADABILITY_UNKNOWN = 0;
  LOADABLE = 1;
  NOT_LOADABLE = 2;
}
Loadability loadability = 7;

Zero now means "the responder expressed no opinion", and the comment says outright that a picker must offer such a row and let load answer. Your reading was right — a simpler responder building rows without setting the field would have reported the whole library as unloadable. Went with the enum over optional bool since it matches ScenarioParticipant.Role and ScenarioStatus.State next door.

2. The state machine and the prose now say the same thing. You were right that both couldn't hold. Resolved by stating the two facts explicitly rather than leaving them to be inferred from an arrow:

• load : any -> [LOADING] -> STOPPED, with LOADING entered only once the load has committed. Resolution and validation happen before anything is disturbed, so a load that fails validation never leaves the state it was called in — which is what makes "a typo'd id cannot kill a running exercise" true rather than merely intended.
• A load that succeeds does terminate a running exercise: the run ends, and the state comes to rest in STOPPED at tick 0. That restores what dropping | NO_SCENARIO had removed, and the line now says explicitly that the STOPPED box is reachable by load from any state, not only from NO_SCENARIO.

3. run_id dropped from LoadResponse. Your reset argument is the one that settled it — load -> start -> reset -> start producing two runs from tick 0 under one id defeats the point of the id. LoadResponse is now just scenario_id + scenario_hash, and a new "Run identity" paragraph in the header says a run id is minted when a run begins (start from STOPPED, or restore in scenario_checkpoint/v1), never by load, and is observed on ScenarioStatus.run_id. The reset and stop bullets now state that the run ends there, so the next start begins a new one. RestoreResponse.run_id keeps meaning exactly what it says.

Polish — all four taken

• step_size_ms -> google.protobuf.Duration step_size, so a picker holding a listed scenario against the running one compares against ScenarioStatus.step_size directly instead of converting. Added the duration.proto import.
• Overloaded zeros removed. vessel_count and duration_ticks are optional, with per-field comments replacing the shared one. Present 0 vessels is now a real answer (an environment-only scenario). For duration_ticks I did not re-introduce 0 as a sentinel for open-ended: absent means there is no declared end to report, and the comment states that the listing deliberately does not distinguish open-ended from undetermined, because a picker renders both the same way and a responder guessing between them would be inventing the distinction. If you'd rather have those two separable on the wire, say so and I'll add it — it wants a second field, not a magic value.
• list -> list_scenarios, with ListScenariosRequest / ListScenariosResponse. You were right that it broke the precedent the description cites; no reason to be the odd one out.
• reason is defined: set when NOT_LOADABLE, may explain a LOADABILITY_UNKNOWN, and MUST be empty when LOADABLE — so a caller never has to decide whether text there is a warning about a scenario it can load.

Also dropped the "twelve lines below" cross-reference in LoadResponse, which these edits would have made stale.

Verification

No interfaces.yaml change needed, as you noted — procedures come from the descriptor, and nothing in the SDKs or tests names these procedures.

Regenerated both SDKs and the docs; the registry reports the rename and the defaults behave:

get_procedures('scenario_control','v1')
  -> ['list_scenarios', 'load', 'start', 'pause', 'stop', 'reset', 'step', 'set_rate']
ScenarioInfo().loadability == LOADABILITY_UNKNOWN     # safe default
vessel_count / duration_ticks / step_size unset       # explicit presence
'run_id' not in LoadResponse fields

Python SDK suite 164 passed / 1 skipped, JS 18 passing.