Skip to content

fix(chat): hold a restored queue instead of showing it as work in flight - #23971

Open
Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:fix/restart-queued-lifecycle
Open

Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:fix/restart-queued-lifecycle

Conversation

@Pearcekieser

@Pearcekieser Pearcekieser commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: After a restart, a chat whose turn was cut while messages were queued shows no work in flight, offers Resume, and never sends the queued messages without the person.

A turn is running and a message is queued behind it. The app restarts. The chat comes back with "This turn was interrupted when the app restarted. Review the partial output above, then resume to continue." and the queued message as a card. Nothing runs (running: false), yet:

  • the sidebar row spins "In progress", the board files the chat under Working and the command center marks it running;
  • the queue card's hourglass keeps animating;
  • Resume is hidden, and POST /continue answers 409 slot_queue_pending on every press;
  • the only way out is a new message, which runs first, and then the restored card sends itself behind it;
  • a SIGKILL in the few seconds after a queued message starts its turn brings that message back twice, as the interrupted turn's opener row and as a card.

Why it matters

Every chat that has a queued message when the app restarts (an update, a crash, a watchdog exit) is affected. The UI says work is in progress when nothing is, hides the one control the notice tells the person to use, and later sends words the person typed to a process that no longer exists, out of order. The double-restore case can run the same prompt twice.

Not a goal

What changed (motivation → approach → change)

Cause: a queue restored from disk has no owner. Nothing drains it on boot, by design (session.md: restored entries are handed back as cards, not dispatched). But every reader still asked queue_depth whether work was coming, and the drain picked restored entries up after any later turn.

Restored entries are now held (is_held_queue_entry). The drain passes them by, and only the person's Send now on the card starts one. Send now marks the entry released, so the deferred drain after a stopped turn or memory preparation still runs it.

"Is work coming?" readers now count queue_ready_depth: Continue's refusal, the completion sound, session status and the members-thread reset. The slot frame carries queue_held_depth and each held entry meta.held (types/index.chat.ts). The store carries held through wire.ts and queue.ts (including a card a first pane hydrate keeps), and selectContinuable (selectors.ts) looks past a held card. The sidebar, board, command center and card (website/src/components/QueueStack.tsx) stop showing a held card as working, and the card reads "Held" (new strings in every website/src/i18n/locales/*.json catalog). Held cards no longer count as user intervention for recovery gates. A Resume skips the written rows of held sends and of held sends the person discarded, so it continues the cut request. Both recovery paths apply it: the native-resume preamble and the cold-start replay (build_session_replay(skip_row=...) in src/kiro_crew/context.py), which drops the rows from the disk transcript and the live window alike. Discard records the entry id in the transcript's session metadata (discarded_queued_sends, newest 64) before it removes the entry, one discard per slot at a time, with a compare-and-swap through update_metadata_if; if the write fails the card stays held and the route answers 503. Agents see the split too: the session-control status and read payloads carry queue_held_depth, session_status and session_read_message in src/kiro_crew/mcp_dashboard_session_runs.py report held cards apart from queued ones (_queue_phrase, forwarded on the src/kiro_crew/mcp_dashboard.py facade), and the goal-conductor skill says held cards do not run on their own.

The turn marker's metadata merge now writes the post-dequeue queue atomically with the opener copy. The two land together or neither does: with no retained copy, when another queue writer committed first, or when the turn merged several crewmate sends (the copy holds only the last), the queue key is left to the paired row-and-queue save. Send now parked on memory preparation is answered as accepted and audited as dashboard_interrupt with outcome parked, and the client (website/src/hooks/useQueuedMessageActions.ts) releases that latch if a restart hands the card back held. The turn-completion chime (website/src/hooks/websocket/turnCompletion.ts) counts only ready entries too. meta.held overrides every content classifier, so a held prompt that happens to read like a system delivery is still an actionable card on every surface. A crewmate chat (ChatPane with crewmate) draws held entries as Held cards with Send now instead of sent bubbles, with no typing bubble for them. A held entry whose row is already written (meta.rowWritten) has no Edit, and Cancel discards it: the entry goes, the row stays, nothing returns to the composer.

Backwards compatibility

Compatible: new fields are additive (queue_held_depth, meta.held), and a client that ignores them behaves as before. A gateway without them reads as zero held.

Behaviour change: restored cards no longer send themselves after the next turn; they wait for Send now, an edit or a delete. This is the documented session.md contract, now enforced. Session status reports idle instead of queued for a slot whose only queued entries are held.

Removes nothing: no field, route or capability is removed. The restored-entry drain path is still reachable, through the card's Send now.

Tests

  • test/test_restart_held_queue.py: held vs ready counts and projection; restore holds every entry; drain skips held entries, keeps their order, runs a named or released card, never falls through to a held head after a purge, does not park held cards on memory preparation, is not reached by the sub-agent hold's system scan (a restored steer), and does not flush a deferred note into a cron turn behind a held head; Continue runs ahead of held cards, refuses a duplicate press, still refuses on a ready entry; held cards are not user intervention; dequeue and turn marker land in one write, the opener copy and the shortened queue land together or not at all (no copy retained, or a newer committed queue), and a Send now parked on memory preparation is answered as accepted; repeated restarts keep one notice and the same cards.
  • test/test_chat_mode_security_send.py: the four restored-entry security tests now reach restored text through Send now, the only path that runs it.
  • website/src/test/heldQueueAfterRestart.test.tsx: wire and hydrate carry held (including a crewmate row kept in place); Resume is offered past a held card and withheld behind a ready one; lanes and command center treat a held queue as idle; the card shows "Held" with no hourglass.
  • test/test_chat_slot_facade_contract.py pins the new queue_held_depth key.
  • ChatPane.queueActions.test.tsx, useQueuedMessageActions.test.tsx, crewmateBubbles.test.ts: a crewmate's held entry is a Held card with Cancel and no typing bubble; a parked Send now stays latched until a restart re-hydrates the card held.
  • TestResumeNeverContinuesAHeldSend: the interrupted-turn preamble skips the row of a held send and of a discarded one; the cold-start replay skips both rows whether they come from disk or the live window. The cancel test in test/test_dashboard_chat_handlers_coverage.py checks Discard records the id in session metadata.
  • TestAgentsAreToldHeldIsNotQueued: the agent-facing session tools report held cards apart, and an older gateway's payload still reads as queued.

Proof: not provable -- other: the new tests import the held-queue API, so prove.py's revert errors at collection. Run by hand on main c24c7bc with that API shimmed, 14 of the 20 backend tests fail and the other 6 pass, pinning behaviour main already has.

Manual verification

Reproduced on a disposable gateway (fake ACP backend, loopback, throwaway home), never the live install, with a driver script that restarts at each boundary:

Scenario main c24c7bc this PR
SIGTERM mid-turn, 1 queued running:false, queue_depth:1, sidebar "In progress", hourglass, no Resume, /continue 409 sidebar "Turn interrupted · Resume", static Held card, Resume on notice and composer
Same, 2 queued both restored, stuck both held, in order
Resume, then SIGTERM again 409 every press 200, recovery runs, card stays held, Resume offered again after the second restart
New send after restart new message runs, then the restored card runs itself new message runs, card stays held
SIGKILL just after the queued entry starts its turn entry is both the interrupted opener and a card one opener row, empty queue
Browser: Resume, then Send now on the card n/a recovery ran, then the card ran once, queue empty

Not fixed, separate causes: a tab created under 5 s before SIGKILL is not reopened (its transcript, marker and queue are on disk); a force Stop's queue clear rides the 5 s flush.

Screenshots / video

Before (main): "In progress" spinner, animated hourglass, no Resume.

before

After: "Turn interrupted · Resume", Held card, Resume on the notice and the composer.

after

Related Issues

Fixes #23970
Closes #16780

Supersedes #20029. Its meta.restored projection and Resume gate are covered here, and its open review blocker (restored cards drain on their own after the recovery turn) is answered by keeping them held.

