Skip to content

Always run full install on finalrun upgrade - #132

Merged
droid-ash merged 2 commits into
mainfrom
claude/busy-wescoff-63e1b4
Apr 28, 2026
Merged

droid-ash merged 2 commits into
mainfrom
claude/busy-wescoff-63e1b4

Conversation

@droid-ash

@droid-ash droid-ash commented Apr 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • finalrun upgrade previously auto-detected --ci (binary-only) mode when ~/.finalrun/runtime/<version>/manifest.json wasn't on disk, which surfaced a confusing (binary-only (--ci)) label in the upgrade output for users who never explicitly opted into binary-only.
  • Drops the auto-detection AND the --ci flag from the upgrade subcommand. finalrun upgrade now always re-runs the full install script — same defaults as a fresh install.
  • Users who want a binary-only refresh can invoke install.sh directly with --ci, or set CI=1 / FINALRUN_NON_INTERACTIVE=1 — install.sh already honors both env vars (CI providers all set CI=1 so CI runners stay binary-only without needing the upgrade shim to second-guess them).

Why this is safe for users currently on binary-only

The full install path is idempotent — it checks for an existing runtime before downloading, maybe_skip_platform_setup() skips the Android/iOS prompt when host tools are already detected, the platform prompt has a 30s timeout (won't hang), and skills update is a no-op when up-to-date. So a user who originally chose --ci will, on their next finalrun upgrade, just gain the runtime tarball + (optionally) host-tool setup — no destructive behavior.

Behavior diff

Scenario Before After
finalrun upgrade with no runtime on disk silently became --ci runs full install
finalrun upgrade with runtime on disk full install full install (unchanged)
finalrun upgrade --ci binary-only flag removed (use install.sh | bash -s -- --ci instead)
CI=1 finalrun upgrade binary-only via shim binary-only via install.sh's CI auto-detect
finalrun upgrade --version 0.1.10 mode auto-detected full install pinned to 0.1.10

Test plan

  • finalrun upgrade on a machine without the runtime tarball → installs runtime, prompts for platform, runs doctor
  • finalrun upgrade on a machine with the runtime → unchanged from current behavior
  • finalrun upgrade --version 0.1.12 → pins installer to v0.1.12 via FINALRUN_VERSION env var
  • CI=1 finalrun upgrade → install.sh enters CI mode on its own, binary-only path
  • finalrun upgrade --ci → fails with "unknown option" (flag removed)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Removed the --ci command-line flag from the upgrade command. The upgrade process now uses the installer's default behavior for all upgrade operations, eliminating conditional binary-only installation modes.

Previously the upgrade subcommand auto-detected `--ci` (binary-only)
mode when the runtime tarball wasn't on disk, which surfaced a
confusing `(binary-only (--ci))` label in the upgrade output for
users who never explicitly opted into binary-only.

Drop the auto-detection and the `--ci` flag from `finalrun upgrade`
entirely. Upgrade now always re-runs the full install script. Users
who want a binary-only refresh can invoke install.sh directly with
`--ci`, or set `CI=1` / `FINALRUN_NON_INTERACTIVE=1` — install.sh
already honors those env vars.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 28, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@droid-ash has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 14 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6e565702-7978-4989-b604-81c21bbe010e

📥 Commits

Reviewing files that changed from the base of the PR and between d56476e and 1145e26.

📒 Files selected for processing (1)
  • packages/cli/src/upgradeCommand.ts
📝 Walkthrough

Walkthrough

This PR removes the --ci command-line flag from the upgrade command and eliminates the logic that auto-detected runtime installation to conditionally enable binary-only mode. The upgrade process now always uses the installer's default behavior without platform-specific or installation-state-aware variations.

Changes

Cohort / File(s) Summary
CLI flag removal
packages/cli/bin/finalrun.ts
Removes the --ci flag from the finalrun upgrade command definition and stops passing ci value to runUpgrade.
Upgrade command simplification
packages/cli/src/upgradeCommand.ts
Eliminates auto-detection logic for runtime tarball installation and the conditional --ci appending. Simplifies installer invocation to unconditional bash pipeline; removes related helper function and filesystem/os/path imports. Updates UpgradeOptions interface to remove ci?: boolean field.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • arnoldlaishram
  • srinidhi-lwt

Poem

🐰 No more binary modes, just simple and clean,
The upgrade path's now sleek, if you know what I mean,
We hopped away flags with a swift little bound,
The installer runs free—default behavior's profound! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Always run full install on finalrun upgrade' directly and accurately summarizes the main change: removing auto-detection of binary-only mode so that the upgrade command always performs a full install.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/busy-wescoff-63e1b4

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 and usage tips.

@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 the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/upgradeCommand.ts`:
- Around line 8-10: The upgrade command currently strips all FINALRUN_* env vars
except FINALRUN_DIR/FINALRUN_VERSION before spawning the installer, so
FINALRUN_NON_INTERACTIVE=1 is lost; update the env-filtering logic used in the
upgrade flow (where you build the spawn env for the installer) to preserve
FINALRUN_NON_INTERACTIVE (or translate it to set CI=1 in the child env) so that
running `FINALRUN_NON_INTERACTIVE=1 finalrun upgrade` behaves like the
documentation; adjust the code that constructs the filteredEnv/childEnv in
upgradeCommand.ts (and the similar block around lines 36-44) to include
FINALRUN_NON_INTERACTIVE (or add CI=1 when present).
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 54490f95-bec1-4c64-90c7-e6d100f915c0

📥 Commits

Reviewing files that changed from the base of the PR and between 8555295 and d56476e.

📒 Files selected for processing (2)
  • packages/cli/bin/finalrun.ts
  • packages/cli/src/upgradeCommand.ts
💤 Files with no reviewable changes (1)
  • packages/cli/bin/finalrun.ts

Comment thread packages/cli/src/upgradeCommand.ts
The upgrade command strips all FINALRUN_* env vars before spawning
the installer to keep this binary's debugging overrides from leaking
into the next version's install location. But the documentation and
PR description both said `FINALRUN_NON_INTERACTIVE=1 finalrun upgrade`
is the way to opt into binary-only mode — and the scrub silently
dropped that var, so the documented escape hatch didn't work.

Carry FINALRUN_NON_INTERACTIVE over alongside FINALRUN_DIR. install.sh
and install.ps1 both already honor it (CI=1 still works too — it
travels through unchanged because the scrub only targets FINALRUN_*).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@droid-ash droid-ash self-assigned this Apr 28, 2026
@droid-ash droid-ash added the bug Something isn't working label Apr 28, 2026
@droid-ash
droid-ash merged commit 0b1b6cb into main Apr 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant