feat(run): --shell=ADDRESS names which target gets the terminal

[raphaelvigee]

The defect

--shell was declared with require_equals = true + default_missing_value = "", so heph run --shell=//pkg:a //pkg:... parsed fine. But self.args.shell.is_some() was the only consumer, and the captured String was never read anywhere in the tree. The value was accepted and thrown away — against this repo's fail-or-fix rule.

#225 recorded it as pre-existing and out of scope, and sharpened it: the ShellNeedsSingleTarget message it added tells the user to name the member they want, which is precisely the form that was being ignored.

What the value now means

form	behavior
--shell	unchanged — #225's inference: the single target the run names. A group of exactly one member is that member (carve-out intact); a group of two or more, or a multi-target selection, is refused.
--shell=ADDRESS	ADDRESS is entered instead of run. Every other target in the run builds with no terminal.
--shell=ADDRESS naming a group	refused where the members are in hand — a transparent target never executes, so there is no sandbox to enter.
--shell=ADDRESS that no target has	warned about before the run, in one memoized spec lookup; the build proceeds.
--shell=ADDRESS that exists but this run never reached	warned about after the run, on the success path only; the build proceeds.
--shell / --shell=ADDRESS with no terminal	refused, never downgraded to a plain build.

A bad --shell= value warns; it does not fail the build. --shell is an interactive convenience, and a target that turns out not to be in the run is no reason to throw away a build that did exactly what was asked — its outputs are real and cached either way.

It is never silent, though: the user asked for a shell and has to hear that they are not getting one, so both warnings name the address, state the consequence outright, and give the command that finds the right address. The split is about when: a typo is decidable up front, so it is reported while the build is still running rather than after waiting one out; graph membership needs the walk, which is the run. A single shell_settled flag covers both, so the same address is never reported twice.

The two refusals are separate cases and stay refusals: --shell on something that is not a single target, and --shell=<a group> — a transparent target never executes, so there is no sandbox to enter.

ADDRESS may be any address the run resolves — a selected target, a member a group inlines, or a dependency of either. heph run --shell=//pkg:lib //pkg:app builds app's graph and drops you into lib's sandbox. That is the workflow this mainly unlocks, and it is why the mechanism below matters.

heph run --shell=//pkg:a with no positional now runs that target rather than answering "missing TARGET_ADDRESS/LABEL" to a command line that named one.

Why the shell target lives on RequestState, not ResultOptions

Two reasons, both reducing to a ResultOptions does not reach every frame:

• It travels down the selection and group-inlining legs only. Dependency resolution resets it (ResultOptions::default() in inputs_result_exec) and meta has no opts in scope at all. An addr named with --shell= would be unreachable the moment it appeared as a dependency — the same silent discard this PR exists to delete, in a subtler form.
• mem_locked_result is keyed by Addr alone and bakes the winning caller's shell/interactive into the shared cell. Were "does this frame shell?" a function of the frame's options, an addr reached both as a selected target and as some sibling's dependency would resolve differently depending on which frame won that race. //pkg:helper named on the CLI and also a dep of //pkg:app is the ordinary case, and meta resolves it with OutputMatcher::None while inputs_result_exec uses Exact(names) — two mem_result cells, one mem_locked_result cell. Read from the request it is a pure function of the addr, so every cell computes the same answer.

ShellTarget is set once, before resolution starts; Engine::new_state_full's signature is unchanged.

The #225 invariant holds by construction

At most one live terminal wrapper (TtyReader on fd 0) per request. Exactly one address matches, and inner_result_addr — the tree's only ExecuteOptions construction — hands every other frame (shell: false, interactive: None). probe.max_live == 1 is asserted on both the group and selection legs.

classify_failure now reads "did this frame actually get the terminal" from the same place rather than from opts. Believing opts there says a silent target streamed, which suppresses the process-log tail of a failing one and renders an empty failure box — the regression #225 fixed for group members, re-entered through the other door.

Diagnosability

Every try: line is runnable verbatim. A template carrying try: is a command an agent will execute for real, so the placeholder moved out of it:

--shell needs exactly one target; //pkg:g is a group with 2 members
  members: //pkg:a, //pkg:b
  name the member to shell into — //pkg:g still builds as asked
  try: heph run --shell=//pkg:a //pkg:g

--shell needs exactly one target; //pkg selects many
  name the one to shell into: heph run --shell=<address> //pkg
  try: heph query //pkg

The two refusals above fail the run. The two bad-value cases only warn, and the build carries on:

WARN --shell names no known target, so no shell will be opened; the build
     continues — check the address with `heph query //pkg` shell=//pkg:typo

WARN --shell target was not part of this run, so no shell was opened; nothing
     in the run resolved that address — list what it does contain with
     `heph query <selection>` shell=//pkg:b request=//pkg:a

Each names the address, states the consequence in the same breath ("no shell will be opened" / "was opened"), and ends on a command that finds the right address — because the failure mode a warning permits is a typo quietly costing the user their shell.

--shell's long help leads with three examples, including the dependency form.

Compatibility

BREAKING — CLI behavior, not parsing. The clap grammar for --shell is byte-for-byte unchanged.

--shell=X previously parsed but silently discarded X, behaving identically to bare --shell. As of this commit, X is meaningful: it names the target that gets the terminal. Any script or agent that was passing --shell=X and relying on the old infer-or-refuse behavior will now get a shell in X instead. This is a deliberate behavior change on an already-accepted input, not a new parse error — there is no CLI schema version to bump, and no prior release shipped a documented contract for what --shell=X did, so this lands as a stated break rather than a migration.

compatibility wrote that sentence when a bad X still failed the run; the exit-code half has since softened — an X that is not part of the run now warns and the build succeeds — but the change of meaning on an accepted input, which is the part that matters to anyone already passing --shell=X, is unchanged.

The refusals that remain (not a single target, --shell=<group>, no terminal) exit 1, the project's single command-failure code. Nothing bakes the flag's arity or the message text: gen-docs and completions are generated live from the clap tree, and the changed Display text is asserted only in crates/engine's own unit tests. ResultOptions is unchanged; ShellTarget and the two RequestState methods are purely additive.

Verification

22 tests (17 --shell, 5 CLI arg-layer); the full --shell + terminal + group suite is 28 and passes. cargo fmt --check and cargo clippy --all-targets --locked -- -D warnings clean from the workspace root.

The three warning tests assert on the warning itself, not merely on "the build survived" — a warning nobody emits is the same as no warning. tracing delivers rather than returns it, and it fires on tokio worker threads where a thread-local subscriber is never consulted, so a capturing subscriber is installed once globally and each test names an address no other test uses. tracing-subscriber becomes an engine dev-dependency for this; it is already the workspace's subscriber (the binary uses it), so it is not a second copy of anything — the lockfile diff is one line.

Mutation-verified red — 12 mutations, each killing its test:

mutation	test that fails
chokepoint never matches the named addr	shell_target_picks_which_group_member_is_entered, ..._which_selected_target_is_entered, shell_target_reaches_a_dependency_of_the_run
batch check ignores errors	a_shell_refusal_survives_a_batch_that_did_not_fail_fast
classify_failure reads opts.interactive again	a_failing_sibling_of_the_shell_target_keeps_its_log_tail
pre-flight disabled	shell_target_that_does_not_exist_warns_before_the_run
pre-flight does not settle_shell (so both sites warn)	shell_target_that_does_not_exist_warns_before_the_run — "warned exactly once", 2 vs 1
end-of-run warning disabled	shell_target_outside_the_run_warns_and_the_build_still_succeeds, ..._outside_a_selection_warns_once
--shell=<group> refusal disabled	shell_target_naming_a_group_names_its_members_instead
addressed no-terminal guard dropped	shell_target_without_a_terminal_is_refused
require_equals dropped	bare_shell_names_no_target
num_args = 0..=0	shell_captures_its_value
parsed value discarded again (the original defect)	resolve_shell_target_distinguishes_the_three_forms, shell_target_resolves_relative_to_the_current_package
parse error swallowed	unparseable_shell_target_is_rejected

Three of these carry more than "the test works":

• Under the classify_failure mutation, fix(engine): group members are dependencies; they never get the terminal #225's own failing_member_of_multi_member_group_keeps_its_log_tail stays green while mine reddens — so the new test is not duplicating fix(engine): group members are dependencies; they never get the terminal #225's coverage, it is holding a door fix(engine): group members are dependencies; they never get the terminal #225 never had to close.
• The "parsed value discarded again" mutation is the original defect restored; that it reddens is the direct evidence the value can no longer be silently thrown away.
• The settle_shell mutation proves the de-duplication is real rather than incidental: without it the same mistyped address is reported twice, once by each site.

Re-verified in full after rebasing onto dfc4e5cc (#241).

Re-verified in full after rebasing onto dfc4e5cc (#241) — all 11 still red, 33 tests green.

Review board

• product-vision: SHIP WITH CHANGES — consulted at design (where it recommended rejecting the value; the user decided the opposite and specified these semantics) and again on the finished UX. Its five changes are in: runnable try: lines, the typo case moved before the run, the examples-first long help, value_name = ADDRESS (the value is parsed by parse_addr_with_base, so "TARGET" over-promised), and a not-in-run message that leads with what happened to the user's build. That last one was a typed error leading with "the build succeeded, but…"; on the decision below it became a warning, which says the same thing by construction — the build simply succeeds.
• code-quality: BLOCKED on two, both fixed. (1) The batch owner judged the shell claim even when per-target failures were collected — and fail_fast is off by default — so --shell=<group> inside a selection replaced the actionable group message with "not part of this run", every clause of which was false. (2) The terminal wrapper left in ResultOptions on the Matcher::Addr leg told classify_failure that silent targets had streamed, dropping their log tails. It also confirmed the mem_locked_result race analysis, that the fix closes rather than moves it, and that Relaxed is sound here (the flag publishes no data; every read is separated from every write by a completed await). Its MINORs on allocation-per-frame, the claim_matched_stream overload, the nested Option, and the missing Debug are all fixed.
• compatibility: COMPATIBLE, with the stated break above.

User decision after review: a --shell= target that turns out not to be in the run was a typed ShellTargetNotInRun that failed the build. That is too strong for an interactive flag — it does not justify discarding a build that did what was asked — so it is now a warning and the run proceeds. The pre-flight stayed (warning early beats warning late), and ShellTargetNotInRun was deleted rather than left as an unreachable variant.

Known asymmetry, deliberately not fixed here

--shell=<single-member group> is refused where bare --shell <same group> enters its member. Forwarding the claim through an alias has to preserve the property the whole design rests on — that the answer is a pure function of the addr, resolved identically by every frame, before any frame can observe it — and code-quality traced why routing it through opts cannot work. That is a larger change than this one. The refusal names the member and the exact command to run, so it is actionable rather than a dead end.

Also noted, not fixed: "cannot use --shell in non-interactive mode" is still an untyped bail! with no addr and no next action. It predates this PR and is the message an agent in CI hits first; typing it belongs with the other request-shape errors.

No per-platform divergence: engine control flow and clap parsing, no cfg, nothing in tui/tty.rs touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x