Make mobile issue capture idempotent across retries (#178)

Closes #177
2026-08-07 07:55:30 +00:00 · 2026-08-07 07:55:30 +00:00 · 1947f878e9
commit 1947f878e9
parent 9293dde1af 6f54d5764b
5 changed files with 316 additions and 19 deletions
--- a/README.md
+++ b/README.md
@ -20,7 +20,10 @@ create issue comments, close assigned issues, inspect/comment on assigned pull
 requests, merge assigned pull requests, and submit pull-request reviews.
 Pull-request replies and mobile My Work issue and PR comments use Gitea's
 issue-comment API; mobile issue capture requires issue
-creation and assignment permission. Closing an assigned issue, native Comment,
+creation and assignment permission. Issue capture persists a per-draft idempotency key,
+so retrying after a timeout or reload replays a confirmed creation instead of posting a
+duplicate; callers integrating directly should preserve the `Idempotency-Key` header
+with the unchanged payload until a `201` response is confirmed. Closing an assigned issue, native Comment,
 Approve, and Request changes reviews, and assigned-PR merge require repository
 write permission. Native Comment, Approve, and Request changes reviews support
 head-scoped draft comments anchored to changed lines; the dashboard validates each
--- a/frontend/create-issue-sheet.js
+++ b/frontend/create-issue-sheet.js
@ -1,4 +1,14 @@
-function createIssueCapture({ fetchJson, storage }) {
+function newIssueOperationId() {
+  if (globalThis.crypto?.randomUUID) return globalThis.crypto.randomUUID();
+  if (globalThis.crypto?.getRandomValues) {
+    const bytes = new Uint8Array(16);
+    globalThis.crypto.getRandomValues(bytes);
+    return Array.from(bytes, byte => byte.toString(16).padStart(2, '0')).join('');
+  }
+  return String(Date.now()) + '-' + Math.random().toString(16).slice(2);
+}
+
+function createIssueCapture({ fetchJson, storage, createOperationId = newIssueOperationId }) {
  const storageKey = 'stackchain.issue-capture.v1';
  let pending = null;
  const safeLabelIds = value => Array.from(new Set(
@ -6,30 +16,44 @@ function createIssueCapture({ fetchJson, storage }) {
  )).slice(0, 20);
  const emptyDraft = () => ({ repository: '', title: '', body: '', labelIds: [] });

+  function loadStored() {
+    try {
+      const parsed = JSON.parse(storage.getItem(storageKey) || 'null');
+      if (!parsed || typeof parsed !== 'object') return {...emptyDraft(), operationId: ''};
+      return {
+        repository: String(parsed.repository || ''),
+        title: String(parsed.title || ''),
+        body: String(parsed.body || ''),
+        labelIds: safeLabelIds(parsed.labelIds),
+        operationId: String(parsed.operationId || '').slice(0, 128),
+      };
+    } catch (_error) {
+      return {...emptyDraft(), operationId: ''};
+    }
+  }
+
+  function writeStored(record) {
+    try { storage.setItem(storageKey, JSON.stringify(record)); }
+    catch (_error) { /* Keep the form as the in-memory fallback. */ }
+  }
+
  function saveDraft(draft) {
+    const previous = loadStored();
    const safe = {
      repository: String(draft?.repository || ''),
      title: String(draft?.title || ''),
      body: String(draft?.body || ''),
      labelIds: safeLabelIds(draft?.labelIds),
    };
-    try { storage.setItem(storageKey, JSON.stringify(safe)); }
-    catch (_error) { /* Keep the form as the in-memory fallback. */ }
+    const unchanged = ['repository', 'title', 'body'].every(key => previous[key] === safe[key]) &&
+      JSON.stringify(previous.labelIds) === JSON.stringify(safe.labelIds);
+    writeStored({...safe, operationId: unchanged ? previous.operationId : ''});
    return safe;
  }

  function loadDraft() {
-    try {
-      const parsed = JSON.parse(storage.getItem(storageKey) || 'null');
-      return parsed && typeof parsed === 'object' ? {
-        repository: String(parsed.repository || ''),
-        title: String(parsed.title || ''),
-        body: String(parsed.body || ''),
-        labelIds: safeLabelIds(parsed.labelIds),
-      } : emptyDraft();
-    } catch (_error) {
-      return emptyDraft();
-    }
+    const {operationId: _operationId, ...draft} = loadStored();
+    return draft;
  }

  function clearDraft() {
@ -52,10 +76,16 @@ function createIssueCapture({ fetchJson, storage }) {
  function submit(draft) {
    if (pending) return pending;
    const saved = saveDraft(draft);
+    const stored = loadStored();
+    const operationId = stored.operationId || String(createOperationId()).slice(0, 128);
+    writeStored({...saved, operationId});
    const repository = saved.repository.split('/').map(encodeURIComponent).join('/');
    pending = fetchJson('api/v1/repos/' + repository + '/issues', {
      method: 'POST',
-      headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
+      headers: {
+        Accept: 'application/json', 'Content-Type': 'application/json',
+        'Idempotency-Key': operationId,
+      },
      body: JSON.stringify({
        title: saved.title, body: saved.body, label_ids: saved.labelIds,
      }),
--- a/src/main.py
+++ b/src/main.py
@ -6,7 +6,7 @@ from contextlib import asynccontextmanager
 from pathlib import Path
 from typing import Any, Literal

-from fastapi import FastAPI, HTTPException, Path as PathParam, Query
+from fastapi import FastAPI, Header, HTTPException, Path as PathParam, Query
 from fastapi.responses import JSONResponse
 from fastapi.staticfiles import StaticFiles
 from pydantic import BaseModel, Field, PositiveInt, field_validator, model_validator
@ -55,6 +55,8 @@ EVENT_STREAM_TIMEOUT_SECONDS = 5.0
 READINESS_TIMEOUT_SECONDS = 5.0
 REVIEW_DETAIL_TIMEOUT_SECONDS = 5.0
 ISSUE_ACTION_TIMEOUT_SECONDS = 5.0
+ISSUE_CREATION_IDEMPOTENCY_TTL_SECONDS = 600.0
+ISSUE_CREATION_IDEMPOTENCY_MAX_ENTRIES = 256
 NOTIFICATION_MUTATION_TIMEOUT_SECONDS = 5.0
 NOTIFICATION_PAGE_TIMEOUT_SECONDS = 5.0
 WORK_PAGE_TIMEOUT_SECONDS = 5.0
@ -80,6 +82,9 @@ _live_section_retry_at: dict[str, float | None] = {
 }
 _live_snapshot_refreshing_sections: set[str] = set()
 _read_notification_ids: set[int] = set()
+_issue_creation_operations: dict[
+    str, tuple[tuple[Any, ...], asyncio.Task, float]
+] = {}


 class ContextPayloadError(ValueError):
@ -952,7 +957,12 @@ async def repository_labels(owner: str, repo: str):


@app.post("/api/v1/repos/{owner}/{repo}/issues", status_code=201)
-async def create_assigned_issue(creation: IssueCreation, owner: str, repo: str):
+async def create_assigned_issue(
+    creation: IssueCreation,
+    owner: str,
+    repo: str,
+    idempotency_key: str | None = Header(default=None, max_length=128),
+):
    repository = f"{owner}/{repo}"

    async def create_issue():
@ -980,8 +990,48 @@ async def create_assigned_issue(creation: IssueCreation, owner: str, repo: str):
            repository, creation.title, creation.body, login, creation.label_ids
        )

+    operation = create_issue()
+    if idempotency_key:
+        now = time.monotonic()
+        expired = [
+            key for key, (_, task, created_at) in _issue_creation_operations.items()
+            if task.done()
+            and now - created_at >= ISSUE_CREATION_IDEMPOTENCY_TTL_SECONDS
+        ]
+        for key in expired:
+            _issue_creation_operations.pop(key, None)
+        fingerprint = (
+            repository, creation.title, creation.body, tuple(creation.label_ids)
+        )
+        existing = _issue_creation_operations.get(idempotency_key)
+        if existing is not None:
+            operation.close()
+            if existing[0] != fingerprint:
+                raise HTTPException(status_code=409, detail="Idempotency key already used")
+            task = existing[1]
+        else:
+            while len(_issue_creation_operations) >= ISSUE_CREATION_IDEMPOTENCY_MAX_ENTRIES:
+                completed = [
+                    key for key, (_, task, _) in _issue_creation_operations.items()
+                    if task.done()
+                ]
+                if not completed:
+                    operation.close()
+                    raise HTTPException(
+                        status_code=503,
+                        detail="Issue creation is busy; please retry",
+                        headers={"Retry-After": "1"},
+                    )
+                oldest = min(
+                    completed, key=lambda key: _issue_creation_operations[key][2]
+                )
+                _issue_creation_operations.pop(oldest)
+            task = asyncio.create_task(operation)
+            _issue_creation_operations[idempotency_key] = (fingerprint, task, now)
+        operation = asyncio.shield(task)
+
    try:
-        result = await asyncio.wait_for(create_issue(), timeout=ISSUE_ACTION_TIMEOUT_SECONDS)
+        result = await asyncio.wait_for(operation, timeout=ISSUE_ACTION_TIMEOUT_SECONDS)
    except HTTPException:
        raise
    except Exception:
--- a/tests/test_issue_api.py
+++ b/tests/test_issue_api.py
@ -6,6 +6,13 @@ import pytest
 from src import gitea_proxy, main


+@pytest.fixture(autouse=True)
+def clear_issue_creation_operations():
+    main._issue_creation_operations.clear()
+    yield
+    main._issue_creation_operations.clear()
+
+
@pytest.mark.anyio
 async def test_create_issue_endpoint_derives_self_assignment_and_returns_confirmed_issue(monkeypatch):
    calls = []
@ -60,6 +67,166 @@ async def test_create_issue_endpoint_derives_self_assignment_and_returns_confirm
    assert calls == [("stackchain/api", "Capture mobile work", "Context", "timmy", [3])]


+@pytest.mark.anyio
+async def test_create_issue_replays_one_upstream_result_for_concurrent_idempotent_requests(monkeypatch):
+    calls = 0
+    started = asyncio.Event()
+    release = asyncio.Event()
+
+    async def user():
+        return {"login": "timmy"}
+
+    async def available_repos():
+        return [{"full_name": "stackchain/api"}]
+
+    async def create(repository, title, body, assignee, label_ids):
+        nonlocal calls
+        calls += 1
+        started.set()
+        await release.wait()
+        return {
+            "id": 81, "number": 17, "title": title, "state": "open",
+            "repository": repository, "labels": [], "assignees": [assignee],
+            "updated_at": "2026-08-07T03:00:00Z",
+            "url": "https://forge.example/stackchain/api/issues/17",
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "current_user", user)
+    monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
+    monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
+    transport = httpx.ASGITransport(app=main.app)
+    headers = {"Idempotency-Key": "capture-177-concurrent"}
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        first = asyncio.create_task(client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "Capture work"}, headers=headers
+        ))
+        await started.wait()
+        second = asyncio.create_task(client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "Capture work"}, headers=headers
+        ))
+        await asyncio.sleep(0)
+        release.set()
+        responses = await asyncio.gather(first, second)
+
+    assert [response.status_code for response in responses] == [201, 201]
+    assert [response.json()["number"] for response in responses] == [17, 17]
+    assert calls == 1
+
+
+@pytest.mark.anyio
+async def test_create_issue_rejects_changed_payload_for_an_existing_idempotency_key(monkeypatch):
+    calls = []
+
+    async def user():
+        return {"login": "timmy"}
+
+    async def available_repos():
+        return [{"full_name": "stackchain/api"}]
+
+    async def create(repository, title, body, assignee, label_ids):
+        calls.append(title)
+        return {
+            "id": 81, "number": 17, "title": title, "state": "open",
+            "repository": repository, "labels": [], "assignees": [assignee],
+            "updated_at": "2026-08-07T03:00:00Z",
+            "url": "https://forge.example/stackchain/api/issues/17",
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "current_user", user)
+    monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
+    monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
+    transport = httpx.ASGITransport(app=main.app)
+    headers = {"Idempotency-Key": "capture-177-payload-conflict"}
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        created = await client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "First title"}, headers=headers
+        )
+        conflict = await client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "Changed title"}, headers=headers
+        )
+
+    assert created.status_code == 201
+    assert conflict.status_code == 409
+    assert conflict.json()["detail"] == "Idempotency key already used"
+    assert calls == ["First title"]
+
+
+@pytest.mark.anyio
+async def test_create_issue_retry_recovers_result_after_the_first_request_times_out(monkeypatch):
+    calls = 0
+
+    async def user():
+        return {"login": "timmy"}
+
+    async def available_repos():
+        return [{"full_name": "stackchain/api"}]
+
+    async def create(repository, title, body, assignee, label_ids):
+        nonlocal calls
+        calls += 1
+        await asyncio.sleep(0.03)
+        return {
+            "id": 81, "number": 17, "title": title, "state": "open",
+            "repository": repository, "labels": [], "assignees": [assignee],
+            "updated_at": "2026-08-07T03:00:00Z",
+            "url": "https://forge.example/stackchain/api/issues/17",
+        }
+
+    monkeypatch.setattr(main, "ISSUE_ACTION_TIMEOUT_SECONDS", 0.01)
+    monkeypatch.setattr(main.gitea_proxy, "current_user", user)
+    monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
+    monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
+    transport = httpx.ASGITransport(app=main.app)
+    headers = {"Idempotency-Key": "capture-177-timeout-recovery"}
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        timed_out = await client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "Slow capture"}, headers=headers
+        )
+        await asyncio.sleep(0.03)
+        recovered = await client.post(
+            "/api/v1/repos/stackchain/api/issues", json={"title": "Slow capture"}, headers=headers
+        )
+
+    assert timed_out.status_code == 503
+    assert recovered.status_code == 201
+    assert recovered.json()["number"] == 17
+    assert calls == 1
+
+
+@pytest.mark.anyio
+async def test_create_issue_idempotency_registry_evicts_oldest_entry_at_size_limit(monkeypatch):
+    async def user():
+        return {"login": "timmy"}
+
+    async def available_repos():
+        return [{"full_name": "stackchain/api"}]
+
+    async def create(repository, title, body, assignee, label_ids):
+        return {
+            "id": 81, "number": 17, "title": title, "state": "open",
+            "repository": repository, "labels": [], "assignees": [assignee],
+            "updated_at": "2026-08-07T03:00:00Z",
+            "url": "https://forge.example/stackchain/api/issues/17",
+        }
+
+    monkeypatch.setattr(main, "ISSUE_CREATION_IDEMPOTENCY_MAX_ENTRIES", 2)
+    monkeypatch.setattr(main.gitea_proxy, "current_user", user)
+    monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
+    monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        for index in range(3):
+            response = await client.post(
+                "/api/v1/repos/stackchain/api/issues",
+                json={"title": f"Capture {index}"},
+                headers={"Idempotency-Key": f"capture-177-bounded-{index}"},
+            )
+            assert response.status_code == 201
+
+    assert len(main._issue_creation_operations) == 2
+    assert "capture-177-bounded-0" not in main._issue_creation_operations
+
+
@pytest.mark.anyio
 async def test_gitea_create_issue_posts_self_assignment_and_normalizes_confirmation():
    requests = []
