feat: add proto setup interactive wizard with STT configuration - #92
Merged
Merged
Conversation
- prepare-release.yml: fires on PR merge to dev (not main); version bump PR targets dev instead of main - release.yml: triggers on dev→main PR merge instead of commit message on push; adds sync-back step to keep dev aligned with main after release Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(ui): apply ASCII logo gradient by X column, not string index ink-gradient maps colors by character index across the whole string, so the p descender (last two lines) always got the tail/pink color regardless of its leftward visual position. Fix: render each logo line separately with its own <Gradient>, padded to logoWidth so column X maps to the same gradient fraction on every line. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): remove Static windowing that caused messages to disappear Ink's <Static> tracks rendered items by array INDEX, not React key. It stores the last array length and slices from that index on each render. When the array stops growing (constant length), the index overshoots and nothing new is printed — causing streamed messages to vanish. PR #45 introduced two patterns that broke this invariant: 1. STATIC_HISTORY_WINDOW=200 in MainContent.tsx — sliding window kept the array at a constant 204 items (3 fixed + 200 history + banner), so after the 201st history item nothing was ever printed by Static. 2. MAX_HISTORY_ITEMS=500 in useHistoryManager.ts — pruning the front of the array kept it at exactly 500 items, same effect. 3. Same AGENT_STATIC_HISTORY_WINDOW=200 windowing in AgentChatView.tsx. Fix: pass all history items to Static (array only ever grows). Remove TruncatedHistoryBanner from within Static (it can't update once committed to the terminal anyway, and its conditional insertion shifted existing indices on first appearance). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Automaker <automaker@localhost> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…rotection) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… failure
- schemaValidator: add Array.isArray guard so array tool params return
'Value of params must be an object' immediately instead of reaching AJV
- openai converter: return plain string content for text-only tool messages
instead of [{type:'text',...}] array — LiteLLM and most OpenAI-compatible
local providers only accept string content and crash on array content parts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Single-text tool responses (validation errors, simple outputs) now return
content as a plain string instead of [{type:'text',text:'...'}] array.
Many OpenAI-compatible providers (LiteLLM, local models) only accept string
content in tool messages and crash with 'Can only get item pairs from a
mapping' on array content.
Multi-part responses (text+media, multi-text blocks, unsupported media
placeholders) keep array format to preserve all content parts.
Reverts the overly broad Array.isArray guard in schemaValidator — AJV already
rejects arrays for object-typed schemas, and the guard incorrectly blocked
valid array inputs for 2020-12 prefixItems schemas.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on cascade When a weak/local model hits max_tokens and produces empty responses, tool errors accumulate in context causing subsequent calls to also fail with NO_RESPONSE_TEXT. Add trimToolErrorsFromContext() to strip trailing model-tool-call + user-tool-error pairs (up to 6 pairs), then attempt one final recovery call with the cleaned context before giving up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
When a model hits max_tokens mid-tool-call (producing Shell {}), the
truncation error gets re-added to context making the next turn also
overflow. At the start of each sendMessageStream, detect the cascade
(truncation-guidance marker in the last user turn) and pre-trim:
1. Remove the error tool-call pairs (trimToolErrorsFromContext)
2. Cap any large preceding tool responses to 10K chars
This prevents the Shell {} → error → Shell {} loop that affected both
weak models and frontier models (Claude Sonnet) on large tool outputs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… history When dev→main is squash-merged, the tag-to-tag git log only shows "chore: release" commits which get filtered, silently skipping the Discord post. Add a fallback that checks origin/dev (which retains the individual commits at release time) and a post-discord.yml workflow for manual backfill. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
ACP tests were hardcoded to use `methodId: 'openai'` and the e2e workflow passed OPENAI_API_KEY, which is not configured in CI. Since protoCLI uses Anthropic as its primary provider, update everything to use Anthropic auth: - authMethods.ts: expose USE_ANTHROPIC instead of USE_OPENAI - acp-integration.test.ts: change authenticate to methodId 'anthropic', update openaiModel selector to anthropicModel, skip qwen-oauth test (Qwen-specific model type, no equivalent in protoCLI) - acp-cron.test.ts: same authenticate change - e2e.yml: pass ANTHROPIC_API_KEY instead of OpenAI secrets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The LiteLLM gateway uses USE_OPENAI auth (OPENAI_API_KEY + OPENAI_BASE_URL + OPENAI_MODEL). The v0.25.17 change to Anthropic auth was incorrect. Reverts all test and workflow changes back to openai methodId and OPENAI_* secrets. The actual fix required is adding the three gateway secrets (OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL) to GitHub repository secrets. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ACP tests require gateway credentials that are not configured in CI. Since ACP is not currently in use, skip these tests automatically when OPENAI_API_KEY is absent rather than failing the E2E job. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
- sdk.ts: route Langfuse-only log/metric exporters to OTLP endpoints instead of ConsoleLog/MetricExporter to prevent terminal spam - loggingContentGenerator: wrap generateContentStream in llm.generate span; pass span into loggingStreamWrapper and close with token counts on success/error - agent-headless: create agent.execute span under turn context; wrap runReasoningLoop in otelContext.with() for proper child span linkage - harnessTelemetry: remove dead recordSprintContract() never called - gemini.tsx: register shutdownTelemetry() in cleanup so OTel SDK flushes spans before interactive REPL exits Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
- Persist cron jobs to disk so scheduled tasks survive session restarts - Inject capability manifest (active MCP tools) into system prompt to prevent hallucinated tool names - Add per-type memory staleness thresholds (project=21d, reference=7d) with inline freshness warnings - Route memory extractions through a proposal lane (/memory proposals|accept|reject) instead of writing directly - Track repeated tool denials in permission-blockers.json and inject reminders into system prompt - Add post-turn evolve pipeline: headless agent detects reusable workflow patterns every 3 turns and drafts SKILL.md candidates - Formalize prompt section volatility tags (stable/workspace/run) with assemblePromptSections() and CACHE_BOUNDARY_SENTINEL for provider-side caching Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…op malformed tool_calls - sdk.ts: useOtlp now requires config.getTelemetryEnabled(), so Langfuse-only users no longer spawn a gRPC exporter aimed at the localhost:4317 default and spam ECONNREFUSED. Closes the regression from 10c1bd0 that re-defaulted the endpoint. - streamingToolCallParser.ts: getCompletedToolCalls() now returns malformed=true when every parse strategy (JSON.parse, string auto-close, jsonrepair) fails or jsonrepair returns a non-object. Inlined jsonrepair so the fallback can be distinguished from a successful repair. - converter.ts: on a malformed tool_call during streaming, drop the functionCall entirely and emit a visible text note. Prevents poisoned tool_calls from entering conversation history, which was causing LiteLLM/Pydantic to fail subsequent turns with "Can only get item pairs from a mapping" when vLLM streams interleaved prose+JSON via a mismatched Qwen chat template. - Tests: regression for Langfuse-only no-OTLP path; malformed-flag parser coverage; converter drops malformed stream chunks and emits recovery text. Root cause remains upstream (vLLM --tool-call-parser qwen3_xml for Qwen3); this is defense-in-depth so proto degrades gracefully instead of hard-failing.
Interactive CLI command to configure model providers without manual JSON editing. Walks through provider selection, base URL, API key (masked input), live model discovery via /models endpoint, default model picker, and optional STT endpoint configuration. STT defaults to the same base URL as the configured provider so voice input works out of the box. - proto setup (yargs subcommand) - /setup (in-session slash command, points to terminal wizard) - fetchAvailableModels() utility with OpenAI-standard parsing - 16 unit tests for model discovery + slash command Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
WalkthroughAdds an interactive CLI setup wizard and model discovery, introduces a proposal-based memory workflow with accept/reject, implements permission-blocking and a background skill-evolution pass, improves streaming tool-call parsing and cron job persistence, and adjusts telemetry gating and various prompt/cron/storage integrations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/core/coreToolScheduler.ts (1)
1193-1201:⚠️ Potential issue | 🟠 MajorOnly record denials for explicit user rejection, not generic aborts.
Line 1193 includes
signal.aborted, but Line 1199 persists a denial unconditionally in that branch. Session/tool aborts can then be miscounted as permission denials, leading to incorrect persistent blocking behavior.💡 Suggested fix
- if (outcome === ToolConfirmationOutcome.Cancel || signal.aborted) { + const wasExplicitDeny = outcome === ToolConfirmationOutcome.Cancel; + if (wasExplicitDeny || signal.aborted) { // Use custom cancel message from payload if provided, otherwise use default const cancelMessage = payload?.cancelMessage || 'User did not allow tool call'; this.setStatusInternal(callId, 'cancelled', cancelMessage); - // Record the denial so persistent blockers can warn the agent next session - this.config - .getPermissionBlockerService?.() - ?.recordDenial(toolCall.request.name); + if (wasExplicitDeny) { + // Record explicit denial so persistent blockers reflect user intent. + this.config + .getPermissionBlockerService?.() + ?.recordDenial(toolCall.request.name); + } } else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/core/coreToolScheduler.ts` around lines 1193 - 1201, The current branch treats both explicit user cancellation and generic aborts the same, causing recordDenial to run even for aborted signals; change the logic so only explicit user rejections trigger persistent denials: keep the cancel message and setStatusInternal(callId, 'cancelled', ...), but call this.config.getPermissionBlockerService?.()?.recordDenial(toolCall.request.name) only when outcome === ToolConfirmationOutcome.Cancel (i.e., not when signal.aborted) so aborts are not recorded as denials.
🧹 Nitpick comments (6)
packages/core/src/services/evolveService.ts (2)
54-59: FixevolveSkilsDirtypo for clarity and future safety.
evolveSkilsDiris misspelled in variable names and context key (Lines 54, 99, 122). Rename toevolveSkillsDirconsistently to reduce accidental key mismatches later.Also applies to: 99-100, 122-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/evolveService.ts` around lines 54 - 59, Rename the misspelled identifier evolveSkilsDir to evolveSkillsDir everywhere in this module: update the variable declaration (const evolveSkilsDir), all references that read/write it, and any context key strings or object properties named "evolveSkilsDir" to "evolveSkillsDir" (e.g., usages around where the variable is created and later accessed). Ensure consistency for any functions or methods referencing it (search for evolveSkilsDir and replace), and update any exported names or imports from other files if they reference the old identifier so callers use evolveSkillsDir instead.
62-63: Avoid swallowing directory creation failures silently.Lines 62-63 suppress all mkdir errors, which makes operational failures hard to debug. At least log the error at debug level before continuing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/evolveService.ts` around lines 62 - 63, The two await fs.mkdir calls for evolveSkilsDir and proposalsDir currently swallow all errors; change them to capture errors and log at debug level before continuing (e.g., replace .catch(() => {}) with .catch(err => logger.debug(...) ) or wrap each mkdir in try/catch), referencing the mkdir calls for evolveSkilsDir and proposalsDir in evolveService.ts; ensure you use the repository's existing logger instance (e.g., processLogger or logger) and include the error object and path in the debug message.packages/core/src/telemetry/sdk.test.ts (1)
299-301: Optional: assertNodeSDKwas constructed before indexing the first call.Adding
expect(sdkCalls.length).toBe(1)before dereferencingsdkCalls[0]makes failures clearer and avoids accidental undefined access in this test path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/telemetry/sdk.test.ts` around lines 299 - 301, The test dereferences sdkCalls[0] without confirming NodeSDK was actually constructed; add an assertion like expect(sdkCalls.length).toBe(1) (or similar) before accessing sdkCalls[0] so the test fails with a clear message instead of throwing on undefined—update the assertions around NodeSDK/mock.calls and the spanProcessors extraction to include this check (references: NodeSDK, sdkCalls, spanProcessors).packages/core/src/services/permissionBlockerService.ts (1)
61-71: Consider edge case: denial count could grow unbounded.The count increments indefinitely. While this is fine for the prompt display ("denied 5x"), consider whether there's value in capping it or adding decay logic for very old denials.
This is a minor consideration for future enhancement — the current implementation is functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/permissionBlockerService.ts` around lines 61 - 71, The recordDenial method in permissionBlockerService currently increments denial counts without bound (recordDenial → this.denials), which could lead to unbounded growth; modify recordDenial to enforce a max cap (e.g., clamp count to MAX_DENIAL_COUNT) and/or implement decay by resetting or halving counts older than a threshold using lastSeenAt so very old denials don’t accumulate; update the DenialRecord handling in recordDenial and any persistent save/load logic so the cap/decay is respected when using saveToDisk and when reading from this.denials.packages/core/src/services/cronScheduler.ts (1)
291-292: Consider batching disk writes intick().
saveToDisk()is called inside the loop for each fired job. If multiple jobs fire in the same tick, this causes redundant disk writes.♻️ Suggested optimization
tick(now?: Date): void { const currentDate = now ?? new Date(); const currentMs = currentDate.getTime(); + let dirty = false; for (const job of this.jobs.values()) { // Check expiry if (currentMs >= job.expiresAt) { this.jobs.delete(job.id); + dirty = true; continue; } // ... matching logic ... if (!job.recurring) { this.jobs.delete(job.id); } - this.saveToDisk(); + dirty = true; if (this.onFire) { this.onFire(job); } } + + if (dirty) { + this.saveToDisk(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/cronScheduler.ts` around lines 291 - 292, The code currently calls saveToDisk() inside the per-job loop in tick(), causing redundant writes when multiple jobs fire in the same tick; change tick() to batch writes by deferring saveToDisk() until after the loop (or set a dirty flag like "needsPersist" inside the loop and call saveToDisk() once if set), referencing the CronScheduler.tick() loop and the saveToDisk() method so only a single disk write occurs per tick even when multiple jobs fire.packages/core/src/core/prompts.ts (1)
1278-1295: Consider deterministic sorting to reduce prompt churn.Ordering currently follows insertion order of
Map/arrays. Sorting servers and tool names before render makes capability manifests stable across runs and reduces avoidable prompt diffs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/core/prompts.ts` around lines 1278 - 1295, The MCP tools and skills are rendered in insertion order causing non-deterministic prompt churn; update the rendering in the block that uses mcpToolsByServer and activeSkills so you iterate deterministically: for mcpToolsByServer, get server keys, sort them, and for each server sort the tools list (prior to tools.join) before pushing the line; for activeSkills, sort the activeSkills array (or a shallow copy) by skill.name before slicing descriptions and emitting `/skill.name` lines; keep the same symbols mcpToolsByServer, tools, skill, and activeSkills so only the iteration order changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/setup/handler.ts`:
- Around line 543-544: The code is persisting raw environment secrets into
settings by calling settings.setValue(scope, `env.${envKey}`, apiKey) with the
value returned from promptApiKey(); change this so you do NOT write back an
env-sourced secret. Update promptApiKey() usage (or its return) to indicate
source (e.g., return { key, source } or provide a boolean like isFromEnv) and in
the handler guard the write: only call settings.setValue(scope, `env.${envKey}`,
apiKey) when the key was entered interactively (not sourced from process.env);
if you still need to record that the env key exists, store a non-secret sentinel
(e.g., 'ENV') or a metadata flag instead of the raw secret.
- Around line 377-488: The promptText and promptMaskedText helpers directly
toggle stdin.setRawMode and write to stdout, bypassing the CLI's Ink-based UI;
replace these raw-mode implementations with Ink-driven components used by the
setup wizard (so the interactive prompting composes with the rest of
packages/cli). Refactor calls to promptText and promptMaskedText into Ink
prompts (e.g., create a PromptText and MaskedPrompt React components) that
render using Ink hooks (useInput/useStdout) and return values via
props/callbacks or a Promise wrapper used by the handler; remove direct
stdin.setRawMode/stdout.write usage from promptText/promptMaskedText (or convert
them into adapters that simply mount the Ink component and resolve when it
completes) so all terminal I/O is handled through Ink's input/output APIs and
the components can be composed with the existing CLI UI.
- Around line 321-324: The version-presence check incorrectly only matches
numeric versions so a base like '/v1beta' doesn't count and you end up appending
'/v1' to produce '/v1beta/v1'; update the check around the normalised variable
(the "Ensure /v1 is present" block that tests normalised.match(...)) to use a
regex that accepts alphanumeric version segments (e.g., '/v' followed by letters
and/or digits) instead of only digits so paths like '/v1beta' are treated as
valid and you won't append '/v1'.
- Around line 37-42: The Anthropic preset is incorrectly using
AuthType.USE_OPENAI; update the anthropic preset object in handler.ts to set
authType to AuthType.USE_ANTHROPIC so the CLI saves and routes Anthropic
credentials through the correct auth flow (modify the anthropic object's
authType property accordingly).
In `@packages/cli/src/commands/setup/modelDiscovery.ts`:
- Around line 89-99: The code in modelDiscovery.ts appends "/v1" by checking
only purely numeric version suffixes; update the regex checks on the normalised
variable to treat versioned bases like "/v1beta" as already-versioned. Replace
the existing /\/v\d+$/ and /\/v\d+\// tests with patterns that allow
alphanumeric version suffixes (e.g. /\/v\d+[A-Za-z0-9_-]*$/ and
/\/v\d+[A-Za-z0-9_-]*\//) so strings like "/v1beta" or "/v2alpha" are detected
and "/v1" is not appended.
In `@packages/core/src/core/openaiContentGenerator/streamingToolCallParser.ts`:
- Around line 306-312: The error log in streamingToolCallParser.ts currently
emits raw buffer previews via debugLogger.error (the call that includes index,
bufferPreview: buffer.slice(0,200), bufferLength), which can leak sensitive
data; change this to avoid logging raw content by removing bufferPreview or
replacing it with a non-sensitive artifact (e.g., bufferLength plus a
deterministic hash/digest of the preview or a fully redacted marker). Update the
debugLogger.error invocation in the parsing routine that handles malformed
tool_call arguments (the function invoking debugLogger.error with index and
buffer) to only emit structural metadata (index, bufferLength) and optionally a
SHA-256 or other stable hash of the slice instead of the plain text preview.
In `@packages/core/src/core/prompts.ts`:
- Around line 1280-1293: The code currently injects raw manifest fields into the
system prompt in the mcpToolsByServer loop and the activeSkills loop (see the
for (const [server, tools] of mcpToolsByServer) block and the for (const skill
of activeSkills) block), so sanitize server, each tool, skill.name and
skill.description before formatting: strip or replace newlines and control
characters, remove or escape markdown-sensitive characters (e.g., *, _, `, #, >,
-, [, ], (, )), trim and then apply existing truncation for descriptions; use
the sanitized values when building lines and when joining tools to prevent
prompt-structure injection.
In `@packages/core/src/memory/proposalStore.ts`:
- Around line 81-107: acceptProposal and rejectProposal currently trust
proposalFilePath and can rename/unlink arbitrary files; before performing
fs.rename or fs.unlink validate that proposalFilePath resolves inside the
proposals directory for the given scope. Use getMemoryDir(scope, cwd) (or
getMemoryDir for reject by deriving scope if available) and compute an absolute
resolved path for both memoryDir and proposalFilePath (e.g., path.resolve) then
ensure the proposal path is a child of memoryDir (e.g., check
path.relative(memoryDir, proposalPath) does not start with '..' and is not equal
to '..'); if the check fails return null (for acceptProposal) or false (for
rejectProposal) and do not perform fs operations. Also keep existing mkdir and
regenerateIndex calls only after the validation passes.
In `@packages/core/src/services/evolveService.ts`:
- Around line 37-49: The code uses a process-global turnsSinceLastReview and
lacks an in-flight guard, causing cadence to be shared across Configs and
allowing overlapping runEvolvePass executions; change to per-Config state (e.g.,
a WeakMap<Config, { turns: number; running: boolean }>) keyed by the Config
instance and update runEvolvePass to read/increment the per-Config turns counter
(using SKILL_REVIEW_INTERVAL) and return early per-instance, and add an
in-flight guard (set running=true before the async work in runEvolvePass and
clear it in a finally block) so concurrent calls for the same Config are
serialized.
---
Outside diff comments:
In `@packages/core/src/core/coreToolScheduler.ts`:
- Around line 1193-1201: The current branch treats both explicit user
cancellation and generic aborts the same, causing recordDenial to run even for
aborted signals; change the logic so only explicit user rejections trigger
persistent denials: keep the cancel message and setStatusInternal(callId,
'cancelled', ...), but call
this.config.getPermissionBlockerService?.()?.recordDenial(toolCall.request.name)
only when outcome === ToolConfirmationOutcome.Cancel (i.e., not when
signal.aborted) so aborts are not recorded as denials.
---
Nitpick comments:
In `@packages/core/src/core/prompts.ts`:
- Around line 1278-1295: The MCP tools and skills are rendered in insertion
order causing non-deterministic prompt churn; update the rendering in the block
that uses mcpToolsByServer and activeSkills so you iterate deterministically:
for mcpToolsByServer, get server keys, sort them, and for each server sort the
tools list (prior to tools.join) before pushing the line; for activeSkills, sort
the activeSkills array (or a shallow copy) by skill.name before slicing
descriptions and emitting `/skill.name` lines; keep the same symbols
mcpToolsByServer, tools, skill, and activeSkills so only the iteration order
changes.
In `@packages/core/src/services/cronScheduler.ts`:
- Around line 291-292: The code currently calls saveToDisk() inside the per-job
loop in tick(), causing redundant writes when multiple jobs fire in the same
tick; change tick() to batch writes by deferring saveToDisk() until after the
loop (or set a dirty flag like "needsPersist" inside the loop and call
saveToDisk() once if set), referencing the CronScheduler.tick() loop and the
saveToDisk() method so only a single disk write occurs per tick even when
multiple jobs fire.
In `@packages/core/src/services/evolveService.ts`:
- Around line 54-59: Rename the misspelled identifier evolveSkilsDir to
evolveSkillsDir everywhere in this module: update the variable declaration
(const evolveSkilsDir), all references that read/write it, and any context key
strings or object properties named "evolveSkilsDir" to "evolveSkillsDir" (e.g.,
usages around where the variable is created and later accessed). Ensure
consistency for any functions or methods referencing it (search for
evolveSkilsDir and replace), and update any exported names or imports from other
files if they reference the old identifier so callers use evolveSkillsDir
instead.
- Around line 62-63: The two await fs.mkdir calls for evolveSkilsDir and
proposalsDir currently swallow all errors; change them to capture errors and log
at debug level before continuing (e.g., replace .catch(() => {}) with .catch(err
=> logger.debug(...) ) or wrap each mkdir in try/catch), referencing the mkdir
calls for evolveSkilsDir and proposalsDir in evolveService.ts; ensure you use
the repository's existing logger instance (e.g., processLogger or logger) and
include the error object and path in the debug message.
In `@packages/core/src/services/permissionBlockerService.ts`:
- Around line 61-71: The recordDenial method in permissionBlockerService
currently increments denial counts without bound (recordDenial → this.denials),
which could lead to unbounded growth; modify recordDenial to enforce a max cap
(e.g., clamp count to MAX_DENIAL_COUNT) and/or implement decay by resetting or
halving counts older than a threshold using lastSeenAt so very old denials don’t
accumulate; update the DenialRecord handling in recordDenial and any persistent
save/load logic so the cap/decay is respected when using saveToDisk and when
reading from this.denials.
In `@packages/core/src/telemetry/sdk.test.ts`:
- Around line 299-301: The test dereferences sdkCalls[0] without confirming
NodeSDK was actually constructed; add an assertion like
expect(sdkCalls.length).toBe(1) (or similar) before accessing sdkCalls[0] so the
test fails with a clear message instead of throwing on undefined—update the
assertions around NodeSDK/mock.calls and the spanProcessors extraction to
include this check (references: NodeSDK, sdkCalls, spanProcessors).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3ee776bc-2737-48f6-977a-954de4c84625
⛔ Files ignored due to path filters (1)
packages/core/src/core/__snapshots__/prompts.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (33)
packages/cli/src/commands/setup.tspackages/cli/src/commands/setup/handler.tspackages/cli/src/commands/setup/modelDiscovery.test.tspackages/cli/src/commands/setup/modelDiscovery.tspackages/cli/src/config/config.tspackages/cli/src/services/BuiltinCommandLoader.tspackages/cli/src/ui/commands/memoryCommand.tspackages/cli/src/ui/commands/setupCommand.test.tspackages/cli/src/ui/commands/setupCommand.tspackages/cli/src/ui/hooks/useGeminiStream.tspackages/core/src/config/config.tspackages/core/src/config/storage.tspackages/core/src/core/client.test.tspackages/core/src/core/client.tspackages/core/src/core/coreToolScheduler.tspackages/core/src/core/openaiContentGenerator/converter.test.tspackages/core/src/core/openaiContentGenerator/converter.tspackages/core/src/core/openaiContentGenerator/streamingToolCallParser.test.tspackages/core/src/core/openaiContentGenerator/streamingToolCallParser.tspackages/core/src/core/prompts.tspackages/core/src/index.tspackages/core/src/memory/index.tspackages/core/src/memory/memoryAge.tspackages/core/src/memory/memoryExtractor.tspackages/core/src/memory/memoryStore.tspackages/core/src/memory/proposalStore.tspackages/core/src/services/cronScheduler.test.tspackages/core/src/services/cronScheduler.tspackages/core/src/services/evolveService.tspackages/core/src/services/memory-consolidation.tspackages/core/src/services/permissionBlockerService.tspackages/core/src/telemetry/sdk.test.tspackages/core/src/telemetry/sdk.ts
Comment on lines
+377
to
+488
| function promptText(label: string): Promise<string> { | ||
| return new Promise<string>((resolve, reject) => { | ||
| stdout.write(label); | ||
|
|
||
| if (!stdin.setRawMode) { | ||
| reject( | ||
| new Error( | ||
| 'Raw mode not available. Please run in an interactive terminal.', | ||
| ), | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const wasRaw = stdin.isRaw; | ||
| stdin.setRawMode(true); | ||
| stdin.resume(); | ||
| stdin.setEncoding('utf8'); | ||
|
|
||
| let input = ''; | ||
|
|
||
| const onData = (chunk: string) => { | ||
| for (const char of chunk) { | ||
| switch (char) { | ||
| case '\r': | ||
| case '\n': | ||
| stdin.removeListener('data', onData); | ||
| stdin.setRawMode(wasRaw); | ||
| stdout.write('\n'); | ||
| resolve(input); | ||
| return; | ||
| case '\x03': // Ctrl+C | ||
| stdin.removeListener('data', onData); | ||
| stdin.setRawMode(wasRaw); | ||
| stdout.write('\n'); | ||
| reject(new Error('Interrupted')); | ||
| return; | ||
| case '\x08': // Backspace | ||
| case '\x7F': // Delete | ||
| if (input.length > 0) { | ||
| input = input.slice(0, -1); | ||
| stdout.write('\x1B[D \x1B[D'); | ||
| } | ||
| break; | ||
| default: | ||
| if (char.charCodeAt(0) >= 32) { | ||
| input += char; | ||
| stdout.write(char); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| stdin.on('data', onData); | ||
| }); | ||
| } | ||
|
|
||
| function promptMaskedText(label: string): Promise<string> { | ||
| return new Promise<string>((resolve, reject) => { | ||
| stdout.write(label); | ||
|
|
||
| if (!stdin.setRawMode) { | ||
| reject( | ||
| new Error( | ||
| 'Raw mode not available. Please run in an interactive terminal.', | ||
| ), | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const wasRaw = stdin.isRaw; | ||
| stdin.setRawMode(true); | ||
| stdin.resume(); | ||
| stdin.setEncoding('utf8'); | ||
|
|
||
| let input = ''; | ||
|
|
||
| const onData = (chunk: string) => { | ||
| for (const char of chunk) { | ||
| switch (char) { | ||
| case '\r': | ||
| case '\n': | ||
| stdin.removeListener('data', onData); | ||
| stdin.setRawMode(wasRaw); | ||
| stdout.write('\n'); | ||
| resolve(input); | ||
| return; | ||
| case '\x03': // Ctrl+C | ||
| stdin.removeListener('data', onData); | ||
| stdin.setRawMode(wasRaw); | ||
| stdout.write('\n'); | ||
| reject(new Error('Interrupted')); | ||
| return; | ||
| case '\x08': | ||
| case '\x7F': | ||
| if (input.length > 0) { | ||
| input = input.slice(0, -1); | ||
| stdout.write('\x1B[D \x1B[D'); | ||
| } | ||
| break; | ||
| default: | ||
| if (char.charCodeAt(0) >= 32) { | ||
| input += char; | ||
| stdout.write('*'); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| stdin.on('data', onData); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move the wizard prompts onto Ink instead of manual raw-mode I/O.
These helpers toggle raw mode and paint the terminal directly, which bypasses the CLI’s UI framework and will be harder to compose with the rest of packages/cli.
As per coding guidelines, packages/cli/**/*.{ts,tsx}: Use Ink (React for CLI) for building terminal user interfaces.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup/handler.ts` around lines 377 - 488, The
promptText and promptMaskedText helpers directly toggle stdin.setRawMode and
write to stdout, bypassing the CLI's Ink-based UI; replace these raw-mode
implementations with Ink-driven components used by the setup wizard (so the
interactive prompting composes with the rest of packages/cli). Refactor calls to
promptText and promptMaskedText into Ink prompts (e.g., create a PromptText and
MaskedPrompt React components) that render using Ink hooks (useInput/useStdout)
and return values via props/callbacks or a Promise wrapper used by the handler;
remove direct stdin.setRawMode/stdout.write usage from
promptText/promptMaskedText (or convert them into adapters that simply mount the
Ink component and resolve when it completes) so all terminal I/O is handled
through Ink's input/output APIs and the components can be composed with the
existing CLI UI.
Comment on lines
+306
to
+312
| debugLogger.error( | ||
| 'Failed to parse tool_call arguments; marking malformed', | ||
| { | ||
| index, | ||
| bufferPreview: buffer.slice(0, 200), | ||
| bufferLength: buffer.length, | ||
| }, |
There was a problem hiding this comment.
Avoid logging raw tool-call argument previews.
Lines 306-312 log the first 200 characters of a malformed arguments buffer at error level. These buffers can easily contain file contents, prompts, secrets, or PII, so a parser failure becomes a sensitive-data logging path. Log only structural metadata here, or a fully redacted/hash-based preview.
🔒 Suggested change
debugLogger.error(
'Failed to parse tool_call arguments; marking malformed',
{
index,
- bufferPreview: buffer.slice(0, 200),
bufferLength: buffer.length,
+ depth: this.depths.get(index) ?? 0,
+ inString: this.inStrings.get(index) ?? false,
},
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/core/openaiContentGenerator/streamingToolCallParser.ts`
around lines 306 - 312, The error log in streamingToolCallParser.ts currently
emits raw buffer previews via debugLogger.error (the call that includes index,
bufferPreview: buffer.slice(0,200), bufferLength), which can leak sensitive
data; change this to avoid logging raw content by removing bufferPreview or
replacing it with a non-sensitive artifact (e.g., bufferLength plus a
deterministic hash/digest of the preview or a fully redacted marker). Update the
debugLogger.error invocation in the parsing routine that handles malformed
tool_call arguments (the function invoking debugLogger.error with index and
buffer) to only emit structural metadata (index, bufferLength) and optionally a
SHA-256 or other stable hash of the slice instead of the plain text preview.
Comment on lines
+1280
to
+1293
| for (const [server, tools] of mcpToolsByServer) { | ||
| lines.push(` • ${server}: ${tools.join(', ')}`); | ||
| } | ||
| sections.push(lines.join('\n')); | ||
| } | ||
|
|
||
| if (activeSkills.length > 0) { | ||
| const lines: string[] = ['**Skills (invoke via /skill or agent tool):**']; | ||
| for (const skill of activeSkills) { | ||
| const desc = skill.description | ||
| ? ` — ${skill.description.slice(0, 80)}${skill.description.length > 80 ? '…' : ''}` | ||
| : ''; | ||
| lines.push(` • /${skill.name}${desc}`); | ||
| } |
There was a problem hiding this comment.
Sanitize manifest fields before embedding into the system prompt.
Line 1281 and Line 1292 inject server, tool, skill.name, and skill.description directly into markdown. If any of these contain newlines/markdown control text, they can break prompt structure and inject unintended instructions.
🔧 Suggested hardening diff
+function sanitizePromptText(value: string): string {
+ return value
+ .replace(/[\r\n\t]+/g, ' ')
+ .replace(/[<>`]/g, '')
+ .trim();
+}
+
export function buildCapabilityManifest(
mcpToolsByServer: Map<string, string[]>,
activeSkills: Array<{ name: string; description: string }>,
): string | null {
const sections: string[] = [];
@@
if (mcpToolsByServer.size > 0) {
const lines: string[] = ['**MCP tools available this session:**'];
for (const [server, tools] of mcpToolsByServer) {
- lines.push(` • ${server}: ${tools.join(', ')}`);
+ const safeServer = sanitizePromptText(server);
+ const safeTools = tools.map(sanitizePromptText);
+ lines.push(` • ${safeServer}: ${safeTools.join(', ')}`);
}
sections.push(lines.join('\n'));
}
@@
for (const skill of activeSkills) {
- const desc = skill.description
- ? ` — ${skill.description.slice(0, 80)}${skill.description.length > 80 ? '…' : ''}`
+ const safeName = sanitizePromptText(skill.name);
+ const safeDescription = sanitizePromptText(skill.description ?? '');
+ const desc = safeDescription
+ ? ` — ${safeDescription.slice(0, 80)}${safeDescription.length > 80 ? '…' : ''}`
: '';
- lines.push(` • /${skill.name}${desc}`);
+ lines.push(` • /${safeName}${desc}`);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/core/prompts.ts` around lines 1280 - 1293, The code
currently injects raw manifest fields into the system prompt in the
mcpToolsByServer loop and the activeSkills loop (see the for (const [server,
tools] of mcpToolsByServer) block and the for (const skill of activeSkills)
block), so sanitize server, each tool, skill.name and skill.description before
formatting: strip or replace newlines and control characters, remove or escape
markdown-sensitive characters (e.g., *, _, `, #, >, -, [, ], (, )), trim and
then apply existing truncation for descriptions; use the sanitized values when
building lines and when joining tools to prevent prompt-structure injection.
Comment on lines
+81
to
+107
| export async function acceptProposal( | ||
| proposalFilePath: string, | ||
| scope: MemoryScope, | ||
| cwd?: string, | ||
| ): Promise<string | null> { | ||
| const memoryDir = getMemoryDir(scope, cwd); | ||
| const destPath = path.join(memoryDir, path.basename(proposalFilePath)); | ||
|
|
||
| try { | ||
| await fs.mkdir(memoryDir, { recursive: true }); | ||
| await fs.rename(proposalFilePath, destPath); | ||
| await regenerateIndex(scope, cwd); | ||
| return destPath; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reject (delete) a proposal file. | ||
| */ | ||
| export async function rejectProposal( | ||
| proposalFilePath: string, | ||
| ): Promise<boolean> { | ||
| try { | ||
| await fs.unlink(proposalFilePath); | ||
| return true; |
There was a problem hiding this comment.
Enforce proposals-directory boundary before rename/delete.
Line 91 and Line 106 trust proposalFilePath without validation. This allows moving/deleting arbitrary files if an unexpected path reaches these APIs.
🔒 Suggested guardrail diff
export const PROPOSALS_DIR_NAME = 'proposals';
+
+function isPathInside(parentDir: string, candidatePath: string): boolean {
+ const rel = path.relative(parentDir, candidatePath);
+ return rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel);
+}
@@
export async function acceptProposal(
proposalFilePath: string,
scope: MemoryScope,
cwd?: string,
): Promise<string | null> {
+ const proposalsDir = getProposalsDir(scope, cwd);
+ const resolvedSource = path.resolve(proposalFilePath);
+ const resolvedProposalsDir = path.resolve(proposalsDir);
+ if (!isPathInside(resolvedProposalsDir, resolvedSource)) return null;
+
const memoryDir = getMemoryDir(scope, cwd);
- const destPath = path.join(memoryDir, path.basename(proposalFilePath));
+ const destPath = path.join(memoryDir, path.basename(resolvedSource));
@@
- await fs.rename(proposalFilePath, destPath);
+ await fs.rename(resolvedSource, destPath);
@@
export async function rejectProposal(
- proposalFilePath: string,
+ proposalFilePath: string,
+ scope: MemoryScope,
+ cwd?: string,
): Promise<boolean> {
+ const proposalsDir = getProposalsDir(scope, cwd);
+ const resolvedSource = path.resolve(proposalFilePath);
+ const resolvedProposalsDir = path.resolve(proposalsDir);
+ if (!isPathInside(resolvedProposalsDir, resolvedSource)) return false;
+
try {
- await fs.unlink(proposalFilePath);
+ await fs.unlink(resolvedSource);
return true;
} catch {
return false;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/memory/proposalStore.ts` around lines 81 - 107,
acceptProposal and rejectProposal currently trust proposalFilePath and can
rename/unlink arbitrary files; before performing fs.rename or fs.unlink validate
that proposalFilePath resolves inside the proposals directory for the given
scope. Use getMemoryDir(scope, cwd) (or getMemoryDir for reject by deriving
scope if available) and compute an absolute resolved path for both memoryDir and
proposalFilePath (e.g., path.resolve) then ensure the proposal path is a child
of memoryDir (e.g., check path.relative(memoryDir, proposalPath) does not start
with '..' and is not equal to '..'); if the check fails return null (for
acceptProposal) or false (for rejectProposal) and do not perform fs operations.
Also keep existing mkdir and regenerateIndex calls only after the validation
passes.
Comment on lines
+37
to
+49
| let turnsSinceLastReview = 0; | ||
|
|
||
| /** | ||
| * Call after each agent turn completes. Runs skill candidate detection every | ||
| * SKILL_REVIEW_INTERVAL turns. Fire-and-forget; errors are logged only. | ||
| */ | ||
| export async function runEvolvePass( | ||
| config: Config, | ||
| recentMessages: Array<{ role: string; text: string }>, | ||
| ): Promise<void> { | ||
| turnsSinceLastReview++; | ||
| if (turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return; | ||
| turnsSinceLastReview = 0; |
There was a problem hiding this comment.
Global turn state can cross-contaminate sessions and allow overlapping runs.
Lines 37-49 use a process-global counter, so separate Config instances share cadence. Also, there is no in-flight guard around Lines 88-103, so multiple evolve passes can overlap under fast consecutive turns.
Suggested fix
-let turnsSinceLastReview = 0;
+type EvolveState = { turnsSinceLastReview: number; inProgress: boolean };
+const evolveStateByConfig = new WeakMap<Config, EvolveState>();
+
+function getEvolveState(config: Config): EvolveState {
+ let state = evolveStateByConfig.get(config);
+ if (!state) {
+ state = { turnsSinceLastReview: 0, inProgress: false };
+ evolveStateByConfig.set(config, state);
+ }
+ return state;
+}
@@
export async function runEvolvePass(
config: Config,
recentMessages: Array<{ role: string; text: string }>,
): Promise<void> {
- turnsSinceLastReview++;
- if (turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return;
- turnsSinceLastReview = 0;
+ const state = getEvolveState(config);
+ if (state.inProgress) return;
+
+ state.turnsSinceLastReview++;
+ if (state.turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return;
+ state.turnsSinceLastReview = 0;
@@
- try {
+ state.inProgress = true;
+ try {
@@
} catch (err) {
logger.debug('Evolve pass skipped:', err);
+ } finally {
+ state.inProgress = false;
}
}Also applies to: 88-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/services/evolveService.ts` around lines 37 - 49, The code
uses a process-global turnsSinceLastReview and lacks an in-flight guard, causing
cadence to be shared across Configs and allowing overlapping runEvolvePass
executions; change to per-Config state (e.g., a WeakMap<Config, { turns: number;
running: boolean }>) keyed by the Config instance and update runEvolvePass to
read/increment the per-Config turns counter (using SKILL_REVIEW_INTERVAL) and
return early per-instance, and add an in-flight guard (set running=true before
the async work in runEvolvePass and clear it in a finally block) so concurrent
calls for the same Config are serialized.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- Fix Anthropic preset using wrong AuthType (USE_OPENAI → USE_ANTHROPIC) - Fix URL normalisation clobbering /v1beta and other versioned paths (Gemini, custom endpoints) — both in modelDiscovery and STT URL builder - Don't persist API keys that came from env vars (prevent silent promotion of env-only secrets to plaintext settings.json) - Add regression tests for /v1beta and /v1/path URL patterns (16→18) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/cli/src/commands/setup/handler.ts (1)
392-503: 🛠️ Refactor suggestion | 🟠 MajorUse Ink prompts here instead of manual raw-mode I/O.
These helpers bypass the CLI UI framework and will be hard to compose with the rest of
packages/cli. Please move text and masked input onto Ink components/adapters instead of toggling raw mode directly. As per coding guidelines,packages/cli/**/*.{ts,tsx}: Use Ink (React for CLI) for building terminal user interfaces.#!/bin/bash # Verify the setup handler is doing manual terminal I/O while the CLI package uses Ink patterns. rg -n "setRawMode\\(|stdout\\.write\\(|stdin\\.on\\('data'" packages/cli/src/commands/setup/handler.ts fd -e ts -e tsx . packages/cli/src | xargs rg -n "from ['\"]ink['\"]|useInput\\(|render\\("🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/setup/handler.ts` around lines 392 - 503, The promptText and promptMaskedText helpers perform manual raw-mode terminal I/O (stdin.setRawMode, stdin.on('data'), stdout.write) which bypasses the project's Ink-based CLI UI; replace these functions with Ink-based prompts/adapters (e.g., ink-text-input or Ink's useInput within React components) so input and masked input are rendered and handled as Ink components instead of toggling raw mode directly; remove usage of stdin.setRawMode, stdin.resume, stdin.on('data'), and stdout.write inside promptText and promptMaskedText and refactor callers to render an Ink component that returns the entered value (plain or masked) via props/callbacks or a promise adapter that mounts/unmounts the Ink component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/setup/handler.ts`:
- Line 95: Remove the direct process.exit(...) calls from the shared
runSetupWizard handler and any helper functions it calls (referenced in this
file around runSetupWizard and the blocks corresponding to the
success/cancel/failure flows), and instead return an explicit status value or
throw an Error to indicate success, cancellation, or failure; update
runSetupWizard to propagate that status/exception to its caller and ensure the
top-level CLI entrypoint (the command handlers that call runSetupWizard) decides
whether to call process.exit based on the returned status or caught exception.
- Around line 572-577: When sttConfig is null the code currently leaves previous
STT values intact; update the block handling sttConfig so that the else branch
explicitly disables voice and clears stale STT settings: in the handler where
sttConfig is checked (the block calling settings.setValue(scope,
'voice.enabled', ...), settings.setValue(scope, 'voice.sttEndpoint', ...) and
settings.setValue(scope, 'voice.sttEnvKey', ...)), add an else that calls
settings.setValue(scope, 'voice.enabled', false) and clears or sets ''/null for
'voice.sttEndpoint' and 'voice.sttEnvKey' to ensure the old backend is not left
active.
- Line 74: The call to fetchAvailableModels(baseUrl, apiKey) omits
preset.authType so discovery always uses OpenAI-style Authorization and parsing;
update the call to pass preset.authType and update fetchAvailableModels to
accept an authType parameter (or branch inside setup handler before calling) and
handle provider-specific headers and response parsing for "openai" vs
"anthropic" vs "gemini" (e.g., use X-Api-Key + anthropic-version for Anthropic,
x-goog-api-key / query params for Gemini) or short-circuit and skip
auto-discovery for non-OpenAI-compatible presets; locate uses of
fetchAvailableModels and preset.authType to implement the branch and ensure
model-parser logic inside fetchAvailableModels (or the new branch) correctly
maps each provider's response shape to the common models format.
---
Duplicate comments:
In `@packages/cli/src/commands/setup/handler.ts`:
- Around line 392-503: The promptText and promptMaskedText helpers perform
manual raw-mode terminal I/O (stdin.setRawMode, stdin.on('data'), stdout.write)
which bypasses the project's Ink-based CLI UI; replace these functions with
Ink-based prompts/adapters (e.g., ink-text-input or Ink's useInput within React
components) so input and masked input are rendered and handled as Ink components
instead of toggling raw mode directly; remove usage of stdin.setRawMode,
stdin.resume, stdin.on('data'), and stdout.write inside promptText and
promptMaskedText and refactor callers to render an Ink component that returns
the entered value (plain or masked) via props/callbacks or a promise adapter
that mounts/unmounts the Ink component.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 117d0959-c659-4041-854f-340f6d2eeec2
📒 Files selected for processing (3)
packages/cli/src/commands/setup/handler.tspackages/cli/src/commands/setup/modelDiscovery.test.tspackages/cli/src/commands/setup/modelDiscovery.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/commands/setup/modelDiscovery.ts
- packages/cli/src/commands/setup/modelDiscovery.test.ts
|
|
||
| // Step 4: Discover models | ||
| writeStdoutLine('\n⏳ Discovering models...\n'); | ||
| const { models, error } = await fetchAvailableModels(baseUrl, apiKey); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic API official docs: list models required headers x-api-key anthropic-version Gemini API official docs: models.list REST endpoint and authentication format
💡 Result:
Anthropic API (Claude): List models endpoint: GET https://api.anthropic.com/v1/models Required headers: - X-Api-Key: $ANTHROPIC_API_KEY (your secret API key from console) - anthropic-version: 2023-06-01 (or current version) Example: curl https://api.anthropic.com/v1/models \ -H 'anthropic-version: 2023-06-01' \ -H "X-Api-Key: $ANTHROPIC_API_KEY" Content-Type: application/json is also typically required for API calls, but since this is GET with no body, it may be optional. Gemini API (Google Generative Language API): List models endpoint: GET https://generativelanguage.googleapis.com/v1beta/models Query params (optional): pageSize (max 1000, default 50), pageToken Authentication: Header x-goog-api-key: $GEMINI_API_KEY (API key from Google AI Studio) No body. Content-Type: application/json optional for GET. Example (inferred from docs patterns): curl "https://generativelanguage.googleapis.com/v1beta/models" \ -H "x-goog-api-key: $GEMINI_API_KEY" Note: Vertex AI has a different endpoint (projects.locations.models.list) using OAuth, but the query refers to the direct Gemini API.
Citations:
- 1: https://docs.anthropic.com/en/api/models
- 2: https://platform.claude.com/docs/en/api/models/list
- 3: https://docs.anthropic.com/en/api/overview
- 4: https://ai.google.dev/api
- 5: https://ai.google.dev/api/models
🏁 Script executed:
# First, let's find the files mentioned in the review
fd -t f "handler.ts" packages/cliRepository: protoLabsAI/protoCLI
Length of output: 149
🏁 Script executed:
# Check the handler.ts file around line 74
head -80 packages/cli/src/commands/setup/handler.ts | tail -20Repository: protoLabsAI/protoCLI
Length of output: 734
🏁 Script executed:
# Look at the fetchAvailableModels function definition
fd -t f "modelDiscovery.ts" packages/cliRepository: protoLabsAI/protoCLI
Length of output: 114
🏁 Script executed:
# Examine the modelDiscovery.ts file - first check its size
wc -l packages/cli/src/commands/setup/modelDiscovery.tsRepository: protoLabsAI/protoCLI
Length of output: 118
🏁 Script executed:
# Check the fetch call and parser sections mentioned in review (lines 28-45 and 101-143)
cat -n packages/cli/src/commands/setup/modelDiscovery.ts | sed -n '25,50p'Repository: protoLabsAI/protoCLI
Length of output: 990
🏁 Script executed:
# Check the parser section
cat -n packages/cli/src/commands/setup/modelDiscovery.ts | sed -n '98,150p'Repository: protoLabsAI/protoCLI
Length of output: 1696
🏁 Script executed:
# Look for preset definitions with authType
fd -t f "preset" packages/cli | head -20Repository: protoLabsAI/protoCLI
Length of output: 46
🏁 Script executed:
# Search for authType mentions in the codebase
rg "authType" packages/cli -B 2 -A 2Repository: protoLabsAI/protoCLI
Length of output: 50376
🏁 Script executed:
# Find PROVIDER_PRESETS definition
rg "PROVIDER_PRESETS" packages/cli -B 5 -A 10Repository: protoLabsAI/protoCLI
Length of output: 8158
🏁 Script executed:
# Search for preset type definition
rg "PROVIDER_PRESETS\s*[:=]" packages/cli -A 30Repository: protoLabsAI/protoCLI
Length of output: 2233
🏁 Script executed:
# Look for preset-related files
fd -t f "preset" packages/cliRepository: protoLabsAI/protoCLI
Length of output: 46
🏁 Script executed:
# Check for Anthropic and Gemini preset definitions
rg -i "anthropic|gemini" packages/cli/src/commands/setup -B 2 -A 2Repository: protoLabsAI/protoCLI
Length of output: 2769
Pass preset.authType to fetchAvailableModels() to support Anthropic and Gemini model discovery.
The call on line 74 passes only baseUrl and apiKey, but the discovery function hardcodes Authorization: Bearer ... headers and the parser only handles OpenAI-style response shapes. Anthropic requires X-Api-Key + anthropic-version headers, and Gemini requires x-goog-api-key header with query parameters. The preset object (with authType) is available but not passed to the discovery function, so Anthropic and Gemini presets cannot discover models during setup. Either branch the discovery logic by preset.authType to use the correct headers and response parsing for each provider, or skip auto-discovery for non-OpenAI-compatible presets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup/handler.ts` at line 74, The call to
fetchAvailableModels(baseUrl, apiKey) omits preset.authType so discovery always
uses OpenAI-style Authorization and parsing; update the call to pass
preset.authType and update fetchAvailableModels to accept an authType parameter
(or branch inside setup handler before calling) and handle provider-specific
headers and response parsing for "openai" vs "anthropic" vs "gemini" (e.g., use
X-Api-Key + anthropic-version for Anthropic, x-goog-api-key / query params for
Gemini) or short-circuit and skip auto-discovery for non-OpenAI-compatible
presets; locate uses of fetchAvailableModels and preset.authType to implement
the branch and ensure model-parser logic inside fetchAvailableModels (or the new
branch) correctly maps each provider's response shape to the common models
format.
| return; | ||
| } | ||
| writeStderrLine('Setup cancelled — no model selected.\n'); | ||
| process.exit(1); |
There was a problem hiding this comment.
Remove process.exit() from this shared wizard handler.
runSetupWizard() is documented as reusable from both proto setup and /setup, so these exits will terminate the entire interactive session on success, cancel, or failure instead of just ending the wizard. Return a status or throw, and let the top-level CLI entrypoint decide whether the process should exit.
Also applies to: 136-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup/handler.ts` at line 95, Remove the direct
process.exit(...) calls from the shared runSetupWizard handler and any helper
functions it calls (referenced in this file around runSetupWizard and the blocks
corresponding to the success/cancel/failure flows), and instead return an
explicit status value or throw an Error to indicate success, cancellation, or
failure; update runSetupWizard to propagate that status/exception to its caller
and ensure the top-level CLI entrypoint (the command handlers that call
runSetupWizard) decides whether to call process.exit based on the returned
status or caught exception.
Comment on lines
+572
to
+577
| // Persist voice / STT settings | ||
| if (sttConfig) { | ||
| settings.setValue(scope, 'voice.enabled', sttConfig.enabled); | ||
| settings.setValue(scope, 'voice.sttEndpoint', sttConfig.endpoint); | ||
| settings.setValue(scope, 'voice.sttEnvKey', sttConfig.envKey); | ||
| } |
There was a problem hiding this comment.
Skipping STT should not keep the previous voice backend active.
When sttConfig is null, this leaves any existing voice.enabled, voice.sttEndpoint, and voice.sttEnvKey untouched. Re-running setup and choosing “Skip for now” can therefore keep /voice pointed at the old provider even though the wizard implied voice was skipped. Add an explicit else branch that disables voice and clears stale STT settings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup/handler.ts` around lines 572 - 577, When
sttConfig is null the code currently leaves previous STT values intact; update
the block handling sttConfig so that the else branch explicitly disables voice
and clears stale STT settings: in the handler where sttConfig is checked (the
block calling settings.setValue(scope, 'voice.enabled', ...),
settings.setValue(scope, 'voice.sttEndpoint', ...) and settings.setValue(scope,
'voice.sttEnvKey', ...)), add an else that calls settings.setValue(scope,
'voice.enabled', false) and clears or sets ''/null for 'voice.sttEndpoint' and
'voice.sttEnvKey' to ensure the old backend is not left active.
4 tasks
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
Interactive CLI command to configure model providers without manual JSON editing.
proto setupwizard flow:GET /modelscall, paginated selector~/.proto/settings.jsonFiles added:
packages/cli/src/commands/setup.ts— yargs subcommandpackages/cli/src/commands/setup/handler.ts— wizard logicpackages/cli/src/commands/setup/modelDiscovery.ts—/modelsendpoint clientpackages/cli/src/ui/commands/setupCommand.ts—/setupslash commandFiles modified:
config.ts— register yargs commandBuiltinCommandLoader.ts— register slash commandSummary by CodeRabbit
New Features
setupcommand for interactive provider, API key, and model configuration (with model discovery)/memorysubcommands to manage proposals (list, accept, reject)Improvements