feat(server): bulk event delete endpoint

[TimeToBuildBob]

Adds POST /api/0/buckets/<bucket_id>/events/delete with body {"ids": [1, 2, 3]}, plus AwClient::delete_events. Refs #60 and ActivityWatch/aw-server#55.

• Returns the number of events that existed and were deleted. Unknown and duplicate ids are ignored; an empty list returns 0; an unknown bucket is a 404.
• Reuses Datastore::delete_events_by_id (already list-based; the datastore worker runs it as one operation) and invalidates the query cache for the removed events' ranges, like the single-delete route.
• Kept the existing DELETE .../events/<id> route. The issue suggests replacing it, but removing it breaks clients, so I left that call to maintainers.
• Not included: delete-by-period, VACUUM / secure_delete (separate concerns from the issue discussion), and the Python aw-server parity port (follow-up).

Verified: new test_bulk_delete_events (mixed known/unknown/duplicate ids, empty list, unknown bucket) passes; cargo fmt clean. The local clippy hook fails only on the pre-existing aw-query/src/lib.rs duplicate-attribute error (fixed by #771); I committed with SKIP=cargo-clippy for that hook alone.

[greptile-apps]

Confidence Score: 5/5

[High risk] Adds bulk event deletion endpoint to the server API.

The PR appears safe to merge based on the reviewed changes.

Findings

1.  Unknown bucket returns success ▶
2.  Datastore failures appear successful ▶
3.  Large deletes block event writes ▶
4.  Partial deletes leave stale cache ▶
5.  Concurrent deletes overcount events ▶
6.  Changed ranges leave stale cache ▶

Summary

Adds a bulk event-delete endpoint and Rust client method. The latest change moves event lookups inside the write lock so the returned count and cache invalidation reflect the events actually deleted. The existing single-delete route remains available.

_{Reviews (3) · Last reviewed commit: "fix(server): hold write lock across bulk..."}

[codecov]

Codecov Report

❌ Patch coverage is 90.32258% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.79%. Comparing base (656f3c9) to head (030493b).
⚠️ Report is 183 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-client-rust/src/lib.rs	0.00%	5 Missing ⚠️
aw-server/src/endpoints/bucket.rs	85.18%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #781       +/-   ##
===========================================
+ Coverage   70.81%   83.79%   +12.98%     
===========================================
  Files          51       82       +31     
  Lines        2916    10757     +7841     
===========================================
+ Hits         2065     9014     +6949     
- Misses        851     1743      +892     

☔ 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]

Addressed the four P1 findings from Greptile in 559558b:

1. Unknown bucket returns success — added an upfront get_bucket check so an unknown bucket returns 404 even when ids is empty.
2. Datastore failures appear successful — replaced Err(_) => {} with an explicit Err(DatastoreError::NoSuchEvent(_, _)) => {} arm; all other errors now propagate.
3. Large deletes block event writes — moved the write-lock acquisition to after the per-ID lookup loop; reads no longer hold the write lock.
4. Partial deletes leave stale cache — cache ranges are now invalidated on both success and failure paths.

All 23 existing server tests still pass.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

🤖 AI code review

Adds a bulk-delete endpoint POST /api/0/buckets/<bucket_id>/events/delete taking a JSON body {"ids": [...]}, returning u64 count of deleted events, with an upper bound on the number of ids, deduplication, query-cache invalidation, and a corresponding async client method AwClient::delete_events. Routes are registered in endpoints/mod.rs and an integration test covers success, empty, unknown bucket, and oversized cases.

Safe to merge — no P0/P1 findings

Confidence 5/5

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

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

• aw-client-rust/src/lib.rs — Adds async method delete_events that POSTs ids to the new endpoint and parses a u64 response.
• aw-server/src/endpoints/bucket.rs — Adds BulkDeleteRequest struct and bucket_events_delete_many handler with MAX_BULK_DELETE_IDS cap, dedup, and cache invalidation.
• aw-server/src/endpoints/mod.rs — Registers the new bucket_events_delete_many route in the /api/0/buckets mount.
• aw-server/tests/api.rs — Adds test_bulk_delete_events covering mixed known/unknown/duplicate ids, empty list, unknown bucket, and oversized request.

Previous review passes

commit	score	findings	engine	when
559558be3125	3/5	1	llm	2026-10-02 09:59 UTC
41c8d0822b70	4/5	1	llm	2026-10-03 11:43 UTC
49174d01f66a	4/5	2	llm	2026-10-03 13:42 UTC
49174d01f66a	2/5	3	llm	2026-10-03 13:43 UTC
49174d01f66a	3/5	1	llm	2026-10-03 13:47 UTC
49174d01f66a	3/5	2	llm	2026-10-03 13:48 UTC
030493b3fbd6	5/5	0	llm	2026-10-07 13:34 UTC

_{Reviewed 030493b3fbd6 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 550s · about this reviewer}

_{Maintainer commands}

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

[TimeToBuildBob]

