fix(upgrade): verify binary replacement to prevent false upgrade failure (#230)

[decode2]

What

The gentle-ai self-binary upgrade showed a false ✗ failure in the TUI ("Upgrading gentle-ai via binary (X → Y)") even when the binary was correctly downloaded and replaced — gentle-ai --version confirmed the new version and the binary timestamp matched.

Why (#230)

The ✓/✗ marker is driven solely by the upgrade step's returned error. For the self-binary upgrade, that error came from atomicReplace — a single os.Rename — with no check of what actually landed on disk. On hardened self-replace environments (overlayfs, immutable distros, SELinux/AppArmor policies) the running binary's rename can report an error even though the replace completed, so a successful upgrade was reported as a failure.

Fix (scoped to internal/update/upgrade/download.go)

• atomicReplace performs a single atomic rename (via a renameFn seam for testability, mirroring the existing httpClient/lookPathFn pattern). On Unix this is atomic: on success dst is the new binary; on failure dst is left untouched (the previous binary, or absent on first install). The caller already guards against Windows, where a running binary cannot be renamed over.
• New replaceAndVerify trusts a reported rename success, and only when the rename reports an error does it hash the on-disk dst and compare it to the SHA256 of the extracted binary — converting the reported error to success iff the content provably matches. A genuine failure returns the real error and leaves the previous binary intact.

Non-masking guarantee: a reported error is converted to success only when the on-disk bytes equal the expected hash exactly. A real failure (missing/old/mismatched binary) is never reported as success.

Tests

TestReplaceAndVerify (runs cross-platform via the renameFn seam) covers:

• clean rename → success;
• benign rename error after the correct bytes landed → success (the fix(tui): upgrade shows ✗ failure marker but binary is correctly updated #230 case);
• genuine rename error with content mismatch → error, previous binary preserved (non-masking);
• first install (dst absent) → success.

go test ./internal/update/upgrade/... is green; gofmt/go vet clean. No changes to the TUI/executor/spinner code — the fix is confined to the download/replace path.

Closes #230

Summary by CodeRabbit

• Bug Fixes

  • Improved upgrade reliability by verifying the installed binary’s contents after replacement.
  • Reduced false failures in hardened environments when file replacement reports an error even though the new binary was installed correctly.

• Tests

  • Added coverage for successful replacement, error-masked replacement, failed replacement, and first-install scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6c1865e5-8eee-4085-85a0-68546131ab4c

📥 Commits

Reviewing files that changed from the base of the PR and between 87f7fab and 71c5868.

📒 Files selected for processing (2)

• internal/update/upgrade/download.go
• internal/update/upgrade/download_test.go

---

📝 Walkthrough

Walkthrough

The upgrade download flow now hashes the extracted binary and verifies replacement via a replaceAndVerify helper, which uses an overridable renameFn seam. On rename error, it re-hashes the destination file and treats matching content as success rather than a false failure. Tests cover multiple rename scenarios.

Changes

Verified binary replacement in upgrade flow

Layer / File(s)	Summary
Rename seam and hash helper internal/update/upgrade/download.go	Adds package-level renameFn variable defaulting to os.Rename for test overrideability.
Content-verified replacement logic internal/update/upgrade/download.go	Download hashes the extracted binary with SHA256 and calls new replaceAndVerify, which renames via renameFn and, only on rename error, re-hashes dst to confirm success against the expected hash.
Replace-and-verify test coverage internal/update/upgrade/download_test.go	Adds TestReplaceAndVerify covering successful rename, benign rename error with matching content, failure-path error with preserved prior binary, and first-install case; also reformats a struct in TestExpectedChecksumFor.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Download
  participant HashFile
  participant ReplaceAndVerify
  participant RenameFn
  participant Filesystem

  Download->>HashFile: compute SHA256 of extracted binary
  Download->>ReplaceAndVerify: call with src, dst, wantHash
  ReplaceAndVerify->>RenameFn: rename src to dst
  RenameFn->>Filesystem: attempt rename
  Filesystem-->>RenameFn: success or error
  alt rename succeeded
    RenameFn-->>ReplaceAndVerify: nil error
    ReplaceAndVerify-->>Download: success
  else rename returned error
    RenameFn-->>ReplaceAndVerify: error
    ReplaceAndVerify->>HashFile: hash dst content
    HashFile-->>ReplaceAndVerify: dst hash
    alt dst hash matches wantHash
      ReplaceAndVerify-->>Download: success
    else mismatch
      ReplaceAndVerify-->>Download: error
    end
  end

Loading

Suggested labels: type:bug

Suggested reviewers: Alan-TheGentleman

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title matches the main fix: it describes verifying binary replacement to avoid false upgrade failures.
Linked Issues check	✅ Passed	The change addresses #230 by treating rename errors as success only when the on-disk binary hash matches.
Out of Scope Changes check	✅ Passed	The added test seam and tests support the same upgrade-replacement fix; no unrelated scope is evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}