Sibling sweep

Swept: git grep -nE "queue_depth|len\(slot\._queue\)|if slot\._queue|not slot\._queue" -- src/ and git grep -n "queue_depth" -- website/src
Sibling: src/kiro_crew/dashboard/chat_api/continue_turn.py -- changed here
Sibling: src/kiro_crew/dashboard/chat_utils_completion.py -- changed here
Sibling: src/kiro_crew/dashboard/handlers/members_thread_turns.py -- changed here
Sibling: src/kiro_crew/dashboard/session_control_parts/session_status.py -- changed here
Sibling: src/kiro_crew/mcp_dashboard_session_runs.py -- changed here (agent-facing queued count)
Sibling: src/kiro_crew/builtin_skills/goal-conductor/SKILL.md -- changed here
Sibling: website/src/pages/chat/sessionLane.ts -- changed here
Sibling: website/src/pages/chat/command-center/model.ts -- changed here (node state); the options-suppression read keeps raw queue_depth, since a held card may still be the answer
Sibling: website/src/lib/closeBusyGate.ts -- not affected -- closing still discards unsent words, so a held card keeps the confirm
Sibling: src/kiro_crew/dashboard/chat_turn/queue_cycle.py -- not affected -- its queue reads feed _start_next_queued_turn, which now skips held entries

Pattern harvest

Rule candidate: review-prompt
Pattern: a predicate that asks "will this run on its own?" reads raw queue/ownership state that a restart can leave without a live owner

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (docs/system-specs/modules/session.md)
  • No secrets, credentials, or internal references in the diff

@Pearcekieser
Pearcekieser requested review from a team and KiroCrewCaptain as code owners October 25, 2026 07:45
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 25, 2026
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of b245065d88217dc493579a68135dd29579aab58f via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. The hold restores the rule session.md already pins: restored cards are "handed back as queue CARDS, not dispatched" and "must never be auto-resent". One duplicated line of skill prose is left.

Notes

  • Item 9 — duplicate: the new goal-conductor sentence "Messages it reports as held after a restart do not run unless the person sends them" repeats what _queue_phrase in src/kiro_crew/mcp_dashboard_session_runs.py now puts in every session_read_message and session_status reply: "held after a restart (run only if the person sends them)". The pipeline-conductor skill (line 1554) has the same queue_depth caveat and got no edit, which shows the tool's own text already does this job. Subtraction: delete the added sentence from src/kiro_crew/builtin_skills/goal-conductor/SKILL.md and let the tool output carry it.

What this change ships

Inventory (10 items) — 9 justified

Intent: after a restart, a chat with messages queued behind a cut turn should look idle, offer Resume, and never send those messages unless the person does. FIX.

The change has 11 observable differences; this list keeps the 10 a person is most likely to notice and folds the recovery-gate change into item 1.

  1. Restored queue cards no longer send themselves after a later turn; they wait for Send now, an edit or a delete. Recovery gates stop counting them as the user stepping in. — justified
  2. Resume is offered, and Continue no longer answers 409 slot_queue_pending, when the only queued cards are held. — justified
  3. The sidebar, board, command center, completion chime and session status treat a held-only queue as idle. — justified
  4. A held card shows "Held" with a pause icon instead of the animated hourglass, in all 12 locales. — justified
  5. Send now releases the hold. A Send now parked on memory preparation is answered as accepted and audited as parked. — justified
  6. Cancel on a held card whose row is already written now discards it and keeps the row. Before, it was refused with 409. The discard is recorded in discarded_queued_sends. — justified
  7. Resume and the cold-start replay skip the rows of held and discarded sends. — justified
  8. The dequeue and the turn marker land in one atomic write, so a SIGKILL can no longer bring a message back twice. — justified
  9. Agents see queue_held_depth and "held" wording in status and read output, plus a new goal-conductor skill sentence. — duplicate of _queue_phrase output, skill line only
  10. In a crewmate chat, held entries draw as Held cards with Send now and no typing bubble, and Edit is hidden once the row is written. — justified

[FIRST-PRINCIPLES-REVIEWED] b245065

Description read

[DESCRIPTION-READ] 575fa09478de10c3cb455d3bbc36f150d9397c21494b9d65027d58d12ab71c23

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5, fork) — ✅ PASS (a BLOCK with no Trigger and no Outcome does not block)

UX-level review of b245065d88217dc493579a68135dd29579aab58f via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

cannot evaluate: missing the crewmate chat's new Held cards and the rowWritten held card with no Edit. The two screenshots only show the main chat pane before and after.

Evidence gaps

  • Crewmate chat Held card (ChatPane.tsx, crewmate && crewmateHeld.length > 0 && <QueueStack … onInterrupt={onInterruptQueued} …>): this adds a new surface. A crewmate chat now shows queue cards with Send now where it used to show sent bubbles. Neither attachment shows it. Close it with a screenshot of a crewmate chat after a restart, with a held entry.
  • Held card whose row is already written (m.meta?.rowWritten !== true hides Edit, and Cancel discards without returning text to the composer): no screenshot shows this state. Close it with a screenshot of that card and its reduced controls.
  • Board and command center (sessionLane.ts, command-center/model.ts): these now treat a held queue as idle, not Working/running. No attachment shows either surface. Close it with a screenshot of the board and the command center with a held chat.

Notes

  • held_hint lives only in a title tooltip and says "Send it now, edit it, or delete it". On a rowWritten card there is no Edit, and the X button is labelled "Cancel queued message". Use a hint that matches the controls the card actually shows, and the same verb ("Cancel"/"Discard") as the button.
  • In attachment-02.png, the "HELD" badge in uppercase amber looks the same as the queued text next to it. Drop the badge's opacity or use a muted token so the badge reads as a status, not part of the message.

[UX-REVIEWED] b245065

Description read

[DESCRIPTION-READ] cf647dcf6fecaf9b6d4ce07bbf735aff2aea7de2d8cb6fbab2821278ca2f668b

That sha256 names everything this verdict read: the PR description, plus the 5 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of b245065d88217dc493579a68135dd29579aab58f via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the fix is at the root (restored entries are held, and the readers that ask whether work is coming count only ready entries), and screenshots of this revision are attached.

Notes

  • The atomic dequeue in the turn marker and the discard list stored in session metadata (discarded_queued_sends) fix separate problems from hiding held cards. Each has its own failure handling (the stale-witness refusal, the compare-and-swap with a 503 answer), so they could ship and be reverted as separate PRs.
  • _HELD_DISCARD_LOCKS is a module-level dict keyed by slot name that is never pruned. A discard lock the slot owns would end with the slot, and that slot-identity check would go away.

[DESIGN-REVIEWED] b245065

Description read

[DESCRIPTION-READ] d7b2a8d9b79ee77d9cc120cb2b6b574d69b656b0af649b203f79df593b420d29

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings (all downgraded on adjudication)

Reviewed b245065d88217dc493579a68135dd29579aab58f via the fork AI-review pipeline; updated in place on each push.

Opus 5.5 adjudication downgraded every blocking finding. Downgraded: the remedy each one needs is disproportionate to the harm it prevents. Adjudication downgraded all 2 blocking finding(s): the remedy each one requires is disproportionate to the harm it prevents.

Review details

BLOCKING -- src/kiro_crew/dashboard/chat_api/turn_stop.py:953 -- Preserve the selected card when parking Send now
release_held_queue_entry(slot._queue, queue_id)
Trigger: While memory preparation is pending, Resume inserts a recovery entry ahead of held cards; the person then presses Send now on a held card. _start_next_queued_turn parks before promoting the selection, and _drain_parked_queues later dispatches without required_queue_id.
Outcome: Resume runs first and the selected message runs afterward, despite Send now accepting that specific card.
Anchor: defect
Fix: Retain the selected queue ID and explicit dispatch override when parking, then forward both to the deferred drain.

BLOCKING -- src/kiro_crew/dashboard/chat_turn/prompt_assembly.py:104 -- Clear discarded identity before sending a resurrected card
skipped = {item.get("id") for item in slot._queue if is_held_queue_entry(item)} | discarded
Trigger: Discard a row-written held card, then restart before queue removal is flushed. Its persisted queue entry restores; Send now reuses the tagged user row without clearing discarded_queued_sends. Interrupt that turn and press Resume: both recovery paths apply this filter.
Outcome: Recovery excludes the request that actually ran, selecting an older request instead of continuing the interrupted send.
Anchor: defect
Fix: Durably remove the selected ID from discarded_queued_sends before accepting Send now, refusing dispatch if that update fails.

[BLOCK-MERGE-DOWNGRADED] b245065
[GPT-REVIEWED] b245065

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F1 and F2 are both downgraded: neither is worth blocking the merge.

F1 (LOW).

  • Conditions, all confirmed:
    • Resume puts its recovery entry at the head of the queue (continue_turn.py:288).
    • While memory preparation is still running, the drain parks and returns (queue_drain.py:461, which the patch narrows at its line 728).
    • The parked drain later runs without naming a card (queue_drain.py:320), so the head entry runs first.
  • What happens: both requests still run, in the order the person clicked (Resume, then Send now). The released card is picked up at the next turn end (patch queue_drain.py:766-770). Nothing is lost and it is visible. It can only happen in the startup window while memory is being prepared.
  • Real fix: the parked-slot map (queue_drain.py:258-272) would need to remember the chosen card and the Send now override, and pass both to the deferred drain. That is new state kept for this one path only.

F2 (LOW).

  • Conditions, all confirmed:

    • The discard record is written first (patch prompt_queue.py:190), and removing the entry from the saved queue waits for the next periodic save (chat_slot.py:796-805). A restart inside that window brings the card back.
    • Send now only lifts the hold and leaves the discard record in place (patch turn_stop.py:415).
    • The recovery filter then excludes that id (patch prompt_assembly.py:613).
  • What happens: the row stays in the transcript, and Resume continues an older request where the person can see it. There is no automatic correction; the person can retype the request. Reaching it takes a restart within seconds of a discard, then sending the card they discarded, then an interrupt, then Resume.

  • Real fix: a blocking metadata write plus a refusal branch on every Send now, which costs more than this rare, visible case.

    [ADJUDICATION] b245065 total=2 uphold=0 downgrade=2
    DOWNGRADE F1 src/kiro_crew/dashboard/chat_api/turn_stop.py:953 reason=disproportionate-remedy
    DOWNGRADE F2 src/kiro_crew/dashboard/chat_turn/prompt_assembly.py:104 reason=disproportionate-remedy
    [GPT-ADJUDICATED] b245065

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed b245065d88217dc493579a68135dd29579aab58f via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] b245065

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 25, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: sub-agent hold system scan reached held entries

  • span=d0cfad1260e9 issue (blocking): the sub-agent dequeue still dispatches held restored steers

fixed in 9fed98063: _dequeue_next_system_message now skips held entries (releases_past_subagent_hold(item) and not is_held_queue_entry(item)), so a restored steer runs only on its own Send now.
Regression: test_a_held_steer_is_not_released_by_the_sub_agent_hold fails on b90f6dc and passes on 9fed98063.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: deferred-note flush read the raw queue head

  • span=156d59329fce issue (blocking): a deferred note can flush into a cron or sub-agent turn behind a held card

fixed in 9fed98063: the flush guard reads _next_dispatchable_entry (the named card, else the first entry not held), so a cron entry behind a held head withholds the note as before.
Regression: test_a_note_is_not_flushed_into_a_cron_turn_behind_a_held_card fails on b90f6dc and passes on 9fed98063; the source-shape pin in test_gateway_appkit_endpoints_note.py names the new guard.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • praise: one owner for restored entries, one count for "is work coming?"

Acknowledged. The round-1 fixes keep that shape: the sub-agent scan and the note flush now read the same held predicate (is_held_queue_entry) instead of the raw queue head.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • note: the Held card and the "Turn interrupted · Resume" row are only in the after screenshot

Acknowledged. Both were driven in a real browser against a disposable gateway: Resume ran the cut turn with the card still Held, then Send now ran the card once and emptied the queue. No UI changed in round 1.

@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch from b90f6dc to f674220 Compare October 25, 2026 08:45
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Pushed f674220. It carries the two round-1 fixes (dispositions above) and one locale fix for the reds on b90f6dc:

  • Frontend Tests (1), (2), (3) and Frontend Coverage Merge failed on the register rules in src/i18n/style (bn, fr, hi, ru). The new held_hint used the formal address. It now uses তুমি / tu / तुम / ты forms; all 12 locale style tests and i18n:check pass locally.
  • Backend Tests (3.12, 4) failed on test_governance_policy_viewer.py::TestNoPolicyDefault::test_enterprise_unnamed_scope_reports_the_baseline (PlatformCompositionError). The same test fails locally on current main 813e25c, whose own CI run is red, so it is not from this diff.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: turn-marker merge carries the post-dequeue queue

  • span=c146d09d67c5 issue (blocking): a racing pre-dequeue save preserves the consumed prompt

fixed in f9c1219: the opener copy and the shortened queue now land together or not at all. On a stale witness the merge drops queued_prompts AND writes turn_in_flight_prompt: None, so the earlier writer's paired save holds the entry exactly once (as a card if it snapshotted before the dequeue, as its row if after).
Regression: test_a_stale_queue_snapshot_drops_the_opener_copy_too (one copy after restore) and the updated test_a_newer_committed_queue_is_not_written_over; both fail on f674220.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: Send now release before a parked dispatch

  • span=5a68e05795fe issue (blocking): Send now releases a held card despite returning a refusal

fixed in f9c1219: the idle /interrupt path now answers 200 {"ok": true, "outcome": "parked"} when the drain parked on memory preparation and the card is still queued. The client treats any success as accepted and keeps its pending latch until the card retires (useQueuedMessageActions.onInterrupt), so the controls do not come back for a card that is about to run.
Regression: test_a_parked_send_now_is_answered_as_accepted fails on f674220 (409).

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: turn-marker merge carries the post-dequeue queue

  • span=096d6517711e issue (blocking): a prompt with no retained opener copy is dropped by the shortened queue

fixed in f9c1219: queued_prompts joins the marker merge only when slot._turn_in_flight_prompt is set. With no copy (an oversize prompt) the queue key stays with the paired row-and-queue save, so after a death the entry comes back as a held card.
Regression: test_without_an_opener_copy_the_queue_key_is_left_alone fails on f674220.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • suggestion: the session-status queue_held_depth key has no code reader

fixed in f9c1219: removed from the session-status row. Its status field still reports idle rather than queued for a slot whose only queued entries are held, which is the reader-facing part. The slot frame keeps queue_held_depth, read by the sidebar, board and command center.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • praise: held at the drain, one count for "will this drain?"

Acknowledged. Round 2 keeps that shape; its changes are in the turn-marker write (opener copy and shortened queue land together) and the parked Send now answer.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • note: no first-time reader has seen the Held card or the sidebar row

Acknowledged. The branch lives on a fork, so the blind read is not available to this PR; the screenshots and the browser run (Resume, then Send now) are the evidence. No UI changed in this round.

@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch from f674220 to f9c1219 Compare October 25, 2026 09:44
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Oct 25, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • thought (non-blocking): Item 6, the discard path and its persisted id list, is heavier than the goal

Kept, with the reason. The 409 queue_item_already_sent on a held, row-written entry was GPT's round-4 finding (span 42cfccb1d7fb): after a restart nothing sends that entry, so with the 409 the person cannot remove the card and Send now is the only exit, which resends a message they may no longer want. Discard is the smallest way out that does not write the row twice.

