Skip to content

fix(server): only offer claude update for native Claude installs - #14170

Open
AI091 wants to merge 2 commits into
pingdotgg:mainfrom
AI091:fix/claude-native-update-owner
Open

AI091 wants to merge 2 commits into
pingdotgg:mainfrom
AI091:fix/claude-native-update-owner

Conversation

@AI091

@AI091 AI091 commented Sep 28, 2026 •

Copy link
Copy Markdown

On Omarchy, providers come from mise, and ~/.local/bin/claude is a small wrapper that execs the mise shim. T3 Code treated any file at that path as Claude's native install and offered one-click claude update. Claude sees mise, prints "managed by a package manager" and exits 0. So the Update button does nothing, and the popup then says Claude is still outdated.

Claude is now treated like Codex. One-click update is offered only when the path proves the native installer owns it: the real path runs through ~/.local/share/claude/, or claude.exe on Windows. Anything else, including mise wrappers, stays manual-only and still shows the version gap, the same as Codex for mise since #9927 and #10085.

Proper mise support (mise upgrade <tool>) is left out on purpose. #9225 tried it and was closed because the provider layer is being rewritten for V2.

  • Before: Update runs, nothing changes, and the popup says Claude still needs an update.

    before

    before.mp4

  • After: the popup points to provider settings instead of offering a no-op Update.

    after

    after.mp4

ClaudeDriver.test.ts checks both layouts: a symlinked native launcher keeps claude update, and a wrapper script is manual-only.

Claude Opus 5.5 via Claude Code in T3 Code.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected Claude installation detection so wrapper commands are no longer treated as native installations, while native installs continue to be recognized. This ensures update commands are only offered for supported native installations.

A wrapper script at ~/.local/bin/claude (e.g. Omarchy's mise wrapper)
matched the native-installer path check, so T3 Code offered
`claude update`. Claude detects mise, prints "managed by a package
manager" and exits 0, leaving the provider outdated.

Require the real path to run through ~/.local/share/claude/ on POSIX,
keeping the Windows claude.exe path check. Unproven installs stay
manual-only, matching Codex.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 28, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The production change is a narrow correction that prevents one-click Claude updates for unproven wrapper paths while preserving native-install updates. Human review is required because the new test file adds a file-level suppression for a static-analysis diagnostic.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ae4ed5f6-548a-4ef7-b102-0af1ee1e1f1e

📥 Commits

Reviewing files that changed from the base of the PR and between 43f5457 and f78d504.

📒 Files selected for processing (1)
  • apps/server/src/provider/Drivers/ClaudeDriver.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The Claude path classifier no longer treats ~/.local/bin/claude as native. Tests check maintenance-command resolution for native and wrapper layouts.

Changes

Claude maintenance detection

Layer / File(s) Summary
Native Claude path detection
apps/server/src/provider/Drivers/ClaudeDriver.ts, apps/server/src/provider/Drivers/ClaudeDriver.test.ts
The classifier excludes ~/.local/bin/claude and retains the existing native path checks. Tests verify that the native layout resolves an update command and the wrapper layout resolves no update command.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f78d5

Native Unix symlinks into Claude’s share directory remain eligible for the native update, while wrapper launchers remain manual-only. No actionable merge risk is established.

Architecture Summary

Architecture risk: 🔵 Low · up to f78d5

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/provider/Drivers/ClaudeDriver.ts: isClaudeNativeCommandPath no longer classifies paths ending in /.local/bin/claude as native. Paths ending in /.local/bin/claude.exe and paths containing /.local/share/claude/ remain classified as native; the added comment describes the launcher distinction.
  • observed — Modified behavior in apps/server/src/provider/Drivers/ClaudeDriver.test.ts: Adds an isolated test layer and temporary Claude installation fixtures. On non-Windows hosts, verifies that the native layout resolves an update command for the binary, while the wrapper layout resolves null; the test also fails if the disabled driver makes an HTTP request or spawns a process.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: offering Claude updates only for native Claude installations.
Description check ✅ Passed The description clearly explains the change, the problem, the rationale, scope, and UI impact. It includes before-and-after evidence and test coverage, although it does not reproduce the template head…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant