Skip to content

feat(minimal): add min task run — declared tasks in ephemeral sessions - #1139

Merged
norrietaylor merged 4 commits into
mainfrom
feat/min-task-run
Jul 31, 2026
Merged

feat(minimal): add min task run — declared tasks in ephemeral sessions#1139
norrietaylor merged 4 commits into
mainfrom
feat/min-task-run

Conversation

@norrietaylor

@norrietaylor norrietaylor commented Jul 31, 2026

Copy link
Copy Markdown
Member

Running a project task means hand-building a session, attaching, and remembering to destroy it; min task run <task> now does the whole loop in an ephemeral box — create (named task-<task>-<hex>), exec the task with output streamed through, exit with the task's exit code, destroy — and --keep retains the box as an attachable session.

  • Exec rides the native channel with MINIMAL_SESSION_ID set, sending the canonical in-box min task run spelling.
  • Errors name the fix: no blueprint → min init; unknown task → the declared list. A hidden top-level run catches the bare spelling.
  • Unit tests cover the CLI surface and name mint; e2e asserts the loop end-to-end.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f

Note

Add min task run command to execute declared tasks in ephemeral sessions

  • Adds min task run <task> CLI command that creates a named ephemeral session, uploads the workspace, runs the declared task via SSH exec with MINIMAL_SESSION_ID set, streams stdout/stderr to the user, and destroys the session on completion unless --keep is passed.
  • Validates the task name against minimal.toml client-side before creating a session, with clear error messages for unknown or malformed task names.
  • Adds a hidden min run ... catch-all subcommand that always errors with a redirect message pointing to min task run.
  • Ctrl-C during a task run triggers best-effort session teardown within a 10s timeout (or retains the session with --keep) and exits with code 130.
  • Behavioral Change: min task run routes tracing logs to stderr and propagates the task's exit code directly; non-zero exits suppress error printing in main.

Macroscope summarized 9b294d5.

Summary by CodeRabbit

  • New Features

    • Added min task run <task> for executing project tasks in temporary sessions.
    • Supports optional project paths and --keep to retain sessions after execution.
    • Streams task output and returns the task’s exit code directly.
    • Added session-aware execution support for task runs.
  • Bug Fixes

    • Improved interruption cleanup and session retention behavior.
    • Added compatibility handling for legacy min run arguments.
  • Tests

    • Added coverage for command parsing, output, exit codes, cleanup, retention, and task errors.

Running a project task meant activate, attach, run, remember to
destroy. min task run <task> does the loop: creates a task-named
session for the cwd, execs the canonical in-box task-run over the
exec channel with output streamed through, exits with the task's
code, and destroys the box — or keeps it attachable with --keep.
A hidden top-level run catches the bare spelling with copy naming
the canonical forms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Task execution

Layer / File(s) Summary
Session-aware exec channels
crates/minimal/src/client.rs
The client supports exec channels with an optional session ID and sets MINIMAL_SESSION_ID before the exec request.
Task command contracts and dispatch
crates/minimal/src/lib.rs, crates/minimal/src/main.rs
The CLI adds task run, hidden run compatibility handling, argument parsing, dispatch, stdout routing, and task exit-code propagation.
Task validation and session setup
crates/minimal/src/task.rs
Task execution validates projects and tasks, starts the daemon, creates sessions, uploads workspace data, and applies loadout gates.
Streaming, status, and cleanup
crates/minimal/src/task.rs
The runner streams SSH input and output, captures exit status, handles Ctrl-C, and destroys or retains sessions.
End-to-end task validation
scripts/session-e2e.sh
The end-to-end proof checks task output, exit codes, cleanup, retention, unknown-task errors, and hidden-command behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CLI
  participant TaskRunner
  participant Session
  participant TaskProcess
  User->>CLI: min task run task
  CLI->>TaskRunner: dispatch task arguments
  TaskRunner->>Session: create and prepare ephemeral session
  TaskRunner->>Session: execute task
  Session->>TaskProcess: stream command input
  TaskProcess-->>Session: stream stdout, stderr, and exit status
  Session-->>TaskRunner: return task result
  TaskRunner->>Session: destroy or retain session
  TaskRunner-->>CLI: return task exit code
  CLI-->>User: print task output and exit
Loading

Possibly related issues

  • gominimal/inbox#461 — Directly covers min task run, ephemeral sessions, streaming, exit-code propagation, cleanup, retention, CLI support, and end-to-end coverage.

Possibly related PRs

Suggested reviewers: twitchyliquid64

Poem

A rabbit starts a task in flight,
Streams its output left and right.
Sessions vanish, or remain,
Exit codes hop through the lane.
“min task run!” the burrow cheers.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the addition of the min task run command for declared tasks in ephemeral sessions.
Description check ✅ Passed The description clearly explains the behavior, lifecycle, error handling, and tests, but it omits the template's explicit Checklist section.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/min-task-run

