feat(sessions): give every one-way transition a way back (#416) #449
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/416-reversible-transitions"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes the five stuck states in #416 that had no existing owner. Two commits.
Commit 1 — reversing four one-way transitions
Four states could be entered and never left. No endpoint reversed any of them, so a mis-click or a dead worker left editing the database by hand as the only repair:
confirm_sessionaccepts onlyproposed, andreopen_session_for_votingexcludes both — nothing anywhere assigns a status back. "Mark completed" fires the notes harvest and the Discord fan-out irreversibly.generating.iterate_draftrefuses a generating draft and only one draft may exist per entry, so the single available exit wasdiscard_draft— a harddb.deletetaking the GM's inline edits,iteration_count,last_directiveand the LLM output with it. Trading away a session's work to unstick a status field is not a recovery path.Adds
POST /sessions/{id}/uncomplete,/uncancel,/unapprove, andPOST /campaigns/{id}/lore/{entry}/drafts/{draft}/reset— each with the UI affordance to reach it, all low-emphasis: these are recovery paths, not normal steps.Both session reversals are audited, because they rewrite a state players and Discord have already seen. Un-cancelling reconstructs the prior status from
confirmed_timerather than storing it — nothing recorded it, and a column to hold a guess is not worth a migration.The draft reset only marks the draft
failed. That is the whole fix: iterate, approve and discard are all already reachable fromfailed, so the GM regains every option by choice rather than by force, with their work intact. A 15-minute floor keeps a slow LLM call from being mistaken for a dead one.What deliberately stays done
Reversal is not a rewrite of history, and three tests pin that so nobody later "improves" it into one:
LoreEntry.gm_notes. They are plain text a GM may have edited since, so there is no safe automatic unmerge.harvest_notes_on_completeis idempotent per card, so re-completing will not double-append.Commit 2 — the stalled lore run, and the double-run behind it
The fifth state, plus a defect the issue does not mention. These had to ship together.
The UI hid the re-run action for all three in-progress states, so a run whose worker died left the session showing "Extracting lore candidates…" with nothing to click. The obvious fix is to stop hiding it — except the endpoint behind that button guarded on
pendingonly.extractingandmatchingfell straight through, committingpendingover a live run and queueing a second extraction over the same transcript: duplicate proposals, doubled tokens. Nothing had hit it because the button was hidden — which is precisely the rule being relaxed. Relaxing the UI without closing the guard would have converted a latent double-run into a one-click one.Adds
sessions.lore_generation_progress_at(migrationa8b0c1d2e3f4), stamped on enqueue and at each phase transition.updated_atcould not serve: it carriesonupdate=func.now(), so any unrelated write to the session — a title edit, an attendance change — resets it, and it is not exposed inSessionResponseanyway.Named for progress, not start, deliberately: a long but healthy run keeps proving it is alive rather than ageing into "stuck" and inviting the very duplicate this prevents. A start-only stamp would force the threshold above the slowest legitimate run, which is the multi-hour window we are trying to remove. NULL reads as "unknown, leave alone", so rows predating the column never trigger a spurious retry offer — #398's watchdog still frees them.
Verification
Mutation-checked throughout:
pending-only guard restored: both concurrency tests fail.freshpinned true: the supersede test fails.The migration was applied over the full chain against a scratch database and downgraded back off, both clean.
Frontend tests were run in a container (there is no local node). That caught a real bug reading alone had missed: the
renderPagehelper gained asessionsPageparameter but still hardcoded an empty list internally, so the new session row never reached the component.1,439 backend tests pass (was 1,409). 449 frontend (was 437). Lint and format clean at CI's pinned ruff 0.4.4.
#416 after this
Five of five unowned states are addressed. The Workbench "stuck pending" item needed no new work —
GenerationHistory.jsxnever filters by status, sopendingandfailedrows already render with a badge and a working re-open button, and #398's watchdog now gives them a terminal state within the hour. Reasoning recorded on the issue; happy to reopen if you read it differently.Of the six states cross-referenced to other issues, three have shipped (#398, #399, and the audio half here); #413, #411 and the two bot/reconciliation issues remain open and are tracked there.
🤖 Generated with Claude Code