Skip to content

fix(lifecycle): runnable uv install one-liner when maybe_migrate_to_uv hits uv-not-found - #774

Merged
github-actions[bot] merged 3 commits into
mainfrom
fix/uvless-migration-reason-string
May 14, 2026
Merged

fix(lifecycle): runnable uv install one-liner when maybe_migrate_to_uv hits uv-not-found#774
github-actions[bot] merged 3 commits into
mainfrom
fix/uvless-migration-reason-string

Conversation

@robotrocketscience

Copy link
Copy Markdown
Owner

Summary

  • v3.0.1 added maybe_migrate_to_uv() (Auto-migrate non-uv aelfrice installs to uv tool on first 3.0.1 setup #733) to auto-migrate pipx/pip installs to uv tool install aelfrice on first aelf setup. The migration is gated on shutil.which("uv"); a pipx-only user without uv hits the skip path.
  • The skip-reason previously pointed at the docs URL only — no executable suggestion. This PR surfaces both Astral curl and brew install lines verbatim so the operator can copy-paste one and rerun.

Why not auto-install uv?

Bootstrapping a third-party installer unprompted (curl | sh from inside our migration path) has a much larger blast radius than what a memory tool should do without user consent. The docs/INSTALL.md Prerequisites section already documents uv as required up front. This PR only sharpens the runtime error path for users who skipped that.

What changes

  • src/aelfrice/lifecycle.py: reason string for the shutil.which("uv") is None branch now names both install channels.
  • tests/test_migrate_to_uv.py: assert both forms appear in result.reason.
  • CHANGELOG.md: [Unreleased] / Fixed bullet describing the change and the rationale.

No behaviour change beyond the operator-facing text.

Test plan

  • uv run pytest tests/test_migrate_to_uv.py tests/test_setup_migrate_order.py tests/test_cli_setup.py tests/test_lifecycle.py → 74 passed, 2 skipped
  • Bench-smoke / full pytest matrix runs in CI on this PR

Out of scope

  • Auto-installing uv (see "Why not" above).
  • Reflowing INSTALL.md — Prerequisites already documents uv up front and the migration callout is already present.

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 10 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: b594a724-ada5-4545-9878-1a7dcb9eeb1c

📥 Commits

Reviewing files that changed from the base of the PR and between e7a521c and fb3fb3c.

⛔ Files ignored due to path filters (1)
  • CHANGELOG.md is excluded by !**/CHANGELOG.md
📒 Files selected for processing (2)
  • src/aelfrice/lifecycle.py
  • tests/test_migrate_to_uv.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/uvless-migration-reason-string

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.

@sourcery-ai sourcery-ai 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.

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:fermat:2026-05-14T04:33:12Z]

…te_to_uv reason

When a pipx- or pip-installed user hits the migration path without uv on
PATH, the prior reason text pointed at the docs URL only. Operators reading
a single stderr line want a command they can paste — surface both the
Astral curl install and the brew alternative inline. Doc URL kept for the
'I want to read first' path.
…reason

Pins both halves of the surfaced suggestion — `curl -LsSf
https://astral.sh/uv/install.sh | sh` and `brew install uv` — so a future
edit that drops either form fails the test rather than silently regressing
the operator-facing message.
…to_uv reason

Lands under [Unreleased] / Fixed as a follow-up to #733. Describes the
operator-facing message change, why uv is still not auto-installed
(third-party bootstrapping unprompted is out of scope), and points to
docs/INSTALL.md Prerequisites for the up-front documentation.
@robotrocketscience
robotrocketscience force-pushed the fix/uvless-migration-reason-string branch from 7a079a6 to fb3fb3c Compare May 14, 2026 04:33
@robotrocketscience

Copy link
Copy Markdown
Owner Author

Rebased onto github/main (3 atomic commits, all signed G: feat → test → docs). Verified:

  • Diff: maybe_migrate_to_uv reason string adds runnable curl … | sh and brew install uv lines; test asserts both forms present in result.reason; CHANGELOG [Unreleased] bullet added.
  • No behavior change beyond operator-facing text. The shutil.which("uv") is None skip path is unchanged structurally; only the reason field differs.
  • Bootstrap-via-curl pattern is in the message, not auto-invoked — addresses the original gripe without inheriting the auto-install blast-radius.
  • Atomic commit ordering correct (feature → test → changelog).
  • All required checks green after rebase (pytest 3.12/3.13, secrets-scan, pattern-scan, history-scan, CodeQL, deptry, vulture, typos, commit-msg-prefix, pr-title-prefix, pr-body-issue-link, bench-smoke).
  • Discretion grep on diff vs main: clean.

Labeling ready-to-merge.

@robotrocketscience robotrocketscience added the ready-to-merge Trigger merge-train: FF main to this PR's head label May 14, 2026
@github-actions
github-actions Bot merged commit fb3fb3c into main May 14, 2026
27 checks passed
@github-actions github-actions Bot removed the ready-to-merge Trigger merge-train: FF main to this PR's head label May 14, 2026
@github-actions

Copy link
Copy Markdown

merge-train: merged fb3fb3cmain via FF push.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:fermat:2026-05-14T04:38:23Z]

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