feat: integrate Cortex SDK and enhance Tauri app support: - #258
feat: integrate Cortex SDK and enhance Tauri app support:#258yashdev9274 wants to merge 1 commit into
Conversation
- Added Cortex SDK for unified TypeScript access to AI models and tools, streamlining development for AI agents. - Updated .gitignore to include Tauri app directories for cleaner builds. - Enhanced package.json scripts for easier development and building of the Jarvis app. - Implemented timeout handling for upstream requests in the server proxy service to prevent hanging requests. - Improved token budget management with dynamic daily limits and updated messaging for user feedback.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR adds streamed tool-call parsing, timeout and error handling, voice chat and TTS APIs, configurable token budgets, Jarvis commands, dependency pinning, and Cortex SDK documentation. ChangesServer AI and voice capabilities
Jarvis development tooling
Cortex SDK specification
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant VoiceChatClient
participant VoiceChatEndpoint
participant ElevenLabsTTS
participant AudioPlayer
VoiceChatClient->>VoiceChatEndpoint: send conversation messages
VoiceChatEndpoint->>VoiceChatClient: return reply
VoiceChatClient->>ElevenLabsTTS: request reply audio
ElevenLabsTTS->>VoiceChatClient: return MP3
VoiceChatClient->>AudioPlayer: play reply audio
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (26)
apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts-182-211 (1)
182-211: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not suppress callback failures.
The
catchhandles both malformed JSON and exceptions fromonChunk,onReasoning, andonToolCall. If a callback fails, this code silently continues and can return a successful response. Limit thecatchtoJSON.parse, then let callback failures propagate.Proposed fix
- try { - const event = JSON.parse(trimmed) - switch (event.type) { + let event: any + try { + event = JSON.parse(trimmed) + } catch { + continue + } + switch (event.type) { case "text": fullResponse += event.content onChunk?.(event.content) break + // Keep the remaining cases unchanged. - } - } catch { /* skip malformed */ }🤖 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 182 - 211, Update the event-processing loop around JSON.parse so the catch only handles malformed JSON parsing; separate parsing from event dispatch before the switch. Let failures from onChunk, onReasoning, and onToolCall propagate to the caller instead of being swallowed, while preserving the existing skip behavior for invalid JSON.apps/supercode-cli/server/src/voice/voiceChatClient.ts-119-121 (1)
119-121: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winBound the conversation history.
The default
maxRounds = 0permits unlimited turns. Every request sends the complete history. Long sessions will eventually exceed request or model context limits. Preserve the system message and retain a fixed number of recent turns before the next request.Also applies to: 136-140
🤖 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/voice/voiceChatClient.ts` around lines 119 - 121, Update the conversation-building logic around the ChatMessage history and default maxRounds so each request preserves the system message while retaining only a fixed number of most recent turns; ensure maxRounds = 0 no longer permits unlimited history, and apply the same bounded-history behavior to the additional history handling around the referenced request path.apps/supercode-cli/server/src/voice/voiceChatClient.ts-22-33 (1)
22-33: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a bounded timeout to the voice server calls.
chatWithLLM()awaits/api/voice/chatwithout a signal, so a stalled server can hold the voice loop onchatWithLLM()for an unbounded time. Use a shared bounded fetch helper for both/api/voice/chatand/api/voice/tts, and handle abort failures as recoverable fallbacks where needed.🤖 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/voice/voiceChatClient.ts` around lines 22 - 33, Introduce a shared bounded fetch helper in voiceChatClient.ts and use it in both chatWithLLM() at lines 22-33 and the TTS call at lines 56-63 for /api/voice/chat and /api/voice/tts; pass an abort signal with the configured timeout, and handle timeout/abort failures as recoverable fallbacks where each voice flow requires it.apps/supercode-cli/server/src/voice/voiceChatClient.ts-135-140 (1)
135-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCommit the user turn only after the chat request succeeds.
A failed
chatWithLLMcall leaves the user message inmessages. The next successful request then includes an unpaired prior turn and changes the conversation unexpectedly. Build request messages separately, then append both messages only after a reply is received.Proposed fix
- messages.push({ role: "user", content: userText }) + const userMessage: ChatMessage = { role: "user", content: userText } + const requestMessages = [...messages, userMessage] // 2. Get LLM reply - const reply = await chatWithLLM(messages) - messages.push({ role: "assistant", content: reply }) + const reply = await chatWithLLM(requestMessages) + messages.push(userMessage, { role: "assistant", content: reply })🤖 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/voice/voiceChatClient.ts` around lines 135 - 140, Update the conversation flow around chatWithLLM so the user message is included in a separate request-messages collection without mutating messages before the request succeeds. After chatWithLLM returns successfully, append both the user message and assistant reply to messages together, preserving the existing roles and content.apps/supercode-cli/server/src/index.ts-1021-1029 (1)
1021-1029: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThe ConcentrateAI and Supercode Cloud streaming branches are duplicated and now diverge. Both branches implement the same SSE read loop, pending-tool-call accumulation, parser flush, non-streaming fallback, and token accounting. The code was copied, so every defect exists twice, and the two copies have already drifted apart in flush placement and in handling of
flushed.calls. Extract the shared loop into one helper so a single fix applies to both providers.
apps/supercode-cli/server/src/index.ts#L1021-L1029: replace thebreakin the catch block with a flag, so the handler still reaches the parser flush, the fallback, and thefinishevent.apps/supercode-cli/server/src/index.ts#L1274-L1279: apply the identicalbreakfix to the Supercode Cloud catch block.apps/supercode-cli/server/src/index.ts#L1004-L1019: move the pending-tool-call flush out of thewhileloop so it runs once after the stream ends.apps/supercode-cli/server/src/index.ts#L1259-L1272: move the mirrored pending-tool-call flush out of itswhileloop as well.apps/supercode-cli/server/src/index.ts#L1091-L1092: assigninputTokensandoutputTokensonly whenfbData.usageis present; otherwise estimate.apps/supercode-cli/server/src/index.ts#L1325-L1326: apply the identical token-accounting fix to the Supercode Cloud fallback.After the fixes land, extract the common streaming logic into a single function that takes the upstream URL, headers, and body. That removes the drift permanently.
🤖 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 1021 - 1029, apps/supercode-cli/server/src/index.ts lines 1021-1029 and 1274-1279: update both streaming catch blocks to set an abort flag instead of breaking, allowing parser flush, fallback handling, and the finish event to run. Lines 1004-1019 and 1259-1272: move pending-tool-call flushing outside each read loop so it executes once after streaming ends. Lines 1091-1092 and 1325-1326: use fbData.usage for token counts only when present; otherwise estimate. Extract the duplicated ConcentrateAI and Supercode Cloud streaming logic, including SSE reading, accumulation, flushing, fallback, and accounting, into one helper accepting the upstream URL, headers, and body, then route both branches through it.apps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.ts-29-38 (1)
29-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe split-chunk test only splits the closing marker, not the opening marker.
Chunks here are
"I'll check.","[TOOL_CALL]\n...\n[/","TOOL_CALL]". The opener arrives whole inside one chunk. The parser handles that path.No test splits an opener across chunks. That path is broken in
apps/supercode-cli/server/src/lib/embedded-tool-calls.ts(see the comment on Lines 96-116 there): the partial opener is emitted as text and the tool call is lost.Add a test for the opener split so the regression is caught.
💚 Proposed test to add
test("square bracket opener split across two chunks", () => { const out = drain( parseStreamedContent(), ["I'll check.[TOOL_", "CALL]\nrun_command --command=\"git diff\"\n[/TOOL_CALL]"], ) expect(out.text).toBe("I'll check.") expect(out.calls).toEqual([ { name: "run_command", args: { command: "git diff" }, id: "" }, ]) }) test("xml opener split across two chunks", () => { const out = drain( parseStreamedContent(), ["Sure.<tool_", "call><invoke name=\"read_file\"><parameter name=\"path\">/x</parameter></invoke></tool_call>"], ) expect(out.text).toBe("Sure.") expect(out.calls.length).toBe(1) })🤖 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/lib/__tests__/embedded-tool-calls.test.ts` around lines 29 - 38, Add regression coverage in the streamed content parser tests for opening markers split across chunks: add square-bracket and XML opener cases using split chunk boundaries, then assert the preceding text remains unchanged and the tool call is parsed rather than emitted or lost. Use the existing drain and parseStreamedContent helpers and preserve the established call assertions.apps/supercode-cli/server/src/index.ts-1914-1914 (1)
1914-1914: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove or use the dead token and timing variables.
chatStartat Lines 1914 and 1958 is assigned and never read.inputTokensandoutputTokensat Lines 1946-1947 and 1992-1993 are computed and never read.Both
/api/voice/chatprovider branches therefore perform no usage recording. The other chat routes in this file record usage and enforce the daily budget. These endpoints do not.Either record usage for the authenticated user, matching the other routes, or delete the unused variables.
Also applies to: 1946-1947, 1958-1958, 1992-1993
🤖 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 1914, Update both provider branches in the /api/voice/chat handler to record token usage and timing for the authenticated user, reusing the existing usage-recording and daily-budget enforcement pattern from the other chat routes; ensure chatStart, inputTokens, and outputTokens are consumed by that flow rather than left unused.apps/supercode-cli/server/src/lib/embedded-tool-calls.ts-193-224 (1)
193-224: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEmbedded tool calls carry an empty
id, so parallel calls cannot be correlated.Both
parseXmlBlockandparseSquareBlocksetid: "".tryParseJsonDescriptordoes the same at Line 287.apps/supercode-cli/server/src/index.tsforwards this directly astoolCallId: call.idat Lines 964 and 1214-1219.Structured
delta.tool_callspaths forward a real upstream id. Embedded calls do not. If one turn produces two embedded tool calls, the client receives two events withtoolCallId: ""and cannot match results to calls.Generate a unique id in the parser.
🐛 Proposed fix to generate ids
+let embeddedCallSeq = 0 + +function nextCallId(): string { + embeddedCallSeq += 1 + return `embedded_${Date.now().toString(36)}_${embeddedCallSeq}` +} + /** XML shape: <invoke name="X"><parameter name="Y">value</parameter></invoke> */Then replace each
id: ""withid: nextCallId()inparseXmlBlock,parseSquareBlock, andtryParseJsonDescriptor.Update the tests in
apps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.tsthat assertid: ""if you apply this change.🤖 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/lib/embedded-tool-calls.ts` around lines 193 - 224, Generate a unique identifier for every embedded tool call by replacing the empty IDs in parseXmlBlock, parseSquareBlock, and tryParseJsonDescriptor with the shared nextCallId() helper. Update embedded-tool-calls tests that currently expect id: "" while preserving parsing behavior and ensuring separate calls receive distinct IDs.apps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.ts-124-131 (1)
124-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis test does not exercise the unknown-tool rejection branch.
The fixture is
{ "name": "not_a_tool", ... }with a space after{.findJsonDescriptorStartmatches/\{"name"\s*:/, which requires"name"immediately after{. The scanner therefore returns -1 and treats the whole string as prose.The
knownTools.has(name)check atapps/supercode-cli/server/src/lib/embedded-tool-calls.tsLine 284 is never reached. If that check were removed, this test would still pass.Remove the space so the descriptor is detected and then rejected by name.
💚 Proposed fix
test("bare json with unknown tool name is left as prose", () => { const out = drain( parseStreamedContent(), - ['here is some { "name": "not_a_tool", "parameters": {"q":1} } prose'], + ['here is some {"name":"not_a_tool","parameters":{"q":1}} prose'], ) expect(out.text).toContain("prose") + expect(out.text).toContain("not_a_tool") expect(out.calls.length).toBe(0) })🤖 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/lib/__tests__/embedded-tool-calls.test.ts` around lines 124 - 131, Update the JSON fixture in the “bare json with unknown tool name is left as prose” test to place the name field immediately after the opening brace, allowing findJsonDescriptorStart to detect it and exercise the knownTools.has(name) rejection branch while preserving the expected prose output and zero tool calls.apps/supercode-cli/server/src/index.ts-1968-1981 (1)
1968-1981: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe OpenRouter request has no timeout.
The ConcentrateAI branch passes
signal: AbortSignal.timeout(30_000)at Line 1934. This branch passes nosignal. If OpenRouter stalls, the handler holds the request until the default socket timeout.Apply the same timeout for consistent behavior across providers.
🐛 Proposed fix
temperature: 0.7, stream: false, }), + signal: AbortSignal.timeout(30_000), })🤖 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 1968 - 1981, Add the same 30-second timeout signal used by the ConcentrateAI request to the OpenRouter fetch call in the surrounding handler. Update the fetch options for the OpenRouter request while preserving its existing method, headers, body, and response handling.apps/supercode-cli/server/src/lib/embedded-tool-calls.ts-118-134 (1)
118-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAn unclosed
{"name":sequence buffers the rest of the stream and then discards it.
tryParseJsonDescriptorreturnsincompletefor every input wherefindBalancedClosefails.pumpthen stores the whole tail inpending. Each laterpushre-appends and re-fails, so no text is ever emitted again for that turn.flush()drops the buffer, so the user loses all remaining content.The comment at Lines 270-271 states that the wait is capped, but no cap exists in
tryParseJsonDescriptoror inpump.Add the cap. If the held descriptor exceeds a byte limit, treat it as prose: emit the
{and rescan fromnextOpen + 1.🐛 Proposed fix to bound the incomplete-descriptor buffer
+const MAX_PENDING_JSON = 8192 +if (parsed?.incomplete) { - // Descriptor start but not yet closed — hold the tail for next chunk. - pending = block.slice(nextOpen) - i = block.length - break + const tailLen = block.length - nextOpen + if (tailLen > MAX_PENDING_JSON) { + // Never closed within the cap — treat as prose and rescan. + i = nextOpen + 1 + continue + } + // Descriptor start but not yet closed — hold the tail for next chunk. + pending = block.slice(nextOpen) + i = block.length + 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 `@apps/supercode-cli/server/src/lib/embedded-tool-calls.ts` around lines 118 - 134, Bound incomplete descriptor buffering in tryParseJsonDescriptor/pump using the documented byte limit. When the held {"name": sequence exceeds that limit, treat it as prose by emitting the opening brace and resume scanning from nextOpen + 1; otherwise preserve the existing pending behavior for genuinely incomplete descriptors and ensure flush does not discard unbounded content.apps/supercode-cli/server/src/lib/embedded-tool-calls.ts-169-176 (1)
169-176: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
flush()always returns empty and discards buffered prose.
remainingis assigned but never read. Both branches return the same emptyParsedBlock. The function therefore drops everything held inbuf, including plain trailing prose that contains no marker at all.This contradicts the documented contract at Lines 76-77 ("release whatever is left (normally just trailing prose)").
It also makes the consumer code dead. In
apps/supercode-cli/server/src/index.ts, Lines 1033-1042 and Lines 1346-1350 readflushed.textandflushed.calls, which can never be non-empty.Distinguish a truncated marker from trailing prose. Drop only the marker case.
🐛 Proposed fix for `flush()`
flush(): ParsedBlock { const remaining = buf buf = "" if (!remaining) return { text: "", calls: [], pending: "" } - // No more chunks coming: if the tail still holds an unclosed opener it - // is a truncated marker — drop it rather than leak raw markup. - return { text: "", calls: [], pending: "" } + // No more chunks coming: if the tail still holds an unclosed opener it + // is a truncated marker — drop it rather than leak raw markup. + const hasOpener = + remaining.includes("[TOOL_CALL]") || + remaining.includes("[tool_call]") || + remaining.includes("<tool_call>") || + findJsonDescriptorStart(remaining, 0) !== -1 + if (hasOpener) return { text: "", calls: [], pending: "" } + return { text: remaining, calls: [], pending: "" } },🤖 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/lib/embedded-tool-calls.ts` around lines 169 - 176, Update flush() to process the buffered remaining content instead of returning an always-empty ParsedBlock. Preserve the existing behavior for an unclosed marker opener by dropping that truncated marker, but return trailing plain prose in text and emit any complete buffered calls through the same parsing logic used by the parser; keep the consumer paths reading flushed.text and flushed.calls functional.apps/supercode-cli/server/src/index.ts-1850-1877 (1)
1850-1877: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
/api/voice/ttshas no input length cap and no upstream timeout.
textis validated only as a non-empty string. A caller can submit a very large body. ElevenLabs bills per character, so an authenticated caller can consume the account's credits in a single request.The
fetchat Line 1862 also passes nosignal. If ElevenLabs stalls, the handler holds the connection until the default socket timeout. The streaming routes in this file useAbortSignal.timeout(UPSTREAM_TIMEOUT_MS)for exactly this reason.🛡️ Proposed fix
+const TTS_MAX_CHARS = 5000 +const { text } = req.body if (!text || typeof text !== "string") { res.status(400).json({ error: "text is required" }) return } + if (text.length > TTS_MAX_CHARS) { + res.status(400).json({ error: `text exceeds ${TTS_MAX_CHARS} characters` }) + return + }output_format: "mp3_44100_128", }), + signal: AbortSignal.timeout(30_000), }, )🤖 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 1850 - 1877, Update the /api/voice/tts handler around the text validation and ElevenLabs fetch to enforce the existing request-size convention, rejecting text that exceeds the allowed character limit before calling the provider. Pass AbortSignal.timeout(UPSTREAM_TIMEOUT_MS) in the fetch options, matching the streaming routes’ upstream timeout behavior.apps/supercode-cli/server/src/index.ts-1344-1350 (1)
1344-1350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe Supercode Cloud flush drops tool calls and runs after the emptiness checks.
This block diverges from the equivalent ConcentrateAI block at Lines 1033-1042 in two ways.
First, it reads only
flushed.text. It ignoresflushed.calls. The ConcentrateAI block emits both. Any tool call released byflush()is lost on this path.Second, it runs after the non-streaming fallback at Line 1289 and after the last-resort check at Line 1332. Flushed text therefore cannot satisfy
!fullContent.trim(). A turn whose remaining content sits in the parser buffer emits the "The model returned an empty response" error and then emits the text.Move the flush before the emptiness checks and handle
flushed.calls.🐛 Proposed fix
Move this block to immediately after the stream
catchat Line 1279, before the token estimation at Line 1282:+ // Release any trailing prose still held by the embedded-tool-call + // parser (narration emitted after the final marker). + const flushed = embedded.flush() + if (flushed.text) { + fullContent += flushed.text + res.write(JSON.stringify({ type: "text", content: flushed.text }) + "\n") + } + for (const call of flushed.calls) { + sawToolCalls = true + emittedToolCalls = true + res.write(JSON.stringify({ + type: "tool-call", + toolName: call.name, + args: call.args, + toolCallId: call.id, + }) + "\n") + } + // If streaming didn't include usage data, estimate from content.Then delete the block at Lines 1344-1350.
This currently has no visible effect because
flush()inapps/supercode-cli/server/src/lib/embedded-tool-calls.tsalways returns empty. It becomes a live defect once that function is fixed.🤖 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 1344 - 1350, Move the `embedded.flush()` block to immediately after the stream `catch`, before token estimation and all emptiness checks, so flushed text contributes to `fullContent` before fallback validation. In that block, preserve the existing text emission and also emit every entry in `flushed.calls` using the same tool-call response format as the equivalent ConcentrateAI flush block. Remove the later flush block to avoid duplicate processing.apps/supercode-cli/server/src/lib/token-budget.ts-6-6 (1)
6-6: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject non-positive and non-finite budget overrides.
Number(...) || 1_000_000accepts values such as-1andInfinity. With-1,checkDailyTokenBudgetrejects every request becauseused >= -1. The CLI also passes the negative value toprogressBar, where a positive usage value can produce a negative repeat count and throw. Accept only a positive safe integer, then use the fallback or fail startup.Proposed validation
-export const DAILY_BUDGET_TOKENS = Number(process.env.SUPERCODE_DAILY_TOKEN_BUDGET) || 1_000_000 +const configuredDailyBudget = Number(process.env.SUPERCODE_DAILY_TOKEN_BUDGET) +export const DAILY_BUDGET_TOKENS = + Number.isSafeInteger(configuredDailyBudget) && configuredDailyBudget > 0 + ? configuredDailyBudget + : 1_000_000🤖 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/lib/token-budget.ts` at line 6, Update the DAILY_BUDGET_TOKENS initialization and its checkDailyTokenBudget usage to accept an override only when it is finite, positive, and a safe integer; otherwise use the existing 1,000,000 fallback or fail startup consistently. Ensure invalid values such as -1, Infinity, NaN, and unsafe integers never reach progressBar or budget comparisons.cortex-sdk.md-94-117 (1)
94-117: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse distinct names for the default model and model selector.
The examples use
gateway.model("...")and also passmdGateway.modelas aLanguageModel. A normal object cannot expose the same property as both a model object and a callable selector. Use separate names such as.modeland.getModel(id), then update the examples and types.Proposed contract
- Returns { model(id?): LanguageModel, listModels(): ModelInfo[] } + Returns { + model: LanguageModel + getModel(id?: string): LanguageModel + listModels(): Promise<ModelInfo[]> + }Also applies to: 392-397
🤖 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 `@cortex-sdk.md` around lines 94 - 117, Separate the configured default model from model selection in the gateway API: keep `.model` as the LanguageModel getter and introduce `.getModel(id)` for selecting a model. Update the examples around `createGateway`, `gateway.model`, and `mdGateway.model`, plus the corresponding types and the additional affected usage, so no property is both callable and a model object.cortex-sdk.md-24-30 (1)
24-30: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the
createGateway()return type consistent.The table says
createGateway()returns aLanguageModel. The examples and Phase 2 use a gateway client withlistModels()and model selection. Define and document oneGatewayClientreturn shape.Also applies to: 117-117, 392-397
🤖 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 `@cortex-sdk.md` around lines 24 - 30, Define a single GatewayClient return shape for createGateway(), including the LanguageModel access and listModels/model-selection capabilities used by the examples and Phase 2. Update the table, examples, and Phase 2 references to consistently document and use GatewayClient instead of claiming createGateway() directly returns LanguageModel.cortex-sdk.md-90-92 (1)
90-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn a promise from
listModels().The examples await
gateway.listModels(), but Phase 2 declareslistModels(): ModelInfo[]. Model discovery calls an upstream provider and is asynchronous. UselistModels(): Promise<ModelInfo[]>consistently.Also applies to: 392-395
🤖 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 `@cortex-sdk.md` around lines 90 - 92, Update the Phase 2 `listModels` declaration to return `Promise<ModelInfo[]>` instead of `ModelInfo[]`, matching the asynchronous upstream model discovery and the existing `await gateway.listModels()` examples. Apply the same return type consistently in the additional declaration referenced by the comment.cortex-sdk.md-346-353 (1)
346-353: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDeclare the OAuth launcher dependency.
Phase 4 requires the
openpackage, but the package manifest does not declare it. Addopenas a direct runtime dependency or replace it with a runtime-neutral browser-launch adapter.Also applies to: 442-445
🤖 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 `@cortex-sdk.md` around lines 346 - 353, Add the OAuth launcher dependency required by Phase 4 to the manifest’s runtime dependencies, preferably declaring open directly; alternatively, replace the launcher usage with an existing runtime-neutral browser-launch adapter and update the OAuth flow accordingly.cortex-sdk.md-217-227 (1)
217-227: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMove tool-loop guards into an enforceable API.
stepCountIs(8), sentinel injection, and repetition detection run at the orchestration/request layer.SupercodeAgentonly exposes.modeland.tools, while the consumer callsstreamTextdirectly. Add anagent.streamTextor middleware API, or document these guards as consumer responsibilities.Also applies to: 290-295, 454-488
🤖 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 `@cortex-sdk.md` around lines 217 - 227, Update SupercodeAgent and its usage documentation so tool-loop guards—stepCountIs(8), sentinel injection, and repetition detection—are enforced through an agent.streamText or equivalent middleware API; alternatively, explicitly document them as consumer responsibilities wherever direct streamText usage is shown. Ensure the behavior is consistent across the referenced orchestration and example sections.cortex-sdk.md-220-224 (1)
220-224: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject duplicate tool names during merging.
{ ...agent.tools }and the merged-tools contract silently overwrite duplicate names. Agent Handler and Composio can then expose the wrong implementation under one tool name. Detect duplicates and throw, or namespace tools before returning.tools.Also applies to: 473-474
🤖 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 `@cortex-sdk.md` around lines 220 - 224, Update the tool-merging logic that produces agent.tools to reject duplicate tool names instead of silently overwriting them; detect collisions between Agent Handler and Composio tools and throw an error, or namespace the conflicting tools before returning .tools. Ensure the same behavior is applied to the related merged-tools usage.cortex-sdk.md-164-168 (1)
164-168: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftExpose authentication for server-proxied Composio mode.
createComposio({ serverUrl })does not expose an access token or token provider. The existing server contract requires anAuthorizationbearer token. A standalone SDK cannot assume CLI token storage.Add
accessTokenorgetAccessTokento the configuration and document its use.Proposed configuration
- Accepts { apiKey?, serverUrl? } + Accepts { + apiKey? + serverUrl? + accessToken?: string | (() => Promise<string>) + }Also applies to: 433-448
🤖 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 `@cortex-sdk.md` around lines 164 - 168, Update the createComposio configuration examples and corresponding documentation to expose authentication for server-proxied mode: support an accessToken or getAccessToken alongside serverUrl, and document that it supplies the required bearer token without relying on CLI token storage. Apply the same change to the additional server-proxy configuration section.cortex-sdk.md-492-505 (1)
492-505: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd the workspace dependency required by Phase 6.
Phase 6 imports
cortex-sdkfromapps/supercode-cli/server, but the integration plan only relies on workspace auto-discovery. Addcortex-sdk: "workspace:*"to each consuming package. Workspace discovery alone does not declare the dependency for isolated installs.As per coding guidelines, internal package dependencies in
package.jsonmust useworkspace:*.Also applies to: 536-545
🤖 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 `@cortex-sdk.md` around lines 492 - 505, Add the internal dependency "cortex-sdk": "workspace:*" to the package.json files for each consuming package under apps/supercode-cli/server, ensuring every package importing createGateway, createAgentHandler, or createComposio declares the workspace dependency explicitly.Source: Coding guidelines
cortex-sdk.md-56-60 (1)
56-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse one public type vocabulary.
The document uses
AgentHandlerConfigandAgentHandlerOptions,ComposioConfigandComposioOptions, andConnectionStatusandConnectorStatus. Choose canonical names and update every API and implementation phase before creating the exported types.Also applies to: 64-66, 369-370, 461-466
🤖 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 `@cortex-sdk.md` around lines 56 - 60, Standardize the public type vocabulary throughout the document and implementation plan by selecting one canonical name for each pair: AgentHandlerConfig/AgentHandlerOptions, ComposioConfig/ComposioOptions, and ConnectionStatus/ConnectorStatus. Update every API reference, phase description, and exported-type definition consistently before introducing the public types.cortex-sdk.md-279-288 (1)
279-288: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBound retries before tool side effects.
A valid tool-only response can contain no text. If the fallback treats empty text as an empty stream, it can replay the request after a tool call was emitted. That can duplicate non-idempotent actions. Do not retry after a tool call or partial output. Define an idempotency policy for retried requests.
🤖 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 `@cortex-sdk.md` around lines 279 - 288, Update the Empty-Stream Fallback behavior in the SDK documentation to prohibit retries once a tool call or any partial output has been emitted, including valid tool-only responses with no text. Define the idempotency policy for requests eligible for the non-streaming fallback, ensuring retries are limited to safely repeatable requests and cannot duplicate non-idempotent tool actions.cortex-sdk.md-323-340 (1)
323-340: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd CommonJS exports for subpaths.
The root export has a
requirecondition, but./gateway,./agent-handler, and./composioonly definetypesandimport. With the planned CJS build,require("cortex-sdk/gateway")will fail package exports resolution.Proposed export entry
"./gateway": { "types": "./dist/gateway/index.d.ts", - "import": "./dist/gateway/index.js" + "import": "./dist/gateway/index.js", + "require": "./dist/gateway/index.cjs" }🤖 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 `@cortex-sdk.md` around lines 323 - 340, Update the package exports entries for "./gateway", "./agent-handler", and "./composio" to include their CommonJS require targets alongside the existing types and import conditions, using the corresponding generated .cjs files. Preserve the root export and existing ESM/type targets.
🟡 Minor comments (10)
apps/supercode-cli/server/package.json-72-72 (1)
72-72: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse an exact patch version for Zod.
In package manifests,
3.25is expanded to the3.25.xrange, so it can resolve below3.25.2. If this dependency should be pinned to3.25.2, updateapps/supercode-cli/server/package.jsonto"3.25.2"and update the Bun lockfile.🤖 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/package.json` at line 72, Update the zod dependency in the server package manifest from the partial version 3.25 to the exact version 3.25.2, then regenerate the Bun lockfile so it records the pinned version consistently.apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts-100-103 (1)
100-103: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReject invalid timeout values.
Line 100 accepts negative and non-finite values from
SUPERCODE_REQUEST_TIMEOUT_MS. A negative timeout can abort every proxy request immediately. Use a finite value greater than zero, or fall back to the 120-second default.Proposed fix
- const timeoutMs = Number(process.env.SUPERCODE_REQUEST_TIMEOUT_MS) || 120_000 + const configuredTimeoutMs = Number(process.env.SUPERCODE_REQUEST_TIMEOUT_MS) + const timeoutMs = + Number.isFinite(configuredTimeoutMs) && configuredTimeoutMs > 0 + ? configuredTimeoutMs + : 120_000🤖 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 100 - 103, Update the timeoutMs initialization in the proxy request flow to accept SUPERCODE_REQUEST_TIMEOUT_MS only when it parses to a finite value greater than zero; otherwise use the existing 120-second default. Keep the setTimeout behavior and controller abort handling unchanged.apps/supercode-cli/server/src/voice/voiceChatClient.ts-1-7 (1)
1-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the required filename, aliases, and import groups.
Rename
voiceChatClient.tstovoice-chat-client.tsand update its import sites. Replace the relative internal imports with@/aliases. Separate the Node imports from internal alias imports with a blank line.As per coding guidelines, "Use kebab-case for non-component filenames" and "Use absolute imports with
@/path aliases ... Order imports ... with blank lines between groups."🤖 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/voice/voiceChatClient.ts` around lines 1 - 7, Rename voiceChatClient.ts to voice-chat-client.ts and update all import sites accordingly. In the renamed module, replace the relative imports for getStoredToken, voiceCaptureFlow, and stripForSpeech with the configured `@/` aliases, keeping Node imports grouped separately above them with a blank line.Source: Coding guidelines
apps/supercode-cli/server/src/index.ts-1889-1891 (1)
1889-1891: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winThe 500 handler returns the raw error string to the caller.
String(error)can include internal paths, upstream hostnames, or stack detail depending on the thrown value. Return a fixed message and log the detail server-side.🛡️ Proposed fix
} catch (error) { - res.status(500).json({ error: String(error) }) + console.error("[voice] request failed", error) + res.status(500).json({ error: "Internal server error" }) }Apply the same change to both handlers.
Also applies to: 2000-2002
🤖 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 1889 - 1891, Update both 500-error catch handlers near the shown response blocks to stop returning String(error) in the JSON response; log the caught error server-side, then return a fixed generic error message to callers while preserving the 500 status.apps/supercode-cli/server/src/lib/embedded-tool-calls.ts-249-254 (1)
249-254: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe descriptor-start regex rejects whitespace after
{.
/\{"name"\s*:/allows whitespace only before the colon. A model that emits{ "name": "run_command", "parameters": { ... } }is not detected, so the descriptor leaks into the chat as prose.Allow whitespace after the opening brace.
🐛 Proposed fix
- const re = /\{"name"\s*:/g + const re = /\{\s*"name"\s*:/gThis change also affects the test at
apps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.tsLine 127, which currently uses{ "name": "not_a_tool" ... }. That test passes today only because the space prevents detection, not because the known-tool check rejects it.🤖 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/lib/embedded-tool-calls.ts` around lines 249 - 254, Update the descriptor-start regex in findJsonDescriptorStart to allow optional whitespace between the opening brace and "name", while preserving the existing whitespace handling before the colon. Update the affected embedded-tool-calls test so its not_a_tool case verifies rejection by the known-tool check rather than relying on the preceding space to prevent detection.apps/supercode-cli/server/src/cli/ai/chat/chat.ts-365-365 (1)
365-365: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not advertise unavailable web tools.
This note always names Exa and Firecrawl as available. Later,
streamAIResponseremovesexa_*andfirecrawl_*tools when no provider key or MCP connector exists. The model can therefore select a capability that is absent from its tool schema. Build this note after tool filtering, or state that the model must use only tools present in the tool schema.🤖 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/chat/chat.ts` at line 365, The Chat Mode Note in the promptContent construction must not advertise Exa or Firecrawl unless those tools remain available. Update the flow around streamAIResponse and its tool filtering so the note is generated after filtering, or dynamically lists only tools present in the final tool schema while preserving the existing approval and mandatory tool-use guidance.apps/supercode-cli/server/src/cli/commands/slashCommands/token-limit.ts-147-150 (1)
147-150: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle budgets below 1,000 tokens.
DAILY_BUDGET_TOKENSaccepts values below 1,000, but this branch always divides by 1,000. A budget of500displays as1K budget, and a budget of1displays as0K budget. Add a raw-number branch or reuseformatTokensfromapps/supercode-cli/server/src/lib/token-budget.ts.🤖 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/commands/slashCommands/token-limit.ts` around lines 147 - 150, Update the budgetLabel calculation in the token-limit command to handle DAILY_BUDGET_TOKENS values below 1,000 without rounding them to an incorrect K value. Add a raw-token branch for sub-1,000 budgets or reuse formatTokens from token-budget.ts, while preserving the existing M and K formatting for larger budgets.cortex-sdk.md-50-54 (1)
50-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one filename for the ConcentrateAI provider.
The package tree uses
concentreai.ts, but Phase 2 usesconcentrateai.ts. Use one spelling in the tree, implementation phase, exports, and imports.Also applies to: 378-386
🤖 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 `@cortex-sdk.md` around lines 50 - 54, Standardize the ConcentrateAI provider filename spelling across the package tree, Phase 2 documentation, exports, and imports. Use the existing `concentreai.ts` or `concentrateai.ts` spelling consistently everywhere, including all references to the provider module.cortex-sdk.md-549-560 (1)
549-560: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
tasksinstead ofpipelinein the Turborepo block.This repo uses Turborepo v2 and already defines
tasksinturbo.json, so the documentation should add thecortex-sdkbuildtask undertasksinstead of the v1pipelinekey.🤖 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 `@cortex-sdk.md` around lines 549 - 560, Update the documented turbo.json example to use the existing Turborepo v2 tasks block instead of pipeline, adding the cortex-sdk build task with its dependsOn and outputs settings under tasks while preserving the shown configuration.cortex-sdk.md-269-273 (1)
269-273: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNarrow the
aipeer range to the supported major.This release declares
@ai-sdk/mcpv2, which targets the AI SDK v5 tooling, but the peer range also acceptsaiv4 and v6. Keepaiv5, or add explicit compatibility CI for v4/v6 before publishing the declared range.🤖 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 `@cortex-sdk.md` around lines 269 - 273, Update the Vercel AI SDK peer-dependency documentation to declare only the supported ai v5 major, removing v4 and v6 from the listed range unless explicit compatibility CI has been added for them.
🧹 Nitpick comments (5)
apps/supercode-cli/server/src/voice/voiceChatClient.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Bun file APIs for temporary audio.
Replace the synchronous
fswrite and delete calls with Bun file APIs. This avoids blocking file operations and follows the server runtime standard. Verify the declared Bun target before selecting the cleanup API.As per coding guidelines, "Prefer
Bun.fileover Node.jsnode:fsreadFile/writeFile for file operations."Also applies to: 70-72, 94-100
🤖 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/voice/voiceChatClient.ts` at line 4, Replace the synchronous writeFileSync and unlinkSync usage in the voice chat temporary-audio flow with Bun.file-based asynchronous file operations, preserving the existing file contents and cleanup behavior. Verify the declared Bun target and use the compatible Bun cleanup API for deleting the temporary file.Source: Coding guidelines
apps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.ts (2)
133-140: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a regression test for an unclosed JSON descriptor.
No test covers content that starts a descriptor and never closes it. That input pins the remainder of the stream in
pendingand then discards it on flush. See the comment onapps/supercode-cli/server/src/lib/embedded-tool-calls.tsLines 118-134.💚 Proposed test to add
test("unclosed json descriptor does not swallow the rest of the stream", () => { const out = drain( parseStreamedContent(), ['{"name":"run_command","parameters":{"command":"x"', " and then a lot more prose follows here"], ) expect(out.text).toContain("prose") })🤖 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/lib/__tests__/embedded-tool-calls.test.ts` around lines 133 - 140, Add a regression test alongside the existing JSON descriptor tests for an unclosed descriptor followed by prose, using parseStreamedContent and drain. Assert the flushed output preserves the trailing prose in out.text, ensuring pending incomplete descriptor content does not swallow the remainder of the stream.
4-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
drainwith the exported parser types instead ofany.
embedded-tool-calls.tsexportsEmbeddedToolCallandParsedBlock. Using them makes these tests fail at type-check time if the parser contract changes. The currentanyannotations remove that protection.♻️ Proposed refactor
import { test, expect } from "bun:test" -import { parseStreamedContent } from "../embedded-tool-calls" +import { parseStreamedContent } from "../embedded-tool-calls" +import type { EmbeddedToolCall, ParsedBlock } from "../embedded-tool-calls" -function drain(parser: any, chunks: string[]) { +type StreamParser = { push(chunk: string): ParsedBlock; flush(): ParsedBlock } + +function drain(parser: StreamParser, chunks: string[]): { text: string; calls: EmbeddedToolCall[] } { const text: string[] = [] - const calls: any[] = [] + const calls: EmbeddedToolCall[] = []Consider also exporting the parser handle type from
embedded-tool-calls.tsso the test does not restate it.As per coding guidelines: "explicitly export types needed by consumers, and prefer explicit return types on library functions".
🤖 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/lib/__tests__/embedded-tool-calls.test.ts` around lines 4 - 16, Update the test helper drain to use the exported EmbeddedToolCall and ParsedBlock types instead of any, including an explicit return type for its combined result. Export the parser handle type from embedded-tool-calls.ts if needed, then annotate the parser parameter with that type so the test remains checked against the parser contract.Source: Coding guidelines
apps/supercode-cli/server/src/index.ts (1)
1845-1845: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThese new routes use Express, but the path instructions require
Bun.serve().The two new endpoints register on the existing Express app. The path instructions for this file specify
Bun.serve()with routes support.Migrating the whole file is out of scope for this PR. Track the migration separately, or confirm that Express is an accepted exception here.
As per path instructions: "apps/supercode-cli/server/**/{index,server}.{ts,tsx}: Use
Bun.serve()with WebSocket and routes support instead of Express.js for server implementations".Also applies to: 1897-1897
🤖 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 1845, Replace the new Express registrations for the voice TTS endpoints in the `/api/voice/tts` and related route handlers with entries in the file’s `Bun.serve()` routes configuration, preserving their existing request and response behavior. Do not migrate unrelated Express routes; if the existing server structure cannot support this scoped change, track the migration separately rather than adding another Express exception.Source: Path instructions
cortex-sdk.md (1)
260-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the Composio dependency declaration.
The document calls
@composio/corean optional peer dependency, but the manifest declares it as an optional direct dependency. Choose one contract. UsepeerDependencieswith optional peer metadata for consumer-provided local mode, or update the documentation to describe an optional runtime dependency.Also applies to: 346-353
🤖 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 `@cortex-sdk.md` around lines 260 - 263, Align the Composio dependency contract between the `Composio` documentation section and the package manifest: either declare `@composio/core` in `peerDependencies` with optional peer metadata, or describe it as an optional runtime dependency wherever documented. Apply the same choice consistently in all referenced Composio documentation sections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70913a90-ef3b-493c-a511-f49abcbb746a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitignoreapps/supercode-cli/server/package.jsonapps/supercode-cli/server/src/cli/ai/chat/chat.tsapps/supercode-cli/server/src/cli/ai/server-proxy-service.tsapps/supercode-cli/server/src/cli/commands/slashCommands/token-limit.tsapps/supercode-cli/server/src/index.tsapps/supercode-cli/server/src/lib/__tests__/embedded-tool-calls.test.tsapps/supercode-cli/server/src/lib/embedded-tool-calls.tsapps/supercode-cli/server/src/lib/token-budget.tsapps/supercode-cli/server/src/voice/voiceChatClient.tscortex-sdk.mdpackage.json
| } catch (err: any) { | ||
| // Upstream hung/froze and AbortSignal.timeout fired — we treat this | ||
| // as "stream produced nothing", emit whatever we already have (if | ||
| // anything) and fall through to the non-streaming fallback below. | ||
| if (err?.name !== "AbortError") { | ||
| res.write(JSON.stringify({ type: "error", message: `Upstream failure: ${err?.message ?? String(err)}` }) + "\n") | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
break in this catch block is either a syntax error or it skips stream termination.
The try opens at Line 940 and the while (true) loop closes at Line 1020. The catch at Line 1021 is therefore outside the loop. A break statement inside it has no enclosing loop or switch within the catch.
Two outcomes are possible:
- If no outer loop encloses this try/catch,
breakis an illegal break statement and the module fails to parse. - If an outer loop does enclose it,
breakexits that outer loop. The handler then skips the parser flush at Lines 1033-1042, the non-streaming fallback at Line 1055, and thefinishevent at Line 1121. The client receives anerrorline but never a stream terminator.
Neither outcome is correct. Replace break with code that stops reading and continues to the termination path.
🐛 Proposed fix
} catch (err: any) {
// Upstream hung/froze and AbortSignal.timeout fired — we treat this
// as "stream produced nothing", emit whatever we already have (if
// anything) and fall through to the non-streaming fallback below.
if (err?.name !== "AbortError") {
res.write(JSON.stringify({ type: "error", message: `Upstream failure: ${err?.message ?? String(err)}` }) + "\n")
- break
}
}If the intent is to skip the fallback for non-abort errors, set a flag and test it at Line 1055 instead:
+ let upstreamFailed = false if (err?.name !== "AbortError") {
res.write(JSON.stringify({ type: "error", message: `Upstream failure: ${err?.message ?? String(err)}` }) + "\n")
- break
+ upstreamFailed = true
}- if (!fullContent.trim() && !emittedToolCalls) {
+ if (!upstreamFailed && !fullContent.trim() && !emittedToolCalls) {The same pattern exists at Lines 1274-1279.
Run this to confirm whether an outer loop encloses the try/catch:
#!/bin/bash
# Description: Inspect control flow around the streaming catch blocks.
set -euo pipefail
fd -t f 'index.ts' apps/supercode-cli/server/src --max-depth 1 --exec sed -n '860,945p' {}
echo "=== catch block 1 ==="
fd -t f 'index.ts' apps/supercode-cli/server/src --max-depth 1 --exec sed -n '1015,1035p' {}
echo "=== catch block 2 ==="
fd -t f 'index.ts' apps/supercode-cli/server/src --max-depth 1 --exec sed -n '1270,1285p' {}🤖 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 1021 - 1029, Replace the
invalid or prematurely exiting break statements in both streaming catch blocks
with control flow that records the non-AbortError failure, stops further
reading, and proceeds through parser flushing and stream termination; ensure the
fallback is skipped when appropriate while the response still emits its finish
terminator.
| app.post("/api/voice/chat", async (req, res) => { | ||
| try { | ||
| const { messages, model: modelParam, provider = "concentrateai" } = req.body | ||
| if (!messages || !Array.isArray(messages) || messages.length === 0) { | ||
| res.status(400).json({ error: "messages array is required" }) | ||
| return | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
/api/voice/chat has no authentication.
/api/voice/tts calls getUserFromBearer at Line 1847 and returns 401 for an unauthenticated caller. This handler performs no such check.
Any caller who can reach this route proxies arbitrary prompts to ConcentrateAI or OpenRouter using process.env.CONCENTRATEAI_API_KEY and process.env.OPENROUTER_API_KEY. The caller controls messages, model, and provider. The request bypasses the token-budget enforcement applied to the other chat routes in this file.
Add the same bearer check used by /api/voice/tts.
🔒️ Proposed fix
app.post("/api/voice/chat", async (req, res) => {
try {
+ const user = await getUserFromBearer(req)
+ if (!user) { res.status(401).json({ error: "Unauthorized" }); return }
+
const { messages, model: modelParam, provider = "concentrateai" } = req.body
if (!messages || !Array.isArray(messages) || messages.length === 0) {
res.status(400).json({ error: "messages array is required" })
return
}📝 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.
| app.post("/api/voice/chat", async (req, res) => { | |
| try { | |
| const { messages, model: modelParam, provider = "concentrateai" } = req.body | |
| if (!messages || !Array.isArray(messages) || messages.length === 0) { | |
| res.status(400).json({ error: "messages array is required" }) | |
| return | |
| } | |
| app.post("/api/voice/chat", async (req, res) => { | |
| try { | |
| const user = await getUserFromBearer(req) | |
| if (!user) { res.status(401).json({ error: "Unauthorized" }); return } | |
| const { messages, model: modelParam, provider = "concentrateai" } = req.body | |
| if (!messages || !Array.isArray(messages) || messages.length === 0) { | |
| res.status(400).json({ error: "messages array is required" }) | |
| return | |
| } |
🤖 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 1897 - 1903, Add the
same getUserFromBearer authentication check used by /api/voice/tts to the
/api/voice/chat handler before processing req.body, returning HTTP 401 for
unauthenticated callers. Preserve the existing validation and chat flow for
authenticated requests.
| while (i < block.length) { | ||
| // Find the next opener (square, xml, or bare JSON descriptor). | ||
| let nextOpen = -1 | ||
| let openKind: "square" | "xml" | "json" = "square" | ||
| const squareAt = block.indexOf("[TOOL_CALL]", i) | ||
| const squareAtLow = block.indexOf("[tool_call]", i) | ||
| const xmlAt = block.indexOf("<tool_call>", i) | ||
| const jsonAt = findJsonDescriptorStart(block, i) | ||
| let sq = squareAt | ||
| if (squareAtLow !== -1 && (sq === -1 || squareAtLow < sq)) sq = squareAtLow | ||
| if (sq !== -1) { nextOpen = sq; openKind = "square" } | ||
| if (xmlAt !== -1 && (nextOpen === -1 || xmlAt < nextOpen)) { nextOpen = xmlAt; openKind = "xml" } | ||
| if (jsonAt !== -1 && (nextOpen === -1 || jsonAt < nextOpen)) { nextOpen = jsonAt; openKind = "json" } | ||
|
|
||
| if (nextOpen === -1) { | ||
| text.push(block.slice(i)) | ||
| break | ||
| } | ||
|
|
||
| // Emit everything before the opener. | ||
| text.push(block.slice(i, nextOpen)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Partial opener markers split across chunks leak to the client.
pump emits the whole remaining block as text when it finds no complete opener. It does not hold back a trailing partial opener. If a chunk ends mid-marker, the parser leaks the marker fragment and loses the tool call.
Example sequence: push("Checking.[TOOL_") then push("CALL]\nrun_command --command=\"git diff\"\n[/TOOL_CALL]").
- Call 1 returns
text: "Checking.[TOOL_"andpending: "", sobufis cleared. - Call 2 never sees a complete
[TOOL_CALL]opener. The client receivesCALL]\nrun_command --command="git diff"\n[/TOOL_CALL]as raw prose, and no tool call is emitted.
The same failure applies to <tool_call> and to the {"name": JSON descriptor start. MiniMax streams token-sized deltas, so openers split across chunk boundaries are expected, not rare. This is the exact garbage-leak the module header describes preventing.
Withhold any trailing suffix of the block that is a proper prefix of a supported opener.
🐛 Proposed fix to buffer partial openers
if (nextOpen === -1) {
- text.push(block.slice(i))
+ const rest = block.slice(i)
+ const hold = partialOpenerLength(rest)
+ if (hold > 0) {
+ text.push(rest.slice(0, rest.length - hold))
+ pending = rest.slice(rest.length - hold)
+ } else {
+ text.push(rest)
+ }
break
}Add the helper alongside findJsonDescriptorStart:
const OPENERS = ["[TOOL_CALL]", "[tool_call]", "<tool_call>", '{"name"']
// Length of the trailing suffix of `s` that is a proper prefix of an opener.
function partialOpenerLength(s: string): number {
const max = Math.min(s.length, Math.max(...OPENERS.map((o) => o.length)) - 1)
for (let len = max; len > 0; len--) {
const suffix = s.slice(s.length - len)
if (OPENERS.some((o) => o.length > len && o.startsWith(suffix))) return len
}
return 0
}Note that flush() must then release a held partial opener that never completed, or a truncated tail is silently dropped.
🤖 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/lib/embedded-tool-calls.ts` around lines 96 -
116, Update pump’s no-opener path to retain any trailing suffix that is a proper
prefix of a supported opener ([TOOL_CALL], [tool_call], <tool_call>, or the JSON
descriptor) in the pending buffer instead of emitting it as text, while still
emitting preceding content. Add a helper near findJsonDescriptorStart to
calculate this partial-opener length, and update flush() to release any held
partial opener as text when the stream ends.
1 participant
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation