Test/multi display usage tracking - #1
Merged
Conversation
Adds a second Swift helper, screen-memory-appstate, which answers a newline-delimited command protocol on stdin. It reports the frontmost application and, per display, that display's frontmost window with the share of the screen it covers. It never reads kCGWindowName, so usage tracking keeps working if screen recording permission is revoked. The process is long-lived because usage polls every two seconds. Three features build on it: Per-display review. A toolbar filter narrows the day to one display and persists across scrubbing, playback and day changes; a day where that display has no data falls back to showing all screens and restores the choice on return. Clicking a screenshot opens a lightbox with zoom, pan, frame stepping, and Copy / Save / Reveal in Finder. Those three IPC handlers take the relative path and reject anything that resolves outside the storage root. App exclusion. Apps listed in Settings are skipped on a display while they are its frontmost window and cover at least a configurable share of it, default 80%. Coverage rather than strict fullscreen equality is what catches maximized and near-fullscreen windows. Only that display is skipped, and skipping happens before the file write, so no JPEG, no row and no OCR. The threshold applies live. Usage tracking. A new Usage view shows per-app totals and an hour-by-hour strip for the selected day. Tracking runs independently of capture, so it continues while recording is paused, and idle time closes the open segment. Segments are written on open and updated each tick so a crash cannot lose an interval. Totals also feed the AI summary. Migration 0001 only creates app_usage and its index. Existing Drizzle databases take it incrementally; the legacy migrator needs no change because its staging database runs the whole migration folder. Everything degrades to a no-op when the helper binary is absent: capture, timeline and lightbox work as before, the exclusion settings render disabled, and the Usage view shows an empty state.
Adds a CI workflow that runs the formatter check, lint, typecheck, the Swift helper build and a full electron-builder package on every push and pull request, uploading the signed .dmg and .zip as an artifact. The helper binaries are checked explicitly because a missing extraResources entry otherwise fails late inside electron-builder. Release stays opt-in: it keeps its v* tag trigger and gains a manual dispatch so a build can be published from any branch, defaulting to a prerelease. Its publish target now follows the repository running it, so a fork releases to itself rather than failing against the upstream repo.
The legacy database runs in WAL mode, so it is three files, not one. The migration treated it as one and lost data in two ways. The backup was a plain copyFileSync of the main file. Any rows committed but not yet checkpointed still live in the -wal, so a database that was force-quit backed up short — and the row-count check couldn't catch it, because it compared the staging database against that same lossy backup. Both sides were missing the rows and agreed. Checkpoint first, then copy, then verify against the live database instead. The swap unlinked the main file and renamed staging over it, leaving the old -wal behind for SQLite to replay into the migrated database. A repro against real files confirms it: the swapped-in database reads back the old rows, not the migrated ones. The swap now parks the original and its sidecars aside by rename and removes them only once the new database is in place, so no instant exists where neither is there. An interrupted swap used to leave no database at all, which the next launch read as a fresh install and silently replaced the user's history with an empty one; recoverInterruptedSwap() now runs before that check and finishes or unwinds the swap.
electron-builder names the release after the package.json version rather than the git ref, so pushing v0.2.0 without bumping package.json quietly published to v0.0.1. The workflow now reconciles the two and fails with the fix spelled out instead of releasing under the wrong tag; a manual run takes the version as an input and bumps package.json so the derived tag matches. Republishing was worse than wrong — electron-builder refuses to touch a published release older than two hours and still exits 0, so a rerun looked green while uploading nothing, and inside that window it would overwrite a live release's assets instead. A guard now fails the run when the tag is already published, manual runs default to a draft, and a concurrency group keeps two runs from racing for the same tag.
Four ways the helper wrapper misbehaved when the child process wasn't healthy. Any request with no live child spawned one immediately, straight past the respawn backoff — against a helper that crashes on startup, the 2s usage poll restarted it forever. Requests now fail soft while a respawn is pending, which is what every caller already expects, and the backoff gets to do its job. Exit was tracked with 'exit', which never fires when spawn itself fails, so a binary that isn't executable or isn't a valid Mach-O left a dead child in place and every later request timed out against it. Tracking 'close' covers both, and isAvailable() now checks the executable bit so that case reports the feature as off instead. stop() left the door open: pump() flipped `started` back on, so a request racing shutdown would spawn a helper as the app was quitting. The pipes had no 'error' listeners. Writing to a helper that just died raises EPIPE there, and an 'error' event with nothing listening is fatal in Node. Detaching a killed child now also drops its stdout 'data' listener, so a line still buffered in it can't settle whichever request the next child is serving.
Two independent holes let an excluded app land in the archive. capture() read the front window only after desktopCapturer.getSources() had already grabbed the pixels, so an app that was frontmost at grab time but dismissed before the check was captured anyway; the mirror case (an app coming forward during the grab) slipped through a before-only check. It now reads both sides of the grab and skips the display if either read sees an excluded app. Reading twice only means something if the second read is fresh, so getState() takes a max age and the post-grab call passes 0 to bypass the 1s memo the pre-grab call just filled. In the helper, a display's frontmost window was the first window with any non-empty intersection, so a window overlapping a neighbouring display by a pixel became that display's dominant app and suppressed the real one. Each window is now assigned to the single display it predominantly occupies, computed once for all displays. The Swift change is unverified locally: the Command Line Tools install on this machine has an SDK/compiler mismatch, so build:swift fails. CI builds it.
The lightbox was rendered inside whichever branch of ScreenshotViewer was active, so playback advancing the timestamp flipped the component between its focused and grid branches and remounted the dialog, throwing away zoom, pan and the frame the user had arrowed to. The branches now assign to one variable and the lightbox renders at a fixed position after it, keyed by the shot it is showing. Opening it also stops playback, since frames changing underneath an open lightbox was never useful. Display labels were derived independently in ScreenshotViewer, so a display could be "Display 2" in the toolbar and "Display 1" in the lightbox header. The labels from useDisplayFilter are threaded through and the local numbering is only a fallback for a display the filter has not seen. The coverage threshold input's min/max only coloured the field: a typed 0 or 500 was saved verbatim. It now persists only values in 1-100 and clamps on blur, keeping the field editable while a partial value is typed.
getUsageTotals summed min(ended_at, end) - max(started_at, start) without a floor, so a segment lying entirely outside the range contributed negative time and could cancel out real usage. The per-segment term is now wrapped in max(0, ...). Both range queries filtered on ended_at >= start, which idx_app_usage_started cannot serve, so the scan grew with the whole table. Segments close on idle, sleep, lock and quit, so none can outlive a day; adding started_at >= start - 1 day bounds the scan without changing the result. Idle time was credited to the tick that noticed the idle rather than to the last input, handing the app up to the full 120s threshold of time the user was not there. IdleDetector exposes the idle seconds and the segment closes at the last-active instant. Segments now carry startedAt so a close can never write an ended_at before the start. pickApplication returned null for both "user cancelled" and "could not read that bundle", so a failed pick looked like a cancel and the app was silently not added. Unreadable bundles now throw. useAppUsage reported available: true with no segments when the load failed, rendering a database error as "no app usage recorded for this day". The error is carried through and shown as an error. Adjacent bands in the hour strip whose bundle ids hashed to neighbouring hues read as one block; each band draws a hairline of page background on its leading edge, and its tooltip names the app.
The migrations folder and the two Swift binaries are present in any dev checkout, so a packaging regression that drops them is invisible until a user runs the built app and migrations fail. Both workflows now check the packaged .app for a parseable, non-empty _journal.json, at least one .sql migration, and both helper binaries present, executable and Mach-O. Exercised against a fixture in all four states: complete, missing journal, missing helper, and a non-Mach-O file in a helper's place.
Each carries the project-specific knowledge that is otherwise re-derived every session: the four edits an IPC method needs, the storage-root path rule, the kCGWindowName permission constraint, the broken local Swift toolchain, and the bug classes this codebase actually produces.
The previous fix assigned each window to the single display it overlapped most and let a display consider only windows homed to it. That reopened the hole it was closing, in a worse form: a window can cover display B entirely while having more raw area on the larger display A, so B ends up with no dominant window at all, no bundle id reaches the exclusion check, and B is captured with the excluded app filling it. Concretely, a laptop at 1728x1117 beside a 2560x1440 external, tops aligned, with a window dragged across the boundary: 100% of the laptop but only 1.93M px there against 2.41M on the external, so the laptop reports nothing. Before the exclusivity rule it was correctly skipped. Replaced with a floor: a window has to occupy at least 2% of a display before it can be that display's frontmost window. A one-pixel spill no longer masks the app behind it, and a window filling two displays is reported on both. Also removes the tie-break where two displays with identical bounds left the second one permanently empty. Still not compiled locally — the Command Line Tools install on this machine has an SDK/compiler mismatch. CI builds it.
Number('') is 0 and Number.isFinite(0) is true, so clearing the field and
clicking away took the finite branch, clamped 0 up to 1 and saved it — the
DEFAULT_THRESHOLD fallback was unreachable. The setting applies live, so
from then on any excluded app that was merely frontmost and covered 1% of
a display suppressed that display's screenshots entirely: a small Messages
window in a corner silently blanks a whole monitor out of the archive.
An empty or unparseable field now means "no new value" and leaves the
saved one alone. type="number" also yields "" for partial entries like
"-" or "1e", which took the same path.
Day queries only look back MAX_SEGMENT_SPAN_MS for a segment that might overlap them. That bound was justified by an argument — segments close on idle, sleep, lock and quit — rather than enforced, so anything that keeps the HID idle timer from reaching 120s (caffeinate -u, a jiggler, some remote-desktop stacks) could extend one row past a day. It would then vanish from every day it covers but the first, with no warning: the app used all day shows zero. UsageService now closes and reopens the segment once it reaches the span, which makes the bound an invariant instead of an assumption. The constant moved to shared/constants.ts so the query and the writer cannot drift. Also corrects the previous commit message on this file: the max(0, ...) added there is defensive, not a fix. The WHERE clause was already the full overlap predicate, so the clamped term could only go negative on a row with ended_at < started_at, which nothing can write.
getState(0) skipped the cache and then returned this.inFlightState unconditionally, so a caller demanding the world *now* was handed whatever round-trip happened to be open — possibly one that started before the thing it was trying to observe. Capture's post-grab read depends on that freshness, and got it only by accident of timing. An in-flight request is the same staleness as the cache, so it now answers to the same age limit. The finally clears the field only when it is still the current request, since a fresher one can now replace it. Hardening: I could not construct a sequence where the old code produced a wrong capture decision, because the pre-grab read drains any pending request before getSources() starts.
The check claimed to catch a helper built for the wrong architecture, but `file | grep Mach-O` accepts an x86_64 binary in an arm64 bundle just as happily — exactly the build that spawns, dies, and surfaces as "feature unavailable" rather than as a broken package. It now compares lipo -archs against an expected arch, passed explicitly by both workflows. Exercised against fixtures in six states: complete, wrong-arch helper, non-Mach-O helper, missing helper, empty journal, and no .sql migrations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.