The id list exists because Discard keeps the row: GPT's round-6 finding (span 9dd41fe0962a) showed a kept row becomes what a later Resume continues, including after another restart reloads it from disk, where the live queue no longer names it. The cost is one metadata read per Resume (_discarded_sends, off the event loop), only on the cold-replay and native-resume recovery paths, and a list capped at 64 ids.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): Backend Tests (3.12, 2) on 951347c failed only test/test_native_skill_projection_rewrites.py::test_two_concurrent_releases_each_prune_their_own_view ("the first release's view lingered"). That is the open flake #23965, present since #23913; this PR does not touch skill projection, and the test passed 5 of 5 local runs. A rerun of that job once the round completes should clear it.

@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch from 951347c to 2ddc2a9 Compare October 25, 2026 21:57
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

chore (non-blocking): same-tree retrigger, no code change. 2ddc2a9 has the tree of 951347c, re-dated only, to rerun GPT 6.1 Review (it ended "review incomplete" with no findings on 951347c) and Backend Tests shard 2 (flake #23965, which the Coverage Gate follows).

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 25, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking): Persist the discard before removing the held entry span=74de941341ef

Upheld, fixed in the next push. api_chat_slot_queue_cancel (chat_api/prompt_queue.py) now commits record_discarded_send before queue_remove_by_id. If the write raises OSError or does not commit, the route answers 503 queue_discard_not_recorded and the card stays queued and held, so Discard can be retried. The write yields, so the route then re-reads the entry: if a Send now released or started it in that window, the id is taken back out (forget_discarded_send) and the route answers 409 queue_item_unavailable. From that check to the removal there is no await, which serializes it with dispatch on the event loop.

Tests in test/test_dashboard_chat_handlers_coverage.py: a failing metadata write returns 503 and leaves the card and broadcasts untouched; a release during the write returns 409 and leaves no recorded id.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking, security): Concurrent discards lose recorded IDs span=03a412867a51

Upheld, fixed in the next push. record_discarded_send (responsive_chat.py) now writes through update_metadata_if, the locked seam, as a compare-and-swap: the guard checks under the transcript lock that the list is still the one the caller read, and on a mismatch the caller re-reads and retries (8 tries). Two discards that read the same list can no longer both commit; the second retries on top of the first. require_write=True makes a skipped write report failure, which the cancel route turns into its 503.

Test: test_concurrent_discards_keep_every_id commits a second discard between the first one's read and write; both ids survive.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking, security): Audit accepted parked interrupts; and Preserve the selected card's priority while parking span=5a68e05795fe

Audit (turn_stop.py:972): upheld, fixed in the next push. The parked branch of the interrupt route now emits dashboard_interrupt with outcome="parked" and the same {slot, queue_id} metadata as the started branch, before it returns. test_a_parked_send_now_is_answered_as_accepted asserts the event.

Priority (turn_stop.py:953): not changed; the adjudication downgraded it (disproportionate-remedy). The sequence is Resume, then Send now, both while memory preparation is pending. The deferred drain runs them in the order the person pressed them: the recovery Resume put at the queue head first, then the released card. Nothing is lost or auto-sent, and the card does run. Moving the card ahead of a Resume the person also asked for would add a new ordering rule between parking and promotion.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 25, 2026
@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch from 2ddc2a9 to 75caafb Compare October 25, 2026 22:31
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 25, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking, security): Shortening the queue can lose an earlier merged crewmate send span=c146d09d67c5

Upheld, fixed in the next push. _begin_local_turn_marker (chat_turn/turn_marker.py) now checks _opens_a_merged_send_run: more than one user row tagged with a queue entry id after the newest conversational assistant row means the turn consumed several sends, and the opener copy holds only the last. In that case neither queued_prompts nor the opener copy is written; both stay with the paired row-and-queue save, the same rule the stale-snapshot case already follows.

Test: TestOpenerCopyAndQueueLandTogether::test_a_merged_send_run_keeps_every_send_on_disk dequeues two sends, writes the marker, dies before the next save, and restores. Each send comes back exactly once. On 75caafb the first send is lost.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking): Concurrent discards erase a successful discard's durable record span=74de941341ef

