fix(action_items): content-hash idempotency on POST /v1/action-items

[builtbycnob]

Summary

database.action_items.create_action_item allocated a fresh Firestore document id on every call (action_items_ref.add(...)), so a flaky-network retry from any HTTP client produced a duplicate row. This PR adds opt-in idempotency:

• db layer — create_action_item(uid, data, idempotency_key=...) looks up an existing document with that key before writing; returns the existing id on hit, persists the key on miss.
• router layer — POST /v1/action-items now computes sha256(f"{uid}:{normalized_description}") and passes it through. A retried POST of the same task collapses to the original document.

Why

Production observation on an affected account: 8 distinct task descriptions × 5–6 backend duplicates each (~50 phantom rows). The dominant duplicating path is the staged-task promotion gap (#7092), but the desktop client also has a retryUnsyncedItems loop that re-pushes manual tasks whose initial sync didn't complete. Without a server-side idempotency key, every retry produced a fresh Firestore document — and once the row had a backendId, the desktop's syncTaskActionItems couldn't dedup it any longer.

This PR closes the retry-amplification gap so even an aggressive client cannot mint duplicates by hammering the endpoint.

Diff

database/action_items.py:

def create_action_item(uid: str, action_item_data: dict,
                        idempotency_key: Optional[str] = None) -> str:
    ...
    if idempotency_key:
        existing_query = action_items_ref.where(
            filter=FieldFilter('idempotency_key', '==', idempotency_key)
        ).limit(1)
        for doc in existing_query.stream():
            return doc.id

    ...
    if idempotency_key:
        action_item_data['idempotency_key'] = idempotency_key

    doc_ref = action_items_ref.add(action_item_data)[1]
    return doc_ref.id

routers/action_items.py:

def _content_idempotency_key(uid: str, description: str) -> str:
    normalized = (description or '').strip().lower()
    return hashlib.sha256(f"{uid}:{normalized}".encode('utf-8')).hexdigest()


@router.post("/v1/action-items", ...)
def create_action_item(request: ..., uid: str = ...):
    ...
    idempotency_key = _content_idempotency_key(uid, request.description)
    action_item_id = action_items_db.create_action_item(
        uid, action_item_data, idempotency_key=idempotency_key
    )

Backwards compatibility

• The idempotency_key parameter defaults to None. Callers that do not pass it (developer API, agentic tools, sync paths, batch creation) keep the old behaviour exactly — no key is written, no lookup happens.
• The new field on Firestore documents is additive. Reads that don't request it remain unaffected.

Tests

7 new unit tests in backend/tests/unit/test_action_item_idempotency.py, wired into backend/test.sh:

• db-layer: no-key path is backwards compatible (no idempotency_key field written, fresh doc id), idempotency hit returns the existing id without writing, idempotency miss writes the key on the new doc.
• router-layer: _content_idempotency_key is stable for same input, normalizes case + whitespace, separates by uid, separates by description.

$ pytest backend/tests/unit/test_action_item_idempotency.py -v
============================== 7 passed in 0.14s ==============================

What this fixes (and what it does not)

• ✅ Retried POST /v1/action-items collapses to the original document — no more duplicate rows from desktop client retries or webhook redelivery.
• ✅ Companion to fix(staged_tasks): dedup promote_staged_task against active action_items #7092 (promotion-path dedup) and fix(desktop/sync): adopt orphan action_items by description only #7091 (client-side orphan adoption). Together they close all three gaps that produced the 50-phantom-row observation.
• ❌ Does not retroactively dedup existing duplicates. A separate one-shot migration is still recommended.
• ❌ Does not add idempotency to other create_action_item callers (routers/developer.py, tools/action_item_tools.py, routers/staged_tasks.py:promote_staged_task). Those paths can be migrated in follow-ups by passing an appropriate key — the plumbing is now in place.

Risk

Low. The dedup query is a single Firestore where(... == key).limit(1) — bounded cost, no fan-out. Behaviour without a key is unchanged. The new idempotency_key field is opt-in, so existing tests/migrations are unaffected.

If two concurrent POSTs from the same uid+description race past the lookup before either commits, both will write — but they'll both write with the same key, so the next read can be reconciled and a follow-up migration can collapse them. This matches the behaviour of optimistic-style idempotency in production HTTP APIs.

Test plan

• pytest backend/tests/unit/test_action_item_idempotency.py -v — 7 passed
• black --line-length 120 --skip-string-normalization clean
• Manual on staging: POST /v1/action-items twice with same description; second call returns the original id with no new Firestore document.
• Regression: POST with a unique description; confirm normal creation, idempotency_key field present on the document.
• Regression: existing developer-API path (routers/developer.py:create_action_item) without key continues to allocate fresh ids.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR adds content-hash idempotency to POST /v1/action-items so that flaky-network retries of the same task description are collapsed to the original Firestore document instead of minting a duplicate.

• P1 — permanent idempotency block: The Firestore lookup in create_action_item matches documents in any state, including completed=True. Once a user creates and completes "Buy milk", every future POST /v1/action-items for "Buy milk" silently returns the old completed document ID — no new task is ever written. The response carries completed: true, contradicting the caller's intent. Scope the query to non-completed (and non-deleted) documents, or add a short recency window on created_at.
• P2 — separator injection in key derivation: _content_idempotency_key concatenates uid and description with a bare : delimiter. Two (uid, description) pairs that differ only in where the : falls produce the same hash. Firebase UIDs are currently alphanumeric so this is low-risk today, but a length-prefix encoding is more robust.

Confidence Score: 3/5

Not safe to merge as-is: permanent content-hash idempotency silently prevents re-creation of tasks with the same description after completion.

One P1 finding that causes a present behavioral regression (completing then re-creating a same-named task silently returns the stale completed ID forever), plus one P2 on the key encoding. The P1 is on the core write path and directly contradicts the feature's stated goal of being transparent to users.

backend/database/action_items.py — the idempotency query at lines 86–95 needs a state filter or TTL before this is safe to ship.

Important Files Changed

Filename	Overview
backend/database/action_items.py	Adds optional idempotency_key parameter to create_action_item; the lookup matches any document state (including completed=True), which permanently blocks re-creation of tasks with the same description.
backend/routers/action_items.py	Adds _content_idempotency_key helper and threads the key into the db call; the naive UID:description concatenation has a theoretical separator-injection weakness.
backend/tests/unit/test_action_item_idempotency.py	Seven unit tests covering db-layer idempotency hit/miss/no-key and router-layer key stability; all pass. No test covers the case where an existing document is completed=True.
backend/test.sh	Adds the new test file to the test suite runner.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[POST /v1/action-items] --> B[Compute content idempotency key\nsha256 of user-id plus normalized description]
    B --> C[call create_action_item with key]
    C --> D{key provided?}
    D -- No --> G[prepare data and set timestamps]
    D -- Yes --> E[Firestore query\nwhere idempotency_key equals key\nlimit 1]
    E --> F{document found?}
    F -- Yes, any state including completed --> Z[return existing doc id WARNING]
    F -- No --> G
    G --> H[append idempotency_key field to payload]
    H --> I[action_items_ref.add]
    I --> K[return new doc id]
    Z --> L[get_action_item to fetch full doc]
    K --> L
    L --> M[ActionItemResponse - may show completed equals true]

Loading

_{Reviews (1): Last reviewed commit: "chore(test.sh): wire up test_action_item..." | Re-trigger Greptile}

[greptile-apps]

Permanent content-hash idempotency silently swallows legitimate re-creation

The lookup matches any document in any state. If a user completes "Buy milk" and then POSTs a new "Buy milk" task, existing_query.stream() returns the already-completed document and the function returns its ID — no new document is written. The caller receives an ActionItemResponse with completed: true, directly contradicting the completed: false payload they sent.

Because the key is a stable SHA-256 hash with no expiry and no state filter, this is a permanent block: any task description that was ever created (and later completed) can never be re-created as a fresh task for that user.

Fix: scope the lookup to documents that are neither completed nor soft-deleted, or add a recency window on created_at that is just wide enough to cover legitimate retries (e.g. a few minutes).

[greptile-apps]

Separator-injection collision in key derivation

The key is built by naively concatenating uid and description with a single : delimiter: f"{uid}:{normalized}". If a uid value ever contains : (e.g. a system-generated or federated identity like "org:user123"), the hash for (uid="org", description="user123:task") would equal the hash for (uid="org:user123", description="task"), silently mapping one user's task onto another user's idempotency slot.

Firebase Auth UIDs are currently alphanumeric, so this is low-risk today, but fragile as a long-term contract. Use a length-prefixed or otherwise unambiguous encoding:

normalized = (description or '').strip().lower()
payload = f"{len(uid)}:{uid}:{normalized}"
return hashlib.sha256(payload.encode('utf-8')).hexdigest()