feat(trusty-mpm): add n [name] picker command for launching a named session - #4978
Open
mac-duetto wants to merge 2 commits into
Open
feat(trusty-mpm): add n [name] picker command for launching a named session#4978mac-duetto wants to merge 2 commits into
n [name] picker command for launching a named session#4978mac-duetto wants to merge 2 commits into
Conversation
… session The picker's only way to create a session was to type the next free launch slot — a number that shifts as sessions come and go, and that carries no name, so a project with several workstreams produced `tm-trusty-tools-01…05` with nothing to tell them apart. Every other spawn surface (`tm session new --name-hint`, the MCP `tm_new_session` tool, the daemon's `SpawnRequest`) already accepts a name. `n` launches a new session auto-named, the same outcome as the integer choice; `n <name>` launches one whose name derives from the argument. The prefix is stripped like `d<N>` and `r<N> <name>`, so `nauth` ≡ `n auth`, and a whitespace-only remainder launches unnamed rather than sending an empty hint. The name rides through raw as `name_hint` — `trusty_common::session_naming::resolve_session_name` already lowercases, collapses separators, truncates and falls back to `local`, so no validation is duplicated CLI-side, and the daemon-allocated `NN` serial makes collisions impossible. Additive: the `[N] launch new session` entry stays, and `launch_new_session_and_attach` omits the `name_hint` key entirely for `None`, leaving the unnamed request's wire shape byte-identical to what `session::start`'s tests pin. Closes #4965 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
…side The `n [name]` command shipped as `strip_prefix(['n','N'])`, which is unbounded: EVERY n-initial word became a spawn with no confirmation. The menu line itself reads "launch new session", so an operator typing `new` got a real cloned, provisioned, attached session named `tm-ew-01`; `n` is also already this tool's own "no" answer in the `[y/N]` delete confirm. The `d2`/`r1` no-space precedent is safe only because their remainder must still parse as a live slot number — `n` had no such gate. The grammar is now an exact token plus a required separator: `n`/`N` alone launches unnamed, `n <name>` launches named, and `new`, `no`, `nn`, `n1` and `nauth` are all unrecognised. The no-space `nauth` form is dropped. The name is also kebab-cased in the picker now, before it is sent. Forwarding it raw had two bad outcomes. The daemon's `resolve_session_name` routes a `name_hint` through `leaf_slug_from_dir`, whose `Path::file_name()` step drops everything before the last `/` — so `n feature/auth` and `n hotfix/auth` both landed as `tm-auth-NN`, defeating the point of naming them. And a name that sanitizes to nothing (`n !!!`) fell through to the daemon's `local` fallback leaf, producing `tm-local-NN`: a name in a namespace shared with every other project on the daemon, LESS identifiable than the default the operator was trying to improve on. That case now refuses instead of spawning. Sanitizing CLI-side reuses the canonical algorithm rather than duplicating it: `trusty_common::session_naming::leaf_slug_from_hint` is the private `sanitize_slug` under the same `MAX_FOLDER_LEN` cap the hint path already used, minus the `file_name()` step. Its output is always a single kebab-case component within the cap, so the daemon's own pass over it is a provable no-op — pinned by a fixed-point test. `leaf_slug_from_dir` now delegates to it, so the two cannot drift. Also: the `n` legend row spells out `tm-<name>-NN`, since the adjacent `r<N> <new-name>` row takes a verbatim full session name and the two `<name>` arguments otherwise read as interchangeable; the legend moved to `session_picker_render` (which exists for exactly this, and keeps `session_picker.rs` under the 500-SLOC cap) as a pure function, so both menus are column-aligned — the populated one was ragged — and the wording is testable; and `launch_new_session_and_attach`'s `name_hint` plumbing has wire tests of its own, where it previously had none. Refs #4965 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4965
Adds an
ncommand to the interactive session picker (baretm/tm ls) so a new session can be launched with a meaningful name instead of the daemon-derivedtm-<repo>-NN.Grammar
n/N(exact)n <name>/N <name>(separator required)new,no,nn,n1,nauthUnrecognised— unchangedThe integer "launch new session" entry is unchanged. This is purely additive.
Two commits
c8ccb317— the feature as originally specified in #4965.7fdbf4d7— fixes from an adversarial review that returned WARN. The first commit usedstrip_prefix(['n','N']), which consumed any n-prefixed input and returned a decision that unconditionally clones a repo, creates a worktree, starts tmux, and attaches — with no confirm step. The menu line literally reads launch new session, so an operator typingnewwould get a real session namedtm-ew-01. The delete confirm is spelled[y/N], sonalready means "no" inside this tool.The
d<N>/r<N> <name>no-space precedent is only safe because their remainder must resolve to a live slot —d9on a 3-session list isUnrecognised.nhad no such gate. It is now an exact-match token plus a required separator.Sanitization moved CLI-side
Previously the raw string was forwarded and the daemon sanitized it. Two bad outcomes:
n feature/authandn hotfix/authboth producedtm-auth-NN— the daemon'sresolve_session_nameroutes the hint throughleaf_slug_from_dir, whosePath::file_name()discards everything before the last/. That defeats the point of the feature.n !!!sanitized to empty and fell back totm-local-NN, a name in a leaf namespace shared with every other project on the daemon and less identifiable than the default.The name is now kebab-cased in the picker before it is sent. Rather than exposing the private
sanitize_slug(which would also mean exposingMAX_FOLDER_LEN, inviting drift), this adds one public item —leaf_slug_from_hintintrusty-common— and refactorsleaf_slug_from_dirto delegate to it. No second kebab-caser.leaf_slug_from_hint_is_a_fixed_point_for_its_own_outputproves the daemon's own pass is a no-op rather than asserting it.A name that sanitizes to empty is now rejected with a message instead of silently spawning. When the sanitized name differs from what was typed, one line is printed before launch:
tm: naming it 'my-auth-fix' (from 'My Auth Fix!').Other changes
session_picker.rsintosession_picker_render.rsascommand_legend(Option<u32>) -> Vec<String>. Inline, the file hit 515 SLOC against the repo's 500 cap andcheck_line_cap.shrefused it. This also made the column alignment and thetm-<name>-NNwording assertable — 4 tests.nlegend line is self-documenting (launch new session as tm-<name>-NN), since the adjacentr<N> <name>takes a verbatim full session name whilen <name>takes a leaf.launch_new_session_and_attach'sname_hintplumbing had zero coverage. Two wire tests now assert the key is present onSomeand absent (not null) onNone.Test ladder — rung 4
trusty-commongained public API, so this is a cross-crate change.Pre-existing failures on this base — not from this change
Each was verified against the stashed base before being set aside. Nothing was
#[ignore]d or narrowed to make a gate green.ensure_managed_config_dir_emits_the_frozen_skill_warning— fails ~1 run in 4, passes in isolation.#[serial]only serializes against other#[serial]tests, and installing a subscriber perturbs the process-global interest cache. Failed 1-of-4 on base.compose_session_instructions_display_matches_live_prompt[_with_override]—tmandtrusty-mpmare duplicate bin targets over the samemain.rs, socargo test -p trusty-mpmruns the test twice concurrently against the same on-disk project state. Failed 1-of-3 on base; green when the targets run separately.cargo check --workspace—trusty-code-guiandtrusty-mpm-guipanic intauri::generate_context!becauseui/distis not built in this worktree. Identical exit 101 on base.CI may be flaky on this PR for those reasons, unrelated to the change.
🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools