refactor(runtime): compile configured capabilities - #233
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR moves harness selection and artifact availability into a compiled runtime configuration, then registers MCP tool groups from that compiled representation.
Confidence Score: 4/5The runtime composition is internally consistent, but the exported createMcpServer compatibility break should be fixed before merging. Existing package consumers can still import createMcpServer from the public entry point, but they can no longer supply the previously accepted ServerConfig without a compile failure or missing-field runtime failure. Files Needing Attention: src/server.ts, src/runtime-config.ts
|
| Filename | Overview |
|---|---|
| src/harness.ts | Compiles each legacy harness mode into instructions and a behavior-preserving set of tool groups. |
| src/runtime-config.ts | Adds the runtime compilation boundary for harness and artifact capability state. |
| src/server.ts | Registers compiled tool groups correctly, but changes the exported createMcpServer contract incompatibly. |
| src/runtime-config.test.ts | Covers all legacy harness mappings and artifact capability states. |
| src/server.test.ts | Updates internal test construction to compile ServerConfig before creating an MCP server. |
| package.json | Adds runtime configuration tests to the test script while retaining dist/server.js as the public package entry. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
SC[ServerConfig] --> CR[compileRuntime]
CR --> RH[Compiled harness tool groups]
CR --> AC[Artifact capability]
RH --> MS[createMcpServer]
AC --> MS
MS --> TR[Register configured MCP tools]
Reviews (1): Last reviewed commit: "refactor(runtime): remove superseded har..." | Re-trigger Greptile
| export function createMcpServer( | ||
| config: ServerConfig, | ||
| config: RuntimeConfig, |
There was a problem hiding this comment.
Exported server config contract breaks
When an existing package consumer calls the exported createMcpServer with the previously accepted ServerConfig, the function now requires derived runtimeHarness and artifactCapability fields, causing TypeScript compilation to fail or untyped consumers to fail when those fields are accessed during server creation.
Knowledge Base Used: MCP server and workspace API
|
Closing this stacked PR because the v1.1 configuration/runtime refactor is being collapsed into one review PR. The commits are preserved in the combined branch. |
|
Superseded by the combined v1.1 refactor PR #238. |
Harness and artifact policy is currently reconstructed at multiple server registration sites. This layer compiles configuration once into concrete harness tool groups, model instructions, and artifact availability so server assembly consumes resolved capabilities instead of asking which mode is active.
Stacked on #232.
Stack created with GitHub Stacks CLI • Give Feedback 💬