From 816996b4ec7e306d4f8e1cf18058ef29d38290a1 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 22 Aug 2026 19:09:31 +0000 Subject: [PATCH 1/5] feat: a tab is named after the spec, and keeps that name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- README.md | 106 +++-- rust/devlaunch-core/public-api.txt | 4 +- rust/devlaunch-core/src/flows/launch.rs | 425 ++++++++++++++++++--- rust/devlaunch-core/src/flows/provision.rs | 335 ++++++++++++++-- rust/dl/src/launch.rs | 2 + rust/dl/tests/launch.rs | 2 + 6 files changed, 757 insertions(+), 117 deletions(-) diff --git a/README.md b/README.md index 7ef5e29a..7dc4cc48 100644 --- a/README.md +++ b/README.md @@ -480,11 +480,11 @@ zellij has not thereby asked for unnamed containers. ## Naming the terminal after the workspace -Every launch writes the workspace id to the terminal as its title, just before the +Every launch names the terminal after the workspace it is opening, just before the session takes over: ``` -ESC ] 2 ; devlaunch-main-zovomobo BEL +ESC ] 2 ; blooop/devlaunch@main BEL ``` That is one escape sequence to whichever stream dl was given, and the point of @@ -497,25 +497,29 @@ It is on unless you turn it off: | Variable | Description | |----------|-------------| -| `DEVLAUNCH_NO_TITLE=1` | Do not write a terminal title. Everything else about the launch is unchanged | +| `DEVLAUNCH_NO_TITLE=1` | Do not name the terminal — neither the escape below nor the profile edit under [What keeps it named](#what-keeps-it-named). Everything else about the launch is unchanged | A "no" variable, where `DEVLAUNCH_ZELLIJ` is an opt-in one, because the two are not the same size of decision. That one installs a session into a container; this one -writes an escape sequence that the next shell prompt overwrites anyway. - -**It is the workspace id, not the spec you typed.** The id is the only string that -is always available and always bounded: every launch has one — a triple, a bare -name, a path, a URL — whereas `owner/repo` still has its branch unresolved at that -point and `./some/dir` is not a spec at all. It is also already the container's -hostname — so the title dl writes and the `user@host` an interactive prompt paints -over it agree instead of disagreeing. - -It stays short enough for a tab bar, but not by anything dl does: only a -`owner/repo@branch` launch gets its id from the 47-character derivation under -[Workspace IDs](#workspace-ids). A bare workspace name or a `./path` arrives as -what you typed, and what keeps *those* short is devpod, which refuses to create or -report a workspace whose name runs past 48 characters — so a longer one ends the -launch before there is a session to name. +writes an escape sequence and one line into a profile. + +**It is the spec you typed, resolved — not the workspace id.** `dl blooop/devlaunch` +names the pane `blooop/devlaunch@main`, with the branch filled in as the launch +resolved it. The [id](#workspace-ids) is a worse name for two reasons: it carries no +owner at all, 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. + +The other three ways of naming a workspace have no triple to resolve, so they keep +the id: a bare `dl myworkspace` *is* its id, and `dl ./some/dir` or a plain URL +never had a branch for an `@` to precede. + +It stays short enough for a tab bar, but not by anything dl does. A spec is bounded +by the id it derived — a triple whose parts overrun 47 characters is refused before +there is a session — and a bare workspace name or a `./path` arrives as what you +typed. What keeps *those* short is devpod, which refuses to create or report a +workspace whose name runs past 48 characters, so a longer one ends the launch before +there is a session to name. **Written to stderr, and only when stderr is a terminal.** stdout belongs to the completion machinery and to `wf`, which parse it. The tty check is on stderr for @@ -523,26 +527,50 @@ the same reason: `dl -- make test > log` has redirected stdout and still ha terminal worth naming, while a run whose stderr is a pipe would only be writing escapes into somebody else's capture. -### What overwrites it, and the one case worth knowing +### What keeps it named -A terminal title has exactly one value and the last writer sets it, so anything in -the session that writes its own title wins. Two do, routinely: +A terminal title has exactly one value and the last writer sets it. An interactive +shell overwrites dl's within a second of arriving: Ubuntu's stock `~/.bashrc` puts +`\e]0;\u@\h: \w\a` at the *front* of `PS1`, so every prompt renames the pane after +the container's hostname — which is the workspace id, the name we just went to +some trouble not to use. -- **An interactive shell's prompt.** Ubuntu's stock `~/.bashrc` puts - `\e]0;\u@\h: \w\a` in `PS1`, so `dl someone/repo` shows the workspace name until - the first prompt renders and `vscode@devlaunch-main-zovomobo: ~/repo` after it. - Nothing is lost — that string is the container's hostname, which is the same - workspace id — so this case is already labelled and dl does not fight it. -- **claude.** It writes the title continuously from its own read of what the - session is doing, which would leave a `dl -- claude` pane named after the - task rather than the workspace within a second. `aid` therefore starts claude - with `CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1`, so the workspace name is what - stands. What claude is doing is on screen inside the pane; which workspace the - pane *is* is not otherwise anywhere. +So the setup pass appends one line to the profile a login shell reads: - This is `aid`'s doing and not `dl`'s: a `dl -- claude ...` you typed - yourself is your command, and dl does not rewrite it. Set the variable yourself - if you want the same result from the long form. +``` +case $- in *i*) PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac +``` + +Appended, and that is the whole mechanism: two escapes in one prompt are applied in +order, so the last one sets the title. Nothing is rewritten — the visible +`vscode@devlaunch-main-zovomobo:~/repo$` still says the hostname, and only the tab +changes. (A `PROMPT_COMMAND` cannot do this job: bash runs that *before* it prints +`PS1`, so the stock escape would land afterwards and win.) Interactive shells only, +so a `dl -- cmd` one-shot, which reads the same profile through `bash -lc`, is +untouched. + +It is written once — the line carries a content-hash comment the next launch +recognises — and it rides the same round trip as the hostname stage, so it costs no +extra trip. + +**It is installed when a workspace enters Running, not on every attach.** A +workspace that is already up keeps whatever its profile was given, so +`DEVLAUNCH_NO_TITLE=1 dl ` silences dl's own escape and leaves the prompt's; +`dl recreate` is what re-decides it. That is the same bargain the hostname +stage makes, and for the same reason — the alternative is a round trip per attach. + +### The one other writer worth knowing + +**claude** writes the title continuously from its own read of what the session is +doing, which would leave a `dl -- claude` pane named after the task rather than +the workspace within a second. `aid` therefore starts claude with +`CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1`, so the workspace name is what stands. What +claude is doing is on screen inside the pane; which workspace the pane *is* is not +otherwise anywhere. + +This is `aid`'s doing and not `dl`'s: a `dl -- claude ...` you typed yourself +is your command, and dl does not rewrite it. Set the variable yourself if you want +the same result from the long form. Two multiplexer limits are worth stating, because neither is dl's to fix: @@ -556,8 +584,8 @@ Two multiplexer limits are worth stating, because neither is dl's to fix: **zellij tab names are not this.** A zellij *tab* is renamed only by `zellij action rename-tab` or a plugin; no escape sequence reaches it, which is why this names the pane instead. The window title zellij then publishes to the outer -terminal is ` | `, so the workspace id is what shows up in a -kitty tab bar. +terminal is ` | `, so the spec is what shows up in a kitty tab +bar. ## GitHub Authentication @@ -873,6 +901,10 @@ the 64-byte hostname limit on its own, but tools that stack their own prefixes o container name have about 17 characters to work with, so a tool that wants more is the one that has to shorten. +The id is *not* what you read on a tab. That shows `owner/repo@branch` — see [Naming +the terminal after the workspace](#naming-the-terminal-after-the-workspace). The id +addresses the workspace; the spec names it. + Branch names must be safe as both git refs and directory names — a name with a space or a leading dash is rejected rather than quietly rewritten. diff --git a/rust/devlaunch-core/public-api.txt b/rust/devlaunch-core/public-api.txt index e76315c7..5fece989 100644 --- a/rust/devlaunch-core/public-api.txt +++ b/rust/devlaunch-core/public-api.txt @@ -1073,7 +1073,7 @@ impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::Unquot pub trait devlaunch_core::flows::launch::ColdMachinery<'r> pub fn devlaunch_core::flows::launch::ColdMachinery::open(&mut self) -> core::result::Result, devlaunch_core::flows::launch::ColdRefused> pub trait devlaunch_core::flows::launch::Provision -pub fn devlaunch_core::flows::launch::Provision::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion) -> core::result::Result<(), devlaunch_core::flows::provision::DevpodMissing> +pub fn devlaunch_core::flows::launch::Provision::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result<(), devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::plan(&str) -> core::result::Result pub fn devlaunch_core::flows::launch::resolve_triple(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut dyn devlaunch_core::flows::launch::ColdMachinery<'_>, &devlaunch_core::domain::workspace_id::WorkspaceId, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result pub mod devlaunch_core::flows::lifecycle @@ -2002,7 +2002,7 @@ impl core::fmt::Debug for devlaunch_core::flows::provision::Switches pub fn devlaunch_core::flows::provision::Switches::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Copy for devlaunch_core::flows::provision::Switches impl core::marker::StructuralPartialEq for devlaunch_core::flows::provision::Switches -pub fn devlaunch_core::flows::provision::provision_tools(&dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, devlaunch_core::flows::provision::Switches, core::option::Option<&devlaunch_core::flows::provision::HostLayout>, core::option::Option<&devlaunch_core::flows::provision::verdict_cache::VerdictCache>, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result +pub fn devlaunch_core::flows::provision::provision_tools(&dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, devlaunch_core::flows::provision::Switches, core::option::Option<&str>, core::option::Option<&devlaunch_core::flows::provision::HostLayout>, core::option::Option<&devlaunch_core::flows::provision::verdict_cache::VerdictCache>, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result pub mod devlaunch_core::flows::repo_manager pub enum devlaunch_core::flows::repo_manager::CacheNotice pub devlaunch_core::flows::repo_manager::CacheNotice::AdoptedBareClone diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 6d486c8d..6cd73327 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -977,6 +977,7 @@ pub trait Provision { runner: &dyn Runner, workspace_id: &str, occasion: PassOccasion, + title: Option<&str>, ) -> Result<(), DevpodMissing>; } @@ -996,6 +997,7 @@ impl Provision for NoProvisioning { _runner: &dyn Runner, _workspace_id: &str, _occasion: PassOccasion, + _title: Option<&str>, ) -> Result<(), DevpodMissing> { Ok(()) } @@ -1056,10 +1058,11 @@ pub(crate) fn workspace_up( token: &HostToken, provision: &dyn Provision, request: &UpRequest<'_>, + title: Option<&str>, notices: &mut dyn Notices, ) -> Result { timing::stage_result(timing::Stage::DevpodUp, || { - up_under_stage(context, host, token, provision, request, notices) + up_under_stage(context, host, token, provision, request, title, notices) }) } @@ -1069,6 +1072,7 @@ fn up_under_stage( token: &HostToken, provision: &dyn Provision, request: &UpRequest<'_>, + title: Option<&str>, notices: &mut dyn Notices, ) -> Result { let options = context_options( @@ -1123,7 +1127,7 @@ fn up_under_stage( // hostname nobody set stays unset. Nothing the host can read tells that // apart from a container that never stopped. provision - .provision_tools(context.runner(), identity, PassOccasion::TopUp) + .provision_tools(context.runner(), identity, PassOccasion::TopUp, title) .map_err(|DevpodMissing| NotRun::NotInstalled)?; return Ok(UpOutcome::SkippedSiblingWon); } @@ -1166,7 +1170,7 @@ fn up_under_stage( // the session reads a prompt, whatever any remembered verdict says about // the tools. provision - .provision_tools(context.runner(), identity, PassOccasion::AfterUp) + .provision_tools(context.runner(), identity, PassOccasion::AfterUp, title) .map_err(|DevpodMissing| NotRun::NotInstalled)?; } drop(serialization); @@ -1756,23 +1760,32 @@ pub(crate) fn dotfiles_update( /// recent tmux), and the outer title needs `set-titles on`. The *pane* title /// always takes it. Both are the user's config, not a call dl can make. /// -/// # Why the workspace id and not the spec the user typed +/// # What the title says, and why it is not the id /// -/// The id is the only string that is uniformly available and uniformly bounded. -/// Every placement has one -- a triple, a bare name, a path, a URL -- whereas the -/// raw spec is `owner/repo` with the branch still unresolved in one arm and a -/// `./path` in another. And it is already the container's hostname, so the title -/// dl writes and the `user@host` an interactive prompt repaints over it agree -/// instead of disagreeing. +/// The name is the caller's to choose ([`Launch::titled`]): the **resolved spec**, +/// `owner/repo@ref`, for a launch that had one, and the workspace id for the three +/// arms that did not — a bare name, a path, a URL. /// -/// The bound is worth being exact about, because only one arm gets it from -/// [`WorkspaceId`]'s own 47-character cap. A bare name and a path leaf reach here -/// as the raw spec and the directory's basename, neither of which this crate -/// shortens. What bounds *those* is devpod: it refuses to create or report a -/// workspace whose name exceeds 48 characters, so a longer one fails its `up` or -/// is never found, and either way the launch ends before the handover. So the -/// title is short because a workspace with a long name cannot exist, not because -/// anything here truncates. +/// This used to be the id always, on the grounds that it is the one string every +/// placement has and that it is already the container's hostname, so the title dl +/// writes and the `user@host` an interactive prompt repaints over it agreed instead +/// of disagreeing. The first half still holds and is why the id is still the answer +/// wherever there is no triple. The second was worth less than it looked: the prompt +/// overwrites this title within a second of the session starting either way, so the +/// agreement bought a moment of consistency at the price of every tab being named +/// after a hash. An id carries no owner at all, so a fork and its upstream are two +/// tabs spelled the same; and it spells the ref as a slug, so `feature/auth` reads +/// as `feature-auth` and a long ref loses whole segments. +/// +/// The bound is worth being exact about, because no arm gets it from [`WorkspaceId`]'s +/// own 47-character cap any more. A spec is bounded by the id it derived — a triple +/// with an unsafe or overlong part is refused before a session exists — and a bare +/// name or path leaf reaches here as the raw spec and the directory's basename, +/// neither of which this crate shortens. What bounds *those* is devpod: it refuses to +/// create or report a workspace whose name exceeds 48 characters, so a longer one +/// fails its `up` or is never found, and either way the launch ends before the +/// handover. So the title is short because a workspace with a long name cannot +/// exist, not because anything here truncates. #[derive(Clone, Debug, PartialEq, Eq)] pub enum TerminalTitle { /// Write this, exactly. @@ -1782,12 +1795,15 @@ pub enum TerminalTitle { } impl TerminalTitle { - /// What this host wants for this workspace. - pub(crate) fn from_host(host: &Host, workspace_id: &str) -> Self { + /// What this host wants written for *name*. + /// + /// *name* is what a person should read, not what devpod is addressed by — see the + /// type's own docs and [`Launch::titled`] for which of the two it is. + pub(crate) fn from_host(host: &Host, name: &str) -> Self { if switched_on(host.no_title.as_deref()) || !host.stderr_tty { return Self::Off; } - match sanitize_title(workspace_id) { + match sanitize_title(name) { Some(text) => Self::Write(format!("\x1b]2;{text}\x07")), None => Self::Off, } @@ -1804,21 +1820,22 @@ impl TerminalTitle { } } -/// A workspace id with everything a terminal would read as an instruction taken -/// out, or `None` if that leaves nothing worth writing. +/// A name with everything a terminal would read as an instruction taken out, or +/// `None` if that leaves nothing worth writing. /// /// Defence at the boundary the bytes are formed at, and deliberately not sold as -/// more than that: no reachable spec is known to get an escape this far. Two arms -/// hand over a string this crate never validated -- `Plan::Existing`'s raw spec -/// and `Plan::Creatable`'s path leaf -- but both are gated on devpod agreeing the -/// workspace exists or can be created, and devpod's own name rules refuse -/// anything with a control in it. The filter is what makes that a local -/// guarantee rather than one borrowed from another program's validation, which is -/// the difference between a safe title and a title that is safe until devpod -/// loosens a rule. Dropping controls rather than escaping them keeps the sink -/// with nothing to decide. -fn sanitize_title(workspace_id: &str) -> Option { - let text: String = workspace_id.chars().filter(|ch| !ch.is_control()).collect(); +/// more than that: no reachable name is known to get an escape this far. A spec +/// cannot, because [`WorkspaceId::new`] refused every part that is not a word +/// character, dot, slash or dash before this launch had an id at all. Two arms hand +/// over a string this crate never validated -- `Plan::Existing`'s raw spec and +/// `Plan::Creatable`'s path leaf -- but both are gated on devpod agreeing the +/// workspace exists or can be created, and devpod's own name rules refuse anything +/// with a control in it. The filter is what makes that a local guarantee rather than +/// one borrowed from two other programs' validation, which is the difference between +/// a safe title and a title that is safe until devpod loosens a rule. Dropping +/// controls rather than escaping them keeps the sink with nothing to decide. +fn sanitize_title(name: &str) -> Option { + let text: String = name.chars().filter(|ch| !ch.is_control()).collect(); let text = text.trim(); if text.is_empty() { None @@ -1854,6 +1871,7 @@ fn sanitize_title(workspace_id: &str) -> Option { pub(crate) fn attach_workspace( session: &SessionContext<'_>, workspace_id: &str, + title: TerminalTitle, command: Option<&str>, forward: &mut dyn FnMut(&str), notices: &mut dyn Notices, @@ -1863,10 +1881,11 @@ pub(crate) fn attach_workspace( // about to belong to something else, and after that this process may not // print again for hours. `Off` is said too, so the sink is what decides // nothing rather than the caller deciding twice. - notices.say(LaunchNotice::TerminalTitle(TerminalTitle::from_host( - session.host, - workspace_id, - ))); + // + // Decided by the caller and not here, because what a workspace is *called* + // is a fact about the spec that was launched and this function is given only + // an id — see [`Launch::titled`]. + notices.say(LaunchNotice::TerminalTitle(title)); if command.is_none() && matches!( DotfilesRefresh::from_host(session.host), @@ -2445,6 +2464,18 @@ pub struct Launch<'a, 'r, 'l> { /// Where this launch's notices go, as they happen. A `Vec` in a test that wants /// the sequence, the binary's printer in production. notices: &'a mut dyn Notices, + /// The triple this launch resolved, when the spec was one. + /// + /// Set by [`Self::place_triple`] and read only by [`Self::titled`]: it is the + /// only place a launch holds the `owner`, `repo` and `ref` after they have been + /// collapsed into a [`Placement`]'s id, and the terminal title is the one thing + /// that wants them back. `None` for a bare name, a path and a URL, which have + /// no triple to remember. + /// + /// A field rather than a third value threaded out of `place`, because it is a + /// fact about this launch and not a step's answer: five arms reach a session and + /// every one of them goes through [`Self::attach`] carrying nothing but an id. + resolved: Option, } impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { @@ -2466,6 +2497,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { forward, token: HostToken::new(), notices, + resolved: None, } } @@ -2561,6 +2593,11 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { Ok(workspace) => workspace, Err(unsafe_name) => return Ok(Err(LaunchRefusal::UnsafeSpec(unsafe_name))), }; + // Remembered for the terminal title, which is the one later step that wants + // the triple rather than the id it derives: an id carries no owner and + // spells the ref as a slug, so `blooop/devlaunch@feature/auth` is a name + // this launch can put on a tab and nothing downstream could reconstruct. + self.resolved = Some(workspace.clone()); // A devpod that could not be run ends the launch here, before the clone: // it is the probe Python raises `DevpodNotInstalled` out of. let resolved = resolve_triple(self.context, self.cold, &workspace, &mut *self.notices) @@ -2734,6 +2771,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { self.context.runner(), placement.workspace_id(), PassOccasion::TopUp, + self.container_title(placement.workspace_id()).as_deref(), ) .map_err(|DevpodMissing| LaunchAborted::DevpodNotRun(NotRun::NotInstalled))?; return Ok(Launched::AlreadyRunning); @@ -2766,12 +2804,14 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { Placement::Known { .. } | Placement::Listed { .. } => Naming::Anonymous, }; let request = UpRequest::new(placement.source(), naming); + let title = self.container_title(placement.workspace_id()); let outcome = workspace_up( self.context, self.host, &self.token, self.provision, &request, + title.as_deref(), &mut *self.notices, ) .map_err(LaunchAborted::DevpodNotRun)?; @@ -2801,12 +2841,14 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { .with_ide(verb.ide()) .with_rebuild(verb.rebuild()) .with_devcontainer(devcontainer); + let title = self.container_title(placement.workspace_id()); let outcome = workspace_up( self.context, self.host, &self.token, self.provision, &request, + title.as_deref(), &mut *self.notices, ) .map_err(LaunchAborted::DevpodNotRun)?; @@ -2831,10 +2873,12 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { workspace_id: &str, command: Option<&str>, ) -> Result { + let title = TerminalTitle::from_host(self.host, &self.titled(workspace_id)); let context = SessionContext::new(self.context.runner(), self.host, &self.token); let session = attach_workspace( &context, workspace_id, + title, command, self.forward, &mut *self.notices, @@ -2847,6 +2891,58 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { } } + /// What to call this workspace where a person reads it, rather than where devpod + /// is addressed. + /// + /// The spec when this launch resolved one — `owner/repo@ref`, the three parts + /// exactly as the user's own spec spelled them — and the workspace id otherwise. + /// + /// The spec is the better name for the two reasons an id is a worse one. It + /// carries the **owner**, which an id + /// ([`devlaunch_core::domain::workspace_id`](crate::domain::workspace_id)) does + /// not hold at all, so a fork and its upstream are two tabs named the same. And + /// it spells the **ref**, where an id holds a slug of one: `feature/auth` is + /// `feature-auth` in an id, indistinguishable from the branch of that name, and a + /// long ref loses whole segments. Neither is recoverable from the id afterwards, + /// which is why the triple is remembered rather than read back. + /// + /// The id for the other three arms, and not as a fallback so much as the only + /// name they have: a bare workspace name *is* the id, and a path or URL spec + /// never had a ref for an `@` to precede. Bounded for the same reasons as before + /// — see [`TerminalTitle`] — and a spec is bounded by the id it derived, since a + /// triple whose parts overflow 47 characters is refused before this. + fn titled(&self, workspace_id: &str) -> String { + match &self.resolved { + Some(workspace) => format!( + "{}/{}@{}", + workspace.owner(), + workspace.repo(), + workspace.git_ref() + ), + None => workspace_id.to_owned(), + } + } + + /// The name a shell in this container should keep putting on the terminal, or + /// `None` when this host wants none. + /// + /// The same name [`Self::titled`] gives dl's own write, and the same switch — + /// `DEVLAUNCH_NO_TITLE` — decides both, so one variable governs the whole + /// feature rather than half of it. + /// + /// `stderr_tty` is deliberately *not* consulted, where + /// [`TerminalTitle::from_host`] does consult it. That flag answers "is there a + /// terminal to write to right now", which is the right question for one escape + /// this process is about to emit and the wrong one for a line installed in a + /// profile: `dl up` is a prewarm with its output redirected, and the + /// interactive session that arrives later is the one the line is for. + fn container_title(&self, workspace_id: &str) -> Option { + if switched_on(self.host.no_title.as_deref()) { + return None; + } + Some(self.titled(workspace_id)) + } + /// A session outcome, with the two never-ran arms lifted to [`LaunchAborted`]. fn session( &mut self, @@ -3065,23 +3161,24 @@ mod tests { } } - /// Records which workspaces had tools lent to them, and on which occasion. + /// Records which workspaces had tools lent to them, on which occasion, and under + /// what name the container was told to title a terminal. /// - /// The occasion is recorded beside the id rather than instead of it, because - /// the two are asserted separately: most tests here are about *whether* a path - /// provisions at all, and only the ones about the pass's cost care which - /// occasion it named. Both come out of one list so a test cannot read them out - /// of step with each other. + /// All three in one list rather than three, because they are asserted + /// separately: most tests here are about *whether* a path provisions at all, + /// only the ones about the pass's cost care which occasion it named, and only + /// the title tests care about the third. One list is what keeps a test from + /// reading them out of step with each other. #[derive(Debug, Default)] struct RecordingProvision { - passes: Mutex>, + passes: Mutex)>>, /// A devpod that goes missing when the pass is asked for, which is the one /// thing a pass can answer. lost_devpod: bool, } impl RecordingProvision { - fn passes(&self) -> Vec<(String, PassOccasion)> { + fn passes(&self) -> Vec<(String, PassOccasion, Option)> { self.passes .lock() .unwrap_or_else(PoisonError::into_inner) @@ -3091,14 +3188,22 @@ mod tests { fn provisioned(&self) -> Vec { self.passes() .into_iter() - .map(|(workspace_id, _)| workspace_id) + .map(|(workspace_id, _, _)| workspace_id) .collect() } fn occasions(&self) -> Vec { self.passes() .into_iter() - .map(|(_, occasion)| occasion) + .map(|(_, occasion, _)| occasion) + .collect() + } + + /// What each pass was told to have the container's shells title a terminal. + fn titles(&self) -> Vec> { + self.passes() + .into_iter() + .map(|(_, _, title)| title) .collect() } } @@ -3109,11 +3214,12 @@ mod tests { _runner: &dyn Runner, workspace_id: &str, occasion: PassOccasion, + title: Option<&str>, ) -> Result<(), DevpodMissing> { self.passes .lock() .unwrap_or_else(PoisonError::into_inner) - .push((workspace_id.to_owned(), occasion)); + .push((workspace_id.to_owned(), occasion, title.map(str::to_owned))); if self.lost_devpod { return Err(DevpodMissing); } @@ -3124,13 +3230,16 @@ mod tests { fn no_notices() -> Vec { Vec::new() } - /// A sink that drops devpod's own session chatter, for the tests that are not /// about it. A plain `fn` pointer, because that is an `FnMut(&str)` a struct /// can hold by value and hand out as `&mut dyn FnMut(&str)`. fn nowhere(_line: &str) {} /// [`attach_workspace`] with the chatter thrown away. + /// + /// Titles the workspace after its id, which is what the bare-name arm does: these + /// tests are given an id and no triple, so there is no spec for [`Launch::titled`] + /// to prefer. The launch-level tests are where the spec form is pinned. fn attaching( scene: &Scene, token: &HostToken, @@ -3139,7 +3248,15 @@ mod tests { notices: &mut Vec, ) -> Result { let session = SessionContext::new(&scene.runner, &scene.host, token); - attach_workspace(&session, workspace_id, command, &mut nowhere, notices) + let title = TerminalTitle::from_host(&scene.host, workspace_id); + attach_workspace( + &session, + workspace_id, + title, + command, + &mut nowhere, + notices, + ) } /// Backdate `path`'s modification time by `by`. @@ -3326,6 +3443,7 @@ mod tests { &token, provision, request, + None, &mut inner, ); (outcome, inner) @@ -3575,6 +3693,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -3612,6 +3731,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3644,6 +3764,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3674,6 +3795,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3705,6 +3827,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -4559,6 +4682,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -4596,6 +4720,7 @@ mod tests { workspace_id: "myws", }, ), + None, &mut no_notices(), ); @@ -4720,6 +4845,7 @@ mod tests { workspace_id: "myws", }, ), + None, &mut notices, ); let _ = attaching(&logged_in, &token, "myws", None, &mut notices); @@ -5215,6 +5341,109 @@ mod tests { ); } + #[test] + fn a_triple_names_the_terminal_after_the_spec_and_not_after_the_id() { + // What a person reads on the tab is the spec they typed, resolved. The id is + // still what devpod is addressed by in the same launch -- the `status` and + // `ssh` below -- so this is the one place the two names part company. + // + // `feature/auth` is the ref that makes the difference matter rather than + // merely look nicer: the id spells it `feature-auth`, which is also the name + // of a different branch this repository could have, so the id is a tab name + // that cannot say which of the two the session is in. + let workspace = + WorkspaceId::new("blooop", "devlaunch", "feature/auth").expect("a safe triple"); + let mut scene = Scene::new().with_running(&workspace.value()); + scene.host.stderr_tty = true; + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + + let launched = launch.run( + "blooop/devlaunch@feature/auth", + &LaunchVerb::Attach { + command: Some("echo hi".to_owned()), + }, + None, + ); + + assert_eq!( + launched, + Ok(Launched::Session(Session::RemoteExit { status: 0 })) + ); + assert!( + parts.said.iter().any(|notice| notice + == &LaunchNotice::TerminalTitle(TerminalTitle::Write( + "\x1b]2;blooop/devlaunch@feature/auth\x07".to_owned() + ))), + "{:?}", + parts.said + ); + // And the id is what devpod was given, unchanged by any of this. + assert_eq!(workspace.value(), "devlaunch-feature-auth-poliseno"); + assert!( + scene + .devpod_commands() + .iter() + .all(|call| call.contains(&workspace.value())), + "{:?}", + scene.devpod_commands() + ); + } + + #[test] + fn a_bare_name_names_the_terminal_after_the_id_because_that_is_all_it_has() { + // The other three arms have no triple to prefer, and for a bare name the id + // *is* what the user typed. Pinned beside the spec case so that a change to + // one has to say what it means for the other. + let mut scene = Scene::new().with_running("myws"); + scene.host.stderr_tty = true; + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + + let launched = launch.run( + "myws", + &LaunchVerb::Attach { + command: Some("echo hi".to_owned()), + }, + None, + ); + + assert_eq!( + launched, + Ok(Launched::Session(Session::RemoteExit { status: 0 })) + ); + assert!( + parts.said.iter().any(|notice| notice + == &LaunchNotice::TerminalTitle(TerminalTitle::Write( + "\x1b]2;myws\x07".to_owned() + ))), + "{:?}", + parts.said + ); + } + #[test] fn a_warm_bare_name_attach_is_one_status_and_one_ssh() { let scene = Scene::new().with_running("myws"); @@ -5516,6 +5745,98 @@ mod tests { ); } + #[test] + fn the_pass_is_told_to_have_the_container_keep_titling_after_the_spec() { + // The other half of the title, and the half that lasts. dl's own escape is + // overwritten by the first interactive prompt; this is the name the pass + // installs in the container's profile so every prompt after that writes it + // again. Same string as the escape, same switch governing it -- one name, + // two places it has to reach. + let workspace = + WorkspaceId::new("blooop", "devlaunch", "feature/auth").expect("a safe triple"); + let scene = Scene::new().with_running(&workspace.value()); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("blooop/devlaunch@feature/auth", &LaunchVerb::Up, None) + }; + + assert_eq!(launched, Ok(Launched::AlreadyRunning)); + assert_eq!( + parts.provision.titles(), + vec![Some("blooop/devlaunch@feature/auth".to_owned())] + ); + } + + #[test] + fn the_title_switch_reaches_the_container_and_not_just_dls_own_escape() { + // `DEVLAUNCH_NO_TITLE` has to govern both halves or it governs neither: a + // host that silenced the escape and still had its profile edited would find + // the variable did nothing it could see. + let mut scene = Scene::new().with_running("myws"); + scene.host.no_title = Some("1".to_owned()); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("myws", &LaunchVerb::Up, None) + }; + + assert_eq!(launched, Ok(Launched::AlreadyRunning)); + assert_eq!(parts.provision.titles(), vec![None]); + } + + #[test] + fn a_headless_up_still_installs_the_name_the_next_session_will_read() { + // `stderr_tty` is false here, which is `dl up` redirected -- a prewarm. + // dl writes no escape of its own, correctly: there is no terminal to write + // to. The container is still taught the name, because the session that + // arrives later is the one it is for, and asking this pass to guess whether + // one ever will is a question it cannot answer. + let scene = Scene::new().with_running("myws"); + assert!(!scene.host.stderr_tty, "the premise of this test"); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("myws", &LaunchVerb::Up, None) + }; + + assert_eq!(launched, Ok(Launched::AlreadyRunning)); + assert_eq!(parts.provision.titles(), vec![Some("myws".to_owned())]); + } + /// `dl up` is what a user types to fix a workspace, so it is the worst /// verb to answer "already running" for a create that never finished: the /// container is up, nothing in it is set up, and the one command documented as @@ -6104,6 +6425,7 @@ mod tests { workspace_id: "brand-new", }, ), + None, &mut no_notices(), ) }); @@ -6169,6 +6491,7 @@ mod tests { workspace_id: "brand-new", }, ), + None, &mut no_notices(), ) }); diff --git a/rust/devlaunch-core/src/flows/provision.rs b/rust/devlaunch-core/src/flows/provision.rs index 0b9a5e4b..df256dec 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -196,6 +196,10 @@ pub(crate) const HOSTNAME_STAGE: StageName = StageName::new("hostname"); /// of round trips: it rides the pass every entry into Running already pays. pub(crate) const ZELLIJ_STAGE: StageName = StageName::new("zellij"); +/// The stage that teaches the shell to keep naming the terminal after this +/// workspace. Rides the same trip as the two above it. +pub(crate) const TITLE_STAGE: StageName = StageName::new("title"); + // =========================================================================== // quoting // =========================================================================== @@ -465,6 +469,40 @@ pub(crate) fn profile_prepend(line: &str, on_failure: Option<&str>) -> String { ) } +/// The `PS1` edit that keeps a pane named after *title* for the whole session. +/// +/// dl writes an OSC 2 of its own just before the handover, and for a one-shot +/// `dl -- cmd` that is the end of it: nothing in that session renders a prompt. +/// An **interactive** session overwrites it within a second, because Ubuntu's stock +/// `~/.bashrc` puts `\e]0;\u@\h: \w\a` at the *front* of `PS1` — so every prompt +/// renames the pane after the container's hostname, which is the workspace id. This +/// line is what makes dl's name the one that stands. +/// +/// **Appended to `PS1`, and that is the whole mechanism.** A `PROMPT_COMMAND` cannot +/// do it: bash runs that *before* it expands and prints `PS1`, so the stock title +/// escape would be written afterwards and win. Two escapes in one prompt string are +/// resolved by the terminal in order, so the last one sets the title — which is why +/// this goes on the end rather than replacing anything. Nothing is rewritten, so a +/// `PS1` an image or a dotfile built stays exactly as it was, prompt text included: +/// the visible `user@host:path$` still names the hostname, and only the tab changes. +/// +/// Interactive shells only (`case $- in *i*`), because a login shell that renders no +/// prompt has no use for one and `bash -lc` reads this file on every `dl -- +/// cmd`. +/// +/// *title* is interpolated as its own quoted word rather than into the double-quoted +/// assignment, so a name holding a `$` or a backtick is text and not shell. A spec +/// cannot hold either — [`WorkspaceId`](crate::domain::workspace_id::WorkspaceId) +/// refused every character but word ones, dots, slashes and dashes — but the other +/// three placements title after a bare devpod name or a path leaf, which this crate +/// never validated. +fn profile_title_line(title: &str) -> String { + format!( + r#"case $- in *i*) PS1="$PS1\[\e]2;"{}"\a\]" ;; esac"#, + quote(title) + ) +} + /// The twelve hex characters of SHA-256 that name one appended line. fn mark_digest(line: &str) -> String { let digest = Sha256::digest(line.as_bytes()); @@ -922,10 +960,25 @@ impl Stage { /// installing it is tool provisioning; `DEVLAUNCH_NO_ZELLIJ` covers only zellij, so /// a host can keep the `gh`/`claude` guarantee and still stop the stage. Neither /// touches the hostname, which is not tools work under either variable. +/// +/// `title` is the name a shell in this container should keep putting on the terminal, +/// or `None` for a launch that wants none — which is `DEVLAUNCH_NO_TITLE`, decided by +/// the caller from the same [`Host`](crate::flows::launch::Host) that decides dl's +/// own write. Neither tools switch touches it either: naming a pane is no more tool +/// provisioning than naming a container is. +/// +/// **The stage runs on the pass, so the name is installed when a workspace enters +/// Running and not on every attach.** A workspace already up keeps whatever its +/// profile was given, so `DEVLAUNCH_NO_TITLE=1 dl ` silences dl's own write and +/// leaves the prompt's — `dl recreate` is what re-decides it. That is the same +/// bargain the hostname stage makes, and for the same reason: the alternative is a +/// round trip per attach, which is what moving the hostname off the attach bought +/// back (#157). pub(crate) fn setup_stages( workspace: &str, tools: ToolsSwitch, zellij: ZellijSwitch, + title: Option<&str>, ) -> Vec { let mut stages = vec![ // The hostname appears in the bash prompt (user@hostname:path$), which is @@ -939,6 +992,32 @@ pub(crate) fn setup_stages( ) .quieter(), ]; + if let Some(title) = title { + stages.push( + Stage::new( + TITLE_STAGE, + // Two statements, so a nested `bash -c` for the reason the zellij + // stage has one: a stage is interpolated into `if ; then`, + // which is one line. The profile is resolved and appended to exactly + // as the PATH writers do it, so all four edits find each other's + // dedupe marks in the one file bash will actually read. + format!( + "bash -c {}", + quote( + &[ + profile_resolution("$HOME"), + profile_prepend(&profile_title_line(title), None), + ] + .join("\n") + ) + ), + ) + // Quieter for the hostname stage's reason: an image that will not let + // this be written is a tab with a duller name, not a launch to warn + // about. + .quieter(), + ); + } if let (ToolsSwitch::Install, ZellijSwitch::Install) = (tools, zellij) { stages.push(Stage::new( ZELLIJ_STAGE, @@ -1682,28 +1761,38 @@ pub struct DevpodMissing; /// there is one, it is read on a [`PassOccasion::TopUp`] and written after any pass /// that probed provisioned; see [`verdict_cache`] for what makes a remembered /// verdict still true. +/// +/// `title` sits beside `switches` rather than inside it, and that is the lifetime +/// and not a judgement about where it belongs: [`Switches`] is `Copy` and built by +/// `from_env`, and a borrowed field would make it `Switches<'_>` everywhere it is +/// held. One more positional parameter is the cheaper of the two, and the parameter +/// it follows is the one it goes with. +#[allow(clippy::too_many_arguments)] pub fn provision_tools( runner: &dyn Runner, workspace: &str, occasion: PassOccasion, switches: Switches, + title: Option<&str>, host: Option<&HostLayout>, verdicts: Option<&VerdictCache>, events: &mut dyn Notices, ) -> Result { timing::stage_result(timing::Stage::Tools, || { provision( - runner, workspace, occasion, switches, host, verdicts, events, + runner, workspace, occasion, switches, title, host, verdicts, events, ) }) } /// [`provision_tools`] without the stage guard around it. +#[allow(clippy::too_many_arguments)] fn provision( runner: &dyn Runner, workspace: &str, occasion: PassOccasion, switches: Switches, + title: Option<&str>, host: Option<&HostLayout>, verdicts: Option<&VerdictCache>, events: &mut dyn Notices, @@ -1722,7 +1811,7 @@ fn provision( // Before the pass, not after it: see [`VerdictCache::observe`]. let observed = verdicts.and_then(|verdicts| verdicts.observe(workspace)); - let found = match setup_pass(runner, workspace, switches, events) { + let found = match setup_pass(runner, workspace, switches, title, events) { Ok(found) => found, Err(refusal) => return refused(workspace, refusal, events), }; @@ -1834,9 +1923,10 @@ fn setup_pass( runner: &dyn Runner, workspace: &str, switches: Switches, + title: Option<&str>, events: &mut dyn Notices, ) -> Result { - let stages = setup_stages(workspace, switches.tools, switches.zellij); + let stages = setup_stages(workspace, switches.tools, switches.zellij, title); let call = Call::new([ "ssh", workspace, @@ -2445,6 +2535,7 @@ fi "myws", PassOccasion::AfterUp, switches, + None, Some(host), None, &mut events, @@ -2813,7 +2904,7 @@ fi // The round trip Python's own tests assert with `shlex.split(runner.script())`: // whatever the quoting, the remote shell has to recover exactly `bash`, the // flag, and one script. - let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install); + let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None); for (payload, flag, script) in [ ( format!("bash -lc {}", quote(&setup_script(&stages))), @@ -2840,7 +2931,7 @@ fi fn the_zellij_stage_is_one_word_the_pass_shell_hands_to_a_nested_bash() { // The stage is interpolated into `if ; then`, so the quoting has to // survive being read by the *pass's* shell before the nested bash sees it. - let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install); + let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None); let command = &stages[1].command; let words = shlex::split(command).expect("a stage a shell can read"); assert_eq!( @@ -2887,6 +2978,7 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Install, + None, )); assert_eq!( with_zellij, @@ -2896,6 +2988,7 @@ fi "myws", ToolsSwitch::Skip, ZellijSwitch::Install, + None, )); assert_eq!( opted_out, @@ -3661,6 +3754,7 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Install, + None, )); assert!(script.contains(&probe_script())); } @@ -3670,7 +3764,7 @@ fi // Order, and it is not cosmetic: the probe exits early when a tool is // missing, which is the commonest cold-path answer, so a stage placed behind // it would report "not reached" on the very launches the fold exists for. - let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install); + let stages = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None); let script = setup_script(&stages); let probe_at = script.find(&probe_script()).expect("the probe is in there"); for stage in &stages { @@ -3692,6 +3786,7 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Install, + None, )); assert!(!script.contains("set -e")); } @@ -3704,6 +3799,7 @@ fi name, ToolsSwitch::Install, ZellijSwitch::Install, + None, )); assert!(script.contains(&format!("hostname {}", quote(name)))); assert!(!script.contains("hostname myws;")); @@ -3740,6 +3836,7 @@ fi workspace, ToolsSwitch::Install, ZellijSwitch::Install, + None, )); let ran = bash_with( &script, @@ -3763,7 +3860,7 @@ fi fn outcome_of(report: &str, stage: StageName) -> Option { stage_outcomes( report, - &setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install), + &setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None), ) .into_iter() .find(|outcome| outcome.stage == stage) @@ -3822,10 +3919,11 @@ fi /// the one stage because every assertion below is about how one reported line is /// *read*. fn hostname_outcome(report: &str) -> Vec { - let stages: Vec = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install) - .into_iter() - .filter(|stage| stage.name == HOSTNAME_STAGE) - .collect(); + let stages: Vec = + setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None) + .into_iter() + .filter(|stage| stage.name == HOSTNAME_STAGE) + .collect(); assert!(!stages.is_empty(), "the pass no longer names the container"); stage_outcomes(report, &stages) } @@ -3934,7 +4032,8 @@ fi setup_script(&setup_stages( "myws", ToolsSwitch::Install, - ZellijSwitch::Install + ZellijSwitch::Install, + None )) ); } @@ -4061,6 +4160,7 @@ fi Switches::INSTALLING, None, None, + None, &mut events, ); @@ -4181,7 +4281,8 @@ fi setup_script(&setup_stages( "myws", ToolsSwitch::Skip, - ZellijSwitch::Install + ZellijSwitch::Install, + None )) ); assert!(words[2].contains("sudo hostname myws")); @@ -4226,16 +4327,189 @@ fi // DEVLAUNCH_NO_ZELLIJ: the narrower opt-out // ======================================================================= + // ------------------------------------------------------- the title stage + + #[test] + fn a_title_puts_a_stage_in_the_pass_and_no_title_leaves_it_out() { + // The stage exists only when there is a name to install, so a host that + // turned titles off pays no part of it — not the append, and not the line in + // the profile that a later launch would have to reason about. + let named: Vec = setup_stages( + "myws", + ToolsSwitch::Install, + ZellijSwitch::Install, + Some("blooop/devlaunch@main"), + ) + .iter() + .map(|stage| stage.name) + .collect(); + let unnamed: Vec = + setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None) + .iter() + .map(|stage| stage.name) + .collect(); + + assert!(named.contains(&TITLE_STAGE), "{named:?}"); + assert!(!unnamed.contains(&TITLE_STAGE), "{unnamed:?}"); + } + + #[test] + fn neither_tools_switch_touches_the_title_stage() { + // The same line the hostname stage is on: naming a pane is not tool + // provisioning, so a host that installs nothing still gets its tabs named. + // `DEVLAUNCH_NO_TITLE` is the one variable that stops it. + for (tools, zellij) in [ + (ToolsSwitch::Skip, ZellijSwitch::Skip), + (ToolsSwitch::Skip, ZellijSwitch::Install), + (ToolsSwitch::Install, ZellijSwitch::Skip), + ] { + let names: Vec = + setup_stages("myws", tools, zellij, Some("blooop/devlaunch@main")) + .iter() + .map(|stage| stage.name) + .collect(); + + assert!( + names.contains(&TITLE_STAGE), + "{tools:?} {zellij:?} {names:?}" + ); + } + } + + #[test] + fn the_title_line_appends_to_ps1_so_it_is_the_last_write_of_every_prompt() { + // The mechanism, spelled out because it is the whole reason this works. + // Ubuntu's stock `~/.bashrc` puts `\e]0;\u@\h: \w\a` at the *front* of PS1, + // so a prompt renames the pane after the hostname -- which is the workspace + // id, the thing the spec is here to replace. Two escapes in one prompt are + // applied in order, so the last one wins and this one has to be appended. + // + // A `PROMPT_COMMAND` would lose: bash runs that before it prints PS1, so the + // stock escape would come afterwards. + let line = profile_title_line("blooop/devlaunch@main"); + + // Bare rather than single-quoted, because `shell::quote` leaves a word made + // only of `[A-Za-z0-9_@%+=:,./-]` alone -- and every one of those is inert on + // the right of an assignment, where a shell splits no words and expands no + // globs. A name that needs quotes gets them; the test below is that one. + assert_eq!( + line, + r#"case $- in *i*) PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac"# + ); + // `$PS1` first, so nothing an image or a dotfile put in the prompt is + // rewritten -- only added to. + assert!(line.contains(r#"PS1="$PS1"#), "{line}"); + // And it is inert in the shells that render no prompt, which is every + // `dl -- cmd`: `bash -lc` reads the profile too. + assert!(line.starts_with("case $- in *i*)"), "{line}"); + } + + #[test] + fn a_title_holding_shell_metacharacters_is_text_and_not_shell() { + // A spec cannot hold these -- `WorkspaceId` refused every character but word + // ones, dots, slashes and dashes -- but the other three placements title + // after a bare devpod name or a path leaf, which this crate never validated. + // So the name is its own quoted word rather than interpolated into the + // double-quoted assignment. + let line = profile_title_line("$(touch /tmp/pwned)`id`'x"); + + assert!( + line.contains(r#"'$(touch /tmp/pwned)`id`'"'"'x'"#), + "{line}" + ); + } + + #[test] + fn the_title_stage_edits_the_profile_a_login_shell_will_actually_read() { + // The append rides the same `$PROFILE` resolution the PATH writers use, so + // all of them edit the one file bash reads and find each other's dedupe + // marks there. A stage that picked `~/.profile` in an image shipping a + // `~/.bash_profile` would write to a file nothing sources -- the failure the + // resolution exists for -- and would do it silently. + let stages = setup_stages( + "myws", + ToolsSwitch::Install, + ZellijSwitch::Install, + Some("blooop/devlaunch@main"), + ); + let stage = stages + .iter() + .find(|stage| stage.name == TITLE_STAGE) + .expect("the title stage"); + + assert!(stage.command.contains(".bash_profile"), "{}", stage.command); + assert!(stage.command.contains(PROFILE_MARK), "{}", stage.command); + } + + #[test] + fn a_real_bash_over_the_title_stage_leaves_a_profile_that_titles_the_pane() { + // The stage's effect rather than its text: run the composed script with a + // real bash over a real HOME, then source what it wrote and read PS1 back. + // What this pins is the ordering the whole feature rests on -- our OSC 2 ends + // up *after* the stock title escape, so it is the write that stands. + let scratch = scratch(); + let home = scratch.path(); + let stages = setup_stages( + "myws", + ToolsSwitch::Skip, + ZellijSwitch::Skip, + Some("blooop/devlaunch@main"), + ); + let stage = stages + .iter() + .find(|stage| stage.name == TITLE_STAGE) + .expect("the title stage"); + + let install = std::process::Command::new("bash") + .args(["-c", &stage.command]) + .env("HOME", home) + .output() + .expect("bash to run the stage"); + assert!(install.status.success(), "{install:?}"); + + // Ubuntu's stock interactive PS1, then the profile the stage just wrote. + let read_back = std::process::Command::new("bash") + .args([ + // `-i` and not `set -i`: the line guards on `$-` holding an `i`, + // which only a shell *started* interactive has. That guard is the + // feature -- it keeps the edit out of every `bash -lc` one-shot -- so + // a test that sidestepped it would be pinning a different line. + "-i", + "-c", + r#"PS1='\[\e]0;\u@\h: \w\a\]\u@\h:\w\$ '; . "$HOME/.profile"; printf '%s' "$PS1""#, + ]) + .env("HOME", home) + .output() + .expect("bash to read the profile back"); + let ps1 = String::from_utf8_lossy(&read_back.stdout).to_string(); + + let stock = ps1.find(r"\e]0;").expect("the stock title escape"); + let ours = ps1.find(r"\e]2;").expect("our title escape"); + assert!(ours > stock, "ours must come last: {ps1:?}"); + assert!(ps1.contains("blooop/devlaunch@main"), "{ps1:?}"); + + // Appended once, however many times the pass runs: the dedupe mark is what + // keeps a profile from growing one escape per launch. + let again = std::process::Command::new("bash") + .args(["-c", &stage.command]) + .env("HOME", home) + .output() + .expect("bash to run the stage again"); + assert!(again.status.success(), "{again:?}"); + let profile = std::fs::read_to_string(home.join(".profile")).expect("the profile"); + assert_eq!(profile.matches(r"\e]2;").count(), 1, "{profile:?}"); + } #[test] fn the_zellij_opt_out_drops_only_the_zellij_stage() { // The whole reason for a second variable: a host that wants no zellij // installed keeps everything `DEVLAUNCH_NO_TOOLS` would have cost it — // the container is still named, and the pass still probes for the `gh` // and `claude` the workspace is guaranteed. - let names: Vec = setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Skip) - .iter() - .map(|stage| stage.name) - .collect(); + let names: Vec = + setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Skip, None) + .iter() + .map(|stage| stage.name) + .collect(); assert!(!names.contains(&ZELLIJ_STAGE), "{names:?}"); assert!(names.contains(&HOSTNAME_STAGE), "{names:?}"); @@ -4267,10 +4541,11 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Skip, + None, )); for tools in [ToolsSwitch::Install, ToolsSwitch::Skip] { assert_eq!( - setup_script(&setup_stages("myws", tools, ZellijSwitch::Skip)), + setup_script(&setup_stages("myws", tools, ZellijSwitch::Skip, None)), without, "{tools:?}" ); @@ -4279,7 +4554,8 @@ fi setup_script(&setup_stages( "myws", ToolsSwitch::Skip, - ZellijSwitch::Install + ZellijSwitch::Install, + None )), without ); @@ -4287,7 +4563,8 @@ fi setup_script(&setup_stages( "myws", ToolsSwitch::Install, - ZellijSwitch::Install + ZellijSwitch::Install, + None )), without, "the stage is there when nothing asked for it to go" @@ -4411,6 +4688,7 @@ fi "myws", occasion, Switches::INSTALLING, + None, Some(¬hing_to_lend()), Some(verdicts), &mut Vec::new(), @@ -4531,6 +4809,7 @@ fi "myws", PassOccasion::TopUp, Switches::INSTALLING, + None, Some(¬hing_to_lend()), Some(&verdicts), &mut Vec::new(), @@ -4616,6 +4895,7 @@ fi "myws", occasion, Switches::INSTALLING, + None, Some(¬hing_to_lend()), None, &mut Vec::new(), @@ -4724,7 +5004,7 @@ fi FailureLevel::Warning ); let levels: Vec<(StageName, FailureLevel)> = - setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install) + setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None) .iter() .map(|stage| (stage.name, stage.failure_level)) .collect(); @@ -5185,7 +5465,7 @@ fi ) -> (PathBuf, Output, String) { let (home, sysbin, log) = zellij_sandbox(scratch, has_zellij, pixi_exit); let ran = bash_with( - &setup_script(&setup_stages("myws", switches.tools, switches.zellij)), + &setup_script(&setup_stages("myws", switches.tools, switches.zellij, None)), &[ ("HOME", &home.to_string_lossy()), ("PATH", &sysbin.to_string_lossy()), @@ -5217,7 +5497,7 @@ fi // goes through carries a zellij stage. No dotfiles, no devcontainer.json, no // repo cooperation — the ask comes from the invocation. let names: Vec = - setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install) + setup_stages("myws", ToolsSwitch::Install, ZellijSwitch::Install, None) .iter() .map(|stage| stage.name) .collect(); @@ -5346,10 +5626,11 @@ fi // the hostname stage, which is not tools work and is deliberately left // outside that switch — a machine that turned tool installs off has not // thereby asked for unnamed containers. - let names: Vec = setup_stages("myws", ToolsSwitch::Skip, ZellijSwitch::Install) - .iter() - .map(|stage| stage.name) - .collect(); + let names: Vec = + setup_stages("myws", ToolsSwitch::Skip, ZellijSwitch::Install, None) + .iter() + .map(|stage| stage.name) + .collect(); assert!(!names.contains(&ZELLIJ_STAGE), "{names:?}"); assert!(names.contains(&HOSTNAME_STAGE), "{names:?}"); diff --git a/rust/dl/src/launch.rs b/rust/dl/src/launch.rs index 719343e4..ce2e3aa7 100644 --- a/rust/dl/src/launch.rs +++ b/rust/dl/src/launch.rs @@ -154,6 +154,7 @@ impl Provision for ToolProvisioning { runner: &dyn Runner, workspace_id: &str, occasion: PassOccasion, + title: Option<&str>, ) -> Result<(), DevpodMissing> { // The events stream through the same sink as the launch's own notices — // one line on stderr at the moment core says it, which is Python's order: @@ -164,6 +165,7 @@ impl Provision for ToolProvisioning { workspace_id, occasion, self.switches, + title, self.host.as_ref(), Some(&self.verdicts), &mut render::Saying, diff --git a/rust/dl/tests/launch.rs b/rust/dl/tests/launch.rs index 407eb3c6..e915d472 100644 --- a/rust/dl/tests/launch.rs +++ b/rust/dl/tests/launch.rs @@ -607,6 +607,7 @@ fn a_cold_triple_prepares_a_clone_creates_the_workspace_and_attaches() { "Creating workspace clone at {{ROOT}}/cache/devlaunch/repos/blooop/devlaunch/{COLD}" ), &format!("{COLD}: the hostname setup stage did not report; it may not have run."), + &format!("{COLD}: the title setup stage did not report; it may not have run."), &format!("{COLD}: the zellij setup stage did not report; it may not have run."), &format!("SSH command: devpod ssh {COLD}"), ] @@ -714,6 +715,7 @@ fn up_on_a_running_workspace_says_so_and_still_provisions_the_tools() { // pass that tops its tools up runs, because that is when it was found. &format!("Workspace {MAIN} is already running.") as &str, &format!("{MAIN}: the hostname setup stage did not report; it may not have run."), + &format!("{MAIN}: the title setup stage did not report; it may not have run."), &format!("{MAIN}: the zellij setup stage did not report; it may not have run."), ] ); From 0caa21451d08bfed901a9d585dd36ad13df4d53c Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 22 Aug 2026 19:44:02 +0000 Subject: [PATCH 2/5] fix: opening a workspace by its id renamed its tab back to the hash 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`. --- README.md | 7 ++ rust/devlaunch-core/src/flows/launch.rs | 118 ++++++++++++++++++++---- rust/dl/tests/launch.rs | 1 - 3 files changed, 107 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 7dc4cc48..96cc52da 100644 --- a/README.md +++ b/README.md @@ -553,6 +553,13 @@ It is written once — the line carries a content-hash comment the next launch recognises — and it rides the same round trip as the hostname stage, so it costs no extra trip. +**Only a spec is installed this way.** `dl myworkspace` teaches the container +nothing: the id is already its hostname, so the stock prompt writes that anyway. It +also cannot, safely — the line is recognised by a hash of its own text, so a second, +different name for one workspace would not replace the first but sit after it, and +the last one wins. Keying on the spec alone means a workspace has at most one such +line, ever. + **It is installed when a workspace enters Running, not on every attach.** A workspace that is already up keeps whatever its profile was given, so `DEVLAUNCH_NO_TITLE=1 dl ` silences dl's own escape and leaves the prompt's; diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 6cd73327..0f35a93f 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -1848,6 +1848,21 @@ fn sanitize_title(name: &str) -> Option { // the attach // =========================================================================== +/// `owner/repo@ref`, the three parts of a triple as the user's own spec spelled +/// them. +/// +/// One rendering, for the two places a launch names a workspace to a person: the +/// escape dl writes and the line the pass installs. Two spellings of it would be two +/// names for one workspace, and the tab would change when the prompt repainted. +fn spec_of(workspace: &WorkspaceId) -> String { + format!( + "{}/{}@{}", + workspace.owner(), + workspace.repo(), + workspace.git_ref() + ) +} + /// Hand the workspace to the user: ssh in, and nothing else. /// /// One trip, and it is the session. This used to pay a `devpod ssh` of its own in @@ -2771,7 +2786,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { self.context.runner(), placement.workspace_id(), PassOccasion::TopUp, - self.container_title(placement.workspace_id()).as_deref(), + self.container_title().as_deref(), ) .map_err(|DevpodMissing| LaunchAborted::DevpodNotRun(NotRun::NotInstalled))?; return Ok(Launched::AlreadyRunning); @@ -2804,7 +2819,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { Placement::Known { .. } | Placement::Listed { .. } => Naming::Anonymous, }; let request = UpRequest::new(placement.source(), naming); - let title = self.container_title(placement.workspace_id()); + let title = self.container_title(); let outcome = workspace_up( self.context, self.host, @@ -2841,7 +2856,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { .with_ide(verb.ide()) .with_rebuild(verb.rebuild()) .with_devcontainer(devcontainer); - let title = self.container_title(placement.workspace_id()); + let title = self.container_title(); let outcome = workspace_up( self.context, self.host, @@ -2913,22 +2928,28 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { /// triple whose parts overflow 47 characters is refused before this. fn titled(&self, workspace_id: &str) -> String { match &self.resolved { - Some(workspace) => format!( - "{}/{}@{}", - workspace.owner(), - workspace.repo(), - workspace.git_ref() - ), + Some(workspace) => spec_of(workspace), None => workspace_id.to_owned(), } } /// The name a shell in this container should keep putting on the terminal, or - /// `None` when this host wants none. + /// `None` when there is none worth installing. /// - /// The same name [`Self::titled`] gives dl's own write, and the same switch — - /// `DEVLAUNCH_NO_TITLE` — decides both, so one variable governs the whole - /// feature rather than half of it. + /// The **spec** and only the spec, where [`Self::titled`] falls back to the id. + /// Two reasons, and the second is the load-bearing one. + /// + /// A container told to title after its own id is told nothing: the id is already + /// its hostname, so the stock prompt writes exactly that anyway. And the line is + /// deduped by a hash of its own text, so a name that varies for one workspace + /// does not replace the line — it adds another, and the last append is the one + /// every prompt then obeys. A workspace opened once as `blooop/devlaunch@main` + /// and once by its id would end up permanently titled after the id. Keying on the + /// spec alone makes the line a pure function of the triple, so a workspace has at + /// most one, ever. + /// + /// `DEVLAUNCH_NO_TITLE` still decides it, so one variable governs both halves of + /// the feature rather than half of it. /// /// `stderr_tty` is deliberately *not* consulted, where /// [`TerminalTitle::from_host`] does consult it. That flag answers "is there a @@ -2936,11 +2957,11 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { /// this process is about to emit and the wrong one for a line installed in a /// profile: `dl up` is a prewarm with its output redirected, and the /// interactive session that arrives later is the one the line is for. - fn container_title(&self, workspace_id: &str) -> Option { + fn container_title(&self) -> Option { if switched_on(self.host.no_title.as_deref()) { return None; } - Some(self.titled(workspace_id)) + self.resolved.as_ref().map(spec_of) } /// A session outcome, with the two never-ran arms lifted to [`LaunchAborted`]. @@ -5745,6 +5766,54 @@ mod tests { ); } + #[test] + fn only_a_spec_is_installed_in_the_container_so_a_name_cannot_pile_up() { + // One workspace, opened both ways: by spec, and later by the id it derived. + // The profile line is deduped by a hash of its own text, so a second, + // different name for one workspace would not replace the first -- it would + // append, and the last append is what every prompt obeys. A single launch by + // id would have renamed the tab back to the hash for good. + // + // So the pass is told the spec or nothing. Nothing is the honest answer for + // an id: it is already the container's hostname, so the stock prompt writes + // exactly that anyway and the line would buy nothing to lose. + let workspace = WorkspaceId::new("blooop", "devlaunch", "main").expect("a safe triple"); + let scene = Scene::new().with_running(&workspace.value()); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + let _ = launch.run("blooop/devlaunch@main", &LaunchVerb::Up, None); + } + { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + let _ = launch.run(&workspace.value(), &LaunchVerb::Up, None); + } + + assert_eq!( + parts.provision.titles(), + vec![Some("blooop/devlaunch@main".to_owned()), None] + ); + } + #[test] fn the_pass_is_told_to_have_the_container_keep_titling_after_the_spec() { // The other half of the title, and the half that lasts. dl's own escape is @@ -5814,7 +5883,8 @@ mod tests { // to. The container is still taught the name, because the session that // arrives later is the one it is for, and asking this pass to guess whether // one ever will is a question it cannot answer. - let scene = Scene::new().with_running("myws"); + let workspace = WorkspaceId::new("blooop", "devlaunch", "main").expect("a safe triple"); + let scene = Scene::new().with_running(&workspace.value()); assert!(!scene.host.stderr_tty, "the premise of this test"); let updater = SelfInvocation::new("dl"); let completion = scene.cache_dir().join("completion.json"); @@ -5830,11 +5900,23 @@ mod tests { &mut parts.chatter, &mut parts.said, ); - launch.run("myws", &LaunchVerb::Up, None) + launch.run("blooop/devlaunch@main", &LaunchVerb::Up, None) }; assert_eq!(launched, Ok(Launched::AlreadyRunning)); - assert_eq!(parts.provision.titles(), vec![Some("myws".to_owned())]); + assert_eq!( + parts.provision.titles(), + vec![Some("blooop/devlaunch@main".to_owned())] + ); + // And nothing was written to the terminal that is not there. + assert!( + !parts.said.iter().any(|notice| matches!( + notice, + LaunchNotice::TerminalTitle(TerminalTitle::Write(_)) + )), + "{:?}", + parts.said + ); } /// `dl up` is what a user types to fix a workspace, so it is the worst diff --git a/rust/dl/tests/launch.rs b/rust/dl/tests/launch.rs index e915d472..b84601cc 100644 --- a/rust/dl/tests/launch.rs +++ b/rust/dl/tests/launch.rs @@ -715,7 +715,6 @@ fn up_on_a_running_workspace_says_so_and_still_provisions_the_tools() { // pass that tops its tools up runs, because that is when it was found. &format!("Workspace {MAIN} is already running.") as &str, &format!("{MAIN}: the hostname setup stage did not report; it may not have run."), - &format!("{MAIN}: the title setup stage did not report; it may not have run."), &format!("{MAIN}: the zellij setup stage did not report; it may not have run."), ] ); From 57960aa7360e78b36a4b885a1ed22463d6527f47 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 22 Aug 2026 19:47:49 +0000 Subject: [PATCH 3/5] fix: a dash login shell printed the title escape instead of setting it `$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. --- README.md | 10 +++-- rust/devlaunch-core/src/flows/provision.rs | 52 +++++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 96cc52da..58687571 100644 --- a/README.md +++ b/README.md @@ -538,16 +538,18 @@ some trouble not to use. So the setup pass appends one line to the profile a login shell reads: ``` -case $- in *i*) PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac +case $- in *i*) [ -n "$BASH_VERSION" ] && PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac ``` Appended, and that is the whole mechanism: two escapes in one prompt are applied in order, so the last one sets the title. Nothing is rewritten — the visible `vscode@devlaunch-main-zovomobo:~/repo$` still says the hostname, and only the tab changes. (A `PROMPT_COMMAND` cannot do this job: bash runs that *before* it prints -`PS1`, so the stock escape would land afterwards and win.) Interactive shells only, -so a `dl -- cmd` one-shot, which reads the same profile through `bash -lc`, is -untouched. +`PS1`, so the stock escape would land afterwards and win.) Interactive bash only: +`bash -lc` reads the same profile on every `dl -- cmd` one-shot, and `\[`, `\e` +and `\a` mean nothing to dash — which is `/bin/sh`, and which reads `~/.profile` too +— so an unguarded line would print the escape at every prompt instead of acting on +it. It is written once — the line carries a content-hash comment the next launch recognises — and it rides the same round trip as the hostname stage, so it costs no diff --git a/rust/devlaunch-core/src/flows/provision.rs b/rust/devlaunch-core/src/flows/provision.rs index df256dec..c4496180 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -486,9 +486,15 @@ pub(crate) fn profile_prepend(line: &str, on_failure: Option<&str>) -> String { /// `PS1` an image or a dotfile built stays exactly as it was, prompt text included: /// the visible `user@host:path$` still names the hostname, and only the tab changes. /// -/// Interactive shells only (`case $- in *i*`), because a login shell that renders no -/// prompt has no use for one and `bash -lc` reads this file on every `dl -- -/// cmd`. +/// Interactive **bash** only, and both halves of that are load-bearing. +/// `case $- in *i*` keeps the edit out of the login shells that render no prompt, +/// which is every `dl -- cmd`, since `bash -lc` reads this file too. And +/// `$BASH_VERSION` keeps it out of the shells that would render it *literally*: +/// `$PROFILE` may be `~/.profile`, which any POSIX login shell reads, and `\[`, +/// `\e` and `\a` mean nothing to dash — `/bin/sh` on Debian and Ubuntu — so an +/// unguarded append puts the escape on screen at every prompt instead of in the +/// title. It is the same test Ubuntu's own `~/.profile` makes before it sources +/// `~/.bashrc`. /// /// *title* is interpolated as its own quoted word rather than into the double-quoted /// assignment, so a name holding a `$` or a backtick is text and not shell. A spec @@ -498,7 +504,7 @@ pub(crate) fn profile_prepend(line: &str, on_failure: Option<&str>) -> String { /// never validated. fn profile_title_line(title: &str) -> String { format!( - r#"case $- in *i*) PS1="$PS1\[\e]2;"{}"\a\]" ;; esac"#, + r#"case $- in *i*) [ -n "$BASH_VERSION" ] && PS1="$PS1\[\e]2;"{}"\a\]" ;; esac"#, quote(title) ) } @@ -4394,7 +4400,7 @@ fi // globs. A name that needs quotes gets them; the test below is that one. assert_eq!( line, - r#"case $- in *i*) PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac"# + r#"case $- in *i*) [ -n "$BASH_VERSION" ] && PS1="$PS1\[\e]2;"blooop/devlaunch@main"\a\]" ;; esac"# ); // `$PS1` first, so nothing an image or a dotfile put in the prompt is // rewritten -- only added to. @@ -4404,6 +4410,42 @@ fi assert!(line.starts_with("case $- in *i*)"), "{line}"); } + #[test] + fn a_shell_that_is_not_bash_leaves_its_prompt_alone() { + // `$PROFILE` is one of `~/.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 an unguarded append puts + // `\[\e]2;blooop/devlaunch@main\a\]` on screen at every prompt. That is worse + // than no title: it is a corrupted one. + // + // The guard is the same `$BASH_VERSION` test Ubuntu's own `~/.profile` uses + // before it sources `~/.bashrc`. Asserted as an implication so this reads the + // same whichever shell `sh` is on the machine running it. + let line = profile_title_line("blooop/devlaunch@main"); + let script = format!( + "PS1=untouched\n{line}\nprintf '%s\\n%s\\n' \"${{BASH_VERSION:+bash}}\" \"$PS1\"" + ); + + let out = std::process::Command::new("sh") + .args(["-i", "-c", &script]) + .output() + .expect("sh to run the line"); + let said = String::from_utf8_lossy(&out.stdout).to_string(); + let mut lines = said.lines(); + let is_bash = lines.next() == Some("bash"); + let ps1 = lines.next().expect("the prompt back"); + + if is_bash { + assert_ne!( + ps1, "untouched", + "bash should have been appended to: {said:?}" + ); + } else { + assert_eq!(ps1, "untouched", "a non-bash prompt was edited: {said:?}"); + } + } + #[test] fn a_title_holding_shell_metacharacters_is_text_and_not_shell() { // A spec cannot hold these -- `WorkspaceId` refused every character but word From 2ae964f01db44f9be92f740535e61db10fa49bf2 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 22 Aug 2026 19:50:15 +0000 Subject: [PATCH 4/5] fix: a ref's trailing newline reached the profile unfiltered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- rust/devlaunch-core/src/flows/launch.rs | 47 ++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 0f35a93f..1a8b61d3 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -2948,6 +2948,11 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { /// spec alone makes the line a pure function of the triple, so a workspace has at /// most one, ever. /// + /// Filtered by [`sanitize_title`], the same way the escape is, because the two + /// halves must not disagree about what a name may hold. `is_safe_name` accepts + /// one trailing newline, so `main\n` is a ref — and a newline reaching the + /// profile line lands inside the quoted word, splitting one `PS1` assignment + /// across two physical lines of a file every login sources. /// `DEVLAUNCH_NO_TITLE` still decides it, so one variable governs both halves of /// the feature rather than half of it. /// @@ -2961,7 +2966,10 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { if switched_on(self.host.no_title.as_deref()) { return None; } - self.resolved.as_ref().map(spec_of) + self.resolved + .as_ref() + .map(spec_of) + .and_then(|spec| sanitize_title(&spec)) } /// A session outcome, with the two never-ran arms lifted to [`LaunchAborted`]. @@ -5814,6 +5822,43 @@ mod tests { ); } + #[test] + fn a_name_holding_a_control_is_filtered_before_it_reaches_the_container() { + // `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. dl's own escape drops controls at the + // boundary it forms bytes at; the profile line is written by a different + // path, and an unfiltered newline lands in a file every login sources, inside + // the quoted word, splitting one PS1 assignment over two physical lines. + // + // One filtered name for both halves, so a title cannot be safe in the escape + // and not in the profile. + let workspace = WorkspaceId::new("blooop", "devlaunch", "main\n").expect("a safe triple"); + let scene = Scene::new().with_running(&workspace.value()); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("blooop/devlaunch@main\n", &LaunchVerb::Up, None) + }; + + assert_eq!(launched, Ok(Launched::AlreadyRunning)); + assert_eq!( + parts.provision.titles(), + vec![Some("blooop/devlaunch@main".to_owned())] + ); + } + #[test] fn the_pass_is_told_to_have_the_container_keep_titling_after_the_spec() { // The other half of the title, and the half that lasts. dl's own escape is From 4eae993a771b06e94e8ab1f7e68ec86eaf973bb6 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 22 Aug 2026 20:06:51 +0000 Subject: [PATCH 5/5] Merge origin/main (the selector's columns) into the title branch 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. --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index bf66ecfc..f86ffdab 100644 --- a/README.md +++ b/README.md @@ -1068,9 +1068,10 @@ the 64-byte hostname limit on its own, but tools that stack their own prefixes o container name have about 17 characters to work with, so a tool that wants more is the one that has to shorten. -The id is *not* what you read on a tab. That shows `owner/repo@branch` — see [Naming -the terminal after the workspace](#naming-the-terminal-after-the-workspace). The id -addresses the workspace; the spec names it. +The id is *not* what you read. A tab shows `owner/repo@branch` — see [Naming the +terminal after the workspace](#naming-the-terminal-after-the-workspace) — and the +selector shows `owner | repo | branch`. The id addresses the workspace; those name +it. Branch names must be safe as both git refs and directory names — a name with a space or a leading dash is rejected rather than quietly rewritten.