Comment @coderabbitai help to get the list of available commands.

@norrietaylor
norrietaylor marked this pull request as ready for review July 31, 2026 15:48
@norrietaylor
norrietaylor requested a review from a team as a code owner July 31, 2026 15:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/minimal/src/client.rs (1)

320-353: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Track the set_env and exec acknowledgements separately. In russh 0.62.4, set_env(true, ...) delivers its CHANNEL_SUCCESS through wait(). This loop can return on that reply before exec(true, command) is acknowledged. An exec failure can then be ignored by bridge_exec, which may wait indefinitely. Use set_env(false, ...) or consume both replies.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimal/src/client.rs` around lines 320 - 353, Update exec_channel so
the set_env request does not cause the acknowledgement loop to return before
exec is acknowledged: use set_env with its reply disabled, or explicitly consume
and distinguish both acknowledgements before returning. Ensure exec_channel
returns only after the exec request succeeds and still propagates failures or
premature channel closure.
🧹 Nitpick comments (1)
crates/minimal/src/client.rs (1)

331-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Share the env-var name with the daemon constant.

The daemon reads this variable through MINIMAL_SESSION_ID_ENV (see crates/minimald/src/exec.rs Lines 790-796). The client hard-codes the string here and again in stream_upload at Line 517. A single shared constant keeps the routing contract in one place.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimal/src/client.rs` around lines 331 - 336, Replace the hard-coded
"MINIMAL_SESSION_ID" values in the client session setup and stream_upload paths
with the shared MINIMAL_SESSION_ID_ENV constant used by the daemon. Ensure both
set_env calls reference that constant so the client and daemon retain one
environment-variable contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/minimal/src/task.rs`:
- Around line 456-462: Update check_task_declared to reject task names whose
leading or trailing whitespace differs from the declared name before lookup
proceeds. Preserve internal spaces, quotes, and shell metacharacters, and leave
valid unchanged task names on the existing lookup path.
- Around line 143-164: Update the Ctrl-C cleanup task around
tokio::signal::ctrl_c and the DestroySession oneshot_rpc to use the existing
10-second cleanup timeout pattern, covering both connection and destruction so
exit(130) cannot be delayed indefinitely. Also correct the nearby Ctrl-C
documentation to reflect that Tokio retains its SIGINT handler, or keep a second
signal listener active during cleanup so repeated interrupts behave as
documented.

In `@scripts/session-e2e.sh`:
- Around line 335-338: Update the leftover-session checks around the `mnl ls`
calls and the `kept` assignment to capture the full listing once in a variable,
then perform `grep`/`head` matching against that variable instead of piping `mnl
ls` directly. Preserve the existing session patterns, error reporting, and
`kept` behavior while avoiding early pipe termination under `pipefail` and `set
-e`.

---

Outside diff comments:
In `@crates/minimal/src/client.rs`:
- Around line 320-353: Update exec_channel so the set_env request does not cause
the acknowledgement loop to return before exec is acknowledged: use set_env with
its reply disabled, or explicitly consume and distinguish both acknowledgements
before returning. Ensure exec_channel returns only after the exec request
succeeds and still propagates failures or premature channel closure.

---

Nitpick comments:
In `@crates/minimal/src/client.rs`:
- Around line 331-336: Replace the hard-coded "MINIMAL_SESSION_ID" values in the
client session setup and stream_upload paths with the shared
MINIMAL_SESSION_ID_ENV constant used by the daemon. Ensure both set_env calls
reference that constant so the client and daemon retain one environment-variable
contract.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1c8376a1-8e33-4820-818f-6a0f30aff618

📥 Commits

Reviewing files that changed from the base of the PR and between 7e690dc and 8f14490.

📒 Files selected for processing (5)
  • crates/minimal/src/client.rs
  • crates/minimal/src/lib.rs
  • crates/minimal/src/main.rs
  • crates/minimal/src/task.rs
  • scripts/session-e2e.sh

Comment thread crates/minimal/src/task.rs
Comment thread crates/minimal/src/task.rs
Comment thread scripts/session-e2e.sh Outdated

@twitchyliquid64 twitchyliquid64 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something for the future might be a new field in the session record to mark ephemeral sessions intended for task exec, along with a TTL etc

@norrietaylor

Copy link
Copy Markdown
Member Author

along with a TTL etc

Absolutely! That would be fantastic

@norrietaylor

Copy link
Copy Markdown
Member Author

Filed a follow-up tracking the ephemeral/task marking on the session record + TTL (with the reaper deliberately coordinated with the broader session-lifecycle design rather than ahead of it). Thanks @twitchyliquid64.

The exec bridge defaulted a missing ExitStatus to 0, so a daemon or
transport death mid-task read as success. A run that ends without a
reported status now prints a diagnostic and exits 1. Also documents
that Ctrl-C tears the session down without relaying SIGINT into the
box — the task is not given a chance to handle it — and that tasks run
non-interactively: a terminal stdin is half-closed immediately (the
daemon exec channel has no PTY), so input reaches the task only when
piped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
@norrietaylor

Copy link
Copy Markdown
Member Author

Addressed the exit-code review feedback in 8aabd89:

  • Missing exit status now fails closed. The exec bridge tracked the remote status as a bare u32 initialized to 0, so a daemon or transport death that closed the channel before an ExitStatus message read as success. The bridge now tracks receipt explicitly (Option<u32>); a run that ends without a reported status prints task ended without reporting an exit status and exits 1. The status resolution is factored into a pure exit_outcome(Option<u32>) with unit coverage for the missing-status path.
  • Ctrl-C behavior documented. The interrupt guard's doc comment and the task run help text now state that SIGINT is consumed on the host — the session is torn down (or kept with --keep) and the process exits 130 without relaying the interrupt into the box, so the task gets no chance to handle it, unlike a local exec.
  • Non-interactive stdin documented. The bridge's doc comment and help text now spell out that tasks run non-interactively: a terminal stdin is half-closed immediately (the daemon exec channel has no PTY — only the shell path does), so input reaches the task only when piped; interactive work belongs in min session attach.

The other gap in the analysis — the daemon mapping signal deaths to unwrap_or(1) instead of 128+signo in crates/minimald/src/exec.rs — predates this PR and is tracked separately; not touched here.

… fix e2e pipefail check

Three review fixes on `min task run`:

- The Ctrl-C handler's connect-and-destroy now runs under the same
  10-second cleanup ceiling as best_effort_destroy, and races a second
  Ctrl-C that abandons the cleanup and exits 130 immediately — an
  unresponsive daemon can delay the exit, never prevent it, and a
  second interrupt is no longer silently swallowed.
- Task names with leading/trailing whitespace are rejected client-side
  before any lookup, debug-quoted so the whitespace is visible: the
  daemon trims the exec command string, so such a name can never
  round-trip into the box.
- The e2e leftover-session checks no longer pipe `mnl ls` into
  `grep -q`, whose early exit SIGPIPEs the ls under pipefail and made
  the check falsely pass; they capture-then-glob per the script's own
  convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
@norrietaylor

Copy link
Copy Markdown
Member Author

Addressed the three CodeRabbit findings in 8a76e7a:

  • Interrupt cleanup is now bounded and re-interruptible (task.rs guard). The Ctrl-C handler's connect + DestroySession runs under the same 10-second cleanup ceiling as best_effort_destroy (with a manual-cleanup hint if it times out), and is raced against a second ctrl_c() listener: a second interrupt during cleanup abandons it and exits 130 immediately. An unresponsive daemon can delay the exit, never prevent it, and a second Ctrl-C is no longer silently swallowed. The guard's doc comment now describes this behavior.
  • Untrimmable task names rejected client-side (check_task_declared). The daemon trims the task name out of the exec command string, so a declared name with leading/trailing whitespace could never round-trip into the box. Such names are now rejected before any config lookup, with the name debug-quoted so the whitespace is visible (e.g. task name " build " has leading or trailing whitespace...). Unit-tested, including that the rejection fires ahead of the minimal.toml discovery.
  • e2e leftover-session checks fixed for pipefail (scripts/session-e2e.sh). The two mnl ls | grep -q checks could falsely pass: grep -q's early exit SIGPIPEs the ls under pipefail, turning a genuine leftover into a non-zero pipeline. Both now capture mnl ls output to a variable and glob-match it — the same convention the script's attach-proof markers prescribe. Shellcheck stays clean.

@norrietaylor
norrietaylor enabled auto-merge (squash) July 31, 2026 17:39
@norrietaylor
norrietaylor merged commit 3b4e3da into main Jul 31, 2026
30 checks passed
@norrietaylor
norrietaylor deleted the feat/min-task-run branch July 31, 2026 18:03
norrietaylor added a commit that referenced this pull request Jul 31, 2026
Conflicts and reconciliation:
- crates/minimal/src/main.rs: stdout_is_data_contract doc + tests — kept
  both the bare-min contract (this branch) and the task-run contract
  (#1139); the match arm union auto-merged.
- crates/minimal/src/lib.rs (semantic, auto-merged textually): cmd_bare
  rewritten against #1142's SmartAttach enum — the picker's create row
  (CreateForCwd) and the first-run NoSessions case both create-and-attach
  on the bare path, scaffold offer suppressed; min session attach keeps
  its scaffold-offering activate_new_for_attach unchanged. Dropped this
  branch's status_label duplicate in favor of the identical helper #1144
  added for the ls table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants