fix(installer): make completion summary dry-run aware

[fullstackjam]

What does this PR do?

Makes the post-apply completion summary dry-run aware, so openboot install --dry-run no longer ends by claiming it installed and configured everything.

Why?

On a TTY, openboot install -p minimal --dry-run runs the wizard, then the linear apply. The apply correctly prints [DRY-RUN MODE - No changes will be made] and [DRY-RUN] Would ... for each step. But showCompletionFromPlan ignored plan.DryRun, so the preview ended with:

=== Installation Complete! ===
✓ OpenBoot has successfully configured your Mac.
What was installed:
  - Git configured with your identity
  - 19 CLI packages
  - 5 GUI applications
Next steps:
  - Restart your terminal to apply changes
  - Run 'brew doctor' to verify Homebrew health

Now showCompletionFromPlan passes dry-run plans to a new showDryRunCompletionFromPlan:

=== Dry run complete — no changes were made ===

  Would install:
    - 19 CLI packages
    - 5 GUI applications

  Next steps:
    - Run the same command without --dry-run to apply these changes

If any step fails, the header is Dry run finished with errors — no changes were made, followed by the same N step(s) had errors warning as before.

The real-run summary is unchanged: the new code is an early return, and the old body is not edited.

Testing

• go vet ./... passes
• Relevant tests added or updated
  • TestShowCompletionFromPlan_DryRunDoesNotClaimChanges (clean and with errors). Written first; it failed before the fix with expected: "Dry run complete — no changes were made", actual: "Installation Complete!".
  • TestShowCompletionFromPlan_RealRunOutput pins the real-run summary line for line, clean and with errors. It passed both before and after the fix.
  • recordReporter (in applycontext_abort_test.go) now also records non-header messages in lines, so tests can check what the summary says. The existing completion tests used NopReporter and only checked NotPanics.
• make test-unit passes (exit 0). archtest re-run uncached: ok.
• Tested locally: OPENBOOT_DISABLE_AUTOUPDATE=1 go run ./cmd/openboot install -p minimal --dry-run --silent (non-TTY, same ApplyContext path) prints the new summary shown above.

Cross-repo checklist

• Does this need a docs/content update in openboot.dev? No. The landing page's terminal mock (src/routes/+page.svelte) shows the real-run summary, which is unchanged.
• Does this change the CLI ↔ server API contract? No.

Notes for reviewer

• No Git line in the dry-run summary. On real runs, applyGitConfig skips when an identity already exists ("Already configured"), so "would configure Git" would be false on most machines. The Git step's own output already says which way it goes.
• Next steps swapped. "Restart your terminal" and "brew doctor" are replaced with "re-run without --dry-run", because nothing was installed.
• Not fixed here (same kind of issue):
  • The real-run summary prints "Git configured with your identity" whenever !plan.PackagesOnly, even when SkipGit is set or git was already configured.
  • runUpdate prints "Update Complete!" under --dry-run.

[fullstackjam]

@claude please review this PR — focus on correctness, design, test coverage for the branches it adds, risk, and how it rolls back. (The automatic claude-review run posted nothing: 1 permission denial, "No buffered inline comments".)

[claude]

Claude finished @fullstackjam's task in 1m 1s —— View job

---

Review: fix(installer): make completion summary dry-run aware

• Gather context (PR diff, changed files)
• Read internal/installer/installer.go changes
• Read test changes (applycontext_abort_test.go, installer_extra_test.go)
• Review correctness, design, test coverage, risk, rollback
• Post final review

Correctness

The fix is sound. showCompletionFromPlan has exactly one call site (installer.go:151, from ApplyContext), so the new if plan.DryRun { ... return } branch at installer.go:219-222 applies uniformly — there's no second code path that could still print the old "Installation Complete!" copy under --dry-run. errCount is len(softErrs), which steps can populate even in dry-run mode (dry-run steps can still hit real failures, e.g. a HEAD request or size check), so the "finished with errors" branch in showDryRunCompletionFromPlan is reachable in practice, not just in tests.

Design

• Early-return dispatch (installer.go:219-222) is the right shape here — it keeps the real-run body completely untouched (confirmed by the diff: nothing inside the old function besides the new guard clause moved or changed), which is exactly what TestShowCompletionFromPlan_RealRunOutput is there to pin down.
• Dropping the Git line from the dry-run summary is a good call and the comment at installer.go:258-260 explains why (identity state is conditional, and the Git step already reported it). This avoids introducing a new false claim while fixing the existing one.
• showScreenRecordingReminderFromPlan is skipped entirely in the dry-run branch rather than called-then-early-returning — behaviorally identical (the function already no-ops on plan.DryRun at line 286) but skips a pointless call. Fine either way.

Test coverage

Good coverage of the branches this PR adds:

• Dry-run clean vs. dry-run-with-errors (TestShowCompletionFromPlan_DryRunDoesNotClaimChanges), asserting both the positive content ("Would install:", counts, re-run hint) and the negative space (none of the old claims — "What was installed:", "successfully configured", "Git configured", "Restart your terminal", "brew doctor" — leak through).
• Real-run output pinned line-for-line pre/post-fix (TestShowCompletionFromPlan_RealRunOutput), which is a good regression guard proving the real-run path is byte-for-byte unchanged.
• recordReporter growing a lines field (capturing Info/Success/Warn/Error/Muted) is a minimal, backward-compatible extension — existing consumers that only read .headers are unaffected.

One small gap: there's no test combining DryRun: true with PackagesOnly: true. Not a real risk since the dry-run summary doesn't reference PackagesOnly at all (the Git line was removed outright rather than gated on it), but worth knowing that's untested territory if that interaction ever needs different copy.

Risk

Low. This is purely cosmetic/output-text logic in a single function, gated behind plan.DryRun, with no changes to control flow, state mutation, or persisted data. The two "not fixed here" items called out in the PR description (real-run Git line ignoring SkipGit/already-configured, and runUpdate claiming success under --dry-run) are pre-existing and correctly scoped out rather than smuggled in.

Rollback

Trivial — a single revert of this commit removes the new showDryRunCompletionFromPlan function and the 4-line dispatch guard, restoring the prior (misleading) behavior with no data/schema/migration concerns. Test-only changes (the recordReporter.lines field) are additive and wouldn't need to be reverted separately even if the production change were reverted alone.

Note

I wasn't able to execute go vet/go test in this sandboxed environment (command execution required interactive approval that wasn't available here) — review is based on static reading of the diff and surrounding code. The PR description states make test-unit and go vet ./... were run locally and passed.