Skip to content

Fix cache-busting reminder injections#419

Open
alvinunreal wants to merge 6 commits intomasterfrom
caching
Open

Fix cache-busting reminder injections#419
alvinunreal wants to merge 6 commits intomasterfrom
caching

Conversation

@alvinunreal
Copy link
Copy Markdown
Owner

@alvinunreal alvinunreal commented Apr 27, 2026

Summary

  • Closes [Bug]: Multiple hooks inject into system messages in cache-busting ways #415.
  • Keeps the orchestrator phase reminder active on every orchestrator turn, but appends it at the final latest user-message tail instead of prepending it or putting it in system.
  • Stops dynamic todo hygiene, post-file-tool, and resumable-session state from mutating the cached system prompt.
  • Preserves runtime nudges by appending them at the tail: file/todo reminders go into tool output as <internal_reminder>, while resumable session context is appended to the latest orchestrator user message before the phase reminder.

What changed

  • phase-reminder now appends <internal_reminder>...</internal_reminder> after the latest orchestrator user text. It runs after resumable-session injection, skips non-orchestrator and internal notification turns, and avoids duplicate reminders.
  • post-file-tool-nudge appends the reminder to Read/Write tool output instead of setting pending state for system injection. This nudge is intentionally retained because post-read/write drift is still a useful trigger.
  • todo-continuation injects hygiene/final-active reminders into the triggering tool output.
  • task-session-manager injects <resumable_sessions> into request-local message context instead of experimental.chat.system.transform.
  • Removed the dynamic system-transform hooks/no-op compatibility shims, so the plugin no longer wires unused cache-sensitive reminder paths.

Why this fixes the issue

OpenCode/provider prompt caching depends on stable cached prefix content. Before this change, runtime state such as file-tool activity, todo state, resumable sessions, and per-request reminders could alter system or prepend content ahead of the user's turn. This PR keeps dynamic reminders out of system and places request-local guidance at the tail/tool-output context, so reminders remain visible without invalidating the stable system prompt.

Verification

  • bun test src/hooks/phase-reminder/index.test.ts src/hooks/post-file-tool-nudge/index.test.ts src/hooks/todo-continuation/index.test.ts src/hooks/todo-continuation/todo-hygiene.test.ts src/hooks/task-session-manager/index.test.ts
  • bunx biome check src/index.ts src/hooks/phase-reminder/index.ts src/hooks/phase-reminder/index.test.ts
  • bun run typecheck
  • bun run build
  • Manual relaunch/log verification showed system hashes staying stable across plain messages, Read-tool nudges, and todo hygiene activity.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR refactors all dynamic reminder injection away from the cached system prompt (experimental.chat.system.transform) into either tool output (<internal_reminder> blocks) or the tail of the latest user-message — preserving cache stability while keeping runtime guidance visible to the model.

  • P1 — shouldInject bypass in todo-hygiene.ts: When todowrite is called and the todo state is final_active, the reminder is appended to the tool output without consulting shouldInject. In production, shouldInject filters to orchestrator sessions only, so sub-agent sessions (explorer, oracle, etc.) with exactly one in-progress todo will receive TODO_FINAL_ACTIVE_REMINDER in their tool output contrary to the design intent.
  • P2 — stale log in todo-hygiene.ts: pending.delete(sessionID) is called before the reasons log on line 195, so reasons is always logged as [].

Confidence Score: 4/5

Safe to merge with the shouldInject bypass fixed first — the rest of the refactor is well-structured and well-tested.

One P1 finding: the final_active todowrite injection path skips the shouldInject guard, meaning sub-agent sessions receive the reminder when they should not. All other changed paths look correct and have updated test coverage. P1 ceiling = 4.

src/hooks/todo-continuation/todo-hygiene.ts — the RESET-path injection block (lines 145–153) needs the shouldInject guard before calling appendReminder.

Important Files Changed

Filename Overview
src/hooks/todo-continuation/todo-hygiene.ts Moved reminder injection from handleChatSystemTransform to handleToolExecuteAfter; introduces injected dedup set, but shouldInject is not checked in the direct todowrite → final_active injection path (P1 bug), and the log on line 195 reads a deleted map entry (P2).
src/hooks/phase-reminder/index.ts Cleanly flips from prepend to append, changes tag to <internal_reminder>, adds duplicate-detection guard; logic is straightforward and well-tested.
src/hooks/post-file-tool-nudge/index.ts Strips out the system-transform and event paths; now appends the nudge directly into tool output via appendReminder; dedup and shouldInject guard preserved correctly.
src/hooks/task-session-manager/index.ts Switches from experimental.chat.system.transform to experimental.chat.messages.transform; injects <resumable_sessions> into the last orchestrator user-message text part; duplicate guard and internal-notification skip are in place.
src/index.ts Removes calls to the three system-transform hooks; wires taskSessionManagerHook.experimental.chat.messages.transform into the messages pipeline; passes output to todoContinuationHook.handleToolExecuteAfter.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[tool.execute.after] --> B{FILE_TOOLS?}
    B -- yes --> C{shouldInject?}
    C -- yes --> D[appendReminder to tool output\npost-file-tool-nudge]
    C -- no --> E[skip]
    B -- no --> F{RESET tool?\ne.g. todowrite}
    F -- yes --> G[clearCycle + getTodoState]
    G --> H{isFinalActive?}
    H -- yes --> I["⚠️ appendReminder final_active\n(shouldInject NOT checked)"]
    H -- no --> J[mark general / return]
    F -- no --> K{active session?}
    K -- yes --> L{shouldInject?}
    L -- yes --> M[getTodoState → appendReminder]
    L -- no --> N[clear + skip]

    subgraph messages_transform [experimental.chat.messages.transform]
        P[scan messages backward\nfor last user msg] --> Q{orchestrator?}
        Q -- yes --> R[append resumable_sessions block\ntask-session-manager]
        R --> S[append phase_reminder at tail]
        Q -- no --> T[skip]
    end
