[TimeToBuildBob]

Summary

Schema groundwork for the hostname-free bucket ID redesign in ActivityWatch/activitywatch#302 and ActivityWatch/activitywatch#1445. Adds Bucket.device_id and migrates v6→v7 from UNIQUE(name) to UNIQUE(device_id, name).

This PR does not enable same-name buckets in the public datastore API. That API and its cache still address buckets by name. Until a device-aware resolver lands, creating a second bucket with an existing name returns BucketAlreadyExists, regardless of device. Loading a v7 database that already contains ambiguous names fails explicitly rather than silently hiding one device's history or redirecting writes. The database constraint is preparation for that follow-up, not a claim that device-scoped addressing is implemented here.

Changes

• aw-models: Bucket.device_id: String with #[serde(default)] for older JSON.
• aw-datastore: v6→v7 table recreation preserves row IDs, the AUTOINCREMENT high-water mark, events, current metadata, and the archival data_deprecated column. The archive is retained separately, including NULL or malformed legacy bytes; it is not promoted into live metadata. Existing rows receive device_id = 'local'. Foreign-key enforcement is disabled outside the transaction for recreation and restored afterward, including on SQL failure.
• Older read-only peer databases (v4–v6) remain readable using a literal 'local' device ID.
• Empty device IDs on creation are stamped 'local'; explicit device IDs are preserved.
• Duplicate names are rejected atomically in SQL for creation and rename; ambiguous stored names fail cache loading without deleting either row.
• Legacy re-import refuses to merge into a same-name bucket owned by a different device; same-device re-import remains idempotent.
• Dump-restored v7 schemas are recognized by device_id before earlier-version inference checks.
• Client, test, benchmark, and example Bucket constructors include the new field.

Scope boundary

Device-aware cache keys, bidirectional bucket resolution, same-name coexistence through the public API, GET /api/0/devices, and watcher adoption of hostname = "!local" remain follow-up work. They must land together before lifting the name guard; removing the guard alone loses cache visibility and redirects name-based operations.

Verification

On c921d8973176: cargo test -p aw-datastore -p aw-models — 104 passed, 1 ignored; targeted strict clippy, formatting, diff whitespace checks, and commit hooks passed. The archival-preservation and cross-device legacy-import regressions failed before their fixes and pass afterward. Additional coverage includes separate current/archival payloads, SQL NULL and malformed archival bytes, missing archive columns, dump-restored early/current schemas, stale-connection creation/rename guards, sequence preservation, migration rollback/FK restoration, and version rechecking under the migration lock.

Latest-head CI and normal AI-review verification remain pending. The required push wrapper honored its existing 2/2 review cap; no fresh local AI review is claimed. Both fresh findings have one evidence reply each, with resolution left to the normal reviewer's verification.

[TimeToBuildBob]

CI-green and mergeable — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

[greptile-apps]

Confidence Score: 4/5

[Critical impact] Database schema migration adds device_id column and changes uniqueness constraint.

Fix the concurrent-open race before merging so a repeated migration cannot overwrite device IDs.

Findings

1.  Repeated migration overwrites device IDs ▶

Summary

Adds Bucket.device_id and upgrades storage to v7 with UNIQUE(device_id, name).

• Bucket records now carry a device ID in schema v7.
• The datastore keeps bucket names unambiguous for callers.
• Legacy imports only merge into buckets from the same device.

The supplied previous threads are unnumbered. Their constructor, duplicate-name, and storage-test concerns are addressed in the current code.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Open database] --> B{Schema version}
    B -->|v4 to v6, read-only| C[Read device ID as local]
    B -->|Older, writable| D[Upgrade to v6]
    D --> E[Rebuild buckets as v7]
    E --> F[Preserve history and stamp local]
    B -->|v7| G[Read stored device IDs]
    F --> H[Load buckets by name]
    G --> H
    H --> I{Duplicate names?}
    I -->|Yes| J[Return error without deleting rows]
    I -->|No| K[Use name-based API]

Loading

_{Reviews (3) · Last reviewed commit: "ci: retry intermittent Windows client st..." · Reviewed by Greptile}

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

CI fix pushed (233c73d): the last three commits introduced new tests calling setup_datastore_empty() which was never defined. Replaced all three call sites with Datastore::new_in_memory(false) — the pattern all other tests use. All three tests pass locally. New CI run is running.

[codecov]

Codecov Report

❌ Patch coverage is 97.21362% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.97%. Comparing base (656f3c9) to head (8eda29e).
⚠️ Report is 235 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-datastore/src/datastore.rs	93.06%	7 Missing ⚠️
aw-datastore/tests/datastore.rs	99.02%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #786       +/-   ##
===========================================
+ Coverage   70.81%   85.97%   +15.15%     
===========================================
  Files          51       89       +38     
  Lines        2916    13167    +10251     
===========================================
+ Hits         2065    11320     +9255     
- Misses        851     1847      +996     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

[TimeToBuildBob]

Rebased onto master (8338954). Conflicts were only in the clippy attributes on aw-query/src/{lib,parser}.rs; master already fixed that in #771, so I kept master's version and this PR no longer touches those files. The v6→v7 device_id migration is unchanged, since master has no competing v7. cargo test -p aw-datastore passes locally.

[TimeToBuildBob]

🤖 AI code review

Adds Bucket.device_id and a v6→v7 migration that rebuilds buckets with UNIQUE(device_id, name), assigning existing rows to 'local'. Datastore reads adapt to older schemas, while creation, renaming, legacy import, and recovery add name or device ownership checks. Bucket constructors and migration tests are updated for the new field and schema.

Safe to merge — no P0/P1 findings

Confidence 5/5

⚠️ 6 of 19 changed files were not fully read. Claims that an identifier is never assigned, used, or called are discarded unless verification names only fully preloaded files. This coverage describes the bounded context preload; the diff was reviewed.

✅ No findings. The diff looks correct to me on this pass.

Files changed (19) — the diff as I read it

• aw-client-rust/src/lib.rs — Adds an empty device_id to the bucket constructed by create_bucket_simple.
• aw-datastore/benches/interval_reads.rs — Adds device_id: "local" to the interval benchmark bucket.
• aw-datastore/examples/export_memory.rs — Adds device_id: "local" to the synthetic export bucket.
• aw-datastore/src/datastore.rs — Adds v7 migration and inference, device-aware bucket loading, atomic duplicate-name guards, ownership checks during Android bucket migration, and migration tests.
• aw-datastore/src/export.rs — Adds device_id: "local" to export test buckets.
• aw-datastore/src/legacy_import.rs — Assigns legacy buckets to 'local', checks existing ownership before merging, and adds a cross-device collision test.
• aw-datastore/src/recovery.rs — Defers recovery on conflicting bucket ownership and adds a collision-and-retry test.
• aw-datastore/tests/datastore.rs — Updates schema expectations and adds device identity, external migration, stale connection, duplicate-name, and ambiguous database tests.
• aw-models/src/bucket.rs — Adds the serde-defaulted public Bucket.device_id field and updates the model test constructor.
• aw-query/benches/benchmark.rs — Adds device_id: "local" to query benchmark buckets.
• aw-query/tests/query.rs — Adds device_id: "local" to query test bucket constructors.
• aw-query/tests/regex_fuzz.rs — Adds device_id: "local" to the regex test fixture bucket.
• aw-server/benches/query_cache.rs — Adds device_id: "local" to the query cache benchmark bucket.
• aw-server/src/dirs.rs — Adds device_id: "local" to the directory recovery fixture bucket.
• aw-sync/src/legacy_clean.rs — Adds device_id: "local" to the legacy cleanup test bucket.
• aw-sync/src/status.rs — Adds device_id: "local" to status test buckets.
• aw-sync/src/v2/segment.rs — Adds device_id: "local" to segment test buckets.
• aw-sync/tests/sync_roundtrip.rs — Adds device_id: "local" to the sync roundtrip bucket helper.
• aw-transform/src/find_bucket.rs — Adds device_id: "local" to the bucket lookup test constructor.

Previous review passes

