chore: bump version to 0.1.19 and enhance tool call handling#129
Merged
Conversation
- Updated package version to 0.1.19. - Improved handling of tool calls in chat and agent modules, allowing for better tracking and execution of tool requests. - Added support for switching to agent mode with user approval, streamlining operations that require multiple tool interactions. - Refactored message processing to accommodate new tool call structures and enhance overall response management.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
WalkthroughThe PR adds a ChangesCLI AI mode switch and tool-call flow
Sequence Diagram(s)sequenceDiagram
participant sendMessage
participant request
participant apiChat as "/api/ai/chat"
participant toolExecution as "tools[toolName].execute"
sendMessage->>request: stream currentMessages
request->>apiChat: POST messages and tools
apiChat-->>request: tool_call deltas
request-->>sendMessage: content + toolCalls + usage
sendMessage->>toolExecution: execute returned toolCalls
sendMessage->>request: replay assistant tool_calls and tool result messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
apps/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.ts (1)
8-8: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueModule-level mutable
pendingModeSwitchis a shared singleton.This works for a single interactive CLI session, but it couples the tool to one global request slot. If
streamAIResponseis ever invoked concurrently (e.g., parallel conversations), the flag/reason can leak across calls. Acceptable for the current single-session flow; flagging for awareness.Also applies to: 32-32
🤖 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/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.ts` at line 8, The module-level pendingModeSwitch singleton is shared across all calls, so state can leak between concurrent streamAIResponse sessions. Refactor switch-to-agent-mode.ts to make the ModeSwitchRequest request-scoped (for example, passed through the relevant tool/streaming path) instead of exported mutable state, and update any consumers of pendingModeSwitch so each invocation has its own isolated request slot.
🤖 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/supercode-cli/server/src/cli/ai/openrouter-service.ts`:
- Around line 141-149: The streamed tool-call handling in openrouter-service
should not silently skip malformed arguments inside the toolCalls parsing loop.
Update the JSON.parse error path in the logic that builds toolCalls from
pendingToolCalls so that a parse failure is surfaced as an error instead of
being ignored, allowing sendMessage() to fail or report the provider tool_calls
issue rather than returning an empty result. Use the existing toolCalls assembly
code path and the pendingToolCalls iteration in openrouter-service to locate the
fix.
- Around line 205-207: Accumulate token usage across all tool-call iterations in
the openrouter service instead of overwriting it with the last result. Update
the logic in openrouter-service’s iteration loop where `accumulatedContent`,
`finishReason`, and `usage` are assigned so `usage` is summed or merged from
each `result.usage` before returning, ensuring the final response reflects total
usage across the full conversation.
- Around line 200-238: The multi-turn tool handling in openrouter-service.ts is
unbounded because the `while (true)` loop can keep re-requesting tools forever;
add a maximum iteration guard to this loop, following the same pattern used by
the existing tool-loop handling in `request`/tool execution flow. Update the
loop around `currentMessages`, `tools`, and `result.toolCalls` to stop after a
fixed number of turns, and return or break cleanly when the limit is reached so
repeated tool requests cannot cause infinite API calls.
- Around line 225-236: The continuation messages added in openrouter-service’s
request flow are losing tool-call linkage because request() currently serializes
every message as only role and String(content). Update the message serialization
path used by request() so assistant tool-call messages preserve tool_calls and
tool result messages preserve tool_call_id, while keeping assistant content null
instead of converting it to "null". Make the fix in the request() message
mapping and the currentMessages handling around tool call continuation so
OpenRouter receives the full metadata needed to correlate tool results.
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts`:
- Around line 137-139: The proxy turn aggregation in server-proxy-service.ts
currently keeps only the last result.usage, so update the accumulation logic
around the result handling to sum usage across all turns instead of overwriting
it, while still preserving accumulatedContent and finishReason. Use the existing
result usage fields in the proxy flow so callers receive the total token usage
from the full multi-call sequence.
- Around line 133-183: The multi-turn tool-call loop in server-proxy-service’s
request/execute flow is unbounded and can run forever if the model keeps asking
for tools. Add a max-iteration guard in the loop around this.request, using a
counter or configurable limit before each request() call, and stop/break with a
clear fallback once the limit is reached. Keep the existing currentMessages/tool
execution behavior intact in the ServerProxyService loop, but ensure the guard
prevents repeated remote calls and local tool execution beyond the cap.
In `@apps/supercode-cli/server/src/index.ts`:
- Line 357: The `ServerProxyService` request assembly is forwarding the local
`tools` object unchanged, which can send non-OpenAI-compatible fields to
upstream providers. Update the payload normalization at the `bodyObj.tools`
assignment sites so tools are serialized into OpenAI-style function definitions
before forwarding, using the existing request-building logic around
`ServerProxyService` and the `bodyObj` construction paths referenced here.
- Around line 296-300: The tool-call argument parsing in the server currently
swallows JSON.parse failures in the tool-call emission path, causing malformed
calls to disappear and workflows to stall. Update the handling in the tool-call
stream logic around the parsing blocks in index.ts so failures are not silently
ignored: either emit an explicit error/finish event tied to the same call or
propagate the parse error through the existing response path so the client sees
the failed tool call. Make the same change in all matching tool-call parsing
locations, including the branches around the other parse sites referenced by the
diff.
In `@apps/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.ts`:
- Around line 10-16: The switch-to-agent tool schema currently makes `reason`
required even though `ModeSwitchRequest.reason` is optional and the `execute`
flow already handles an undefined value. Update `switchToAgentSchema` in
`switch-to-agent-mode.ts` so the `reason` field is optional, keeping it aligned
with `SwitchToAgentArgs`, `ModeSwitchRequest`, and the
`execute`/`pendingModeSwitch` handling.
---
Nitpick comments:
In `@apps/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.ts`:
- Line 8: The module-level pendingModeSwitch singleton is shared across all
calls, so state can leak between concurrent streamAIResponse sessions. Refactor
switch-to-agent-mode.ts to make the ModeSwitchRequest request-scoped (for
example, passed through the relevant tool/streaming path) instead of exported
mutable state, and update any consumers of pendingModeSwitch so each invocation
has its own isolated request slot.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a15a836-8509-45c8-817d-e2f6d74b6eac
📒 Files selected for processing (10)
apps/supercode-cli/server/package.jsonapps/supercode-cli/server/src/cli/ai/chat/chat.tsapps/supercode-cli/server/src/cli/ai/chat/chatAgent.tsapps/supercode-cli/server/src/cli/ai/openrouter-service.tsapps/supercode-cli/server/src/cli/ai/server-proxy-service.tsapps/supercode-cli/server/src/cli/commands/ai/init.tsapps/supercode-cli/server/src/config/agent-config.tsapps/supercode-cli/server/src/index.tsapps/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.tsapps/supercode-cli/server/src/tools/registry.ts
Comment on lines
+141
to
+149
| if (data.choices?.[0]?.finish_reason === "tool_calls") { | ||
| for (const [, call] of Object.entries(pendingToolCalls)) { | ||
| if (call.name && call.args) { | ||
| try { | ||
| const parsed = JSON.parse(call.args) | ||
| toolCalls.push({ toolName: call.name, args: parsed, toolCallId: call.id }) | ||
| onToolCall?.({ toolName: call.name, args: parsed }) | ||
| } catch { /* skip malformed args */ } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return an error when streamed tool arguments cannot be parsed.
Skipping malformed args turns a provider tool_calls finish into an empty toolCalls result, so sendMessage() exits without executing or reporting the requested tool. Surface the parse failure instead.
🤖 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/supercode-cli/server/src/cli/ai/openrouter-service.ts` around lines 141
- 149, The streamed tool-call handling in openrouter-service should not silently
skip malformed arguments inside the toolCalls parsing loop. Update the
JSON.parse error path in the logic that builds toolCalls from pendingToolCalls
so that a parse failure is surfaced as an error instead of being ignored,
allowing sendMessage() to fail or report the provider tool_calls issue rather
than returning an empty result. Use the existing toolCalls assembly code path
and the pendingToolCalls iteration in openrouter-service to locate the fix.
Comment on lines
+200
to
+238
| while (true) { | ||
| const result = await this.request( | ||
| currentMessages, tools, onChunk, onToolCall, signal, onReasoning, | ||
| ) | ||
|
|
||
| accumulatedContent += result.content | ||
| finishReason = result.finishReason | ||
| usage = result.usage | ||
|
|
||
| if (result.toolCalls.length === 0) break | ||
|
|
||
| for (const call of result.toolCalls) { | ||
| const toolFn = tools?.[call.toolName] | ||
| let toolResult: string | ||
|
|
||
| if (toolFn?.execute) { | ||
| try { | ||
| toolResult = await toolFn.execute(call.args) | ||
| } catch (err: any) { | ||
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | ||
| } | ||
| } else { | ||
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | ||
| } | ||
|
|
||
| currentMessages.push({ | ||
| role: "assistant", | ||
| content: null, | ||
| tool_calls: [ | ||
| { id: call.toolCallId, type: "function", function: { name: call.toolName, arguments: JSON.stringify(call.args) } }, | ||
| ], | ||
| }) | ||
| currentMessages.push({ | ||
| role: "tool", | ||
| tool_call_id: call.toolCallId, | ||
| content: toolResult, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the multi-turn tool loop.
while (true) can keep making API calls and executing tools indefinitely if the model repeatedly requests tools. Add a max-iteration guard like the existing tool loop pattern.
Suggested fix
+ const maxToolIterations = 25
+ let toolIterations = 0
+
while (true) {
+ if (++toolIterations > maxToolIterations) {
+ throw new Error(`Exceeded maximum tool-call iterations (${maxToolIterations})`)
+ }
const result = await this.request(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| while (true) { | |
| const result = await this.request( | |
| currentMessages, tools, onChunk, onToolCall, signal, onReasoning, | |
| ) | |
| accumulatedContent += result.content | |
| finishReason = result.finishReason | |
| usage = result.usage | |
| if (result.toolCalls.length === 0) break | |
| for (const call of result.toolCalls) { | |
| const toolFn = tools?.[call.toolName] | |
| let toolResult: string | |
| if (toolFn?.execute) { | |
| try { | |
| toolResult = await toolFn.execute(call.args) | |
| } catch (err: any) { | |
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | |
| } | |
| } else { | |
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | |
| } | |
| currentMessages.push({ | |
| role: "assistant", | |
| content: null, | |
| tool_calls: [ | |
| { id: call.toolCallId, type: "function", function: { name: call.toolName, arguments: JSON.stringify(call.args) } }, | |
| ], | |
| }) | |
| currentMessages.push({ | |
| role: "tool", | |
| tool_call_id: call.toolCallId, | |
| content: toolResult, | |
| }) | |
| } | |
| } | |
| const maxToolIterations = 25 | |
| let toolIterations = 0 | |
| while (true) { | |
| if (++toolIterations > maxToolIterations) { | |
| throw new Error(`Exceeded maximum tool-call iterations (${maxToolIterations})`) | |
| } | |
| const result = await this.request( | |
| currentMessages, tools, onChunk, onToolCall, signal, onReasoning, | |
| ) | |
| accumulatedContent += result.content | |
| finishReason = result.finishReason | |
| usage = result.usage | |
| if (result.toolCalls.length === 0) break | |
| for (const call of result.toolCalls) { | |
| const toolFn = tools?.[call.toolName] | |
| let toolResult: string | |
| if (toolFn?.execute) { | |
| try { | |
| toolResult = await toolFn.execute(call.args) | |
| } catch (err: any) { | |
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | |
| } | |
| } else { | |
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | |
| } | |
| currentMessages.push({ | |
| role: "assistant", | |
| content: null, | |
| tool_calls: [ | |
| { id: call.toolCallId, type: "function", function: { name: call.toolName, arguments: JSON.stringify(call.args) } }, | |
| ], | |
| }) | |
| currentMessages.push({ | |
| role: "tool", | |
| tool_call_id: call.toolCallId, | |
| content: toolResult, | |
| }) | |
| } | |
| } |
🤖 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/supercode-cli/server/src/cli/ai/openrouter-service.ts` around lines 200
- 238, The multi-turn tool handling in openrouter-service.ts is unbounded
because the `while (true)` loop can keep re-requesting tools forever; add a
maximum iteration guard to this loop, following the same pattern used by the
existing tool-loop handling in `request`/tool execution flow. Update the loop
around `currentMessages`, `tools`, and `result.toolCalls` to stop after a fixed
number of turns, and return or break cleanly when the limit is reached so
repeated tool requests cannot cause infinite API calls.
Comment on lines
+205
to
+207
| accumulatedContent += result.content | ||
| finishReason = result.finishReason | ||
| usage = result.usage |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Accumulate usage across tool-call iterations.
accumulatedContent spans all turns, but usage = result.usage reports only the final request. Sum token usage across iterations before returning.
🤖 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/supercode-cli/server/src/cli/ai/openrouter-service.ts` around lines 205
- 207, Accumulate token usage across all tool-call iterations in the openrouter
service instead of overwriting it with the last result. Update the logic in
openrouter-service’s iteration loop where `accumulatedContent`, `finishReason`,
and `usage` are assigned so `usage` is summed or merged from each `result.usage`
before returning, ensuring the final response reflects total usage across the
full conversation.
Comment on lines
+225
to
+236
| currentMessages.push({ | ||
| role: "assistant", | ||
| content: null, | ||
| tool_calls: [ | ||
| { id: call.toolCallId, type: "function", function: { name: call.toolName, arguments: JSON.stringify(call.args) } }, | ||
| ], | ||
| }) | ||
| currentMessages.push({ | ||
| role: "tool", | ||
| tool_call_id: call.toolCallId, | ||
| content: toolResult, | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve tool-call metadata in the next OpenRouter request.
These continuation messages add tool_calls and tool_call_id, but request() serializes messages as only { role, content: String(m.content) }. The next model call loses the tool-call linkage and turns assistant null content into "null", so tool results cannot be correlated.
Suggested fix
- messages: nonSystemMessages.map((m: any) => ({ role: m.role, content: String(m.content) })),
+ messages: nonSystemMessages.map((m: any) => {
+ const msg: any = {
+ role: m.role,
+ content: m.content == null ? null : String(m.content),
+ }
+ if (m.tool_calls) msg.tool_calls = m.tool_calls
+ if (m.tool_call_id) msg.tool_call_id = m.tool_call_id
+ return msg
+ }),🤖 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/supercode-cli/server/src/cli/ai/openrouter-service.ts` around lines 225
- 236, The continuation messages added in openrouter-service’s request flow are
losing tool-call linkage because request() currently serializes every message as
only role and String(content). Update the message serialization path used by
request() so assistant tool-call messages preserve tool_calls and tool result
messages preserve tool_call_id, while keeping assistant content null instead of
converting it to "null". Make the fix in the request() message mapping and the
currentMessages handling around tool call continuation so OpenRouter receives
the full metadata needed to correlate tool results.
Comment on lines
+133
to
+183
| // Multi-turn loop: keep calling the server as long as the AI requests tool calls | ||
| while (true) { | ||
| const result = await this.request(currentMessages, tools, onChunk, onToolCall, signal, onReasoning) | ||
|
|
||
| accumulatedContent += result.content | ||
| finishReason = result.finishReason | ||
| usage = result.usage | ||
|
|
||
| if (result.toolCalls.length === 0) break | ||
|
|
||
| // Execute each tool and build the continuation messages | ||
| for (const call of result.toolCalls) { | ||
| const toolFn = tools?.[call.toolName] | ||
| let toolResult: string | ||
|
|
||
| if (toolFn?.execute) { | ||
| try { | ||
| toolResult = await toolFn.execute(call.args) | ||
| } catch (err: any) { | ||
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | ||
| } | ||
| } else { | ||
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | ||
| } | ||
|
|
||
| // Add the assistant's tool call request to the message history | ||
| currentMessages.push({ | ||
| role: "assistant", | ||
| content: null, | ||
| tool_calls: [ | ||
| { | ||
| id: call.toolCallId, | ||
| type: "function", | ||
| function: { | ||
| name: call.toolName, | ||
| arguments: JSON.stringify(call.args), | ||
| }, | ||
| }, | ||
| ], | ||
| } as any) | ||
|
|
||
| // Add the tool execution result | ||
| currentMessages.push({ | ||
| role: "tool", | ||
| tool_call_id: call.toolCallId, | ||
| content: toolResult, | ||
| } as any) | ||
|
|
||
| this.collectedToolCalls.push(call) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the proxy tool-call loop.
A model that repeatedly requests tools can keep this loop running forever, causing unbounded remote calls and local tool execution. Add a max-iteration guard before each request() call.
Suggested fix
+ const maxToolIterations = 25
+ let toolIterations = 0
+
// Multi-turn loop: keep calling the server as long as the AI requests tool calls
while (true) {
+ if (++toolIterations > maxToolIterations) {
+ throw new Error(`Exceeded maximum tool-call iterations (${maxToolIterations})`)
+ }
const result = await this.request(currentMessages, tools, onChunk, onToolCall, signal, onReasoning)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Multi-turn loop: keep calling the server as long as the AI requests tool calls | |
| while (true) { | |
| const result = await this.request(currentMessages, tools, onChunk, onToolCall, signal, onReasoning) | |
| accumulatedContent += result.content | |
| finishReason = result.finishReason | |
| usage = result.usage | |
| if (result.toolCalls.length === 0) break | |
| // Execute each tool and build the continuation messages | |
| for (const call of result.toolCalls) { | |
| const toolFn = tools?.[call.toolName] | |
| let toolResult: string | |
| if (toolFn?.execute) { | |
| try { | |
| toolResult = await toolFn.execute(call.args) | |
| } catch (err: any) { | |
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | |
| } | |
| } else { | |
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | |
| } | |
| // Add the assistant's tool call request to the message history | |
| currentMessages.push({ | |
| role: "assistant", | |
| content: null, | |
| tool_calls: [ | |
| { | |
| id: call.toolCallId, | |
| type: "function", | |
| function: { | |
| name: call.toolName, | |
| arguments: JSON.stringify(call.args), | |
| }, | |
| }, | |
| ], | |
| } as any) | |
| // Add the tool execution result | |
| currentMessages.push({ | |
| role: "tool", | |
| tool_call_id: call.toolCallId, | |
| content: toolResult, | |
| } as any) | |
| this.collectedToolCalls.push(call) | |
| } | |
| } | |
| const maxToolIterations = 25 | |
| let toolIterations = 0 | |
| // Multi-turn loop: keep calling the server as long as the AI requests tool calls | |
| while (true) { | |
| if (++toolIterations > maxToolIterations) { | |
| throw new Error(`Exceeded maximum tool-call iterations (${maxToolIterations})`) | |
| } | |
| const result = await this.request(currentMessages, tools, onChunk, onToolCall, signal, onReasoning) | |
| accumulatedContent += result.content | |
| finishReason = result.finishReason | |
| usage = result.usage | |
| if (result.toolCalls.length === 0) break | |
| // Execute each tool and build the continuation messages | |
| for (const call of result.toolCalls) { | |
| const toolFn = tools?.[call.toolName] | |
| let toolResult: string | |
| if (toolFn?.execute) { | |
| try { | |
| toolResult = await toolFn.execute(call.args) | |
| } catch (err: any) { | |
| toolResult = JSON.stringify({ error: err.message || "Tool execution failed" }) | |
| } | |
| } else { | |
| toolResult = JSON.stringify({ error: `Tool "${call.toolName}" is not available locally` }) | |
| } | |
| // Add the assistant's tool call request to the message history | |
| currentMessages.push({ | |
| role: "assistant", | |
| content: null, | |
| tool_calls: [ | |
| { | |
| id: call.toolCallId, | |
| type: "function", | |
| function: { | |
| name: call.toolName, | |
| arguments: JSON.stringify(call.args), | |
| }, | |
| }, | |
| ], | |
| } as any) | |
| // Add the tool execution result | |
| currentMessages.push({ | |
| role: "tool", | |
| tool_call_id: call.toolCallId, | |
| content: toolResult, | |
| } as any) | |
| this.collectedToolCalls.push(call) | |
| } | |
| } |
🤖 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/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines
133 - 183, The multi-turn tool-call loop in server-proxy-service’s
request/execute flow is unbounded and can run forever if the model keeps asking
for tools. Add a max-iteration guard in the loop around this.request, using a
counter or configurable limit before each request() call, and stop/break with a
clear fallback once the limit is reached. Keep the existing currentMessages/tool
execution behavior intact in the ServerProxyService loop, but ensure the guard
prevents repeated remote calls and local tool execution beyond the cap.
Comment on lines
+137
to
+139
| accumulatedContent += result.content | ||
| finishReason = result.finishReason | ||
| usage = result.usage |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Accumulate usage across proxy turns.
The response content is accumulated across multiple model calls, but token usage is overwritten with the last turn only. Sum usage fields so callers receive the true total.
🤖 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/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines
137 - 139, The proxy turn aggregation in server-proxy-service.ts currently keeps
only the last result.usage, so update the accumulation logic around the result
handling to sum usage across all turns instead of overwriting it, while still
preserving accumulatedContent and finishReason. Use the existing result usage
fields in the proxy flow so callers receive the total token usage from the full
multi-call sequence.
Comment on lines
+296
to
+300
| try { | ||
| const parsed = JSON.parse(call.args) | ||
| res.write(JSON.stringify({ type: "tool-call", toolName: call.name, args: parsed, toolCallId: call.id }) + "\n") | ||
| } catch { /* skip malformed args */ } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not silently drop malformed tool-call arguments.
If JSON.parse(call.args) fails, the server emits no tool-call event, and the client can treat a tool_calls finish as “no tool calls” and stop. Emit an error/finish event or propagate the parse failure so the user sees the failed tool call instead of a stalled workflow.
Also applies to: 407-410, 493-496
🤖 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/supercode-cli/server/src/index.ts` around lines 296 - 300, The tool-call
argument parsing in the server currently swallows JSON.parse failures in the
tool-call emission path, causing malformed calls to disappear and workflows to
stall. Update the handling in the tool-call stream logic around the parsing
blocks in index.ts so failures are not silently ignored: either emit an explicit
error/finish event tied to the same call or propagate the parse error through
the existing response path so the client sees the failed tool call. Make the
same change in all matching tool-call parsing locations, including the branches
around the other parse sites referenced by the diff.
| if (system && nonSystemMessages.length > 0) { | ||
| bodyObj.messages = [{ role: "system", content: system }, ...bodyObj.messages] | ||
| } | ||
| if (tools) bodyObj.tools = tools |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize tool definitions before forwarding them upstream.
ServerProxyService sends the local tools object, but these provider payloads now forward it as-is. That can send local fields/schemas instead of OpenAI-compatible [{ type: "function", function: ... }] tool definitions, so NVIDIA/ConcentrateAI may reject or ignore tool calling. Normalize the payload before assigning bodyObj.tools.
Also applies to: 443-443
🤖 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/supercode-cli/server/src/index.ts` at line 357, The `ServerProxyService`
request assembly is forwarding the local `tools` object unchanged, which can
send non-OpenAI-compatible fields to upstream providers. Update the payload
normalization at the `bodyObj.tools` assignment sites so tools are serialized
into OpenAI-style function definitions before forwarding, using the existing
request-building logic around `ServerProxyService` and the `bodyObj`
construction paths referenced here.
Comment on lines
+10
to
+16
| const switchToAgentSchema = z.object({ | ||
| reason: z | ||
| .string() | ||
| .describe( | ||
| "Brief explanation of why agent mode is needed for this task", | ||
| ), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
cat -n apps/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.tsRepository: yashdev9274/supercli
Length of output: 1758
reason is required in the schema but implied optional in design.
The SwitchToAgentArgs type is inferred from switchToAgentSchema (lines 10-16), which currently requires reason as a mandatory string. However, the ModeSwitchRequest interface (lines 3-6) explicitly defines reason as optional (reason?: string). Additionally, while the execute function (line 31) destructures reason directly, assigning undefined to reason in pendingModeSwitch (line 32) is valid since the interface allows it.
For the tool to be robust, reason should be optional in the schema to allow the AI caller to omit it.
Changes required:
- Add
.optional()to thereasonfield inswitchToAgentSchema.
Proposed change
const switchToAgentSchema = z.object({
reason: z
.string()
+ .optional()
.describe(
"Brief explanation of why agent mode is needed for this task",
),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const switchToAgentSchema = z.object({ | |
| reason: z | |
| .string() | |
| .describe( | |
| "Brief explanation of why agent mode is needed for this task", | |
| ), | |
| }) | |
| const switchToAgentSchema = z.object({ | |
| reason: z | |
| .string() | |
| .optional() | |
| .describe( | |
| "Brief explanation of why agent mode is needed for this task", | |
| ), | |
| }) |
🤖 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/supercode-cli/server/src/tools/definitions/switch-to-agent-mode.ts`
around lines 10 - 16, The switch-to-agent tool schema currently makes `reason`
required even though `ModeSwitchRequest.reason` is optional and the `execute`
flow already handles an undefined value. Update `switchToAgentSchema` in
`switch-to-agent-mode.ts` so the `reason` field is optional, keeping it aligned
with `SwitchToAgentArgs`, `ModeSwitchRequest`, and the
`execute`/`pendingModeSwitch` handling.
This was referenced Jun 27, 2026
Merged
This was referenced Jul 5, 2026
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 by CodeRabbit
New Features
Bug Fixes