Add backup/rollback support and structured logging#34
Merged
fullstackjam merged 8 commits intomainfrom Apr 19, 2026
Merged
Conversation
Introduces `openboot update` subcommand with three modes beyond the default latest-version upgrade: - `--version X.Y.Z` pins a specific release (URL-resolved, SHA-256 verified) - `--rollback` restores the newest pre-upgrade backup - `--list-backups` prints the backup directory Each direct-download upgrade now copies the running binary to ~/.openboot/backup/ before atomic rename; backupRetention=5. Homebrew- managed installs reject --version / --rollback with a clear hint. DownloadAndReplace now takes an explicit version (empty = /latest/, for back-compat with AutoUpgrade). fetchChecksums signature updated accordingly; test fixed.
…unner The Runner interface now exposes RunInteractive for subcommands that need /dev/tty attached (sudo prompts for `brew upgrade`). The two direct exec.Command call sites in Update() and PreInstallChecks() are migrated to currentRunner().RunInteractive, closing the L1 test boundary for upgrade and index-refresh flows. ResolveFormulaNames now also goes through Runner.Output. The remaining exec.Command sites (brewInstallCmd and its callers) stay exempt: they need HOMEBREW_NO_AUTO_UPDATE env + custom stdout pipe wiring for StickyProgress streaming that Runner cannot express cleanly. A Runner-exempt comment now flags them. Adds runner_test.go with a recordingRunner that asserts routing and error propagation for both Run and RunInteractive.
Adds internal/logging that wires slog.SetDefault to a multi-handler: captures all debug-level records to a daily rotating JSON file in ~/.openboot/logs/ (0700 dir, 0600 file), and mirrors records to stderr at LevelWarn by default (LevelDebug with --verbose). - Retention: openboot-YYYY-MM-DD.log files older than 14 days are pruned in a background goroutine on startup. Only openboot-*.log is touched. - Fallback: if the log dir or file cannot be opened, Init never errors — it reports once on stderr and keeps stderr-only logging. - Session marker: one session_start record per process (version, pid, args). Wires logging.Init in PersistentPreRunE and flushes via a deferred closer in Execute. Adds a minimal install_started / install_completed log pair in the installer to prove file-sink wiring end-to-end. Tests cover dir/file permissions, session_start emission, append (not truncate) across invocations, retention pruning (with injected clock), fallback on unwritable dirs, and verbose/non-verbose level wiring.
|
👋 Thanks for opening this pull request! Before merging:
@fullstackjam will review this soon. Thanks for contributing! 🚀 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
The log dir needs the owner-exec bit (0700) to be traversable; gosec's G302 rule flags any Chmod mode above 0600 since it can't distinguish files from dirs. Follows the existing nolint pattern used elsewhere.
os.Args is the audit subject of the session_start record by design. slog handlers quote attribute values, so there is no injection path into the surrounding log line.
… redact args Based on self-review of PR #34: - updater: DownloadAndReplace now takes (targetVersion, currentVersion). Drops the SetCurrentVersionForTesting / currentBinaryVersionFn pair that leaked a test-named setter into production CLI code. - updater: exports TrimVersionPrefix; removes the duplicated trimV helper from internal/cli/update.go. - cli/update: new test seams (updateIsHomebrewInstall, updateGetLatestVersion, updateDownloadAndReplace, updateRollbackFn, updateListBackupsFn, updateGetBackupDir) let L1 cover the cobra subcommand without fork/exec. - cli/update_test.go (new): 19 cases covering mutex-flag validation, Homebrew-refuse branches (pin, rollback, latest) plus allowed list-backups, semver validation, dry-run paths (pin/latest/rollback with+without backups), delegation + error wrapping for download and rollback, and the runListBackups branches (empty / populated / list-error / dir-error). - logging: session_start now passes os.Args through RedactArgs, replacing the value of "--flag=value" entries whose flag name contains any of {token, password, secret, key, credential} (case-insensitive) with "<redacted>". Space-separated form is a documented non-goal. - logging: adds a multiHandler WithAttrs/WithGroup test covering the fan-out paths, plus a table-driven RedactArgs test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds pre-upgrade binary backup and rollback functionality, structured logging to disk, and refactors the update command to support version pinning and rollback operations.
Why?
This enables users to safely upgrade OpenBoot with the ability to roll back to a previous version if needed. It also introduces always-on structured logging to
~/.openboot/logs/for debugging and audit purposes, independent of the--verboseflag.Changes
Backup & Rollback (
internal/updater/backup.go,internal/updater/backup_test.go)~/.openboot/backup/with a timestamped filename including the version (e.g.,openboot-1.2.3-20240419T103000Z)openboot update --rollbackrestores the newest backupopenboot update --list-backupsshows available backupsValidateSemver()function validates version strings (X.Y.Z format, optional leading 'v')checksumsURL()andbinaryURL()for cleaner version-specific download pathsStructured Logging (
internal/logging/logging.go,internal/logging/logging_test.go)~/.openboot/logs/openboot-YYYY-MM-DD.login JSON format--verbose)session_startrecord with version, PID, and argumentsUpdate Command Refactor (
internal/cli/update.go)updatesubcommand with mutually-exclusive modes:openboot update— upgrade to latest release (default)openboot update --version X.Y.Z— pin to exact versionopenboot update --rollback— restore most recent backupopenboot update --list-backups— show available backups--dry-runflag for all modes--versionand--rollbackfor Homebrew-managed installsCore Updater Changes (
internal/updater/updater.go)DownloadAndReplace()now accepts aversionparameter (empty string uses/latest/for back-compat)backupCurrentBinary()before atomic renamefetchChecksums()to accept version parameterBrew Runner Abstraction (
internal/brew/runner.go,internal/brew/runner_test.go)RunInteractive()method: Routes commands that need TTY access (e.g., sudo prompts) through a dedicated pathSetRunner()allows tests to inject fake runners without fork/execUpdate()ininternal/brew/brew.goto useRunInteractive()forbrew upgradeTest Coverage
Testing
go vet ./...passes--dry-runflag for all update modesNotes for reviewer
https://claude.ai/code/session_01Syu9VBJBk7uwnjXrK7tHPV