Skip to content

fix(cli,store): one run-id matcher, and it checks the run exists - #325

Open
skakri wants to merge 1 commit into
mainfrom
fix/one-run-id-matcher
Open

skakri wants to merge 1 commit into
mainfrom
fix/one-run-id-matcher

Conversation

@skakri

@skakri skakri commented Aug 29, 2026

Copy link
Copy Markdown
Member

Closes #317.

ratatoskr-store and ratatoskr-cli each resolved a run-id prefix, independently. The copy in the CLI is where ratatoskr runs rm --force "" found a run to delete, and the dashboard was never exposed to the same bug because it resolves through the store. #315 fixed that copy. It did not remove the reason there were two — and as long as both existed, the next rule added to one was a rule the other silently lacked.

Why the obvious dedupe was unsafe, and what changed

Store::resolve_run short-circuited a full-length id straight back to Some without looking for it:

if prefix.len() >= UUID_LEN {
    return Ok(Some(prefix.to_string()));
}

Routing the CLI through that as-is would have been a regression, not a cleanup: the six subcommands that resolve a prefix include rm, which deletes what it resolves, so an id nobody had ever stored would have resolved to itself and been reported deleted.

A full-length id is now resolved like any other prefix. The saving was an index seek on the primary key; what it cost was the store answering about a run that does not exist. Everything resolve_run returns now exists.

serve is unaffected. It already keeps an unmatched id as-is, deliberately, so its handlers answer an unknown run the same way for a short id as for a long one — which is exactly what a None for a full id now reaches:

.resolve_run(run_id).await?.unwrap_or_else(|| run_id.to_string())

What is left in the CLI

A resolve that supplies the wording, and no matching of its own. The store answers None for "nothing matches", and someone who typed a prefix wants to know which of the two ways it failed; ambiguity arrives as the store's own error, which already says to use more of the id.

The one message that changes is the ambiguous case: `run-ab` matches 2 runs becomes `run-ab` names more than one run — use more of the id. The count is not worth reinstating — the store's LIMIT 2 means it stops as soon as it knows the answer is "more than one", and counting the rest to phrase the message would be a second query for a word.

Corrected while here

The issue's comparison table says the store returns Ok(None) on ambiguity. It returns Err(StoreError::AmbiguousRun). So "distinguish none from several" was already true of the store; only the wording differed.

Verified

  • The four CLI matcher tests now drive the store instead of a hand-built list — empty, unique, ambiguous, no-match — so they prove the decision the subcommands actually reach.
  • A fifth covers the behaviour change: a full-length id that names no run is refused. Control: restoring the short-circuit fails exactly that test.
  • Full workspace green (974), clippy -D warnings clean, fmt clean.

Closes #317.

`ratatoskr-store` and `ratatoskr-cli` each resolved a run-id prefix, and the
copy in the CLI is where `runs rm --force ""` found a run to delete. #315 fixed
that copy; it did not remove the reason there were two. A rule added to one was
a rule the other silently lacked.

Collapsing them was unsafe while `Store::resolve_run` handed a full-length id
straight back without looking for it: an id nobody had ever stored resolved to
itself, and the six subcommands routed through the matcher include ones that
delete what they resolve. A full id is now resolved like any other prefix. The
saving was an index seek; what it cost was an answer about a run that does not
exist.

`serve` is unaffected: it already keeps an unmatched id as-is so its handlers
answer an unknown run the same way for a short id as for a long one, which is
what a `None` for a full id now reaches.

The CLI keeps a `resolve` that supplies the wording — the store answers `None`
for "nothing matches", and someone who typed a prefix wants to know which of
the two ways it failed — but the matching is the store's, once. Its four tests
now drive the store rather than a hand-built list, and a fifth covers the
full-length id that names no run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli/store: two run-id matchers, and the CLI's is the one that was wrong

1 participant