Claude finished @Yeraze's task —— View job

PR Review: fix(meshcore): keep focus on the CLI command line after sending

Tasks:

• Read changed files
• Review code quality and best practices
• Check for potential bugs
• Evaluate test coverage
• Post final review

Summary

This is a clean, well-scoped fix. The approach is idiomatic React and the implementation is correct. Here's a detailed breakdown:

Code Quality

CliConsoleBody.tsx — The implementation is solid:

• commandInputRef (line 154) + refocusAfterSendRef (line 155) follow the existing pattern of refs in this component.
• Setting refocusAfterSendRef.current = true at the start of runAndLog (line 209) before setSending(true) is correct — it ensures the flag is always set before the effect can fire.
• The useEffect at lines 241–246 is correctly guarded: it fires only when sending transitions to false AND the flag is set, so it won't steal focus on initial mount or on unrelated disabled transitions. This addresses the comment in the PR description precisely.
• commandInputRef.current?.focus() at line 244 respects the disabled prop — no focus is attempted if the console is already locked (e.g., remote console logged-out state). That's a nice edge case handled correctly.

One minor observation: The refocusAfterSendRef flag is reset to false inside the effect (line 243) — but if disabled is true at the time sending clears, focus is skipped and the flag is cleared anyway. This means if the console becomes disabled during a send (a session expiry race), focus won't be restored when disabled later goes back to false. This is likely an acceptable trade-off (it's an unusual edge case and the fix is for the common path), but worth noting.

Test Coverage

CliConsoleBody.test.tsx — Good regression test:

• The vi.mock('react-i18next', ...) mock is correct and follows the project pattern.
• The test covers the primary path: typing a command → clicking Send → asserting document.activeElement === input.
• The expect(input.value).toBe('') assertion at line 40 validates the field-clear behavior, which is a secondary but important UX check.

Coverage gaps (optional, non-blocking):

1. The test doesn't cover the quick-action or danger-confirm paths. Those are covered by the description ("routes through the single runAndLog choke point"), but explicit tests for those paths would increase confidence.
2. No test for the disabled guard in the focus effect (i.e., that focus is NOT restored when disabled=true at the time sending clears). This is a minor omission.

Performance

No concerns. Using useRef + a boolean flag avoids any unnecessary renders. The useEffect dependency array ([sending, disabled]) is minimal and correct.

Security

No concerns.

Verdict

LGTM. The fix is minimal, correct, and well-tested for the primary regression path. The edge case around disabled flipping during a send is unlikely in practice and the trade-off is reasonable.