fix(doctor): gate TUI plugin advice on package exports#4667
Conversation
There was a problem hiding this comment.
2 issues found across 2 files
Confidence score: 4/5
- This PR looks safe to merge overall, with risk centered on
doctormessaging rather than core runtime behavior. - The highest-impact issue is in
src/cli/doctor/checks/tui-plugin-config.ts: the check can warn for any registered TUI entry, so validfile:-based configs may be incorrectly flagged and create user-facing false positives. - The "unresolvable" branch text in
src/cli/doctor/checks/tui-plugin-config.tsappears too specific to namedoh-my-openagent/tuientries, which can mislead users whentui.registeredis true because of afile:entry. - Pay close attention to
src/cli/doctor/checks/tui-plugin-config.ts- warning conditions and fix guidance should distinguish named package entries from validfile:registrations.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
[sisyphus-dev] Cubic found a valid false-positive case here: the doctor check was treating valid |
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: All existing tests still pass, new tests cover the edge cases, and the logic correctly gates TUI plugin advice on actual package exports without altering unrelated behavior.
Re-trigger cubic
|
[sisyphus-dev] Verified this PR against #4598: doctor now gates |
Summary
Fixes #4598 and #4303 by preventing doctor from recommending or blessing
oh-my-openagent/tuiwhen the actually registered installed package does not export./tui.This supersedes #4655. That PR fixed the main symptom but Cubic found an actionable issue: package detection could choose the wrong package when canonical and legacy package folders both exist. This PR resolves that by deriving the package export status from the actual server plugin entry in
opencode.json.Changes
package.jsonexports for the registered server plugin entry (oh-my-openagent,oh-my-opencode, or file entry)../tuiexport andtui.jsonlacks the TUI entry, pass with a clear “TUI subpath not shipped” message instead of recommending a broken entry../tuiexport andtui.jsonalready contains the broken entry, warn and tell the user to remove it.QA
PATH="$HOME/.bun/bin:$PATH" bun test src/cli/doctor/checks/tui-plugin-config.test.ts --bailfailed before the fix because doctor still warned to add an unshipped TUI entry.PATH="$HOME/.bun/bin:$PATH" bun test src/cli/doctor/checks/tui-plugin-config.test.ts --bail— 10 pass / 0 failPATH="$HOME/.bun/bin:$PATH" bun run typecheck— passPATH="$HOME/.bun/bin:$PATH" bun run build— passRelated
Summary by cubic
Doctor now checks the server plugin package’s
exportsand only suggests or validates.../tuiwhen./tuiis shipped. Adds support forfile:TUI entries and fixes wrong advice in mixed canonical/legacy installs; fixes #4598 and addresses #4303.exportsfrom the package behind the registered server plugin inopencode.json(oh-my-openagent,oh-my-opencode, orfile:)../tuiisn’t exported and no TUI entry exists, pass with “TUI subpath not shipped”../tuiisn’t exported but a named TUI entry exists, warn and suggest removing it.file:TUI entries pointing to our package as valid when they export./tui; avoid false warnings.Written for commit 8fe5af7. Summary will update on new commits.