Skip to content

🤖 refactor: simplify compaction notification tests#3269

Merged
ammario merged 1 commit into
mainfrom
simplify-notifications
May 12, 2026
Merged

🤖 refactor: simplify compaction notification tests#3269
ammario merged 1 commit into
mainfrom
simplify-notifications

Conversation

@ammar-agent
Copy link
Copy Markdown
Collaborator

Summary

Simplifies the notification/compaction follow-up test surface after #3261 by removing dead ContinueMessage factory code and replacing repeated AgentSession/WorkspaceStore scenario setup with focused fixtures. Net diff removes 1,091 lines while preserving the behavioral coverage for post-compaction notification suppression, background handoffs, queued follow-ups, and legacy pending-follow-up recovery.

Background

#3261 intentionally localized notification policy in the browser aggregator. This follow-up keeps that behavior intact but trims older helper code and test boilerplate that made the related compaction/notification paths harder to review.

Implementation

  • Removed unused ContinueMessage builder/rebuilder types and their standalone tautological tests; repo search found no production callsites.
  • Refactored agentSession.continueMessageAgentId.test.ts around a shared AgentSession/history harness and semantic compaction-summary fixtures.
  • Refactored WorkspaceStore.test.ts background completion scenarios around activity/stream event fixture helpers.
  • Updated one stale test comment that still referred to the removed ContinueMessage helper.

Validation

  • bun test src/browser/stores/WorkspaceStore.test.ts src/node/services/agentSession.continueMessageAgentId.test.ts src/browser/utils/messages/StreamingMessageAggregator.test.ts src/browser/utils/chatCommands.test.ts
  • make static-check

Risks

Low. Production change is limited to deleting unused common-message helper exports. Test refactors keep the same behavioral scenarios but centralize repeated fixture construction, so the main risk is fixture abstraction hiding a future scenario mismatch.


Generated with mux • Model: openai:gpt-5.5 • Thinking: high • Cost: $66.83

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please review this follow-up simplification PR, especially the removal of dead ContinueMessage helpers and the refactored compaction/notification test fixtures.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 106ea5b802

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/common/types/message.ts
@ammar-agent ammar-agent force-pushed the simplify-notifications branch from 106ea5b to 32b17df Compare May 12, 2026 04:15
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the mobile build issue by removing the stale mobile import of the deleted helper and verified mobile tests locally.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammario ammario merged commit fd4fcb4 into main May 12, 2026
23 checks passed
@ammario ammario deleted the simplify-notifications branch May 12, 2026 04:38
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.

2 participants