Skip to content

feat(supervisor): allow disabling client auto-start - #678

Merged
jdx merged 3 commits into
jdx:mainfrom
risu729:agent/add-supervisor-auto-start-setting
Jul 28, 2026
Merged

feat(supervisor): allow disabling client auto-start#678
jdx merged 3 commits into
jdx:mainfrom
risu729:agent/add-supervisor-auto-start-setting

Conversation

@risu729

@risu729 risu729 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add supervisor.auto_start with a backwards-compatible default of true
  • prevent implicit client launches when the setting is disabled while leaving explicit supervisor commands unchanged
  • provide actionable connection errors for service-managed installations
  • expose the setting through the settings CLI, environment variables, schema, and documentation

Root cause

Clients currently call start_if_not_running() before connecting to IPC. During
system startup, a shell hook or another client can therefore spawn an unmanaged
supervisor before a systemd- or launchd-managed instance is ready. The unmanaged
process then wins ownership of the supervisor state and socket but lacks any
capabilities configured on the service.

Validation

  • mise run ci-dev
    • 553 Rust tests passed
    • 308 Bats tests passed

Summary by CodeRabbit

  • New Features
    • Added a settings.supervisor.auto_start option to control whether client commands automatically start the supervisor.
    • Added PITCHFORK_SUPERVISOR_AUTO_START support for configuring this behavior via environment variable.
    • Explicit supervisor start and run commands remain available regardless of the setting.
  • Bug Fixes
    • When auto-start is disabled, client commands no longer launch an unmanaged supervisor and instead provide actionable connection guidance.
  • Documentation
    • Updated the boot/start guide and environment variable docs to clarify behavior with external service managers.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f77098b-4d29-4bee-8ebe-66f955c09817

📥 Commits

Reviewing files that changed from the base of the PR and between e6da927 and a151932.

📒 Files selected for processing (2)
  • docs/guides/boot-start.md
  • test/errors.bats
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/errors.bats

📝 Walkthrough

Walkthrough

Adds supervisor.auto_start, configurable through TOML, environment variables, and CLI settings commands. IPC clients now honor the setting before starting a fallback supervisor and provide conditional connection guidance. Tests, schema definitions, and user documentation cover the behavior.

Changes

Supervisor auto-start control

Layer / File(s) Summary
Settings contract and persistence
settings.toml, src/cli/settings.rs, src/settings.rs, test/settings.bats
Defines supervisor.auto_start, supports CLI get/set operations, and tests default, environment, TOML, and persistence behavior.
IPC auto-start and connection guidance
src/ipc/client.rs, test/errors.bats
Prevents fallback supervisor startup when disabled and updates connection errors with conditional instructions.
Configuration and usage documentation
docs/public/schema.json, docs/reference/environment-vars.md, docs/guides/boot-start.md
Documents the schema property, environment variable, and managed-supervisor configuration.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant IpcClient
  participant Settings
  participant Supervisor
  Client->>IpcClient: Request connection
  IpcClient->>Settings: Read supervisor.auto_start
  alt Enabled
    IpcClient->>Supervisor: Start if not running
    Supervisor-->>IpcClient: Connection available
  else Disabled
    IpcClient-->>Client: Return actionable connection error
  end
Loading

Possibly related PRs

  • jdx/pitchfork#606: Both changes modify the supervisor connection flow in src/ipc/client.rs.
  • jdx/pitchfork#616: Both changes update supervisor-start guidance in IPC connection errors.

Poem

A rabbit toggled one small key,
And stopped stray supervisors free.
Managed daemons now stay in line,
While helpful errors gently shine.
The burrow boots just when decreed. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a way to disable client auto-start for the supervisor.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds an opt-out for automatic supervisor startup while preserving explicit supervisor commands.

  • Introduces the backwards-compatible supervisor.auto_start setting with configuration-file, environment-variable, settings CLI, and schema support.
  • Prevents IPC clients from implicitly launching the supervisor when the setting is disabled.
  • Adds service-manager-oriented connection guidance and documents the new behavior.
  • Adds settings and end-to-end error-path coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/ipc/client.rs Gates implicit supervisor startup on the new setting and selects actionable connection guidance without changing explicit startup paths.
settings.toml Defines the boolean supervisor auto-start setting, its compatible default, environment variable, and generated documentation.
src/cli/settings.rs Integrates the new field with settings CLI get and set operations.
src/settings.rs Verifies default, environment-variable, and configuration-file loading for the generated setting.
test/errors.bats Covers the disabled auto-start connection failure and its service-manager guidance.
test/settings.bats Covers persistence and retrieval of the setting through the CLI.

Reviews (3): Last reviewed commit: "Merge branch 'main' into agent/add-super..." | Re-trigger Greptile

@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: 1

🤖 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 `@docs/guides/boot-start.md`:
- Around line 83-91: Update the global configuration example in “Prevent
fallback supervisor starts” to use the top-level [supervisor] table instead of
the project-specific [settings.supervisor] namespace, while preserving
auto_start = false.
🪄 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: dac1423a-fa81-4d0d-a580-20102661d245

📥 Commits

Reviewing files that changed from the base of the PR and between 04714cc and e6da927.

📒 Files selected for processing (9)
  • docs/guides/boot-start.md
  • docs/public/schema.json
  • docs/reference/environment-vars.md
  • settings.toml
  • src/cli/settings.rs
  • src/ipc/client.rs
  • src/settings.rs
  • test/errors.bats
  • test/settings.bats

Comment thread docs/guides/boot-start.md
@risu729
risu729 marked this pull request as ready for review July 28, 2026 01:17
@jdx
jdx merged commit 40068b6 into jdx:main Jul 28, 2026
14 checks passed
@risu729
risu729 deleted the agent/add-supervisor-auto-start-setting branch July 28, 2026 01:32
@jdx jdx mentioned this pull request Jul 28, 2026
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