Merge pull request 'Make Search watch admission truthful and recoverable' (#1296) from timmy/1295-truthful-search-watch-admission into main
This commit is contained in:
commit
bbf397454d
|
|
@ -48,6 +48,7 @@
|
||||||
watching:'Starting watch…', unwatching:'Stopping watch…',
|
watching:'Starting watch…', unwatching:'Stopping watch…',
|
||||||
watched:'Watching · available in Following. Future activity will appear in Updates.',
|
watched:'Watching · available in Following. Future activity will appear in Updates.',
|
||||||
unwatched:'Stopped watching · removed from Following. Assignment and planning are unchanged.',
|
unwatched:'Stopped watching · removed from Following. Assignment and planning are unchanged.',
|
||||||
|
'watch-partial':'Watching in Gitea, but Following could not sync. Tap Stop watching, then Watch issue to repair.',
|
||||||
'watch-error':(state.error?.message || 'Watch status was not changed.') + ' Retry.',
|
'watch-error':(state.error?.message || 'Watch status was not changed.') + ' Retry.',
|
||||||
})[state.status] || '';
|
})[state.status] || '';
|
||||||
root.renderSearchPreviewWatch = (detail, state, button) => {
|
root.renderSearchPreviewWatch = (detail, state, button) => {
|
||||||
|
|
@ -385,7 +386,9 @@
|
||||||
publish({ status:watching ? 'watching' : 'unwatching', item:current, detail });
|
publish({ status:watching ? 'watching' : 'unwatching', item:current, detail });
|
||||||
watchRequest = watch(detail, watching).then(result => {
|
watchRequest = watch(detail, watching).then(result => {
|
||||||
current = { ...current, watching:result?.watching === true };
|
current = { ...current, watching:result?.watching === true };
|
||||||
publish({ status:watching ? 'watched' : 'unwatched', item:current, detail:current, result });
|
const status = result?.following_synced === false
|
||||||
|
? 'watch-partial' : (watching ? 'watched' : 'unwatched');
|
||||||
|
publish({ status, item:current, detail:current, result });
|
||||||
return result;
|
return result;
|
||||||
}).catch(error => {
|
}).catch(error => {
|
||||||
publish({ status:'watch-error', item:current, detail, error });
|
publish({ status:'watch-error', item:current, detail, error });
|
||||||
|
|
|
||||||
|
|
@ -113,6 +113,27 @@ class FollowingStore:
|
||||||
)
|
)
|
||||||
return snapshot
|
return snapshot
|
||||||
|
|
||||||
|
def preflight(self, login: str, raw_item: dict, watching: bool) -> dict:
|
||||||
|
"""Validate a requested change and capacity without mutating the registry."""
|
||||||
|
login = self._login(login)
|
||||||
|
item = self._normalize_item(raw_item)
|
||||||
|
if not isinstance(watching, bool):
|
||||||
|
raise ValueError("watching is invalid")
|
||||||
|
if not watching:
|
||||||
|
return item
|
||||||
|
identity = self._identity(item)
|
||||||
|
with self._connect() as connection:
|
||||||
|
row = connection.execute(
|
||||||
|
"SELECT revision, items FROM following_issues WHERE login = ?", (login,)
|
||||||
|
).fetchone()
|
||||||
|
current, _legacy = self._snapshot(row, login)
|
||||||
|
if (
|
||||||
|
len(current["items"]) >= self.limit
|
||||||
|
and not any(self._identity(candidate) == identity for candidate in current["items"])
|
||||||
|
):
|
||||||
|
raise ValueError(f"following is limited to {self.limit} issues")
|
||||||
|
return item
|
||||||
|
|
||||||
def set_watching(self, login: str, raw_item: dict, watching: bool) -> dict:
|
def set_watching(self, login: str, raw_item: dict, watching: bool) -> dict:
|
||||||
login = self._login(login)
|
login = self._login(login)
|
||||||
item = self._normalize_item(raw_item)
|
item = self._normalize_item(raw_item)
|
||||||
|
|
|
||||||
|
|
@ -608,6 +608,8 @@ async def work_preview(repository: str, kind: str, number: int) -> dict:
|
||||||
"title": issue.get("title", "") if isinstance(issue.get("title"), str) else "",
|
"title": issue.get("title", "") if isinstance(issue.get("title"), str) else "",
|
||||||
"body": issue.get("body", "") if isinstance(issue.get("body"), str) else "",
|
"body": issue.get("body", "") if isinstance(issue.get("body"), str) else "",
|
||||||
"state": state,
|
"state": state,
|
||||||
|
"updated_at": issue.get("updated_at", "")
|
||||||
|
if isinstance(issue.get("updated_at"), str) else "",
|
||||||
"author": author.get("login", "") if isinstance(author.get("login"), str) else "",
|
"author": author.get("login", "") if isinstance(author.get("login"), str) else "",
|
||||||
"labels": [
|
"labels": [
|
||||||
label["name"] for label in labels
|
label["name"] for label in labels
|
||||||
|
|
|
||||||
46
src/main.py
46
src/main.py
|
|
@ -3738,26 +3738,43 @@ async def mutate_global_search_preview_subscription(
|
||||||
repository, preview = await _search_preview_subscription_target(owner, repo, number, kind)
|
repository, preview = await _search_preview_subscription_target(owner, repo, number, kind)
|
||||||
login = await _confirmed_login()
|
login = await _confirmed_login()
|
||||||
store = _following_store()
|
store = _following_store()
|
||||||
await asyncio.to_thread(store.get, login)
|
following_item = {
|
||||||
|
"repository": repository,
|
||||||
|
"number": number,
|
||||||
|
"title": preview.get("title", ""),
|
||||||
|
"state": preview.get("state", ""),
|
||||||
|
"updated_at": preview.get("updated_at", ""),
|
||||||
|
"url": preview.get("url", ""),
|
||||||
|
}
|
||||||
|
await asyncio.to_thread(store.preflight, login, following_item, watching)
|
||||||
result = await asyncio.wait_for(
|
result = await asyncio.wait_for(
|
||||||
gitea_proxy.set_issue_subscription(repository, number, watching),
|
gitea_proxy.set_issue_subscription(repository, number, watching),
|
||||||
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
|
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
|
||||||
)
|
)
|
||||||
if result.get("watching") is not watching:
|
if result.get("watching") is not watching:
|
||||||
raise RuntimeError("Gitea did not confirm subscription state")
|
raise RuntimeError("Gitea did not confirm subscription state")
|
||||||
following = await asyncio.to_thread(
|
try:
|
||||||
store.set_watching,
|
following = await asyncio.to_thread(
|
||||||
login,
|
store.set_watching,
|
||||||
{
|
login,
|
||||||
"repository": repository,
|
following_item,
|
||||||
"number": number,
|
watching,
|
||||||
"title": preview.get("title", ""),
|
)
|
||||||
"state": preview.get("state", ""),
|
except Exception:
|
||||||
"updated_at": preview.get("updated_at", ""),
|
try:
|
||||||
"url": preview.get("url", ""),
|
compensated = await asyncio.wait_for(
|
||||||
},
|
gitea_proxy.set_issue_subscription(repository, number, not watching),
|
||||||
watching,
|
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
|
||||||
)
|
)
|
||||||
|
except Exception:
|
||||||
|
compensated = None
|
||||||
|
if not isinstance(compensated, dict) or compensated.get("watching") is not (not watching):
|
||||||
|
return JSONResponse({
|
||||||
|
**result,
|
||||||
|
"following_synced": False,
|
||||||
|
"error": "Watching in Gitea, but Following could not sync. Retry this action.",
|
||||||
|
})
|
||||||
|
raise
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|
@ -3768,6 +3785,7 @@ async def mutate_global_search_preview_subscription(
|
||||||
)
|
)
|
||||||
return JSONResponse({
|
return JSONResponse({
|
||||||
**result,
|
**result,
|
||||||
|
"following_synced": True,
|
||||||
"following_revision": following["revision"],
|
"following_revision": following["revision"],
|
||||||
"following_count": len(following["items"]),
|
"following_count": len(following["items"]),
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -838,6 +838,37 @@ process.stdout.write(JSON.stringify({{calls,status:final.status,number:final.det
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_search_preview_preserves_authoritative_watch_and_offers_following_repair():
|
||||||
|
script = f"""
|
||||||
|
const createSearchPreview = require({json.dumps(str(SEARCH_PREVIEW))});
|
||||||
|
(async () => {{
|
||||||
|
const states = [];
|
||||||
|
const detail = {{repository:'stackchain/api',number:42,kind:'issue',state:'open',claimable:true,watching:false}};
|
||||||
|
const preview = createSearchPreview({{
|
||||||
|
fetchJson:async()=>detail,
|
||||||
|
watch:async()=>({{watching:true,following_synced:false,error:'Watching in Gitea, but Following could not sync. Retry this action.'}}),
|
||||||
|
onState:state=>states.push(state),
|
||||||
|
}});
|
||||||
|
await preview.open(detail);
|
||||||
|
await preview.setWatching(true);
|
||||||
|
const final=states.at(-1);
|
||||||
|
process.stdout.write(JSON.stringify({{
|
||||||
|
status:final.status,
|
||||||
|
watching:final.detail.watching,
|
||||||
|
message:globalThis.searchPreviewWatchStatus(final),
|
||||||
|
}}));
|
||||||
|
}})().catch(error=>{{console.error(error);process.exit(1);}});
|
||||||
|
"""
|
||||||
|
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||||
|
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
assert json.loads(result.stdout) == {
|
||||||
|
"status": "watch-partial",
|
||||||
|
"watching": True,
|
||||||
|
"message": "Watching in Gitea, but Following could not sync. Tap Stop watching, then Watch issue to repair.",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def test_mobile_search_preview_exposes_touch_safe_watch_action():
|
def test_mobile_search_preview_exposes_touch_safe_watch_action():
|
||||||
html = dashboard_bundle_text()
|
html = dashboard_bundle_text()
|
||||||
css = (FRONTEND / "dashboard.css").read_text()
|
css = (FRONTEND / "dashboard.css").read_text()
|
||||||
|
|
|
||||||
|
|
@ -42,7 +42,12 @@ async def test_confirmed_watch_updates_account_following_collection(monkeypatch,
|
||||||
following = await client.get("/api/v1/following")
|
following = await client.get("/api/v1/following")
|
||||||
|
|
||||||
assert watched.status_code == 200
|
assert watched.status_code == 200
|
||||||
assert watched.json() == {"watching": True, "following_revision": 1, "following_count": 1}
|
assert watched.json() == {
|
||||||
|
"watching": True,
|
||||||
|
"following_synced": True,
|
||||||
|
"following_revision": 1,
|
||||||
|
"following_count": 1,
|
||||||
|
}
|
||||||
assert following.status_code == 200
|
assert following.status_code == 200
|
||||||
assert following.headers["cache-control"] == "no-store"
|
assert following.headers["cache-control"] == "no-store"
|
||||||
assert following.json() == {"revision": 1, "items": [{
|
assert following.json() == {"revision": 1, "items": [{
|
||||||
|
|
@ -53,3 +58,125 @@ async def test_confirmed_watch_updates_account_following_collection(monkeypatch,
|
||||||
"updated_at": "2026-08-23T03:00:00Z",
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
"url": "https://forge.example/stackchain/api/issues/42",
|
"url": "https://forge.example/stackchain/api/issues/42",
|
||||||
}]}
|
}]}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_full_following_collection_rejects_watch_before_gitea_mutation(monkeypatch, tmp_path):
|
||||||
|
store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=b"b" * 32, limit=1)
|
||||||
|
store.set_watching("timmy", {
|
||||||
|
"repository": "stackchain/api", "number": 7, "title": "Already followed",
|
||||||
|
"state": "open", "updated_at": "2026-08-23T02:00:00Z",
|
||||||
|
"url": "https://forge.example/stackchain/api/issues/7",
|
||||||
|
}, True)
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
async def preview(repository, kind, number):
|
||||||
|
return {
|
||||||
|
"repository": repository, "kind": kind, "number": number,
|
||||||
|
"title": "Another issue", "state": "open",
|
||||||
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
|
"url": "https://forge.example/stackchain/api/issues/42", "claimable": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
async def set_subscription(*args):
|
||||||
|
calls.append(args)
|
||||||
|
return {"watching": True}
|
||||||
|
|
||||||
|
async def user():
|
||||||
|
return {"login": "timmy"}
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "_following_store", lambda: store)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "work_preview", preview)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "set_issue_subscription", set_subscription)
|
||||||
|
monkeypatch.setattr(main, "current_user", user)
|
||||||
|
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.put(
|
||||||
|
"/api/v1/repos/stackchain/api/issues/42/preview/subscription?kind=issue"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 503
|
||||||
|
assert calls == []
|
||||||
|
assert store.get("timmy")["items"][0]["number"] == 7
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_following_write_failure_compensates_confirmed_gitea_watch(monkeypatch, tmp_path):
|
||||||
|
store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=b"c" * 32)
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
async def preview(repository, kind, number):
|
||||||
|
return {
|
||||||
|
"repository": repository, "kind": kind, "number": number,
|
||||||
|
"title": "Recoverable watch", "state": "open",
|
||||||
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
|
"url": "https://forge.example/stackchain/api/issues/42", "claimable": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
async def set_subscription(repository, number, watching):
|
||||||
|
calls.append(watching)
|
||||||
|
return {"watching": watching}
|
||||||
|
|
||||||
|
async def user():
|
||||||
|
return {"login": "timmy"}
|
||||||
|
|
||||||
|
def fail_write(*args):
|
||||||
|
raise OSError("disk unavailable")
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "_following_store", lambda: store)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "work_preview", preview)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "set_issue_subscription", set_subscription)
|
||||||
|
monkeypatch.setattr(main, "current_user", user)
|
||||||
|
monkeypatch.setattr(store, "set_watching", fail_write)
|
||||||
|
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.put(
|
||||||
|
"/api/v1/repos/stackchain/api/issues/42/preview/subscription?kind=issue"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 503
|
||||||
|
assert response.json() == {"error": "Watch status was not changed. Please retry."}
|
||||||
|
assert calls == [True, False]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_uncompensated_following_write_reports_authoritative_partial_outcome(monkeypatch, tmp_path):
|
||||||
|
store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=b"d" * 32)
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
async def preview(repository, kind, number):
|
||||||
|
return {
|
||||||
|
"repository": repository, "kind": kind, "number": number,
|
||||||
|
"title": "Split watch", "state": "open",
|
||||||
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
|
"url": "https://forge.example/stackchain/api/issues/42", "claimable": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
async def set_subscription(repository, number, watching):
|
||||||
|
calls.append(watching)
|
||||||
|
return {"watching": True}
|
||||||
|
|
||||||
|
async def user():
|
||||||
|
return {"login": "timmy"}
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "_following_store", lambda: store)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "work_preview", preview)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "set_issue_subscription", set_subscription)
|
||||||
|
monkeypatch.setattr(main, "current_user", user)
|
||||||
|
monkeypatch.setattr(store, "set_watching", lambda *args: (_ for _ in ()).throw(OSError()))
|
||||||
|
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.put(
|
||||||
|
"/api/v1/repos/stackchain/api/issues/42/preview/subscription?kind=issue"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json() == {
|
||||||
|
"watching": True,
|
||||||
|
"following_synced": False,
|
||||||
|
"error": "Watching in Gitea, but Following could not sync. Retry this action.",
|
||||||
|
}
|
||||||
|
assert calls == [True, False]
|
||||||
|
|
|
||||||
|
|
@ -285,6 +285,7 @@ async def test_search_preview_subscription_mutation_revalidates_target_and_confi
|
||||||
expected_count = 1 if watching else 0
|
expected_count = 1 if watching else 0
|
||||||
assert response.json() == {
|
assert response.json() == {
|
||||||
"watching": watching,
|
"watching": watching,
|
||||||
|
"following_synced": True,
|
||||||
"following_revision": expected_count,
|
"following_revision": expected_count,
|
||||||
"following_count": expected_count,
|
"following_count": expected_count,
|
||||||
}
|
}
|
||||||
|
|
@ -784,6 +785,7 @@ async def test_work_preview_normalizes_details_and_only_allows_unassigned_open_i
|
||||||
"title": "Repair queue",
|
"title": "Repair queue",
|
||||||
"body": "Keep mobile operators moving.",
|
"body": "Keep mobile operators moving.",
|
||||||
"state": "open",
|
"state": "open",
|
||||||
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
"html_url": "http://127.0.0.1:3000/stackchain/api/issues/42",
|
"html_url": "http://127.0.0.1:3000/stackchain/api/issues/42",
|
||||||
"user": {"login": "alex"},
|
"user": {"login": "alex"},
|
||||||
"labels": [{"name": "P1"}, None],
|
"labels": [{"name": "P1"}, None],
|
||||||
|
|
@ -803,6 +805,7 @@ async def test_work_preview_normalizes_details_and_only_allows_unassigned_open_i
|
||||||
"title": "Repair queue",
|
"title": "Repair queue",
|
||||||
"body": "Keep mobile operators moving.",
|
"body": "Keep mobile operators moving.",
|
||||||
"state": "open",
|
"state": "open",
|
||||||
|
"updated_at": "2026-08-23T03:00:00Z",
|
||||||
"author": "alex",
|
"author": "alex",
|
||||||
"labels": ["P1"],
|
"labels": ["P1"],
|
||||||
"assignees": [],
|
"assignees": [],
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user