feat(processes): start = "interactive-shell" and attachable devenv up#2890
Draft
domenkozar wants to merge 9 commits into
Draft
feat(processes): start = "interactive-shell" and attachable devenv up#2890domenkozar wants to merge 9 commits into
start = "interactive-shell" and attachable devenv up#2890domenkozar wants to merge 9 commits into
Conversation
processes.<name>.start.enable (and the new global process.start default) now accept "shell": such processes start when entering an interactive devenv shell and stop when you exit it, replacing the devenv up -d && devenv shell two-step plus the matching exit + devenv processes down. devenv up now attaches to an already-running native manager over its control socket and starts the up-enabled processes instead of failing with "Processes already running". Fixes #2863 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eractive-shell
Rework `devenv up` attaching to a running native manager so dependency
ordering and readiness are driven by the daemon's task scheduler instead of
re-derived CLI-side. A new `ApiRequest::Up { names }` is forwarded to the
scheduler, which brings processes up through the same `wait_for_task_deps` +
`run_process` path as a cold start. This fixes a dependent being launched
before an out-of-subset `after` dependency, drops the brittle per-process
Start+poll loop (no more bailing on a clean one-shot exit or half-starting),
and removes the duplicated client-side topo sort and readiness wait.
Also:
- Rename the `start.enable` / `process.start` value `"shell"` to
`"interactive-shell"`, making clear it only applies to interactive
`devenv shell` sessions; warn when a non-interactive shell skips them.
- `devenv test` (and any detached caller) again refuses to attach to a
foreign manager instead of blocking in the foreground view.
- An explicit `devenv up <name>` for an unknown process fails loudly rather
than silently succeeding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The detached daemon spawns as the last step of start_shell_processes, so a failure inside it returned through `?` and skipped the on-exit teardown, orphaning the daemon while the user was dropped back to their outer shell with no session left to stop it. Stop it via down() (which reconstructs the manager from the PID file) before propagating the error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A process that exits on its own stays `Active` in the manager with a terminal `Exited`/`GaveUp` phase (only an explicit stop produces `Stopped`), and `rearm_waiting` refuses to touch an `Active` entry. The attach path skipped only `Starting`/`Ready`, so for a self-exited process it tried to re-arm (a no-op), then `launch_waiting` bailed and the process was silently never relaunched. `start_with_deps` now normalizes a terminal `Active` entry to `Stopped` via `stop_and_keep` (which aborts the dead supervisor and tailers and releases its ports, keeping the TUI row) before re-arming, so the existing launch path relaunches it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ests `devenv up` attaching to a running manager forwards `ApiRequest::Up` to the task scheduler over the control socket, but `set_up_handler` ran only after `run_with_parent_activity` returned, i.e. after every auto-started process was ready. The API socket is reachable well before that, and in `--background` mode (`devenv shell`) the PID file an attaching `devenv up` waits on is written first too, so an `Up` arriving mid-startup was rejected with "no process scheduler to handle up". The foreground in-process `devenv up` never registered a handler at all, so a second `devenv up` attaching to it always failed. Register the handler before the cold start so requests buffer until the foreground loop drains them once startup completes, and route both the daemon and the foreground `up` path through a shared `run_foreground_with_up` helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The interactive `devenv shell` status line now reports how many processes are running alongside it (e.g. `watching 5 files | 3 processes`, or `running | 3 processes` when nothing is watched), updating live as processes start, become ready, or exit. A poller queries the native manager's control socket and pushes the Starting/Ready count to the session over a new `ShellCommand::ProcessCount`. The poller runs concurrently with the build that gates the initial Spawn, so a ProcessCount can reach the session first; the session absorbs those pre-Spawn updates rather than rejecting them as an unexpected command. Also dogfoods `start.enable = "interactive-shell"` for the docs `mkdocs serve` process. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🔍 Suggested ReviewersBased on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:
Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏 This comment was automatically generated by git-blame-auto-reviewer Last updated: 2026-06-02T18:54:22.203Z |
Deploying devenv with
|
| Latest commit: |
09a771a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5f577f58.devenv.pages.dev |
| Branch Preview URL: | https://process-start-shell.devenv.pages.dev |
`start_with_deps` skipped only `Starting`/`Ready` processes, so a second `devenv up <name>` issued while the process was still `Waiting` on an unmet dependency re-armed it and spawned a second detached dep-waiter. When the dependency resolved, both waiters woke; the loser called `launch_waiting` on the now-active entry and logged a spurious "not in waiting state" error. Treat `Waiting` like `Starting`/`Ready` and leave it alone: a live dep-waiter is already in flight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A foreground `devenv up` (no -d) that found a manager already running used to bail "Processes already running"; it now attaches and streams a live view until Ctrl-C or the daemon exits. In a script, pipe, or CI that view can never be interrupted, so the command blocked forever. Gate the attach on an interactive terminal (stdin and stderr are TTYs and not CI) and otherwise fail fast as before. Restore the regression check in the processes-daemon-down test, asserting on the message so a hang killed by the timeout is not mistaken for a clean reject. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Is per-process starting required or a nice to have? How much can we shrink the PR if we keep the global |
Member
Author
|
We know people want to start some processes at shell and potentially others later on. |
|
The way I see it, it is important to address user concerns, but it's also important to keep the project maintainable. Adding 1000 lines of code, scattered over so many files and introducing several new edge cases, all for one QoL feature - that does not seem maintainable. I'd advise reconsidering the solution. Maybe it's possible to add it on top somehow, rather than modify the core functionality of the native process manager? |
|
I tried implementing per-process starting with minimized changes to the process manager. See the 2nd commit on my PR, I'll also leave a summary there #2912 Maybe it's worth discussing this on a call? I may join devenv.sh Discord if you see the need. |
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
2 participants
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.
Summary
Adds
processes.<name>.start.enable = "interactive-shell"(and a globalprocess.startdefault): such processes start when you enter an interactivedevenv shelland stop when you exit, replacing thedevenv up -d && devenv shelltwo-step plus the matchingexit+devenv processes down.devenv upnow attaches to an already-running native manager (fromdevenv up -dordevenv shell) over its control socket and starts the up-enabled processes, instead of failing with "Processes already running". Dependency ordering (after/before) is driven by the daemon's own task scheduler via a newApiRequest::Upforwarded toTasks::start_with_deps, so out-of-subset and already-running dependencies resolve against the live task graph exactly like a cold start.The interactive shell status line shows a live count of running processes (e.g.
watching 5 files | 3 processes).Native process manager only.
Fixes #2863.
What's here
start.enableaccepts"interactive-shell"; newprocess.startglobal default; assertion requiring the native manager.devenv upattaches viaApiRequest::Up→ daemon scheduler (start_with_deps); honours the positional subset and dependency ordering.--backgrounddaemon mode writes the PID file early so shell entry isn't blocked on process readiness.Fixes folded in (from self-review)
devenv up. Such a process staysActivewith a terminal phase, whichrearm_waitingrefused to touch, so it was silently never relaunched.start_with_depsnow normalizes it viastop_and_keepfirst. (+ regression test)Uphandler before the daemon serves requests. It was set only after every auto-started process became ready, and the foreground in-processupnever set it at all, so an attachingdevenv upduring startup (or to a foregroundup) was rejected with "no process scheduler to handle up". Both paths now go through a sharedrun_foreground_with_uphelper.Test plan
cargo nextest run -p devenv-tasks -p devenv-processes— 249 passed (incl. newstart_with_deps_relaunches_exited_process).nix build— passes.tests/process-up-attach,tests/process-up-attach-deps,tests/process-start-shell— not yet run locally.Known follow-ups (not blocking this draft)
devenv upagainst a running manager currently streams until interrupted; gate on a TTY so scripts fail fast.devenv up <name>reports success even if the running daemon's graph lacks<name>(return/diff the scheduled set).devenv up <name>should start astart.enable = falseprocess (attach path currently force-starts it; cold start does not).devenv up <dependent>cancel the dependent (Exited@readyisNeverSatisfiable); deliberate semantics decision.🤖 Generated with Claude Code