⚠️ Likely duplicate: #778

This PR appears to substantially overlap with ActivityWatch/aw-server-rust#778:

fix(datastore): detect a corrupt database at startup, checkpoint WAL on close

Reason: Changed files and diffs suggest overlapping intent. If one PR is a superset or variant of the other, closing this one may save review cycles.

Not a blocker — review proceeds normally. If this is a false positive, dismiss this comment and continue.

[TimeToBuildBob]

Addressed the Greptile re-review (second pass on 559558b):

Fixed in 559558b (confirmed):

• Unknown bucket returns success → get_bucket() called before empty-ids early return
• Datastore failures appear successful → all errors except NoSuchEvent now propagate
• Partial deletes leave stale cache → cache invalidated in both success and error paths

Partially addressed:

• Large deletes block writes → reads moved outside the write lock; per-ID batching deferred (low-priority path)

Known limitations accepted for maintainer review:

• Concurrent deletes overcount → count is based on pre-lock collection; full fix requires delete_events_by_id to return actual rows deleted (datastore API change)
• Changed ranges leave stale cache → TOCTOU between range collection and lock; full fix requires reading ranges inside the lock (increases contention)

The Greptile score remains 3/5 after the second round. The two remaining findings are genuine but would require deeper datastore API changes or locking model rework. Leaving for maintainer review.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

CI-green and mergeable (Greptile 5/5) — 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.

[TimeToBuildBob]

Third round on this PR — fixed the root cause both reviewers had flagged, rather than deferring it.

Fixed in 41c8d08: the write lock is now acquired before the get_event collection loop and held through delete_events_by_id + cache invalidation — the same read-modify-invalidate shape as bucket_events_create and bucket_events_delete_by_id. This closes the TOCTOU that produced both remaining P1s:

• Concurrent deletes overcount: the count is now the number of ids that existed at delete time; a second concurrent request sees them already gone and returns 0.
• Changed ranges leave stale cache: ranges are collected under the same guard that deletes, so the invalidated range is exactly the removed event's range.

This does move the per-ID reads back inside the lock (Greptile's earlier "large deletes block event writes" note). Correctness wins here — range collection is part of the read-modify-invalidate sequence and cannot be made cache-correct outside it. Bulk delete is a client-driven cleanup path, not a hot path, and the datastore worker already serialises its operations; batching the O(n) lookups into a single datastore request is a reasonable follow-up to shorten the lock-held duration.

Verified: cargo fmt --check -p aw-server clean; cargo test -p aw-server → 23 passed (incl. test_bulk_delete_events). All review threads from this and the prior rounds are now resolved.

Cross-repo, so no self-merge — leaving this for maintainer review.

[TimeToBuildBob]

Convergence — ready for maintainer review

Reviewed at head 49174d01. Summary of where this landed after the last three rounds.

Fixed in the branch (real findings):

• 5b98ea0 — the O(n²) duplicate-id scan is now a HashSet dedup (O(n)).
• 49174d0 — MAX_BULK_DELETE_IDS cap; oversized requests get 413 before the write lock is taken, with a regression test.
• 41c8d082 — the read-collect-delete-invalidate sequence is back under the write lock. This closed the TOCTOU/stale-cache regression that appeared when the reads were briefly moved outside the lock.

Dispositioned with reasons (threads replied + resolved, not left open):

• Lock held across the per-id lookups — accepted trade-off. The range read is part of the read-modify-invalidate transaction, so it must be serialized with the delete; an earlier revision moved it outside the lock and both reviewers flagged the resulting stale cache as the worse bug. The round-trips are in-process crossbeam sends, the cap bounds the work, and bulk delete is a client-driven cleanup path. Eliminating them needs a batched-by-id read in aw-datastore — a datastore API addition, out of scope here, tracked as the follow-up.
• Returned count under concurrency — not applicable as written: every event-deleting handler (bucket_events_create, bucket_events_heartbeat, bucket_events_delete_by_id, this one) takes the same state.write_lock, so the named interleaver cannot run while this handler holds it.
• Test does not pin the surviving ids — reasonable test-strengthening, informational; recorded as a follow-up, not a defect in the handler.

Status: CI green (ubuntu / macOS / windows / clippy / format / Android / coverage); Greptile 5/5 at 41c8d082 (the two later commits implement its own cap/contention suggestions); mergeable/clean. The reviews have converged — the only remaining item is an accepted trade-off pending a datastore follow-up.

Bob has pull-only access to this repo, so the merge is a maintainer click.

[TimeToBuildBob]

Rebased onto master (fcc1ede) to clear the conflict.

Dropped fix(aw-query): remove duplicated clippy allows: master already carries the canonical fix for that (#771 moved clippy::block_scrutinee into parser.rs), so the commit only conflicted. The remaining 5 commits are the bulk-delete endpoint itself, unchanged.

Local check on the new head 030493b: cargo clippy --workspace -- -D warnings clean, cargo test -p aw-server passes (incl. test_bulk_delete_events).