[stefan-ernst]

Closes #496.

Time logging was blocking: the command palette's log-time entries navigated to
/time (their events had no listeners), the timer stop silently booked a
worklog, and a timer could not start without a project. This makes time logging
one in-place surface and lets the server fall back to a recoverable draft when
booking information is incomplete.

Behavior

• One global surface. A single TimeLogHost is mounted in the desktop and
  mobile shells. The command palette, item detail, /time, mobile, and the
  hotkey open the modal in place instead of navigating.
• Context-aware prefill. An open item detail (modal or page route), then the
  active timer, then the last-used project.
• Timers may start without a project. Project and description are optional
  at start; supplied associations are still authorization-checked.
• Stop books or drafts. Complete booking info (active, bookable project plus
  a description) books silently. Otherwise the server persists a user-owned
  draft worklog and the modal opens prefilled to complete it.
• Draft recovery. A Drafts (N) badge in the Time & Projects sidebar menu,
  plus a drafts card on the desktop Time view and the mobile timer view.
• Sidebar submenu. Time & Projects becomes a dropdown with a start/stop
  timer, project search, drafts badge, and navigation group.

Server

• time_worklog_drafts and time_worklog_bookings tables (SQLite + Postgres),
  with an optimistic version and a per-source-timer unique index.
• active_timers.project_id is now nullable, with a SQLite table-rebuild
  migration that preserves running timers and the timer-per-user unique index.
• Draft CRUD and book are ownership-scoped: another user's draft returns the
  same 404 as a missing one. Booking consumes exactly one owned draft atomically
  and replays idempotently by caller key.
• v1 keeps its frozen complete-only contract; a v1 client stopping an
  incomplete v2 timer gets a validation error and the timer is retained.
• v2 adds /time/worklog-drafts (list/get/patch/delete/book); the stop body
  identifies the intended timer and a retry recovers the original result.
• AI start_timer accepts an optional project/description; stop_timer
  reports booked vs draft.

Tests

Go tests live in the sibling core-tests repo on the matching
feat/496-seamless-time-tracking branch. Coverage: migration upgrade (running
timers preserved, nullability, uniqueness, draft tables), book-or-draft stop,
retry recovery, draft ownership 404s, idempotent booking replay, optimistic
version conflicts, and black-box v2 draft lifecycle/ownership.

Not yet covered: Playwright browser tests for the shell integration and the
sidebar submenu.

[github-actions]

Size Change: 0 B 🆕

Total Size: 0 B

_{compressed-size-action}

[stefan-ernst]

Review: changes requested

Reviewed head 1dbc20d. Four findings:

1. [P1] Draft booking bypasses project permissions — internal/services/timer_service.go:619
   Prepare() checks project status, not booking authorization. Confirmed via HTTP: a nonmember gets 404 creating a worklog directly, but 200 booking a draft into the same restricted project. Enforce CanBookTimeOnProject before booking.

2. [P1] Picker cache survives account changes — frontend/src/lib/stores/timeLogStore.svelte.js:42–59
   The singleton caches projects, customers, and items indefinitely, with no authentication reset. Logging out and signing in as another user without reloading exposes the previous user's cached data. Clear identity-scoped state and guard against stale requests.

3. [P2] Draft PATCH does not preserve omitted fields — internal/restapi/v2/time_drafts.go:88–101
   The merge-PATCH endpoint passes a zero-filled replacement into validation instead of merging with the draft. Confirmed: a request containing only version: 1 and description: Updated only returns 400 (invalid date format), requiring an omitted date. Merge supplied fields with stored values before validation.

4. [P2] UI never identifies the timer being stopped — frontend/src/lib/api/time.js:121–124
   TimerStore.stop() still calls timerApi.stop() without an ID. If another tab stops timer A and starts B, the stale tab's stop ends B. Pass the displayed timer's ID to activate the server's intended-timer protection.

Validation: Existing focused service and HTTP tests passed; two added HTTP regression checks reproduced findings 1 and 3. Browser and PostgreSQL suites were not run. The regression checks were added only in an isolated test worktree; no production changes were made.

Posted as a comment because GitHub does not allow a formal request-changes review from the PR author's account.

[stefan-ernst]

All four findings addressed in 91e0d79 (core) and 19bef255 (core-tests).

1. Draft booking bypasses project permissions — BookDraft now calls CanBookTimeOnProject after Prepare and returns ErrTimerForbidden on denial, mapped to the same 404 as a direct worklog create. The rejection happens before the atomic book, so neither the worklog nor the draft is consumed. Regression test: TestBookDraft_RequiresProjectBookingPermission (member-restricted project, asserts no worklog and the draft is retained).

