fix(heartbeat): stop losing, duplicating and misattributing coding activity

[AnnatarHe]

Why

The server builds coding sessions per machine from heartbeats (server/tasks/activity.sessions.go). A session closes after a 10-minute gap, and effort is session_end - session_start. A heartbeat that is lost, duplicated, recorded against the wrong project, or reported under a different machine name skews the measured effort.

What changed

• A save is recorded once, in the right project. FileSaveListener sent every save to every open project, so with N projects open one save became N write heartbeats, each named after a different project's basePath. Now only the first open project that has the file open records it. If none does, it falls back to ProjectLocator.guessProjectForFile.
• Editors restored at startup are tracked. ShellTimeProjectService is created lazily, by the first fileOpened event, after the restored tabs already exist. EditorFactoryListener.editorCreated never fired for those tabs, so typing in them sent nothing until the tab was reopened. The collector now registers its document and caret listeners once, on EditorFactory.eventMulticaster, filtered to this project's editors. It no longer registers per editor. The old per-editor listeners also outlived their editors (leak) and stacked up when a file was reopened.
• Settings changes take effect without a restart. ShellTimeConfigurable.apply() never called ShellTimeProjectService.updateSettings(). Unchecking "enabled" didn't stop caret or document collection, or the sending. Enabling after the IDE started disabled never started tracking. Interval changes were ignored. apply() now updates every open project, and updateSettings() stops, starts or resumes the collector and the sender.
• Nothing is lost on project close or IDE exit. dispose() used to clear the pending queue and the failed-retry queue. It now flushes them first, waiting at most 2 s.
• The machine name matches the CLI's. InetAddress.getLocalHost().hostName passes gethostname() through getaddrinfo and a reverse getnameinfo on Unix, so it can return e.g. host.lan or an FQDN. The CLI, VS Code and Neovim all report plain gethostname(). When the names differed, JetBrains activity landed in separate sessions from terminal and AI activity on the same computer: overlapping time was counted twice, and sessions were fragmented. The name now comes from /proc/sys/kernel/hostname, then hostname, then env vars, with InetAddress last. It is computed once; before, it was looked up for every heartbeat on the EDT. ⚠️ Heartbeats already stored keep the old name.
• Projects under directories like build/ are tracked. The exclude patterns matched the absolute path, so a project located under any build/, out/, target/ or vendor/ directory (e.g. /home/ci/build/repo) was never tracked. They now match against the path inside the project.
• Edits that leave the caret in place count. The same-caret duplicate check now applies to navigation only.

Testing

• The JetBrains build couldn't run in my environment, because its network policy blocks download.jetbrains.com and www.jetbrains.com, so ./gradlew test / buildPlugin could not fetch the IntelliJ Platform. CI (testing.yml) is the authoritative compile and test run for this PR.
• What I could run: I compiled the real SystemUtils.kt and ProjectUtils.kt in a scratch Kotlin 2.3 / JDK 17 project. It used compile-only stubs for the three IntelliJ types ProjectUtils imports, which shouldExclude doesn't touch. All 17 tests in the real SystemUtilsTest and ProjectUtilsTest pass, including the new ones:
  • excluded directory names above the project root
  • Windows separators
  • the machine name equals the kernel hostname on Linux
• The collector, service, listener and settings changes use long-standing platform APIs (EditorEventMulticaster.addDocumentListener/addCaretListener(listener, disposable), EditorFactory.getEditors(document, project), Disposer.newDisposable(parent, name), ProjectLocator.guessProjectForFile, FileEditorManager.isFileOpen). I reviewed them by hand, but they are not compiled yet.

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01X5wQC8wrU4kY1jBfbzdquX

---

Generated by Claude Code

