Skip to content

feat: [ENG-2468] register OpenClaude as a connector agent#548

Merged
bao-byterover merged 3 commits intomainfrom
feat/ENG-2468
Apr 28, 2026
Merged

feat: [ENG-2468] register OpenClaude as a connector agent#548
bao-byterover merged 3 commits intomainfrom
feat/ENG-2468

Conversation

@DatPham-6996
Copy link
Copy Markdown
Collaborator

OpenClaude is a multi-provider Claude Code fork that reuses Claude Code's config paths (~/.claude/, .mcp.json, CLAUDE.md, .claude/skills), so the existing connector implementations work as-is — only declarative registry entries are needed. Adds OpenClaude with default 'mcp' and supported ['mcp', 'skill', 'rules']. Hook is omitted from v1 since Claude Code's hook detector relies on agent-internal markers that need separate verification against OpenClaude's hook chain.

Summary

  • Problem: brv connectors install already supports 23 host agents (Claude Code, Cursor, Windsurf, OpenClaw, etc.) but has no entry for OpenClaude, the multi-provider Claude Code fork. Users running OpenClaude have to either edit .mcp.json by hand or repurpose the Claude Code connector with no clear signal that it applies.
  • Why it matters: OpenClaude shows up in the wild and shares Claude Code's config paths exactly, so a discoverable first-class entry costs ~21 lines and lights up the full connectors UX (list / install / switch / uninstall / status / doctor) for OpenClaude users with zero new code.
  • What changed: Added OpenClaude to the agent enum and registered declarative entries in the MCP, skill, and rules connector config maps — all mirroring Claude Code's paths. Default connector is mcp; skill and rules available via --type.
  • What did NOT change (scope boundary): No connector implementation (*-connector.ts) was modified; no new files; no template-service entry (OpenClaude falls through like Claude Code); no hook connector entry (deferred to a follow-up); no documentation/README updates in this PR.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes ENG-2468
  • Related #

Root cause (bug fixes only, otherwise write N/A)

  • Root cause: N/A (additive feature, not a bug fix)
  • Why this was not caught earlier: N/A

Test plan

  • Coverage added:
    • Unit test (existing tests cover the registry)
    • Integration test
    • Manual verification only
  • Test file(s): test/unit/infra/connectors/** (243 tests), test/commands/connectors/**, test/unit/infra/connectors/connector-manager-migration.test.ts
  • Key scenario(s) covered:
    • TypeScript Record<Agent, ...> exhaustiveness on AGENT_CONNECTOR_CONFIG ensures every agent in AGENT_VALUES has a capability entry — npm run typecheck is the contract that guarantees the registry stays consistent.
    • Existing connector-manager-migration test ("no orphaned MCP agents when all MCP-configured agents support MCP") covers the cross-map invariant for the new entry.
    • Manual verification: invoking brv connectors install OpenClaude --type mcp writes the expected mcpServers.brv entry to .mcp.json; --type skill writes to .claude/skills/byterover/SKILL.md; --type rules appends a marked block to CLAUDE.md.

User-visible changes

  • New entry OpenClaude appears in the interactive picker shown by brv connectors install.
  • brv connectors install OpenClaude is now accepted as a direct invocation (defaults to --type mcp); --type skill and --type rules are also supported.
  • brv connectors list will show OpenClaude once an OpenClaude connector is installed.
  • No changes to existing agents — Claude Code, OpenClaw, and the other 21 agents behave identically.

Evidence

  • npm run typecheck → clean (no errors)
  • npm run lint → 0 errors, 224 pre-existing warnings (none in the files this PR touches)
  • npm run build✓ built in 2.74s
  • Targeted test runs:
    • npx mocha "test/unit/infra/connectors/**/*.test.ts" "test/commands/connectors/**/*.test.ts"243 passing
    • npx mocha "test/unit/infra/connectors/connector-manager-migration.test.ts" "test/unit/core/domain/entities/brv-config.test.ts"65 passing (includes "no orphaned MCP agents" cross-map invariant)

Checklist

OpenClaude is a multi-provider Claude Code fork that reuses Claude Code's
config paths (~/.claude/, .mcp.json, CLAUDE.md, .claude/skills), so the
existing connector implementations work as-is — only declarative
registry entries are needed. Adds OpenClaude with default 'mcp' and
supported ['mcp', 'skill', 'rules']. Hook is omitted from v1 since
Claude Code's hook detector relies on agent-internal markers that need
separate verification against OpenClaude's hook chain.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 25, 2026

Claude finished @DatPham-6996's task in 2m 13s —— View job


PR Review: feat: [ENG-2468] register OpenClaude as a connector agent

  • Gather context and understand the PR
  • Read changed files
  • Compare with existing patterns (Claude Code entries)
  • Analyze correctness, consistency, and code quality
  • Post review feedback

Summary

Score: 3 / 5

This is a clean, minimal declarative addition with good scope discipline. The rules and skill entries are correct. However, there are two issues worth addressing before merge: a functional divergence in the MCP serverConfig and a style inconsistency in the supported array ordering.


Issues

1. serverConfig does not mirror Claude Code — may break MCP install (functional)

File: src/server/infra/connectors/mcp/mcp-connector-config.ts:213–220

The PR description states OpenClaude "shares Claude Code's config paths exactly" and the entries "mirror Claude Code's paths." However, the MCP serverConfig for OpenClaude uses the bare DEFAULT_SERVER_CONFIG:

// OpenClaude (new)
serverConfig: DEFAULT_SERVER_CONFIG,  // { command: 'brv', args: ['mcp'] }

…while Claude Code's entry explicitly includes type: 'stdio' and env: {}:

// Claude Code (existing)
serverConfig: {
  type: 'stdio',
  command: 'brv',
  args: ['mcp'],
  env: {},
},

Since OpenClaude reads .mcp.json using the same parser as Claude Code, and Claude Code's MCP spec requires type: "stdio" to correctly identify the transport, installing without it may silently produce a config that OpenClaude ignores or rejects. The empty env: {} also guards against inheriting unintended parent-process environment variables — omitting it is a minor security regression.

Fix this →


2. supported array ordering is inconsistent with every other agent (style)

File: src/server/core/domain/entities/agent.ts:87

Every other agent that supports all three connector types uses ['rules', 'mcp', 'skill']:

// All existing tri-support agents
supported: ['rules', 'mcp', 'skill'],

OpenClaude's entry flips the order:

// OpenClaude (new)
supported: ['mcp', 'skill', 'rules'],

The supported array is surfaced to clients verbatim via agent-dto-mapper.tssupportedConnectorTypes: [...config.supported], so the order may affect the sequence in which connector type options appear in the TUI/UI picker. More importantly, it's a consistency violation that will puzzle future maintainers.

Fix this →


Observations (non-blocking)

default: 'mcp' vs Claude Code's default: 'skill' — intentional and well-reasoned. Defaulting to mcp is the conservative choice while hook support remains unverified. No issue.

Hook omissionhook is correctly absent from both HOOK_CONNECTOR_CONFIGS and AGENT_CONNECTOR_CONFIG[OpenClaude].supported. The comment in hook-connector-config.ts even has a checklist for this ("To add hook support for a new agent: 1. Add here, 2. Add 'hook' to supported"). Properly deferred.

Rules and skill entriesCLAUDE.md / append and .claude/skills exactly mirror Claude Code. Correct.

Template service — no agent-specific guideHeaders entry needed; OpenClaude falls through to the empty-header default, same as Claude Code. Correct.

TypeScript exhaustiveness — adding OpenClaude to AGENT_VALUES forces a compile error if AGENT_CONNECTOR_CONFIG is missing the key, which is a robust contract. The existing "no orphaned MCP agents" test in connector-manager-migration.test.ts also covers the cross-map invariant. Good structural safety.


What's working well

  • Minimal blast radius: 5 files, 21 lines, zero new implementations
  • Alphabetical ordering maintained throughout AGENT_VALUES and all config maps
  • Scope boundary discipline: no unrelated changes
  • TypeScript compile-time exhaustiveness check serves as a test harness for the registry

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 25, 2026

Claude finished @DatPham-6996's task in 55s —— View job


PR Review: feat: [ENG-2468] register OpenClaude as a connector agent

  • Gather context and understand the PR
  • Read changed files
  • Compare with existing patterns (Claude Code entries)
  • Analyze correctness, consistency, and code quality
  • Post review feedback

Summary

Score: 5 / 5

Both issues from the previous review have been addressed. The PR is clean, minimal, correct, and consistent — ready to merge.


Previous Issues — Now Resolved

serverConfig now mirrors Claude Code exactly

File: src/server/infra/connectors/mcp/mcp-connector-config.ts:213–225

OpenClaude's serverConfig now explicitly includes type: 'stdio' and env: {}, exactly matching Claude Code's entry:

OpenClaude: {
  configPath: '.mcp.json',
  format: 'json',
  mode: 'auto',
  scope: 'project',
  serverConfig: {
    type: 'stdio',
    command: 'brv',
    args: ['mcp'],
    env: {},
  },
  serverKeyPath: STANDARD_KEY_PATH,
},

This ensures the generated .mcp.json is valid for OpenClaude's parser and guards against unintended environment variable inheritance.

supported array ordering is now consistent

File: src/server/core/domain/entities/agent.ts:85–88

OpenClaude's supported now uses ['rules', 'mcp', 'skill'], matching every other tri-support agent:

OpenClaude: {
  default: 'mcp',
  supported: ['rules', 'mcp', 'skill'],
},

Remaining Observations (all positive)

  • Rules connector (rules-connector-config.ts:74–77): CLAUDE.md / append — correct mirror of Claude Code. ✓
  • Skill connector (skill-connector-config.ts:63–66): .claude/skills for both globalPath and projectPath — correct mirror of Claude Code. ✓
  • shared/types/agent.ts:19: OpenClaude inserted in correct alphabetical position between Kiro and OpenClaw. ✓
  • default: 'mcp': Intentional and well-reasoned given unverified hook chain. ✓
  • Hook omission: Correctly absent from both HOOK_CONNECTOR_CONFIGS and supported. ✓
  • TypeScript exhaustiveness: Record<Agent, AgentConnectorConfig> ensures compile-time safety for any future registry drift. ✓
  • Blast radius: 5 files, 26 lines added, zero new implementations. ✓

ncnthien
ncnthien previously approved these changes Apr 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

Claude finished @DatPham-6996's task in 2m 27s —— View job


PR Review: feat: [ENG-2468] register OpenClaude as a connector agent

  • Gather context and understand the PR
  • Read changed files and git diff
  • Compare with existing patterns (Claude Code entries)
  • Analyze correctness, consistency, and code quality
  • Post review feedback

Summary

Score: 5 / 5

The PR is clean, minimal, and correct. The third commit (36df4ed feat: [ENG-2468]: Change default skill type) changed default: 'mcp'default: 'skill', which is actually an improvement — OpenClaude now mirrors Claude Code's default exactly across every config dimension except the deferred hook. Ready to merge.


Latest Change: default: 'skill' (third commit)

File: src/server/core/domain/entities/agent.ts:85–88

OpenClaude: {
  default: 'skill',    // was 'mcp' in the previous iteration
  supported: ['rules', 'mcp', 'skill'],
},

The previous review noted 'mcp' was intentional as a "conservative choice while hook support is unverified." Switching to 'skill' is correct: the hook entry is what's unverified, not the skill entry — and .claude/skills works identically for OpenClaude since it shares that path. Claude Code itself defaults to 'skill', so this change makes OpenClaude's UX identical to Claude Code out-of-the-box. Well-reasoned follow-through.


Full Registry Verification

Config dimension Claude Code OpenClaude Match
default 'skill' 'skill'
supported ['rules', 'hook', 'mcp', 'skill'] ['rules', 'mcp', 'skill'] ✅ (hook deferred)
MCP configPath .mcp.json .mcp.json
MCP serverConfig.type 'stdio' 'stdio'
MCP serverConfig.command 'brv' 'brv'
MCP serverConfig.args ['mcp'] ['mcp']
MCP serverConfig.env {} {}
Rules filePath CLAUDE.md CLAUDE.md
Rules writeMode 'append' 'append'
Skill globalPath .claude/skills .claude/skills
Skill projectPath .claude/skills .claude/skills
Hook config present absent ✅ (correctly deferred)

Observations (non-blocking)

PR description is slightly stale — the body still says "Default connector is mcp" but the code now has default: 'skill'. The code is correct; the description is behind. Not a merge blocker, but worth noting for the changelog or a quick description edit.

supported ordering['rules', 'mcp', 'skill'] consistent with all other tri-support agents. ✅

Alphabetical positionOpenClaude sits between Kiro and OpenClaw in both AGENT_VALUES and all four config maps. ✅

TypeScript exhaustivenessRecord<Agent, AgentConnectorConfig> ensures a compile error if any registry map drifts out of sync. Robust safety net. ✅

Hook omission — correctly absent from both HOOK_CONNECTOR_CONFIGS and supported. ✅

Blast radius — 5 files, 26 lines added, zero new implementations, zero deletions. ✅

Copy link
Copy Markdown
Collaborator

@cuongdo-byterover cuongdo-byterover left a comment

Choose a reason for hiding this comment

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

good from my side

Copy link
Copy Markdown
Collaborator

@ncnthien ncnthien left a comment

Choose a reason for hiding this comment

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

you're doing good job 1

Copy link
Copy Markdown
Collaborator

@ncnthien ncnthien left a comment

Choose a reason for hiding this comment

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

lgtm

@ncnthien
Copy link
Copy Markdown
Collaborator

lgm

Copy link
Copy Markdown
Collaborator

@bao-byterover bao-byterover left a comment

Choose a reason for hiding this comment

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

LGTM

@bao-byterover bao-byterover merged commit c710de7 into main Apr 28, 2026
6 checks passed
@bao-byterover bao-byterover deleted the feat/ENG-2468 branch April 28, 2026 02:16
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.

4 participants