Skip to content

fix(typecheck): add plugin command view types - #1565

Merged
kevincodex1 merged 2 commits into
Twigpine:mainfrom
chioarub:codex/typecheck-plugin-command-types
Jun 10, 2026
Merged

kevincodex1 merged 2 commits into
Twigpine:mainfrom
chioarub:codex/typecheck-plugin-command-types

Conversation

@chioarub

@chioarub chioarub commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Refs #1486

Summary

  • add the shared plugin command ViewState and PluginSettingsProps types used by the plugin command views
  • add a discriminated UnifiedInstalledItem union for installed plugin/MCP rows
  • annotate narrow local state and update payloads where TypeScript inferred never[] or null too tightly

Validation

  • bun run typecheck still fails on unrelated baseline errors from fix: (All-call) TypeScript cleanup PRs needed #1486
  • plugin command diagnostics reduced from 33 to 1 on this branch
  • total src/* diagnostics reduced from 1043 to 1011 versus the captured baseline

Duplicate check

  • Checked open PRs for overlap on the touched plugin command files before opening this PR; none were touching these files.
  • The remaining plugin diagnostic is ManagePlugins.tsx importing ../../components/mcp/types.js, which is covered separately by fix(typecheck): add MCP component view types #1564.

Summary by CodeRabbit

  • Refactor
    • Improved internal type safety and code organization across plugin management and settings for greater stability.
  • Bug Fixes
    • More reliable plugin enable/disable and uninstall flows, including recovery when direct uninstall fails.
  • UI/Settings
    • More consistent plugin settings and marketplace/error handling behavior, reducing occasional UI/state issues.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 334ac7c6-6fb1-405f-b4ec-c635694dd209

📥 Commits

Reviewing files that changed from the base of the PR and between 2929e8b and ab3651a.

📒 Files selected for processing (1)
  • src/commands/plugin/ManagePlugins.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: smoke-and-tests
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise

Files:

  • src/commands/plugin/ManagePlugins.tsx
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/commands/plugin/ManagePlugins.tsx
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes fix: skip assertMinVersion for third-party providers #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • src/commands/plugin/ManagePlugins.tsx
🔇 Additional comments (1)
src/commands/plugin/ManagePlugins.tsx (1)

1521-1521: LGTM!


📝 Walkthrough

Walkthrough

This PR strengthens TypeScript type safety across the plugin management UI by introducing new public type definitions and refactoring components to use explicit types for state, intermediate records, and exported signatures.

Changes

Plugin Type System

Layer / File(s) Summary
Core type definitions and contracts
src/commands/plugin/types.ts, src/commands/plugin/unifiedTypes.ts
Adds PluginSettingsProps, PluginManageAction, MarketplaceManageAction, a discriminated ViewState union, and UnifiedInstalledItem/supporting types.
PluginSettings component type adoption
src/commands/plugin/PluginSettings.tsx
Introduces MarketplaceLoadFailure and ErrorsTabContentProps; types internal state (ViewState, TabId, `string
ManagePlugins component type adoption
src/commands/plugin/ManagePlugins.tsx
Declares commandPathList/agentPathList/skillPathList as string[], hooksList/mcpServersList as unknown[]; refactors failed-plugin uninstall recovery and confirm-project-uninstall handlers to construct typed enabledPlugins and pass via settingsUpdate.
UnifiedInstalledCell exported signature
src/commands/plugin/UnifiedInstalledCell.tsx
Adds explicit parameter type Props and return type React.ReactNode for the exported component.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Suggested reviewers

  • jatmn
  • kevincodex1
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning PR touches plugins/MCP (listed risk surface) but PR description/objectives do not explicitly call out the risk surface or blocker status as required by the check. Review should explicitly document: (1) what plugin/MCP risk surface is affected, (2) whether changes introduce functional/security implications, (3) blocker status for merge.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding TypeScript types for plugin command views to fix typecheck issues.
Description check ✅ Passed The description covers what changed and why, but the Testing section is incomplete and the Notes section lacks specifics about testing methodology.
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.
No Hidden Policy Change ✅ Passed PR adds TypeScript type definitions and refactors settings update calls with no hidden policy, trust-model, routing, telemetry/network, or permission-policy changes.

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

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/commands/plugin/ManagePlugins.tsx`:
- Around line 1514-1523: The code creates a local variable enabledPlugins and
then redundantly spreads it into the object passed to updateSettingsForSource;
change the call to updateSettingsForSource('localSettings', { enabledPlugins:
enabledPlugins }) (i.e., pass the enabledPlugins record directly) instead of
using {...enabledPlugins}; locate enabledPlugins, updateSettingsForSource,
getSettingsForSource and pluginId_8 in ManagePlugins.tsx to make this simple
substitution.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2824b5c1-fe63-4ca8-b3f9-bb8218d327db

📥 Commits

Reviewing files that changed from the base of the PR and between 9e942da and 2929e8b.

📒 Files selected for processing (5)
  • src/commands/plugin/ManagePlugins.tsx
  • src/commands/plugin/PluginSettings.tsx
  • src/commands/plugin/UnifiedInstalledCell.tsx
  • src/commands/plugin/types.ts
  • src/commands/plugin/unifiedTypes.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise

Files:

  • src/commands/plugin/types.ts
  • src/commands/plugin/UnifiedInstalledCell.tsx
  • src/commands/plugin/unifiedTypes.ts
  • src/commands/plugin/PluginSettings.tsx
  • src/commands/plugin/ManagePlugins.tsx
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/commands/plugin/types.ts
  • src/commands/plugin/UnifiedInstalledCell.tsx
  • src/commands/plugin/unifiedTypes.ts
  • src/commands/plugin/PluginSettings.tsx
  • src/commands/plugin/ManagePlugins.tsx
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • src/commands/plugin/types.ts
  • src/commands/plugin/UnifiedInstalledCell.tsx
  • src/commands/plugin/unifiedTypes.ts
  • src/commands/plugin/PluginSettings.tsx
  • src/commands/plugin/ManagePlugins.tsx
🔇 Additional comments (6)
src/commands/plugin/types.ts (1)

1-36: LGTM!

src/commands/plugin/unifiedTypes.ts (1)

1-59: LGTM!

src/commands/plugin/PluginSettings.tsx (1)

31-39: LGTM!

Also applies to: 348-348, 366-385, 737-770, 989-1003

src/commands/plugin/ManagePlugins.tsx (2)

239-313: LGTM!


1471-1478: LGTM!

src/commands/plugin/UnifiedInstalledCell.tsx (1)

11-11: LGTM!

Comment thread src/commands/plugin/ManagePlugins.tsx Outdated

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution. I do not see any actionable issues from my review.

@kevincodex1 LGTM

@kevincodex1
kevincodex1 merged commit fc0a4b5 into Twigpine:main Jun 10, 2026
3 checks passed
@chioarub
chioarub deleted the codex/typecheck-plugin-command-types branch June 10, 2026 05:56
deagwon97 pushed a commit to deagwon97/openclaude that referenced this pull request Jun 11, 2026
* fix(typecheck): add plugin command view types

* fix(typecheck): simplify plugin settings update
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.

3 participants