Skip to content

The opaque paths want a rendered-id type, not WorkspaceId: what happens to an id that fails the check? #535

Description

@blooop

Where this comes from

#316 asked to thread WorkspaceId through ~69 signatures taking
workspace_id: &str, so an unvalidated string could no longer satisfy them.
#534 migrated 5 and found the
ticket has a boundary through the middle of it. Two independent traces of every
production call site agree:

build agent reviewer (independent audit)
TRIPLE (a triple is in hand) 2 3
OPAQUE (only a rendered id) 60 59
MIXED 0 0

The reviewer established something stronger than absence: 0 MIXED is
unreachable, not merely unobserved.
There are exactly two places a triple could
survive downward (launch::place_triple, dl::target::triple) and each reduces
its WorkspaceId to a String inside its own body. No production call site
anywhere passes workspace.value() into any of the 62.

And three of the opaque paths would actively regress if migrated.
listing::unsaved_work_in, reached from the dl <ws> rm guard, looks records up
by recorded id; demanding a triple would miss every old-scheme record and
silently disarm the unsaved-work guard. remove_workspace_by_id has that exact
regression written in its own doc comment.

So #316 as literally written is not achievable, and it closed with #534 rather
than parking an impossible target on map #299. This ticket is where the finding
lives instead.

The question

The 59 opaque paths do not want a WorkspaceId. They want a rendered-id
newtype
— a validated id, as against a validated triple — whose
constructors are the only legitimate producers of one.

The id reaching those paths is often not a triple's derivation at all. It can be:

  1. a triple's derivation (WorkspaceId::value),
  2. a ref-less URL's hashed id,
  3. a path spec's directory leaf,
  4. a bare name the user typed,
  5. an id metadata.json recorded under a derivation that has since moved.

Placement already carries a String for exactly this reason, and the comment
beside it says a triple is not recoverable from an id. dl --purge and
dl --prune are defined over devpod workspaces and clone directories with no
record at all; dl <ws> kill skips resolution deliberately.

The design decision is not the type, it is the behaviour at each boundary
above all: what should happen to a devpod list entry that fails the check? Today
nothing checks, so nothing can fail. That is the question this ticket exists to
settle, and it is why #534 did not answer it in passing.

The small mechanical half, which is not blocked on that

Two triple-shaped signatures survive and are invisible to a workspace_id: &str
grep, so they escaped #316's census:

  • workspace_clone::prepare_cold — taking loose strings makes
    PrepareColdError::UnsafeTriple production-unreachable.
  • lifecycle::state::recorded_devpod_workspace_id — reached from a call site
    Derive a workspace id once, and make the triple the way in #534 itself wrote, flattening a &WorkspaceId into three strings two lines
    after removing the same shape next door.

Both can be migrated the way #534 migrated its five, independently of the design
question above.

Three smaller residues from #534's review

  • WorktreeInfo::as_an_older_dl_recorded_it (a #[cfg(test)] builder for the
    real state where a record's triple and id disagree) is used at three sites where
    the halves do agree — the escape hatch spent as the general-purpose builder.
  • Three new hardcoded golden ids sit outside domain::workspace_id, against a
    rule the repo states verbatim in a sibling module.
  • The sibling-clone test added beside the caching test does not discriminate: it
    passes on main too. The pointer-identity test is the one doing the work.

What #534 did land, for context

WorkspaceId derives once at new() instead of on every value() call (a
SHA-256 over the folded triple, three slug passes and a truncation budget, which a
launch asks for at every layer it hands the id down through), value() lends
&str, and the three signatures that genuinely had a triple in hand stopped
flattening it. Deliberately no Deref<Target = str>: it would let a validated
triple be spent as a plain string by coercion at every call site, which is the
erasure the work is undoing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions