Skip to content

fix(engine): group members are dependencies; they never get the terminal - #225

Merged
raphaelvigee merged 1 commit into
masterfrom
raphaelvigee/interactive-group-tty
Jul 28, 2026
Merged

fix(engine): group members are dependencies; they never get the terminal#225
raphaelvigee merged 1 commit into
masterfrom
raphaelvigee/interactive-group-tty

Conversation

@raphaelvigee

@raphaelvigee raphaelvigee commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

The bug

heph run <addr> on a terminal hands the target a real stdin: the wrapper in src/commands/run.rs pauses the TUI, builds a TtyReader on fd 0, and runs the target attached to it. TtyReader documents that only one may exist at a time, and the TUI pause it depends on is a plain bool, not a refcount.

The transparent-group fan-out in result_addr_impl propagated opts.interactive to every member (only --shell cleared it). So heph run //pkg:group with ≥2 uncached members built a TtyReader per member on the same fd. The first to finish resumed the TUI — re-enabling raw mode, restarting the crossterm EventStream, clearing Ctrl-C suppression — underneath its still-running siblings, and TtyReader::drop restored blocking mode on fd 0 while a sibling was still driving it through AsyncFd, leaving a blocking read(0) on a tokio worker that the request's cancellation token cannot reach.

multi_member_group_forwards_the_terminal_to_no_member reproduces it: with the fix reverted, the wrapper's concurrency high-water mark is 2.

Not a new rule

Dependencies have never been interactive. The terminal goes to the single target the user named, never to something the engine pulled in on its behalf.

A transparent group of two or more members is exactly that case: inlining is an implementation detail, and what the members are is the run's dependencies. The group gate added here and the ResultOptions::default() that dependency resolution already uses are one principle enforced at two points, not two rules that happen to agree.

A group of one is a name, not a fan-out — there is nothing to call a dependency — so it hands the terminal straight through and heph run //:dev keeps behaving like running its one member, at any nesting depth. That is why the gate is "one distinct member", not "not a group".

The third enforcement point is Engine::result, which clears interactive for any non-Addr matcher: a selection names no single target to give the terminal to. Together the three guarantee at most one live terminal wrapper per request. Each now has a regression test; before this PR only the matcher leg existed and none was covered.

Deliberately not refcounting the pause: that makes two writers on one fd deterministic rather than absent.

