Conversation
dl writes the workspace id as an OSC 2 title just before the session takes the terminal, and aid starts claude with CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 so that name is the one that stands. DEVLAUNCH_NO_TITLE=1 turns it off. One escape sequence to stderr rather than a multiplexer command, so zellij, tmux, byobu and a bare terminal are all served without dl detecting which it is in. stderr because stdout is parsed by the completion machinery and by wf, and the tty guard is on the same stream it writes to.
`*ch != '\u{7f}'` was dead: char::is_control() is the Cc category, which
contains U+007F, verified by compiling it rather than by reading the docs.
No behaviour changes, so there is no failing test to show; the DEL and
U+009C cases the clause was nominally about are now asserted instead, so the
narrower filter is covered rather than believed.
…where Both were wrong about *why* the code is safe, which is the kind of comment that survives the thing it described. The length note credited WorkspaceId's 47-character cap, but only the triple arm derives its id there; a bare name and a path leaf arrive as the raw spec and a directory basename, which this crate never shortens. What bounds those is devpod refusing to create or report a name over 48 characters. The sanitizer's note claimed the title is written before the refusal that would catch an escaped spec. It is not: an unknown name is refused in stage one, before the handover, so the injection it described is not reachable. The filter is defence at the boundary the bytes are formed at, which is worth having and worth not overselling. Comments only, so there is no test that could go red on either.
Same defect as the code comments one commit back, in the two places a reader would actually meet it: the 47-character derivation bounds only the owner/repo@branch arm, and devpod's 48-character name limit is what bounds the rest. Docs only.
Reviewer's GuideImplements workspace-aware terminal titling for devlaunch sessions, including an OSC 2-based title written to stderr before handover, a host/env-driven opt-out switch, sanitization and tty guards, and adjusts aid’s Claude agent to stop competing terminal titles, with tests and docs updated accordingly. Sequence diagram for workspace terminal titling at handoversequenceDiagram
participant User
participant dl
participant Saying
participant Terminal
participant Session
User->>dl: launch workspace
dl->>dl: TerminalTitle::from_host(host, workspace_id)
dl->>Saying: say(TerminalTitle)
alt stderr is a terminal and DEVLAUNCH_NO_TITLE is not enabled
Saying->>Terminal: write OSC 2 to stderr
Saying->>Terminal: flush()
Terminal-->>Terminal: update pane or window title
else disabled or stderr is not a terminal
Saying-->>Terminal: no title bytes
end
dl->>Session: hand over terminal
Sequence diagram for aid disabling Claude terminal titlessequenceDiagram
participant aid
participant Bash
participant Claude
participant Terminal
aid->>aid: build_agent_command()
aid->>Bash: run CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 claude
Bash->>Claude: start Claude
Claude-->>Terminal: render session without competing title updates
Note over Terminal: workspace title written by dl remains visible
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
# Conflicts: # CHANGELOG.md
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
TerminalTitle and the LaunchNotice variant carrying it are public surface, so the committed snapshot no longer matched and the job failed as designed. The snapshot is hand-written rather than regenerated: cargo-public-api needs a nightly toolchain for its rustdoc-JSON backend and this container has only the stable pin, so the job itself is what confirms the entries are right.
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Retrospective wf-review. #358 and #371 are two iterations on one surface ~1h20m
apart, so they were reviewed together against current main (a5f7ed8) and the
consolidated report is on #371:
#371 (review)
Most of what #358 introduced was superseded or corrected by #371 and is not restated
here. Three things attributable to #358 still stand on main:
-
The escape-safety design holds.
sanitize_title(rust/devlaunch-core/src/flows/launch.rs:1837)
drops the wholeCccategory, so ESC, BEL, DEL and the 8-bit ST cannot terminate the
OSC early; the stderr-only +stderr_ttyguard keeps the bytes out of piped output;
andLaunchNotice::TerminalTitlecorrectly renders to no line
(rust/dl/src/render.rs:1676) so nothing leaks into a collected report. Mutating the
filter to a no-op reddens two tests, and mutating\x1b]2;to\x1b]0;reddens five
unit tests plus the real-pty test. This was the likeliest place for a real defect and
it is clean. -
#358-specific, non-blocking: the title is never restored.
attach_workspace
(launch.rs:1903) says the notice unconditionally, ahead ofworkspace_ssh, and
nothing writes a title on the way out. A refused session leaves the host terminal
named after a workspace it never entered, and every one-shotdl <ws> -- cmd— which
is everyaidlaunch — leaves the tab reading the workspace name afterdlexits.
Fix: bracket the session with XTWINOPS,CSI 22;2tbefore the OSC andCSI 23;2t
after; an empty OSC 2 would blank the name rather than restore it, which the code
already correctly refuses to do. -
#358-specific, non-blocking:
TerminalTitle::Write(String)(launch.rs:1792) is
a public tuple variant holding raw bytes thatdl's sink writes verbatim to stderr
(render.rs:1711). It is in the frozen public API
(devlaunch-core/public-api.rest.txt:983-995), so the sanitising invariant is not
enforced by the type. A private field plus a constructor would make it unskippable.
aid's CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 (rust/aid/src/rewrite.rs:73) survives
intact and interacts correctly with #371's PS1 half: a bash -lc one-shot never
matches case $- in *i*, so the two halves cannot contend.
Verdict: Comment. No blocking finding is attributable to #358 on current main;
the two blocking findings on this surface belong to #371 and are enumerated there.
What
dlwrites the workspace id to the terminal as an OSC 2 title just before thesession takes over, and
aidstarts claude withCLAUDE_CODE_DISABLE_TERMINAL_TITLE=1so that name is the one that stands.DEVLAUNCH_NO_TITLE=1turns the whole thing off.In zellij that shows up as
<session> | devlaunch-main-zovomobo; in a bare kittyor xterm it is the window title; in tmux it is the pane title.
Why an escape sequence rather than
zellij action rename-tabBecause the escape reaches every multiplexer at once and a command reaches one.
dl writes to the stream it was handed and whoever owns that pty parses it, so
zellij, tmux, byobu-on-tmux and a plain terminal are all served without dl
detecting which it is inside.
Two things were measured rather than assumed, against zellij 0.45:
rename-tabor a plugin moves it. So the pane title is the only lever thatreaches the outer tab bar, which is what this uses.
and zellij publishes it outward as
<session> | <pane title>.Why claude's own title has to be turned off
A terminal title has one value and the last writer sets it. claude writes one
continuously from its own read of what the session is doing, so the two are not
two signals but one contest that claude wins within a second. What claude is
doing is already on screen inside the pane; which workspace the pane is is not
otherwise anywhere.
Scoped to
aid, which is what decided to start claude — adl <ws> -- claude ...somebody typed themselves is their command and not aid's to rewrite. It needed no
new plumbing: aid's agent table is already an env prefix on a payload that runs
under
bash -lc, so this is one more entry beside theIS_SANDBOX=1alreadythere, and no host variable is forwarded.
Two decisions worth review
wf, so escapes there could corrupt what they read. The tty guard is on stderrfor the same reason, which also means
dl <ws> -- make test > logkeeps itstitle.
does not — a bare
owner/repostill has its branch unresolved at the handover,and
./some/diris not a spec at all — and it is already the containerhostname, so dl's title and the
user@hosta prompt paints over it are the samestring. The cost is that it drops the owner and spends columns on the hash tail.
Tests
the 8-bit String Terminator).
refresh — after the session starts this process may not print for hours.
collected report.
aid/tests/interactive.rs): the escape really comes out ofthe shipped binary, and
DEVLAUNCH_NO_TITLE=1really silences it. The off-testwas mutation-checked by breaking the guard — it fails, so it is not vacuous.
Self-review
Three follow-up commits, all from the adversarial pass: one dead clause in the
control filter (
char::is_controlalready covers U+007F, verified by compilingit), and two places where my own comments credited the wrong reason the title
stays short — the 47-character derivation bounds only the
owner/repo@brancharm;devpod's 48-character name limit bounds the rest.
Attacked and found solid: the background boot in
aid(its stderr is a log file,so no stray title), the notice-collecting
say_launchpath (only carries targetresolution, never a handover), and
Hostconstruction (one production call site,so the feature cannot be silently off).
🤖 Generated with Claude Code
Summary by Sourcery
Identify each launched workspace in terminal and multiplexer titles while keeping the behavior safe, opt-outable, and stable during Claude sessions.
New Features:
aid.Enhancements:
Build:
Documentation:
DEVLAUNCH_NO_TITLEsetting.Tests:
Release 0.7.0 is in this PR
rust/Cargo.tomlgoes 0.6.1 → 0.7.0 (minor: this adds a feature), with theCargo.lockrefresh and the## [0.7.0]changelog headingAuto-publishneedsto build release notes from. Merging this to
mainpublishes 0.7.0 to PyPIand conda and tags
v0.7.0— the workflow triggers on a version change between acommit and its parent, so there is nothing further to run by hand.
The bump is its own commit (
Release 0.7.0), so dropping it and cutting therelease separately is a one-commit revert if you would rather keep this PR to the
feature.
origin/mainwas merged in to clear aCHANGELOG.mdconflict — main released0.6.1 while this was in progress. Both entries survive; nothing was overwritten.