commit	score	findings	engine	when
51c818c85f55	3/5	2	llm	2026-10-10 15:22 UTC
2bdf6a297bff	5/5	0	llm	2026-10-10 17:37 UTC
2bdf6a297bff	5/5	0	llm	2026-10-10 18:23 UTC
ebd4ad0e3344	5/5	0	llm	2026-10-10 19:07 UTC
d74c27eb425b	4/5	1	llm	2026-10-17 20:03 UTC
d74c27eb425b	5/5	0	llm	2026-10-17 21:03 UTC
18e5f1c1ee75	2/5	3	subscription	2026-10-27 00:22 UTC
1e850133eae8	3/5	2	subscription	2026-10-27 01:41 UTC
3cf44de58913	3/5	1	subscription	2026-10-27 07:41 UTC
df7278ac4f25	5/5	0	subscription	2026-10-27 08:42 UTC

_{Reviewed 8eda29ec7c33 · panel: codex:gpt-6.1-sol · subscription engine · 86s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

Outage coverage: 1-pass, not merge evidence. Subscription review supplied findings while full consensus capacity was unavailable. The consensus merge gate remains unsatisfied.

[TimeToBuildBob]

Fixed in ca57e35: removed the device_id guard in create_bucket — as the AI review (3/3 consensus) correctly pointed out, it rejected legitimate same-name/different-device inserts, making UNIQUE(device_id, name) unusable for its stated purpose. The UNIQUE constraint is now the sole duplicate guard (same device_id + name still returns BucketAlreadyExists via the constraint-violation path). Test flipped from asserting rejection to asserting coexistence plus the same-device duplicate rejection; cargo test -p aw-datastore passes (34/34), clippy and fmt clean.

[TimeToBuildBob]

Pushed 7c9ad1a to fix the real history-hiding bug. Duplicate names now return BucketAlreadyExists before insertion, and loading an already-ambiguous v7 database fails explicitly without deleting either row. Both new regressions failed on the previous head, then passed with the fix; 85 tests passed, 1 ignored, workspace clippy and commit hooks passed.

This corrects my earlier ca57e35 update: permitting two SQL rows did not make them safely addressable through the name-keyed cache. The PR body now scopes this as schema groundwork, not completed same-name coexistence. Keep the guard until device-aware cache keys and addressing land together.

The in-band review resolved the original P1 on the new head. Its new migration claim assumes duplicate names in a v6 schema that enforces UNIQUE(name); rejected with schema evidence on that thread. The test-coverage nit is covered by the separate ambiguity regression and was also replied to and resolved. No open findings from our reviewer remain; its historical raw score is retained. No Greptile re-trigger.

New-head CI: 7 checks in progress, not yet verified green. No merge performed; the next monitoring cycle checks those results.

[TimeToBuildBob]

Merge-order hazard across three of my datastore PRs. Each one is green on its own, but no pairing of them is safe:

• feat(categories): local manual event overrides outside watcher data #782 and feat(datastore): add device_id column, v6→v7 migration (UNIQUE(device_id, name)) #786 both claim schema v7, with different migrations. Git conflicts on that pair. The obvious resolution keeps both if version < 7 blocks, so a DB migrated by either one never gets the other's change.
• fix(datastore): infer schema version when a restored DB has user_version 0 #776 + feat(datastore): add device_id column, v6→v7 migration (UNIQUE(device_id, name)) #786 merges cleanly in git but is broken. v7 drops data_deprecated, so _infer_db_version returns 2 for a v7 schema. A restored v7 DB (user_version 0) would replay the v2→v7 migrations on top of a v7 table. I reproduced it on a local merge of feat(datastore): add device_id column, v6→v7 migration (UNIQUE(device_id, name)) #786 and fix(datastore): infer schema version when a restored DB has user_version 0 #776: fix(datastore): infer schema version when a restored DB has user_version 0 #776's own infer_db_version_identifies_current_schema fails with left: 2, right: 7. fix(datastore): infer schema version when a restored DB has user_version 0 #776 + feat(categories): local manual event overrides outside watcher data #782 is the same case: inference stops at 6.

Proposed order, with me rebasing each PR after the previous one lands: #776 → #786 (extend inference: device_id column ⇒ 7) → #782 (renumber to v8, inference: sidecar table ⇒ 8). If you'd rather merge in a different order, the PR merged second takes v8, and #776's inference gets extended either way.

[TimeToBuildBob]

Merged master now that #776 landed, and fixed the hazard from my earlier comment in 51c818c: _infer_db_version checks device_id first and returns 7. Without that, a dump-restored v7 DB inferred as v2 because v7 drops data_deprecated. The early-schema dump-restore test now asserts v7 dropped data_deprecated. cargo test -p aw-datastore: 44 + 43 passed. #782 still needs to renumber to v8 once this merges.

[TimeToBuildBob]

Triggered one fresh CI attempt with tree-identical commit 2bdf6a297bff after the failed-job rerun was denied (no repository admin rights). The failing Windows step was cargo test -p aw-client-rust --features cli: test_wait_for_start_retries_without_blocking_executor timed out after 10s. That same test passed in the preceding workspace step of the same job, and its implementation is unchanged from successful master Build run 38055186799. Windows build, datastore/model tests, and sync-v2 tests passed. This supports an intermittent client test failure, not a migration regression; no client behavior or deadlines were changed. Push completed through the PM wrapper, honoring its already-spent review cap. GitHub confirms the new head; checks have not yet appeared, so CI is not claimed green. Next monitoring cycle checks this single retry; a repeated failure needs a focused client-test investigation rather than another automatic retry.

[TimeToBuildBob]

Rebased onto master (ebd4ad0) to clear the conflict introduced when #806/#807 landed.

The single conflict was in get_stored_buckets: master now reads a bucket-list signature before the query so a read-only reader can detect cache staleness (#807), while this PR builds the SELECT with a version-gated {device_id_expr}. Resolution keeps both — signature read first, then the v6-safe 'local' fallback for read-only peer DBs opened at v4–v6.

Verified on the resolved head:

• cargo test -p aw-datastore -p aw-models — 46 unit + 44 integration + 15 model tests passed (1 ignored)
• cargo clippy -p aw-datastore -- -D warnings — clean
• cargo check --workspace --all-targets — clean (only pre-existing warnings)

CI is re-running on ebd4ad0; mergeable again. Still waiting only on a maintainer click.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Merge recommendation

Adjudicating the new P1s on head 18e5f1c (AI review of that SHA, 2026-10-27 00:22 UTC). Did not trigger Greptile (attempt cap). Pushed 1e85013 with the two blocking fixes.

Fixed (this session, head 1e85013):

• 0b2bef1a2cbe P1 _migrate_v3_to_v4 version regress — exclusive lock + version recheck, same pattern as v5/v6/v7. Regression: pre_v7_migrations_recheck_version_after_taking_the_lock.
• 8289d8f1fe81 P1 migrate_test_bucket_names unguarded rename — SQL destination lookup, same-device merge only, NOT EXISTS name guard. Regression: test_stale_connection_migrate_test_bucket_names_rejects_ambiguous_name.

Remaining (non-blocking):

• a5a3f870bc07 P1 recover_database matches by name (recovery.rs:154, outside the diff, no inline thread). Dismissed, accepted tradeoff. Windows recovery unions Local/Roaming of the same install. Both source snapshot and target run DatastoreInstance::new(..., true) before the name match, so v7 stamps every existing bucket device_id='local'. Cross-device recovery is not this path; device-aware addressing is the stacked follow-up (feat(datastore): preserve history through legacy bucket aliases #836). Matching on (device_id, name) belongs there, not as a silent production bug of this schema PR.

Verified against: classifications checked on 18e5f1c1ee75; fixes landed in 1e850133eae8.

CI: 7/7 green on 18e5f1c. New head 1e85013 just pushed; hosted checks not claimed green yet.

Domain risk: v6→v7 table recreation; concurrent startup from v3; Android aw-watcher-android-test* rename at startup. After merge, #782 must renumber its schema to v8.

Convergence: Greptile detector verdict: converged, unresolved P1/P2/P3 = 0. round_convergence.stable_rounds = 0 (only the 2026-10-06 round is ingested; quote with that caveat). Our two inline threads stay open for reviewer verification on 1e85013.

Not merge-ready until CI is green on 1e85013 and the reviewer clears those two threads. Maintainer owns the merge click (pull-only). No Greptile re-trigger.

[TimeToBuildBob]

Merge recommendation — fixes pushed; new-head verification pending

Pushed 3cf44de. Correcting my earlier recovery dismissal: migration stamps pre-v7 buckets as local, but preserves explicit ownership in an already-v7 database. Same-install Local/Roaming recovery therefore cannot safely assume matching names imply matching devices.

• Fixed: P1 37eee57831f6, cross-device recovery union. Recovery now compares device IDs under the existing target write transaction and defers the whole source on a mismatch, rolling back any union work and leaving no completion marker. The regression failed on 1e85013 (one foreign event inserted), then passed. It also verifies a later retry succeeds when both buckets have the same non-local device ID.
• Fixed: P2 9e21f5546518, stale reader ownership. Cache reload refreshes schema version, including inference for dump-restored files, rather than permanently using the version seen at open. The two-connection regression failed with local != peer-abc before the fix and passes now. Replied on its inline thread and resolved it with a verified-fix disposition; no unresolved threads from our reviewer remain. The P1 was summary-only, outside the diff.
• Remaining: no newly dismissed findings. The previously accepted non-blocking rename_bucket error-label race remains unchanged: the atomic UPDATE protects state; only a concurrent failure's error label can vary. This is not a new dismissal or a claim of whole-head clearance.
• Verified against: findings reproduced on reviewed head 1e850133eae8; fixes and local verification on 3cf44de58913. 137 tests passed, 1 ignored (cargo test -p aw-datastore -p aw-models); strict datastore clippy, formatting, diff whitespace and commit hooks passed.
• CI: all seven checks passed on the old head. No check runs had appeared for 3cf44de at the post-push check; new-head CI is not verified green. The push wrapper honored its existing review cap, so no fresh in-band AI review is claimed. Await automatic AI-review-sweep coverage of this head before final adjudication.
• Domain risk: SQLite table recreation, concurrent migration, and Windows preserved-history recovery. Native Windows recovery remains a useful maintainer smoke test; this session ran real SQLite fixtures on Linux, not a native Windows upgrade. feat(categories): local manual event overrides outside watcher data #782 still needs its schema renumbering after this PR merges.
• Convergence: detector verdict: converged, Greptile unresolved P1/P2/P3 = 0; round_convergence.stable_rounds = 0 (only the 2026-10-06 round is ingested, so this is not evidence of two stable review rounds). Greptile's footer remains at ebd4ad0; the current-head AI review supplied the actionable findings. No Greptile re-trigger and no merge.

Hold for CI and automatic review of 3cf44de; then maintainer judgment.

[TimeToBuildBob]

Merge recommendation — hold for new-head verification

Pushed df7278a, fixing the P1 found by the latest AI review of 3cf44de58913.

• Fixed this session: 9ee6839ce33f, stale inferred-version persistence. The version write now locks exclusively and re-reads the version/schema before persisting, so an opener cannot overwrite a completed v7 migration with its stale v6 inference. The two-connection regression failed with peer-abc becoming local before the fix and passes afterward. Replied on the inline finding and resolved it with a verified-fix disposition; no open findings from our reviewer remain.
• Remaining: no new dismissals. The previously accepted non-blocking rename error-label race remains: the atomic UPDATE prevents duplicate names; only the follow-up error classification can race. Its existing inline dismissal is unchanged.
• Verified against: P1 reproduced on reviewed head 3cf44de58913; fix and local verification on df7278ac4f25. 138 tests passed, 1 ignored across datastore/model suites. Strict datastore clippy, formatting, whitespace checks and commit hooks (including workspace clippy) passed.
• CI: on df7278ac4f25, format passed; the other six checks were running at the snapshot. New-head CI is not yet green. The push wrapper performed no fresh review, so this is not whole-head review clearance. Await automatic AI-review-sweep coverage before final adjudication.
• Domain risk: concurrent SQLite startup and the v6→v7 table rebuild; native Windows preserved-history recovery remains a maintainer smoke-test boundary. feat(categories): local manual event overrides outside watcher data #782 still needs schema renumbering after this PR merges.
• Convergence: detector verdict converged, Greptile unresolved P1/P2/P3 = 0; round_convergence.stable_rounds = 0 (only the 2026-10-06 round is ingested). Greptile's footer still names ebd4ad0; the newer AI pass supplied this actionable P1. No Greptile re-trigger or merge.

Next: automatic CI/review of df7278ac4f25, then maintainer judgment.