Upheld, fixed in the next push. This span's second hit, so the fix closes the race class rather than this ordering. The held, row-written discard now runs in _discard_held_written_entry (chat_api/prompt_queue.py) under a per-slot asyncio.Lock, keyed by slot name and identity. Under the lock it rechecks the entry before recording (absent: 404, no longer held: 409, nothing recorded), records, rechecks, and removes the entry, all inside the lock. A second DELETE of the same card waits, then finds it gone and answers 404 without touching the record. The only remaining forget_discarded_send is for a card a Send now released during this request's own write, so the record it takes back is always its own.

Test: test_two_tabs_discarding_one_held_card_keep_its_record holds the first record write open while a second DELETE arrives; the statuses are 200 and 404 and the id stays recorded. On 75caafb the loser answers 409 after taking the record back.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • issue (blocking): First pane hydration preserves a queue card without its held state span=1968d4b62770

Upheld, fixed in the next push. hydrateQueuedBubbles (website/src/store/chat/queue.ts) now applies the server's held and rowWritten to a matching retained card in both base and live, so a card a queue_push drew before the restart takes the held state when the pane first hydrates.

Test in website/src/test/heldQueueAfterRestart.test.tsx: a live card kept by a first pane hydrate (preserveLiveQueue) comes out held and row-written, and selectContinuable offers Resume. On 75caafb the card keeps neither flag.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 25, 2026
@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch 2 times, most recently from 50df9c1 to b245065 Compare October 26, 2026 01:01
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • thought (non-blocking): Preserve the selected card when parking Send now span=5a68e05795fe

Not changed; downgraded on adjudication again. Same answer as on 2ddc2a9: Resume, then Send now, both while memory preparation is pending, run in the order the person pressed them. Nothing is lost or auto-sent, and the card runs right after the recovery turn. The parked command is now audited (dashboard_interrupt, outcome parked).

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • thought (non-blocking): Clear discarded identity before sending a resurrected card span=9dd41fe0962a

Not changed in this PR; downgraded on adjudication. The trigger takes three faults in a row: a restart between a Discard's committed record and the next queue save, a Send now on the card that restart brought back, and a second interruption of that turn followed by Resume. The cost is that this one Resume continues the earlier request instead of the resent one; nothing runs twice and nothing is lost, since the row stays in the transcript. If it is worth closing, the smaller remedy is at restore rather than at Send now: a restored entry whose id is in discarded_queued_sends is one the person already discarded, so the restore should drop it instead of handing it back as a card.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 26, 2026
A queue restored after a restart has no owner: nothing drains it on boot, by
design. Every reader still treated a non-empty queue as work about to run. The
sidebar spun "In progress", the board filed the chat under Working, the queue
card's hourglass kept flipping, and Continue refused with slot_queue_pending,
so the restart notice's "resume to continue" had no working control. A later
send then ran first and the restored card ran on its own behind it.

Restored entries are now held (is_held_queue_entry). The drain passes them by
and starts one only when the person names it (Send now). Continue, the
completion sound and session status count queue_ready_depth. The slot frame
carries queue_held_depth and each held entry meta.held, so the sidebar, board,
command center and queue card stop showing a held card as working.

The turn marker's metadata merge also writes the post-dequeue queue in the same
atomic write as the opener copy. Written apart, a SIGKILL between them restored
the consumed entry twice: as the interrupted turn's row and as a queue card.
@Pearcekieser
Pearcekieser force-pushed the fix/restart-queued-lifecycle branch from b245065 to a14783c Compare October 26, 2026 02:31
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

chore (non-blocking): rebased on main, no behaviour change. a14783c is b245065 on main 455e8ec, a clean rebase with no conflicts. It picks up #24142, the fix for flake #23965 that failed Backend Tests shard 2 twice here. Backend shard 5 on b245065 failed test_memory_files_substitution.py::test_pruning_removes_from_the_seam_and_leaves_local_files_alone ("assert 2 == 1"). That is open issue #24190: a hardcoded 2026-09-25 date is now older than keep_days=30. It fails on every branch, and this PR does not touch memory files.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 26, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

1 participant