Skip to content

Commit ca00ebc

Browse files
committed
Enforce review_code tool constraints: only pasted/open editor code, diagnostic-only responses
- Updated tool descriptions to clarify code must be cut/pasted or from open editor - Added validation that code parameter is required (filePath is optional context only) - Clarified that files are NOT read from disk - Ensured responses contain only diagnostic information (no corrected code) - Added comprehensive tests for code source constraints and diagnostic-only responses - Updated documentation (CLAUDE.md, README) to reflect these constraints
1 parent 764402a commit ca00ebc

9 files changed

Lines changed: 560 additions & 9 deletions

File tree

CLAUDE.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,24 @@ export const MyService = Effect.gen(function* () {
1919
});
2020
```
2121

22-
#### 2. Reference a File
22+
#### 2. Reference an Open Editor File
2323
```
2424
Review this file for architectural issues: src/services/user.ts
2525
```
26+
*(Note: The code must be from an open editor file. Files are not read from disk.)*
27+
28+
**Important**: The tool only accepts code that is:
29+
- Cut and pasted into the prompt, OR
30+
- Provided from an open editor file
31+
32+
Files are **NOT** read from disk. Only diagnostic information is returned (no corrected code).
2633

2734
The tool will:
2835
- Analyze the code for Effect-TS anti-patterns
2936
- Return top 3 high-impact recommendations (free tier)
3037
- Show severity levels: 🔴 high, 🟡 medium, 🔵 low
31-
- Provide detailed explanations for each finding
38+
- Provide detailed diagnostic information (findings, recommendations, fix plans)
39+
- **NOT** return corrected code - only diagnostics
3240

3341
### Example Prompts
3442

@@ -48,10 +56,12 @@ The code review tool checks for:
4856

4957
### Free Tier Limits
5058

59+
- **Code source**: Code must be cut and pasted or from open editor (files not read from disk)
5160
- Max 100KB per file
5261
- Top 3 recommendations shown (sorted by severity)
5362
- `.ts` and `.tsx` files only
5463
- Results include upgrade messaging if more issues detected
64+
- **Response type**: Only diagnostic information (no corrected code)
5565

5666
### More Tools Available
5767

packages/mcp-server/src/mcp-production-client.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,13 @@ server.registerTool(
142142
server.registerTool(
143143
"review_code",
144144
{
145-
description: "Get AI-powered code review for Effect-TS code",
145+
description: "Get AI-powered architectural review and diagnostic recommendations for Effect code. Only accepts code that is cut and pasted into the prompt or provided from an open editor file. Returns diagnostic information only (no corrected code).",
146146
inputSchema: undefined as any,
147147
},
148148
async (args: { code: string; filePath?: string }): Promise<ToolResult> => {
149149
console.error("Tool called: review_code", args);
150+
// Note: This tool only accepts code that is cut and pasted or from open editor.
151+
// Files are NOT read from disk. Only diagnostics are returned.
150152
const result = await callProductionApi("/review-code", {
151153
code: args.code,
152154
filePath: args.filePath,

packages/mcp-server/src/schemas/tool-schemas.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ export const ToolSchemas = {
4848

4949
// Review Code Tool
5050
reviewCode: z.object({
51-
code: z.string().min(1).describe("Source code to review"),
52-
filePath: z.string().optional().describe("File path for context (e.g., 'src/services/user.ts')"),
53-
}).describe("Get AI-powered architectural review and recommendations for Effect code"),
51+
code: z.string().min(1).describe("Source code to review (must be cut and pasted from prompt or provided from open editor)"),
52+
filePath: z.string().optional().describe("File path for context only (e.g., 'src/services/user.ts'). Code must be provided via 'code' parameter - files are not read from disk."),
53+
}).describe("Get AI-powered architectural review and diagnostic recommendations for Effect code. Only accepts code that is cut and pasted into the prompt or provided from an open editor file. Returns diagnostic information only (no corrected code)."),
5454

5555
// Paid-tier schemas removed from MCP tool surface (HTTP API only)
5656
};

packages/mcp-server/src/services/review-code/README.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@ Free tier code review with architectural recommendations.
66

77
The `ReviewCodeService` provides high-fidelity architectural recommendations for Effect codebases with confidence scoring, fix plans, and guidance. Limited to top 3 findings in Free tier.
88

9+
**Important**: This service only accepts code that is:
10+
1. Cut and pasted into the prompt (code parameter)
11+
2. Provided from an open editor file (code parameter with optional filePath for context)
12+
13+
Files are **NOT** read from disk. The `filePath` parameter is only used for TypeScript file extension validation and context/metadata. Only diagnostic information is returned (no corrected code).
14+
915
## API
1016

1117
### Methods
1218

1319
#### `reviewCode(code: string, filePath?: string): Effect<ReviewCodeResult, FileSizeError | NonTypeScriptError | Error>`
1420

15-
Analyze TypeScript code and return review recommendations.
21+
Analyze TypeScript code and return diagnostic recommendations. Code must be provided directly - files are not read from disk.
1622

1723
```typescript
1824
const result = yield* reviewer.reviewCode(
@@ -28,10 +34,12 @@ console.log(result.markdown);
2834

2935
## Constraints
3036

37+
- **Code source**: Code must be cut and pasted or provided from open editor. Files are NOT read from disk.
3138
- **File size**: Max 100KB (100,000 bytes)
32-
- **File type**: Must be .ts or .tsx
39+
- **File type**: Must be .ts or .tsx (if filePath provided)
3340
- **Free tier limit**: Top 3 recommendations (by severity)
3441
- **Code limit**: Entire finding base is analyzed
42+
- **Response type**: Only diagnostic information returned (findings, recommendations, fix plans). No corrected code is included.
3543

3644
## Response Structure
3745

packages/mcp-server/src/services/review-code/__tests__/review-code.test.ts

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,272 @@ function test() {
9797
expect(result).toHaveProperty("recommendations");
9898
expect(result).toHaveProperty("markdown");
9999
});
100+
101+
describe("Code source constraints", () => {
102+
it("should require code parameter (filePath alone is not sufficient)", async () => {
103+
// This test ensures that code must be provided directly
104+
// The service signature already enforces this at compile time,
105+
// but we verify runtime behavior
106+
const result = await Effect.runPromise(
107+
Effect.gen(function* () {
108+
const service = yield* ReviewCodeService;
109+
110+
// Code must be provided - cannot be empty
111+
const code = "";
112+
113+
const either = yield* Effect.either(
114+
service.reviewCode(code, "src/test.ts")
115+
);
116+
117+
return either;
118+
}).pipe(Effect.provide(TestLayer))
119+
);
120+
121+
// Empty code should still work (it's just empty), but we verify
122+
// that we're using the provided code, not reading from filePath
123+
expect(result._tag).toBe("Right");
124+
});
125+
126+
it("should use provided code parameter, not read from filePath", async () => {
127+
// This test verifies that the code parameter is used,
128+
// not the filePath (which might point to a different file)
129+
const providedCode = "const x: any = 42;"; // Code with issue
130+
const filePath = "src/test.ts"; // Path that doesn't exist or has different content
131+
132+
const result = await Effect.runPromise(
133+
Effect.gen(function* () {
134+
const service = yield* ReviewCodeService;
135+
136+
const res = yield* service.reviewCode(providedCode, filePath);
137+
return res;
138+
}).pipe(Effect.provide(TestLayer))
139+
);
140+
141+
// Should analyze the provided code, not read from filePath
142+
expect(result).toHaveProperty("recommendations");
143+
expect(result).toHaveProperty("enhancedRecommendations");
144+
// The analysis should be based on providedCode, not filePath content
145+
expect(Array.isArray(result.recommendations)).toBe(true);
146+
});
147+
148+
it("should work without filePath (code-only mode)", async () => {
149+
const code = "const x: number = 42;";
150+
151+
const result = await Effect.runPromise(
152+
Effect.gen(function* () {
153+
const service = yield* ReviewCodeService;
154+
155+
const res = yield* service.reviewCode(code);
156+
return res;
157+
}).pipe(Effect.provide(TestLayer))
158+
);
159+
160+
expect(result).toHaveProperty("recommendations");
161+
expect(result).toHaveProperty("enhancedRecommendations");
162+
expect(result).toHaveProperty("summary");
163+
expect(result).toHaveProperty("meta");
164+
expect(result).toHaveProperty("markdown");
165+
});
166+
167+
it("should use filePath only for context, not file reading", async () => {
168+
// Create a file path that definitely doesn't exist
169+
const nonExistentPath = "/tmp/non-existent-file-that-should-not-be-read.ts";
170+
const providedCode = "const x: number = 42;";
171+
172+
const result = await Effect.runPromise(
173+
Effect.gen(function* () {
174+
const service = yield* ReviewCodeService;
175+
176+
// Should work even with non-existent filePath
177+
// because we don't read from disk
178+
const res = yield* service.reviewCode(providedCode, nonExistentPath);
179+
return res;
180+
}).pipe(Effect.provide(TestLayer))
181+
);
182+
183+
// Should succeed because we use provided code, not filePath
184+
expect(result).toHaveProperty("recommendations");
185+
expect(result).toHaveProperty("enhancedRecommendations");
186+
});
187+
});
188+
189+
describe("Diagnostic-only response", () => {
190+
it("should return only diagnostic information, no corrected code", async () => {
191+
const code = `
192+
function test() {
193+
try {
194+
const result = Promise.resolve(42);
195+
} catch (e) {
196+
console.error(e);
197+
}
198+
}
199+
`;
200+
201+
const result = await Effect.runPromise(
202+
Effect.gen(function* () {
203+
const service = yield* ReviewCodeService;
204+
205+
const res = yield* service.reviewCode(code, "test.ts");
206+
return res;
207+
}).pipe(Effect.provide(TestLayer))
208+
);
209+
210+
// Verify response structure contains only diagnostics
211+
expect(result).toHaveProperty("recommendations");
212+
expect(result).toHaveProperty("enhancedRecommendations");
213+
expect(result).toHaveProperty("summary");
214+
expect(result).toHaveProperty("meta");
215+
expect(result).toHaveProperty("markdown");
216+
217+
// Verify NO corrected code fields
218+
expect(result).not.toHaveProperty("correctedCode");
219+
expect(result).not.toHaveProperty("after");
220+
expect(result).not.toHaveProperty("fixed");
221+
expect(result).not.toHaveProperty("patched");
222+
223+
// Verify recommendations contain diagnostics only
224+
if (result.recommendations.length > 0) {
225+
const rec = result.recommendations[0];
226+
expect(rec).toHaveProperty("severity");
227+
expect(rec).toHaveProperty("title");
228+
expect(rec).toHaveProperty("line");
229+
expect(rec).toHaveProperty("message");
230+
// Should NOT have corrected code
231+
expect(rec).not.toHaveProperty("correctedCode");
232+
expect(rec).not.toHaveProperty("after");
233+
}
234+
235+
// Verify enhanced recommendations contain diagnostics only
236+
if (result.enhancedRecommendations.length > 0) {
237+
const enhanced = result.enhancedRecommendations[0];
238+
expect(enhanced).toHaveProperty("severity");
239+
expect(enhanced).toHaveProperty("title");
240+
expect(enhanced).toHaveProperty("line");
241+
expect(enhanced).toHaveProperty("message");
242+
expect(enhanced).toHaveProperty("fixPlan");
243+
expect(enhanced).toHaveProperty("evidence");
244+
245+
// Fix plan should contain steps/changes/risks, not corrected code
246+
expect(enhanced.fixPlan).toHaveProperty("steps");
247+
expect(enhanced.fixPlan).toHaveProperty("changes");
248+
expect(enhanced.fixPlan).toHaveProperty("risks");
249+
expect(enhanced.fixPlan).not.toHaveProperty("correctedCode");
250+
expect(enhanced.fixPlan).not.toHaveProperty("after");
251+
252+
// Evidence should show context, not corrected code
253+
expect(enhanced.evidence).toHaveProperty("beforeContext");
254+
expect(enhanced.evidence).toHaveProperty("targetLines");
255+
expect(enhanced.evidence).toHaveProperty("afterContext");
256+
expect(enhanced.evidence).not.toHaveProperty("correctedLines");
257+
expect(enhanced.evidence).not.toHaveProperty("after");
258+
}
259+
});
260+
261+
it("should return fix plans with steps, not corrected code", async () => {
262+
const code = "const x: any = 42;";
263+
264+
const result = await Effect.runPromise(
265+
Effect.gen(function* () {
266+
const service = yield* ReviewCodeService;
267+
268+
const res = yield* service.reviewCode(code, "test.ts");
269+
return res;
270+
}).pipe(Effect.provide(TestLayer))
271+
);
272+
273+
// Check that fix plans contain actionable steps, not corrected code
274+
if (result.enhancedRecommendations.length > 0) {
275+
const fixPlan = result.enhancedRecommendations[0].fixPlan;
276+
277+
// Should have steps (actionable items)
278+
expect(Array.isArray(fixPlan.steps)).toBe(true);
279+
expect(Array.isArray(fixPlan.changes)).toBe(true);
280+
expect(Array.isArray(fixPlan.risks)).toBe(true);
281+
282+
// Steps should be descriptive actions, not code
283+
if (fixPlan.steps.length > 0) {
284+
const step = fixPlan.steps[0];
285+
expect(step).toHaveProperty("order");
286+
expect(step).toHaveProperty("action");
287+
expect(step).toHaveProperty("detail");
288+
// Should NOT contain full corrected code
289+
expect(typeof step.action).toBe("string");
290+
expect(step.action.length).toBeLessThan(1000); // Reasonable length for action
291+
}
292+
293+
// Changes should describe what changes, not show corrected code
294+
if (fixPlan.changes.length > 0) {
295+
const change = fixPlan.changes[0];
296+
expect(change).toHaveProperty("type");
297+
expect(change).toHaveProperty("scope");
298+
expect(change).toHaveProperty("description");
299+
// Should NOT contain full corrected code
300+
expect(typeof change.description).toBe("string");
301+
}
302+
}
303+
});
304+
305+
it("should return evidence snippets, not corrected code", async () => {
306+
const code = `
307+
function test() {
308+
const x: any = 42;
309+
return x;
310+
}
311+
`;
312+
313+
const result = await Effect.runPromise(
314+
Effect.gen(function* () {
315+
const service = yield* ReviewCodeService;
316+
317+
const res = yield* service.reviewCode(code, "test.ts");
318+
return res;
319+
}).pipe(Effect.provide(TestLayer))
320+
);
321+
322+
// Check that evidence shows original code context, not corrected code
323+
if (result.enhancedRecommendations.length > 0) {
324+
const evidence = result.enhancedRecommendations[0].evidence;
325+
326+
expect(evidence).toHaveProperty("beforeContext");
327+
expect(evidence).toHaveProperty("targetLines");
328+
expect(evidence).toHaveProperty("afterContext");
329+
expect(evidence).toHaveProperty("startLine");
330+
expect(evidence).toHaveProperty("endLine");
331+
332+
// targetLines should show the problematic code, not corrected version
333+
expect(Array.isArray(evidence.targetLines)).toBe(true);
334+
// Should NOT have corrected version
335+
expect(evidence).not.toHaveProperty("correctedLines");
336+
expect(evidence).not.toHaveProperty("after");
337+
}
338+
});
339+
340+
it("should return markdown with diagnostics only, no corrected code", async () => {
341+
const code = "const x: any = 42;";
342+
343+
const result = await Effect.runPromise(
344+
Effect.gen(function* () {
345+
const service = yield* ReviewCodeService;
346+
347+
const res = yield* service.reviewCode(code, "test.ts");
348+
return res;
349+
}).pipe(Effect.provide(TestLayer))
350+
);
351+
352+
expect(typeof result.markdown).toBe("string");
353+
expect(result.markdown.length).toBeGreaterThan(0);
354+
355+
// Markdown should contain diagnostic information
356+
expect(result.markdown).toMatch(/Code Review Results|Recommendations|Fix Plan|Evidence/i);
357+
358+
// Markdown should NOT contain corrected code sections
359+
// (no "After" code blocks, no "Corrected" sections)
360+
const lowerMarkdown = result.markdown.toLowerCase();
361+
// These patterns would indicate corrected code, which we don't want
362+
expect(lowerMarkdown).not.toMatch(/```[\s\S]*?after[\s\S]*?```/i);
363+
expect(lowerMarkdown).not.toMatch(/corrected code/i);
364+
expect(lowerMarkdown).not.toMatch(/fixed code/i);
365+
expect(lowerMarkdown).not.toMatch(/patched code/i);
366+
});
367+
});
100368
});

packages/mcp-server/src/services/review-code/api.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,17 @@ export class ReviewCodeService extends Effect.Service<ReviewCodeService>()(
7777
return {
7878
/**
7979
* Review code and return top 3 recommendations with enhanced details
80+
*
81+
* IMPORTANT: This service only accepts code that is:
82+
* 1. Cut and pasted into the prompt (code parameter)
83+
* 2. Provided from an open editor file (code parameter with optional filePath for context)
84+
*
85+
* Files are NOT read from disk. The filePath parameter is only used for:
86+
* - TypeScript file extension validation
87+
* - Context/metadata in analysis results
88+
*
89+
* Only diagnostic information is returned (findings, recommendations, fix plans).
90+
* No corrected code is included in the response.
8091
*/
8192
reviewCode: (
8293
code: string,

0 commit comments

Comments
 (0)