feat(minimal): add min task run — declared tasks in ephemeral sessions - #1139
Conversation
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
📝 WalkthroughWalkthroughChangesTask execution
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
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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 winTrack the
set_envandexecacknowledgements separately. Inrussh 0.62.4,set_env(true, ...)delivers itsCHANNEL_SUCCESSthroughwait(). This loop can return on that reply beforeexec(true, command)is acknowledged. Anexecfailure can then be ignored bybridge_exec, which may wait indefinitely. Useset_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 valueShare the env-var name with the daemon constant.
The daemon reads this variable through
MINIMAL_SESSION_ID_ENV(seecrates/minimald/src/exec.rsLines 790-796). The client hard-codes the string here and again instream_uploadat 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
📒 Files selected for processing (5)
crates/minimal/src/client.rscrates/minimal/src/lib.rscrates/minimal/src/main.rscrates/minimal/src/task.rsscripts/session-e2e.sh
twitchyliquid64
left a comment
There was a problem hiding this comment.
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
Absolutely! That would be fantastic |
|
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
|
Addressed the exit-code review feedback in 8aabd89:
The other gap in the analysis — the daemon mapping signal deaths to |
… 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
|
Addressed the three CodeRabbit findings in 8a76e7a:
|
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.
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 (namedtask-<task>-<hex>), exec the task with output streamed through, exit with the task's exit code, destroy — and--keepretains the box as an attachable session.MINIMAL_SESSION_IDset, sending the canonical in-boxmin task runspelling.min init; unknown task → the declared list. A hidden top-levelruncatches the bare spelling.🤖 Generated with Claude Code
https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
Note
Add
min task runcommand to execute declared tasks in ephemeral sessionsmin task run <task>CLI command that creates a named ephemeral session, uploads the workspace, runs the declared task via SSH exec withMINIMAL_SESSION_IDset, streams stdout/stderr to the user, and destroys the session on completion unless--keepis passed.minimal.tomlclient-side before creating a session, with clear error messages for unknown or malformed task names.min run ...catch-all subcommand that always errors with a redirect message pointing tomin task run.--keep) and exits with code 130.min task runroutes 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
min task run <task>for executing project tasks in temporary sessions.--keepto retain sessions after execution.Bug Fixes
min runarguments.Tests