A tab is named after the spec, and keeps that name - #371
Conversation
`dl blooop/devlaunch` named its pane `devlaunch-main-zovomobo`. The id is
a poor name for two reasons: it carries no owner, so a fork and its
upstream are two tabs spelled the same, and it spells the branch as a
slug, so `feature/auth` reads as `feature-auth` — the name of a different
branch the same repository could have. Neither is recoverable from the id
afterwards. The pane is now `blooop/devlaunch@feature/auth`.
No reconstruction is involved: a launch resolved that triple on the way
in, and `Launch` now remembers it rather than keeping only the id its
`Placement` collapsed to. The other three placements have no triple and
keep the id — a bare name *is* its id, and a path or URL never had a
branch for an `@` to precede.
That much lasts about a second in an interactive session. Ubuntu's stock
`~/.bashrc` puts `\e]0;\u@\h: \w\a` at the *front* of PS1, so every
prompt renames the pane after the hostname, which is the id. So the
setup pass now appends one line to the profile a login shell reads:
case $- in *i*) PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac
Appended, because two escapes in one prompt are applied in order and the
last one sets the title — a `PROMPT_COMMAND` would lose, since bash runs
that before it prints PS1. Nothing is rewritten, so the visible
`user@host:path$` still says the hostname and only the tab changes. It
rides the hostname stage's existing round trip, is deduped by the same
content-hash mark the PATH writers use, and is inert in the `bash -lc`
one-shots that read the same profile. A test drives it through a real
bash and reads PS1 back, because the ordering is the whole feature.
`DEVLAUNCH_NO_TITLE` governs both halves. It is installed when a
workspace enters Running rather than per attach, the same bargain the
hostname stage makes for the same round-trip reason; README says so.
`public-api.txt` is edited by hand for the two threaded signatures; CI
regenerates it.
The profile line is deduped by a hash of its own text, so a name that varies for one workspace does not replace the line -- it appends another, and the last append is what every prompt then obeys. A workspace opened as `blooop/devlaunch@main` and later as `devlaunch-main-zovomobo` ended up with both lines and permanently titled after the id, which is the string the spec exists to replace. One launch by id was enough. The container is now told the spec or nothing. Nothing is the honest answer for an id: it is already the hostname, so the stock prompt writes exactly that anyway and the line would buy nothing to lose. Keying on the spec makes the line a pure function of the triple, so a workspace has at most one, ever. dl's own escape still falls back to the id -- it names a pane before any prompt exists, and for a bare name the id is what the user typed. The test that first caught this asserted the property of `setup_stages`, which cannot provide it: two distinct titles really do append two lines. It is replaced by one at the layer that decides there is only ever one name -- `Launch::container_title`.
`$PROFILE` resolves to `~/.bash_profile`, `~/.bash_login` or `~/.profile`, and the last of those is read by any POSIX login shell -- `/bin/sh` is dash on Debian and Ubuntu. `\[`, `\e` and `\a` mean nothing to dash, which renders a prompt literally, so the appended line put `\[\e]2;blooop/devlaunch@main\a\]` on screen at every prompt. A corrupted prompt is worse than an unnamed tab, and the `case $- in *i*` guard does not catch it: dash's interactive shells are interactive too. Guarded on `$BASH_VERSION`, which is the same test Ubuntu's own `~/.profile` makes before it sources `~/.bashrc`. The test asserts the implication rather than the shell, so it reads the same wherever `sh` is bash.
`is_safe_name` accepts one trailing newline — Python's `$` anchor did, and the quirk is ported deliberately — so `main\n` is a ref and `blooop/devlaunch@main\n` is a name a launch can carry. dl's own escape drops controls at the boundary it forms bytes at, but the profile line is written down a different path, and the newline landed inside the quoted word: one PS1 assignment split across two physical lines of a file every login sources. Both halves now take the same filtered name, so a title cannot be safe in the escape and unsafe in the profile.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideTerminal titles now use the resolved spec (owner/repo@ref) instead of the workspace id, and the spec-based name is propagated through launch, attach, and provisioning so shells keep the tab named correctly via a new profile-editing stage, with safeguards and tests around title content, shell behavior, and configuration switches. Sequence diagram for resolved workspace terminal namingsequenceDiagram
participant User
participant Launch
participant Provision
participant Container
participant Terminal
User->>Launch: run(owner/repo@ref)
Launch->>Launch: place_triple()
Launch->>Launch: titled(workspace_id)
Launch->>Provision: provision_tools(..., title)
Provision->>Container: setup_stages(..., title)
Container-->>Provision: profile updated
Launch->>Terminal: TerminalTitle::from_host(name)
Terminal-->>User: pane named owner/repo@ref
Flow diagram for persistent terminal title setupflowchart TD
A["Launch resolves owner/repo@ref"] --> B["Launch remembers resolved WorkspaceId"]
B --> C["container_title filters the spec"]
C --> D{DEVLAUNCH_NO_TITLE}
D -- enabled --> E["Skip title stage"]
D -- disabled --> F["Append deduplicated PS1 title line"]
F --> G["Interactive Bash appends OSC 2 after stock PS1"]
G --> H["Prompts keep the pane named after the spec"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
# Conflicts: # rust/devlaunch-core/src/flows/provision.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The two features now sit together, so the Workspace IDs section can point at both places a workspace is named rather than only at the tab.
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Retrospective wf-review of #358 and #371 as one surface — terminal tab/pane
naming — judged against current main (a5f7ed8). Both merged with no review
(Sourcery rate-limited). Every finding below was checked to still reproduce on main
and is attributed to the PR whose line survives; anything #371 already fixed in #358
is not listed. The two axes were run as independent passes and are not merged or
reranked.
The escape-safety hunt came back clean, and that is worth saying explicitly
because it was the likeliest place for a real defect. sanitize_title
(rust/devlaunch-core/src/flows/launch.rs:1837) drops the whole Cc category, so
ESC, BEL, DEL and the 8-bit ST are all gone before the OSC is formed; the same filter
runs on the profile half (launch.rs:2973); shell::quote interpolates the name as
its own word; and a spec cannot hold $ or a backtick because is_safe_name refuses
everything but word characters, ., / and -, which is what keeps the PS1
value out of reach of bash's prompt-time command substitution. Mutating the filter
to a no-op turns a_spec_cannot_smuggle_a_second_escape_into_the_title and
a_name_that_is_nothing_but_controls_is_no_title_at_all red; mutating \x1b]2; to
\x1b]0; reddens five unit tests plus the real-pty test; mutating the PS1 append
to a prepend reddens the ordering test and the real-bash test. Test strength on
this surface is good — no vacuous test was found. Nothing herdr-shaped was left
dangling by #376 either: grep -ri herdr rust/ README.md docs/ is empty.
Standards
Repo-documented standards first. README.md §Naming the terminal after the
workspace is the written spec for this surface, and CLAUDE.md requires it be kept
true; docs/rust-rewrite-plan.md supplies the layering rules.
1. #371 — the "keeps that name" mechanism does not survive a nested shell
README.md:704 promises the name survives "an interactive shell", and the whole of
§What keeps it named rests on one line appended to $PROFILE
(provision.rs:506, provision.rs:1025). $PROFILE resolves only to
.bash_profile/.bash_login/.profile (provision.rs:430) — files a login
shell reads. PS1 is not exported, and Ubuntu's ~/.bashrc sets it unconditionally
for every interactive shell. So the first nested interactive bash inside the
workspace — bash, tmux, or a pane of the zellij session this repo's own
DEVLAUNCH_ZELLIJ installs inside the container — reads ~/.bashrc only, gets the
stock \e]0;\u@\h prompt, and renames the pane back to the hostname (the id).
Driven through real bash, stock Ubuntu skeleton, our line appended to .profile:
login -i : \[\e]0;\u@\h: \w\a\]\u@\h:\w\$ \[\e]2;blooop/devlaunch@main\a\]
nested -i : \[\e]0;\u@\h: \w\a\]\u@\h:\w\$
The root cause is reusing profile_prepend — a writer built for PATH, which is
exported and is inherited — to install a variable that is neither.
Fix: append the same guarded line to ~/.bashrc (in addition to, or instead of,
$PROFILE). Ubuntu's ~/.profile sources ~/.bashrc, so a line at the end of
.bashrc lands after the stock PS1 in the login case too, and the existing
content-hash dedupe keeps it single.
2. #371 — "a workspace has at most one such line, ever" is false for case variants
README.md:734: "Keying on the spec alone means a workspace has at most one such
line, ever." — and launch.rs:2949 says the same. But WorkspaceId::suffix()
lowercases owner and repo before hashing (deliberately: NVIDIA/cuda-samples and
nvidia/cuda-samples are one repository, one container), while spec_of
(launch.rs:1857) formats the owner and repo as typed. One workspace, two spec
strings, two content-hash marks, two appended lines:
id = cuda-samples-main-libemaka (identical for both)
spec A = NVIDIA/cuda-samples@main
spec B = nvidia/cuda-samples@main
line A = case $- in *i*) [ -n "$BASH_VERSION" ] && PS1="$PS1\[\e]2;"NVIDIA/cuda-samples@main"\a\]" ;; esac
line B = case $- in *i*) [ -n "$BASH_VERSION" ] && PS1="$PS1\[\e]2;"nvidia/cuda-samples@main"\a\]" ;; esac
Reached by launching the same workspace under two spellings across a stop/start (the
AfterUp pass always travels). This is the same shape as the defect #371's own
self-review fixed for ids — the last append wins permanently, so a later
dl NVIDIA/… gets a tab reading nvidia/… and no further launch can change it.
Fix: key spec_of on the same normalisation suffix() uses — lowercase owner and
repo, keep the ref verbatim — so the line is a pure function of the workspace
rather than of the spelling.
3. #371 — a trusted verdict swallows the title stage, and the feature silently never applies
provision.rs:1814 returns CachedProvisioned before setup_pass on a
PassOccasion::TopUp. The title is not part of the verdict key (trusted() is
workspace id + workspace_result.json mtime), so a workspace whose pass ran with no
title keeps no title for as long as it stays up. Proven with a test against the real
flow:
// 1. `dl <workspace-id>`: no triple -> title None; pass runs, verdict recorded.
provision_tools(&by_id, "myws", TopUp, INSTALLING, None, .., Some(&verdicts), ..)
== Ok(Provisioning::AlreadyProvisioned); // by_id.count() == 1
// 2. `dl owner/repo@main` on the still-running workspace:
provision_tools(&by_spec, "myws", TopUp, INSTALLING, Some("owner/repo@main"), .., Some(&verdicts), ..)
== Ok(Provisioning::CachedProvisioned); // by_spec.count() == 0dl/src/launch.rs:167 passes Some(&self.verdicts) in production, so this is the
shipped path. It also covers the upgrade case: any workspace already running when a
user upgrades to 0.8.0+ never gets the line. README.md:737 documents "installed
when a workspace enters Running, not on every attach", which does not describe this
— the workspace did enter Running, just without a name to install.
Fix: either include the title in the marker the verdict is checked against, or
hoist the title stage in front of the cache short-circuit (it is one line in a script
that is already being composed, and the cache exists to save a trip, so the cheap
version is to let a Some(title) on a TopUp defeat trusted()).
4. #371 — the documented length bound does not exist
launch.rs:1780-1789 and README.md:689-694 both assert "a triple with an unsafe or
overlong part is refused before a session exists" and "the title is short because a
workspace with a long name cannot exist". WorkspaceId::new enforces no length at
all; value() truncates to 47. Proven:
let ws = WorkspaceId::new("owner", "repo", &"a".repeat(300)).unwrap();
assert_eq!(ws.value().len(), 47); // the id is capped
assert_eq!(spec_of(&ws).len(), 311); // the title is notSo devpod's 48-character rule never sees the title, and a long branch name puts a
311-character OSC on the terminal and 311 characters into a PS1 re-emitted at every
prompt. Cosmetic, but the two paragraphs that say it cannot happen are load-bearing
documentation under CLAUDE.md's "keep README up to date".
Fix: truncate the title (keeping the ref's tail, which is the identifying half),
or correct both paragraphs to say the title is unbounded.
5. #358 — the title is never restored, and is written before the session is known to exist
attach_workspace (launch.rs:1903) says the notice unconditionally, ahead of
workspace_ssh. A refused session leaves the host terminal named after a workspace it
never entered, and a normal dl <ws> -- cmd (every aid launch) leaves the tab
reading blooop/devlaunch@main after dl has exited. Nothing writes a title on the
way out.
Fix: bracket the session with XTWINOPS — CSI 22;2t to push the title before the
OSC and CSI 23;2t to pop it after — which xterm, kitty, foot, wezterm and tmux
honour and everything else ignores. Writing an empty OSC 2 would blank the name rather
than restore it, which the code already correctly refuses to do.
6. Fowler baseline
- Mysterious Name (#371).
profile_prepend(provision.rs:460) appends, and its
doc still opens "One PATH line appended to$PROFILE" (provision.rs:440) — it now
also carries aPS1line, which is the one case where the PATH-shaped assumption
(exported, inherited) is wrong. See finding 1. Rename and restate the contract. - Primitive Obsession / long parameter list (#358 introduced, #371 widened).
title: Option<&str>is threaded raw through seven signatures —
Provision::provision_tools,workspace_up,up_under_stage,
provision::provision_tools,provision,setup_pass,setup_stages— and paid for
with two#[allow(clippy::too_many_arguments)](provision.rs:1779,1798). The
sanitising that makes it safe lives in the privateLaunch::container_title, while
the publicprovision::provision_tools(inpublic-api.rest.txt:1970) takes an
unfiltered&str. AContainerTitle(String)newtype whose only constructor runs
sanitize_titlewould collapse the parameter and move the invariant into the type. - Illegal state representable (#358).
TerminalTitle::Write(String)
(launch.rs:1792) is a public tuple variant holding raw bytes thatdl'sSaying
sink writes verbatim to stderr (dl/src/render.rs:1711). Any caller of the frozen
public API can construct one with arbitrary escapes. Private field plus a constructor
would make the sanitising unskippable. - Checked and not findings: Duplicated Code (
titledandcontainer_titlediffer
on purpose and each says why), Speculative Generality, Middle Man, Refused Bequest.
Spec
No spec available — for either PR.
Neither #358 nor #371 closes a ticket: closingIssuesReferences is empty for both, no
commit on either branch (3703cf7, 2a8f4f9, 32703a9, 683da23; 816996b,
0caa214, 57960aa, 2ae964f) carries a Closes/Fixes/#n reference, and the
branch names (tabname, title-spec) are not wayfinder/devlaunch-N nodes. A search
of the tracker for title in:title, tab in:title, pane, terminal title,
DEVLAUNCH_NO_TITLE and which workspace returns nothing describing this behaviour.
Per the method, no requirements are inferred from the code, so this axis reports
nothing. The behavioural findings above are held against README.md, which is a
repo-documented standard rather than a spec, and are filed under Standards for that
reason.
Verdict
Comment (retrospective — both PRs are merged; --request-changes is unavailable on
a same-account PR anyway).
Nothing here is a security defect: the escape-injection surface the design worried
about most is genuinely closed, on both halves, and the tests hold under mutation.
Blocking, had this been reviewed before merge — two of them are the feature not
actually doing what its README says:
- Finding 1 — the name is lost at the first nested shell or zellij pane, which is
most of what "and keeps that name" was for. - Finding 3 — a trusted verdict means the line is never installed at all for a spec
launch onto a running workspace.
Non-blocking: findings 2, 4, 5 and the four Fowler items. Finding 2 is a small, exact
fix (normalise spec_of the way suffix() normalises) and is worth taking with
finding 1.
1 participant
dl blooop/devlaunchnamed its panedevlaunch-main-zovomobo. The id is a poorname for two reasons: it carries no owner, so a fork and its upstream are two tabs
spelled the same, and it spells the branch as a slug, so
feature/authreads asfeature-auth— the name of a different branch the same repository could have.Neither is recoverable from the id afterwards. The pane is now
blooop/devlaunch@feature/auth.No reconstruction is involved: a launch resolved that triple on the way in, and
Launchnow remembers it rather than keeping only the id itsPlacementcollapsedto. The other three placements have no triple and keep the id — a bare name is its
id, and a path or URL never had a branch for an
@to precede.Keeping the name
That much lasts about a second in an interactive session. Ubuntu's stock
~/.bashrcputs
\e]0;\u@\h: \w\aat the front ofPS1, so every prompt renames the paneafter the hostname, which is the id. So the setup pass appends one line to the
profile a login shell reads:
Appended, because two escapes in one prompt are applied in order and the last one
sets the title — a
PROMPT_COMMANDwould lose, since bash runs that before itprints
PS1. Nothing is rewritten, so the visibleuser@host:path$still says thehostname and only the tab changes. It rides the hostname stage's existing round
trip, is deduped by the same content-hash mark the PATH writers use, and is inert in
the
bash -lcone-shots that read the same profile. A test drives it through a realbash and reads
PS1back, because the ordering is the whole feature.DEVLAUNCH_NO_TITLEgoverns both halves.Two bargains worth knowing
Installed when a workspace enters Running, not per attach. So
DEVLAUNCH_NO_TITLE=1 dl <ws>on a running workspace silences dl's escape but notthe prompt's;
recreatere-decides it. Same bargain the hostname stage makes, forthe same round-trip reason (#157). README says so.
Only a spec is installed. A container told to title after its own id is told
nothing — the id is already its hostname. It also cannot be, safely: see the second
finding below.
Self-review found three defects, all fixed here
deduped by a hash of its own text, so a second, different name for one workspace
appends rather than replaces, and the last append is what every prompt obeys. One
dl devlaunch-main-zovomobowas enough to undo the spec permanently. Keying onthe spec alone makes the line a pure function of the triple, so a workspace has at
most one, ever.
$PROFILEcan be~/.profile, which any POSIX login shell reads, and/bin/shis dash on Debianand Ubuntu — where
\[,\eand\amean nothing, so the prompt showed\[\e]2;blooop/devlaunch@main\a\]at every line. A corrupted prompt is worse thanan unnamed tab, and
case $- in *i*does not catch it: dash's interactive shellsare interactive too. Guarded on
$BASH_VERSION, the same test Ubuntu's own~/.profilemakes before sourcing~/.bashrc.is_safe_nameacceptsone trailing newline deliberately, so
main\nis a ref; the newline landed insidethe quoted word, splitting one
PS1assignment over two physical lines of a fileevery login sources. Both halves now take the same filtered name.
Checks
cargo test --workspace(1614),clippy -D warnings,fmt --check.public-api.txtis edited by hand for the two threaded signatures — this container has no nightly to
regenerate it with, so CI's diff is the real check.
Note for review: the effect inside a real workspace has not been driven end to end
here — the PS1 ordering and the profile edit are covered by real-bash tests, but no
dlrun against a live container. Worth one manual check on the host.🤖 Generated with Claude Code
Summary by Sourcery
Name launched repository workspaces after their resolved specifications and persist those names across interactive shell prompts.
New Features:
owner/repo@branch, while retaining workspace IDs for non-repository placements.Bug Fixes:
Enhancements:
DEVLAUNCH_NO_TITLEconsistently to both the immediate terminal title and persistent shell configuration.Documentation:
Tests:
Chores: