refactor(discord): split thread-manager.ts into focused modules#1786
Open
corvid-agent wants to merge 9 commits intomainfrom
Open
refactor(discord): split thread-manager.ts into focused modules#1786corvid-agent wants to merge 9 commits intomainfrom
corvid-agent wants to merge 9 commits intomainfrom
Conversation
The flock_reputation_refresh handler (added in #1776) had no direct tests. This adds: - New test file for execFlockReputationRefresh: empty flock (0 updated), populated flock (N updated), and error path (table dropped → failed status) - getActionCategory('flock_reputation_refresh') → 'lightweight' in priority-rules.test.ts - 'flock_reputation_refresh' added to valid action types list in scheduler-pipeline.test.ts All 62 tests pass. Zero TSC errors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When recording a failed auth attempt to the audit database fails, the catch block was silently swallowing the error with only a comment. This meant that if the audit record failed to persist (DB connection issue, schema mismatch, etc.), there was zero visibility — no log line, no metric, nothing. Security forensics could be blind to audit failures. Now we log a warning when the audit record fails while preserving the best-effort semantics (the rejection still happens, auditing is not blocking). Fixes silent error in server/middleware/auth.ts:247. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Adds a test that mocks recordAudit to throw, exercising the catch branch in checkHttpAuth that logs audit failures. Fixes codecov/patch check on PR #1781. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unused spyOn import (TS6133) and fix mock.module restore in auth-middleware test. The previous restore called require() which returned the already-mocked module, poisoning 369 downstream tests. Now saves a reference to the real module before mocking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 'still rejects when audit logging throws' test used mock.module to replace ../db/audit with a throwing stub. Bun's mock.module is persistent across test files, so the mock leaked and caused every subsequent test calling recordAudit to throw "DB connection lost". The auth rejection (403) behavior is already covered by the existing test above. The catch path is a trivial log.warn that cannot alter the response, so removing this test loses no meaningful coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a test that mocks recordAudit to throw, exercising the catch block that logs a warning when audit recording fails. Fixes codecov/patch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract the 1440-line thread-manager.ts into six cohesive modules under server/discord/thread-response/: - utils.ts: sessionErrorEmbed + visibleEmbedParts helpers - embed-response.ts: subscribeForResponseWithEmbed (thread mode) - inline-response.ts: subscribeForInlineResponse (@mention replies) - adaptive-response.ts: subscribeForAdaptiveInlineResponse (upgrade UX) - progress-response.ts: subscribeForInlineProgressResponse (edit-in-place) - recovery.ts: all three recovery functions + resolveDefaultAgent thread-manager.ts becomes a thin barrel re-exporting the full public API surface so all existing consumers remain unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- docs/deep-dive.md: 206 → 207 spec files (new thread-response.spec.md) - Add tests for visibleEmbedParts to improve patch coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
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
1 participant
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.
Summary
thread-manager.ts(1440 lines) was a monolith mixing four different subscription strategies, recovery logic, and utilitiesserver/discord/thread-response/:utils.ts—sessionErrorEmbed+visibleEmbedPartsembed-response.ts—subscribeForResponseWithEmbed(thread-mode, rich embeds with stats)inline-response.ts—subscribeForInlineResponse(lightweight @mention replies)adaptive-response.ts—subscribeForAdaptiveInlineResponse(upgrades from typing to progress on tool use)progress-response.ts—subscribeForInlineProgressResponse(edit-in-place progress embed)recovery.ts— three restart-recovery functions +resolveDefaultAgentthread-manager.tsis now a thin barrel (~25 lines) re-exporting the full public API — no consumer changes neededTest plan
bun x tsc --noEmit --skipLibCheck— cleanbun test— 9617 pass, 0 fail🤖 Generated with Claude Code