fix(import): return descriptive error messages and roll back failed imports

[TimeToBuildBob]

Carries #165 by @behdadmansouri. Thanks for this! It resolves the # TODO in import_bucket and replaces the import endpoint's HTML 500s with JSON messages. This is the Python-server half of ActivityWatch/activitywatch#394; the webui half (ActivityWatch/aw-webui#1019) already renders message.

#165 conflicted with the query-cache change (#174), and I don't have push access to the fork. So this PR is #165 rebased, with both original commits kept under @behdadmansouri's authorship, plus one follow-up commit:

• Rollback gap: if a bucket was created but its events then failed, that bucket wasn't in imported, so it was left behind. Bucket IDs are now all checked up front. On failure, every bucket this import created is deleted, via ServerAPI.delete_bucket so the query cache is invalidated. import_all now runs under the write lock.
• Error classes: an existing bucket raises ValueError. Only KeyError/TypeError/ValueError become a 400. Anything else stays a 500 and is not reported as a client error. Routine user mistakes are logged as a one-line warning instead of a traceback (both of Greptile's P2s on fix(import): return descriptive error messages and success response #165).
• Batch scope: a bucket ID repeated within one export, or a non-object buckets, is rejected with a 400. Multipart uploads are imported as one batch, so a failing file rolls back the files before it.
• Tests: tests/test_import.py (9 cases, incl. repeated IDs, non-object buckets, multipart rollback), added to make test.

Live check (aw-server --testing, peewee storage, clean data dir)

Case	master	this PR
valid import	null 200	{"message": "Import successful"} 200
same import again	HTML 500 IntegrityError: UNIQUE constraint failed	400 Import failed: Bucket 'carry-a' already exists. Delete it first or rename the bucket before importing.
missing buckets key	HTML 500 KeyError	400 Import failed: missing field 'buckets'
new bucket with a bad event	HTML 500, bucket left behind	400 Import failed: Event.__init__() got an unexpected keyword argument 'not', rolled back
malformed JSON	400 (unchanged)	400 (unchanged)

pytest tests/test_server.py tests/test_profile.py tests/test_profile_config.py tests/test_query_cache.py tests/test_import.py → 75 passed. ruff (pinned 0.15.22) and black are clean. mypy reports the same 9 errors as on master.

Differences from aw-server-rust

• Same error shape: both return {"message": ...} with a 4xx/5xx status.
• Different duplicate-bucket semantics: aw-server-rust merges events into an existing bucket and skips duplicates. This PR rejects the import with a 400, which is still a strict improvement over today's 500. Matching Rust's merge would be a bigger change; I'd do it as a separate follow-up if that's the behavior we want.
• Different success body: Rust returns an empty 200, this returns {"message": "Import successful"}. aw-webui handles both.

Separate problem found while testing (not caused by this PR)

aw-core 0.5.18 allows peewee>=3,<5. Under peewee 4.5.2, the peewee storage is broken on master. POST /api/0/buckets/<id> returns 500, and the stored created then makes every GET /api/0/buckets/ fail (iso8601.ParseError: expected string or bytes-like object, got 'datetime.datetime' in BucketModel.json). This is ActivityWatch/aw-core#157, already fixed by ActivityWatch/aw-core#158, but that fix landed after v0.5.18 and isn't released yet. The lockfile pins peewee 3.17.0, so locked installs are fine. An unpinned pip install aw-server would pull peewee 4 until aw-core 0.5.19 ships. The live check above used peewee 3.17.0.

Supersedes #165.

[greptile-apps]

Confidence Score: 5/5

[High risk] Changes import logic and error handling for buckets.

The PR appears safe to merge based on the current review findings.

Findings

1.  Rollback details never reach clients ▶

Summary

The PR makes import failures return descriptive JSON responses, prechecks bucket IDs, and rolls back buckets created by a failed import. The latest change gives incomplete rollbacks a distinct exception so the 500 response can name affected buckets. Import regression tests are added to make test.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Import request] --> B[Collect buckets and check IDs]
  B --> C[Import batch]
  C -->|Success| D[200 JSON success]
  C -->|Import fails| E[Attempt rollback of created buckets]
  E -->|Rollback succeeds| F[Return import error]
  E -->|Rollback incomplete| G[500 JSON naming buckets reported as remaining]

Loading

_{Reviews (5) · Last reviewed commit: "fix(import): return incomplete-rollback ..."}

[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.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

🤖 AI code review

This PR reworks the import endpoint in aw_server. It adds an up-front duplicate-bucket and duplicate-ID check in ServerAPI.import_all, wraps import_all in the write lock, and on failure rolls back every bucket created by the import via delete_bucket, raising ImportRollbackError when rollback is incomplete. The REST layer now catches ImportRollbackError as a 500 and KeyError/TypeError/ValueError as a 400 with a JSON message, and multipart uploads are merged into one batch. It adds tests/test_import.py and wires it into make test.

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

• Makefile — Adds tests/test_import.py to the pytest invocation in the test target.
• aw_server/api.py — Adds ImportRollbackError, duplicate checks in import_bucket and import_all, write-lock on import_all, and rollback of created buckets on failure.
• aw_server/rest.py — Rewrites ImportAllResource.post to merge multipart files into one batch and map exceptions to 400/500 JSON responses.
• tests/test_import.py — Adds nine tests covering success, duplicate buckets, rollback, malformed input, multipart rollback, and rollback-failure reporting.

Previous review passes

commit	score	findings	engine	when
e1cd9a0bd856	4/5	1	llm	2026-10-02 01:58 UTC

_{Reviewed 934f7b8e46fd · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 17s · about this reviewer}

_{Maintainer commands}

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

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

@greptileai review

[greptile-apps]

Rollback details never reach clients

When deleting a bucket fails during rollback, this RuntimeError names the buckets left stored. The import endpoint does not catch it, so the client receives a generic 500 response instead of those details. The client cannot tell which buckets must be removed before retrying.

Knowledge Base Used: REST request handling

[TimeToBuildBob]

Fixed in 934f7b8 — import_all now raises a dedicated ImportRollbackError, and the REST handler catches it and returns its message as a JSON 500 (instead of a generic HTML 500) naming the buckets that remain stored. The client now learns which buckets to delete before retrying.

Both rollback tests now assert the remaining bucket IDs appear in the response body, and they fail without this change (verified by reverting rest.py).

[TimeToBuildBob]

@greptileai review