fix(agent): honor session workdir defaults#1380
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughConversation-specific working directories are now resolved and propagated to terminal and skill executions. Terminal creation and skill spawn plans prefer an explicit request cwd, then the per-session workdir, then an ACP temp fallback; SKILL_ROOT/DEEPCHAT_SKILL_ROOT env vars are always provided to skill runs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NewAgentPresenter
participant LLMProviderPresenter
participant SkillExecutionService
participant TerminalPTY
Client->>NewAgentPresenter: createSession(sessionId, projectDir)
NewAgentPresenter->>NewAgentPresenter: initializeSessionRuntime()
NewAgentPresenter->>NewAgentPresenter: agent.initSession()
NewAgentPresenter->>LLMProviderPresenter: setAcpWorkdir(sessionId, agentId, projectDir)
LLMProviderPresenter-->>NewAgentPresenter: ack / register session workdir
Client->>NewAgentPresenter: sendMessage(sessionId, message)
NewAgentPresenter->>NewAgentPresenter: syncAcpSessionWorkdir()
NewAgentPresenter->>LLMProviderPresenter: setAcpWorkdir(sessionId, agentId, projectDir)
NewAgentPresenter->>SkillExecutionService: buildSpawnPlan(skillRoot, conversationId)
SkillExecutionService->>LLMProviderPresenter: getWorkdirForConversation(conversationId)
LLMProviderPresenter-->>SkillExecutionService: sessionWorkdir or null
SkillExecutionService-->>NewAgentPresenter: spawnPlan { cwd: sessionWorkdir|skillRoot, env: ...SKILL_ROOT... }
NewAgentPresenter->>TerminalPTY: spawn(command, { cwd: spawnPlan.cwd, env: spawnPlan.env })
TerminalPTY-->>NewAgentPresenter: process started
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0d1319634
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+181
to
+184
| const resolvedWorkdir = await this.resolveConversationWorkdir(conversationId) | ||
| const normalizedWorkdir = resolvedWorkdir?.trim() | ||
| if (normalizedWorkdir) { | ||
| return path.resolve(normalizedWorkdir) |
There was a problem hiding this comment.
skill_run against stale session workdirs
When a conversation's saved projectDir has gone stale (for example the repo was renamed or deleted), resolveExecutionCwd() now returns that path unconditionally and both the foreground path here and the background executor use it as cwd. In Node, spawning with a missing cwd raises ENOENT, so skill_run fails before the script even starts even though the skill itself is still present under skillRoot. Before this change the runner always used skillRoot, so missing session workspaces did not brick skill execution.
Useful? React with 👍 / 👎.
Comment on lines
+1373
to
+1377
| } catch (error) { | ||
| console.warn('[NewAgentPresenter] Failed to sync ACP workdir for session:', { | ||
| conversationId, | ||
| agentId, | ||
| projectDir: normalizedProjectDir, |
There was a problem hiding this comment.
This helper is awaited from createSession(), sendMessage(), and queuePendingInput() specifically to make the ACP workdir authoritative before the first turn runs, but the new catch converts every persistence/update failure into a warning. If setAcpWorkdir() fails (for example on a transient SQLite write error), acpProvider.coreStream() later reads the old record and AcpSessionPersistence.getWorkdir() falls back to the default home directory, so the agent's first command executes in the wrong workspace with no visible error. That makes the regression very hard to diagnose for affected users.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/newAgentPresenter/index.ts (1)
275-288:⚠️ Potential issue | 🟠 MajorResolve ACP aliases before inferring
providerId.Line 278 and Line 333 still compare the raw session agent id against
getAcpAgents(). For legacy ACP ids such askimi-cliorclaude-code-acp,getSessionState()can be empty after a restart, so these branches never mark the session as ACP and the newsetAcpWorkdir(...)path is skipped entirely.🛠️ Suggested fix
- let providerId = state?.providerId ?? '' + const resolvedAgentId = resolveAcpAgentAlias(session.agentId) + let providerId = state?.providerId ?? '' if (!providerId) { const acpAgents = await this.configPresenter.getAcpAgents() - if (acpAgents.some((item) => item.id === session.agentId)) { + if (acpAgents.some((item) => item.id === resolvedAgentId)) { providerId = 'acp' } } @@ - let providerId = (await agent.getSessionState(sessionId))?.providerId ?? '' + const resolvedAgentId = resolveAcpAgentAlias(currentSession.agentId) + let providerId = (await agent.getSessionState(sessionId))?.providerId ?? '' if (!providerId) { const acpAgents = await this.configPresenter.getAcpAgents() - if (acpAgents.some((item) => item.id === currentSession.agentId)) { + if (acpAgents.some((item) => item.id === resolvedAgentId)) { providerId = 'acp' } }Also applies to: 330-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/newAgentPresenter/index.ts` around lines 275 - 288, When inferring providerId (the providerId variable and the branch that calls this.configPresenter.getAcpAgents()), normalize/resolve legacy ACP agent aliases before comparing against acpAgents (i.e., don't compare raw session.agentId); call the existing configPresenter method that returns the canonical ACP id for a given agent (e.g., a resolve/getCanonicalAcpId helper on configPresenter) on session.agentId and use that normalized id in the acpAgents.some(...) check, then propagate that same normalized logic to the other branch that determines ACP (the code path that later calls assertAcpSessionHasWorkdir and syncAcpSessionWorkdir / setAcpWorkdir) so legacy ids like "kimi-cli" or "claude-code-acp" are recognized as ACP.
🧹 Nitpick comments (4)
test/main/presenter/skillPresenter/skillExecutionService.test.ts (1)
106-116: Assert resolver invocation to lock the propagation contractConsider asserting that
resolveConversationWorkdiris called with the conversation id, so future regressions can’t bypass the resolver silently.✅ Suggested test addition
const plan = await (service as never).buildSpawnPlan( { skill: 'ocr', script: 'scripts/run.py', args: ['--lang', 'en'] }, 'conv-1' ) + expect(resolveConversationWorkdir).toHaveBeenCalledWith('conv-1') expect(plan.cwd).toBe('/workspace/session')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/skillPresenter/skillExecutionService.test.ts` around lines 106 - 116, Add an assertion that the resolver for the conversation workdir was invoked with the conversation id when building the spawn plan: after calling (service as never).buildSpawnPlan(...) assert that resolveConversationWorkdir was called with 'conv-1' (or the variable used for the conversation id) to ensure the propagation contract is enforced; locate the mock or spy for resolveConversationWorkdir in the test setup (or create one if missing) and use the test framework's call/spy assertion (e.g., toHaveBeenCalledWith or equivalent) to validate the invocation alongside the existing expectations on plan.cwd and plan.env.PATH.test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts (1)
150-151: Prefer public session registration in tests over private map mutation.Using
registerSessionWorkdirkeeps tests aligned with real lifecycle behavior (including fs handler setup) and reduces brittle coupling to internals.♻️ Proposed test change
- ;(manager as any).sessionWorkdirs.set('session-1', '/tmp/workspace') + manager.registerSessionWorkdir('session-1', '/tmp/workspace') ... - ;(manager as any).sessionWorkdirs.set('session-1', '/tmp/workspace') + manager.registerSessionWorkdir('session-1', '/tmp/workspace')Also applies to: 179-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts` around lines 150 - 151, Replace direct private map mutation of (manager as any).sessionWorkdirs with the public API: call manager.registerSessionWorkdir('session-1', '/tmp/workspace') so the test exercises real lifecycle behavior (including fs handler setup); update the other occurrence at the second location (lines 179-180) similarly. Ensure you reference the manager instance and use the public registerSessionWorkdir method rather than touching sessionWorkdirs directly.test/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.ts (1)
23-27: Consider restoring spies after each test.This avoids accidental mock leakage when additional cases are added later.
♻️ Minimal isolation tweak
-import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' ... beforeEach(() => { vi.clearAllMocks() vi.spyOn(fs, 'mkdirSync').mockImplementation(() => undefined) vi.mocked(spawn).mockReturnValue(createPty() as never) }) + + afterEach(() => { + vi.restoreAllMocks() + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.ts` around lines 23 - 27, Add an afterEach block to restore spies/mocks so they don't leak between tests: afterEach should call vi.restoreAllMocks() (to restore vi.spyOn(fs, 'mkdirSync') and other spies) and vi.clearAllMocks() (to clear calls), ensuring the mocked spawn (vi.mocked(spawn)) and the createPty-related mocks are reset between tests and preventing accidental cross-test interference.src/main/presenter/llmProviderPresenter/acp/acpTerminalManager.ts (1)
40-59: Guard non-existentcwdbefore spawning the terminal.If a provided/session
cwdis stale or deleted, terminal creation can fail hard. Falling back to the controlled temp directory keeps behavior resilient.♻️ Proposed hardening
private resolveTerminalCwd(cwd?: string | null): string { const normalized = cwd?.trim() if (normalized) { - return path.resolve(normalized) + const resolved = path.resolve(normalized) + try { + if (fs.existsSync(resolved) && fs.statSync(resolved).isDirectory()) { + return resolved + } + console.warn(`[ACP Terminal] Invalid cwd "${resolved}", using fallback directory`) + } catch (error) { + console.warn(`[ACP Terminal] Failed to validate cwd "${resolved}", using fallback directory`, error) + } } const fallbackDir = path.join(app.getPath('temp'), 'deepchat-acp', 'terminals') try {Also applies to: 69-69, 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/llmProviderPresenter/acp/acpTerminalManager.ts` around lines 40 - 59, The resolveTerminalCwd function should verify that the provided cwd actually exists and is a directory before returning it; change the early return path in resolveTerminalCwd to check fs.existsSync(normalized) and fs.statSync(normalized).isDirectory() (wrapped in try/catch), and if the check fails, log a warning and fall back to creating/using the fallbackDir (same logic already present) so stale/deleted session cwd values don't cause terminal spawn failures; apply the same existence/is-directory guard wherever terminal spawn uses resolveTerminalCwd or similar cwd resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 1352-1380: The syncAcpSessionWorkdir helper currently swallows
setAcpWorkdir errors, causing callers (init/send/queue/fork) to proceed with an
incorrect workdir; change it to surface failures by either removing the
try/catch so the exception bubbles from syncAcpSessionWorkdir (propagating the
error from llmProviderPresenter.setAcpWorkdir) or by returning an explicit
failure result (e.g., boolean or Result) and updating all callers to check that
result and abort/cleanup when it indicates failure; keep the existing log but do
not suppress the error—ensure syncAcpSessionWorkdir, resolveAcpAgentAlias and
llmProviderPresenter.setAcpWorkdir are updated consistently so callers can react
to a failed sync.
In `@src/main/presenter/skillPresenter/skillExecutionService.ts`:
- Around line 174-200: In resolveExecutionCwd, validate the path returned from
resolveConversationWorkdir before using it as the cwd: after obtaining
resolvedWorkdir (in the resolveExecutionCwd method), call fs.promises.stat() (or
fs.stat) to ensure the path exists and is a directory; if stat fails or the path
is not a directory, log a warning (including conversationId and the invalid
path) and fall back to normalizedSkillRoot; keep resolveConversationWorkdir
error handling as-is but add this extra validation step before returning
path.resolve(normalizedWorkdir).
In `@test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts`:
- Around line 217-222: The assertion hardcodes '/tmp/deepchat-acp/sessions'
which breaks on Windows; update the test to build a platform-safe expectation by
importing Node's path and asserting cwd contains the platform-joined subpath
instead of the full hardcoded string—for example, in acpProcessManager.test.ts
change the createTerminal expectation (the expect.objectContaining block for
createTerminal) to use cwd:
expect.stringContaining(path.join('deepchat-acp','sessions')) (keep sessionId:
'missing-session' and command: 'pwd' the same) so the test uses path.join and is
path-separator agnostic.
In `@test/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.ts`:
- Around line 38-44: Tests assume a Unix shell and POSIX paths; make the spawn
expectations platform-aware by branching on process.platform (or using
path.normalize) so the assertion matches the correct shell executable and cwd
format. Update the assertions around spawn in acpTerminalManager.test.ts (the
mocked spawn checks) to: check cwd using path.normalize or
expect.stringContaining(path.normalize('/tmp/workspace')) and assert the shell
argument with a platform-specific matcher (e.g., allow '/bin/bash' or 'bash' on
POSIX and 'cmd.exe' or 'powershell' on win32), or use
expect.stringMatching(/bash|cmd|powershell/i) for the first arg—apply the same
change to the other spawn assertion block (the one at lines ~55-63).
---
Outside diff comments:
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 275-288: When inferring providerId (the providerId variable and
the branch that calls this.configPresenter.getAcpAgents()), normalize/resolve
legacy ACP agent aliases before comparing against acpAgents (i.e., don't compare
raw session.agentId); call the existing configPresenter method that returns the
canonical ACP id for a given agent (e.g., a resolve/getCanonicalAcpId helper on
configPresenter) on session.agentId and use that normalized id in the
acpAgents.some(...) check, then propagate that same normalized logic to the
other branch that determines ACP (the code path that later calls
assertAcpSessionHasWorkdir and syncAcpSessionWorkdir / setAcpWorkdir) so legacy
ids like "kimi-cli" or "claude-code-acp" are recognized as ACP.
---
Nitpick comments:
In `@src/main/presenter/llmProviderPresenter/acp/acpTerminalManager.ts`:
- Around line 40-59: The resolveTerminalCwd function should verify that the
provided cwd actually exists and is a directory before returning it; change the
early return path in resolveTerminalCwd to check fs.existsSync(normalized) and
fs.statSync(normalized).isDirectory() (wrapped in try/catch), and if the check
fails, log a warning and fall back to creating/using the fallbackDir (same logic
already present) so stale/deleted session cwd values don't cause terminal spawn
failures; apply the same existence/is-directory guard wherever terminal spawn
uses resolveTerminalCwd or similar cwd resolution.
In `@test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts`:
- Around line 150-151: Replace direct private map mutation of (manager as
any).sessionWorkdirs with the public API: call
manager.registerSessionWorkdir('session-1', '/tmp/workspace') so the test
exercises real lifecycle behavior (including fs handler setup); update the other
occurrence at the second location (lines 179-180) similarly. Ensure you
reference the manager instance and use the public registerSessionWorkdir method
rather than touching sessionWorkdirs directly.
In `@test/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.ts`:
- Around line 23-27: Add an afterEach block to restore spies/mocks so they don't
leak between tests: afterEach should call vi.restoreAllMocks() (to restore
vi.spyOn(fs, 'mkdirSync') and other spies) and vi.clearAllMocks() (to clear
calls), ensuring the mocked spawn (vi.mocked(spawn)) and the createPty-related
mocks are reset between tests and preventing accidental cross-test interference.
In `@test/main/presenter/skillPresenter/skillExecutionService.test.ts`:
- Around line 106-116: Add an assertion that the resolver for the conversation
workdir was invoked with the conversation id when building the spawn plan: after
calling (service as never).buildSpawnPlan(...) assert that
resolveConversationWorkdir was called with 'conv-1' (or the variable used for
the conversation id) to ensure the propagation contract is enforced; locate the
mock or spy for resolveConversationWorkdir in the test setup (or create one if
missing) and use the test framework's call/spy assertion (e.g.,
toHaveBeenCalledWith or equivalent) to validate the invocation alongside the
existing expectations on plan.cwd and plan.env.PATH.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e99f9f3-c558-484f-bf68-006d981ed046
📒 Files selected for processing (13)
docs/specs/skill-runtime-hardening/spec.mdsrc/main/presenter/llmProviderPresenter/acp/acpProcessManager.tssrc/main/presenter/llmProviderPresenter/acp/acpTerminalManager.tssrc/main/presenter/newAgentPresenter/index.tssrc/main/presenter/skillPresenter/index.tssrc/main/presenter/skillPresenter/skillExecutionService.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tstest/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.tstest/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.tstest/main/presenter/newAgentPresenter/integration.test.tstest/main/presenter/newAgentPresenter/newAgentPresenter.test.tstest/main/presenter/newAgentPresenter/usageDashboard.test.tstest/main/presenter/skillPresenter/skillExecutionService.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/presenter/skillPresenter/skillExecutionService.ts (1)
190-221: Avoid duplicate warning logs for one fallback event.When workdir resolution fails/non-directory, the method logs a specific warning and then also logs “Missing conversation workdir.” This can create noisy, misleading duplicate warnings for a single failure.
🧹 Suggested adjustment
private async resolveExecutionCwd(conversationId: string, skillRoot: string): Promise<string> { const normalizedSkillRoot = path.resolve(skillRoot) if (!this.resolveConversationWorkdir) { return normalizedSkillRoot } try { const resolvedWorkdir = await this.resolveConversationWorkdir(conversationId) const normalizedWorkdir = resolvedWorkdir?.trim() if (normalizedWorkdir) { const resolvedPath = path.resolve(normalizedWorkdir) try { const stat = await fs.promises.stat(resolvedPath) if (stat.isDirectory()) { return resolvedPath } logger.warn( '[SkillExecutionService] Conversation workdir is not a directory, falling back to skill root', { conversationId, invalidWorkdir: resolvedPath } ) + return normalizedSkillRoot } catch (error) { logger.warn( '[SkillExecutionService] Conversation workdir is invalid, falling back to skill root', { conversationId, invalidWorkdir: resolvedPath, error } ) + return normalizedSkillRoot } } } catch (error) { logger.warn('[SkillExecutionService] Failed to resolve conversation workdir', { conversationId, error }) + return normalizedSkillRoot } logger.warn( '[SkillExecutionService] Missing conversation workdir, falling back to skill root', { conversationId, skillRoot: normalizedSkillRoot } ) return normalizedSkillRoot }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/skillPresenter/skillExecutionService.ts` around lines 190 - 221, The method in SkillExecutionService emits multiple logger.warn messages for the same fallback (e.g., when resolvedPath is not a directory or resolution throws), causing duplicate "Missing conversation workdir" warnings; change the control flow so only one warning is emitted per fallback: either return/exit the function after logging the specific warning for resolvedPath (the logger.warn calls that include conversationId and invalidWorkdir) or introduce a boolean flag (e.g., workdirResolved or didFallback) and set it when you log the specific warning, then skip the final logger.warn that logs "Missing conversation workdir" (which uses normalizedSkillRoot and conversationId) if that flag is set; update logic around the try/catch blocks where resolvedPath, error, normalizedSkillRoot and conversationId are used to ensure a single consolidated warning is logged.src/main/presenter/newAgentPresenter/index.ts (1)
293-298: Consider guarding redundant sync calls with a dirty check.
syncAcpSessionWorkdiris called on everysendMessageandqueuePendingInput, even when the workdir hasn't changed since the last sync. Per theacpProvider.updateAcpWorkdirimplementation (context snippet), this is idempotent—the session is only cleared if the resolved path differs—but it still incurs unnecessary async I/O.A simple optimization would be to track the last-synced workdir per session and skip the call when unchanged.
Also applies to: 348-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/newAgentPresenter/index.ts` around lines 293 - 298, Add a per-session "lastSyncedWorkdir" check to avoid redundant async syncs: in the callers sendMessage and queuePendingInput, before calling syncAcpSessionWorkdir(providerId, sessionId, session.agentId, session.projectDir ?? null) compare session.projectDir (or resolved workdir) against a stored lastSyncedWorkdir on the session (or in a small in-memory map keyed by sessionId); only call syncAcpSessionWorkdir when the value differs, and update lastSyncedWorkdir after a successful sync. Keep reference to syncAcpSessionWorkdir, sendMessage, queuePendingInput, and acpProvider.updateAcpWorkdir to locate where to add the check and update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 293-298: Add a per-session "lastSyncedWorkdir" check to avoid
redundant async syncs: in the callers sendMessage and queuePendingInput, before
calling syncAcpSessionWorkdir(providerId, sessionId, session.agentId,
session.projectDir ?? null) compare session.projectDir (or resolved workdir)
against a stored lastSyncedWorkdir on the session (or in a small in-memory map
keyed by sessionId); only call syncAcpSessionWorkdir when the value differs, and
update lastSyncedWorkdir after a successful sync. Keep reference to
syncAcpSessionWorkdir, sendMessage, queuePendingInput, and
acpProvider.updateAcpWorkdir to locate where to add the check and update.
In `@src/main/presenter/skillPresenter/skillExecutionService.ts`:
- Around line 190-221: The method in SkillExecutionService emits multiple
logger.warn messages for the same fallback (e.g., when resolvedPath is not a
directory or resolution throws), causing duplicate "Missing conversation
workdir" warnings; change the control flow so only one warning is emitted per
fallback: either return/exit the function after logging the specific warning for
resolvedPath (the logger.warn calls that include conversationId and
invalidWorkdir) or introduce a boolean flag (e.g., workdirResolved or
didFallback) and set it when you log the specific warning, then skip the final
logger.warn that logs "Missing conversation workdir" (which uses
normalizedSkillRoot and conversationId) if that flag is set; update logic around
the try/catch blocks where resolvedPath, error, normalizedSkillRoot and
conversationId are used to ensure a single consolidated warning is logged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c784ae9d-be6b-4828-aa10-9be868fdc80b
📒 Files selected for processing (7)
src/main/presenter/newAgentPresenter/index.tssrc/main/presenter/skillPresenter/skillExecutionService.tstest/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.tstest/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.tstest/main/presenter/newAgentPresenter/newAgentPresenter.test.tstest/main/presenter/skillPresenter/skillExecutionService.test.tstest/setup.ts
✅ Files skipped from review due to trivial changes (2)
- test/setup.ts
- test/main/presenter/newAgentPresenter/newAgentPresenter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/main/presenter/skillPresenter/skillExecutionService.test.ts
This was referenced Mar 30, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Documentation
Bug Fixes
Tests