fix(heartbeat): stop losing coding activity

[AnnatarHe]

Why

The server builds coding sessions from heartbeats (server/tasks/activity.sessions.go). A session closes after a 10-minute gap, and effort is session_end - session_start. Every heartbeat that is dropped shortens the measured effort.

What changed

• Heartbeats are re-queued when the daemon send fails. send_heartbeats used to empty the queue and drop the batch on any error, while :ShellTimeStatus reported the heartbeats as "queued". The new heartbeat.requeue() puts them back. The queue is capped at 5000 and drops the oldest first.
• Heartbeats are flushed on exit. Before, nothing was flushed on exit, so a session shorter than the 2-minute flush interval (open, edit for 90 s, :wq) was lost completely. sender.start() now registers a VimLeavePre autocmd that calls the new sender.flush_sync(), which waits up to 1.5 s.
• Edits that leave the cursor in place count again. The same-cursor duplicate check now applies to navigation events only (BufEnter, cursor moves). Edits that keep the cursor in place (x, dd) were being skipped.
• The .git pattern escapes its dot. In a Lua pattern, '/.git/' also matches /egit/, /_git/ and similar.
• UUIDs are built from uv.random. uuid() reseeded math.random with os.time() + os.clock()*1e6 on every call, so seeds overlap across Neovim instances. heartbeat_id is globally unique on the server and inserts use ON CONFLICT DO NOTHING, so a collision silently drops a heartbeat. It now uses uv.random(16). The fallback, math.random, is seeded once.

Correction: the first commit also switched the autocmds to args.buf, to fix what looked like :wa recording every write against the current buffer. That bug doesn't exist. During BufWritePost Neovim makes the written buffer current (aucmd_prepbuf), which I checked with a real :wa. The test that seemed to show the bug relied on nvim_exec_autocmds not switching buffers, and Neovim nightly does switch, so CI failed there. bf135ae reverts that part.

Testing

./scripts/test.sh with plenary passes on Neovim 0.11.4 and on nightly (0.13.0-dev): 263 passed, 0 failed. The new specs cover:

• requeue and the cap
• re-queue on a failed send
• flush_sync, including its timeout
• registration and removal of the VimLeavePre autocmd
• the .git vs egit pattern
• edits with an unchanged cursor
• the UUID fallback path

Unrelated issue I noticed but didn't change: ./scripts/test.sh <file> fails with "cannot open … {minimal_init = …}", because PlenaryBustedFile takes no options argument.

Companion PRs fix the same kinds of bugs in the VS Code and JetBrains plugins.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X5wQC8wrU4kY1jBfbzdquX

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Review

Overall this is a solid, well-scoped fix. The args.buf change, the navigation-only duplicate check, the requeue with a cap, and the uv.random UUID are all correct. The new tests cover the behavior well, including the fallback and timeout paths. It follows the CLAUDE.md conventions. A few points to consider, none blocking:

Potential issues

1. In-flight timer flush at exit (sender.lua, flush_sync): if the 2-minute timer's send is still in flight when VimLeavePre fires, its batch has already been removed from the queue. flush_sync only waits for its own (possibly empty) send, so Neovim can exit before the earlier batch finishes or is requeued. The window is small. Tracking an in_flight flag and waiting on it as well would close it.
2. Hidden-buffer writes and the duplicate check (heartbeat.lua, on_event): when get_cursor returns nil, nil (e.g. :wa on a buffer with no window), update_last_activity stores nil positions for that file. This is harmless today because writes bypass the duplicate check. Skipping the update when the position is nil would keep the state clean.
3. Retries can produce duplicates: if the daemon received a batch but the response timed out, requeueing resends it. This is safe given the server's ON CONFLICT DO NOTHING on heartbeat_id, but a comment next to requeue would stop anyone from "fixing" it later.
4. Slow exit when the daemon is down: flush_sync can block exit for up to 1.5 s. Skipping it when is_connected is already false would avoid that.

Nits / tests

• The exit flush tests call the real sender.start(). That starts a real timer and socket.is_connected_sync() via vim.schedule, so they may touch /tmp/shelltime.sock. Stubbing is_connected_sync would keep them hermetic.
• The UUID fallback test doesn't revert the uv.random stub if an assertion fails. Use after_each for the revert.
• trim_queue copies the list on every overflow, which is O(5000) per heartbeat while the daemon is down and the queue is full. Heartbeats are debounced, so this is cheap in practice. Trimming in batches would avoid it.
• The ./scripts/test.sh <file> breakage you noted is worth a follow-up, since CLAUDE.md documents that command.

Security: no concerns. Using OS randomness for IDs is an improvement.

LGTM with the optional follow-ups above.

[claude]

Review

Overall this is a solid, well-scoped fix, and the tests cover each behavior change. I found no blockers. Notes below, roughly by importance. I haven't run the suite myself; this is from reading the diff.

Potential issues

1. Exit flush misses in-flight sends (sender.lua, flush_sync). send_heartbeats empties the queue before the socket call. If the periodic timer has a send in flight when the user quits, flush_sync sees an empty queue and returns true immediately. Neovim then exits while that batch is still in the air, and if it fails, the re-queue never gets another chance. This is a narrow window. A small in_flight counter, with flush_sync also waiting for it to reach 0, would close it.
2. flush_sync return value is ambiguous. It returns true when the callback fires, even if the send failed (the callback gets success=false). The doc says "True if the send finished within the timeout", which matches, but a caller could easily read it as "delivered". Consider returning the success flag too, or renaming it.
3. Retries can duplicate heartbeats. If the daemon processed a batch but the reply timed out, the batch is re-sent. You note that heartbeat_id plus ON CONFLICT DO NOTHING covers this. Worth a one-line comment at the requeue call site so the next reader knows the retry is deliberately idempotent.
4. Windows path pattern (heartbeat.lua). [/\\]%.git[/\\] is correct. The hidden-.git check still doesn't cover a file named .git (a worktree pointer file). That's fine, just noting it.

Nits

• trim_queue uses vim.list_slice, which copies the table. It only runs on overflow, so there's no real cost. On add_heartbeat, overflow is at most 1, so table.remove(pending_heartbeats, 1) is O(n) too. Either is fine at 5000 entries.
• unpack is the global LuaJIT one. It's fine in Neovim, but table.unpack or unpack would be more portable.
• random_bytes calls vim.uv or vim.loop on each call. CLAUDE.md mentions vim.loop. This is harmless, but you could hoist it to module level.
• The seeded fallback seeds with hrtime + pid. That's good enough for a fallback.
• Tests: the UUID fallback test only reverts the stub on the success path. If an assertion fails, uv.random stays stubbed for later specs. Move the revert into after_each, or use finally/pcall.
• Tests: exit flush calls sender.stop() at the end of each test. A failed assertion before it would leak the augroup and timer. An after_each would be safer.
• There's no test that VimLeavePre actually invokes flush_sync. The registration test only checks that the autocmd exists. Firing nvim_exec_autocmds('VimLeavePre', ...) with a stubbed flush_sync would cover it.
• The note about ./scripts/test.sh <file> failing is useful. A follow-up PR would be welcome.

Security / performance

• Nothing concerning. uv.random is a better source of IDs than the clock-seeded math.random. The 5000-item cap bounds memory. The 1.5 s exit wait is bounded.

Nice work on the correction note about args.buf. Verifying that against real Neovim behavior rather than keeping the change was the right call.