Keep task reminders transactional across interrupts

[brynary]

Summary

• Keep generated task reminders request-local until the assistant response commits.
• Drop uncommitted reminders when an LLM round is interrupted.
• Add regression coverage for interrupt-and-steer after reminder injection.

Root cause

An interrupted assistant turn was discarded, but its pre-round system reminder remained in durable history. The steering message then followed that system message, producing an invalid message sequence that OpenRouter rejected.

Test plan

• cargo nextest run -p fabro-agent
• cargo +nightly-2026-04-14 fmt --check --all
• cargo +nightly-2026-04-14 clippy -p fabro-agent --all-targets --no-deps -- -D warnings
• git diff --check

[Copilot]

Pull request overview

This PR fixes an interruption edge case in fabro-agent session handling by making task-reminder system messages “transactional”: reminders are staged per request and only written to durable history once the assistant turn successfully commits, preventing invalid message role sequences after an interrupt + steer.

Changes:

• Stage task reminders per-round (task_reminder_if_needed) and inject them into the outbound LLM request without persisting them immediately.
• Commit the reminder into Session.history only when the assistant response is recorded, so interrupted rounds don’t leave stray system messages behind.
• Add a regression test that reproduces “interrupt after reminder injection, then steer” and asserts request ordering and durable history correctness.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[brynary]

Ran a three-way simplification review (reuse, quality, efficiency) on this branch and pushed one cleanup commit (9c152cc):

• Single-owned wire conversion: staged the pending reminder as a Message and extracted Message::to_llm_message (mirroring to_session_message), so build_request and History::convert_to_messages can no longer drift on how a system turn becomes a wire message.
• Test fixture reuse: replaced the one-off BlockingAfterFirstOutputProvider with request capture + an EventsThenPending variant on the existing ScriptedStreamProvider, and added a shared make_session_with_provider_and_tools helper used by both task-reminder tests.
• Tighter assertions: both reminder tests now compare against task_reminder::TASK_REMINDER_TEXT exactly (instead of a "<system-reminder>" substring that also matches other prompts), with failure output that prints the actual messages. Renamed the regression test to interrupted_round_does_not_commit_task_reminder.

Net −16 lines. Verified: cargo nextest run -p fabro-agent (626 passed), nightly clippy -D warnings clean, fmt clean, workspace check clean.

Noted but intentionally skipped: the double effective_tools() call per round predates this PR and is negligible next to the LLM call; one open product question is whether a steer should suppress the reminder for that round, since the restaged reminder still lands after the user's steering message.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.