Skip to content

Fix type#1241

Closed
xiaoFjun-eng wants to merge 6 commits into
claude-code-best:mainfrom
xiaoFjun-eng:fixType
Closed

Fix type#1241
xiaoFjun-eng wants to merge 6 commits into
claude-code-best:mainfrom
xiaoFjun-eng:fixType

Conversation

@xiaoFjun-eng
Copy link
Copy Markdown
Contributor

@xiaoFjun-eng xiaoFjun-eng commented May 19, 2026

修复claude-for-chrome-mcp中的type和interface类型缺失.

Summary by CodeRabbit

  • Refactor

    • Improved structured error logging and richer error details across multiple modules for better diagnostics.
    • Strengthened typing for logging and event-handling internals to reduce runtime issues.
  • Bug Fixes

    • Improved macOS permission probing and reporting for computer-use features.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe26bc74-82ca-499d-ad94-7ff744299a85

📥 Commits

Reviewing files that changed from the base of the PR and between 9982181 and a26b07b.

📒 Files selected for processing (4)
  • packages/@ant/claude-for-chrome-mcp/src/types.ts
  • packages/@ant/computer-use-mcp/src/types.ts
  • packages/@ant/ink/src/core/dom.ts
  • packages/@ant/ink/src/core/events/terminal-event.ts

📝 Walkthrough

Walkthrough

Adds a structured LoggerDetail type and toLoggerDetail() helper, updates Logger signatures and logging sites across MCP packages and adapters, re-exports the new logging types, and tightens Ink event-handler prop typing with a typed setEventHandler helper.

Changes

Structured Logging Refactor

Layer / File(s) Summary
Logger contracts and helpers
packages/@ant/claude-for-chrome-mcp/src/types.ts, packages/@ant/computer-use-mcp/src/types.ts
LoggerDetail and toLoggerDetail() added; Logger methods changed from (...args: unknown[]) to (message: string, detail?: LoggerDetail).
Public API exports and Chrome Bridge metadata
packages/@ant/claude-for-chrome-mcp/src/index.ts, packages/@ant/claude-for-chrome-mcp/src/types.ts
LoggerDetail and toLoggerDetail re-exported; Chrome Bridge telemetry metadata types added and ClaudeForChromeContext.trackEvent signature updated.
Error logging in @ant/claude-for-chrome-mcp
packages/@ant/claude-for-chrome-mcp/src/bridgeClient.ts, packages/@ant/claude-for-chrome-mcp/src/mcpSocketClient.ts, packages/@ant/claude-for-chrome-mcp/src/toolCalls.ts
WebSocket construction, JSON parse, permission-request, security-validation, socket error, and tool-call error paths updated to use toLoggerDetail(...) and structured/multiline logger calls.
Error logging in @ant/computer-use-mcp
packages/@ant/computer-use-mcp/src/pixelCompare.ts, packages/@ant/computer-use-mcp/src/toolCalls.ts
Pixel-compare validation and tool-call exception logging switched to toLoggerDetail(err) and multiline structured logging.
Logger implementations in application adapters
src/utils/claudeInChrome/mcpServer.ts, src/utils/computerUse/hostAdapter.ts
DebugLogger updated to accept detail?: LoggerDetail and format messages via format(message, detail ?? '') while preserving severity mappings.

Event Handler Type Safety in Ink Framework

Layer / File(s) Summary
Event handler property type definitions
packages/@ant/ink/src/core/dom.ts, packages/@ant/ink/src/core/events/terminal-event.ts, packages/@ant/ink/src/core/reconciler.ts
EventHandlerProps imported as a type and _eventHandlers fields narrowed from Record<string, unknown> to Partial<EventHandlerProps>.
Type-safe event handler application logic
packages/@ant/ink/src/core/reconciler.ts
setEventHandler made generic over EventHandlerProps keys; applyProp and commitUpdate call the typed helper with constrained key/value casts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • KonghaYao

"I hop through logs with tidy paws,
Errors find structure, not just gauze.
Event handlers learn their types,
Debuggers sing through clearer lines.
Hooray — a rabbit's tidy cause!" 🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix type' is vague and generic. While the PR does fix type-related issues, the title lacks specificity about what types are being fixed or in which modules, making it unclear without reading the full description. Consider using a more specific title that indicates the scope and nature of the fix, such as 'Refactor Logger interface to use structured LoggerDetail type' or 'Add missing type declarations to claude-for-chrome-mcp and related modules'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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