2. Picker cache survives account changes — timeLogStore gained a reset() that clears picker data, drafts, and modal state, and a generation counter so an in-flight response from a previous account is ignored. It is wired into resetAuthenticatedShellState(), which runAuthenticatedShellBootstrap already calls on every authenticated bootstrap, so logout/login clears it without a reload.

3. Draft PATCH does not preserve omitted fields — the endpoint now reads the stored draft and merges supplied fields over it before validation (mirroring the worklog PATCH merge), so omitted date/duration keep their values. The draft path also tolerates a preserved sub-minute interval, which rounds to zero minutes but is a valid stored clock range. Regression test: TestV2DraftPatchPreservesOmittedFields.

4. UI never identifies the timer being stopped — timerStore.stop() passes activeTimer.id to timerApi.stop(), activating the server's intended-timer protection and retry recovery so a stale tab cannot end a newer timer.

Re-ran: ./internal/services/..., ./internal/repository/..., ./internal/database/..., ./internal/restapi/v2/..., the time-focused black-box suite, frontend typecheck, and the OpenAPI metadata/parity checks — all pass. Browser and PostgreSQL suites still not run.

[stefan-ernst]

Re-review of 91e0d79

The project booking authorization fix and intended-timer ID fix check out. The identity reset and merge-PATCH fixes still have two gaps:

1. [P1] Guard pending draft opens across identity resets — frontend/src/lib/stores/timeLogStore.svelte.js:89–92

   The generation check protects picker loads and refreshDrafts(), but openDraft() awaits its GET and then unconditionally calls open(). If account A starts opening a draft, the shell resets on logout/login, and the old response completes afterward, the modal reopens with account A’s draft description and associations in account B’s shell. A deterministic Vitest regression reproduces this: begin openDraft(), call reset(), resolve the old GET, then assert the modal remains closed and editingDraft remains null; the modal instead opens. Capture and check the generation around this await too.

2. [P2] Description-only PATCH still rejects same-second timer drafts — internal/services/time_worklog_service.go:397–398

   Start and stop persist whole Unix seconds, so a timer stopped within its starting second legitimately produces start_time == end_time and duration_minutes == 0. mergeDraftPatch() now preserves that interval, but PrepareDraft() still rejects endUnix <= startUnix with HTTP 400 (stored worklog interval is invalid). The original HTTP reproduction still fails on the new head. The newly added TestV2DraftPatchPreservesOmittedFields waits 1100 ms before stopping, avoiding rather than covering this case. Permit preservation of this server-produced zero-length draft interval without weakening finalized-worklog validation, and cover it deterministically.

Validation: The updated focused service/HTTP suite passes. The original restricted-project HTTP regression now passes; the original immediate-stop PATCH regression still fails. Frontend checks pass for stale picker-response rejection and passing the displayed timer ID, but fail for an in-flight draft open crossing reset. Browser and PostgreSQL suites were not run.

Commands run from the isolated review directory:

./core-tests-pr506-r2/overlay.sh ./core-pr506-r2 -- -tags=test -count=1 -run "Test(StopActiveTimer|DraftOwnership|BookDraft|UpdateDraft|DraftsInvisible|V2TimerDraft|V2Draft)" ./internal/services/... ./tests/...
./core-tests-pr506/overlay.sh ./core-pr506-r2 -- -tags=test -count=1 -run TestPR506Review ./tests/...
VITEST_RUNTIME=bun ./core-tests-pr506-r2/run-overlay-script.sh ./core-pr506-r2 scripts/run-frontend-tests.sh src/lib/stores/timeLogStore.review.test.js src/lib/stores/timerStore.test.js

Review-only regression checks remain in isolated test worktrees; no production changes were made.

[stefan-ernst]

Both gaps fixed in 8c817e2 (core) and 82749c08 (core-tests).

1. Guard pending draft opens across identity resets — openDraft() now captures the generation before its GET and re-checks it before calling open(), so a reset during the request leaves the modal closed with editingDraft null. Deterministic Vitest regression added (timeLogStore.svelte.test.js): begin openDraft(), reset(), resolve the old GET, assert show === false and editingDraft === null; plus a reset-clears-caches case.

2. Description-only PATCH rejects same-second timer drafts — the preserved-interval check now allows start_time == end_time on the draft path (requirePositiveDuration == false) while Prepare still requires a strictly positive span for finalized worklogs. Deterministic service test TestPrepareDraft_AllowsZeroLengthTimerInterval covers both sides, and TestV2DraftPatchPreservesOmittedFields no longer sleeps so the HTTP path exercises an immediate stop.

Re-ran ./internal/services/..., ./internal/repository/..., ./internal/database/..., ./internal/restapi/v2/..., the frontend Vitest regression, and the OpenAPI checks — all pass. Browser and PostgreSQL suites still not run.