Make mobile issue capture idempotent across retries #178

Merged
rockachopa merged 1 commits from timmy/177-idempotent-mobile-issue-create into main 2026-08-07 07:55:31 +00:00
5 changed files with 316 additions and 19 deletions

View File

@ -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. requests, merge assigned pull requests, and submit pull-request reviews.
Pull-request replies and mobile My Work issue and PR comments use Gitea's Pull-request replies and mobile My Work issue and PR comments use Gitea's
issue-comment API; mobile issue capture requires issue 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 Approve, and Request changes reviews, and assigned-PR merge require repository
write permission. Native Comment, Approve, and Request changes reviews support write permission. Native Comment, Approve, and Request changes reviews support
head-scoped draft comments anchored to changed lines; the dashboard validates each head-scoped draft comments anchored to changed lines; the dashboard validates each

View File

@ -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'; const storageKey = 'stackchain.issue-capture.v1';
let pending = null; let pending = null;
const safeLabelIds = value => Array.from(new Set( const safeLabelIds = value => Array.from(new Set(
@ -6,30 +16,44 @@ function createIssueCapture({ fetchJson, storage }) {
)).slice(0, 20); )).slice(0, 20);
const emptyDraft = () => ({ repository: '', title: '', body: '', labelIds: [] }); 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) { function saveDraft(draft) {
const previous = loadStored();
const safe = { const safe = {
repository: String(draft?.repository || ''), repository: String(draft?.repository || ''),
title: String(draft?.title || ''), title: String(draft?.title || ''),
body: String(draft?.body || ''), body: String(draft?.body || ''),
labelIds: safeLabelIds(draft?.labelIds), labelIds: safeLabelIds(draft?.labelIds),
}; };
try { storage.setItem(storageKey, JSON.stringify(safe)); } const unchanged = ['repository', 'title', 'body'].every(key => previous[key] === safe[key]) &&
catch (_error) { /* Keep the form as the in-memory fallback. */ } JSON.stringify(previous.labelIds) === JSON.stringify(safe.labelIds);
writeStored({...safe, operationId: unchanged ? previous.operationId : ''});
return safe; return safe;
} }
function loadDraft() { function loadDraft() {
try { const {operationId: _operationId, ...draft} = loadStored();
const parsed = JSON.parse(storage.getItem(storageKey) || 'null'); return draft;
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();
}
} }
function clearDraft() { function clearDraft() {
@ -52,10 +76,16 @@ function createIssueCapture({ fetchJson, storage }) {
function submit(draft) { function submit(draft) {
if (pending) return pending; if (pending) return pending;
const saved = saveDraft(draft); 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('/'); const repository = saved.repository.split('/').map(encodeURIComponent).join('/');
pending = fetchJson('api/v1/repos/' + repository + '/issues', { pending = fetchJson('api/v1/repos/' + repository + '/issues', {
method: 'POST', method: 'POST',
headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, headers: {
Accept: 'application/json', 'Content-Type': 'application/json',
'Idempotency-Key': operationId,
},
body: JSON.stringify({ body: JSON.stringify({
title: saved.title, body: saved.body, label_ids: saved.labelIds, title: saved.title, body: saved.body, label_ids: saved.labelIds,
}), }),

View File

@ -6,7 +6,7 @@ from contextlib import asynccontextmanager
from pathlib import Path from pathlib import Path
from typing import Any, Literal 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.responses import JSONResponse
from fastapi.staticfiles import StaticFiles from fastapi.staticfiles import StaticFiles
from pydantic import BaseModel, Field, PositiveInt, field_validator, model_validator 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 READINESS_TIMEOUT_SECONDS = 5.0
REVIEW_DETAIL_TIMEOUT_SECONDS = 5.0 REVIEW_DETAIL_TIMEOUT_SECONDS = 5.0
ISSUE_ACTION_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_MUTATION_TIMEOUT_SECONDS = 5.0
NOTIFICATION_PAGE_TIMEOUT_SECONDS = 5.0 NOTIFICATION_PAGE_TIMEOUT_SECONDS = 5.0
WORK_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() _live_snapshot_refreshing_sections: set[str] = set()
_read_notification_ids: set[int] = set() _read_notification_ids: set[int] = set()
_issue_creation_operations: dict[
str, tuple[tuple[Any, ...], asyncio.Task, float]
] = {}
class ContextPayloadError(ValueError): 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) @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}" repository = f"{owner}/{repo}"
async def create_issue(): 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 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: 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: except HTTPException:
raise raise
except Exception: except Exception:

View File

@ -6,6 +6,13 @@ import pytest
from src import gitea_proxy, main 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 @pytest.mark.anyio
async def test_create_issue_endpoint_derives_self_assignment_and_returns_confirmed_issue(monkeypatch): async def test_create_issue_endpoint_derives_self_assignment_and_returns_confirmed_issue(monkeypatch):
calls = [] 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])] 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 @pytest.mark.anyio
async def test_gitea_create_issue_posts_self_assignment_and_normalizes_confirmation(): async def test_gitea_create_issue_posts_self_assignment_and_normalizes_confirmation():
requests = [] requests = []

View File

@ -395,6 +395,53 @@ Promise.all([first, duplicate]).then(results => process.stdout.write(JSON.string
assert output["results"][1]["number"] == 17 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(): def test_issue_capture_loads_repository_labels_with_priorities_first():
script = f""" script = f"""
const createIssueCapture = require({json.dumps(str(CREATE_ISSUE_SHEET))}); const createIssueCapture = require({json.dumps(str(CREATE_ISSUE_SHEET))});