diff --git a/README.md b/README.md index 55f84ad..40c6c0a 100644 --- 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 diff --git a/frontend/create-issue-sheet.js b/frontend/create-issue-sheet.js index 324e37d..d9fac54 100644 --- 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, }), diff --git a/src/main.py b/src/main.py index 6786f3d..072d901 100644 --- 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: diff --git a/tests/test_issue_api.py b/tests/test_issue_api.py index 9598b6f..5435a18 100644 --- 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 = [] diff --git a/tests/test_my_work.py b/tests/test_my_work.py index b0cfdc9..59faeda 100644 --- 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))});