Skip to content

Review the whole setup change set with one confirmation - #110

Merged
lawrencecchen merged 1 commit into
mainfrom
feat/setup-review-screen
Jul 30, 2026
Merged

lawrencecchen merged 1 commit into
mainfrom
feat/setup-review-screen

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

What changes

sr setup installed and started the daemon with no reviewable plan and only --no-install. It now shows the complete change set and applies it on one Enter press.

Subrouter is ready to set up this computer.

  = Run Subrouter in the background after you sign in (LaunchAgent) (already done)
  = Configure Codex to route through Subrouter (already done)
  + Configure Claude Code to route through Subrouter

Press Enter to apply.
[d] details   [e] edit options   [Ctrl-C] cancel

That is real output from sr setup --plan on a configured machine: already-satisfied work is listed and marked rather than hidden, so re-running is safe and the screen still shows the whole picture.

Why one confirmation instead of per-item prompts

A question per item produces half-installed machines. A user who declines background startup individually gets a daemon that stops working at the next reboot, in a state we neither test nor support. Enter-to-apply is already opt-in because nothing changes until the key is pressed, and e keeps the uncommon choices one keystroke away.

Flags

  • --plan prints the change set and exits, and returns before the login and storage writes so it genuinely changes nothing
  • --yes applies without the review screen
  • --no-background, --no-config deselect without needing the screen

A noninteractive terminal without --yes prints the plan and exits. EOF during review cancels. Silence is not consent.

Terminal detection uses term.IsTerminal rather than a character-device check: /dev/null is a character device, and sr setup < /dev/null previously entered the interactive path and read approval from EOF. Verified both < /dev/null and a real pipe now report noninteractive terminal: re-run with --yes to apply.

Two things running it surfaced

The credential line only appears when a vault exists to hand accounts to. Previously a machine with 16 local accounts and no vault rendered a deselectable - Keep 16 existing account(s) on this machine, which implies setup might delete them when there was nothing to decide.

That line states its consequence, "stops refreshing them here", rather than calling it adoption. Handing a credential to the vault is what stops two hosts from invalidating each other's refresh tokens (see #108), so a user who reads it as a harmless copy will make the wrong choice.

Tests

Enter applies; EOF cancels; d shows the exact Codex change without applying; e toggles one option without disturbing others; an out-of-range choice is rejected and changes nothing; flags deselect; already-satisfied work is never re-applied; a fully-configured machine says "Nothing to do"; the credential item names the vault and its consequence, and is absent without one; each platform names its own mechanism; and clientRoutesToLocal reads disk rather than trusting a flag, including port mismatch and missing file.

go test ./cmd/... ./internal/... passes.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

sr setup now shows a single, reviewable change set and applies everything on one Enter, preventing partial installs and making re-runs safe. Noninteractive runs print the plan and exit unless --yes is provided.

  • New Features
    • Interactive review screen: shows all actions with status markers, Enter to apply; d for details; e to toggle uncommon options; EOF cancels.
    • Flags: --plan (print and exit), --yes (apply without review), --no-background, --no-config.
    • Noninteractive safety: detects TTY with term.IsTerminal; prints plan and “noninteractive terminal: re-run with --yes to apply” instead of applying.
    • Credentials: only shows the vault option when a team vault is ready, and clearly states it “stops refreshing them here.”
    • Platform clarity: background startup names the mechanism per OS (LaunchAgent, systemd user service, scheduled task).

Written for commit f10f614. Summary will update on new commits.

Review in cubic

sr setup installed and started the daemon with no reviewable plan and only a
--no-install escape hatch. It now shows the complete change set and applies it on
one Enter press, with d for exact diffs and e for the uncommon toggles.

One transaction-level confirmation rather than a question per item: a user who
declines background startup individually gets a daemon that stops working at the
next reboot, which is a state we neither test nor support. Enter-to-apply is
already opt-in because nothing changes until the key is pressed.

Also adds --plan, --yes, --no-background and --no-config. A noninteractive
terminal without --yes prints the plan and exits without touching anything, and
EOF during review cancels: silence is not consent. Terminal detection uses
term.IsTerminal rather than a character-device check, because /dev/null is a
character device and must not be read as approval.

Two things running it surfaced. The credential line only appears when a vault
exists to hand accounts to; a deselectable "keep 16 accounts" line implies setup
might delete them. And it states the consequence ("stops refreshing them here")
rather than calling it adoption, since that ownership transfer is what stops two
hosts from invalidating each other's refresh tokens.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@lawrencecchen, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 233cba88-0ada-4214-8f2b-7ce41fc26e47

📥 Commits

Reviewing files that changed from the base of the PR and between 96ebb68 and f10f614.

📒 Files selected for processing (4)
  • cmd/subrouter/setup_plan.go
  • cmd/subrouter/setup_plan_test.go
  • cmd/subrouter/sr_cloud.go
  • cmd/subrouter/sr_setup.go

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.

@lawrencecchen
lawrencecchen merged commit 609c2f8 into main Jul 30, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the feat/setup-review-screen branch July 30, 2026 00:44
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.

1 participant