Loading

Comments Outside Diff (1)

  1. src/hooks/todo-continuation/todo-hygiene.ts, line 145-153 (link)

    P1 shouldInject guard bypassed in todowrite → final_active path

    The shouldInject check (line 168) only runs on the non-RESET branch. When todowrite is processed and the state is final_active, appendReminder fires immediately without consulting shouldInject. In production, createTodoHygiene is wired with shouldInject: (sessionID) => isOrchestratorSession(sessionID), so any sub-agent session (explorer, oracle, etc.) that calls todowrite while exactly one todo remains in-progress will have TODO_FINAL_ACTIVE_REMINDER injected into its tool output — the opposite of the intended behaviour.

    // Before injecting, honour the shouldInject guard:
    if (options.shouldInject && !options.shouldInject(input.sessionID)) {
      active.delete(input.sessionID);
      return;
    }
    mark(input.sessionID, 'final_active');
    appendReminder(output, TODO_FINAL_ACTIVE_REMINDER);

    Fix in Claude Code

Fix All in Claude Code

Reviews (2): Last reviewed commit: "Keep phase reminder at final message tai..." | Re-trigger Greptile

@dhaern
Copy link
Copy Markdown
Collaborator

dhaern commented Apr 27, 2026

Summary

  • Closes [Bug]: Multiple hooks inject into system messages in cache-busting ways #415.
  • Keeps the orchestrator phase reminder active on every orchestrator turn, but appends it at the latest user-message tail instead of prepending it or putting it in system.
  • Stops dynamic todo hygiene, post-file-tool, and resumable-session state from mutating the cached system prompt.
  • Preserves runtime nudges by appending them at the tail: file/todo reminders go into tool output as <internal_reminder>, while resumable session context is appended to the latest orchestrator user message.

What changed

  • phase-reminder now appends <reminder>...</reminder> after the latest orchestrator user text. It skips non-orchestrator and internal notification turns and avoids duplicate reminders.
  • post-file-tool-nudge appends the reminder to Read/Write tool output instead of setting pending state for system injection.
  • todo-continuation injects hygiene/final-active reminders into the triggering tool output.
  • task-session-manager injects <resumable_sessions> into request-local message context instead of experimental.chat.system.transform.
  • Removed the dynamic system-transform hooks/no-op compatibility shims, so the plugin no longer wires unused cache-sensitive reminder paths.

Why this fixes the issue

OpenCode/provider prompt caching depends on stable cached prefix content. Before this change, runtime state such as file-tool activity, todo state, resumable sessions, and per-request reminders could alter system or prepend content ahead of the user's turn. This PR keeps dynamic reminders out of system and places request-local guidance at the tail/tool-output context, so reminders remain visible without invalidating the stable system prompt.

Verification

  • bun test src/hooks/phase-reminder/index.test.ts src/hooks/post-file-tool-nudge/index.test.ts src/hooks/todo-continuation/index.test.ts src/hooks/todo-continuation/todo-hygiene.test.ts src/hooks/task-session-manager/index.test.ts
  • bunx biome check src/index.ts src/hooks/post-file-tool-nudge/index.ts src/hooks/post-file-tool-nudge/index.test.ts src/hooks/todo-continuation/index.ts src/hooks/todo-continuation/index.test.ts src/hooks/todo-continuation/todo-hygiene.ts src/hooks/todo-continuation/todo-hygiene.test.ts src/hooks/task-session-manager/index.ts src/hooks/task-session-manager/index.test.ts
  • bun run typecheck
  • bun run build
  • Manual relaunch/log verification showed system hashes staying stable across plain messages, Read-tool nudges, and todo hygiene activity.

I like the broader direction here: moving runtime reminders out of system.transform seems like the right fix for #415.

One thing I’m unsure about is the todo-hygiene transport. In #416, the reminder stayed pending in memory after tool.execute.after and was consumed synchronously in messages.transform, so it remained request-local and separate from tool output content.

This PR instead appends the reminder directly to output.output.

Do you think that could risk persisting the reminder as part of the tool result/history, or losing it when output.output is not a string? If not, I’m fine with this direction — I just wanted to double-check that the dynamic todo behavior from #416 is preserved.

@alvinunreal
Copy link
Copy Markdown
Owner Author

@greptileai

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Multiple hooks inject into system messages in cache-busting ways

2 participants