Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces Subagent Orchestrator V1 for DeepChat, enabling parent agents to delegate tasks to configured child subagent sessions. Adds comprehensive spec/plan documentation, session database schema extensions for parent-child relationships, a new Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 11
🧹 Nitpick comments (10)
test/renderer/components/ChatSidePanel.test.ts (1)
79-81: Strengthen session-scoping coverage in this test.Line 79 validates the matching session payload, but Line 81 only checks total call count.
Consider adding a mismatch case to ensure a differentsessionIddoes not open the browser panel.🧪 Suggested test hardening
handler?.({}, { windowId: 7, sessionId: 'session-1' }) expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1) + + handler?.({}, { windowId: 7, sessionId: 'session-2' }) + expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/ChatSidePanel.test.ts` around lines 79 - 81, Add a negative test case that calls handler with a non-matching sessionId and assert it does not trigger sidepanelStore.openBrowser again: after the existing call handler?.({}, { windowId: 7, sessionId: 'session-1' }) and expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1), invoke handler?.({}, { windowId: 7, sessionId: 'other-session' }) and assert expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1) (or assert not called with the mismatched payload); use the existing handler and sidepanelStore.openBrowser mocks to validate session-scoping behavior.test/main/presenter/newAgentPresenter/sessionManager.test.ts (1)
79-82: Add one non-default mapping test for new subagent fields.Current assertions validate defaults well; add a case where DB fields are explicitly populated to protect mapping logic from regressions.
🧪 Suggested test addition
+ it('maps explicit subagent fields from DB record', () => { + sqlitePresenter.newSessionsTable.get.mockReturnValue({ + id: 's2', + agent_id: 'deepchat', + title: 'Child', + project_dir: '/tmp/proj', + is_pinned: 0, + is_draft: 0, + session_kind: 'subagent', + parent_session_id: 'p1', + subagent_enabled: 1, + subagent_meta_json: '{"slotId":"slot-1"}', + created_at: 1, + updated_at: 2 + }) + + const record = manager.get('s2') + expect(record?.sessionKind).toBe('subagent') + expect(record?.parentSessionId).toBe('p1') + expect(record?.subagentEnabled).toBe(true) + expect(record?.subagentMeta).toEqual({ slotId: 'slot-1' }) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/newAgentPresenter/sessionManager.test.ts` around lines 79 - 82, Add a new unit test in sessionManager.test.ts that covers a non-default mapping of the new subagent fields: construct a DB session object with sessionKind: 'regular', parentSessionId set to a non-null value, subagentEnabled: true, and subagentMeta populated with a sample object, then call the same mapping function used in the existing tests (the presenter/session mapping helper used in this file) and assert the mapped result exposes the non-default values for parentSessionId, subagentEnabled, and subagentMeta; name the test something like "maps non-default subagent fields" and place it alongside the other mapping tests to guard against regressions.src/main/presenter/sqlitePresenter/tables/newSessions.ts (1)
67-73: Add an index for parent-child session lookups.The new
parent_session_idfilter will otherwise scan and sort the fullnew_sessionstable whenever the subagents view refreshes. An index onparent_session_id— ideally paired withupdated_at— will keep this query predictable as session history grows. You'll want the sameCREATE INDEXin the migration path too, not just fresh-table creation.📈 Suggested index
CREATE TABLE IF NOT EXISTS new_sessions ( ${columns.join(',\n ')} ); CREATE INDEX IF NOT EXISTS idx_new_sessions_agent ON new_sessions(agent_id); + CREATE INDEX IF NOT EXISTS idx_new_sessions_parent_updated + ON new_sessions(parent_session_id, updated_at DESC); CREATE INDEX IF NOT EXISTS idx_new_sessions_updated ON new_sessions(updated_at DESC);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/sqlitePresenter/tables/newSessions.ts` around lines 67 - 73, Add a composite index on parent_session_id (and include updated_at for ordering) to the new_sessions schema: update the CREATE TABLE/INDEX block that returns the SQL in newSessions (the template that currently creates idx_new_sessions_agent and idx_new_sessions_updated) to also CREATE INDEX IF NOT EXISTS idx_new_sessions_parent_updated ON new_sessions(parent_session_id, updated_at DESC); and mirror that same CREATE INDEX statement in the migration path that applies schema changes so existing databases get the new index as well.src/renderer/src/pages/NewThreadPage.vue (1)
365-377: Normalize bothresolveDeepChatAgentConfig()branches.Only the fallback branch currently passes through
normalizeDeepChatSubagentConfig(). Keeping the presenter branch raw makes this page depend on another layer already having expanded the new subagent fields, which is easy for mocks and alternate implementations to miss.♻️ Keep a single config invariant here
const resolveDeepChatAgentConfig = async (agentId: string): Promise<DeepChatAgentConfig> => { if (typeof configPresenter.resolveDeepChatAgentConfig === 'function') { - return configPresenter.resolveDeepChatAgentConfig(agentId) + return normalizeDeepChatSubagentConfig( + await configPresenter.resolveDeepChatAgentConfig(agentId) + ) } const systemPrompt = await configPresenter.getSetting?.('default_system_prompt')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/pages/NewThreadPage.vue` around lines 365 - 377, The two code paths in resolveDeepChatAgentConfig must return the same normalized shape: when configPresenter.resolveDeepChatAgentConfig exists, await its result and pass it through normalizeDeepChatSubagentConfig before returning (instead of returning the raw presenter value), so both branches produce a DeepChatAgentConfig; keep the fallback branch as-is (using getSetting) and ensure you preserve typing/async behavior and any default fields when calling normalizeDeepChatSubagentConfig.src/renderer/settings/components/DeepChatAgentsSettings.vue (1)
440-452: Consider using a consistent Select component instead of native<select>.The subagent target dropdown uses a native
<select>element while other dropdowns in this component use shadcn'sDropdownMenuorPopoverwithSelect. This creates visual inconsistency in the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/settings/components/DeepChatAgentsSettings.vue` around lines 440 - 452, The native <select> for subagent targets should be replaced with the project's consistent Select component (used elsewhere) to match visual style; locate the block rendering getSubagentTargetValue(slot), handleSubagentTargetChange(slot, $event) and iterating subagentTargetOptions, and render those options via the Select/SelectTrigger/SelectContent pattern used in this file (keeping the same value binding from getSubagentTargetValue and invoking handleSubagentTargetChange with the selected value), ensure keys use agentOption.value and labels use agentOption.label so behavior and data flow remain identical.src/renderer/src/components/sidepanel/WorkspacePanel.vue (1)
100-125: The highlight conditionchild.id === props.sessionIdis always false.In
childSessions, all sessions are filtered to haveparentSessionId === props.sessionId. Therefore, no child session'sidcan equalprops.sessionId, making the highlight conditional always evaluate to the "not selected" branch. This is likely copy-paste from similar patterns.Consider removing the dead highlight logic or replacing it with actual selection tracking if child selection highlighting is intended.
♻️ Simplify by removing unused highlight logic
<button v-for="child in childSessions" :key="child.id" class="flex w-full items-start gap-2 px-3 py-2 text-left text-xs transition-colors" - :class=" - child.id === props.sessionId - ? 'bg-accent text-accent-foreground' - : 'text-muted-foreground hover:bg-accent/60 hover:text-foreground' - " + class="text-muted-foreground hover:bg-accent/60 hover:text-foreground" type="button" `@click`="handleSubagentOpen(child.id)" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 100 - 125, The highlight conditional child.id === props.sessionId is dead because childSessions are filtered by parentSessionId === props.sessionId; remove or replace it: either drop the ternary and always apply the "not selected" class, or implement proper child selection tracking (introduce a selectedChildId ref/prop or use existing selected state) and change the condition to child.id === selectedChildId; update the click handler (handleSubagentOpen) to set selectedChildId if you choose selection tracking so the button highlights the clicked child correctly.src/renderer/src/components/message/MessageBlockToolCall.vue (1)
351-362: Consider adding basic property validation after JSON parsing.The
parseSubagentProgressfunction casts the parsed JSON directly toSubagentProgressPayloadwith only anArray.isArray(parsed?.tasks)check. If the JSON structure doesn't fully match the expected type (e.g.,modeis missing or invalid), downstream code accessingprogress.modecould produce unexpected results.♻️ Optional: Add minimal schema validation
const parseSubagentProgress = (value: unknown): SubagentProgressPayload | null => { if (typeof value !== 'string' || !value.trim()) { return null } try { const parsed = JSON.parse(value) as SubagentProgressPayload - return Array.isArray(parsed?.tasks) ? parsed : null + if (!Array.isArray(parsed?.tasks)) { + return null + } + if (parsed.mode !== 'parallel' && parsed.mode !== 'chain') { + return null + } + return parsed } catch { return null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/message/MessageBlockToolCall.vue` around lines 351 - 362, The parseSubagentProgress function currently only checks that parsed?.tasks is an array; update it to perform minimal schema validation after JSON.parse: ensure parsed is an object, parsed.mode exists and is a string (and optionally one of the allowed mode values used elsewhere), parsed.tasks is an array, and each task item has the expected basic properties (e.g., id and status as strings or the minimal types your code expects). If any of these checks fail return null; keep the existing try/catch around JSON.parse and only cast/return the parsed object when all validations pass. Use the function name parseSubagentProgress and the SubagentProgressPayload shape to guide which fields to validate.docs/specs/subagent-orchestrator/spec.md (1)
78-83: Add an index forparent_session_idin the migration plan.The workspace list, restart recovery, and parent→child lookup paths will all query by this field. Without an index, those flows degrade to full scans as
new_sessionsgrows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/specs/subagent-orchestrator/spec.md` around lines 78 - 83, Add a DB index on parent_session_id to avoid full-table scans: in the migration that alters the new_sessions table (where you added subagent_enabled, session_kind, parent_session_id, subagent_meta_json) add an index creation for parent_session_id (e.g. CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_new_sessions_parent_session_id ON new_sessions(parent_session_id)) in the up path and drop it in the down path; ensure the migration uses CONCURRENTLY if supported and update any schema/verification tests or CI migration checks to expect the new index.src/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.ts (1)
70-80: Deferred promise pattern works but could use standard approach.The pattern is functional, but the empty initial
resolvefunction assignment could be confusing. Consider using TypeScript's non-null assertion or a well-known deferred utility.Alternative using non-null assertion
const createDeferred = (): MutableTaskState['completion'] => { - let resolve = () => {} + let resolve!: () => void const promise = new Promise<void>((innerResolve) => { resolve = innerResolve }) return { promise, resolve } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.ts` around lines 70 - 80, The createDeferred function currently initializes resolve to a no-op which is confusing; change it to use TypeScript's non-null assertion so resolve is declared but assigned inside the Promise: replace "let resolve = () => {}" with "let resolve!: () => void" (keeping the Promise initializer that assigns innerResolve to resolve) so MutableTaskState['completion'] returns a properly-typed promise and resolve without a placeholder no-op; alternatively import/use a shared deferred utility and return its completion shape to avoid manual boilerplate.src/main/presenter/newAgentPresenter/index.ts (1)
421-427: Unsafe cast afterbuildSessionWithState.
buildSessionWithStatecan throw ifresolveAgentImplementationfails (e.g., agent not found). While the session record check is present, the subsequent call isn't wrapped in try-catch, so any error propagates. Consider usingtryBuildSessionWithStatewith a null guard or keeping the current pattern but documenting that errors propagate intentionally.Optional: Use try-catch for consistent error messaging
const record = this.sessionManager.get(sessionId) if (!record) { throw new Error(`Subagent session not found after creation: ${sessionId}`) } - return (await this.buildSessionWithState(record)) as SessionWithState + try { + return await this.buildSessionWithState(record) + } catch (error) { + throw new Error(`Failed to build subagent session state: ${sessionId}`) + }🤖 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 421 - 427, The code unsafely casts the result of buildSessionWithState to SessionWithState without handling errors that buildSessionWithState may throw (e.g., from resolveAgentImplementation); wrap the call in a try-catch and rethrow or return a clearer error, or call tryBuildSessionWithState and guard the null result before casting, while keeping the existing sessionManager.get check; specifically update the block that uses sessionManager.get(sessionId) so it calls tryBuildSessionWithState(record) and checks for null (or wrap await this.buildSessionWithState(record) in try-catch) and produce a consistent error message referencing sessionId and the underlying failure.
🤖 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/deepchatAgentPresenter/dispatch.ts`:
- Around line 784-795: The current guard only updates the UI when
extractSubagentToolState returns subagentProgress, so a final-only payload
(subagentFinal) is dropped; change the conditional in dispatch.ts to run
updateSubagentToolCallBlock whenever either subagentState.subagentProgress or
subagentState.subagentFinal is present (e.g., if (subagentState.subagentProgress
|| subagentState.subagentFinal)), keep the same content conversion logic using
toolResponseToText for non-string content, and pass both
subagentState.subagentProgress and subagentState.subagentFinal to
updateSubagentToolCallBlock so final-only updates are persisted.
In `@src/main/presenter/deepchatAgentPresenter/index.ts`:
- Around line 2941-2959: The call to this.toolPresenter.callTool is using
this.getAbortSignalForSession(sessionId) which is often undefined because the
paused-generation controller was cleared; create and register a dedicated
AbortController for this deferred subagent tool run before invoking callTool
(keyed by sessionId and toolCall.id), pass its signal into callTool, and ensure
you unregister/abort that controller in the finally/cleanup path so
cancelGeneration(sessionId) (and cancel logic) can find and cancel this running
subagent_orchestrator; updateSubagentToolCallProgress remains unchanged but use
toolCall.id when registering/removing the controller so cancelGeneration can
reference it.
In `@src/main/presenter/deepchatAgentPresenter/internalSessionEvents.ts`:
- Around line 72-106: extractWaitingInteraction currently scans blocks backwards
and returns the last pending action, but respondToolInteraction expects the
earliest unresolved action for a message; change the scan order to iterate from
the start (index 0 to blocks.length-1) so the function returns the earliest
pending interaction. Keep the same matching conditions (block.type !== 'action',
block.status !== 'pending', block.extra?.needsUserAction !== true,
!block.tool_call?.id) and return shapes (type 'permission' or 'question',
messageId, toolCallId, actionBlock as JSON clone) when the first matching block
is found so the resume path and respondToolInteraction receive the same pending
entry.
- Around line 51-61: The current toDisplayLines used by
buildAssistantResponseMarkdown trims each line and removes blank lines, which
destroys leading indentation and empty-line formatting (breaking fenced code,
nested lists, YAML) when mirroring child responses; change toDisplayLines to
only split on CRLF/newline and return lines as-is (do not .trim() and do not
.filter(Boolean)) so extractBlockText -> toDisplayLines ->
buildAssistantResponseMarkdown preserves original whitespace and blank lines.
- Around line 109-117: The emitDeepChatInternalSessionUpdate function currently
calls emitter.emit('update', update) synchronously which lets exceptions thrown
by subscribers bubble up; modify emitDeepChatInternalSessionUpdate to wrap the
emitter.emit call in a try-catch so subscriber failures are isolated, catch any
thrown error and log it (use the existing logger if available or console.error)
and swallow or handle the error so it does not propagate to callers like
setSessionStatus or emitMessageRefresh; keep
subscribeDeepChatInternalSessionUpdates unchanged and reference emitter.emit and
emitDeepChatInternalSessionUpdate when making the change.
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 1229-1251: The method setSessionSubagentEnabled currently
force-casts the result of tryBuildSessionWithState to SessionWithState even
though tryBuildSessionWithState can return null; change this by removing the
unsafe cast, capturing the result into a variable, checking if it's null, and
throwing a descriptive error (e.g., "Failed to build session state for
sessionId") before returning; ensure this check happens after
sessionManager.update and emitSessionListUpdated so callers never receive a null
value from setSessionSubagentEnabled.
In `@src/main/presenter/sqlitePresenter/tables/newSessions.ts`:
- Around line 209-215: The current logic adds "session_kind = 'regular'"
whenever filters.includeSubagents !== true, which unintentionally excludes child
sessions when filters.parentSessionId is provided; change the condition so that
session_kind='regular' is only added when includeSubagents is explicitly false
(or when parentSessionId is not provided). Concretely, update the check around
conditions.push("session_kind = 'regular'") to skip adding that condition if
filters.parentSessionId is set (or use filters.includeSubagents === false
instead of !== true), so that queries with filters.parentSessionId return child
sessions as expected; adjust any affected params handling accordingly
(references: filters.includeSubagents, filters.parentSessionId,
conditions.push("session_kind = 'regular'")).
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 982-992: The computed showSubagentToggle currently allows the
toggle for any non-ACP active session with sessionKind === 'regular'; update the
active-session branch to also require the active session's agentType to be
'deepchat' so the toggle is only shown for DeepChat sessions. Specifically, in
the computed showing logic (showSubagentToggle) change the hasActiveSession
branch to return true only when sessionStore.activeSession?.sessionKind ===
'regular' AND sessionStore.activeSession?.agentType === 'deepchat' (keep
existing isAcpAgent and selectedAgentType checks and use null-safe access).
In `@src/renderer/src/i18n/ru-RU/chat.json`:
- Line 172: Translate the English value for the key workspace.sections.artifacts
in the ru-RU bundle to Russian (replace "Artifacts" with an appropriate Russian
translation like "Артефакты") so the ru-RU chat.json no longer contains
mixed-language UI; update the value for the key "artifacts" in the existing
object entry to the Russian string.
In `@src/renderer/src/i18n/zh-HK/settings.json`:
- Line 1630: The value for the i18n key subagentsTitle is still in English;
update the value for "subagentsTitle" to a Traditional Chinese translation
(e.g., "子代理") so it matches the surrounding zh-HK strings, keeping the key name
unchanged and preserving JSON formatting in settings.json.
In
`@test/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.test.ts`:
- Around line 43-55: The test's workdir source is ambiguous because
resolveConversationWorkdir, parentSession.projectDir, and
childSession.projectDir currently all equal the same path, so the assertion
doesn't confirm the tool actually uses the resolver; update the fixture so these
three values differ (e.g., give parentSession.projectDir and
childSession.projectDir distinct paths and have
resolveConversationWorkdir.mockResolvedValue return the expected handoff path)
and assert that SubagentOrchestratorTool uses resolveConversationWorkdir (the
mocked function) rather than copying either session.projectDir; change the
buildSessionInfo calls for parentSession and childSession to set unique
projectDir values and adjust the resolveConversationWorkdir mock return
accordingly.
---
Nitpick comments:
In `@docs/specs/subagent-orchestrator/spec.md`:
- Around line 78-83: Add a DB index on parent_session_id to avoid full-table
scans: in the migration that alters the new_sessions table (where you added
subagent_enabled, session_kind, parent_session_id, subagent_meta_json) add an
index creation for parent_session_id (e.g. CREATE INDEX CONCURRENTLY IF NOT
EXISTS idx_new_sessions_parent_session_id ON new_sessions(parent_session_id)) in
the up path and drop it in the down path; ensure the migration uses CONCURRENTLY
if supported and update any schema/verification tests or CI migration checks to
expect the new index.
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 421-427: The code unsafely casts the result of
buildSessionWithState to SessionWithState without handling errors that
buildSessionWithState may throw (e.g., from resolveAgentImplementation); wrap
the call in a try-catch and rethrow or return a clearer error, or call
tryBuildSessionWithState and guard the null result before casting, while keeping
the existing sessionManager.get check; specifically update the block that uses
sessionManager.get(sessionId) so it calls tryBuildSessionWithState(record) and
checks for null (or wrap await this.buildSessionWithState(record) in try-catch)
and produce a consistent error message referencing sessionId and the underlying
failure.
In `@src/main/presenter/sqlitePresenter/tables/newSessions.ts`:
- Around line 67-73: Add a composite index on parent_session_id (and include
updated_at for ordering) to the new_sessions schema: update the CREATE
TABLE/INDEX block that returns the SQL in newSessions (the template that
currently creates idx_new_sessions_agent and idx_new_sessions_updated) to also
CREATE INDEX IF NOT EXISTS idx_new_sessions_parent_updated ON
new_sessions(parent_session_id, updated_at DESC); and mirror that same CREATE
INDEX statement in the migration path that applies schema changes so existing
databases get the new index as well.
In `@src/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.ts`:
- Around line 70-80: The createDeferred function currently initializes resolve
to a no-op which is confusing; change it to use TypeScript's non-null assertion
so resolve is declared but assigned inside the Promise: replace "let resolve =
() => {}" with "let resolve!: () => void" (keeping the Promise initializer that
assigns innerResolve to resolve) so MutableTaskState['completion'] returns a
properly-typed promise and resolve without a placeholder no-op; alternatively
import/use a shared deferred utility and return its completion shape to avoid
manual boilerplate.
In `@src/renderer/settings/components/DeepChatAgentsSettings.vue`:
- Around line 440-452: The native <select> for subagent targets should be
replaced with the project's consistent Select component (used elsewhere) to
match visual style; locate the block rendering getSubagentTargetValue(slot),
handleSubagentTargetChange(slot, $event) and iterating subagentTargetOptions,
and render those options via the Select/SelectTrigger/SelectContent pattern used
in this file (keeping the same value binding from getSubagentTargetValue and
invoking handleSubagentTargetChange with the selected value), ensure keys use
agentOption.value and labels use agentOption.label so behavior and data flow
remain identical.
In `@src/renderer/src/components/message/MessageBlockToolCall.vue`:
- Around line 351-362: The parseSubagentProgress function currently only checks
that parsed?.tasks is an array; update it to perform minimal schema validation
after JSON.parse: ensure parsed is an object, parsed.mode exists and is a string
(and optionally one of the allowed mode values used elsewhere), parsed.tasks is
an array, and each task item has the expected basic properties (e.g., id and
status as strings or the minimal types your code expects). If any of these
checks fail return null; keep the existing try/catch around JSON.parse and only
cast/return the parsed object when all validations pass. Use the function name
parseSubagentProgress and the SubagentProgressPayload shape to guide which
fields to validate.
In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue`:
- Around line 100-125: The highlight conditional child.id === props.sessionId is
dead because childSessions are filtered by parentSessionId === props.sessionId;
remove or replace it: either drop the ternary and always apply the "not
selected" class, or implement proper child selection tracking (introduce a
selectedChildId ref/prop or use existing selected state) and change the
condition to child.id === selectedChildId; update the click handler
(handleSubagentOpen) to set selectedChildId if you choose selection tracking so
the button highlights the clicked child correctly.
In `@src/renderer/src/pages/NewThreadPage.vue`:
- Around line 365-377: The two code paths in resolveDeepChatAgentConfig must
return the same normalized shape: when
configPresenter.resolveDeepChatAgentConfig exists, await its result and pass it
through normalizeDeepChatSubagentConfig before returning (instead of returning
the raw presenter value), so both branches produce a DeepChatAgentConfig; keep
the fallback branch as-is (using getSetting) and ensure you preserve
typing/async behavior and any default fields when calling
normalizeDeepChatSubagentConfig.
In `@test/main/presenter/newAgentPresenter/sessionManager.test.ts`:
- Around line 79-82: Add a new unit test in sessionManager.test.ts that covers a
non-default mapping of the new subagent fields: construct a DB session object
with sessionKind: 'regular', parentSessionId set to a non-null value,
subagentEnabled: true, and subagentMeta populated with a sample object, then
call the same mapping function used in the existing tests (the presenter/session
mapping helper used in this file) and assert the mapped result exposes the
non-default values for parentSessionId, subagentEnabled, and subagentMeta; name
the test something like "maps non-default subagent fields" and place it
alongside the other mapping tests to guard against regressions.
In `@test/renderer/components/ChatSidePanel.test.ts`:
- Around line 79-81: Add a negative test case that calls handler with a
non-matching sessionId and assert it does not trigger sidepanelStore.openBrowser
again: after the existing call handler?.({}, { windowId: 7, sessionId:
'session-1' }) and expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1),
invoke handler?.({}, { windowId: 7, sessionId: 'other-session' }) and assert
expect(sidepanelStore.openBrowser).toHaveBeenCalledTimes(1) (or assert not
called with the mismatched payload); use the existing handler and
sidepanelStore.openBrowser mocks to validate session-scoping behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba710e02-f220-4cd4-a6b6-df9b1719ce30
📒 Files selected for processing (82)
docs/specs/subagent-orchestrator/plan.mddocs/specs/subagent-orchestrator/spec.mddocs/specs/subagent-orchestrator/tasks.mdsrc/main/presenter/agentRepository/index.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/deepchatAgentPresenter/dispatch.tssrc/main/presenter/deepchatAgentPresenter/index.tssrc/main/presenter/deepchatAgentPresenter/internalSessionEvents.tssrc/main/presenter/index.tssrc/main/presenter/newAgentPresenter/index.tssrc/main/presenter/newAgentPresenter/sessionManager.tssrc/main/presenter/sqlitePresenter/tables/baseTable.tssrc/main/presenter/sqlitePresenter/tables/newSessions.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tssrc/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/toolPresenter/runtimePorts.tssrc/renderer/settings/components/DeepChatAgentsSettings.vuesrc/renderer/src/components/chat-input/McpIndicator.vuesrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/components/chat/ChatTopBar.vuesrc/renderer/src/components/chat/messageListItems.tssrc/renderer/src/components/message/MessageBlockToolCall.vuesrc/renderer/src/components/sidepanel/WorkspacePanel.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/da-DK/common.jsonsrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/en-US/common.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fa-IR/common.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/fr-FR/common.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/he-IL/common.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ja-JP/common.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ko-KR/common.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/pt-BR/common.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/ru-RU/common.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-CN/common.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-HK/common.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/i18n/zh-TW/common.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/pages/ChatPage.vuesrc/renderer/src/pages/NewThreadPage.vuesrc/renderer/src/stores/ui/draft.tssrc/renderer/src/stores/ui/session.tssrc/renderer/src/stores/ui/sidepanel.tssrc/shared/chat.d.tssrc/shared/lib/deepchatSubagents.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/new-agent.presenter.d.tssrc/shared/types/presenters/tool.presenter.d.tssrc/shared/types/presenters/workspace.d.tstest/main/presenter/newAgentPresenter/newAgentPresenter.test.tstest/main/presenter/newAgentPresenter/sessionManager.test.tstest/main/presenter/sqlitePresenter.test.tstest/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.tstest/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.test.tstest/renderer/components/ChatSidePanel.test.tstest/renderer/components/ChatStatusBar.test.tstest/renderer/components/DeepChatAgentsSettings.test.tstest/renderer/components/McpIndicator.test.tstest/renderer/components/message/MessageBlockToolCall.test.ts
Comment on lines
+784
to
+795
| const subagentState = extractSubagentToolState(toolRawData) | ||
| if (subagentState.subagentProgress) { | ||
| updateSubagentToolCallBlock( | ||
| state.blocks, | ||
| tc.id, | ||
| typeof toolRawData.content === 'string' | ||
| ? toolRawData.content | ||
| : toolResponseToText(toolRawData.content), | ||
| subagentState.subagentProgress, | ||
| subagentState.subagentFinal | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Persist subagentFinal even when the tool omits a progress snapshot.
The guard on Line 785 only runs when subagentProgress exists. If the tool returns a final-only payload, extra.subagentFinal is dropped and the renderer never sees the completed subagent state.
🩹 Suggested fix
- if (subagentState.subagentProgress) {
+ if (subagentState.subagentProgress || subagentState.subagentFinal) {
updateSubagentToolCallBlock(
state.blocks,
tc.id,
typeof toolRawData.content === 'string'
? toolRawData.content
: toolResponseToText(toolRawData.content),
- subagentState.subagentProgress,
+ subagentState.subagentProgress ?? subagentState.subagentFinal ?? '',
subagentState.subagentFinal
)
}📝 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 subagentState = extractSubagentToolState(toolRawData) | |
| if (subagentState.subagentProgress) { | |
| updateSubagentToolCallBlock( | |
| state.blocks, | |
| tc.id, | |
| typeof toolRawData.content === 'string' | |
| ? toolRawData.content | |
| : toolResponseToText(toolRawData.content), | |
| subagentState.subagentProgress, | |
| subagentState.subagentFinal | |
| ) | |
| } | |
| const subagentState = extractSubagentToolState(toolRawData) | |
| if (subagentState.subagentProgress || subagentState.subagentFinal) { | |
| updateSubagentToolCallBlock( | |
| state.blocks, | |
| tc.id, | |
| typeof toolRawData.content === 'string' | |
| ? toolRawData.content | |
| : toolResponseToText(toolRawData.content), | |
| subagentState.subagentProgress ?? subagentState.subagentFinal ?? '', | |
| subagentState.subagentFinal | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deepchatAgentPresenter/dispatch.ts` around lines 784 -
795, The current guard only updates the UI when extractSubagentToolState returns
subagentProgress, so a final-only payload (subagentFinal) is dropped; change the
conditional in dispatch.ts to run updateSubagentToolCallBlock whenever either
subagentState.subagentProgress or subagentState.subagentFinal is present (e.g.,
if (subagentState.subagentProgress || subagentState.subagentFinal)), keep the
same content conversion logic using toolResponseToText for non-string content,
and pass both subagentState.subagentProgress and subagentState.subagentFinal to
updateSubagentToolCallBlock so final-only updates are persisted.
Comment on lines
+2941
to
+2959
| const result = await this.toolPresenter.callTool(request, { | ||
| onProgress: (update) => { | ||
| if ( | ||
| update.kind !== 'subagent_orchestrator' || | ||
| update.toolCallId !== (toolCall.id || '') | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| this.updateSubagentToolCallProgress( | ||
| sessionId, | ||
| messageId, | ||
| toolCall.id || '', | ||
| update.responseMarkdown, | ||
| update.progressJson | ||
| ) | ||
| }, | ||
| signal: this.getAbortSignalForSession(sessionId) | ||
| }) |
Contributor
There was a problem hiding this comment.
Register an abort controller for deferred tool execution.
By the time Line 2958 runs, the paused generation controller has already been cleared, so getAbortSignalForSession() is usually undefined here. A long-running subagent_orchestrator started after permission approval therefore cannot be canceled via cancelGeneration(sessionId) until resume begins again.
💡 Suggested fix
- try {
- const result = await this.toolPresenter.callTool(request, {
+ const deferredAbortController = new AbortController()
+ this.abortControllers.set(sessionId, deferredAbortController)
+ try {
+ const result = await this.toolPresenter.callTool(request, {
onProgress: (update) => {
if (
update.kind !== 'subagent_orchestrator' ||
update.toolCallId !== (toolCall.id || '')
) {
return
}
this.updateSubagentToolCallProgress(
sessionId,
messageId,
toolCall.id || '',
update.responseMarkdown,
update.progressJson
)
},
- signal: this.getAbortSignalForSession(sessionId)
+ signal: deferredAbortController.signal
})
const rawData = result.rawData as MCPToolResponse
if (rawData.requiresPermission) {
return {
responseText: this.toolContentToText(rawData.content),
@@
} catch (error) {
const errorText = error instanceof Error ? error.message : String(error)
return {
responseText: `Error: ${errorText}`,
isError: true
}
+ } finally {
+ if (this.abortControllers.get(sessionId) === deferredAbortController) {
+ this.abortControllers.delete(sessionId)
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deepchatAgentPresenter/index.ts` around lines 2941 - 2959,
The call to this.toolPresenter.callTool is using
this.getAbortSignalForSession(sessionId) which is often undefined because the
paused-generation controller was cleared; create and register a dedicated
AbortController for this deferred subagent tool run before invoking callTool
(keyed by sessionId and toolCall.id), pass its signal into callTool, and ensure
you unregister/abort that controller in the finally/cleanup path so
cancelGeneration(sessionId) (and cancel logic) can find and cancel this running
subagent_orchestrator; updateSubagentToolCallProgress remains unchanged but use
toolCall.id when registering/removing the controller so cancelGeneration can
reference it.
Comment on lines
+51
to
+61
| const toDisplayLines = (text: string): string[] => | ||
| text | ||
| .split(/\r?\n/) | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean) | ||
|
|
||
| export const buildAssistantResponseMarkdown = (blocks: AssistantMessageBlock[]): string => | ||
| blocks | ||
| .flatMap((block) => extractBlockText(block)) | ||
| .flatMap((text) => toDisplayLines(text)) | ||
| .join('\n') |
Contributor
There was a problem hiding this comment.
Preserve markdown whitespace in responseMarkdown.
Line 54 trims leading indentation and Line 55 drops blank lines before the text is mirrored into the parent tool-call response. Child answers with fenced code, nested lists, or YAML will lose formatting in the parent card.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deepchatAgentPresenter/internalSessionEvents.ts` around
lines 51 - 61, The current toDisplayLines used by buildAssistantResponseMarkdown
trims each line and removes blank lines, which destroys leading indentation and
empty-line formatting (breaking fenced code, nested lists, YAML) when mirroring
child responses; change toDisplayLines to only split on CRLF/newline and return
lines as-is (do not .trim() and do not .filter(Boolean)) so extractBlockText ->
toDisplayLines -> buildAssistantResponseMarkdown preserves original whitespace
and blank lines.
Comment on lines
+72
to
+106
| export const extractWaitingInteraction = ( | ||
| blocks: AssistantMessageBlock[], | ||
| messageId: string | ||
| ): DeepChatInternalSessionWaitingInteraction | null => { | ||
| for (let index = blocks.length - 1; index >= 0; index -= 1) { | ||
| const block = blocks[index] | ||
| if ( | ||
| block.type !== 'action' || | ||
| block.status !== 'pending' || | ||
| block.extra?.needsUserAction !== true || | ||
| !block.tool_call?.id | ||
| ) { | ||
| continue | ||
| } | ||
|
|
||
| if (block.action_type === 'tool_call_permission') { | ||
| return { | ||
| type: 'permission', | ||
| messageId, | ||
| toolCallId: block.tool_call.id, | ||
| actionBlock: JSON.parse(JSON.stringify(block)) as AssistantMessageBlock | ||
| } | ||
| } | ||
|
|
||
| if (block.action_type === 'question_request') { | ||
| return { | ||
| type: 'question', | ||
| messageId, | ||
| toolCallId: block.tool_call.id, | ||
| actionBlock: JSON.parse(JSON.stringify(block)) as AssistantMessageBlock | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null |
Contributor
There was a problem hiding this comment.
Surface the same pending interaction that the resume path expects.
Line 76 walks backward, but respondToolInteraction() only accepts the earliest unresolved entry from the message. If a child message contains more than one pending action, the parent overlay can advertise a later interaction that the user still cannot answer yet.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deepchatAgentPresenter/internalSessionEvents.ts` around
lines 72 - 106, extractWaitingInteraction currently scans blocks backwards and
returns the last pending action, but respondToolInteraction expects the earliest
unresolved action for a message; change the scan order to iterate from the start
(index 0 to blocks.length-1) so the function returns the earliest pending
interaction. Keep the same matching conditions (block.type !== 'action',
block.status !== 'pending', block.extra?.needsUserAction !== true,
!block.tool_call?.id) and return shapes (type 'permission' or 'question',
messageId, toolCallId, actionBlock as JSON clone) when the first matching block
is found so the resume path and respondToolInteraction receive the same pending
entry.
Comment on lines
+109
to
+117
| export const emitDeepChatInternalSessionUpdate = (update: DeepChatInternalSessionUpdate): void => { | ||
| emitter.emit('update', update) | ||
| } | ||
|
|
||
| export const subscribeDeepChatInternalSessionUpdates = ( | ||
| listener: (update: DeepChatInternalSessionUpdate) => void | ||
| ): (() => void) => { | ||
| emitter.on('update', listener) | ||
| return () => emitter.off('update', listener) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Node.js EventEmitter.emit() invoke listeners synchronously, and do exceptions thrown inside a listener propagate back to the caller of emit()?
💡 Result:
Yes.
EventEmitter.emit()invokes listeners synchronously (it “synchronously calls each of the listeners … in the order they were registered”). [1]- Exceptions thrown inside a listener do propagate back to the caller of
emit()(Node’semit()directly calls each listener—e.g., viaReflectApply(...)—without wrapping the call in atry/catch, so a thrown error will bubble out ofemit()and can be caught by theemit()caller). [2]
Notes:
- Emitting
'error'with no'error'listener is a special case:emit('error', err)will throw (and typically crash the process if uncaught). [1]
Sources: [1] Node.js Events docs (EventEmitter.emit) • [2] Node.js core lib/events.js implementation
Isolate subscriber failures from the internal event bridge.
The emitter.emit() call at line 110 is synchronous and will propagate exceptions thrown by listeners directly back to the caller (emitDeepChatInternalSessionUpdate). A faulty subscriber could throw an exception, breaking chat processing in setSessionStatus() / emitMessageRefresh() and affecting unrelated sessions. Wrap the emit call in a try-catch block to prevent subscriber failures from cascading upstream.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deepchatAgentPresenter/internalSessionEvents.ts` around
lines 109 - 117, The emitDeepChatInternalSessionUpdate function currently calls
emitter.emit('update', update) synchronously which lets exceptions thrown by
subscribers bubble up; modify emitDeepChatInternalSessionUpdate to wrap the
emitter.emit call in a try-catch so subscriber failures are isolated, catch any
thrown error and log it (use the existing logger if available or console.error)
and swallow or handle the error so it does not propagate to callers like
setSessionStatus or emitMessageRefresh; keep
subscribeDeepChatInternalSessionUpdates unchanged and reference emitter.emit and
emitDeepChatInternalSessionUpdate when making the change.
Comment on lines
+209
to
+215
| if (filters?.includeSubagents !== true) { | ||
| conditions.push("session_kind = 'regular'") | ||
| } | ||
| if (filters?.parentSessionId !== undefined) { | ||
| conditions.push('parent_session_id = ?') | ||
| params.push(filters.parentSessionId) | ||
| } |
Contributor
There was a problem hiding this comment.
parentSessionId currently filters out the very rows it's supposed to find.
When callers pass parentSessionId without also remembering includeSubagents: true, Lines 209-210 still add session_kind = 'regular', so child-session lookups return []. That makes the new filter very easy to misuse and breaks the obvious list({ parentSessionId }) call shape.
🐛 Treat parent-scoped queries as subagent queries
- if (filters?.includeSubagents !== true) {
- conditions.push("session_kind = 'regular'")
- }
if (filters?.parentSessionId !== undefined) {
+ conditions.push("session_kind = 'subagent'")
conditions.push('parent_session_id = ?')
params.push(filters.parentSessionId)
+ } else if (filters?.includeSubagents !== true) {
+ conditions.push("session_kind = 'regular'")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/sqlitePresenter/tables/newSessions.ts` around lines 209 -
215, The current logic adds "session_kind = 'regular'" whenever
filters.includeSubagents !== true, which unintentionally excludes child sessions
when filters.parentSessionId is provided; change the condition so that
session_kind='regular' is only added when includeSubagents is explicitly false
(or when parentSessionId is not provided). Concretely, update the check around
conditions.push("session_kind = 'regular'") to skip adding that condition if
filters.parentSessionId is set (or use filters.includeSubagents === false
instead of !== true), so that queries with filters.parentSessionId return child
sessions as expected; adjust any affected params handling accordingly
(references: filters.includeSubagents, filters.parentSessionId,
conditions.push("session_kind = 'regular'")).
Comment on lines
+982
to
+992
| const showSubagentToggle = computed(() => { | ||
| if (isAcpAgent.value) { | ||
| return false | ||
| } | ||
|
|
||
| if (hasActiveSession.value) { | ||
| return sessionStore.activeSession?.sessionKind === 'regular' | ||
| } | ||
|
|
||
| return selectedAgentType.value === 'deepchat' | ||
| }) |
Contributor
There was a problem hiding this comment.
Gate the subagent toggle on the active session's agent type.
For active sessions, this currently shows the toggle for any non-ACP regular session. The feature is supposed to be DeepChat-only, so an ACP/custom regular session can expose a control that writes subagentEnabled on an unsupported session.
🩹 Suggested fix
+const activeSessionAgentType = computed(() =>
+ inferAgentType(sessionStore.activeSession?.agentId)
+)
+
const showSubagentToggle = computed(() => {
if (isAcpAgent.value) {
return false
}
if (hasActiveSession.value) {
- return sessionStore.activeSession?.sessionKind === 'regular'
+ return (
+ activeSessionAgentType.value === 'deepchat' &&
+ sessionStore.activeSession?.sessionKind === 'regular'
+ )
}
return selectedAgentType.value === 'deepchat'
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 982 - 992,
The computed showSubagentToggle currently allows the toggle for any non-ACP
active session with sessionKind === 'regular'; update the active-session branch
to also require the active session's agentType to be 'deepchat' so the toggle is
only shown for DeepChat sessions. Specifically, in the computed showing logic
(showSubagentToggle) change the hasActiveSession branch to return true only when
sessionStore.activeSession?.sessionKind === 'regular' AND
sessionStore.activeSession?.agentType === 'deepchat' (keep existing isAcpAgent
and selectedAgentType checks and use null-safe access).
| "files": "Файлы", | ||
| "git": "Git", | ||
| "artifacts": "Artifacts" | ||
| "artifacts": "Artifacts", |
Contributor
There was a problem hiding this comment.
Localize workspace.sections.artifacts in the Russian bundle.
Line 172 is still English, so the new workspace section renders mixed-language UI in ru-RU.
🌐 Suggested fix
- "artifacts": "Artifacts",
+ "artifacts": "Артефакты",📝 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
| "artifacts": "Artifacts", | |
| "artifacts": "Артефакты", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/i18n/ru-RU/chat.json` at line 172, Translate the English
value for the key workspace.sections.artifacts in the ru-RU bundle to Russian
(replace "Artifacts" with an appropriate Russian translation like "Артефакты")
so the ru-RU chat.json no longer contains mixed-language UI; update the value
for the key "artifacts" in the existing object entry to the Russian string.
| "compactionRetainPairs": "保留最近訊息對", | ||
| "deleteConfirm": "確定要刪除智能體「{name}」嗎？它的歷史會話會自動回退到內建 DeepChat 智能體。" | ||
| "deleteConfirm": "確定要刪除智能體「{name}」嗎？它的歷史會話會自動回退到內建 DeepChat 智能體。", | ||
| "subagentsTitle": "subagent", |
Contributor
There was a problem hiding this comment.
Localize subagentsTitle for zh-HK consistency.
"subagentsTitle": "subagent" is not localized while adjacent labels are Traditional Chinese. This will look inconsistent in the settings UI.
💡 Suggested change
- "subagentsTitle": "subagent",
+ "subagentsTitle": "子智能體",📝 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
| "subagentsTitle": "subagent", | |
| "subagentsTitle": "子智能體", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/i18n/zh-HK/settings.json` at line 1630, The value for the
i18n key subagentsTitle is still in English; update the value for
"subagentsTitle" to a Traditional Chinese translation (e.g., "子代理") so it
matches the surrounding zh-HK strings, keeping the key name unchanged and
preserving JSON formatting in settings.json.
Comment on lines
+43
to
+55
| const parentSession = buildSessionInfo() | ||
| const childSession = buildSessionInfo({ | ||
| sessionId: 'child-session', | ||
| agentName: 'Reviewer Clone', | ||
| sessionKind: 'subagent', | ||
| parentSessionId: parentSession.sessionId, | ||
| subagentEnabled: false, | ||
| availableSubagentSlots: [] | ||
| }) | ||
|
|
||
| const tool = new SubagentOrchestratorTool({ | ||
| resolveConversationWorkdir: vi.fn().mockResolvedValue(parentSession.projectDir), | ||
| resolveConversationSessionInfo: vi |
Contributor
There was a problem hiding this comment.
Make the workdir source unambiguous in this test.
resolveConversationWorkdir, parentSession.projectDir, and childSession.projectDir all resolve to the same /workspace/parent-app value right now. This still passes if the tool skips the resolver and copies either session field instead, so the test doesn't actually pin the handoff behavior it describes.
🧪 Tighten the fixture
- const parentSession = buildSessionInfo()
+ const parentSession = buildSessionInfo({
+ projectDir: '/workspace/parent-session'
+ })
const childSession = buildSessionInfo({
sessionId: 'child-session',
agentName: 'Reviewer Clone',
+ projectDir: '/workspace/child-session',
sessionKind: 'subagent',
parentSessionId: parentSession.sessionId,
subagentEnabled: false,
availableSubagentSlots: []
})
const tool = new SubagentOrchestratorTool({
- resolveConversationWorkdir: vi.fn().mockResolvedValue(parentSession.projectDir),
+ resolveConversationWorkdir: vi.fn().mockResolvedValue('/workspace/resolved-parent'),- expect(handoffMessage).toContain('/workspace/parent-app')
+ expect(handoffMessage).toContain('/workspace/resolved-parent')Also applies to: 120-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.test.ts`
around lines 43 - 55, The test's workdir source is ambiguous because
resolveConversationWorkdir, parentSession.projectDir, and
childSession.projectDir currently all equal the same path, so the assertion
doesn't confirm the tool actually uses the resolver; update the fixture so these
three values differ (e.g., give parentSession.projectDir and
childSession.projectDir distinct paths and have
resolveConversationWorkdir.mockResolvedValue return the expected handoff path)
and assert that SubagentOrchestratorTool uses resolveConversationWorkdir (the
mocked function) rather than copying either session.projectDir; change the
buildSessionInfo calls for parentSession and childSession to set unique
projectDir values and adjust the resolveConversationWorkdir mock return
accordingly.
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
Release Notes
New Features
Documentation