diff --git a/README.md b/README.md index bfd0edd2..f86ffdab 100644 --- a/README.md +++ b/README.md @@ -535,11 +535,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 @@ -552,25 +552,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 @@ -578,26 +582,59 @@ 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*) [ -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 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 +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; +`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: @@ -611,8 +648,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. ## Agent state in a herdr session @@ -1031,6 +1068,11 @@ 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. 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. diff --git a/rust/devlaunch-core/public-api.txt b/rust/devlaunch-core/public-api.txt index 85827fec..434c3217 100644 --- a/rust/devlaunch-core/public-api.txt +++ b/rust/devlaunch-core/public-api.txt @@ -1115,7 +1115,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 @@ -2047,7 +2047,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 7dd371d1..fe15ec4b 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -1017,6 +1017,7 @@ pub trait Provision { runner: &dyn Runner, workspace_id: &str, occasion: PassOccasion, + title: Option<&str>, ) -> Result<(), DevpodMissing>; } @@ -1036,6 +1037,7 @@ impl Provision for NoProvisioning { _runner: &dyn Runner, _workspace_id: &str, _occasion: PassOccasion, + _title: Option<&str>, ) -> Result<(), DevpodMissing> { Ok(()) } @@ -1096,10 +1098,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) }) } @@ -1109,6 +1112,7 @@ fn up_under_stage( token: &HostToken, provision: &dyn Provision, request: &UpRequest<'_>, + title: Option<&str>, notices: &mut dyn Notices, ) -> Result { let options = context_options( @@ -1163,7 +1167,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); } @@ -1206,7 +1210,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); @@ -1796,23 +1800,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. @@ -1822,12 +1835,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, } @@ -1844,21 +1860,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 @@ -1871,6 +1888,21 @@ fn sanitize_title(workspace_id: &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 @@ -1894,6 +1926,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, @@ -1903,10 +1936,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), @@ -2485,6 +2519,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> { @@ -2506,6 +2552,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { forward, token: HostToken::new(), notices, + resolved: None, } } @@ -2601,6 +2648,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) @@ -2774,6 +2826,7 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { self.context.runner(), placement.workspace_id(), PassOccasion::TopUp, + self.container_title().as_deref(), ) .map_err(|DevpodMissing| LaunchAborted::DevpodNotRun(NotRun::NotInstalled))?; return Ok(Launched::AlreadyRunning); @@ -2806,12 +2859,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(); let outcome = workspace_up( self.context, self.host, &self.token, self.provision, &request, + title.as_deref(), &mut *self.notices, ) .map_err(LaunchAborted::DevpodNotRun)?; @@ -2841,12 +2896,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(); let outcome = workspace_up( self.context, self.host, &self.token, self.provision, &request, + title.as_deref(), &mut *self.notices, ) .map_err(LaunchAborted::DevpodNotRun)?; @@ -2871,10 +2928,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, @@ -2887,6 +2946,72 @@ 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) => spec_of(workspace), + None => workspace_id.to_owned(), + } + } + + /// The name a shell in this container should keep putting on the terminal, or + /// `None` when there is none worth installing. + /// + /// 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. + /// + /// 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. + /// + /// `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) -> Option { + if switched_on(self.host.no_title.as_deref()) { + return None; + } + 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`]. fn session( &mut self, @@ -3105,23 +3230,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) @@ -3131,14 +3257,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() } } @@ -3149,11 +3283,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); } @@ -3164,13 +3299,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, @@ -3179,7 +3317,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`. @@ -3366,6 +3512,7 @@ mod tests { &token, provision, request, + None, &mut inner, ); (outcome, inner) @@ -3615,6 +3762,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -3652,6 +3800,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3684,6 +3833,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3714,6 +3864,7 @@ mod tests { &token, &provision, &request, + None, &mut no_notices(), ); @@ -3745,6 +3896,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -4610,6 +4762,7 @@ mod tests { &token, &NoProvisioning, &request, + None, &mut no_notices(), ); @@ -4647,6 +4800,7 @@ mod tests { workspace_id: "myws", }, ), + None, &mut no_notices(), ); @@ -4771,6 +4925,7 @@ mod tests { workspace_id: "myws", }, ), + None, &mut notices, ); let _ = attaching(&logged_in, &token, "myws", None, &mut notices); @@ -5266,6 +5421,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"); @@ -5567,6 +5825,196 @@ 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 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 + // 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 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"); + 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", &LaunchVerb::Up, None) + }; + + assert_eq!(launched, Ok(Launched::AlreadyRunning)); + 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 /// 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 @@ -6155,6 +6603,7 @@ mod tests { workspace_id: "brand-new", }, ), + None, &mut no_notices(), ) }); @@ -6220,6 +6669,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 5c671b31..8b3df8fe 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -204,6 +204,10 @@ pub(crate) const ZELLIJ_STAGE: StageName = StageName::new("zellij"); /// socket mount, and the launch is unharmed by any of those being absent. pub(crate) const HERDR_STAGE: StageName = StageName::new("herdr"); +/// The stage that teaches the shell to keep naming the terminal after this +/// workspace. Rides the same trip as the three above it. +pub(crate) const TITLE_STAGE: StageName = StageName::new("title"); + // =========================================================================== // quoting // =========================================================================== @@ -476,6 +480,46 @@ 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 **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 +/// 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*) [ -n "$BASH_VERSION" ] && 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()); @@ -933,11 +977,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`, or a spec +/// that resolved no triple, both decided by the caller. Last of the four because it +/// is the one that is not a switch, and 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, herdr: HerdrSwitch, + title: Option<&str>, ) -> Vec { let mut stages = vec![ // The hostname appears in the bash prompt (user@hostname:path$), which is @@ -985,6 +1043,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 every edit finds the others' 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(), + ); + } stages } @@ -1708,28 +1792,37 @@ 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, @@ -1748,7 +1841,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), }; @@ -1860,9 +1953,16 @@ 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, switches.herdr); + let stages = setup_stages( + workspace, + switches.tools, + switches.zellij, + switches.herdr, + title, + ); let call = Call::new([ "ssh", workspace, @@ -2471,6 +2571,7 @@ fi "myws", PassOccasion::AfterUp, switches, + None, Some(host), None, &mut events, @@ -2844,6 +2945,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ); for (payload, flag, script) in [ ( @@ -2876,6 +2978,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ); let command = &stages[1].command; let words = shlex::split(command).expect("a stage a shell can read"); @@ -2924,6 +3027,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); assert_eq!( with_zellij, @@ -2934,6 +3038,7 @@ fi ToolsSwitch::Skip, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); assert_eq!( opted_out, @@ -3700,6 +3805,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); assert!(script.contains(&probe_script())); } @@ -3714,6 +3820,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ); let script = setup_script(&stages); let probe_at = script.find(&probe_script()).expect("the probe is in there"); @@ -3737,6 +3844,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); assert!(!script.contains("set -e")); } @@ -3750,6 +3858,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); assert!(script.contains(&format!("hostname {}", quote(name)))); assert!(!script.contains("hostname myws;")); @@ -3787,6 +3896,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Skip, + None, )); let ran = bash_with( &script, @@ -3815,6 +3925,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ), ) .into_iter() @@ -3879,6 +3990,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ) .into_iter() .filter(|stage| stage.name == HOSTNAME_STAGE) @@ -3992,7 +4104,8 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Install, - HerdrSwitch::Report + HerdrSwitch::Report, + None, )) ); } @@ -4119,6 +4232,7 @@ fi Switches::INSTALLING, None, None, + None, &mut events, ); @@ -4241,7 +4355,8 @@ fi "myws", ToolsSwitch::Skip, ZellijSwitch::Install, - HerdrSwitch::Skip + HerdrSwitch::Skip, + None, )) ); assert!(words[2].contains("sudo hostname myws")); @@ -4289,7 +4404,7 @@ fi #[test] fn the_herdr_stage_is_carried_only_when_both_switches_ask_for_it() { let names = |tools, herdr| -> Vec { - setup_stages("myws", tools, ZellijSwitch::Skip, herdr) + setup_stages("myws", tools, ZellijSwitch::Skip, herdr, None) .iter() .map(|stage| stage.name) .collect() @@ -4316,6 +4431,223 @@ 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, + HerdrSwitch::Skip, + Some("blooop/devlaunch@main"), + ) + .iter() + .map(|stage| stage.name) + .collect(); + let unnamed: Vec = setup_stages( + "myws", + ToolsSwitch::Install, + ZellijSwitch::Install, + HerdrSwitch::Skip, + 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, + HerdrSwitch::Skip, + 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"); + + assert_eq!( + line, + 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. + 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, + HerdrSwitch::Skip, + 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, + HerdrSwitch::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 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 the_zellij_opt_out_drops_only_the_zellij_stage() { // The whole reason for a second variable: a host that wants no zellij @@ -4327,6 +4659,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Skip, HerdrSwitch::Skip, + None, ) .iter() .map(|stage| stage.name) @@ -4364,6 +4697,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Skip, HerdrSwitch::Skip, + None, )); for tools in [ToolsSwitch::Install, ToolsSwitch::Skip] { assert_eq!( @@ -4371,7 +4705,8 @@ fi "myws", tools, ZellijSwitch::Skip, - HerdrSwitch::Skip + HerdrSwitch::Skip, + None, )), without, "{tools:?}" @@ -4382,7 +4717,8 @@ fi "myws", ToolsSwitch::Skip, ZellijSwitch::Install, - HerdrSwitch::Skip + HerdrSwitch::Skip, + None, )), without ); @@ -4391,7 +4727,8 @@ fi "myws", ToolsSwitch::Install, ZellijSwitch::Install, - HerdrSwitch::Skip + HerdrSwitch::Skip, + None, )), without, "the stage is there when nothing asked for it to go" @@ -4516,6 +4853,7 @@ fi "myws", occasion, Switches::INSTALLING, + None, Some(¬hing_to_lend()), Some(verdicts), &mut Vec::new(), @@ -4636,6 +4974,7 @@ fi "myws", PassOccasion::TopUp, Switches::INSTALLING, + None, Some(¬hing_to_lend()), Some(&verdicts), &mut Vec::new(), @@ -4721,6 +5060,7 @@ fi "myws", occasion, Switches::INSTALLING, + None, Some(¬hing_to_lend()), None, &mut Vec::new(), @@ -4833,6 +5173,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ) .iter() .map(|stage| (stage.name, stage.failure_level)) @@ -5306,6 +5647,7 @@ fi switches.tools, switches.zellij, switches.herdr, + None, )), &[ ("HOME", &home.to_string_lossy()), @@ -5342,6 +5684,7 @@ fi ToolsSwitch::Install, ZellijSwitch::Install, HerdrSwitch::Report, + None, ) .iter() .map(|stage| stage.name) @@ -5476,6 +5819,7 @@ fi ToolsSwitch::Skip, ZellijSwitch::Install, HerdrSwitch::Skip, + None, ) .iter() .map(|stage| stage.name) diff --git a/rust/dl/src/launch.rs b/rust/dl/src/launch.rs index 43c90e23..79d88ae3 100644 --- a/rust/dl/src/launch.rs +++ b/rust/dl/src/launch.rs @@ -151,6 +151,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: @@ -161,6 +162,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 278e42a1..af911b1e 100644 --- a/rust/dl/tests/launch.rs +++ b/rust/dl/tests/launch.rs @@ -608,6 +608,7 @@ fn a_cold_triple_prepares_a_clone_creates_the_workspace_and_attaches() { ), &format!("{COLD}: the hostname 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!("{COLD}: the title setup stage did not report; it may not have run."), &format!("SSH command: devpod ssh {COLD}"), ] );