fix: eliminate 8 as any in MCP handlers, structured output, and stream events+Claude Soul Document 蒸馏#1258
Merged
Conversation
…m events - Group A: Add : () => AnyObjectSchema type annotations to MCP notification schema constants (useIdeSelection, useIdeLogging, usePrompts, channelNotification) - Group B: Add isStructuredOutputAttachmentMessage type guard for structured output attachment payloads (execAgentHook) - Group C: Add isMessageDeltaStreamEvent type guard for message_delta stream event usage extraction (forkedAgent) These as any casts also exist in the upstream CCB source — this fix provides real type safety without changing any runtime behavior.
fix: 消除 8 个 as any — MCP 通知、结构化输出、流事件的类型安全修复
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRemove unsafe ChangesType Safety Improvements for Notifications and Stream Events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/utils/forkedAgent.ts (1)
505-515: ⚡ Quick winSimplify type guard to avoid repeated internal casts.
The guard repeatedly casts to
StreamEventMessage(lines 510, 511, 512, 513). Refactor using progressive narrowing with theinoperator and const extraction to avoid type assertions within the guard body.♻️ Cleaner implementation without internal casts
function isMessageDeltaStreamEvent( message: Message | StreamEvent, ): message is StreamEventMessage & { event: BetaRawMessageDeltaEvent } { - return ( - message.type === 'stream_event' && - typeof (message as StreamEventMessage).event === 'object' && - (message as StreamEventMessage).event !== null && - 'type' in (message as StreamEventMessage).event && - (message as StreamEventMessage).event.type === 'message_delta' - ) + if (message.type !== 'stream_event') return false + if (!('event' in message)) return false + const event = message.event + if (typeof event !== 'object' || event === null) return false + if (!('type' in event)) return false + return event.type === 'message_delta' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/forkedAgent.ts` around lines 505 - 515, The type guard isMessageDeltaStreamEvent currently repeats casts to StreamEventMessage; refactor it to progressively narrow the union by first checking message.type === 'stream_event', then using the `in` operator to assert the presence of 'event' and extract it into a const (e.g., const event = (message as any).event) to perform the remaining checks (typeof event === 'object' && event !== null && 'type' in event && event.type === 'message_delta'), eliminating repeated (message as StreamEventMessage) casts and keeping the function signature and return type unchanged.src/utils/hooks/execAgentHook.ts (1)
49-54: 💤 Low valueConsider using
inoperator to avoid cast.Line 53 casts to
Messageto access theattachmentproperty. After checkingmessage.type === 'attachment'at line 52, you can use theinoperator to narrow without casting.♻️ Alternative without cast
function isStructuredOutputAttachmentMessage( message: QueryMessage, ): message is StructuredOutputAttachmentMessage { if (message.type !== 'attachment') return false - return (message as Message).attachment?.type === 'structured_output' + if (!('attachment' in message)) return false + return message.attachment?.type === 'structured_output' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/hooks/execAgentHook.ts` around lines 49 - 54, The type-guard isStructuredOutputAttachmentMessage currently casts message to Message to read attachment; replace the cast with an 'in' property check to let TypeScript narrow the union: after confirming message.type === 'attachment', test "'attachment' in message" and then check message.attachment?.type === 'structured_output' so no (message as Message) cast is needed; update the isStructuredOutputAttachmentMessage function accordingly referencing QueryMessage, StructuredOutputAttachmentMessage and the attachment property.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/utils/forkedAgent.ts`:
- Around line 505-515: The type guard isMessageDeltaStreamEvent currently
repeats casts to StreamEventMessage; refactor it to progressively narrow the
union by first checking message.type === 'stream_event', then using the `in`
operator to assert the presence of 'event' and extract it into a const (e.g.,
const event = (message as any).event) to perform the remaining checks (typeof
event === 'object' && event !== null && 'type' in event && event.type ===
'message_delta'), eliminating repeated (message as StreamEventMessage) casts and
keeping the function signature and return type unchanged.
In `@src/utils/hooks/execAgentHook.ts`:
- Around line 49-54: The type-guard isStructuredOutputAttachmentMessage
currently casts message to Message to read attachment; replace the cast with an
'in' property check to let TypeScript narrow the union: after confirming
message.type === 'attachment', test "'attachment' in message" and then check
message.attachment?.type === 'structured_output' so no (message as Message) cast
is needed; update the isStructuredOutputAttachmentMessage function accordingly
referencing QueryMessage, StructuredOutputAttachmentMessage and the attachment
property.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cb8c686-574a-4fe4-8eb3-238278464965
📒 Files selected for processing (7)
src/entrypoints/cli.tsxsrc/hooks/useIdeLogging.tssrc/hooks/useIdeSelection.tssrc/hooks/usePromptsFromClaudeInChrome.tsxsrc/services/mcp/channelNotification.tssrc/utils/forkedAgent.tssrc/utils/hooks/execAgentHook.ts
…to system prompt - prompts.ts: add getModePersonaSection() → injects current mode's systemPrompt as 'mode_persona' dynamic section (first in order, before operational instructions). Previously modes had systemPrompt fields but they were never sent to the model. - modes/personas/claude.ts: 3KB distilled Claude persona from Anthropic's leaked Claude 4.5 Opus Soul Document (70KB → operational extract): core traits, 7 honesty principles, helpfulness/caution balance, collaboration stance, identity stability. - With custom mode YAML (~/.claude/modes/claude.yaml), 7 modes total including the new Claude persona — fully operational at /mode claude. Co-Authored-By: James Feng <47167674+GhostDragon124@users.noreply.github.com>
feat: wire mode persona injection — Claude Soul Document distilled into system prompt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/constants/prompts.ts`:
- Line 66: Replace the relative import of getCurrentMode with the project alias
pattern: change the import that currently reads from '../modes/store.js' to use
the src/* alias (e.g. import { getCurrentMode } from 'src/modes/store') so it
matches the other imports in this file and project conventions; keep the
imported symbol name getCurrentMode unchanged and drop the .js extension if
other imports omit it.
In `@src/modes/personas/claude.ts`:
- Around line 1-8: Update the JSDoc at the top of src/modes/personas/claude.ts
(the Claude persona comment) to remove or clarify the word "leaked": either
state the source as "publicly available" or "inspired by public descriptions" if
not from an unauthorized disclosure, or explicitly note that the document was
obtained and reviewed for confidentiality/IP compliance if it truly derives from
a leak; include a short provenance line indicating whether legal/IP review was
completed and who approved it (or a note that it was not used for legal reasons)
so readers are not left with an implication of unauthorized disclosure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68f41393-f0b4-4983-b464-d76832e84da7
📒 Files selected for processing (2)
src/constants/prompts.tssrc/modes/personas/claude.ts
GhostDragon124
changed the title
Contributor
Author
|
两个问题都修了：
|
- prompts.ts: use 'src/modes/store.js' alias instead of relative '../modes/store.js' to match the file's existing import convention - claude.ts: reword JSDoc to say 'based on publicly available reference document' instead of 'leaked', addressing CodeRabbit review concern
…mplate for YAML config CodeRabbit noted that CLAUDE_PERSONA has no direct imports. This is intentional — it's a reference template for users defining custom modes via ~/.claude/modes/claude.yaml, not a programmatically imported constant.
Owner
|
@GhostDragon124 你的几个分支冲突了，解决一下合并为一个 pr 吧 |
Contributor
Author
已解决冲突，ccb大佬 |
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.
feat: as-any 类型修复 + 模式人格注入（Claude Soul Document 蒸馏）
schema constants (useIdeSelection, useIdeLogging, usePrompts, channelNotification)
output attachment payloads (execAgentHook)
stream event usage extraction (forkedAgent)
These as any casts also exist in the upstream CCB source — this fix provides
real type safety without changing any runtime behavior.
Summary by CodeRabbit
New Features
Refactor