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>
WalkthroughThis pull request introduces a multi-layered memory and tool management system. It adds memory proposal workflows via CLI commands, implements permission blocking for repeatedly-denied tools, introduces an evolve service for periodic skill detection, refactors prompt assembly with volatility-aware caching, persists cron job state across sessions, and updates memory staleness tracking with type-specific thresholds. Changes
Sequence DiagramssequenceDiagram
participant User
participant CLI as Memory Command
participant Store as Proposal Store
participant MemDir as Memory Directory
User->>CLI: /memory proposals
CLI->>Store: listProposals(project & global)
Store->>MemDir: scan proposals/ directory
MemDir-->>Store: proposal files with frontmatter
Store-->>CLI: formatted proposal list
CLI-->>User: display proposals with IDs
User->>CLI: /memory accept <id>
CLI->>Store: find proposal by ID
Store-->>CLI: matching proposal path
CLI->>Store: acceptProposal(path)
Store->>MemDir: move proposal to memory/
Store->>MemDir: regenerateIndex()
MemDir-->>Store: index updated
Store-->>CLI: success
CLI-->>User: "Accepted memory"
sequenceDiagram
participant User
participant Scheduler as Tool Scheduler
participant Confirm as Confirmation UI
participant Blocker as Permission Blocker
participant Prompt as System Prompt
Scheduler->>Scheduler: execute tool call
Scheduler->>Confirm: present confirmation dialog
User->>Confirm: cancel/deny
Confirm->>Scheduler: ToolConfirmationOutcome.Cancel
Scheduler->>Blocker: recordDenial(toolName)
Blocker->>Blocker: increment denial count
Note over Blocker: Check if threshold reached
alt Threshold met
Blocker->>Blocker: persist to disk
end
Scheduler->>Prompt: next turn starts
Prompt->>Blocker: getBlockedTools()
Blocker-->>Prompt: list of blocked tools
Prompt->>Prompt: buildPromptNote()
Prompt->>Prompt: include in system instruction
sequenceDiagram
participant Chat as Gemini Chat
participant Stream as useGeminiStream
participant Evolve as Evolve Service
participant Agent as Headless Agent
participant SkillDir as Skills Directory
Chat->>Stream: turn completed
Stream->>Stream: finally block triggered
Stream->>Evolve: runEvolvePass(config, recentMessages)
Evolve->>Evolve: check turn counter vs SKILL_REVIEW_INTERVAL
alt Trigger conditions met
Evolve->>SkillDir: getExistingSkillNames()
SkillDir-->>Evolve: skill names
Evolve->>Evolve: buildEvolvePrompt(messages, skills)
Evolve->>Agent: execute bounded headless agent
Agent->>SkillDir: write drafted skill files
SkillDir-->>Agent: success
Agent-->>Evolve: completion
Evolve->>Evolve: log completion
else Trigger not met
Evolve->>Evolve: skip this turn
end
Evolve-->>Stream: (fire-and-forget, errors suppressed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/core/coreToolScheduler.ts (1)
1193-1201:⚠️ Potential issue | 🟠 MajorDon’t record blocker denials for generic aborts.
Line [1193] currently treats
signal.abortedas equivalent to explicit user denial. That can incorrectly persist blockers for cancellations not initiated as denials.[suggested fix: record denial only on explicit
ToolConfirmationOutcome.Cancel]💡 Proposed patch
- if (outcome === ToolConfirmationOutcome.Cancel || signal.aborted) { + if (outcome === ToolConfirmationOutcome.Cancel || 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); + // Only explicit user denial should affect persistent blockers. + if (outcome === ToolConfirmationOutcome.Cancel) { + this.config + .getPermissionBlockerService?.() + ?.recordDenial(toolCall.request.name); + }🤖 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 code currently records a permission denial whenever outcome is ToolConfirmationOutcome.Cancel OR signal.aborted; change the logic so the persistent blocker denial (this.config.getPermissionBlockerService?.()?.recordDenial(toolCall.request.name)) is executed only when the outcome is explicitly ToolConfirmationOutcome.Cancel. Keep the existing cancel status handling (this.setStatusInternal(callId, 'cancelled', ...)) for both cases, but guard the recordDenial call behind outcome === ToolConfirmationOutcome.Cancel in the method handling tool confirmation results in coreToolScheduler (the block that uses callId, setStatusInternal, payload?.cancelMessage and toolCall.request.name).packages/core/src/services/cronScheduler.ts (1)
323-326:⚠️ Potential issue | 🟠 Major
destroy()should persist the cleared state.With disk persistence enabled, clearing jobs only in memory means destroyed jobs can reappear next session if the process exits before another save.
💡 Proposed patch
destroy(): void { this.stop(); this.jobs.clear(); + 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 323 - 326, The destroy() method currently stops the scheduler and clears this.jobs only in memory; update it to persist the cleared state to disk by invoking the same persistence routine used elsewhere (e.g., the method that saves jobs to disk such as saveJobs(), persistJobs(), save(), or persist()) right after this.jobs.clear(); ensure you call it only when disk persistence is enabled, await/handle it if it is async, and surface/log any errors so the empty state is durably written.
🤖 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/core/src/core/client.test.ts`:
- Around line 291-297: The current mock of assemblePromptSections in
client.test.ts flattens sections and loses volatility ordering and
cache-boundary behavior; update the test mock to either call the real
assemblePromptSections implementation or implement the same ordering and
cache-boundary logic so tests exercise real assembly semantics: ensure the mock
honors PromptSection.volatility ordering (stable sort/grouping by volatility)
and preserves cacheBoundary boundaries between sections before joining contents.
Target the vi.mocked(assemblePromptSections) replacement so it returns results
consistent with the real assemblePromptSections behavior rather than simply
filtering and joining content.
In `@packages/core/src/core/client.ts`:
- Around line 318-329: The override path wrongly places dynamic content from
getCustomSystemPrompt(overrideSystemPrompt, userMemory, appendSystemPrompt) into
the 'stable' bucket; change the assemblePromptSections call so the returned base
is treated as workspace (or split static vs memory if you can extract static
portion) rather than 'stable' — update the mapping in the override branch that
currently returns [{ volatility: 'stable', content: base }, ...] to use
'workspace' for the dynamic parts (reference: overrideSystemPrompt,
getCustomSystemPrompt, assemblePromptSections, userMemory, appendSystemPrompt).
- Around line 300-312: The capabilityManifest call is passing an empty
activeSkills array (hardcoded `[]`), so skills are never included; update the
block that builds capabilityManifest to compute an activeSkills array from the
tools returned by toolRegistry.getAllTools() (filtering for skill-type tools —
e.g., instances or flags that identify skills) and pass that array to
buildCapabilityManifest instead of `[]` (refer to toolRegistry.getAllTools,
DiscoveredMCPTool, capabilityManifest and buildCapabilityManifest to locate the
code to change).
In `@packages/core/src/core/prompts.ts`:
- Around line 1278-1294: Sanitize all externally-provided capability metadata
before adding to the system prompt: when building the MCP tools block
(mcpToolsByServer) and the skills block (activeSkills), escape or strip
newline/control characters and markdown-special characters from server names
(server), tool names (tools elements), skill names (skill.name) and skill
descriptions (skill.description), and enforce reasonable length limits (e.g.,
truncate descriptions and names) so that interpolated values cannot inject
newlines or markup that would alter the prompt structure; apply this filtering
inside the same prompt-construction flow where mcpToolsByServer and activeSkills
are iterated (the block that builds lines and pushes to sections).
In `@packages/core/src/memory/proposalStore.ts`:
- Around line 87-93: The current rename to destPath (path.join(memoryDir,
path.basename(proposalFilePath))) can overwrite an existing file; modify the
accept/rename logic in this module (where destPath, memoryDir, and
proposalFilePath are used, and regenerateIndex(scope, cwd) is called) to first
check if destPath already exists and then either (a) fail with a clear
error/exception to avoid silent overwrite, or (b) generate a unique
non-colliding destination (e.g., append a numeric or timestamp suffix to the
basename) before calling fs.rename, ensuring you preserve atomic behavior and
still call regenerateIndex(scope, cwd) only after a successful move.
- Around line 81-110: Ensure acceptProposal and rejectProposal validate that the
provided proposalFilePath is actually inside the proposals directory before
mutating files: compute proposalsDir = getProposalsDir(scope, cwd) and resolve
both proposalsDir and proposalFilePath to absolute/real paths, then verify the
proposal path is within proposalsDir (e.g., via path.relative and ensuring it
does not start with '..' and is not equal to '' when normalized) before calling
fs.rename or fs.unlink; if the check fails, return null from acceptProposal and
false from rejectProposal without performing any filesystem operations and do
not call regenerateIndex unless the move succeeded.
In `@packages/core/src/services/cronScheduler.ts`:
- Around line 106-130: Persisted one-shot jobs are dropped because
JSON.stringify turns expiresAt = Infinity into null and loadFromDisk rejects
nulls; update loadFromDisk to treat job.expiresAt === null or job.expiresAt ===
undefined as Infinity before comparing to now (i.e., set job.expiresAt =
Number.POSITIVE_INFINITY when null/undefined), and ensure destroy() calls
saveToDisk() after clearing this.jobs so the cleared state is persisted; adjust
references in loadFromDisk, saveToDisk, and destroy to use the CronJob.expiresAt
normalization and persistence call.
In `@packages/core/src/services/evolveService.ts`:
- Around line 37-50: The global turnsSinceLastReview counter causes
cross-session interference; make the cadence state per-project/session instead:
remove/stop using the module-global turnsSinceLastReview and track counts keyed
by a unique session/project identifier (e.g. a sessionId or projectId available
on Config) using a Map (e.g. evolveTurnsBySession) or store the counter on the
Config object, then update runEvolvePass to read/increment/reset the per-session
counter using SKILL_REVIEW_INTERVAL and the chosen key, and ensure the counter
is reset to 0 when the interval fires so each session's evolve cadence is
isolated.
In `@packages/core/src/services/permissionBlockerService.ts`:
- Around line 61-70: The recordDenial method currently only calls saveToDisk
when updated.count >= DENY_THRESHOLD causing sub-threshold denials to be lost
across restarts; update recordDenial (which updates this.denials and builds
DenialRecord) to persist every update by calling saveToDisk after
this.denials.set(toolName, updated) (or implement a short debounce/persist
strategy if needed) so each denial event is written to disk rather than only
when the threshold is reached.
---
Outside diff comments:
In `@packages/core/src/core/coreToolScheduler.ts`:
- Around line 1193-1201: The code currently records a permission denial whenever
outcome is ToolConfirmationOutcome.Cancel OR signal.aborted; change the logic so
the persistent blocker denial
(this.config.getPermissionBlockerService?.()?.recordDenial(toolCall.request.name))
is executed only when the outcome is explicitly ToolConfirmationOutcome.Cancel.
Keep the existing cancel status handling (this.setStatusInternal(callId,
'cancelled', ...)) for both cases, but guard the recordDenial call behind
outcome === ToolConfirmationOutcome.Cancel in the method handling tool
confirmation results in coreToolScheduler (the block that uses callId,
setStatusInternal, payload?.cancelMessage and toolCall.request.name).
In `@packages/core/src/services/cronScheduler.ts`:
- Around line 323-326: The destroy() method currently stops the scheduler and
clears this.jobs only in memory; update it to persist the cleared state to disk
by invoking the same persistence routine used elsewhere (e.g., the method that
saves jobs to disk such as saveJobs(), persistJobs(), save(), or persist())
right after this.jobs.clear(); ensure you call it only when disk persistence is
enabled, await/handle it if it is async, and surface/log any errors so the empty
state is durably written.
🪄 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: 8f91be83-55ce-42e1-b77a-a819a1f1a88e
⛔ Files ignored due to path filters (1)
packages/core/src/core/__snapshots__/prompts.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (19)
packages/cli/src/ui/commands/memoryCommand.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/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.ts
Comment on lines
+291
to
+297
| vi.mocked(assemblePromptSections).mockImplementation( | ||
| (sections: PromptSection[]) => | ||
| sections | ||
| .filter((s) => s.content) | ||
| .map((s) => s.content) | ||
| .join('\n\n'), | ||
| ); |
There was a problem hiding this comment.
Mock is too simplified for prompt-section behavior.
This mock drops volatility ordering and cache-boundary behavior, so tests can pass while real prompt assembly breaks.
💡 Proposed patch
- vi.mocked(assemblePromptSections).mockImplementation(
- (sections: PromptSection[]) =>
- sections
- .filter((s) => s.content)
- .map((s) => s.content)
- .join('\n\n'),
- );
+ const actualPrompts = await vi.importActual<typeof import('./prompts.js')>(
+ './prompts.js',
+ );
+ vi.mocked(assemblePromptSections).mockImplementation(
+ (sections: PromptSection[]) =>
+ actualPrompts.assemblePromptSections(sections),
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/core/client.test.ts` around lines 291 - 297, The current
mock of assemblePromptSections in client.test.ts flattens sections and loses
volatility ordering and cache-boundary behavior; update the test mock to either
call the real assemblePromptSections implementation or implement the same
ordering and cache-boundary logic so tests exercise real assembly semantics:
ensure the mock honors PromptSection.volatility ordering (stable sort/grouping
by volatility) and preserves cacheBoundary boundaries between sections before
joining contents. Target the vi.mocked(assemblePromptSections) replacement so it
returns results consistent with the real assemblePromptSections behavior rather
than simply filtering and joining content.
Comment on lines
+300
to
312
| // --- workspace: capability manifest of session-specific MCP tools --- | ||
| let capabilityManifest = ''; | ||
| if (toolRegistry && typeof toolRegistry.getAllTools === 'function') { | ||
| const mcpToolsByServer = new Map<string, string[]>(); | ||
| for (const tool of toolRegistry.getAllTools()) { | ||
| if (tool instanceof DiscoveredMCPTool) { | ||
| const existing = mcpToolsByServer.get(tool.serverName) ?? []; | ||
| existing.push(tool.name); | ||
| mcpToolsByServer.set(tool.serverName, existing); | ||
| } | ||
| } | ||
| capabilityManifest = buildCapabilityManifest(mcpToolsByServer, []) ?? ''; | ||
| } |
There was a problem hiding this comment.
The capability manifest never includes active skills from this path.
Line 311 hardcodes [] for activeSkills, so the skills section added in buildCapabilityManifest() can never render here. That means slash-command/skill capabilities are still invisible to the model even when they are available in the session.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/core/client.ts` around lines 300 - 312, The
capabilityManifest call is passing an empty activeSkills array (hardcoded `[]`),
so skills are never included; update the block that builds capabilityManifest to
compute an activeSkills array from the tools returned by
toolRegistry.getAllTools() (filtering for skill-type tools — e.g., instances or
flags that identify skills) and pass that array to buildCapabilityManifest
instead of `[]` (refer to toolRegistry.getAllTools, DiscoveredMCPTool,
capabilityManifest and buildCapabilityManifest to locate the code to change).
Comment on lines
+314
to
+316
| // --- run: per-turn permission blockers --- | ||
| const blockerNote = | ||
| this.config.getPermissionBlockerService?.()?.buildPromptNote() ?? ''; |
There was a problem hiding this comment.
blockerNote is labeled per-turn, but it won't refresh for ongoing chats.
getMainSessionSystemInstruction() is only used when the chat is created/reset and for one-off generateContent() calls. After a permission denial is recorded, this note will stay stale for the active GeminiChat, so the next streamed turn will not see the new blocker guidance unless the chat happens to be rebuilt.
Comment on lines
+318
to
+329
| if (overrideSystemPrompt) { | ||
| const base = getCustomSystemPrompt( | ||
| overrideSystemPrompt, | ||
| userMemory, | ||
| this.config.getModel(), | ||
| appendSystemPrompt, | ||
| ).full + mcpInstructions | ||
| ); | ||
| return assemblePromptSections([ | ||
| { volatility: 'stable', content: base }, | ||
| { volatility: 'workspace', content: mcpInstructions }, | ||
| { volatility: 'workspace', content: capabilityManifest }, | ||
| { volatility: 'run', content: blockerNote }, | ||
| ]); |
There was a problem hiding this comment.
The override path is caching dynamic memory as stable.
getCustomSystemPrompt() already appends userMemory and appendSystemPrompt, and Line 325 then places that combined string in the stable bucket. That regresses the volatility split you preserved in the core-prompt branch: session-specific memory and freshness warnings end up before the cache boundary instead of in the workspace section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/core/client.ts` around lines 318 - 329, The override path
wrongly places dynamic content from getCustomSystemPrompt(overrideSystemPrompt,
userMemory, appendSystemPrompt) into the 'stable' bucket; change the
assemblePromptSections call so the returned base is treated as workspace (or
split static vs memory if you can extract static portion) rather than 'stable' —
update the mapping in the override branch that currently returns [{ volatility:
'stable', content: base }, ...] to use 'workspace' for the dynamic parts
(reference: overrideSystemPrompt, getCustomSystemPrompt, assemblePromptSections,
userMemory, appendSystemPrompt).
Comment on lines
+1278
to
+1294
| if (mcpToolsByServer.size > 0) { | ||
| const lines: string[] = ['**MCP tools available this session:**']; | ||
| 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}`); | ||
| } | ||
| sections.push(lines.join('\n')); |
There was a problem hiding this comment.
Sanitize capability metadata before putting it in the system prompt.
Lines 1281 and 1292 interpolate MCP server names, tool names, skill names, and descriptions verbatim into a high-privilege prompt. Those values are not inherently trusted, so a malicious server or crafted skill metadata can inject newlines/markdown/XML-looking content and steer the model with prompt text you did not author.
Suggested hardening
+function sanitizePromptValue(value: string): string {
+ return value.replace(/[\r\n]+/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(', ')}`);
+ lines.push(
+ ` • ${sanitizePromptValue(server)}: ${tools.map(sanitizePromptValue).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 safeDescription = sanitizePromptValue(skill.description);
+ const desc = safeDescription
+ ? ` — ${safeDescription.slice(0, 80)}${safeDescription.length > 80 ? '…' : ''}`
: '';
- lines.push(` • /${skill.name}${desc}`);
+ lines.push(` • /${sanitizePromptValue(skill.name)}${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 1278 - 1294, Sanitize all
externally-provided capability metadata before adding to the system prompt: when
building the MCP tools block (mcpToolsByServer) and the skills block
(activeSkills), escape or strip newline/control characters and markdown-special
characters from server names (server), tool names (tools elements), skill names
(skill.name) and skill descriptions (skill.description), and enforce reasonable
length limits (e.g., truncate descriptions and names) so that interpolated
values cannot inject newlines or markup that would alter the prompt structure;
apply this filtering inside the same prompt-construction flow where
mcpToolsByServer and activeSkills are iterated (the block that builds lines and
pushes to sections).
Comment on lines
+81
to
+110
| 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; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Enforce proposals-directory boundary before file mutations.
Lines 81-110 move/delete whichever path is passed in. Please validate that proposalFilePath resolves under getProposalsDir(scope, cwd) before rename/unlink.
🔒 Suggested fix
+function assertPathInsideDir(targetPath: string, baseDir: string): void {
+ const resolvedTarget = path.resolve(targetPath);
+ const resolvedBase = path.resolve(baseDir) + path.sep;
+ if (!resolvedTarget.startsWith(resolvedBase)) {
+ throw new Error('Proposal path is outside proposals directory');
+ }
+}
export async function acceptProposal(
proposalFilePath: string,
scope: MemoryScope,
cwd?: string,
): Promise<string | null> {
+ const proposalsDir = getProposalsDir(scope, cwd);
+ assertPathInsideDir(proposalFilePath, proposalsDir);
const memoryDir = getMemoryDir(scope, cwd);
const destPath = path.join(memoryDir, path.basename(proposalFilePath));
try {
@@
export async function rejectProposal(
proposalFilePath: string,
+ scope: MemoryScope,
+ cwd?: string,
): Promise<boolean> {
try {
+ const proposalsDir = getProposalsDir(scope, cwd);
+ assertPathInsideDir(proposalFilePath, proposalsDir);
await fs.unlink(proposalFilePath);
return true;
} catch {🤖 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 - 110, Ensure
acceptProposal and rejectProposal validate that the provided proposalFilePath is
actually inside the proposals directory before mutating files: compute
proposalsDir = getProposalsDir(scope, cwd) and resolve both proposalsDir and
proposalFilePath to absolute/real paths, then verify the proposal path is within
proposalsDir (e.g., via path.relative and ensuring it does not start with '..'
and is not equal to '' when normalized) before calling fs.rename or fs.unlink;
if the check fails, return null from acceptProposal and false from
rejectProposal without performing any filesystem operations and do not call
regenerateIndex unless the move succeeded.
Comment on lines
+87
to
+93
| 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; |
There was a problem hiding this comment.
Prevent accidental overwrite when accepting proposals.
Lines 87-93 rename to memoryDir/<basename>. If that file already exists, this can overwrite or fail depending on platform, risking silent data loss/ambiguous behavior.
🛡️ Suggested fix
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 });
+ const exists = await fs
+ .stat(destPath)
+ .then(() => true)
+ .catch(() => false);
+ if (exists) return null;
await fs.rename(proposalFilePath, destPath);
await regenerateIndex(scope, cwd);
return destPath;
} catch {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const 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; | |
| const destPath = path.join(memoryDir, path.basename(proposalFilePath)); | |
| try { | |
| await fs.mkdir(memoryDir, { recursive: true }); | |
| const exists = await fs | |
| .stat(destPath) | |
| .then(() => true) | |
| .catch(() => false); | |
| if (exists) return null; | |
| await fs.rename(proposalFilePath, destPath); | |
| await regenerateIndex(scope, cwd); | |
| return destPath; |
🤖 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 87 - 93, The current
rename to destPath (path.join(memoryDir, path.basename(proposalFilePath))) can
overwrite an existing file; modify the accept/rename logic in this module (where
destPath, memoryDir, and proposalFilePath are used, and regenerateIndex(scope,
cwd) is called) to first check if destPath already exists and then either (a)
fail with a clear error/exception to avoid silent overwrite, or (b) generate a
unique non-colliding destination (e.g., append a numeric or timestamp suffix to
the basename) before calling fs.rename, ensuring you preserve atomic behavior
and still call regenerateIndex(scope, cwd) only after a successful move.
Comment on lines
+106
to
+130
| private loadFromDisk(): void { | ||
| if (!this.persistPath) return; | ||
| try { | ||
| const raw = fs.readFileSync(this.persistPath, 'utf-8'); | ||
| const jobs = JSON.parse(raw) as CronJob[]; | ||
| const now = Date.now(); | ||
| for (const job of jobs) { | ||
| if (now < job.expiresAt) { | ||
| this.jobs.set(job.id, job); | ||
| } | ||
| } | ||
| } catch { | ||
| // File doesn't exist or is corrupt — start with empty store | ||
| } | ||
| } | ||
|
|
||
| private saveToDisk(): void { | ||
| if (!this.persistPath) return; | ||
| try { | ||
| fs.mkdirSync(path.dirname(this.persistPath), { recursive: true }); | ||
| fs.writeFileSync( | ||
| this.persistPath, | ||
| JSON.stringify([...this.jobs.values()], null, 2), | ||
| 'utf-8', | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node -e 'const raw = JSON.stringify([{expiresAt: Infinity}]); const parsed = JSON.parse(raw); console.log(raw); console.log("expiresAt:", parsed[0].expiresAt, "comparison:", Date.now() < parsed[0].expiresAt);'Repository: protoLabsAI/protoCLI
Length of output: 118
🏁 Script executed:
find . -name "cronScheduler.ts" -type fRepository: protoLabsAI/protoCLI
Length of output: 109
🏁 Script executed:
cat -n ./packages/core/src/services/cronScheduler.tsRepository: protoLabsAI/protoCLI
Length of output: 11710
One-shot job persistence is broken by Infinity serialization.
One-shot jobs set expiresAt = Infinity at line 157, but line 128 persists with JSON.stringify(), which serializes Infinity to null. On load (line 113), the comparison now < job.expiresAt evaluates to false when expiresAt is null, so persisted one-shot jobs are silently dropped.
Additionally, the destroy() method (lines 323–326) clears jobs without calling saveToDisk(), violating the persistence contract—the next process will reload the cleared state.
Proposed patch
+type PersistedCronJob = Omit<CronJob, 'expiresAt'> & { expiresAt: number | null };
private loadFromDisk(): void {
if (!this.persistPath) return;
try {
const raw = fs.readFileSync(this.persistPath, 'utf-8');
- const jobs = JSON.parse(raw) as CronJob[];
+ const jobs = JSON.parse(raw) as PersistedCronJob[];
const now = Date.now();
for (const job of jobs) {
- if (now < job.expiresAt) {
- this.jobs.set(job.id, job);
+ const expiresAt = job.expiresAt ?? Number.POSITIVE_INFINITY;
+ if (now < expiresAt) {
+ this.jobs.set(job.id, { ...job, expiresAt });
}
}
} catch {
// File doesn't exist or is corrupt — start with empty store
}
}
private saveToDisk(): void {
if (!this.persistPath) return;
try {
fs.mkdirSync(path.dirname(this.persistPath), { recursive: true });
+ const persisted: PersistedCronJob[] = [...this.jobs.values()].map((job) => ({
+ ...job,
+ expiresAt: Number.isFinite(job.expiresAt) ? job.expiresAt : null,
+ }));
fs.writeFileSync(
this.persistPath,
- JSON.stringify([...this.jobs.values()], null, 2),
+ JSON.stringify(persisted, null, 2),
'utf-8',
);
} catch {
// Non-fatal — session continues without persistence
}
}
+ destroy(): void {
+ this.stop();
+ this.jobs.clear();
+ if (this.persistPath) 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 106 - 130,
Persisted one-shot jobs are dropped because JSON.stringify turns expiresAt =
Infinity into null and loadFromDisk rejects nulls; update loadFromDisk to treat
job.expiresAt === null or job.expiresAt === undefined as Infinity before
comparing to now (i.e., set job.expiresAt = Number.POSITIVE_INFINITY when
null/undefined), and ensure destroy() calls saveToDisk() after clearing
this.jobs so the cleared state is persisted; adjust references in loadFromDisk,
saveToDisk, and destroy to use the CronJob.expiresAt normalization and
persistence call.
Comment on lines
+37
to
+50
| 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.
Scope evolve cadence state per project/session.
Line 37 with Lines 47-50 uses a module-global counter. If multiple sessions/projects share the same process, one stream can trigger/skip another stream’s evolve pass.
🔧 Suggested fix
-let turnsSinceLastReview = 0;
+const turnsSinceLastReviewByProject = new Map<string, number>();
export async function runEvolvePass(
config: Config,
recentMessages: Array<{ role: string; text: string }>,
): Promise<void> {
- turnsSinceLastReview++;
- if (turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return;
- turnsSinceLastReview = 0;
+ const projectRoot = config.getProjectRoot();
+ const turnsSinceLastReview =
+ (turnsSinceLastReviewByProject.get(projectRoot) ?? 0) + 1;
+ turnsSinceLastReviewByProject.set(projectRoot, turnsSinceLastReview);
+ if (turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return;
+ turnsSinceLastReviewByProject.set(projectRoot, 0);
if (recentMessages.length < 4) return;
- const projectRoot = config.getProjectRoot();
const evolveSkilsDir = path.join(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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; | |
| const turnsSinceLastReviewByProject = new Map<string, number>(); | |
| /** | |
| * 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> { | |
| const projectRoot = config.getProjectRoot(); | |
| const turnsSinceLastReview = | |
| (turnsSinceLastReviewByProject.get(projectRoot) ?? 0) + 1; | |
| turnsSinceLastReviewByProject.set(projectRoot, turnsSinceLastReview); | |
| if (turnsSinceLastReview < SKILL_REVIEW_INTERVAL) return; | |
| turnsSinceLastReviewByProject.set(projectRoot, 0); | |
🤖 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 - 50, The global
turnsSinceLastReview counter causes cross-session interference; make the cadence
state per-project/session instead: remove/stop using the module-global
turnsSinceLastReview and track counts keyed by a unique session/project
identifier (e.g. a sessionId or projectId available on Config) using a Map (e.g.
evolveTurnsBySession) or store the counter on the Config object, then update
runEvolvePass to read/increment/reset the per-session counter using
SKILL_REVIEW_INTERVAL and the chosen key, and ensure the counter is reset to 0
when the interval fires so each session's evolve cadence is isolated.
Comment on lines
+61
to
+70
| recordDenial(toolName: string): void { | ||
| const existing = this.denials.get(toolName) ?? { count: 0, lastSeenAt: 0 }; | ||
| const updated: DenialRecord = { | ||
| count: existing.count + 1, | ||
| lastSeenAt: Date.now(), | ||
| }; | ||
| this.denials.set(toolName, updated); | ||
| if (updated.count >= DENY_THRESHOLD) { | ||
| this.saveToDisk(); | ||
| } |
There was a problem hiding this comment.
Persist each denial event, not only post-threshold.
Current logic drops sub-threshold denials on restart, so repeated denials across sessions may never accumulate to blocked status.
💡 Proposed patch
recordDenial(toolName: string): void {
const existing = this.denials.get(toolName) ?? { count: 0, lastSeenAt: 0 };
const updated: DenialRecord = {
count: existing.count + 1,
lastSeenAt: Date.now(),
};
this.denials.set(toolName, updated);
- if (updated.count >= DENY_THRESHOLD) {
- this.saveToDisk();
- }
+ this.saveToDisk();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| recordDenial(toolName: string): void { | |
| const existing = this.denials.get(toolName) ?? { count: 0, lastSeenAt: 0 }; | |
| const updated: DenialRecord = { | |
| count: existing.count + 1, | |
| lastSeenAt: Date.now(), | |
| }; | |
| this.denials.set(toolName, updated); | |
| if (updated.count >= DENY_THRESHOLD) { | |
| this.saveToDisk(); | |
| } | |
| recordDenial(toolName: string): void { | |
| const existing = this.denials.get(toolName) ?? { count: 0, lastSeenAt: 0 }; | |
| const updated: DenialRecord = { | |
| count: existing.count + 1, | |
| lastSeenAt: Date.now(), | |
| }; | |
| this.denials.set(toolName, updated); | |
| this.saveToDisk(); | |
| } |
🤖 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 - 70,
The recordDenial method currently only calls saveToDisk when updated.count >=
DENY_THRESHOLD causing sub-threshold denials to be lost across restarts; update
recordDenial (which updates this.denials and builds DenialRecord) to persist
every update by calling saveToDisk after this.denials.set(toolName, updated) (or
implement a short debounce/persist strategy if needed) so each denial event is
written to disk rather than only when the threshold is reached.
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. |
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
cron-jobs.jsonon diskproposals/dir;/memory proposals|accept|rejectfor explicit approvalpermission-blockers.jsonand injected into system promptPromptSection/assemblePromptSections()formalizestable → workspace → runordering withCACHE_BOUNDARY_SENTINELfor provider-side prompt cachingTest plan
npx vitest run packages/core/src)npx tsc --noEmit)client.test.tsmock updated forassemblePromptSections🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
/memorysubcommands