Rewrite devboost as a Go typed-resource engine (v2) - #10
Open
rolfsormo wants to merge 33 commits into
Open
Conversation
Adds a legacy_shell module that finds pre-existing zinit, asdf, or nvm setups duplicating what devboost's znap/mise already manage, and disables the redundant lines in place (commented out with a devboost:disabled marker, never deleted) so they stay reviewable and reversible by hand. A new `devboost clean` command permanently removes marked lines once trusted, idempotently and independent of when apply last ran. Also fixes a real bug in the zsh module's own template: it called compinit itself before sourcing znap, which redefines compinit as a no-op and runs its own deferred completion init — so devboost's call did a full, wasted rebuild every shell start. Removing it, together with the legacy tooling fixes, cut a real machine's measured login-shell startup from ~1.44s to ~285-305ms. Closes the startup-lag root cause tracked in #5, #6, #7.
Proves the v2 architecture (typed resources, one diff function shared by plan/apply, Go struct literals instead of YAML) against the smallest real module: znap. plan and apply both call the same ComputeDiff, with dry-run being nothing more than "the caller that doesn't invoke the resulting ops" — no branching inside the diff logic itself, which is what actually closes the plan/apply-drift bug class the current bash module system has already hit in production. Introduces the first two resource kinds: DirExists (a parametrized, reusable diff-kind shorthand) and GitClone (composes it, shells out to git for the actual clone since git's own implementation is the correct primitive to use, not something to reimplement). Verified: real clone + idempotent second run, cross-compiles cleanly to macOS/Linux/Windows across amd64/arm64, and the existing bash tool and its full test suite are completely untouched. Existing bash tree stays authoritative until the full migration lands; this is the first proof of the mechanism, not a cutover.
Resources can now declare DependsOn, replacing bash's implicit, hand-maintained module ordering with an explicit graph (topoSort). Apply and plan traverse differently on purpose: plan diffs everything once, up front, since nothing has actually converged; apply diffs and executes one resource at a time in topological order, so a resource that depends on another sees its real post-execution effect, not a stale pre-execution snapshot — verified with a test that fails loudly if this regresses to a batch diff. Also replaces config's package-level global state (untestable, and the exact shape that produced the earlier ~-expansion bug — a default value skipped expansion because it took a different code path than a real config value) with a plain Config type callers load once and pass around. Added a permanent regression test for that bug. Every new resource kind and the config package now have real unit tests instead of relying solely on end-to-end manual runs.
Five more typed resource kinds, ported faithfully from the bash tool's core helpers and per-module logic: - File: byte-for-byte content diff, backs up before overwrite by default (ports db_write_file/db_backup_file). - BlockInFile: create/append/replace-between-markers (ports db_upsert_block's awk logic). - GitConfig: shells to git config --get/--set, since git's own config parsing is the correct primitive, not something to reimplement. - Package: per-OS provider backends (brew/apt/dnf/pacman) behind one resource kind, matching the architecture doc's "providers are swappable behind one resource type" model. Ports db_install_packages and _db_pkg_map's name-mapping table, including Homebrew self-bootstrap and apt-get update-once-per-run. - LineInFile: the mechanism behind devboost's redundant-tooling dedup (zinit/znap, asdf/mise, nvm/mise) — marks matching lines disabled in place rather than deleting them, and respects a user manually restoring one by hand as an explicit override. Ports core_legacy_shell.sh's marker/snapshot/restore-detection logic. Unlike the bash version, there's no grep/awk dialect-mismatch risk to guard against — Go's regexp is the only engine used for both detection and rewriting. All five have real unit tests, including a regression test for the exact manual-restore behavior the dedup mechanism depends on.
The architecture's one deliberate escape hatch for state that doesn't
fit File/Package/GitConfig/etc. A module still only ever declares
data (ID, Wants) — never imperative logic at the declaration site.
What keeps this from becoming a loophole: CommandGuarded{ID: "x"}
does nothing by itself. ID must match a real Go implementation
hand-registered in core via RegisterCommand; an unregistered ID
errors loudly rather than silently no-opping. There is no generic
"run a script, check the exit code" shortcut — adding a new use costs
the same real Go work as adding a proper typed kind, which is the
point: this must never be the easy path when a real kind is
achievable.
Four straightforward module ports: - starship: static File resource for the prompt config. - direnv: File resource for .direnvrc, configurable content. - git: four GitConfig resources for delta integration. - corepack: first real CommandGuarded use — corepack enable has no direct "already enabled" query, and the bash version doesn't attempt one either, so this is ported faithfully rather than inventing new idempotency logic beyond what existed. Also fixes a real bug these modules' own tests caught immediately: config.Get only recognized string-typed YAML values, so a real boolean (enable: false, unquoted) was silently ignored and the default used instead — meaning every .enable flag in a user's real config would have been useless the moment they wrote it the natural YAML way instead of quoting it as "false". Get now stringifies bool/int/float scalars the same way the bash tool's yq-based reader renders them in plain text, matching real config semantics instead of just what the earlier tests happened to exercise (which only used already-quoted string values).
Adds Config.GetList for reading a YAML list of strings (packages.base and friends), refactored out of Get's traversal logic via a shared lookup helper rather than duplicating the dotted-path walk. Ports modules/module_pkg.sh: installs the configured (or default) base package list via the Package resource kind built earlier — the per-OS name mapping and install logic already lived there, so this module is just supplying the desired package list.
Ports modules/module_services.sh's darwin branch as a CommandGuarded resource (checks brew services list for atuin already started, converges via brew services start). The bash version's Linux branch is purely informational — it never actually converges anything, just logs a suggestion to check systemd — so it isn't forced into a resource with nothing to do; that note belongs with whatever doctor-only informational mechanism lands alongside the security module (task #14).
CommandGuarded now carries a Params any field, threaded through to its registered Satisfied/Converge functions. This was a real gap, not scope creep: tmux's plugin-install step genuinely needs the configured TPM path at Converge time, and the registry (ID -> GuardedCommand) has no other way to receive per-declaration data while keeping the "module only ever declares data, never logic" rule intact — Params is still just data the module supplies, the registered implementation for ID still owns all the actual logic. Ports modules/module_tmux.sh: GitClone for TPM, a dependency-ordered BlockInFile for the tmux.conf block (depends on tmux_tpm — the render needs the resolved TPM path), and a CommandGuarded plugin install/update step (depends on both, ported faithfully as always-pending since TPM's own install script is what's actually idempotent, matching the bash version's fire-and-forget behavior).
Ports modules/module_mise.sh: converges mise-managed global toolchain versions via CommandGuarded (mise use/install are both idempotent no-ops already, and the bash version never checked beforehand either — faithfully always-pending), then — the escape hatch's first real, substantial use — offers to reinstall previously-global npm packages into a new node version when mise's own convergence changes it, including the interactive confirm prompt (Converge just reads stdin directly, same as the bash version reads the terminal directly; no new engine plumbing needed for this). Also fixes a real bug the port's own tests caught: the bash version's npm-globals parser let the literal string "node_modules" (npm's own global module directory, always the first line of `npm list -g --parseable`) leak through as a false-positive package name — its awk filter only checked "does this line have a slash," which that line also satisfies. Confirmed by running the actual awk command against real npm-shaped output before deciding this wasn't specific to the port. Fixed by filtering on name, not position, so it doesn't depend on npm's output ordering. Tracked to backport to the bash tool as task #24 (low priority, low severity: worst case is an npm install -g node_modules that just fails).
Ports modules/module_zsh.sh, the largest module: renderZshDevboost composes the whole .zshrc.devboost file exactly like the bash version's sequential heredoc/echo composition, section by section, each gated on the same config keys (starship, atuin, fzf, mise, direnv, aesthetics, aliases). Includes a regression test asserting the module never reintroduces the direct compinit call removed earlier this session (that bug — devboost calling compinit itself before znap redefines it as a no-op — was the dominant real-world startup-lag finding from the zsh investigation). The include-block injection into ~/.zshrc needed a bespoke, module-local resource kind (zshIncludeBlock) rather than reusing BlockInFile: it has a fourth case BlockInFile's contract has no room for — an unmarked pre-existing line already sourcing .zshrc.devboost must cause a warn-and-skip, not a normal inject, or shell startup would silently pay for everything in .zshrc.devboost twice. This is exactly the double-sourcing bug fixed for real earlier this session; the port has a dedicated regression test for it. Also documents (with tests, not a silent fix) a real weakness inherited from the bash regex: it only excludes '#' immediately adjacent to the match, not general end-of-line comments — logged as task #25 for a deliberate decision rather than changed unilaterally during the port. Exports kinds.BackupFile (was unexported) so this module-local kind can reuse the same backup behavior instead of duplicating it.
…cy_shell) Ports modules/module_legacy_shell.sh as three separate modules, per the architecture doc's tool-first grouping decision (v1's flat redundancy-pair split would read as an unscannable list once dozens of dedup checks accumulate across many tools). Each module owns one pattern, targeting the file the bash version actually used per migration_id (zinit/asdf against ~/.zshrc, nvm against ~/.zprofile — covered by a dedicated test asserting the nvm module doesn't accidentally look at .zshrc). All three reuse the same LineInFile kind built earlier; the shared marker/backup mechanism from core_legacy_shell.sh already lives there. Verified all three regex patterns (zinit dup, asdf source, nvm source) compile and match correctly under Go's RE2 engine before committing to them — confirms the architecture doc's claim that the grep/awk dialect-mismatch risk from the bash version genuinely has no equivalent here, not just asserted. Exports kinds.MarkerFor (was unexported) so callers outside the package — these tests, and eventually clean/doctor — can construct the exact marker string without duplicating the format.
Adds engine.Diagnostic/DiagnosticFunc — a deliberately separate concept from Resource/PendingOp for read-only findings with nothing to converge (a toolchain pinned to 'latest', oh-my-zsh present, a double-sourced .zshrc). Earlier modules (zsh's include-block conflict) used a PendingOp with a no-op Execute for a similar warning-shaped case, but that doesn't generalize: a pure diagnostic was never a pending *change*, so folding it into PendingOp would make apply's "N changes made" accounting lie, and a module with only diagnostics (nothing to install/fix) is a genuinely different shape from a module with nothing to report at all. Diagnostics lets that distinction be real. Ports modules/module_security.sh: the devboost-check alias block (apply-side, a normal BlockInFile resource) and five doctor-only checks (apply-side has nothing to do with these — latest-pinned toolchains, oh-my-zsh presence, double-sourced .zshrc, TPM over HTTP, stale Homebrew index), now expressed as Diagnostics rather than fake resources.
…ull CLI
Adds the module registry (engine/modules/registry.go): every module
presents a uniform {Name, Resources, Diagnostics} shape regardless of
its internal signature differences (Services/Pkg need OS, Security
has both Resources and Diagnostics, most just need config). AllResources
combines every module's desired state into the one list Plan/Apply
already operate on.
Doctor computes ONE combined diff across all modules together (same
graph Plan/Apply use), then groups the results back by module for
readable output — NOT by diffing each module in isolation, which was
the first implementation and broke immediately: security's
alias-block resource depends on zsh's zshrc_devboost resource (see
below), and diffing security alone can never resolve a dependency on
a resource that isn't in its own list. Caught by actually running the
CLI's doctor command against a fresh fake HOME, not just unit tests —
added a regression test for the cross-module case specifically.
Real bug found and fixed while wiring the full registry together:
zsh's .zshrc.devboost resource is a File (full-content overwrite);
security's devboost-check alias resource is a BlockInFile
(append/replace a marked block) targeting the SAME path. With no
explicit ordering, running security before zsh let zsh's File
silently overwrite and destroy security's already-written block —
confirmed with a test exercising both orderings before adding
security_check_alias's DependsOn on zshrc_devboost, which also
required security to skip cleanly (not error) when zsh is disabled,
since that dependency target then wouldn't exist.
Rewrites cmd/devboost-v2/main.go into the real CLI: plan/apply/doctor
subcommands, --config/--help/--version flags, using the registry
instead of the spike's single hardcoded znap module. --dry-run,
--verbose, --yes, and the uninstall/migrate-from-oh-my-zsh subcommands
are intentionally not yet wired — tasks #16/#17.
Adds kinds.RemoveBlock (ports db_remove_block), separate from BlockInFile.Diff/Execute which only ever adds/updates a block — removal is a deliberate, distinct action, same split the bash tool already had. Ports core_main.sh's db_run_uninstall as modules.Uninstall: removes .zshrc.devboost, the devboost block from ~/.zshrc (leaving the user's own content in that file untouched — verified with a test), the devboost block from ~/.tmux.conf, .direnvrc, and the state file. Deliberately does not remove packages/znap/TPM/mise toolchains, same scope the bash version documented. Wired into the CLI as the uninstall subcommand.
Adds kinds.ArchiveDir (moves a directory into the backup root instead of deleting it, named <basename>-<label>-<timestamp> — same naming the bash legacy-tooling dedup mechanism already established) and exports DefaultBackupDir for callers outside the kinds package. Ports core_omz.sh's db_run_migrate_from_oh_my_zsh as modules.MigrateFromOhMyZsh: replicates oh-my-zsh's own uninstaller (archives ~/.oh-my-zsh rather than rm -rf — more conservative than the bash version, costs nothing extra and gives a recovery path), then recovers post-install customizations by diffing the timestamped uninstall backup against the pre-install base and stripping oh-my-zsh's own template lines. All 6 scenarios from the bash test suite ported and passing: refuses without --yes, nothing-to-do, full flow with a pre-install base, full flow without one, nothing beyond template, and --dry-run makes no changes. Deliberately NOT a Resource/ResourceKind — this is a one-shot migration operation with its own --yes gate, not a converge-to- desired-state module, same distinction the bash tool drew by making it an explicit separate subcommand rather than folding it into apply. Finishes wiring the CLI: --dry-run (apply routes to Plan when set, matching the bash tool's DB_DRY_RUN behavior) and --yes flags, plus the migrate-from-oh-my-zsh subcommand. All CLI-surface tasks (#15-18) are now complete — devboost-v2 has apply/plan/doctor/uninstall/ migrate-from-oh-my-zsh with the same flags the bash tool exposes, minus --verbose (nothing has a verbose-output distinction yet).
Ports tests/test-macos.sh's coverage: builds the real devboost-v2 binary, runs plan/apply --dry-run/doctor/apply against a fresh temp HOME, then a second apply to verify idempotency — same tradeoff the bash version already accepted (this touches real Homebrew/git, not just the in-process resource graph). Skipped in -short mode given the cost (measured ~463s for a real full package-list install). Ran for real on this machine: passed, including byte-for-byte idempotent second apply. Completes test-suite parity with the bash tool's coverage — every bash test-*.sh file has an equivalent (per-kind/per-module unit tests for the mechanism-level bash tests, this integration test for the end-to-end one); the three bash3-compat/runtime/source test files have no Go equivalent since they're moot once bash is gone entirely.
Adds install.sh: a small, pure-POSIX-shell dispatcher whose only job is OS/arch detection (including the Rosetta 2 trap — uname -m reports x86_64 under Rosetta even on real Apple Silicon, requiring an extra sysctl check, same as rustup-init.sh and uv's installer both have to handle), fetch the matching prebuilt binary, chmod it executable, and exec it. No application logic lives here — everything real lives in the binary, matching the architecture doc's honest framing: this makes the entry point small enough to fully read before running, it does not make the whole system easier to audit end to end. DEVBOOST_INSTALL_BASE_URL is not yet pointed at a real release — per the "local cross-compilation only for now" decision, no GitHub Actions spend at this stage. Tested for real: builds the actual binary, serves it from a local HTTP server, runs the real script against it, confirms correct platform detection and a successful exec into the real binary's --version output. Also verifies the failure path (no server listening) exits cleanly rather than hanging.
Each module that picks a specific tool (starship, tmux, mise, pkg, security, services, znap, direnv, git/delta, corepack, and the three dedup modules) now documents why that tool/default was chosen, honestly distinguishing well-documented external consensus (tmux escape-time=0, delta over diff-so-fancy, Node LTS channel) from taste-based picks with no single canonical source (starship's truncation_length, tmux's history-limit) from defaults that are now actively questionable given how the ecosystem moved since they were picked (corepack's upcoming unbundling from Node 25+, mise's own docs discouraging devboost's direnv use_mise integration pattern). Backed by dedicated research forks into direnv+mise, delta vs. diff-so-fancy, corepack's Node-bundling status, and the measured startup-lag data behind the three dedup modules, plus earlier research into starship/tmux and mise/pkg/security/znap conventions.
Teaches the research-then-document process this session used to write rationale into all 12 tool-choice modules: check adoption/reputation, prefer first-party docs over third-party consensus, verify a choice is still current (not just historically correct), and write findings into the module's doc comment with an honest confidence level. Real file lives at .agents/skills/devboost-module-author/SKILL.md (the vendor-neutral convention Codex CLI reads directly); .claude/skills is a symlink to .agents/skills, so Claude Code and other tools that read .claude/skills/ (OpenCode, Cursor, Copilot) see the same content with zero risk of drift. Any future skill only needs adding once, under .agents/skills/.
direnv: stop writing the use_mise .direnvrc helper. mise's own docs call that integration pattern deprecated and say they won't fix direnv-compatibility bugs in it. mise activate zsh (already global in .zshrc.devboost) fully replaces it via mise's own directory-change hook — mise's docs confirm this split (global mise activate for toolchains, direnv left alone for unrelated env vars) is the current supported combination, not a deprecated one. direnv stays installed by default; direnv.content still lets a user opt into managed content. corepack: install it via `npm install -g corepack` when missing, instead of silently treating absence as "nothing to do." Node 25+ no longer bundles corepack, so absence is now the expected case on a current toolchain, not a signal there's nothing to converge. This matches the exact replacement workflow Node's own TSC decision names explicitly, rather than working around their decision. Both were flagged as open concerns in the prior rationale-doc pass (1c806a6) and are now resolved rather than just documented as known gaps.
A rationale that cites a star count for the winner but never names what else was considered doesn't actually read as researched. Add a requirement to name the real competing option(s) and the concrete reason they lost, and to say plainly when it was a close call rather than inflating the gap.
The bash tool's db_run_clean (core_legacy_shell.sh) never got ported during the Go migration — a real functionality gap the cutover would otherwise silently drop, since the README already documents `devboost clean` as a real command. Adds Clean(cfg, dryRun), wired as the `clean` subcommand: permanently strips devboost:disabled-marked lines (from the zinit/asdf/nvm dedup modules) out of ~/.zshrc and ~/.zprofile, backing up first, idempotent and order-independent (re-derives what to clean from the live file each run). Exports kinds.MarkerPrefix so clean can match a devboost:disabled line regardless of which migration marked it, mirroring the bash version's marker-prefix grep exactly.
Replaces the bash implementation entirely with the Go-engine CLI that's been built alongside it on this branch. cmd/devboost-v2 becomes cmd/devboost (binary name devboost, version 2.0.0). Removes core/, modules/, build.sh, devboost.sh, devboost.sh.in, and the bash-specific test suite (tests/test-bash*.sh, test-linux.sh, test-macos.sh, test-migrate-oh-my-zsh.sh, test-zshrc-double-source.sh, run-tests.sh, tests/docker/) — all now fully superseded by the Go engine and go test. tests/test-install.sh and tests/test_common.sh are kept: they test install.sh (the real bootstrap dispatcher), which is unrelated to the bash implementation being removed. Rewrites README.md, ARCHITECTURE.md, CONTRIBUTING.md, AGENTS.md (and its CLAUDE.md symlink), .devboost.yaml.example, and .github/workflows/test.yml for the Go reality — verified .devboost.yaml.example actually loads through the real config reader rather than trusting it by inspection. No GitHub release exists yet (cross-compilation is local-only per the architecture decision), so Quick Start leads with build-from-source rather than a curl command that would 404. Also ports the one real functionality gap the cutover would otherwise have silently dropped: `devboost clean` was documented in the README but never implemented in the Go CLI (see the dedicated clean.go commit just before this one). This is a breaking change: MAJOR version bump per AGENTS.md's own versioning rules (new engine, and real default-behavior changes — see CHANGELOG.md's 2.0.0 entry for the direnv/corepack fixes bundled in). Per explicit instruction, this stays on the v2-go-engine branch — MERGE TO MAIN IS NOT DONE HERE AND REQUIRES SEPARATE EXPLICIT APPROVAL.
Adds docs/tool-choice-review-2026-08.md — the first pass of the periodic re-review process from issue #9: 7 parallel research passes, each asked "what would we pick with no history" and "does the current default still hold up," across all 19 tool-choice defaults. Implements the low-risk, high-confidence findings directly: - Swap zsh-users/zsh-syntax-highlighting -> zdharma-continuum/ fast-syntax-highlighting: widely-documented faster successor, and what the pre-existing hand-tuned zinit setup from the original dedup investigation was already using. - Gate tmux-logging behind tmux.plugins.logging.enable (default false): 3-4x smaller adoption than the other three bundled tmux plugins, absent from most "essential setup" roundups. - Fix atuin filter_mode: reverted the base filter_mode to atuin's own upstream default (global) instead of devboost's unjustified "directory" override, which over-restricted interactive ctrl-r search. Added filter_mode_shell_up_key_binding: directory separately, for the quick-recall behavior the original setting likely intended. - Relabel starship's command_timeout/add_newline and pkg's procs doc comments to state plainly that they're a deliberate deviation from upstream and a genuine taste call respectively, rather than implying settled consensus. - Note mise's go pin (1.26) as watch-for-1.27-GA, not yet stale (the channel auto-resolves to a current patched release). Deliberately NOT implemented in this pass: the zsh plugin manager question (znap vs. the review's antidote/sheldon recommendation) is a bigger, riskier change than a drive-by config swap — commented on issue #6 with the benchmark evidence instead, since that issue's original znap->zinit premise doesn't hold up against reproducible benchmark data, but the actual migration decision needs its own dedicated work.
TestSecurityDiagnosticsNoneWhenClean isolated HOME but not the two diagnostics that read real, uncontrolled external state (actual Homebrew index age via `brew --repository` + git log, actual TPM remote if present) — asserting "exactly one non-warning diagnostic" implicitly assumed those always come back clean too. Failed for real on the GitHub Actions macOS runner, whose Homebrew index genuinely was 11 days old — an accurate finding, not a flaky test. Narrowed the test to only assert on what SecurityDiagnostics actually controls from a clean HOME: no toolchain-pinned-to-latest, no oh-my-zsh, no double-sourced-zshrc warning. Caught by PR #10's first real CI run.
test-install.sh's real end-to-end test raced python3 -m http.server's actual bind time with a fixed sleep 1 before curling it — worked locally, but failed for real on GitHub Actions' macOS runner (curl exit 28, connection timeout) where the server genuinely wasn't listening yet after 1 second. Replaced with a short poll loop that waits for the server to actually accept connections before proceeding. Second real bug caught by PR #10's CI run (first was TestSecurityDiagnosticsNoneWhenClean, fixed in 51e8a3c) — neither was flaky, both were tests that worked by accident on the machine they were written on.
Cut a real v2.0.0 pre-release (git tag + gh release, four cross- compiled binaries: darwin/linux x arm64/amd64) off v2-go-engine's tip, specifically to verify the actual curl|sh install.sh flow works end to end before merging — per the concern that shipping "clone the repo and go build" as the install story defeats the whole point of devboost. Verified for real: downloaded the actual published darwin-arm64 binary via install.sh against an explicit release tag URL (GitHub's releases/latest only resolves non-prerelease releases, so the default URL isn't live yet), ran --version and a real `plan` against a sandboxed HOME, both worked correctly. Updates README/ARCHITECTURE/CHANGELOG to state this accurately: a real, verified release exists, but stays marked pre-release (and install.sh's default releases/latest URL stays unresolvable) until this merges to main — build-from-source remains the primary instruction until then, not because nothing works, but because main itself doesn't have install.sh yet.
The previous commit (9a702cf) correctly described this branch's current pre-merge state (no install.sh on main, release marked pre-release). But that framing doesn't belong in docs meant to describe what main will look like after merge — nobody reads these docs as "the state of an open PR," they read them as "how do I use devboost." README's hero command and Quick Start now lead with the real curl|sh one-liner as primary (matches what actually happens once merged + the release is promoted), with review-first and build-from-source as secondary options — not the reverse. Removed every "until this merges"/"once merged to main"/pre-release-status caveat from README, ARCHITECTURE, and CHANGELOG; those described a transient state that stops being true at merge time.
Cross-checked every factual claim in README against actual code before merge, per explicit request. Found and fixed real drift: - README's "What Gets Installed" still listed zsh-users/zsh-syntax-highlighting and unconditional tmux-logging — both changed by the tool-choice review (docs/tool-choice-review- 2026-08-08.md) but the README wasn't updated when that landed. - .devboost.yaml.example was missing tmux.plugins.logging.enable entirely, even though it's now a real, live config key. - CHANGELOG's "Removed" section claimed tmux.plugins was dropped as a bash-only no-op — true for the plugin *list*, but self-contradicted by tmux.plugins.logging.enable existing as a real key. Split into accurate Removed/Tool-choice-review sections. Everything else audited and confirmed accurate: CLI usage docs against cmd/devboost/main.go, toolchain defaults against mise.go, package list against pkg.go, inline config examples verified to actually load via config.Load(), task list cross-checked against real repo state (three genuinely-still-open items filed as issues #11-13; #24 confirmed moot now that the bash tree is gone; #21/#27/#28/#29 confirmed actually done despite stale tracker labels).
Found by a real Linux container test (Ubuntu 24.04): lazygit isn't packaged in Ubuntu's default apt repos. Package.Diff's Execute previously stopped at the first failed apt-get install, meaning that one distro-specific unavailable package silently aborted the entire `apply` — zsh config, starship, tmux config, git config, none of it got written, even though none of that depends on lazygit succeeding. installAll now attempts every missing package regardless of earlier failures and returns a single aggregated error listing everything that failed, so a real apply run converges everything it can and reports exactly what didn't install, rather than failing opaquely on whichever package happens to come first. Does not address the deeper question of whether the engine's DiffAndExecute should skip only resources that depend (via DependsOn) on a failed one while still converging independent resources — pkg has no resources declaring DependsOn on it today even though other modules implicitly assume packages are already installed (see registry.go's own "pkg first" comment). That's real, separate architectural work, not a same-day fix.
This directly contradicted an earlier explicit decision ("no GitHub
Actions spend at this stage") that I should have re-raised rather than
silently overriding when I wrote .github/workflows/test.yml during the
cutover. Cancelled the in-progress run and removed the workflow file
entirely.
The repo is public, so the runs that already happened were on GitHub's
free-tier minutes, not billed — but that's beside the point: this
should have been a conscious re-decision with the user, not an
assumption. Local development cost is effectively free; GitHub Actions
spend is not, and stays opt-in rather than automatic.
Fixed a stale AGENTS.md reference to the now-removed workflow file.
All the actual verification this branch needed (go build/vet/test,
tests/test-install.sh, a real Linux container apply) already happened
locally and doesn't depend on GitHub Actions running anything.
DiffAndExecute previously returned immediately on any resource's Execute error, stopping the entire apply run regardless of whether other resources actually depended on the failed one. Confirmed as a real, current-state blocker via a live Ubuntu container test: several packages genuinely unavailable in apt's default repos caused the Package resource to fail, which then silently prevented zsh config, tmux config, git config, and every other resource from converging at all — even though none of them depend on those packages. DiffAndExecute now returns an ExecutionResult (Applied/Failed/Skipped) instead of a bare []PendingOp: a failed resource is recorded in Failed, everything transitively DependsOn-ing it is recorded in Skipped (without being diffed or executed against state the failure never reached) and attributed to the root-cause failure rather than just its immediate parent, and every resource NOT in that dependency chain still runs normally. The returned error is now reserved for conditions that make the whole run meaningless to continue (a dependency cycle, an unknown dependency, a resource whose Diff itself errors) — a normal per-resource Execute failure is an expected outcome recorded in the result, not a run-aborting error. Apply reports every outcome (Done/Failed/Skipped) and still returns a non-nil error when anything failed, so the CLI exits non-zero, but everything that could converge now does. engine/modules/zsh_security_interaction_test.go and engine/doctor_test.go's existing `_, err := DiffAndExecute(...)` call sites needed no changes — discarding a typed struct via `_` compiles regardless of the return type.
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
1 participant
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.
Summary
Full rewrite of devboost from a hand-written bash module system (separate
plan/applyfunctions per module, which had already drifted out of sync in production — see the 1.3.0 double-sourcing fix) to a Terraform-inspired typed-resource engine in Go. One shared function (engine.ComputeDiff) computes what's out of sync for bothplanandapply, closing that bug class structurally rather than by convention.See ARCHITECTURE.md for the full design and CHANGELOG.md's
2.0.0entry for the complete list of changes.This is a breaking change (MAJOR version bump to 2.0.0 per the project's own versioning rules) — new engine, and real default-behavior changes bundled in (see below).
What's included
DependsOnon resources — a real dependency graph with topological sort and cycle detection, replacing the bash version's implicit registration-order dependency.doctoroutput grouped tool-first by module (diffs the full combined resource graph once, then groups for display — an earlier version diffed per-module in isolation and broke on cross-module dependencies, now covered by a regression test)..agents/skills/devboost-module-author/SKILL.mdfor the process.install.shbootstrap dispatcher, verified end to end against a real published v2.0.0 release (four cross-compiled binaries, built and published by hand — no GitHub Actions release pipeline).docs/tool-choice-review-2026-08.md): 7 parallel research passes re-examining all 19 tool defaults against current data, not just re-justifying old choices. Findings already folded in: fast-syntax-highlighting swap, tmux-logging made opt-in, atuin filter_mode fixed to match upstream's own default, several doc-comment honesty fixes. The zsh-plugin-manager question (znap vs. antidote/sheldon, contradicting issue Migrate devboost's default zsh plugin manager from znap to zinit #6's original zinit proposal) is flagged but deliberately not migrated in this PR — see the review doc and the comment on Migrate devboost's default zsh plugin manager from znap to zinit #6.direnv/corepackfixes: dropped the deprecateduse_misedirenv integration (mise's own docs discourage it) in favor ofmise activate zsh(already global); corepack now self-installs vianpm install -g corepackwhen missing, matching Node's own TSC-recommended path for its post-25 unbundling.Package.Executeno longer aborts the wholeapplyon the first unavailable package — found via a real Linux container test (lazygitisn't in Ubuntu's default apt repos), fixed to attempt every package and aggregate failures.Test plan
All testing is local — this repo deliberately runs no GitHub Actions CI (local development cost is effectively free, GitHub Actions spend is not; a prior
.github/workflows/test.ymlwas removed from this PR for exactly this reason).go build ./...go vet ./...go test ./...— full suite, including the slow real end-to-end test (TestSandboxedApplyPlanDoctorIdempotent) — passing locally./tests/test-install.sh— real fetch+exec test of theinstall.shbootstrap dispatcher against a locally-served binary — passing locallyinstall.sh, ranplanandapplyfor real — confirmed per-OS package name mapping (fd→fd-find), confirmed thePackage.Executefix converges everything else even when one package is genuinely unavailable on the distroNot in scope for this PR
apply(tracked as issue Engine: should a failed resource abort the whole apply, or only its dependents? #14) — thePackage.Executefix in this PR addresses the specific failure mode found in testing, not the general case.