feat(runtime): rebuild MCP and Skill control plane - #15
Merged
Conversation
Hot-apply user MCP servers across all agents, add strict HTTP and STDIO lifecycle management, and expose MCP controls in Settings. Add five-tier Skill discovery, diagnostics, immutable execution overlays, and the Composer picker. BREAKING CHANGE: MCP server configuration now requires type and enabled, and the legacy url, headers, and timeout shape is rejected.
|
This PR has 13,221 reviewable changed lines after ignored/generated files are excluded, above this repository's 10,000-changed-line automatic review limit. The raw diff is 14,456 lines before ignored/generated files are excluded. Most of the diff comes from:
Comment |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (15)
📝 WalkthroughWalkthroughChangesThe pull request replaces legacy MCP management with a live HTTP/STDIO runtime, hot application, lifecycle APIs, and Settings controls. It adds Skill discovery, precedence diagnostics, pagination, prompt projections, MCP runtime and configuration
Skill control plane
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Owner
Author
|
@cubic-dev-ai review this |
@boh5 I have started the AI code review. It will take a few minutes to complete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-core/src/mcp/errors.ts (1)
40-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake error messages match
reason.When
reasonis"aborted"or"timeout", both messages still state that the operation “failed”.tool-adapter.tsreturns these messages with cancellation or timeout result codes. Generate reason-specific messages for consistent tool feedback.🤖 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 `@packages/agent-core/src/mcp/errors.ts` around lines 40 - 56, Update the constructors for the MCP connection and tool execution error classes so their messages use the provided reason: report cancellation for "aborted", timeout for "timeout", and retain failure wording for "failed". Preserve the existing server, tool, and cause details while ensuring tool-adapter result messages remain consistent with their reason codes.
🧹 Nitpick comments (14)
packages/agent-core/src/mcp/naming.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Bun’s native
Bun.CryptoHasherAPI instead ofnode:crypto. Preserve the current SHA-256 updates anddigest("hex")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 `@packages/agent-core/src/mcp/naming.ts` at line 1, Replace the node:crypto createHash usage in the naming module with Bun.CryptoHasher, preserving the existing SHA-256 update sequence and digest("hex") output behavior.Source: Coding guidelines
packages/agent-core/src/mcp/tool-adapter.ts (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the MCP input-validation boundary.
ToolRegistry.#prepareInputappliessafeParsetomcpToolInputSchema, but.catchall(z.unknown())accepts any object.aiInputSchemaonly describes inputs for the model; this code does not validatemcpTool.inputSchemabefore dispatch. If the MCP server is the validation boundary, document the local exception to the strict Zod-schema rule and add a regression test for invalid arguments.🤖 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 `@packages/agent-core/src/mcp/tool-adapter.ts` at line 19, Document that mcpToolInputSchema intentionally accepts arbitrary object arguments because MCP performs validation before dispatch, while aiInputSchema only describes model inputs. Add a regression test covering invalid arguments through ToolRegistry.#prepareInput and confirm they are passed to the MCP tool without local schema rejection.Source: Coding guidelines
packages/agent-core/src/mcp/client.ts (2)
238-278: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated discovery deadline check.
Lines 263 and 266 run the same
this.now() >= deadlinecheck with only a synchronoustools.pushbetween them. One check is enough.♻️ Proposed simplification
throwIfAborted(signal); if (this.now() >= deadline) throw discoveryTimeoutError(timeoutMs); tools.push(...result.tools.map((tool) => tool as McpToolLike)); - if (this.now() >= deadline) throw discoveryTimeoutError(timeoutMs); if (result.nextCursor === undefined) break;🤖 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 `@packages/agent-core/src/mcp/client.ts` around lines 238 - 278, Remove one of the duplicated this.now() >= deadline checks in listTools, retaining a single discovery deadline validation around the synchronous tools.push and pagination handling. Preserve the existing timeout behavior and error flow through discoveryTimeoutError.
371-383: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBounded stderr can split a multi-byte UTF-8 sequence.
Line 378 slices the buffer at a fixed byte count and then decodes. A truncated multi-byte character produces a replacement character in the log. Use the same UTF-8-safe truncation helper that the Skill projection code uses, if one is exported.
🤖 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 `@packages/agent-core/src/mcp/client.ts` around lines 371 - 383, Update `#attachBoundedStderr` to use the exported UTF-8-safe truncation helper used by the Skill projection code instead of slicing the encoded buffer directly at MAX_STDERR_LOG_BYTES. Preserve the existing stderr conversion, redaction, logger event, and serverName context while ensuring truncation never splits a multi-byte character.packages/protocol/src/types.test.ts (1)
181-195: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBind the MCP fixtures to their protocol types.
The fixtures use
as constwith no type relationship toMcpServerStatusorMcpServerInventoryResponse. The assertions only compare an object to its own JSON round trip, so this test passes even if the protocol types change or the fixture stops matching them. The new binding test at Line 63 already usessatisfies; apply the same here.💚 Proposed change
- const statuses = { + const statuses = { docs: { state: "ready", toolCount: 1, warningCount: 0, connectedAt: 123 }, local: { state: "disabled", updatedAt: 124 }, - } as const; - const inventory = { + } satisfies Record<string, McpServerStatus>; + const inventory = { servers: { docs: [{ serverName: "docs", name: "search", registryName: "mcp__docs__search", description: "Search docs" }], local: [], }, - } as const; + } satisfies McpServerInventoryResponse;Add
McpServerStatusandMcpServerInventoryResponseto the type imports.🤖 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 `@packages/protocol/src/types.test.ts` around lines 181 - 195, Bind the statuses and inventory fixtures in the test using the protocol types: add McpServerStatus and McpServerInventoryResponse to the type imports, then apply satisfies McpServerStatus and satisfies McpServerInventoryResponse alongside as const. Keep the existing serializeRoundTrip assertions unchanged.apps/web/src/components/features/ChatInput.tsx (1)
551-565: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep non-option content out of the
listboxcontainer.In the loading state and the "No matching Skills." state, the container keeps
role="listbox"while its only children are<p role="status">elements. Alistboxmust ownoption(orgroup) elements. Screen readers can drop or misreport these paragraphs, and an empty listbox is announced as an empty list.Use the same treatment you already apply to the failed state: render
role="group"when no options exist.♿ Proposed change
- role={skillUseInput !== null && skillInventoryState === "failed" ? "group" : "listbox"} + role={skillUseInput !== null && slashOptionCount === 0 ? "group" : "listbox"}🤖 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 `@apps/web/src/components/features/ChatInput.tsx` around lines 551 - 565, Update the role expression on the slash menu container in the ChatInput render so loading and empty “No matching Skills.” states use role="group" when no selectable options exist, matching the existing failed-state treatment. Preserve role="listbox" for states with actual options, and keep the existing option rendering unchanged.Source: Coding guidelines
apps/web/src/components/features/settings-panels.tsx (1)
483-493: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider narrowing the inventory effect dependency.
The effect depends on the
serversstatus record identity. Each MCP status event replaces that object and triggers a newgetMcpInventory()request. During a connect/discovery sequence this produces repeated fetches. A derived key such as a joinedname:statestring reduces the request count.🤖 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 `@apps/web/src/components/features/settings-panels.tsx` around lines 483 - 493, Update the inventory-loading useEffect dependency around getMcpInventory so it no longer depends directly on the servers object identity. Derive a stable key from each server’s name and state (for example, joined name:state values) and depend on that key instead, while preserving the existing active, expectedRevision, and runtimeAvailable triggers and mounted guards.packages/agent-core/src/mcp/fixtures/stdio-server.ts (1)
6-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: share the fixture tool definitions with the HTTP fixture.
tool()and both request handlers duplicatepaginatedServer()inpackages/agent-core/src/mcp/transports.integration.test.ts(lines 29-53). Extract the tool list and handler registration into one exported helper, and keep only the transport bootstrap here. The two pagination fixtures then cannot drift.🤖 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 `@packages/agent-core/src/mcp/fixtures/stdio-server.ts` around lines 6 - 27, Extract the shared paginated tool definition and request-handler registration from the stdio fixture and the HTTP fixture’s paginatedServer() into one exported helper. Update both fixtures to call that helper while retaining only their transport-specific server bootstrap, preserving the existing pagination and tool-call behavior.packages/agent-core/src/commands/skill.ts (1)
25-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
SkillServicethrough the Skills barrel.Line 26 imports from
../skills/service. ImportSkillServicefrom../skillsinstead. This preserves the public module boundary.Proposed change
+import type { SkillService } from "../skills"; - readonly skillService: import("../skills/service").SkillService; + readonly skillService: SkillService;As per coding guidelines, use barrel exports through
index.tsfiles.🤖 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 `@packages/agent-core/src/commands/skill.ts` around lines 25 - 31, Update the validateSkillActivation input type to reference SkillService through the public ../skills barrel export instead of the internal ../skills/service module, preserving the existing type and behavior.Source: Coding guidelines
packages/agent-core/src/agents/factory-types.ts (1)
6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the tools barrel for
AnyToolDescriptor.Export
AnyToolDescriptorfrompackages/agent-core/src/tools/index.ts, then import it through../tools.🤖 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 `@packages/agent-core/src/agents/factory-types.ts` around lines 6 - 7, Export AnyToolDescriptor from the tools barrel at tools/index.ts, then update the import in factory-types.ts to reference ../tools instead of ../tools/types.Source: Coding guidelines
packages/agent-core/src/skills/package-reader.ts (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
node:cryptowithBun.CryptoHasherin both new hashing sites. Both files importcreateHashfromnode:cryptoalthough Bun provides an equivalent, andpackages/agent-core/src/prompt/compiler.tsalready usesnew Bun.CryptoHasher("sha256").
packages/agent-core/src/skills/package-reader.ts#L1-L1: drop thenode:cryptoimport and build the snapshot digest withBun.CryptoHasher, updatingdigestSnapshot,updateDigestField, andupdateDigestLengthsignatures.packages/agent-core/src/skills/service.ts#L1-L1: drop thenode:cryptoimport and computestableDigestwithBun.CryptoHasher.As per coding guidelines: "Use Bun as the runtime and prefer Bun-native APIs over
node:*imports; usenode:*only when Bun has no suitable alternative."🤖 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 `@packages/agent-core/src/skills/package-reader.ts` at line 1, Replace the node:crypto createHash usage at packages/agent-core/src/skills/package-reader.ts#L1-L1 and packages/agent-core/src/skills/service.ts#L1-L1 with Bun.CryptoHasher. In package-reader.ts, update digestSnapshot, updateDigestField, and updateDigestLength to use the Bun hasher while preserving snapshot digest behavior; in service.ts, compute stableDigest with Bun.CryptoHasher and remove both node:crypto imports.Source: Coding guidelines
229-244: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
SkillPackageResourcePathErrorfor all path validation failures.Lines 229-236 now throw
SkillPackageResourcePathError, but the depth check and theSKILL.mdcheck still throw a plainError. Callers cannot classify path failures reliably. Convert the remaining two throws.♻️ Proposed change
if (segments.length > SKILL_RESOURCE_MAX_DEPTH) { - throw new Error(`Skill resource depth exceeds ${SKILL_RESOURCE_MAX_DEPTH}`); + throw new SkillPackageResourcePathError(`Skill resource depth exceeds ${SKILL_RESOURCE_MAX_DEPTH}`); } if (segments[0]?.toLowerCase() === SKILL_ENTRY_FILE.toLowerCase()) { - throw new Error("SKILL.md is the package entry and cannot be a resource directory"); + throw new SkillPackageResourcePathError("SKILL.md is the package entry and cannot be a resource directory"); }🤖 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 `@packages/agent-core/src/skills/package-reader.ts` around lines 229 - 244, Update validateResourcePath so the depth-limit and SKILL.md entry-file checks also throw SkillPackageResourcePathError, matching the existing error type used for all other invalid resource paths. Preserve the current validation conditions and messages.packages/agent-core/src/skills/projection.ts (1)
16-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce the projection fit search from O(n²) to O(n).
Each iteration re-renders and re-measures every included entry. For a large catalog the loop runs once per omitted entry, so cost grows quadratically with the number of skills. Compute each line once, accumulate prefix byte lengths, then select the largest prefix that fits the budget plus the omission line.
🤖 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 `@packages/agent-core/src/skills/projection.ts` around lines 16 - 27, Update the projection fit logic around renderProjection to avoid re-rendering prefixes in the includedCount loop. Render each normalized entry once, compute cumulative UTF-8 byte lengths, and use those prefix totals with the omission-line size to select the largest fitting prefix within maxBytes; preserve the existing frozen result shape and overflow error.packages/agent-core/src/skills/service.ts (1)
152-199: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce catalog build cost.
Two problems compound here.
sourceNames.includes(name)runs a linear scan for every name and every source, so candidate assembly is quadratic in catalog size.#discoverCandidateis then awaited serially, so every candidateSKILL.mdread blocks the next one.catalogForAgentruns on the prompt-compile path and on everyskill_listcall.Build one
Setper source before the name loop, and resolve candidate discovery with bounded concurrency instead of a fully serial loop.🤖 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 `@packages/agent-core/src/skills/service.ts` around lines 152 - 199, Optimize catalog construction in the surrounding catalog-building method by precomputing a Set of names for each entry in namesBySource, then use Set.has(name) instead of sourceNames.includes(name). Replace the serial awaits around `#discoverCandidate` with bounded-concurrency processing while preserving candidate order, winner selection, inventory entries, diagnostics, and entries output.
🤖 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 `@apps/web/src/components/features/settings-panels.tsx`:
- Line 601: Update the stdio arguments handler in the settings panel’s textarea
onChange flow to remove empty lines after splitting the input by newline before
assigning draft.args. Preserve undefined for entirely empty input, and ensure
stored arguments contain only non-empty lines.
In `@apps/web/src/components/features/SettingsDialog.tsx`:
- Around line 50-58: Update the snapshot-change useEffect to clear
preserveSaveErrorRevision.current in both branches, including when the current
revision matches; ensure no recorded revision marker survives after processing
any snapshot change.
In `@packages/agent-core/src/config/runtime-secret-literals.ts`:
- Around line 88-93: Update collectRuntimeSecretLiterals around the HTTP server
branch so server.url is excluded from runtime secret literal collection, keeping
header and environment collection unchanged; add a regression test confirming an
HTTP URL such as http:a follows the same save and runtime validation behavior.
In `@packages/agent-core/src/execution/session-execution-manager.ts`:
- Line 439: Update deleteSession to remove every deleted session ID from
`#executionSkillSnapshots` before completing session cleanup. Ensure suspended and
non-terminal executions are evicted even when terminalization methods are not
invoked, while preserving the existing agent, store, and directory deletion
behavior.
In `@packages/agent-core/src/mcp/runtime-service.ts`:
- Around line 365-368: Update the losing-candidate branch in `#connectAndPublish`
so candidate.handle.retire() is wrapped with the same `#logCloseFailure` handling
used by the error path, preventing retire failures from rejecting reconnect
operations for superseded connections.
- Around line 185-205: Update testServer and its in-flight test state to enforce
a shared maximum number of concurrent draft tests, rejecting new drafts once the
limit is reached while preserving the existing duplicate testKey guard. Ensure
the count is incremented only for accepted tests and decremented in the existing
promise.finally cleanup so slots are released on every completion path.
- Around line 597-602: Update createServerRedactionPolicy to include only
secret-bearing values extracted from config.args for stdio servers, while
preserving header/env handling for HTTP and other transports. Filter argument
candidates through the existing SecretRedactionPolicy eligibility rules so short
ordinary flags such as --stdio are excluded and longer non-secret arguments are
not redacted; add coverage for both cases.
In `@packages/agent-core/src/mcp/transports.integration.test.ts`:
- Around line 162-179: Replace the immediate sessionCloseCount assertions after
runtime.reconnect("local") and runtime.apply(...) with expectHttpSessionClose
waits for the retired first handle, matching the existing usage later in the
test. Keep the active-session and inventory assertions, and retain the
pre-operation close counts so the helper verifies exactly one additional close
on each path.
In `@packages/agent-core/src/runtime-mcp.test.ts`:
- Line 178: Remove the duplicate calls declaration in the test setup, retain
only one boundaries declaration near the affected test, and keep a single
streamText property in the object literal. Update the relevant test code in
runtime-mcp.test.ts without changing the remaining setup or behavior.
In `@packages/agent-core/src/runtime-skill-command.test.ts`:
- Around line 85-146: Install the test LLM adapter before the test accepts any
session messages, using the existing
setLlmAdapterForTest()/installTestLlmAdapter() pattern from main.test.ts. Update
the runtime test setup around createRuntime and acceptSessionMessage so
execution uses the stubbed adapter and cannot make live model calls; preserve
the existing assertions and cleanup.
In `@packages/agent-core/src/runtime.ts`:
- Around line 2282-2290: Restrict the fallback lookup in testMcpServer to own
keys of BUILTIN_MCP_SERVERS by using Object.hasOwn or an explicit built-in name
membership check before indexing. Preserve draft server precedence and continue
throwing the existing configuration error when neither source contains
serverName.
In `@packages/agent-core/src/skills/projection.test.ts`:
- Around line 41-42: Update the omitted-skills rendering logic and its assertion
in the projection test so omittedCount === 1 uses singular “Skill omitted,”
while counts greater than one retain “Skills omitted.”
In `@packages/agent-core/src/store/helpers.ts`:
- Around line 922-931: Update the available schema’s renderedText field
alongside byteLength so persisted text is directly limited to the 8,000-byte
maximum, using the existing boundedUtf8String helper and preserving the current
byteLength validation.
- Around line 280-285: Update the executionSkills schema to pass the imported
SKILL_SOURCE_TIERS constant to z.enum instead of duplicating the five tier
values inline. Keep the existing name, digest, and resolutionRoot validations
unchanged, ensuring execution records and prompt traces share the same
source-tier definition.
In `@packages/agent-core/src/tools/builtins/skill-read.test.ts`:
- Line 14: Update the SkillService setup in the skill-read test suite to pass an
isolated temporary userAgentsSkillsRoot under tmpRoot, ensuring skill_read
resolves only from the test directory. Keep the unknown-skill test independent
of any skills installed in $HOME/.agents/skills.
In `@packages/agent-core/src/tools/registry.ts`:
- Around line 119-132: Extract the descriptor invariant validation currently
performed by register() into a reusable helper, then invoke it from
executeResolved() before calling `#execute`(). Ensure resolved descriptors with
traits.destructive set but no permissions are rejected consistently with
registered descriptors, while preserving the existing tool-name mismatch
handling.
In `@packages/protocol/src/guards.ts`:
- Around line 219-227: Update isExecutionSkillBinding to validate digest as a
64-character hexadecimal string rather than any string, while preserving the
existing field and source checks. Add rejection coverage for short and malformed
digest values.
In `@packages/protocol/src/types.ts`:
- Around line 973-997: Update README.md to document the MCP configuration schema
represented by ConfigMcpServerSettings, including required type and enabled
fields, HTTP and stdio-specific properties, the connectTimeoutMs,
discoveryTimeoutMs, and callTimeoutMs fields replacing timeout, and the
mcp.disabledBuiltins setting.
---
Outside diff comments:
In `@packages/agent-core/src/mcp/errors.ts`:
- Around line 40-56: Update the constructors for the MCP connection and tool
execution error classes so their messages use the provided reason: report
cancellation for "aborted", timeout for "timeout", and retain failure wording
for "failed". Preserve the existing server, tool, and cause details while
ensuring tool-adapter result messages remain consistent with their reason codes.
---
Nitpick comments:
In `@apps/web/src/components/features/ChatInput.tsx`:
- Around line 551-565: Update the role expression on the slash menu container in
the ChatInput render so loading and empty “No matching Skills.” states use
role="group" when no selectable options exist, matching the existing
failed-state treatment. Preserve role="listbox" for states with actual options,
and keep the existing option rendering unchanged.
In `@apps/web/src/components/features/settings-panels.tsx`:
- Around line 483-493: Update the inventory-loading useEffect dependency around
getMcpInventory so it no longer depends directly on the servers object identity.
Derive a stable key from each server’s name and state (for example, joined
name:state values) and depend on that key instead, while preserving the existing
active, expectedRevision, and runtimeAvailable triggers and mounted guards.
In `@packages/agent-core/src/agents/factory-types.ts`:
- Around line 6-7: Export AnyToolDescriptor from the tools barrel at
tools/index.ts, then update the import in factory-types.ts to reference ../tools
instead of ../tools/types.
In `@packages/agent-core/src/commands/skill.ts`:
- Around line 25-31: Update the validateSkillActivation input type to reference
SkillService through the public ../skills barrel export instead of the internal
../skills/service module, preserving the existing type and behavior.
In `@packages/agent-core/src/mcp/client.ts`:
- Around line 238-278: Remove one of the duplicated this.now() >= deadline
checks in listTools, retaining a single discovery deadline validation around the
synchronous tools.push and pagination handling. Preserve the existing timeout
behavior and error flow through discoveryTimeoutError.
- Around line 371-383: Update `#attachBoundedStderr` to use the exported
UTF-8-safe truncation helper used by the Skill projection code instead of
slicing the encoded buffer directly at MAX_STDERR_LOG_BYTES. Preserve the
existing stderr conversion, redaction, logger event, and serverName context
while ensuring truncation never splits a multi-byte character.
In `@packages/agent-core/src/mcp/fixtures/stdio-server.ts`:
- Around line 6-27: Extract the shared paginated tool definition and
request-handler registration from the stdio fixture and the HTTP fixture’s
paginatedServer() into one exported helper. Update both fixtures to call that
helper while retaining only their transport-specific server bootstrap,
preserving the existing pagination and tool-call behavior.
In `@packages/agent-core/src/mcp/naming.ts`:
- Line 1: Replace the node:crypto createHash usage in the naming module with
Bun.CryptoHasher, preserving the existing SHA-256 update sequence and
digest("hex") output behavior.
In `@packages/agent-core/src/mcp/tool-adapter.ts`:
- Line 19: Document that mcpToolInputSchema intentionally accepts arbitrary
object arguments because MCP performs validation before dispatch, while
aiInputSchema only describes model inputs. Add a regression test covering
invalid arguments through ToolRegistry.#prepareInput and confirm they are passed
to the MCP tool without local schema rejection.
In `@packages/agent-core/src/skills/package-reader.ts`:
- Line 1: Replace the node:crypto createHash usage at
packages/agent-core/src/skills/package-reader.ts#L1-L1 and
packages/agent-core/src/skills/service.ts#L1-L1 with Bun.CryptoHasher. In
package-reader.ts, update digestSnapshot, updateDigestField, and
updateDigestLength to use the Bun hasher while preserving snapshot digest
behavior; in service.ts, compute stableDigest with Bun.CryptoHasher and remove
both node:crypto imports.
- Around line 229-244: Update validateResourcePath so the depth-limit and
SKILL.md entry-file checks also throw SkillPackageResourcePathError, matching
the existing error type used for all other invalid resource paths. Preserve the
current validation conditions and messages.
In `@packages/agent-core/src/skills/projection.ts`:
- Around line 16-27: Update the projection fit logic around renderProjection to
avoid re-rendering prefixes in the includedCount loop. Render each normalized
entry once, compute cumulative UTF-8 byte lengths, and use those prefix totals
with the omission-line size to select the largest fitting prefix within
maxBytes; preserve the existing frozen result shape and overflow error.
In `@packages/agent-core/src/skills/service.ts`:
- Around line 152-199: Optimize catalog construction in the surrounding
catalog-building method by precomputing a Set of names for each entry in
namesBySource, then use Set.has(name) instead of sourceNames.includes(name).
Replace the serial awaits around `#discoverCandidate` with bounded-concurrency
processing while preserving candidate order, winner selection, inventory
entries, diagnostics, and entries output.
In `@packages/protocol/src/types.test.ts`:
- Around line 181-195: Bind the statuses and inventory fixtures in the test
using the protocol types: add McpServerStatus and McpServerInventoryResponse to
the type imports, then apply satisfies McpServerStatus and satisfies
McpServerInventoryResponse alongside as const. Keep the existing
serializeRoundTrip assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 71239106-38b9-434c-b4a1-da80979a3940
📒 Files selected for processing (168)
AGENTS.mdCHANGELOG.mdapps/server/src/app.test.tsapps/server/src/app.tsapps/server/src/errors.tsapps/server/src/routes/attachments.test.tsapps/server/src/routes/compression.test.tsapps/server/src/routes/config.test.tsapps/server/src/routes/files.test.tsapps/server/src/routes/mcp.test.tsapps/server/src/routes/mcp.tsapps/server/src/routes/messages.test.tsapps/server/src/routes/projects.test.tsapps/server/src/routes/sessions.test.tsapps/server/src/routes/skills.test.tsapps/server/src/routes/skills.tsapps/server/src/server-host.test.tsapps/server/src/server-host.tsapps/web/src/api/config.test.tsapps/web/src/api/config.tsapps/web/src/api/mcp.test.tsapps/web/src/api/mcp.tsapps/web/src/api/skills.test.tsapps/web/src/api/skills.tsapps/web/src/components/composite/ExecutionWorkstream.interaction.tsxapps/web/src/components/features/ChatHeader.test.tsxapps/web/src/components/features/ChatInput.test.tsxapps/web/src/components/features/ChatInput.tsxapps/web/src/components/features/ComposerQueueList.interaction.tsxapps/web/src/components/features/SessionComposerDock.interaction.tsxapps/web/src/components/features/SettingsDialog.interaction.tsxapps/web/src/components/features/SettingsDialog.test.tsxapps/web/src/components/features/SettingsDialog.tsxapps/web/src/components/features/TodoProgressButton.interaction.tsxapps/web/src/components/features/settings-helpers.tsapps/web/src/components/features/settings-panels.tsxapps/web/src/components/ui/Dialog.test.tsxapps/web/src/components/ui/Dialog.tsxapps/web/src/context/global-sse.test.tsxapps/web/src/context/settings-modal.tsxapps/web/src/lib/execution-status-presentation.test.tsapps/web/src/lib/execution-workstream.test.tsapps/web/src/routes/session.test.tsxapps/web/src/store/mcp-status-store.test.tsapps/web/src/store/session-store.test.tsconfig.example.jsondesign-system/pages/settings.mddocs/architecture.mddocs/configuration.mddocs/goals/mcp-skill-control-plane-hard-cut-plan-goal.mddocs/goals/mcp-skill-control-plane-hard-cut-progress.mddocs/integrations.mdpackages/agent-core/src/__arch__/architecture.test.tspackages/agent-core/src/__arch__/tool-output-boundaries.test.tspackages/agent-core/src/__arch__/tool-output-policy-matrix.test.tspackages/agent-core/src/agents/configured-agent-mcp.test.tspackages/agent-core/src/agents/configured-agent.test.tspackages/agent-core/src/agents/configured-agent.tspackages/agent-core/src/agents/definitions/analyst.tspackages/agent-core/src/agents/definitions/build.tspackages/agent-core/src/agents/definitions/definitions.test.tspackages/agent-core/src/agents/definitions/discussion.tspackages/agent-core/src/agents/definitions/explore.tspackages/agent-core/src/agents/definitions/lead.tspackages/agent-core/src/agents/definitions/librarian.tspackages/agent-core/src/agents/factory-types.tspackages/agent-core/src/agents/factory.test.tspackages/agent-core/src/agents/factory.tspackages/agent-core/src/agents/query/loop.test.tspackages/agent-core/src/agents/query/loop.tspackages/agent-core/src/agents/query/provider-secret-redaction.integration.test.tspackages/agent-core/src/agents/query/recovery.test.tspackages/agent-core/src/agents/query/types.tspackages/agent-core/src/agents/session-agent-manager.test.tspackages/agent-core/src/agents/session-agent-manager.tspackages/agent-core/src/agents/types.tspackages/agent-core/src/attachments/read-paths.test.tspackages/agent-core/src/background/tasks/title-generation.test.tspackages/agent-core/src/commands/skill.test.tspackages/agent-core/src/commands/skill.tspackages/agent-core/src/commands/types.tspackages/agent-core/src/config/index.tspackages/agent-core/src/config/mcp.test.tspackages/agent-core/src/config/mcp.tspackages/agent-core/src/config/runtime-secret-literals.test.tspackages/agent-core/src/config/runtime-secret-literals.tspackages/agent-core/src/config/server-config-service.test.tspackages/agent-core/src/config/server-config-service.tspackages/agent-core/src/events/session-event-bridge.test.tspackages/agent-core/src/execution/session-execution-manager.test.tspackages/agent-core/src/execution/session-execution-manager.tspackages/agent-core/src/execution/session-tool-batch-scheduler.test.tspackages/agent-core/src/execution/session-tool-batch-scheduler.tspackages/agent-core/src/index.tspackages/agent-core/src/lead-architecture-flows.integration.test.tspackages/agent-core/src/main.test.tspackages/agent-core/src/mcp/builtin-servers.tspackages/agent-core/src/mcp/client.test.tspackages/agent-core/src/mcp/client.tspackages/agent-core/src/mcp/errors.tspackages/agent-core/src/mcp/fixtures/stdio-server.tspackages/agent-core/src/mcp/index.tspackages/agent-core/src/mcp/manager.test.tspackages/agent-core/src/mcp/manager.tspackages/agent-core/src/mcp/naming.test.tspackages/agent-core/src/mcp/naming.tspackages/agent-core/src/mcp/runtime-service.test.tspackages/agent-core/src/mcp/runtime-service.tspackages/agent-core/src/mcp/tool-adapter.test.tspackages/agent-core/src/mcp/tool-adapter.tspackages/agent-core/src/mcp/transports.integration.test.tspackages/agent-core/src/prompt/compiler.test.tspackages/agent-core/src/prompt/compiler.tspackages/agent-core/src/prompt/live-eval.tspackages/agent-core/src/prompt/types.tspackages/agent-core/src/runtime-automations.integration.test.tspackages/agent-core/src/runtime-automations.test.tspackages/agent-core/src/runtime-data/service.test.tspackages/agent-core/src/runtime-mcp.test.tspackages/agent-core/src/runtime-skill-command.test.tspackages/agent-core/src/runtime.tspackages/agent-core/src/session-input/model-selection.test.tspackages/agent-core/src/session-input/service.test.tspackages/agent-core/src/session-input/service.tspackages/agent-core/src/skills/index.tspackages/agent-core/src/skills/package-reader.test.tspackages/agent-core/src/skills/package-reader.tspackages/agent-core/src/skills/pagination.test.tspackages/agent-core/src/skills/pagination.tspackages/agent-core/src/skills/projection.test.tspackages/agent-core/src/skills/projection.tspackages/agent-core/src/skills/service.test.tspackages/agent-core/src/skills/service.tspackages/agent-core/src/skills/types.tspackages/agent-core/src/store/helpers.test.tspackages/agent-core/src/store/helpers.tspackages/agent-core/src/store/logical-execution.test.tspackages/agent-core/src/store/message-phase-hard-cut.test.tspackages/agent-core/src/store/session-store-manager.test.tspackages/agent-core/src/store/store.test.tspackages/agent-core/src/testing/test-execution-fixtures.tspackages/agent-core/src/testing/test-mcp-runtime.tspackages/agent-core/src/tool-output/artifact-lifecycle.test.tspackages/agent-core/src/tool-output/live-bash.integration.test.tspackages/agent-core/src/tools/builtins/skill-list.test.tspackages/agent-core/src/tools/builtins/skill-list.tspackages/agent-core/src/tools/builtins/skill-read.test.tspackages/agent-core/src/tools/builtins/skill-read.tspackages/agent-core/src/tools/concurrency/partition.test.tspackages/agent-core/src/tools/concurrency/partition.tspackages/agent-core/src/tools/errors.test.tspackages/agent-core/src/tools/errors.tspackages/agent-core/src/tools/index.tspackages/agent-core/src/tools/permission/index.tspackages/agent-core/src/tools/permission/mcp.test.tspackages/agent-core/src/tools/permission/mcp.tspackages/agent-core/src/tools/registry.test.tspackages/agent-core/src/tools/registry.tspackages/agent-core/src/tools/types.tspackages/protocol/src/execution.test.tspackages/protocol/src/execution.tspackages/protocol/src/guards.test.tspackages/protocol/src/guards.tspackages/protocol/src/message-phase-hard-cut.test.tspackages/protocol/src/reduce.test.tspackages/protocol/src/reduce.tspackages/protocol/src/types.test.tspackages/protocol/src/types.ts
💤 Files with no reviewable changes (9)
- apps/server/src/routes/projects.test.ts
- packages/agent-core/src/tools/permission/mcp.ts
- packages/agent-core/src/tools/index.ts
- apps/server/src/routes/sessions.test.ts
- apps/server/src/routes/files.test.ts
- packages/agent-core/src/tools/permission/index.ts
- packages/agent-core/src/tools/permission/mcp.test.ts
- packages/agent-core/src/mcp/manager.ts
- packages/agent-core/src/mcp/manager.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 168 files
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
Owner
Author
|
@cubic-dev-ai review this |
Owner
Author
|
@coderabbitai review |
@boh5 I have started the AI code review. It will take a few minutes to complete. |
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/components/features/settings-panels.tsx (2)
615-625: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve secrets when the transport changes.
replaceTransportoverwrites the server object and drops HTTPheadersor STDIOenv. A saved secret placeholder can therefore be deleted when the user changes transport, without an explicit delete action.Block transport changes while
nameLockedis true. Require the user to clear or replace preserved secrets first.Proposed fix
- <Field label="Transport"><select className={selectClass} value={server.type} + <Field label="Transport"><select disabled={nameLocked} className={selectClass} value={server.type}🤖 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 `@apps/web/src/components/features/settings-panels.tsx` around lines 615 - 625, Update replaceTransport to prevent transport changes when nameLocked is true, preserving the current configuration and requiring secrets to be cleared or replaced first. For unlocked servers, retain the existing transport-switch behavior while preserving the relevant HTTP headers or STDIO env values instead of dropping them.
511-514: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInvalidate an in-flight draft test when
configchanges.When the user edits
configduring a draft test, lines 511-514 clear the displayed result. The prior request can then complete and restore tools and warnings for the old draft at lines 577-579.Abort and remove draft-test controllers when
configchanges. Before writing a result, confirm that its controller is still the current controller fortestKey.Proposed fix
useEffect(() => { + for (const controller of draftTestControllersRef.current.values()) controller.abort(); + draftTestControllersRef.current.clear(); setTestResults({}); setActionErrors({}); }, [config]);const result = await testMcpDraft(name, { expectedRevision, config }, { signal: controller.signal }); +if (draftTestControllersRef.current.get(testKey) !== controller) return; setTestResults((current) => ({ ...current, [name]: result.tools }));Also applies to: 574-585
🤖 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 `@apps/web/src/components/features/settings-panels.tsx` around lines 511 - 514, Update the draft-test lifecycle around the config-reset useEffect and the test-result completion logic: abort and remove every in-flight draft-test controller when config changes, then before applying a completed result verify its controller is still the current controller for that testKey. Preserve clearing testResults and actionErrors while preventing stale requests from restoring old tools or warnings.docs/configuration.md (2)
58-63: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDocument the legacy MCP rejection explicitly.
The MCP format is a hard cut. Entries without
typeandenabledare rejected rather than migrated. Add a direct migration warning so existing users do not discover this only after configuration validation fails.Based on PR objectives, MCP configurations must explicitly include
typeandenabled; the legacy format is rejected.🤖 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 `@docs/configuration.md` around lines 58 - 63, Update the MCP configuration documentation near the user-server schema to add a direct migration warning that legacy entries lacking type and enabled are rejected, not migrated. State that every MCP server must explicitly define both fields and existing configurations must be updated before validation succeeds.
58-78: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftPreserve historical documentation files.
docs/configuration.mdmatches thedocs/**rule. Restore its original content and path, then place this new configuration material in an approved current-documentation location or obtain an explicit exception.As per coding guidelines, files under
docs/**are historical work records; preserve their original content and paths.🤖 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 `@docs/configuration.md` around lines 58 - 78, Restore docs/configuration.md to its original historical content without adding the new MCP configuration section. Move that material to an approved current-documentation location, preserving the existing documentation structure, or obtain an explicit exception before modifying the historical file.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@apps/web/src/components/features/settings-panels.tsx`:
- Around line 615-625: Update replaceTransport to prevent transport changes when
nameLocked is true, preserving the current configuration and requiring secrets
to be cleared or replaced first. For unlocked servers, retain the existing
transport-switch behavior while preserving the relevant HTTP headers or STDIO
env values instead of dropping them.
- Around line 511-514: Update the draft-test lifecycle around the config-reset
useEffect and the test-result completion logic: abort and remove every in-flight
draft-test controller when config changes, then before applying a completed
result verify its controller is still the current controller for that testKey.
Preserve clearing testResults and actionErrors while preventing stale requests
from restoring old tools or warnings.
In `@docs/configuration.md`:
- Around line 58-63: Update the MCP configuration documentation near the
user-server schema to add a direct migration warning that legacy entries lacking
type and enabled are rejected, not migrated. State that every MCP server must
explicitly define both fields and existing configurations must be updated before
validation succeeds.
- Around line 58-78: Restore docs/configuration.md to its original historical
content without adding the new MCP configuration section. Move that material to
an approved current-documentation location, preserving the existing
documentation structure, or obtain an explicit exception before modifying the
historical file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d7cfd38-9dea-4935-94ba-99cf3a00a7f5
📒 Files selected for processing (96)
AGENTS.mdREADME.mdapps/server/src/app.test.tsapps/server/src/app.tsapps/server/src/errors.tsapps/server/src/routes/mcp.test.tsapps/server/src/routes/mcp.tsapps/web/src/api/config.test.tsapps/web/src/api/mcp.test.tsapps/web/src/api/mcp.tsapps/web/src/components/composite/ExecutionWorkstream.interaction.tsxapps/web/src/components/features/ChatHeader.test.tsxapps/web/src/components/features/ChatInput.tsxapps/web/src/components/features/SettingsDialog.interaction.tsxapps/web/src/components/features/SettingsDialog.test.tsxapps/web/src/components/features/SettingsDialog.tsxapps/web/src/components/features/TodoProgressButton.interaction.tsxapps/web/src/components/features/settings-helpers.tsapps/web/src/components/features/settings-panels.tsxapps/web/src/lib/execution-status-presentation.test.tsapps/web/src/routes/session.test.tsxapps/web/src/store/session-store.test.tsconfig.example.jsondocs/configuration.mddocs/goals/mcp-skill-control-plane-hard-cut-progress.mdpackages/agent-core/src/__arch__/architecture.test.tspackages/agent-core/src/agents/configured-agent.test.tspackages/agent-core/src/agents/configured-agent.tspackages/agent-core/src/agents/definitions/analyst.tspackages/agent-core/src/agents/definitions/build.tspackages/agent-core/src/agents/definitions/discussion.tspackages/agent-core/src/agents/definitions/explore.tspackages/agent-core/src/agents/definitions/lead.tspackages/agent-core/src/agents/definitions/librarian.tspackages/agent-core/src/agents/factory-types.tspackages/agent-core/src/agents/factory.test.tspackages/agent-core/src/agents/factory.tspackages/agent-core/src/agents/query/loop.test.tspackages/agent-core/src/agents/query/provider-secret-redaction.integration.test.tspackages/agent-core/src/agents/query/recovery.test.tspackages/agent-core/src/agents/session-agent-manager.test.tspackages/agent-core/src/agents/session-agent-manager.tspackages/agent-core/src/agents/types.tspackages/agent-core/src/background/tasks/title-generation.test.tspackages/agent-core/src/config/index.tspackages/agent-core/src/config/mcp.test.tspackages/agent-core/src/config/runtime-secret-literals.test.tspackages/agent-core/src/config/runtime-secret-literals.tspackages/agent-core/src/config/server-config-service.test.tspackages/agent-core/src/config/server-config-service.tspackages/agent-core/src/execution/session-execution-manager.test.tspackages/agent-core/src/execution/session-execution-manager.tspackages/agent-core/src/execution/session-tool-batch-scheduler.test.tspackages/agent-core/src/execution/session-tool-batch-scheduler.tspackages/agent-core/src/index.tspackages/agent-core/src/main.test.tspackages/agent-core/src/mcp/client.test.tspackages/agent-core/src/mcp/client.tspackages/agent-core/src/mcp/errors.tspackages/agent-core/src/mcp/runtime-service.test.tspackages/agent-core/src/mcp/runtime-service.tspackages/agent-core/src/mcp/tool-adapter.test.tspackages/agent-core/src/mcp/tool-adapter.tspackages/agent-core/src/mcp/transports.integration.test.tspackages/agent-core/src/memory/idle-coordinator.test.tspackages/agent-core/src/memory/learning-input.test.tspackages/agent-core/src/prompt/compiler.tspackages/agent-core/src/runtime-skill-command.test.tspackages/agent-core/src/runtime.tspackages/agent-core/src/session-input/service.test.tspackages/agent-core/src/session-input/service.tspackages/agent-core/src/skills/package-reader.tspackages/agent-core/src/skills/pagination.test.tspackages/agent-core/src/skills/pagination.tspackages/agent-core/src/skills/projection.test.tspackages/agent-core/src/skills/projection.tspackages/agent-core/src/skills/service.test.tspackages/agent-core/src/skills/types.tspackages/agent-core/src/store/helpers.test.tspackages/agent-core/src/store/helpers.tspackages/agent-core/src/store/logical-execution.test.tspackages/agent-core/src/store/memory-learning.test.tspackages/agent-core/src/store/message-phase-hard-cut.test.tspackages/agent-core/src/store/session-store-manager.test.tspackages/agent-core/src/store/store.test.tspackages/agent-core/src/testing/test-execution-fixtures.tspackages/agent-core/src/testing/test-mcp-runtime.tspackages/agent-core/src/tools/builtins/skill-read.test.tspackages/protocol/src/execution.test.tspackages/protocol/src/guards.test.tspackages/protocol/src/guards.tspackages/protocol/src/message-phase-hard-cut.test.tspackages/protocol/src/reduce.test.tspackages/protocol/src/reduce.tspackages/protocol/src/types.test.tspackages/protocol/src/types.ts
💤 Files with no reviewable changes (12)
- packages/agent-core/src/agents/definitions/analyst.ts
- packages/agent-core/src/agents/definitions/explore.ts
- packages/agent-core/src/agents/definitions/build.ts
- packages/agent-core/src/agents/definitions/discussion.ts
- packages/agent-core/src/agents/definitions/librarian.ts
- packages/agent-core/src/agents/definitions/lead.ts
- packages/agent-core/src/agents/factory.ts
- packages/agent-core/src/agents/session-agent-manager.ts
- packages/agent-core/src/memory/idle-coordinator.test.ts
- packages/agent-core/src/agents/factory-types.ts
- packages/agent-core/src/config/runtime-secret-literals.test.ts
- packages/agent-core/src/agents/factory.test.ts
🚧 Files skipped from review as they are similar to previous changes (71)
- packages/agent-core/src/agents/query/provider-secret-redaction.integration.test.ts
- packages/agent-core/src/store/message-phase-hard-cut.test.ts
- config.example.json
- packages/agent-core/src/config/runtime-secret-literals.ts
- apps/web/src/api/mcp.test.ts
- packages/agent-core/src/store/session-store-manager.test.ts
- apps/server/src/errors.ts
- packages/agent-core/src/agents/session-agent-manager.test.ts
- apps/web/src/components/features/TodoProgressButton.interaction.tsx
- apps/web/src/components/features/settings-helpers.ts
- packages/agent-core/src/agents/query/recovery.test.ts
- apps/web/src/api/config.test.ts
- packages/agent-core/src/agents/query/loop.test.ts
- packages/agent-core/src/store/store.test.ts
- packages/protocol/src/reduce.ts
- apps/web/src/components/composite/ExecutionWorkstream.interaction.tsx
- apps/web/src/lib/execution-status-presentation.test.ts
- packages/agent-core/src/testing/test-mcp-runtime.ts
- packages/agent-core/src/index.ts
- packages/agent-core/src/testing/test-execution-fixtures.ts
- packages/protocol/src/message-phase-hard-cut.test.ts
- packages/agent-core/src/session-input/service.test.ts
- apps/server/src/app.ts
- packages/protocol/src/execution.test.ts
- packages/agent-core/src/skills/pagination.ts
- packages/agent-core/src/tools/builtins/skill-read.test.ts
- packages/agent-core/src/config/index.ts
- packages/agent-core/src/store/logical-execution.test.ts
- packages/agent-core/src/mcp/client.test.ts
- packages/agent-core/src/mcp/errors.ts
- AGENTS.md
- packages/agent-core/src/prompt/compiler.ts
- packages/protocol/src/guards.ts
- packages/agent-core/src/arch/architecture.test.ts
- packages/agent-core/src/store/helpers.ts
- apps/web/src/components/features/SettingsDialog.tsx
- packages/agent-core/src/skills/projection.ts
- packages/agent-core/src/skills/types.ts
- packages/agent-core/src/mcp/transports.integration.test.ts
- packages/agent-core/src/skills/projection.test.ts
- packages/protocol/src/types.test.ts
- packages/agent-core/src/execution/session-execution-manager.ts
- apps/web/src/components/features/ChatInput.tsx
- packages/agent-core/src/agents/configured-agent.test.ts
- packages/agent-core/src/execution/session-execution-manager.test.ts
- packages/protocol/src/guards.test.ts
- packages/agent-core/src/background/tasks/title-generation.test.ts
- packages/protocol/src/types.ts
- packages/agent-core/src/store/helpers.test.ts
- apps/web/src/components/features/SettingsDialog.interaction.tsx
- packages/agent-core/src/session-input/service.ts
- packages/agent-core/src/skills/pagination.test.ts
- packages/agent-core/src/execution/session-tool-batch-scheduler.test.ts
- apps/web/src/routes/session.test.tsx
- apps/web/src/components/features/SettingsDialog.test.tsx
- apps/web/src/components/features/ChatHeader.test.tsx
- packages/agent-core/src/mcp/client.ts
- apps/web/src/api/mcp.ts
- packages/agent-core/src/skills/package-reader.ts
- packages/agent-core/src/agents/configured-agent.ts
- packages/agent-core/src/agents/types.ts
- packages/agent-core/src/skills/service.test.ts
- apps/web/src/store/session-store.test.ts
- packages/agent-core/src/execution/session-tool-batch-scheduler.ts
- apps/server/src/app.test.ts
- packages/agent-core/src/runtime.ts
- packages/protocol/src/reduce.test.ts
- packages/agent-core/src/main.test.ts
- packages/agent-core/src/mcp/runtime-service.ts
- packages/agent-core/src/mcp/tool-adapter.ts
- packages/agent-core/src/config/server-config-service.ts
Owner
Author
|
Follow-up to the outside-diff review on 893e0a5:
Validated with Web typecheck, 50 Settings interaction tests, and diff-check. @coderabbitai review |
|
|
|
✏️ Learnings added
|
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Owner
Author
|
@cubic-dev-ai review this |
@boh5 I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 42,862 of the 40,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 22 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
Breaking change
MCP server configuration now requires an explicit type and enabled field. The legacy url, headers, and timeout-only shape is rejected with no compatibility fallback.
Validation
Review record
The implementation completed four independent review iterations. Fixes from those reviews include operation-level MCP discovery deadlines, cursor-cycle rejection, cross-project live-update coverage, complete real HTTP/STDIO lifecycle coverage, and graceful remote HTTP session termination.
Summary by CodeRabbit
/skill usewith search, keyboard navigation, validation, and execution-specific Skill selection.