Chore/llm providers expansion sync main by cuongdo-byterover · Pull Request #606 · campfirein/byterover-cli

cuongdo-byterover · 2026-05-05T03:50:24Z

Summary

Problem:
Why it matters:
What changed:
What did NOT change (scope boundary):

Type of change

Scope (select all touched areas)

Linked issues

Closes #
Related #

Root cause (bug fixes only, otherwise write `N/A`)

Root cause:
Why this was not caught earlier:

Test plan

Coverage added:
- Unit test
- Integration test
- Manual verification only
Test file(s):
Key scenario(s) covered:

User-visible changes

List user-visible changes (including defaults, config, or CLI output).
If none, write None.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording

Checklist

Tests added or updated and passing (npm test)
Lint passes (npm run lint)
Type check passes (npm run typecheck)
Build succeeds (npm run build)
Commits follow Conventional Commits format
Documentation updated (if applicable)
No breaking changes (or clearly documented above)
Branch is up to date with main

Risks and mitigations

List real risks for this PR. If none, write None.

Risk:
- Mitigation:

* chore: Bump veversion to 3.10.2 * chore: update CHANGELOG.md

* fix: [ENG-2535] surface daemon version in register ack + drift indicators Companion to the transport-client fix. Wires the daemon's own version through the register ack so MCP and TUI clients can render version-drift state without an extra round-trip, and adds a drift footer to MCP tool responses + a header tag in the TUI when the running daemon was started by a different brv build. - ConnectionCoordinator: accept daemonVersion option, surface it in the client:register ack response. Threaded through TransportHandlers so brv-server can pass the package.json-read version it already holds. - mcp-server: log a one-line stderr drift notice on connect and after every reconnect, when daemon and MCP versions differ. - brv-query-tool / brv-curate-tool: append a drift footer to text responses when client.getDaemonVersion() differs from the MCP version. - drift-footer: pure helper covered by 4 unit tests. - transport-store: add daemonVersion field + setter; transport-initializer pushes it on initial connect and on every reconnect. - header: render `[outdated, daemon vX.Y.Z]` in colors.warning when the TUI's version differs from the running daemon. - Bulk-update transport client mocks across 27 test files to include the new getDaemonVersion() interface method. - repro.sh: split verdict into BUG REPRODUCED vs FIX VERIFIED so the script doubles as a regression check. * chore: [ENG-2543] bump brv-transport-client to 1.1.0 Pin @campfirein/brv-transport-client to the released 1.1.0 tag now that the asymmetric daemon-version gate from ENG-2540 is published, replacing the branch ref that was used while the fix was in review. * fix: [ENG-2541] render daemon-version drift indicator inline in header banner Previously the drift token was rendered on a separate Box below the logo, which broke the header's single-line layout. Fold it into the banner next to the version, with padEnd recomputed so the trailing slashes still fill the row. * fix: [ENG-2541] address PR review nits on drift surfaces mcp-server.ts: flip versionsAreEquivalent args to (client, daemon) so all three drift surfaces (header, footer, log) use a consistent ordering. The function is symmetric so this is purely a readability change for anyone grepping the call sites. header.tsx: replace Boolean(daemonVersion) with daemonVersion !== undefined so TypeScript control-flow analysis narrows daemonVersion to string in the right operand of the && expression.

* chore: Bump veversion to 3.10.3 * chore: update CLAUDE.md * chore: update CHANGELOG.md * chore: pin exact version axios to 1.16.0

…pansion

github-actions · 2026-05-05T03:50:41Z

Claude finished @cuongdo-byterover's task in 3m 12s —— View job

PR Review: Chore/llm providers expansion sync main

Gather context and understand the PR
Read key changed files
Analyze correctness, security, and code quality
Post inline comments for specific issues
Post overall review summary

Summary

Overall score: 4/5

This PR syncs the version-drift detection feature from main into the proj/llm-providers-expansion branch. The change is cohesive and well-reasoned: when the TUI or MCP client connects to a daemon spawned by a different brv build, users now get a visible signal (TUI header badge + MCP tool footer) rather than silently experiencing degraded behavior.

What changed (core feature)

Layer	Change
Daemon	`TransportHandlers` → `ConnectionCoordinator` threads `daemonVersion` into the `client:register` ack
MCP server	Logs drift on connect/reconnect; passes `clientVersion` to tools
MCP tools (`brv-query`, `brv-curate`)	`appendDriftFooter()` appends a note when daemon ≠ client version
TUI store	New `daemonVersion` field + `setDaemonVersion` action
TUI header/logo	Renders `[outdated, daemon vX.Y.Z]` in warning colour when drift detected
Package	Bumped to `3.10.3`; `brv-transport-client` to `1.1.0`; `axios` to `1.16.0`

Strengths

Backward compatibility is explicit and tested: older daemons that don't include daemonVersion in the ack are silently tolerated everywhere — no false positives.
versionsAreEquivalent is used consistently: both the TUI and MCP paths delegate the semver comparison to the same library function, so prerelease/build-metadata variants don't trigger the indicator.
Good unit test coverage: drift-footer.test.ts, log-daemon-version-drift.test.ts, and the new connection-coordinator.test.ts cases cover the boundary conditions explicitly.
Test hygiene: every existing mock factory gets getDaemonVersion: stub(), keeping mocks aligned with the updated ITransportClient interface.

Issues found

setError doesn't clear daemonVersion (minor) — the drift indicator can persist visually during a disconnect/error state. Inline comment on transport-store.ts:98. Fix this →
Drift text can overflow on narrow terminals (minor, acceptable for v1) — calculatePadEnd returns '' rather than truncating, so the header could wrap on very narrow terminals. Inline comment on logo.tsx:159.
Two-return pattern in handleClientRegister (nitpick) — the early return for daemonVersion is functionally correct but can be a single merged return. Inline comment on connection-coordinator.ts:289.
Private method tested via (server as any) (nitpick) — logDaemonVersionDrift in mcp-server.ts is exercised through a cast, losing rename/signature safety. Long-term, extracting it as a free function (like appendDriftFooter) would allow type-safe testing. Inline comment on log-daemon-version-drift.test.ts:35.

Security & correctness

No security concerns. The version strings are read from the daemon ack (controlled infrastructure) and rendered as text — no eval or injection surface. The drift footer is informational-only and doesn't affect protocol behavior.

bao-byterover and others added 4 commits May 4, 2026 15:27

Rerelease/3.10.2 (#603)

bd912b9

* chore: Bump veversion to 3.10.2 * chore: update CHANGELOG.md

Release/3.10.3 (#605)

9a52d84

* chore: Bump veversion to 3.10.3 * chore: update CLAUDE.md * chore: update CHANGELOG.md * chore: pin exact version axios to 1.16.0

Merge remote-tracking branch 'origin/main' into proj/llm-providers-ex…

5eb365e

…pansion

cuongdo-byterover requested review from bao-byterover, leehpham, ncnthien and wzlng as code owners May 5, 2026 03:50

bao-byterover approved these changes May 5, 2026

View reviewed changes