[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

The fixes line up well with the described bugs: single-project save attribution, multicaster-based listeners, settings applied live, flush on dispose, a stable hostname, and exclusion relative to the project root. The new tests cover the pure-function parts well. I couldn't build this here, and the PR says the collector, service and listener changes aren't compiled yet, so CI should be treated as the gate.

Issues worth addressing

1. runBlocking in ShellTimeProjectService.dispose() can freeze the UI for up to 2s per project.
   Project disposal often runs on the EDT, and closing several projects, or exiting the IDE, stacks the waits. Consider these options:

   • Keep the timeout short.
   • Do the final flush from a ProjectManagerListener.projectClosing or an app-level close hook that isn't on the EDT.
   • Make the socket write non-blocking.

   Also note that catch (e: Exception) around runBlocking swallows ProcessCanceledException, which should be rethrown. Please make sure scope is cancelled in dispose() too. I couldn't see that part in the diff.

2. SystemUtils.getMachineName() can still block the EDT.
   by lazy is synchronized. If a heartbeat is created before the scope.launch { getMachineName() } warm-up finishes, the first call runs readLinuxHostname, then hostname with a waitFor of up to 2s, on the calling thread (likely the EDT). The warm-up is racy by design. A safer option is to warm it up in an app-level startup activity, or have createHeartbeat read it lazily off-EDT.

   • On Windows and macOS, hostname is a process spawn. That's fine once, but COMPUTERNAME is already the cheaper source on Windows.
   • The test getMachineName matches the kernel hostname on Linux only covers Linux. The macOS and Windows paths are untested, which is acceptable but worth noting.

3. ShellTimeConfigurable.apply() calls it.service<ShellTimeProjectService>() for every open project.
   The service is created lazily on the first fileOpened event, so this force-instantiates it, and starts tracking, in projects with no editors open. That is probably fine, since updateSettings() calls start(). But apply() runs on the EDT, and start() does sender.start() and collector.start(). Please confirm that none of that does blocking I/O such as socket connect. Otherwise, use getServiceIfCreated and skip projects that haven't started.

4. Thread safety of listenerDisposable.
   start() and stop() can be called from apply() (EDT) and from dispose() or a coroutine. The field is a plain var with a check-then-set. Mark it @Volatile or guard it with synchronized. The same goes for lastActivity, which is now written from both document and caret handlers.

Smaller notes

• handleDocumentChange sets lastActivity unconditionally before the debounce check. This is intended, per the PR description, but add a short comment, because it is subtly different from isDuplicateNavigation.
• findEditor calls FileEditorManager.selectedTextEditor. The multicaster only fires on the EDT, so this is fine, but a short comment would help future readers.
• FileSaveListener.findProject: isFileOpen is checked per project in openProjects order. If the same file is open in two projects, only the first records it. That matches the PR intent. With no open editor, guessProjectForFile may return a different project than the one the user is working in, which is acceptable.
• shouldExclude: matching is case-sensitive, so Build/ on Windows or macOS isn't excluded. This is the same as before. If projectBasePath is / it falls back to the full path, which is fine. It would be worth a test for a file exactly equal to the base path.
• In dispose(), sender.flush() is attempted after sender.stop(). Please confirm that flush() doesn't depend on running state, and that the failed-retry queue is also drained, as the PR claims.
• catch (e: Exception) { } in dispose() has no logging. At least log it when debug is enabled.

Test coverage

The utility tests are good. The riskiest changes have no tests:

• the collector's start/stop and multicaster filtering
• updateSettings transitions (enabled → disabled → enabled)
• FileSaveListener project selection
• the flush-on-dispose path

These would need the IntelliJ test fixtures or MockK. Even a MockK-based test of the updateSettings state transitions would help.

Overall this is a solid improvement. I'd like to see items 1–3 resolved or confirmed as non-issues before merge.

[codecov]

Codecov Report

❌ Patch coverage is 11.49425% with 77 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...helltime/jetbrains/heartbeat/HeartbeatCollector.kt	0.00%	35 Missing ⚠️
...time/jetbrains/services/ShellTimeProjectService.kt	0.00%	17 Missing ⚠️
...otlin/xyz/shelltime/jetbrains/utils/SystemUtils.kt	15.78%	14 Missing and 2 partials ⚠️
...helltime/jetbrains/listeners/FileEditorListener.kt	0.00%	5 Missing ⚠️
...helltime/jetbrains/config/ShellTimeConfigurable.kt	0.00%	3 Missing ⚠️
...tlin/xyz/shelltime/jetbrains/utils/ProjectUtils.kt	87.50%	0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (11.49%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (14.21%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
- Coverage   14.30%   14.21%   -0.10%     
==========================================
  Files          20       20              
  Lines         755      802      +47     
  Branches      172      184      +12     
==========================================
+ Hits          108      114       +6     
- Misses        630      669      +39     
- Partials       17       19       +2     

Files with missing lines	Coverage Δ
...tlin/xyz/shelltime/jetbrains/utils/ProjectUtils.kt	37.09% <87.50%> (+6.73%)	⬆️
...helltime/jetbrains/config/ShellTimeConfigurable.kt	0.00% <0.00%> (ø)
...helltime/jetbrains/listeners/FileEditorListener.kt	0.00% <0.00%> (ø)
...otlin/xyz/shelltime/jetbrains/utils/SystemUtils.kt	17.85% <15.78%> (-17.86%)	⬇️
...time/jetbrains/services/ShellTimeProjectService.kt	0.00% <0.00%> (ø)
...helltime/jetbrains/heartbeat/HeartbeatCollector.kt	0.00% <0.00%> (ø)

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a840c18...bad1500. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AnnatarHe]

CI status: test (build plus unit tests against IntelliJ 2024.3.5) is green, so the collector, service and listener changes compile and pass. Only the two Codecov checks are red.

• codecov/project is not caused by this PR. main is already at 14.30% against a 60% target. This PR moves it to 14.21%.

• codecov/patch (11.49% vs 70%) is caused by this PR. The uncovered lines are in HeartbeatCollector, ShellTimeProjectService, FileSaveListener and ShellTimeConfigurable. Those classes are bound to the platform and have 0% coverage on main as well, because the repo has no platform test harness. Covering them needs one of two things:

  • IntelliJ test fixtures, which means switching to TestFrameworkType.JUnit5, or adding the vintage engine for BasePlatformTestCase
  • substantial MockK static mocking of EditorFactory, FileDocumentManager, ProjectManager and similar

  I couldn't run either where I made this change, because its network blocks the JetBrains downloads. I've asked the author whether to add such a harness here, rather than adding one untested.

Review items 1–3, traced against the code:

1. runBlocking in dispose(). The flush only takes long when the daemon accepts the connection and then hangs.
   • If the daemon is down, the socket file is missing and connect fails immediately.
   • If the daemon is healthy, the round trip takes milliseconds.
   • scope.cancel() is still called at the end of dispose().
2. getMachineName() on the EDT. At worst the first call runs one hostname process spawn, and /proc is read on Linux. Before, InetAddress.getLocalHost() ran a DNS lookup on the EDT for every heartbeat, which is the well-known macOS stall. This is strictly cheaper.
3. apply() creating services. start() registers the multicaster listeners, then launches coroutines for the timer, the version check and the hostname warm-up. None of that does blocking I/O on the EDT.

Also checked: sender.flush() doesn't depend on the running state, and it drains failedHeartbeats before new heartbeats. listenerDisposable is only touched from EDT paths: settings apply, project dispose, and service init on the first fileOpened.

---

Generated by Claude Code