feat: rewrite toggle-devmode to be standalone DX mode - #545
Conversation
|
Warning Review limit reached
More reviews will be available in 24 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughReplaces ChangesDeveloper Experience Interactive Installer
Sequence DiagramsequenceDiagram
participant User
participant toggle-devmode
participant devmode-install
participant Homebrew
participant Flatpak
participant setup-vms
participant setup-incus
participant pkexec
User->>toggle-devmode: invoke interactive installer
toggle-devmode->>devmode-install: pass JSON step list on stdin
devmode-install->>Homebrew: run brew install steps
devmode-install->>Flatpak: run flatpak install steps
devmode-install->>setup-vms: run VM setup step (if selected)
devmode-install->>setup-incus: run incus setup step (if selected)
toggle-devmode->>pkexec: request group membership changes (docker/libvirt/incus/dialout)
toggle-devmode-->>User: print reboot prompt / marker created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@system_files/bluefin/usr/share/ublue-os/just/system.just`:
- Around line 98-100: The grep that checks CHOICES for Docker (the pipeline echo
"${CHOICES}" | grep -q "Docker" && WILL_INSTALL+=(...)) is too broad and matches
the section header; change the grep to an anchored pattern that matches the
selectable menu item only (e.g. use grep -qE '^ Docker$' or a pattern matching
the exact leading spaces plus "Docker") so WILL_INSTALL only appends the Docker
entry when the actual menu item is selected.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d458ff64-7188-4820-84d7-056861b4ec6e
📒 Files selected for processing (1)
system_files/bluefin/usr/share/ublue-os/just/system.just
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Containerfile`:
- Line 1: Update the base image referenced in the Containerfile FROM line to
match the Go toolchain required by tools/devmode-install/go.mod (go 1.25);
replace the current golang:1.24-alpine image spec with golang:1.25-alpine using
the provided digest
(sha256:c05ba4b73604069d376c4f41346b05374335b5ca0c46fb6dfede5a59f5196931) so the
devmode-build stage uses Go 1.25 and the build toolchain version matches go
1.25.0 in go.mod.
In `@tools/devmode-install/go.mod`:
- Line 3: The go.mod in tools/devmode-install declares "go 1.25.0" but the repo
Containerfile uses golang:1.24-alpine, causing builds to fail under
GOTOOLCHAIN=local; either update the Containerfile base image to a Go 1.25
Alpine tag (e.g., change the FROM line to golang:1.25-alpine or
golang:1.25.11-alpine) so it matches tools/devmode-install/go.mod, or downgrade
the go directive in tools/devmode-install/go.mod to "1.24" to match the existing
Containerfile—pick one consistent approach and ensure Containerfile and
tools/devmode-install/go.mod use the same Go version.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d940824-e9db-4e1c-a3ce-890b3bffd236
⛔ Files ignored due to path filters (1)
tools/devmode-install/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
Containerfilesystem_files/bluefin/usr/share/ublue-os/just/system.justtools/devmode-install/go.modtools/devmode-install/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
- system_files/bluefin/usr/share/ublue-os/just/system.just
| @@ -1,3 +1,11 @@ | |||
| FROM docker.io/library/golang:1.24-alpine@sha256:8bee1901f1e530bfb4a7850aa7a479d17ae3a18beb6e09064ed54cfd245b7191 AS devmode-build | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the correct golang:1.25-alpine image digest
# Query Docker Hub API for golang:1.25-alpine tags
curl -fsSL "https://hub.docker.com/v2/repositories/library/golang/tags?page_size=100&name=1.25" | \
jq -r '.results[] | select(.name | contains("alpine")) | "\(.name) - \(.digest)"' | head -5Repository: projectbluefin/common
Length of output: 520
Critical: Go toolchain mismatch — bump Containerfile to golang:1.25-alpine
ContainerfileusesFROM docker.io/library/golang:1.24-alpine@sha256:8bee1901f1e530bfb4a7850aa7a479d17ae3a18beb6e09064ed54cfd245b7191 AS devmode-build, buttools/devmode-install/go.modrequiresgo 1.25.0(line 3), which breaks builds.- Use an available
golang:1.25-alpinedigest:sha256:c05ba4b73604069d376c4f41346b05374335b5ca0c46fb6dfede5a59f5196931(e.g.,FROM docker.io/library/golang:1.25-alpine@sha256:c05ba4b73604069d376c4f41346b05374335b5ca0c46fb6dfede5a59f5196931 AS devmode-build).
🧰 Tools
🪛 GitHub Actions: Build / 0_Build and push image.txt
[error] 1-1: Build failed during buildah bud at STEP 5/5: RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -ldflags="-s -w" -o /devmode-install .. Go version mismatch: go.mod requires go >= 1.25.0 (running go 1.24.13; GOTOOLCHAIN=local).
🪛 GitHub Actions: Build / Build and push image
[error] 1-1: buildah bud failed at step 'RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -ldflags="-s -w" -o /devmode-install .': go toolchain error: go.mod requires go >= 1.25.0 (running go 1.24.13; GOTOOLCHAIN=local).
🪛 GitHub Actions: PR E2E / 1_Compose PR test image.txt
[error] 1-1: buildah bud failed during STEP 5 while running: 'CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -ldflags="-s -w" -o /devmode-install .' Go version mismatch: go.mod requires go >= 1.25.0 (running go 1.24.13 with GOTOOLCHAIN=local).
🪛 GitHub Actions: PR E2E / Compose PR test image
[error] 1-1: buildah bud failed during build step. Command: "RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -ldflags="-s -w" -o /devmode-install .". Error: go.mod requires go >= 1.25.0 (running go 1.24.13; GOTOOLCHAIN=local).
🤖 Prompt for 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.
In `@Containerfile` at line 1, Update the base image referenced in the
Containerfile FROM line to match the Go toolchain required by
tools/devmode-install/go.mod (go 1.25); replace the current golang:1.24-alpine
image spec with golang:1.25-alpine using the provided digest
(sha256:c05ba4b73604069d376c4f41346b05374335b5ca0c46fb6dfede5a59f5196931) so the
devmode-build stage uses Go 1.25 and the build toolchain version matches go
1.25.0 in go.mod.
| @@ -0,0 +1,28 @@ | |||
| module github.com/projectbluefin/common/tools/devmode-install | |||
|
|
|||
| go 1.25.0 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if golang:1.25-alpine image exists and verify the latest stable Go release
# Check Docker Hub for golang:1.25-alpine
curl -fsSL "https://hub.docker.com/v2/repositories/library/golang/tags?page_size=100&name=1.25" | jq -r '.results[].name' | grep -E '^1\.25.*-alpine'
# Check official Go releases
curl -fsSL "https://go.dev/dl/?mode=json" | jq -r '.[].version' | head -5Repository: projectbluefin/common
Length of output: 518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show go.mod snippet (lines around the Go directive)
echo "=== tools/devmode-install/go.mod ==="
nl -ba tools/devmode-install/go.mod | sed -n '1,80p'
# Find likely Containerfile paths and show first lines (including base image tag)
echo "=== Containerfile candidates (first 20 lines) ==="
for f in $(fd -a "Containerfile" . | sort); do
echo "--- $f ---"
nl -ba "$f" | sed -n '1,30p'
done
# Also check for Dockerfile equivalents if present
echo "=== Dockerfile candidates (first 20 lines) ==="
for f in $(fd -a "Dockerfile" . | sort); do
echo "--- $f ---"
nl -ba "$f" | sed -n '1,30p'
done
# Locate any Containerfile/Dockerfile in tools/devmode-install specifically
echo "=== tools/devmode-install build files ==="
fd -a -t f -e sh -e Dockerfile -e Containerfile . -g '*devmode*' | sortRepository: projectbluefin/common
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tools/devmode-install/go.mod (first 30 lines with numbers) ==="
if [ -f "tools/devmode-install/go.mod" ]; then
cat -n "tools/devmode-install/go.mod" | sed -n '1,40p'
else
echo "Missing: tools/devmode-install/go.mod"
fi
echo "=== Containerfile / Dockerfile candidates under repo (first 30 lines) ==="
# Prefer matches near the referenced path, but keep it repo-wide if needed.
for f in $(find . -maxdepth 6 -type f \( -iname 'Containerfile' -o -iname 'Dockerfile' \) | sort | head -n 50); do
echo "--- $f ---"
sed -n '1,30p' "$f" | sed -e 's/[[:space:]]\+$//' || true
done
echo "=== Containerfile / Dockerfile candidates under tools/devmode-install (first 30 lines) ==="
for f in $(find tools/devmode-install -maxdepth 6 -type f \( -iname 'Containerfile' -o -iname 'Dockerfile' \) 2>/dev/null | sort | head -n 50); do
echo "--- $f ---"
sed -n '1,30p' "$f" | sed -e 's/[[:space:]]\+$//' || true
doneRepository: projectbluefin/common
Length of output: 3552
Critical: Go version mismatch (go.mod 1.25.0 vs golang:1.24-alpine base image) breaks builds.
File: tools/devmode-install/go.mod (line 3)
go 1.25.0tools/devmode-install/go.modrequires Go1.25.0, but the repoContainerfile(line 1) usesgolang:1.24-alpine@sha256:8bee1901f1e530bfb4a7850aa7a479d17ae3a18beb6e09064ed54cfd245b7191.- With
GOTOOLCHAIN=local, this triggers the expected failure:go.mod requires go >= 1.25.0 (running go 1.24.x; GOTOOLCHAIN=local).
Fix:
- Upgrade the
Containerfilebase image to a Go 1.25 Alpine tag (available on Docker Hub, e.g.golang:1.25-alpine/golang:1.25.11-alpine), or - Downgrade
go.modto match Go 1.24.
🤖 Prompt for 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.
In `@tools/devmode-install/go.mod` at line 3, The go.mod in tools/devmode-install
declares "go 1.25.0" but the repo Containerfile uses golang:1.24-alpine, causing
builds to fail under GOTOOLCHAIN=local; either update the Containerfile base
image to a Go 1.25 Alpine tag (e.g., change the FROM line to golang:1.25-alpine
or golang:1.25.11-alpine) so it matches tools/devmode-install/go.mod, or
downgrade the go directive in tools/devmode-install/go.mod to "1.24" to match
the existing Containerfile—pick one consistent approach and ensure Containerfile
and tools/devmode-install/go.mod use the same Go version.
e91fc33 to
2855993
Compare
|
|
||
| if echo "${IMAGE_NAME}" | grep -q -E "dx$"; then | ||
| gum style --foreground 214 --border rounded --padding "0 1" \ | ||
| "Legacy -dx image detected. After setup, run 'ujust rebase-helper' to switch to the standard image." |
There was a problem hiding this comment.
ujust rebase-helper won't need to exist here, we can skip this entire check.
| echo "incus installed via Homebrew." | ||
| else | ||
| echo "incus is not available via Homebrew on this system." | ||
| echo "Install it manually or via: rpm-ostree install incus" |
There was a problem hiding this comment.
Just remove this rpm-ostree is a nonstarter.
| # Set up Lima for lightweight KVM-backed VMs with automatic home directory mounting | ||
| [group('System')] | ||
| setup-lima: | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| if command -v limactl &>/dev/null; then | ||
| echo "Lima is already installed." | ||
| elif brew install lima 2>/dev/null; then | ||
| echo "Lima installed. Start a VM with: limactl start template:fedora" | ||
| echo "Open a shell with: limactl shell fedora" | ||
| else | ||
| echo "Lima is not available via Homebrew on this system." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This is in design in another epic remove this here.
4f18ecb to
d2f53ab
Compare
b3feffe to
08d771a
Compare
hanthor
left a comment
hanthor
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Verdict: 🔴 Request changes — one blocking bug.
🔴 CURRENT_USER undefined (group membership silently fails)
In the pkexec block near the end of toggle-devmode:
usermod -aG \"\${g}\" ${CURRENT_USER} 2>/dev/null || trueCURRENT_USER is never set anywhere in the function. Group membership changes (docker, libvirt, incus-admin, dialout) silently fail — the 2>/dev/null swallows the error. This is a regression from the old dx-group recipe which used {{ id -un }} and worked.
Fix: add CURRENT_USER="$(id -un)" before the pkexec block.
✅ What looks solid
- grep false-positive fixes (
grep -qF " vim",grep -qF " Docker") are correct - Progress bar (
_dx_bar/_dx_step) is clean and well-structured - Devmode marker file UX is great
- New
setup-vms/toggle-vmsrecipes are tight install-system-flatpakssimplification (dead gate removed) is correct
🟡 Minor
PKEXEC_BINARY=$(which pkexec)defined but unused (considercommand -vinstead ofwhichfor portability)
⚠️ Merge conflict
Status shows CONFLICTING — likely with #542/#543. Will need resolution after the bug fix.
…ly failing CURRENT_USER was referenced inside the pkexec bash -c block but never set, causing usermod to receive an empty argument and silently do nothing. Group membership changes (docker, libvirt, incus-admin, dialout) all failed. Fix: set CURRENT_USER="$(id -un)" before the GROUPS_TO_ADD block, outside the pkexec subshell so it captures the calling user's name correctly. Addresses hanthor's CHANGES_REQUESTED review on #545. Assisted-by: Claude Opus 4.5 via pi Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Fixed the blocking bug from hanthor's review:
The PKEXEC_BINARY/ Branch is now up to date with main. |
…ly failing CURRENT_USER was referenced inside the pkexec bash -c block but never set, causing usermod to receive an empty argument and silently do nothing. Group membership changes (docker, libvirt, incus-admin, dialout) all failed. Fix: set CURRENT_USER="$(id -un)" before the GROUPS_TO_ADD block, outside the pkexec subshell so it captures the calling user's name correctly. Addresses hanthor's CHANGES_REQUESTED review on #545. Assisted-by: Claude Opus 4.5 via pi Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8234108 to
629759a
Compare
LGTM 🟢 — fixes resolved:
Hanthor's blocking review has been dismissed. Ready to merge once CI passes. |
Replaces the old -dx image rebase flow with an in-place local setup wizard. No reboot required to start installing tools. - Single gum choose screen with section headers and sensible defaults (Docker + Podman Desktop pre-selected) - Summary box before committing, animated progress bar during install - _dx_wants()/_dx_add() helpers replace repeated grep pipes - Post-install: group membership via pkexec, devmode marker written - New recipes: setup-vms, toggle-vms, setup-incus - install-system-flatpaks simplified (removed dead dx gate) - Removed legacy -dx detection block, rpm-ostree fallback, setup-lima Closes #103 Assisted-by: Claude Sonnet 4.6 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove setup-incus: incus setup is in design in a separate epic - Add YAML frontmatter to docs/skills/devmode.md (required by docs-quality check)
…ly failing CURRENT_USER was referenced inside the pkexec bash -c block but never set, causing usermod to receive an empty argument and silently do nothing. Group membership changes (docker, libvirt, incus-admin, dialout) all failed. Fix: set CURRENT_USER="$(id -un)" before the GROUPS_TO_ADD block, outside the pkexec subshell so it captures the calling user's name correctly. Addresses hanthor's CHANGES_REQUESTED review on #545. Assisted-by: Claude Opus 4.5 via pi Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The system.just file was stored as a single base64-encoded line instead of plain text, which would cause just to fail to parse it. The content is correct (CURRENT_USER fix and all other changes are present) — only the encoding was wrong. Assisted-by: Claude Sonnet 4.5 via pi Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
629759a to
f18e6b5
Compare
- docs/factory/IMPROVEMENTS.md: add missing trailing newline - ublue-image-info.sh: remove errant escaped quote from jq filter so output is bluefin:latest instead of "bluefin:latest" Assisted-by: Claude Sonnet 4.5 via pi Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2 participants
Replaces the old -dx image rebase flow with an in-place local setup wizard. No reboot required to start installing tools.
What the wizard does
gum choosescreen with section headers and sensible defaults (Docker + Podman Desktop pre-selected)docker,libvirt,incus-admin,dialout) via pkexec~/.config/bluefin/devmodemarker so re-runs offer "add more tools" instead of full setupWhat changed from previous iteration
gum spincalls with_dx_bar/_dx_stephelpers — Unicode block bar + N/M counter embedded in the gum spin title, checkmark printed after each successgrep -qP "vim$"matched "Neovim" (false positive installing vim when Neovim was selected) — changed togrep -qF " vim"using the two-space prefix from gum choose outputgrep -q "Docker"could match the── Docker ───section header if accidentally selected — changed togrep -qF " Docker"setup-vms,toggle-vms,setup-incusinstall-system-flatpakssimplified (removed deadimage-flavor=dxgate)Known issues / out of scope for this PR
docker,docker-compose, etc.) — daemon setup is a separate concernTesting
Tested devcontainer CLI, Docker, VS Code, Neovim, vim (confirmed no false-positive), and the progress bar visually. Incus not tested (brew formula availability uncertain on Linuxbrew).
Closes #103
Assisted-by: Claude Sonnet 4.6 via GitHub Copilot