Skip to content

fix(tui): sync model preferences across clients - #43564

Open
opencode-agent[bot] wants to merge 1 commit into
v2from
sync-model-prefs
Open

fix(tui): sync model preferences across clients#43564
opencode-agent[bot] wants to merge 1 commit into
v2from
sync-model-prefs

Conversation

@opencode-agent

Copy link
Copy Markdown
Contributor

Summary

  • serialize model preference writes across TUI processes with the existing filesystem lock
  • watch model.json so favorites, recents, and variants update in already-running TUIs
  • apply preference changes as locked read-modify-write mutations so stale clients cannot overwrite unrelated changes

Validation

  • bun test in packages/tui (735 passed, 5 skipped)
  • bun typecheck in packages/tui
  • bun test test/mini-host.test.ts in packages/cli
  • bun typecheck in packages/cli

Requested by: @rekram1-node (Aiden via Slack)

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  1. packages/tui/src/model-preference.ts:139 — watcher.on("error") permanently closes the watcher with no retry, logging, or polling fallback — on filesystems where fs.watch fails (some network/overlay mounts) cross-client sync silently degrades to "never updates" and users get no signal why — at minimum log once; ideally fall back to periodic refresh() (e.g. every few seconds) when the watcher dies.

  2. packages/tui/src/context/local.tsx:367 — mutations are applied optimistically (setPreferences(...)) and then persisted via repository.addRecent/setFavorite/saveVariant; a remote echo arriving between the optimistic set and the locked read-modify-write can momentarily revert the local UI (or drop the just-set value if the watcher fires with pre-write contents) — the revision guard prevents stale delivery but not this ordering window; consider tagging locally-initiated writes so their echo is distinguishable, or accepting the transient flicker explicitly.

  3. packages/tui/src/model-preference.ts:126 — the watcher observes the entire directory of the preferences file and only filters by name; in a busy config/state directory this wakes the process on unrelated churn (and filename === undefined events always pass the filter) — cheap, but on high-event directories the 50 ms-debounced timer resets repeatedly and can starve refreshes; consider a max-deferral (e.g. fire after ≤500 ms regardless of further events).

  4. Nit — packages/tui/src/model-preference.ts:104 — await import("@opencode-ai/util/flock") runs inside every update() call; hoist it to a lazily-cached module-level promise so the hot path stays allocation-free and failures aren't re-attempted per call.

  5. Nit — packages/tui/test/model-preference.test.ts:70 — the subscription test relies on real fs events with a 2 s timeout; on CI runners without working inotify/FSEvents this becomes a guaranteed timeout failure rather than a skip — consider guarding on fs.watch availability or asserting via a direct refresh() invocation as well.

Overall: right fix — serializing read-modify-write through a file lock plus change broadcast solves the lost-update problem between concurrent TUI clients properly, and the moved helpers keep logic testable. Items 1–2 are worth a follow-up thought before merge. Thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants