Skip to content

feat(processes): revert attachable devenv up to simplify#2912

Open
omarcinkonis wants to merge 3 commits into
cachix:process-start-shellfrom
omarcinkonis:process-start-shell
Open

feat(processes): revert attachable devenv up to simplify#2912
omarcinkonis wants to merge 3 commits into
cachix:process-start-shellfrom
omarcinkonis:process-start-shell

Conversation

@omarcinkonis

Copy link
Copy Markdown

Please review this simplification related to my comment #2890 (comment)

@omarcinkonis

Copy link
Copy Markdown
Author

Summary

Adds processes.<name>.start.enable = "interactive-shell" (and a global process.start default): such processes start when you enter an interactive devenv shell and stop when you exit, replacing the devenv up -d && devenv shell two-step plus the matching exit + devenv processes down.

Only the global process.start = "interactive-shell" is implemented. Per-process start.enable = "interactive-shell" is not.

devenv up now attaches to an already-running native manager (from devenv up -d or devenv 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 new ApiRequest::Up forwarded to Tasks::start_with_deps, so out-of-subset and already-running dependencies resolve against the live task graph exactly like a cold start.

Not implemented. devenv up still fails with "Processes already running" when a manager is running.

The interactive shell status line shows a live count of running processes (e.g. watching 5 files | 3 processes).

Still included.

Native process manager only.

Fixes #2863.

What's here

  • start.enable accepts "interactive-shell"; new process.start global default; assertion requiring the native manager.

The global process.start option and the assertion are included. Per-process start.enable = "interactive-shell" is not.

  • devenv up attaches via ApiRequest::Up → daemon scheduler (start_with_deps); honours the positional subset and dependency ordering.

Not implemented.

  • --background daemon mode writes the PID file early so shell entry isn't blocked on process readiness.
  • Live process-count status line.

Fixes folded in (from self-review)

  • Relaunch a self-exited/crashed process on attaching devenv up. Such a process stays Active with a terminal phase, which rearm_waiting refused to touch, so it was silently never relaunched. start_with_deps now normalizes it via stop_and_keep first. (+ regression test)
  • Register the Up handler before the daemon serves requests. It was set only after every auto-started process became ready, and the foreground in-process up never set it at all, so an attaching devenv up during startup (or to a foreground up) was rejected with "no process scheduler to handle up". Both paths now go through a shared run_foreground_with_up helper.

Both depend on the attach feature, so neither is included.

Test plan

  • cargo nextest run -p devenv-tasks -p devenv-processes — 249 passed (incl. new start_with_deps_relaunches_exited_process).
  • nix build — passes.
  • Integration tests tests/process-up-attach, tests/process-up-attach-deps, tests/process-start-shell — not yet run locally.

start_with_deps_relaunches_exited_process, tests/process-up-attach, and tests/process-up-attach-deps are all removed. tests/process-start-shell and tests/process-start-shell-non-native are included.

Known follow-ups (not blocking this draft)

  • Non-interactive foreground devenv up against a running manager currently streams until interrupted; gate on a TTY so scripts fail fast.

Done.

  • devenv up <name> reports success even if the running daemon's graph lacks <name> (return/diff the scheduled set).
  • Decide whether an explicit devenv up <name> should start a start.enable = false process (attach path currently force-starts it; cold start does not).
  • A self-exited dependency makes devenv up <dependent> cancel the dependent (Exited@ready is NeverSatisfiable); deliberate semantics decision.

All three depend on the attach feature and no longer apply.

@omarcinkonis

Copy link
Copy Markdown
Author

Commit summary

Replaces processes.<name>.start.enable = "interactive-shell" with two independent booleans: start.up (default true, controls devenv up) and start.shell (default false, controls interactive shell entry). Global defaults are process.start.up and process.start.shell. start.enable is kept as a deprecated alias that forwards to start.up with a warning. The mixed bool/enum type is gone.

start.shell = true processes start when you enter an interactive devenv shell and stop when you exit. Non-interactive shells (devenv shell -- cmd, piped, --no-reload) warn and skip them. Native process manager only.

What's here

  • start.up and start.shell per-process flags with matching global defaults under process.start.
  • start.enable compat shim: forwards to start.up with a deprecation warning when explicitly set.
  • Assertion that start.shell = true requires the native process manager.
  • Shell entry starts a daemon if none is running and stops it on exit. If a daemon is already running, it starts the shell processes against it and stops only those on exit.
  • processes_running() extended to verify socket connectivity, not just PID file presence, so shell entry doesn't race the daemon's bind step.
  • process.shellStartProcesses Nix output lists the start.shell = true process names, consumed by the Rust shell-entry path.

Test plan

  • cargo check -p devenv-processes -p devenv-tasks: clean.
  • cargo clippy -p devenv-processes -p devenv-tasks: clean (pre-existing warnings in unrelated crates).
  • cargo nextest run -p devenv-processes -p devenv-tasks: all passed.
  • Integration tests process-start-shell, process-start-shell-non-native, process-compose-merge, processes: updated, not yet run locally.

@domenkozar

Copy link
Copy Markdown
Member

The reason I called it interactive-shell is because it only launches for devenv shell, but not devenv shell foo

@domenkozar

Copy link
Copy Markdown
Member

Note that this yields to weird UX: when processes are started with devenv shell, then running devenv up tell you you have to first stop old processes instead of attaching.

@omarcinkonis

Copy link
Copy Markdown
Author

The reason I called it interactive-shell is because it only launches for devenv shell, but not devenv shell foo

How about renaming start.shell -> start.interactive? Or start.interactive-shell?

Note that this yields to weird UX: when processes are started with devenv shell, then running devenv up tell you you have to first stop old processes instead of attaching.

The least complex solution I see is:

  • user starts devenv shell
  • user starts devenv up
  • we prompt the user if they want to restart the processes
  • if yes, we do it automatically, instead of a warning with manual steps

What do you think?

I invited you as a collaborator in case you'd like to make any changes yourself.

@omarcinkonis

omarcinkonis commented Jun 8, 2026

Copy link
Copy Markdown
Author

Regarding the naming, I believe that clarity is more important than covering all edge cases. I think it's best to keep start.shell because the user immediately sees the command it refers to (same as start.up).

Regarding devenv shell foo, this edge case is already covered by a warning: "Not starting start.shell = true process(es) [{}]: this is not an interactive devenv shell. Start them with devenv up.".

Adding an interactive prompt was more difficult than I expected, so I added a devenv up --restart flag instead.

@domenkozar

Copy link
Copy Markdown
Member

Regarding the naming, I believe that clarity is more important than covering all edge cases. I think it's best to keep start.shell because the user immediately sees the command it refers to (same as start.up).

Regarding devenv shell foo, this edge case is already covered by a warning: "Not starting start.shell = true process(es) [{}]: this is not an interactive devenv shell. Start them with devenv up.".

Adding an interactive prompt was more difficult than I expected, so I added a devenv up --restart flag instead.

Could you come to discord and we can discuss it there in details. Thanks a lot to your feedback!

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