Skip to content

fix(reminder-hooks): preserve suppression state across compaction#3497

Open
Disaster-Terminator wants to merge 3 commits intocode-yeongyu:devfrom
Disaster-Terminator:fix/reminder-hooks-preserve-state-across-compaction
Open

fix(reminder-hooks): preserve suppression state across compaction#3497
Disaster-Terminator wants to merge 3 commits intocode-yeongyu:devfrom
Disaster-Terminator:fix/reminder-hooks-preserve-state-across-compaction

Conversation

@Disaster-Terminator
Copy link
Copy Markdown

@Disaster-Terminator Disaster-Terminator commented Apr 18, 2026

Summary

  • Preserve reminder state across session compaction.
  • Prevent reminder hooks from re-arming repeatedly in long sessions.

Changes

  • Stop clearing agent-usage-reminder state on session.compacted.
  • Stop clearing category-skill-reminder state on session.compacted.
  • Cap agent-usage-reminder output per session and keep session.deleted reset behavior.
  • Add regression coverage for compaction and delete/reset behavior.

Testing

bun test src/hooks/agent-usage-reminder/index.test.ts src/hooks/category-skill-reminder/index.test.ts

Related Issues

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 18, 2026

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

@Disaster-Terminator
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Apr 18, 2026
@Disaster-Terminator Disaster-Terminator marked this pull request as ready for review April 18, 2026 02:04
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files

Confidence score: 5/5

  • This PR looks low risk to merge: the only finding is low severity (3/10) and is limited to test quality rather than production behavior.
  • The issue in src/hooks/agent-usage-reminder/index.test.ts is that the delete regression test is non-diagnostic, so it may not reliably prove session.deleted clears reminder state.
  • Pay close attention to src/hooks/agent-usage-reminder/index.test.ts - strengthen the reset-path assertion (or hit the reminder cap before delete) so the regression check is definitive.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/hooks/agent-usage-reminder/index.test.ts">

<violation number="1" location="src/hooks/agent-usage-reminder/index.test.ts:65">
P3: This delete regression test is non-diagnostic: one prior reminder is not enough to prove `session.deleted` cleared state. Assert the reset path directly (or drive the session to the reminder cap before deleting) so the test fails on a broken delete handler.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/hooks/agent-usage-reminder/index.test.ts
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Auto-approved: Correctly prevents reminder loops by preserving suppression state across compaction and introduces a sensible reminder cap with thorough test coverage.

This was referenced Apr 24, 2026
This was referenced Apr 26, 2026
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.

1 participant