Behavior changes

  • A group with ≥2 members runs its members as the dependencies they are: no stdin (reads see EOF, per pluginexec's StdioSpec::Null) and no live stdout/stderr, with each member's output in its sandbox log.txt, surfaced on failure and kept as an artifact. Same as any other dependency.
  • heph run --shell //pkg:alias now works. It previously failed: the group branch cleared interactive while keeping shell, and the member frame then hit the "cannot use --shell in non-interactive mode" guard.
  • --shell on a group that is not a single target, and --shell on a multi-target selection, are both refused by a new typed ShellNeedsSingleTarget naming what was asked for and the exact command to run instead. Both previously produced "cannot use --shell in non-interactive mode" — actively misleading, since the user is on a terminal. The selection form is now refused once, up front, instead of once per matched target.
  • That error is a property of the request, not a target failure, so classify_failure propagates it unchanged (like CycleError/HashUnknownError), including out of a MultiError aggregation. Without it, --shell on a group nested inside a single-member group recorded the user's own input error as the outer group's failure and printed a failed-target box for a group that never executed.
  • The gate counts distinct member addrs, not input entries: a group listing one member twice under different output filters is still one executing target (both entries share the addr-keyed mem_locked_result/mem_execute_cache cells), so it keeps the terminal instead of being told it is "a group with 2 members: //pkg:a, //pkg:a".
  • An empty group is legal; --shell on it now says so and still names an action.
  • A failing member of a multi-member group keeps its process log tail. classify_failure drops the tail for interactive targets on the grounds their output already streamed; a group member never streamed anything, so propagating interactive had been silently making heph run //pkg:group diagnose worse than heph run //....

Diagnosability

The drop is a debug! (addr, members, reason) — not an info! or a warning. Nothing surprising happened: dependencies have never been interactive, so there is no expectation to correct, only a "why didn't I get a prompt?" to answer under -v. The --shell refusals are self-explaining, and heph run --help states the rule next to --shell.

Verification

13 new tests (19 targeted pass), in-crate fake driver/provider harness, no subprocesses, ~1s total. cargo fmt --check and the CI clippy invocation clean.

Mutation-verified red — the group gate (concurrency high-water 2), the matcher leg, the --shell selection refusal, the classify_failure pass-through, and the distinct-member counting each fail with their fix reverted.

One honest gap: the dependency leg is not mutation-provable. meta builds deps while computing the parent's hashin and has no opts in scope at all, so deliberately handing the wrapper to inputs_result_exec changes nothing — every dep is already a memoizer hit by then. deps_never_inherit_the_terminal guards a future refactor rather than a flippable line, and the comment says so.

Review board

  • product-vision: SHIP WITH CHANGES — asked for the single-member carve-out over an unconditional clear, deleting the --shell special case in favour of one rule, the reworded --shell error, a debug! rather than a user-visible warning, and the log-tail regression test. All five are in.
  • code-quality: PASS WITH NITS — traced the invariant closed, no soundness bug. Its MAJOR (dropping the wrapper drops stdout/stderr, not just stdin) was escalated to the user, who confirmed the behavior on the dependency framing above. Its classify_failure-bypass, entry-vs-target-counting and empty-group-message findings are fixed.
  • feature-quality: PASS WITH FOLLOW-UPS — judged the fix correct, cheap and correctly placed, and cleared the 50 ms probe sleep as flake-free (probe.calls is 0-or-1 by construction; the sleep only sharpens max_live). Its four coverage MAJORs are closed.

Noted, not fixed

  • --shell=TARGET silently discards its value (run.rs: require_equals = true + default_missing_value = "", and args.shell.is_some() is the only consumer). Pre-existing, and against this repo's fail-or-fix rule — but out of scope here.
  • The one-root invariant is not type-enforced. run.rs routes Matcher::Addr to result_addr and everything else to result; a future caller passing Matcher::Addr to Engine::result would keep interactive and spawn one result_addr per matched addr. Dormant today.

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x

@raphaelvigee @claude
fix(engine): group members are dependencies; they never get the terminal
ced572a 
`heph run <addr>` on a terminal hands the target a real stdin: the wrapper
in `src/commands/run.rs` pauses the TUI, builds a `TtyReader` on fd 0, and
runs the target attached to it. `TtyReader` documents that only one may
exist at a time, and the TUI pause it depends on is a plain boolean, not a
refcount.

The transparent-group fan-out in `result_addr_impl` propagated
`opts.interactive` to every member (only `--shell` cleared it), so
`heph run //pkg:group` with two or more uncached members built a
`TtyReader` per member on the same fd. The first member to finish resumed
the TUI — re-enabling raw mode, restarting the crossterm `EventStream` and
clearing Ctrl-C suppression — underneath its still-running siblings, and
`TtyReader::drop` restored blocking mode on fd 0 while a sibling was still
driving it through `AsyncFd`, leaving a blocking `read(0)` on a tokio
worker that the request's cancellation token cannot reach. The new
`multi_member_group_forwards_the_terminal_to_no_member` test reproduces
it: pre-fix the wrapper's concurrency high-water mark is 2.

The fix is not a new rule. Dependencies have never been interactive — the
terminal goes to the single target the user named, never to something the
engine pulled in on its behalf. A transparent group of two or more members
is exactly that case: inlining is an implementation detail, and what the
members *are* is the run's dependencies. So the group gate added here and
the `ResultOptions::default()` that dependency resolution already uses are
one principle enforced at two points, not two rules that happen to agree.
`deps_never_inherit_the_terminal` now pins the general form.

A group of one is a *name*, not a fan-out — there is nothing to call a
dependency — so it hands the terminal straight through and
`heph run //:dev` keeps behaving like running its one member, at any
nesting depth. That is why the gate is "one distinct member", not "not a
group".

The third enforcement point is `Engine::result`, which clears
`interactive` for any non-`Addr` matcher: a selection names no single
target to give the terminal to. Together the three guarantee at most one
live terminal wrapper per request. Each now has a regression test; before
this commit only the matcher leg existed and none was covered.

Deliberately *not* refcounting the pause: that would make two writers on
one fd deterministic rather than absent.

Behavior changes, all on the `--shell`/terminal path:

- A group with two or more members runs its members as the dependencies
  they are: no stdin (reads see EOF, per `pluginexec`'s `StdioSpec::Null`)
  and no live stdout/stderr, with each member's output in its sandbox
  `log.txt`, surfaced on failure and kept as an artifact. Same as any
  other dependency.
- `heph run --shell //pkg:alias` now works. It previously failed, because
  the group branch cleared `interactive` while keeping `shell`, and the
  member frame then hit the "cannot use --shell in non-interactive mode"
  guard.
- `--shell` on a group that is not a single target, and `--shell` on a
  multi-target selection, are both refused by a new typed
  `ShellNeedsSingleTarget` naming what was asked for and the exact command
  to run instead. Both previously produced "cannot use --shell in
  non-interactive mode", which is actively misleading — the user *is* on a
  terminal. The selection form is now refused once, up front, instead of
  once per matched target.
- The error is a property of the request, not a target failure, so
  `classify_failure` propagates it unchanged (like `CycleError` /
  `HashUnknownError`), including out of a `MultiError` aggregation. Without
  that, `--shell` on a group nested inside a single-member group recorded
  the user's own input error as the outer group's failure and printed a
  failed-target box for a group that never executed.
- The gate counts *distinct member addrs*, not input entries: a group
  listing one member twice under different output filters is still one
  executing target (both entries share the addr-keyed `mem_locked_result`
  / `mem_execute_cache` cells), so it keeps the terminal instead of being
  told it is "a group with 2 members: //pkg:a, //pkg:a".
- An empty group is legal; `--shell` on it now says so and still names an
  action rather than trailing off with no recovery.
- A failing member of a multi-member group keeps its process log tail.
  `classify_failure` drops the tail for interactive targets on the grounds
  that their output already streamed to the terminal; a group member never
  streamed anything, so propagating `interactive` had been silently making
  `heph run //pkg:group` diagnose worse than `heph run //...`.

Diagnosability: the drop is a `debug!` (`addr`, `members`, `reason`), not
an `info!` or a warning — nothing surprising happened, dependencies have
never been interactive, so there is no expectation to correct, only a "why
didn't I get a prompt?" to answer under `-v`. The `--shell` refusals are
self-explaining, and `heph run --help` states the rule next to `--shell`.

Review board:

- `product-vision`: SHIP WITH CHANGES — asked for the single-member
  carve-out over an unconditional clear, for deleting the `--shell`
  special case in favour of one rule, for the reworded `--shell` error,
  for a `debug!` rather than a user-visible warning, and for the log-tail
  regression test. All five are in.
- `code-quality`: PASS WITH NITS — traced the invariant closed, no
  soundness bug. Its MAJOR (dropping the wrapper drops stdout/stderr, not
  just stdin) was put to the user, who confirmed the behavior: group
  members are dependencies, and dependencies do not stream. Its findings
  on the `classify_failure` bypass, entry-vs-target counting and the
  empty-group message are fixed above.
- `feature-quality`: PASS WITH FOLLOW-UPS — judged the fix correct, cheap
  and correctly placed, and cleared the 50 ms probe sleep as flake-free
  (`probe.calls` is 0-or-1 by construction; the sleep only sharpens
  `max_live`). Its four coverage MAJORs are closed by the new tests.

Verification: 19 targeted tests pass, `cargo fmt --check` and the CI
clippy invocation are clean. Mutation-verified red — the group gate
(concurrency high-water 2), the matcher leg, the `--shell` selection
refusal, the `classify_failure` pass-through and the distinct-member
counting each fail with their fix reverted. The dependency leg is not
mutation-provable: `meta` builds deps while computing the parent's
`hashin` and has no `opts` in scope at all, so handing the wrapper to
`inputs_result_exec` changes nothing — the test guards a future refactor
instead, and the comment says so.

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x
@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/interactive-group-tty branch from d21b19e to ced572a Compare July 28, 2026 08:21
@raphaelvigee raphaelvigee changed the title fix(engine): give the terminal to one target, never to group members fix(engine): group members are dependencies; they never get the terminal Jul 28, 2026
@raphaelvigee
raphaelvigee enabled auto-merge (squash) July 28, 2026 08:30
@raphaelvigee
raphaelvigee merged commit 97419ef into master Jul 28, 2026
18 checks passed
@raphaelvigee
raphaelvigee deleted the raphaelvigee/interactive-group-tty branch July 28, 2026 08:40
raphaelvigee added a commit to hephbuild/hephbuild.github.io that referenced this pull request Jul 28, 2026
@raphaelvigee
docs: document --shell single-target rule and group member semantics
eda99ae 
Reflects hephbuild/heph#225 (97419ef): group members are dependencies
and never receive the terminal; `--shell` on a multi-member group or a
multi-target selection is now rejected with an actionable error,
while a single-member group still shells into its member as an alias.
This was referenced Jul 28, 2026
Closed
Merged
Open
raphaelvigee added a commit that referenced this pull request Jul 29, 2026
@raphaelvigee @claude
feat(run): --shell=ADDRESS names which target gets the terminal
03a431a 
`--shell` declared `require_equals` + `default_missing_value`, so
`heph run --shell=//pkg:a //pkg:...` parsed — but `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 the repo's
fail-or-fix rule. #225 recorded it as pre-existing and out of scope, and
sharpened it: its new `ShellNeedsSingleTarget` message tells the user to
name the member they want, which was precisely the form being ignored.

The value now selects the target. Bare `--shell` keeps #225's inference
(the run's single target, refused when there isn't one, single-member
group carve-out intact); `--shell=ADDRESS` names it explicitly, and every
other target in the run builds with no terminal.

The shell target rides on `RequestState`, not on `ResultOptions`, for two
reasons that both reduce to "a `ResultOptions` does not reach every
frame". It travels down the selection and group-inlining legs only —
dependency resolution resets it and `meta` has no `opts` in scope — so an
addr named with `--shell=` would be unreachable the moment it appeared as
a dependency, which is the same silent discard in a subtler form. And
`mem_locked_result` is keyed by `Addr` alone: were "does this frame
shell?" a function of the frame's options, an addr reached both as a
selected target and as a sibling's dependency would bake whichever frame
won that memoizer race. Read from the request it is a pure function of
the addr, so every frame agrees and a dependency can be entered — which
is the workflow this mainly unlocks.

The at-most-one-terminal invariant #225 established holds by
construction: exactly one address matches, and `inner_result_addr` — the
tree's only `ExecuteOptions` — hands every other frame no terminal.
`classify_failure` now reads "did this frame get the terminal" from the
same place, so a silent target is never told it streamed and never loses
its process-log tail.

Three refusals keep the value from ever being silently dropped. An
address no target has fails before the run, in one memoized spec lookup —
a typo must not cost a full build. `--shell=<group>` is answered where
the members are in hand (a transparent target never executes). An address
that exists but this run's graph never reached raises
`ShellTargetNotInRun` afterwards, on the success path only, so it can
never displace a real diagnosis.

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

BREAKING (CLI behavior, not parsing): `--shell=X` previously parsed but
silently discarded X, behaving identically to bare `--shell`. X is now
meaningful — anyone passing it gets either a shell in X or a hard failure
(exit 1) if X is not part of the run. There is no CLI schema version to
bump and no prior release documented what `--shell=X` did, so this lands
as a stated break rather than a migration.

Review board:
- product-vision SHIP WITH CHANGES — every `try:` line is now runnable
  verbatim (a template there is a command an agent executes for real),
  the not-in-run message leads with the build's success, the typo case
  moved before the run, and the long help leads with examples.
- code-quality BLOCKED on two findings, both fixed: the batch owner
  judged the shell claim even when per-target failures were collected
  (`fail_fast` is off by default), replacing the actionable group refusal
  with a false "not part of this run"; and the terminal wrapper left in
  `ResultOptions` told `classify_failure` that silent targets had
  streamed, dropping their log tails.
- compatibility COMPATIBLE — no versioned artifact records the flag's
  arity or the message text; `ResultOptions` and `new_state_full` are
  unchanged, `ShellTarget` is purely additive.

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 — and that is a larger change than
this one. The refusal names the member and the exact command to run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x
raphaelvigee added a commit that referenced this pull request Jul 29, 2026
@raphaelvigee @claude
feat(run): --shell=ADDRESS names which target gets the terminal
fb4ec18 
`--shell` declared `require_equals` + `default_missing_value`, so
`heph run --shell=//pkg:a //pkg:...` parsed — but `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 the repo's
fail-or-fix rule. #225 recorded it as pre-existing and out of scope, and
sharpened it: its new `ShellNeedsSingleTarget` message tells the user to
name the member they want, which was precisely the form being ignored.

The value now selects the target. Bare `--shell` keeps #225's inference
(the run's single target, refused when there isn't one, single-member
group carve-out intact); `--shell=ADDRESS` names it explicitly, and every
other target in the run builds with no terminal.

The shell target rides on `RequestState`, not on `ResultOptions`, for two
reasons that both reduce to "a `ResultOptions` does not reach every
frame". It travels down the selection and group-inlining legs only —
dependency resolution resets it and `meta` has no `opts` in scope — so an
addr named with `--shell=` would be unreachable the moment it appeared as
a dependency, which is the same silent discard in a subtler form. And
`mem_locked_result` is keyed by `Addr` alone: were "does this frame
shell?" a function of the frame's options, an addr reached both as a
selected target and as a sibling's dependency would bake whichever frame
won that memoizer race. Read from the request it is a pure function of
the addr, so every frame agrees and a dependency can be entered — which
is the workflow this mainly unlocks.

The at-most-one-terminal invariant #225 established holds by
construction: exactly one address matches, and `inner_result_addr` — the
tree's only `ExecuteOptions` — hands every other frame no terminal.
`classify_failure` now reads "did this frame get the terminal" from the
same place, so a silent target is never told it streamed and never loses
its process-log tail.

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. But it is never silent: the user
asked for a shell and has to hear that they are not getting one. An
address no target has is warned about *before* the run, in one memoized
spec lookup, so they learn it while the build is still going rather than
after waiting it out; an address that exists but the run never resolved
is warned about at the end, since graph membership needs the walk. One
`shell_settled` flag covers both so the same address is never reported
twice.

Two refusals remain refusals, and are separate cases: `--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.

CLI-behavior change (not parsing): `--shell=X` previously parsed but
silently discarded X, behaving identically to bare `--shell`. X is now
meaningful. Anyone passing one gets a shell in X, or a warning and a
normal build if X is not part of the run. There is no CLI schema version
to bump and no prior release documented what `--shell=X` did, so this
lands as a stated change rather than a migration.

Review board:
- product-vision SHIP WITH CHANGES — every `try:` line is runnable
  verbatim (a template there is a command an agent executes for real),
  the typo case moved before the run, and the long help leads with
  examples.
- code-quality BLOCKED on two findings, both fixed: the batch owner
  judged the shell claim even when per-target failures were collected
  (`fail_fast` is off by default), talking over the actionable group
  refusal; and the terminal wrapper left in `ResultOptions` told
  `classify_failure` that silent targets had streamed, dropping their
  log tails.
- compatibility COMPATIBLE — no versioned artifact records the flag's
  arity or the message text; `ResultOptions` and `new_state_full` are
  unchanged, `ShellTarget` is purely additive.

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 — and that is a larger change than
this one. The refusal names the member and the exact command to run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x
raphaelvigee added a commit that referenced this pull request Jul 30, 2026
@raphaelvigee @claude
feat(run): --shell=ADDRESS names which target gets the terminal
d821cb4 
`--shell` declared `require_equals` + `default_missing_value`, so
`heph run --shell=//pkg:a //pkg:...` parsed — but `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 the repo's
fail-or-fix rule. #225 recorded it as pre-existing and out of scope, and
sharpened it: its new `ShellNeedsSingleTarget` message tells the user to
name the member they want, which was precisely the form being ignored.

The value now selects the target. Bare `--shell` keeps #225's inference
(the run's single target, refused when there isn't one, single-member
group carve-out intact); `--shell=ADDRESS` names it explicitly, and every
other target in the run builds with no terminal.

The shell target rides on `RequestState`, not on `ResultOptions`, for two
reasons that both reduce to "a `ResultOptions` does not reach every
frame". It travels down the selection and group-inlining legs only —
dependency resolution resets it and `meta` has no `opts` in scope — so an
addr named with `--shell=` would be unreachable the moment it appeared as
a dependency, which is the same silent discard in a subtler form. And
`mem_locked_result` is keyed by `Addr` alone: were "does this frame
shell?" a function of the frame's options, an addr reached both as a
selected target and as a sibling's dependency would bake whichever frame
won that memoizer race. Read from the request it is a pure function of
the addr, so every frame agrees and a dependency can be entered — which
is the workflow this mainly unlocks.

The at-most-one-terminal invariant #225 established holds by
construction: exactly one address matches, and `inner_result_addr` — the
tree's only `ExecuteOptions` — hands every other frame no terminal.
`classify_failure` now reads "did this frame get the terminal" from the
same place, so a silent target is never told it streamed and never loses
its process-log tail.

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. But it is never silent: the user
asked for a shell and has to hear that they are not getting one. An
address no target has is warned about *before* the run, in one memoized
spec lookup, so they learn it while the build is still going rather than
after waiting it out; an address that exists but the run never resolved
is warned about at the end, since graph membership needs the walk. One
`shell_settled` flag covers both so the same address is never reported
twice.

Two refusals remain refusals, and are separate cases: `--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.

CLI-behavior change (not parsing): `--shell=X` previously parsed but
silently discarded X, behaving identically to bare `--shell`. X is now
meaningful. Anyone passing one gets a shell in X, or a warning and a
normal build if X is not part of the run. There is no CLI schema version
to bump and no prior release documented what `--shell=X` did, so this
lands as a stated change rather than a migration.

Review board:
- product-vision SHIP WITH CHANGES — every `try:` line is runnable
  verbatim (a template there is a command an agent executes for real),
  the typo case moved before the run, and the long help leads with
  examples.
- code-quality BLOCKED on two findings, both fixed: the batch owner
  judged the shell claim even when per-target failures were collected
  (`fail_fast` is off by default), talking over the actionable group
  refusal; and the terminal wrapper left in `ResultOptions` told
  `classify_failure` that silent targets had streamed, dropping their
  log tails.
- compatibility COMPATIBLE — no versioned artifact records the flag's
  arity or the message text; `ResultOptions` and `new_state_full` are
  unchanged, `ShellTarget` is purely additive.

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 — and that is a larger change than
this one. The refusal names the member and the exact command to run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x
raphaelvigee added a commit that referenced this pull request Jul 30, 2026
@raphaelvigee @claude
feat(run): --shell=ADDRESS names which target gets the terminal
0026169 
`--shell` declared `require_equals` + `default_missing_value`, so
`heph run --shell=//pkg:a //pkg:...` parsed — but `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 the repo's
fail-or-fix rule. #225 recorded it as pre-existing and out of scope, and
sharpened it: its new `ShellNeedsSingleTarget` message tells the user to
name the member they want, which was precisely the form being ignored.

The value now selects the target. Bare `--shell` keeps #225's inference
(the run's single target, refused when there isn't one, single-member
group carve-out intact); `--shell=ADDRESS` names it explicitly, and every
other target in the run builds with no terminal.

The shell target rides on `RequestState`, not on `ResultOptions`, for two
reasons that both reduce to "a `ResultOptions` does not reach every
frame". It travels down the selection and group-inlining legs only —
dependency resolution resets it and `meta` has no `opts` in scope — so an
addr named with `--shell=` would be unreachable the moment it appeared as
a dependency, which is the same silent discard in a subtler form. And
`mem_locked_result` is keyed by `Addr` alone: were "does this frame
shell?" a function of the frame's options, an addr reached both as a
selected target and as a sibling's dependency would bake whichever frame
won that memoizer race. Read from the request it is a pure function of
the addr, so every frame agrees and a dependency can be entered — which
is the workflow this mainly unlocks.

The at-most-one-terminal invariant #225 established holds by
construction: exactly one address matches, and `inner_result_addr` — the
tree's only `ExecuteOptions` — hands every other frame no terminal.
`classify_failure` now reads "did this frame get the terminal" from the
same place, so a silent target is never told it streamed and never loses
its process-log tail.

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. But it is never silent: the user
asked for a shell and has to hear that they are not getting one. An
address no target has is warned about *before* the run, in one memoized
spec lookup, so they learn it while the build is still going rather than
after waiting it out; an address that exists but the run never resolved
is warned about at the end, since graph membership needs the walk. One
`shell_settled` flag covers both so the same address is never reported
twice.

Two refusals remain refusals, and are separate cases: `--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.

CLI-behavior change (not parsing): `--shell=X` previously parsed but
silently discarded X, behaving identically to bare `--shell`. X is now
meaningful. Anyone passing one gets a shell in X, or a warning and a
normal build if X is not part of the run. There is no CLI schema version
to bump and no prior release documented what `--shell=X` did, so this
lands as a stated change rather than a migration.

Review board:
- product-vision SHIP WITH CHANGES — every `try:` line is runnable
  verbatim (a template there is a command an agent executes for real),
  the typo case moved before the run, and the long help leads with
  examples.
- code-quality BLOCKED on two findings, both fixed: the batch owner
  judged the shell claim even when per-target failures were collected
  (`fail_fast` is off by default), talking over the actionable group
  refusal; and the terminal wrapper left in `ResultOptions` told
  `classify_failure` that silent targets had streamed, dropping their
  log tails.
- compatibility COMPATIBLE — no versioned artifact records the flag's
  arity or the message text; `ResultOptions` and `new_state_full` are
  unchanged, `ShellTarget` is purely additive.

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 — and that is a larger change than
this one. The refusal names the member and the exact command to run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A76DChMohieGMbRbnV1E7x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@raphaelvigee