diff --git a/backend/app/debugsession.py b/backend/app/debugsession.py index bf459014..0f0aa167 100644 --- a/backend/app/debugsession.py +++ b/backend/app/debugsession.py @@ -238,17 +238,43 @@ def release_send(session: DebugSession, token: str) -> None: _write_delivery(session, {k: v for k, v in delivery(session).items() if k != "sending"}) -def set_delivery(session: DebugSession, state: dict) -> None: - """Write a delivery's progress (app/issues/gitea.py keeps it; this module reads it). +class SendClaimLost(Exception): + """This send's claim is no longer on the row: it aged out and another send took it over. The + send must stop — what it uploads from here it can no longer record, so a retry would send it + again — and it must not write, because the row now holds the other send's progress.""" - The claim is not the caller's to carry. `gitea.deliver` threads ONE snapshot of this dict + +def claim_owner(db: Session, session: DebugSession) -> str | None: + """Who holds the row's claim RIGHT NOW, read from the ROW and not from the object. + + `expire_on_commit` is off (app/db.py), so a checkpoint does not re-read: a send that stalled + past its lease still carries its own dead token in memory long after another send took over. + A targeted scalar, not `lock`/`db.refresh` — a refresh here would discard the `issue_url` that + `gitea.deliver` assigns one line before its first progress report, and that link is the one + thing the send must not lose.""" + return db.scalar( + select(DebugSession.draft["delivery"]["sending"]["by"].as_string()).where( + DebugSession.id == session.id + ) + ) + + +def set_delivery(db: Session, session: DebugSession, state: dict, token: str) -> None: + """Write a delivery's progress (app/issues/gitea.py keeps it; this module reads it), and renew + the claim while doing it — a send still reporting progress is still alive, so one slower than + the lease cannot expire while it is still running. + + The claim is not the caller's to carry: `gitea.deliver` threads ONE snapshot of this dict through a whole send, so a progress report that carried the claim wrote a stale one back at - every step, and a send longer than the lease then expired while still running. It is taken from - the row and re-stamped here instead: a send still reporting progress is still alive.""" - claim = _claim(session) + every step. It is stamped from `token` here instead, and only after the ROW confirms the claim + is still ours. Without that check the write is a full-column overwrite with no WHERE: a send + that lost its lease came back and stomped the take-over send's committed links, then put its + own dead token back on an idle session (deep-review round 4). `release_send` has always made + the same check; this is the other half of it.""" + if claim_owner(db, session) != token: + raise SendClaimLost(session.id) state = {k: v for k, v in state.items() if k != "sending"} - if claim: - state["sending"] = _stamp(claim["by"]) + state["sending"] = _stamp(token) _write_delivery(session, state) diff --git a/backend/app/issues/gitea.py b/backend/app/issues/gitea.py index a103c422..aeb24492 100644 --- a/backend/app/issues/gitea.py +++ b/backend/app/issues/gitea.py @@ -20,10 +20,12 @@ Delivery keeps its progress in the session's draft (`delivery`: the issue number upload, the links of those uploaded, whether the body shows the screenshots yet), so a failure half-way keeps the issue that exists and a retry does only what is missing — never a second issue. Every step is checkpointed (the caller's `db.commit`), so what did happen is kept whatever fails -next — and the first checkpoint comes before the first call, so no row lock and no read snapshot is -held across them. The caller claims the session for the send first (`debugsession.claim_send`), -which is what keeps a second send, an autosave or a screenshot out of the way; the claim is renewed -by every checkpoint. `debugsession.not_uploaded` / `images_pending` read that progress. +next — and the first checkpoint comes before the first call, with the tracker's settings read ABOVE +it, so neither a row lock nor a read snapshot nor a pooled connection is held across the calls. The +caller claims the session for the send first (`debugsession.claim_send`) and passes the claim's +token in, which is what keeps a second send, an autosave or a screenshot out of the way; the claim +is renewed by every progress report, and a report whose claim is gone raises rather than writing +over the send that took it. `debugsession.not_uploaded` / `images_pending` read that progress. """ from collections.abc import Callable, Iterator @@ -75,8 +77,10 @@ def target(db: Session) -> dict | None: @contextmanager -def _client(db: Session) -> Iterator[tuple[httpx.Client, str]]: - url, repo, token = _settings(db) +def _client(settings: tuple[str, str, str]) -> Iterator[tuple[httpx.Client, str]]: + # Takes the settings already read, not the session: reading them HERE opened a transaction one + # line after `deliver` had ended one, and held it across the calls below (deep-review round 4). + url, repo, token = settings if not (url and repo and token): raise GiteaError("not_configured") with httpx.Client( @@ -144,14 +148,14 @@ def _private_repo(client: httpx.Client, repo: str) -> dict: def check(db: Session) -> dict: """The admin's "test connection": the repository answers, is private and takes issues.""" - with _client(db) as (client, repo): + with _client(_settings(db)) as (client, repo): data = _private_repo(client, repo) return {"repo": data.get("full_name") or repo, "url": data.get("html_url")} def preview(db: Session, text: str) -> str: """The body as Gitea itself will render it (its markdown flavour, its sanitiser).""" - with _client(db) as (client, repo): + with _client(_settings(db)) as (client, repo): r = _call(client, "POST", "/markdown", json={"Text": text, "Mode": "gfm", "Context": repo}) return r.text @@ -160,12 +164,17 @@ def deliver( db: Session, session: DebugSession, *, + token: str, checkpoint: Callable[[], None], before_create: Callable[[], None] | None = None, on_created: Callable[[], None] | None = None, ) -> str: """Send the session's issue, or finish sending it; returns its URL. + `token` is the caller's claim on this session (`debugsession.claim_send`). Every progress report + renews it and checks the row still holds it, so a send that outlived its lease stops here rather + than writing over the one that took over (`debugsession.SendClaimLost`). + `checkpoint` persists what has happened so far and is called after every step (the caller's commit — this module does not decide when a transaction ends). `before_create` runs right before the tracker is asked to open a NEW issue, after the checks that can refuse it without asking, @@ -176,10 +185,15 @@ def deliver( state = debugsession.delivery(session) files = bundle.files(db, session) attachments = [f for f in files if f.name != "issue.md"] + settings = _settings(db) # Reading the bundle and the tracker's settings opened a transaction; end it before the first # call, so nothing holds a snapshot (and an idle connection) across minutes of tracker I/O. + # The settings are read HERE and handed to `_client`, not read inside it: reading them there + # was a SELECT one line after this commit, which reopened the transaction and held it — idle, + # with its snapshot pinned — across the repository check and the create (deep-review round 4 + # measured it in pg_stat_activity). A comment saying otherwise sat right here. checkpoint() - with _client(db) as (client, repo): + with _client(settings) as (client, repo): _private_repo(client, repo) if "number" not in state: if before_create is not None: @@ -194,9 +208,10 @@ def deliver( "number", "html_url", ) - # Built ON the state read at the start, not in place of it: the caller's claim on this - # send lives there, and replacing it would let a draft or a screenshot change the issue - # under the uploads that follow. + # Built ON the state read at the start so nothing already in it is dropped. The claim + # is NOT among those things, whatever this comment used to say: `set_delivery` strips + # any `sending` passed to it and stamps the row's own from `token` (round 4 — removing + # the spread left the whole suite green, because the claim never travelled here). state = { **state, "number": created["number"], @@ -204,7 +219,24 @@ def deliver( "links": {}, } session.issue_url = created["html_url"] - debugsession.set_delivery(session, state) + try: + debugsession.set_delivery(db, session, state, token) + except debugsession.SendClaimLost: + # The one step where losing the claim costs something irreversible: the issue is + # already open on the tracker and this is what would have recorded its link. We + # still must not write — another send owns the row and is opening its own issue — + # so say which issue was left behind, because nothing else will. + # noqa TRY400: the traceback is one SendClaimLost carrying the session id and + # nothing else; the issue number below is the whole news. + log.error( # noqa: TRY400 + "an issue was created by a send that had already lost its claim", + extra=logs.event( + "issues.gitea.orphaned", + number=created["number"], + session_id=session.id, + ), + ) + raise if on_created is not None: on_created() checkpoint() @@ -231,13 +263,13 @@ def deliver( "pending": [n for n in state["pending"] if n != item.name], "links": {**state["links"], item.name: asset["browser_download_url"]}, } - debugsession.set_delivery(session, state) + debugsession.set_delivery(db, session, state, token) checkpoint() shots = [n for n in state["links"] if n.startswith(debugsession.SCREENSHOT_PREFIX)] if shots and not state.get("images_shown"): images = "\n".join(f"![{n}]({state['links'][n]})" for n in sorted(shots)) body = f"{bundle.body_of(session)}\n\n---\n\n{images}" _call(client, "PATCH", f"/repos/{repo}/issues/{number}", json={"body": body}) - debugsession.set_delivery(session, {**state, "images_shown": True}) + debugsession.set_delivery(db, session, {**state, "images_shown": True}, token) checkpoint() return session.issue_url or "" diff --git a/backend/app/routes/debug_sessions.py b/backend/app/routes/debug_sessions.py index db909b6f..3054364f 100644 --- a/backend/app/routes/debug_sessions.py +++ b/backend/app/routes/debug_sessions.py @@ -195,7 +195,11 @@ _new_issues = RateLimiter(max_events=5, window_seconds=3600) # refused for good (the tracker's own limits) never leaves `pending`. No hand reaches it. _sends = RateLimiter(max_events=60, window_seconds=3600) # The composer's live preview, rendered BY the tracker: one authenticated call each, with our token. -# react-query debounces it to a handful a minute; this bounds what a loop can aim at the tracker. +# The composer's toggle fires it once per switch INTO preview — the editor is the other branch, so +# no keystroke can change the query key while the preview is up — and react-query's cache makes an +# unchanged draft free. (It does NOT debounce, whatever this comment used to say: a key that changes +# per keystroke is one fetch per keystroke, measured. Showing the preview BESIDE the fields would +# need a real debounce first.) This bounds what a loop can aim at the tracker. _previews = RateLimiter(max_events=60, window_seconds=60) @@ -238,10 +242,16 @@ def send_issue( gitea.deliver( db, session, + token=token, checkpoint=db.commit, before_create=count_new_issue, on_created=lambda: _record_created(db, user, session), ) + except debugsession.SendClaimLost: + # This send outlived its lease and another one took the session over while it was still + # talking to the tracker. It stopped rather than writing over that one's progress — and a + # send IS under way, just not ours, which is what the caller needs to hear. + raise refused(409, "issue_sending") from None except gitea.GiteaError as exc: # Keep what did happen: an issue created before an upload failed stays linked, and the # retry does only what is still missing. diff --git a/backend/tests/test_debug_sessions.py b/backend/tests/test_debug_sessions.py index 27094b26..5c1730b0 100644 --- a/backend/tests/test_debug_sessions.py +++ b/backend/tests/test_debug_sessions.py @@ -496,18 +496,24 @@ def test_a_half_sent_session_keeps_its_screenshots_a_finished_one_lets_them_go(d shot = _upload(client, sid, _PNG).json()["screenshot"]["id"] session = db.get(DebugSession, sid) session.issue_url = "https://git.example/o/r/issues/1" - debugsession.set_delivery(session, {"number": 1, "pending": ["server.log"], "links": {}}) + _progress(session, {"number": 1, "pending": ["server.log"], "links": {}}) db.commit() # A send that still owes files names its screenshots by their order: removing one renumbers # what it has left to send. r = client.delete(f"/api/debug-sessions/{sid}/screenshots/{shot}") assert r.status_code == 409 and r.json()["detail"]["reason"] == "screenshot_already_sent" # Finished: the issue has its own copy, and ours is the user's to drop. - debugsession.set_delivery(session, {"number": 1, "pending": [], "links": {}}) + _progress(session, {"number": 1, "pending": [], "links": {}}) db.commit() assert client.delete(f"/api/debug-sessions/{sid}/screenshots/{shot}").status_code == 200 +def _progress(session, state): + """The delivery a finished-or-abandoned send left on the row, with no claim standing on it. + `set_delivery` is the live send's writer and refuses without one, which is the point of it.""" + session.draft = {**(session.draft or {}), "delivery": state} + + def _age_the_claim(session, hours=1): """The claim a send holds, as a request that died an hour ago left it.""" state = debugsession.delivery(session) @@ -556,31 +562,50 @@ def test_a_slow_send_keeps_its_own_claim_alive_and_gives_back_only_that_one(db, session = db.get(DebugSession, sid) token = debugsession.claim_send(session) _age_the_claim(session) + db.commit() assert debugsession.sending(session) is False # One more step reported — proof of life, so the claim stands again. - debugsession.set_delivery(session, {"number": 7, "pending": ["server.log"], "links": {}}) + debugsession.set_delivery( + db, session, {"number": 7, "pending": ["server.log"], "links": {}}, token + ) + db.commit() assert debugsession.sending(session) is True assert debugsession.not_uploaded(session) == ["server.log"] + # And once a send that aged out HAS been taken over, the old one writes nothing: its progress + # report is a whole-column overwrite, so without this it put its own dead token back and took + # the new send's committed links with it. `expire_on_commit` is off, so the old send still has + # its own token in memory — only the row can say whose claim it is. + taken = debugsession.claim_send(session) + db.commit() + with pytest.raises(debugsession.SendClaimLost): + debugsession.set_delivery(db, session, {"number": 9, "pending": [], "links": {}}, token) + assert debugsession.delivery(session)["number"] == 7 + assert debugsession.not_uploaded(session) == ["server.log"] # The claim of a send that took over after ours aged out is not ours to hand back. - debugsession.release_send(session, "a-different-send") - assert debugsession.sending(session) is True debugsession.release_send(session, token) + assert debugsession.sending(session) is True + debugsession.release_send(session, taken) assert debugsession.sending(session) is False -def test_a_users_uploads_wait_for_each_other_across_their_sessions(db): +def test_a_users_uploads_wait_for_each_other_across_their_sessions(db, recording): """The cap below is the user's across ALL their sessions, but a session's row lock only bounds that session — two uploads to two sessions both read the total before either commits, and both - pass. The user's own lock is what makes the cap a cap instead of a guess.""" + pass. The user's own lock is what makes the cap a cap instead of a guess. + + Driven through `add_screenshot`, not through `lock_user`: the primitive works whether or not + the upload calls it, so a test that called it here stayed green with the call site deleted.""" from app.db import SessionLocal - debugsession.lock_user(db, 4242) + client, sid = recording + session = db.get(DebugSession, sid) + debugsession.add_screenshot(db, session, _PNG) # not committed: the lock is ours until it is other = SessionLocal() try: - # A second request for the same user cannot get in to read the total while we hold it... - assert other.scalar(select(func.pg_try_advisory_xact_lock(4242))) is False + # A second upload by this user cannot get in to read the total while this one holds it... + assert other.scalar(select(func.pg_try_advisory_xact_lock(session.user_id))) is False # ...and another user is not queued behind them for it. - assert other.scalar(select(func.pg_try_advisory_xact_lock(4243))) is True + assert other.scalar(select(func.pg_try_advisory_xact_lock(session.user_id + 1))) is True finally: other.rollback() other.close() @@ -883,15 +908,23 @@ def test_the_claim_is_held_through_every_step_of_a_send(gitea_fake, db, make_use _upload(client, sid, _PNG) _ready(client, sid) session = db.get(DebugSession, sid) - debugsession.claim_send(session) + token = debugsession.claim_send(session) db.commit() - held: list[bool] = [] + held: list[tuple[bool, int]] = [] gitea.deliver( db, session, - checkpoint=lambda: (db.commit(), held.append(debugsession.sending(session)))[0], + token=token, + checkpoint=lambda: ( + db.commit(), + held.append((debugsession.sending(session), len(gitea_fake.calls))), + )[0], ) - assert held and all(held) # after the issue was created, and after every upload + # The first checkpoint comes before the tracker is touched at all — that is what keeps the + # bundle's read snapshot, and its connection, out of the calls that follow. + assert held[0] == (True, 0) + # ...and the claim outlasts the issue's creation and every upload after it. + assert len(held) > 1 and all(alive for alive, _ in held) def test_the_issues_trail_is_written_by_the_step_that_writes_the_link( @@ -908,7 +941,7 @@ def test_the_issues_trail_is_written_by_the_step_that_writes_the_link( _upload(client, sid, _PNG) _ready(client, sid) session = db.get(DebugSession, sid) - debugsession.claim_send(session) + token = debugsession.claim_send(session) db.commit() seen: list[tuple[bool, int]] = [] @@ -922,11 +955,15 @@ def test_the_issues_trail_is_written_by_the_step_that_writes_the_link( gitea.deliver( db, session, + token=token, checkpoint=checkpoint, on_created=lambda: routes._record_created(db, user, session), ) - # Every commit that can be seen with the link can be seen with the trail beside it. - assert seen and all(rows == 1 for linked, rows in seen if linked) + # Every commit that can be seen with the link can be seen with the trail beside it — and there + # has to BE one, or the filter selects nothing and `all()` is vacuously true. This assertion + # used to go green over a `deliver` that never wrote the link at all. + linked_rows = [rows for linked, rows in seen if linked] + assert linked_rows and all(rows == 1 for rows in linked_rows) def test_a_send_that_cannot_give_its_claim_back_still_reports_its_own_failure( diff --git a/frontend/src/components/DebugRecorder.tsx b/frontend/src/components/DebugRecorder.tsx index e28f5db2..26ac871b 100644 --- a/frontend/src/components/DebugRecorder.tsx +++ b/frontend/src/components/DebugRecorder.tsx @@ -326,10 +326,20 @@ function StepPrompt({ id, onDone }: { id: string; onDone: (label: string | null) ); } +/** The five words a past session can read as. A union, not `string`, because `chipTone` below + * matches on these same keys: untyped, a rename in one of them left the other matching nothing + * and every chip untoned, with the gate green (deep-review round 4). */ +type HistoryState = + | "recorder.history.sending" + | "recorder.history.draft" + | "recorder.history.unwritten" + | "recorder.history.partial" + | "recorder.history.sent"; + /** What a past session reads as. An issue that still owes the tracker its attachments, or an edit * that shows its screenshots, is NOT "Sent": this list is where a user goes back to check, and a * green chip there is how a half-delivered report gets forgotten. */ -function historyState(s: DebugSession): string { +function historyState(s: DebugSession): HistoryState { // A send under way outranks every other word here: this is the list a user comes back to in // order to check on one, and both "draft" (no issue yet) and "half sent" (the steps it has // already committed) would describe a send that STOPPED. @@ -340,11 +350,20 @@ function historyState(s: DebugSession): string { : "recorder.history.sent"; } +/** The colour that reinforces that word. Exhaustive over `HistoryState`, so a key renamed in + * `historyState` is a compile error here rather than a chip that quietly loses its tone. */ const chipTone = (s: DebugSession): string => { - const state = historyState(s); - if (state === "recorder.history.sent") return "text-success"; - if (state === "recorder.history.sending") return "text-accent"; - return state === "recorder.history.partial" ? "text-warn" : ""; + switch (historyState(s)) { + case "recorder.history.sent": + return "text-success"; + case "recorder.history.sending": + return "text-accent"; + case "recorder.history.partial": + return "text-warn"; + case "recorder.history.draft": + case "recorder.history.unwritten": + return ""; + } }; /** The last sessions, newest first — reopening one opens its composer, with what was written. */ diff --git a/frontend/src/components/IssueComposer.dom.test.tsx b/frontend/src/components/IssueComposer.dom.test.tsx index 1e874157..b1a0ec0a 100644 --- a/frontend/src/components/IssueComposer.dom.test.tsx +++ b/frontend/src/components/IssueComposer.dom.test.tsx @@ -35,6 +35,8 @@ const api = vi.hoisted(() => ({ // What happened, in order: a save is "save" when asked and "saved" when it answered. order: [] as string[], holdSend: null as Promise | null, + // The server's half-failure: the issue is committed, then an upload dies and the call 422s. + failSendAfterCreate: false, })); vi.mock("../lib/api", async (importOriginal) => { const actual = await importOriginal(); @@ -76,8 +78,13 @@ vi.mock("../lib/api", async (importOriginal) => { api.session = { ...api.session, issue_url: "https://git.example/o/r/issues/7", - pending_files: [], + pending_files: api.failSendAfterCreate ? ["server.log"] : [], + // What the server does on the way through: a send that got there shows the images too, + // so the banner (and the Retry in it) goes. Leaving this true made the retry's own + // banner outlive its success, which is not a state the server can produce. + images_pending: false, }; + if (api.failSendAfterCreate) throw new Error("issues_unreachable"); return { ok: true as const, session: api.session }; }, previewDebugIssue: (_id: string, text: string) => { @@ -138,6 +145,7 @@ beforeEach(() => { api.failShots = false; api.order = []; api.holdSend = null; + api.failSendAfterCreate = false; api.sent = 0; api.previewed = []; tracker = null; @@ -326,6 +334,69 @@ describe("the issue composer", () => { ).toBe(true); }); + it("announces the half-sent banner instead of only drawing it", async () => { + // A screen-reader user hears the failure toast — "couldn't send the report" — and nothing else. + // WHICH attachments are missing, and that a Retry now stands beside them, live only in this + // banner, which is inserted already carrying its first message. A live region created together + // with its content is unreliably announced (the rule ui/Toaster.tsx states and follows), so the + // announcement has to come from a ROLE, which announces on insertion. + api.session = baseSession({ + issue_url: "https://git.example/o/r/issues/7", + pending_files: ["server.log"], + }); + mount(); + const said = await screen.findByText(/recorder\.composer\.partial/); + const announced = said.closest('[role="status"]'); + expect(announced).not.toBeNull(); + // ...and the Retry is inside what gets read out, not left behind silently beside it. + expect(announced!.querySelector("button")).not.toBeNull(); + }); + + it("hands the focus on when a send half-fails, not only when it works", async () => { + tracker = { repo: "o/r" }; + // The issue is created and an upload then dies, so the call FAILS with the link already + // committed. The refetch brings the link back, `sent` flips, and "Create issue" — the control + // holding the focus — unmounts under it. Without a hand-off the focus lands on in the + // middle of an open dialog: a screen reader loses its place and the Retry that just appeared + // is several Tabs away, past the dialog's own ✕. + api.failSendAfterCreate = true; + mount(); + const send = (await ready("recorder.composer.send")) as HTMLButtonElement; + send.focus(); + expect(document.activeElement).toBe(send); + fireEvent.click(send); + await waitFor(() => expect(api.sent).toBe(1)); + const retry = await screen.findByRole("button", { name: "recorder.composer.retry" }); + await waitFor(() => expect(document.activeElement).toBe(retry)); + expect(screen.queryByRole("button", { name: "recorder.composer.send" })).toBeNull(); + }); + + it("still builds the zip while a send holds the draft, instead of saving into a refusal", async () => { + tracker = { repo: "o/r" }; + // The zip is the only way out of a send that is stuck — a claim left by a dead request stands + // for ten minutes. Its pre-save is pointless here (the draft is frozen, and the server would + // 409 `issue_sending` and take the export down with it), and unnecessary: `GET /export` has no + // send guard and builds from the saved draft, which is exactly what the send is uploading. + api.session = baseSession({ sending: true }); + const click = vi.spyOn(HTMLAnchorElement.prototype, "click"); + click.mockImplementation(() => void api.order.push("click")); + const real = { make: URL.createObjectURL, revoke: URL.revokeObjectURL }; + URL.createObjectURL = () => "blob:zip"; + URL.revokeObjectURL = () => {}; + try { + mount(); + expect(await screen.findByText("recorder.composer.sendingNow")).not.toBeNull(); + fireEvent.click(await ready("recorder.composer.export")); + await waitFor(() => expect(click).toHaveBeenCalled()); + expect(api.order).toEqual(["export", "click"]); + expect(api.saved).toHaveLength(0); + } finally { + click.mockRestore(); + URL.createObjectURL = real.make; + URL.revokeObjectURL = real.revoke; + } + }); + it("asks to show the images, not to send what is already there", async () => { tracker = { repo: "o/r" }; api.session = baseSession({ diff --git a/frontend/src/components/IssueComposer.tsx b/frontend/src/components/IssueComposer.tsx index 5a77ef87..60a2220f 100644 --- a/frontend/src/components/IssueComposer.tsx +++ b/frontend/src/components/IssueComposer.tsx @@ -121,6 +121,7 @@ function ComposerBody({ ); const [reviewing, setReviewing] = useState(null); const issueLink = useRef(null); + const retryButton = useRef(null); // The log whose review just closed: its Review button takes the focus back. const reviewed = useRef(null); const sent = session.issue_url !== null; @@ -205,14 +206,17 @@ function ComposerBody({ onSuccess: (r) => { qc.setQueryData(qk.debugSession(session.id), r.session); notify({ level: "success", message: t("recorder.composer.sentOk") }); - // The Retry that was just pressed lives in a banner that this answer unmounts. Hand the focus - // to the link that takes its place, the way every other control-that-goes here does. - window.setTimeout(() => issueLink.current?.focus(), 0); }, // A failure half-way still created the issue: show what the server now has. - onSettled: () => { + onSettled: async () => { void qc.invalidateQueries({ queryKey: qk.debugSessions(), exact: true }); - void qc.invalidateQueries({ queryKey: qk.debugSession(session.id), exact: true }); + await qc.invalidateQueries({ queryKey: qk.debugSession(session.id), exact: true }); + // Whichever way it went, the button that was just pressed is gone: a success unmounts the + // Retry's banner, and a half-failure unmounts Create issue (the refetch above brings back the + // issue_url it created before it died). Either leaves the focus on inside an open + // dialog, so hand it on — to whatever now stands where the pressed control was. AFTER the + // refetch, not in onSuccess/onError: before it lands, neither target is rendered yet. + window.setTimeout(() => (retryButton.current ?? issueLink.current)?.focus(), 0); }, }); const locked = sent || send.isPending || sending; @@ -258,10 +262,14 @@ function ComposerBody({ }, [locked]); const [mode, setMode] = useState<"edit" | "preview">("edit"); - // The export is built from the SAVED draft: save what is on screen first. + // The export is built from the SAVED draft: save what is on screen first — except when the + // draft is already frozen (`locked`), where the save is both pointless and refused. While a send + // holds the claim the server 409s `issue_sending`, and that refusal would take the zip with it: + // `GET /export` has no send guard and would have served exactly what the send is uploading, so + // the one tracker-free way out of a stuck send used to fail at the last step (round 4). const exportZip = useMutation({ mutationFn: async () => { - if (!sent) await save.mutateAsync(); + if (!locked) await save.mutateAsync(); return api.debugExport(session.id); }, meta: { failTitle: "recorder.composer.exportFailed" }, @@ -297,7 +305,11 @@ function ComposerBody({ className={`flex flex-wrap items-center justify-between gap-2 rounded-xl border px-3 py-2 text-sm ${ sending ? "border-accent/40 bg-accent/10" : "border-warn/40 bg-warn/10" }`} - aria-live="polite" + // `role="status"` rather than `aria-live`: this box is mounted TOGETHER with its first + // message (a half-failed send inserts it already populated), and a live region created + // with its content is unreliably announced — the rule ui/Toaster.tsx states and follows. + // A role announces on insertion, and its implied atomic re-read is right for one line. + role="status" > {sending @@ -309,6 +321,7 @@ function ComposerBody({ {/* Nothing to retry while it is still going — the button would only be refused. */} {!sending && (