fix: todo-reminder 4-commit series — error-pause, in-progress wording, todowrite guard, orphan detection - #6
Open
arunasp wants to merge 4 commits into
Open
Conversation
…t just MessageAbortedError The plugin's message.updated and session.error handlers only paused reminders on isMessageAbortedError (escape-key abort). Any other assistant-message error falls through unrecognized, so the reminder still fires its default "do not ask for permission" text on the next idle even though the prior turn ended abnormally. One concrete, source-verified way that gap gets hit: a tool-permission deny-rule match returns PermissionV1.DeniedError, but session/tools.ts's ask() wrapper uses Effect.orDie, turning it into a defect that processor.ts's catchCauseIf/halt() squashes into assistantMessage.error - the same status.set(idle) transition as a normal completion (processor.ts:596-624, status.ts:42-44). This is a real code path, not this specific case, so it is not claimed as the confirmed cause of any one incident. NOTE (scope): this does NOT fix the separate, likely more common case where the reminder interrupts a still-incomplete task with NO error at all - a normal session.idle firing between two steps of an unfinished plan. That is the plugin's fundamental idle-triggers-reminder design and is unaddressed by this patch. Generalizes both handlers to hasAssistantMessageError (any error, not just MessageAbortedError), reusing the existing pause-and-toast path. isMessageAbortedError is kept for its other use (checking the plugin's own injected-prompt response for abort). Toast wording generalized to match. No behavior change for the existing abort case; all 28 existing tests pass unmodified. Co-Authored-By: Claude <noreply@anthropic.com>
…ing task"
Evidenced directly (not inferred from source-code speculation, unlike the
prior commit's permission-denial angle): messageFormat's default wording
is 'Continue working on the next pending task now', but triggerStatuses
defaults to [pending, in_progress, open] - so this same wording fires
even when the item still open is already status=in_progress, i.e. the
model was mid-task, not idle-and-stuck. Telling a model to move to 'the
next pending task' while it has a task in flight reads as an instruction
to abandon or skip it, which is a plausible account of a reminder
'interrupting a half-done task' with no error anywhere in the picture.
Adds inProgressMessageFormat (new config field, sensible default) used
instead of messageFormat whenever one of the still-open todos has
status in_progress. Names the specific task via a new {current_task}
interpolation (Todo.content) and tells the model to finish THAT one,
not move on. createReminderMessagePattern's placeholder handling and
the prompt guard are extended to recognize both templates so a
paused session still blocks reminder-shaped injections regardless of
which one produced them.
No behavior change when no todo is in_progress (messageFormat path is
untouched). Two new tests cover both branches; all 30 tests pass.
Co-Authored-By: Claude <noreply@anthropic.com>
…e staleness reconciliation Investigated both reported issues against opencode's actual source (anomalyco/opencode, cloned separately - not guessed): 1. TodoWrite overwrites the whole list, losing unfinished tasks - CONFIRMED. SessionTodo.Service.update (both packages/core/src and packages/opencode/src copies) does an unconditional delete-then-insert of whatever `todos` array it receives - no diff against the prior list, no merge. Todo has no stable id either (packages/schema/src/session-todo.ts: content/status/priority only - the SDK's generated Todo.id field is stale/unused, the live schema has never had one). So anything the model forgets to re-include in a TodoWrite call is silently gone, and there is no "insert a subtask" primitive distinct from "resend the whole list". Fix: new tool.execute.before hook (guardTodoWrite), gated by new config field preserveUnfinishedTodos (default true). Before a todowrite call reaches opencode, fetches the current list, and appends back any todo the new call omitted whose PRIOR status was pending/in_progress (matched by exact content string - the only identity available, so a reworded-but-same task will look like a new one and not merge; this is a real limitation of the underlying data model, not something plugin-side code can fully paper over). Does not enable the model to intentionally send partial lists - its tool description is unchanged - it only stops accidental loss-by-omission. 2. Stale todos when the LLM finishes work without updating status - PARTIALLY addressable, NOT fixable. Confirmed by source search: nothing in opencode connects any other tool call (edit, bash, etc.) to todo state; todowrite is entirely voluntary. No deterministic fix is possible without opencode itself changing that contract. Considered adding a standing instruction via opencode's experimental.chat.system.transform hook (applies to every turn, not just idle-triggered reminders) - opted against it: heavier (new hook, touches every request) for a problem that is not deterministically fixable either way, so the minimal version is preferable. Went with just one more line in the existing periodic reminder templates instead: both messageFormat and inProgressMessageFormat now tell the model to keep todo statuses current as it goes, not only when reminded, in addition to the existing "mark already-finished items complete first" line. Still a nudge at reminder time only, not a guarantee. 5 new tests (guardTodoWrite: backfill, no-op when nothing dropped, completed/cancelled omissions pass through, non-todowrite calls ignored, disabled via config). All 35 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Arunas's own framing of the real recurring problem: session IDs change
(new session, resumed session, compaction) and opencode's todo table has
no cross-session linkage of its own - confirmed against source, both
packages/core/src/session/todo.ts and packages/opencode/src/session/
todo.ts key every todo row by session_id alone, and SessionTodo.Info
(packages/schema/src/session-todo.ts) carries no reference back to any
other session. The session table itself does have project_id/parent_id,
but nothing in the todo write/read path uses them. Once a session ends,
its incomplete todos just sit in the DB - nothing ever revisits them,
and the current session's own reminder logic (pending.length === 0)
correctly has zero awareness that they exist, because they don't belong
to it.
Also directly explains a real, evidenced symptom from this
investigation: a session showing 0 pending todos of its own (confirmed
via direct SQL) is not evidence that "everything's actually done" -
there can be a large amount of abandoned, never-revisited work sitting
under other session IDs in the same project, invisible to any
per-session check.
First pass used a dedicated toast + its own scan trigger on the
zero-pending branch. Arunas asked for something simpler: fold it into
the existing periodic reminder text instead of adding a new
notification path. Redesigned accordingly - getOrphanedTodoTable scans
other sessions in the project directory (client.session.list, confirmed
real and directory-scoped in the installed SDK) once per session
lifetime, caches a plain "sessionID - N open" table string (not a
boolean flag), and it's interpolated via a new {orphan_table}
placeholder directly into messageFormat/inProgressMessageFormat - it
only ever rides along on a reminder this session was already going to
send, never sent as a message of its own. Consequence: a session with
zero pending todos of its own no longer triggers any orphan check at
all (nothing to attach it to) - orphan visibility is now tied to normal
reminder cadence rather than guaranteed on every zero-pending idle. No
toast, no new config for toast styling; same two config fields as
before (warnOrphanedTodos default false, orphanScanLimit default 20).
createReminderMessagePattern (used by the prompt-guard that blocks
reminder-shaped injections on a paused session) extended to treat
{orphan_table} as a wildcard, same treatment as {current_task}, since
its substituted content varies per scan.
4 tests, rewritten for the new mechanism (table appended when found;
session.list never called when disabled; nothing appended when no
orphans exist; session.list called exactly once across repeated
reminders in the same session, reusing the cached table). All 39 tests
pass.
Co-Authored-By: Claude <noreply@anthropic.com>
|
@PhilippPolterauer Any chance to review this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
4-commit fix series for the opencode-todo-reminder plugin:
All 39 existing tests pass. Built and tested locally.