--- a/tests/test_my_work.py
+++ b/tests/test_my_work.py
@ -395,6 +395,53 @@ Promise.all([first, duplicate]).then(results => process.stdout.write(JSON.string
    assert output["results"][1]["number"] == 17


+def test_issue_capture_reuses_its_persisted_idempotency_key_after_reload():
+    script = f"""
+const createIssueCapture = require({json.dumps(str(CREATE_ISSUE_SHEET))});
+const values = new Map();
+const storage = {{
+  getItem:key => values.get(key) || null,
+  setItem:(key,value) => values.set(key,value),
+  removeItem:key => values.delete(key),
+}};
+const calls = [];
+const draft = {{repository:'stackchain/api', title:'Capture work', body:'Context', labelIds:[3]}};
+const first = createIssueCapture({{
+  storage,
+  createOperationId: () => 'operation-177',
+  fetchJson: (_url, options) => {{ calls.push(options.headers['Idempotency-Key']); return Promise.reject(new Error('timeout')); }},
+}});
+first.submit(draft).catch(() => {{
+  const restored = createIssueCapture({{
+    storage,
+    createOperationId: () => 'must-not-replace-operation-177',
+    fetchJson: (_url, options) => {{
+      calls.push(options.headers['Idempotency-Key']);
+      return Promise.resolve({{number:17, title:'Capture work'}});
+    }},
+  }});
+  const before = restored.loadDraft();
+  restored.submit(before).then(issue => process.stdout.write(JSON.stringify({{
+    calls, before, after:restored.loadDraft(), number:issue.number
+  }})));
+}});
+"""
+    result = subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    )
+    output = json.loads(result.stdout)
+
+    assert output["calls"] == ["operation-177", "operation-177"]
+    assert output["before"] == {
+        "repository": "stackchain/api", "title": "Capture work", "body": "Context",
+        "labelIds": [3],
+    }
+    assert output["after"] == {
+        "repository": "", "title": "", "body": "", "labelIds": []
+    }
+    assert output["number"] == 17
+
+
 def test_issue_capture_loads_repository_labels_with_priorities_first():
    script = f"""
 const createIssueCapture = require({json.dumps(str(CREATE_ISSUE_SHEET))});