Skip to content

Commit 2801d66

Browse files
edelaunaedelauna
committed
fix(delegation): read task-local mode in getEnvironmentDetails and validateToolUse
1 parent c6eb8fb commit 2801d66

9 files changed

Lines changed: 249 additions & 6 deletions

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"check-types": "turbo check-types --log-order grouped --output-logs new-only",
1414
"test": "turbo test --log-order grouped --output-logs new-only",
1515
"test:mutation-ci": "node --test scripts/stryker-diff.test.mjs",
16-
"lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts",
16+
"lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-delegated-mode-readers.ts",
1717
"cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts",
1818
"parser-scope:model-check": "node scripts/run-native-tool-call-parser-scoping.mjs",
1919
"mcp:integration-check": "tsx scripts/check-mcp-oauth-integration.ts",

‎scripts/check-delegated-mode-readers.ts‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// check-delegated-mode-readers.ts
2+
//
3+
// Refinement check for the delegated-child mode-reader invariant (issue #1623).
4+
//
5+
// check-provider-handoff-scheduler.ts verifies the write side: that
6+
// selectHandoffExecutionContext stores the task-local mode correctly.
7+
// This script verifies the read side: that the mode observable by
8+
// tool-validation readers is the task-local mode, not the shared provider mode.
9+
//
10+
// The VS Code-dependent readers (getEnvironmentDetails,
11+
// presentAssistantMessage) are covered by their vitest regression tests.
12+
// This script covers the pure-TS parts of the invariant chain and proves
13+
// that the two sources of mode are observably different, so any reader
14+
// that uses the wrong source silently produces wrong behavior.
15+
//
16+
// Invariant: for any delegated child task C with taskMode = M,
17+
// toolAllowedForMode(tool, M) ≠ toolAllowedForMode(tool, providerMode)
18+
// whenever M ≠ providerMode and the two modes differ on the tool's group.
19+
20+
import assert from "node:assert/strict"
21+
22+
import { DEFAULT_MODES } from "../packages/types/src/mode"
23+
24+
import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS, TOOL_ALIASES } from "../src/shared/tools"
25+
import { selectHandoffExecutionContext, type TaskExecutionContext } from "../src/core/task/providerHandoff"
26+
27+
// ---------------------------------------------------------------------------
28+
// Minimal inline mode-allows-tool check.
29+
// Avoids importing src/shared/modes.ts, which pulls in VS Code.
30+
// Only covers built-in modes (no custom modes, no file-regex options).
31+
// That is enough to prove the behavioral divergence this check needs.
32+
// ---------------------------------------------------------------------------
33+
34+
type ModeConfig = (typeof DEFAULT_MODES)[number]
35+
type GroupEntry = ModeConfig["groups"][number]
36+
37+
function groupName(entry: GroupEntry): string {
38+
return Array.isArray(entry) ? entry[0] : (entry as string)
39+
}
40+
41+
function toolAllowedForMode(tool: string, modeSlug: string): boolean {
42+
const resolvedTool = (TOOL_ALIASES as Record<string, string>)[tool] ?? tool
43+
if ((ALWAYS_AVAILABLE_TOOLS as readonly string[]).includes(resolvedTool)) return true
44+
const mode = DEFAULT_MODES.find((m) => m.slug === modeSlug)
45+
if (!mode) return false
46+
for (const entry of mode.groups) {
47+
const groupTools = (TOOL_GROUPS as Record<string, { tools: readonly string[] }>)[groupName(entry)]?.tools ?? []
48+
if (groupTools.includes(resolvedTool)) return true
49+
}
50+
return false
51+
}
52+
53+
// ---------------------------------------------------------------------------
54+
// Scenario: parent in "orchestrator" mode delegates child to "code".
55+
// Regression behavior: both readers used providerMode ("orchestrator").
56+
// Correct behavior: readers use taskMode ("code").
57+
//
58+
// orchestrator groups: [] → apply_diff blocked
59+
// code groups: [...edit] → apply_diff allowed
60+
// ---------------------------------------------------------------------------
61+
62+
const parentCtx: TaskExecutionContext = {
63+
mode: "orchestrator",
64+
apiConfigName: undefined,
65+
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 3 },
66+
}
67+
68+
// 1. Handoff stores the task-local mode, not the parent mode.
69+
const childCtx = selectHandoffExecutionContext(parentCtx, "code", parentCtx.mode, false, undefined)
70+
assert.equal(childCtx.mode, "code", "handoff must store the requested task-local mode")
71+
assert.notEqual(childCtx.mode, parentCtx.mode, "test scenario requires divergent provider and task modes")
72+
73+
// 2. The two modes produce observably different tool-validation outcomes.
74+
assert.equal(toolAllowedForMode("apply_diff", "orchestrator"), false, "orchestrator has no edit group")
75+
assert.equal(toolAllowedForMode("apply_diff", "code"), true, "code has the edit group")
76+
77+
// 3. Regression claim: a reader that consumes providerMode rejects apply_diff;
78+
// a reader that consumes taskMode correctly allows it.
79+
const viaProviderMode = toolAllowedForMode("apply_diff", parentCtx.mode) // "orchestrator" — wrong source
80+
const viaTaskMode = toolAllowedForMode("apply_diff", childCtx.mode) // "code" — correct source
81+
assert.equal(viaProviderMode, false, "stale provider mode rejects apply_diff (regression behavior)")
82+
assert.equal(viaTaskMode, true, "task-local mode allows apply_diff (correct behavior)")
83+
84+
// 4. Additional mode pairs that show the same divergence.
85+
const DIVERGENT_PAIRS: Array<{
86+
label: string
87+
providerMode: string
88+
taskMode: string
89+
probe: string
90+
blockedInProvider: boolean
91+
allowedInTask: boolean
92+
}> = [
93+
// orchestrator → code: edit tools blocked at provider level, allowed at task level
94+
{
95+
label: "orchestrator→code apply_diff",
96+
providerMode: "orchestrator",
97+
taskMode: "code",
98+
probe: "apply_diff",
99+
blockedInProvider: true,
100+
allowedInTask: true,
101+
},
102+
// orchestrator → code: command tools blocked at provider level, allowed at task level
103+
{
104+
label: "orchestrator→code execute_command",
105+
providerMode: "orchestrator",
106+
taskMode: "code",
107+
probe: "execute_command",
108+
blockedInProvider: true,
109+
allowedInTask: true,
110+
},
111+
// code → ask: edit tools allowed at provider level, blocked at task level
112+
{
113+
label: "code→ask apply_diff",
114+
providerMode: "code",
115+
taskMode: "ask",
116+
probe: "apply_diff",
117+
blockedInProvider: false,
118+
allowedInTask: false,
119+
},
120+
// ask → code: edit tools blocked at provider level, allowed at task level
121+
{
122+
label: "ask→code write_to_file",
123+
providerMode: "ask",
124+
taskMode: "code",
125+
probe: "write_to_file",
126+
blockedInProvider: true,
127+
allowedInTask: true,
128+
},
129+
]
130+
131+
for (const pair of DIVERGENT_PAIRS) {
132+
const ctx = selectHandoffExecutionContext(
133+
{ ...parentCtx, mode: pair.providerMode },
134+
pair.taskMode,
135+
pair.providerMode,
136+
false,
137+
undefined,
138+
)
139+
assert.equal(ctx.mode, pair.taskMode, `${pair.label}: handoff must store task-local mode`)
140+
assert.equal(
141+
toolAllowedForMode(pair.probe, pair.providerMode),
142+
!pair.blockedInProvider,
143+
`${pair.label}: wrong provider-mode result`,
144+
)
145+
assert.equal(
146+
toolAllowedForMode(pair.probe, pair.taskMode),
147+
pair.allowedInTask,
148+
`${pair.label}: wrong task-mode result`,
149+
)
150+
// The two sources disagree, so using the wrong one is always observable.
151+
assert.notEqual(
152+
toolAllowedForMode(pair.probe, pair.providerMode),
153+
toolAllowedForMode(pair.probe, pair.taskMode),
154+
`${pair.label}: provider and task mode must differ on this probe tool`,
155+
)
156+
}
157+
158+
// 5. For every built-in mode as a delegation target: selectHandoffExecutionContext
159+
// always stores the requested mode, regardless of parent mode.
160+
for (const mode of DEFAULT_MODES) {
161+
const ctx = selectHandoffExecutionContext(parentCtx, mode.slug, parentCtx.mode, false, undefined)
162+
assert.equal(ctx.mode, mode.slug, `handoff must store ${mode.slug}, not parent mode ${parentCtx.mode}`)
163+
}
164+
165+
console.log(
166+
`Delegated mode reader check passed: ` +
167+
`regression scenario verified, ` +
168+
`${DIVERGENT_PAIRS.length} divergent-mode pairs checked, ` +
169+
`${DEFAULT_MODES.length}/${DEFAULT_MODES.length} built-in modes verified`,
170+
)

‎src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
7777
}),
7878
}),
7979
},
80+
getTaskMode: vi.fn().mockResolvedValue("code"),
8081
say: vi.fn().mockResolvedValue(undefined),
8182
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
8283
}

‎src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () =
5757
}),
5858
}),
5959
},
60+
getTaskMode: vi.fn().mockResolvedValue("code"),
6061
say: vi.fn().mockResolvedValue(undefined),
6162
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
6263
}

‎src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ interface MockTask {
7373
say: ReturnType<typeof vi.fn>
7474
ask: ReturnType<typeof vi.fn>
7575
pushToolResultToUserContent: ReturnType<typeof vi.fn>
76+
getTaskMode: ReturnType<typeof vi.fn>
7677
}
7778

7879
describe("presentAssistantMessage - tool usage attribution", () => {
@@ -115,6 +116,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
115116
say: vi.fn().mockResolvedValue(undefined),
116117
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
117118
pushToolResultToUserContent: vi.fn(),
119+
getTaskMode: vi.fn().mockResolvedValue("code"),
118120
}
119121

120122
mockTask.pushToolResultToUserContent = vi
@@ -316,4 +318,42 @@ describe("presentAssistantMessage - tool usage attribution", () => {
316318
expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled()
317319
})
318320
})
321+
322+
describe("mode delegation regression", () => {
323+
// Regression for issue #1623.
324+
// Before the fix, validateToolUse received the shared provider mode instead
325+
// of the task-local mode, so a child delegated to "architect" mode would have
326+
// its tools validated against "orchestrator".
327+
it("passes the task-local mode to validateToolUse, not the provider mode", async () => {
328+
// Provider says "orchestrator"; task was delegated to "architect".
329+
mockTask.providerRef = {
330+
deref: () => ({
331+
getState: vi.fn().mockResolvedValue({
332+
mode: "orchestrator",
333+
customModes: [],
334+
}),
335+
}),
336+
}
337+
mockTask.getTaskMode = vi.fn().mockResolvedValue("architect")
338+
339+
mockTask.assistantMessageContent = [
340+
{
341+
type: "tool_use",
342+
id: "call_delegation",
343+
name: "read_file",
344+
params: { path: "test.ts" },
345+
nativeArgs: { path: "test.ts" },
346+
partial: false,
347+
},
348+
]
349+
350+
await presentAssistantMessage(mockTask as unknown as Task)
351+
352+
// The key assertion: task-local mode "architect" was passed, not "orchestrator".
353+
const calls = vi.mocked(validateToolUse).mock.calls
354+
expect(calls.length).toBeGreaterThan(0)
355+
expect(calls[0][0]).toBe("read_file")
356+
expect(calls[0][1]).toBe("architect")
357+
})
358+
})
319359
})

‎src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
6060
}),
6161
}),
6262
},
63+
getTaskMode: vi.fn().mockResolvedValue("code"),
6364
say: vi.fn().mockResolvedValue(undefined),
6465
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
6566
}

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,10 @@ export async function presentAssistantMessage(cline: Task) {
344344

345345
// Fetch state early so it's available for toolDescription and validation
346346
const state = await cline.providerRef.deref()?.getState()
347-
const { mode, customModes, experiments: stateExperiments, disabledTools } = state ?? {}
347+
const { customModes, experiments: stateExperiments, disabledTools } = state ?? {}
348+
// Read the task-local mode, not the shared provider mode.
349+
// A delegated child task may run in a different mode than its parent.
350+
const taskMode = await cline.getTaskMode()
348351

349352
const toolDescription = (): string => {
350353
switch (block.name) {
@@ -617,7 +620,7 @@ export async function presentAssistantMessage(cline: Task) {
617620

618621
validateToolUse(
619622
block.name as ToolName,
620-
mode ?? defaultModeSlug,
623+
taskMode,
621624
customModes ?? [],
622625
toolRequirements,
623626
block.params,
@@ -924,7 +927,7 @@ export async function presentAssistantMessage(cline: Task) {
924927
}
925928

926929
const result = await customTool.execute(customToolArgs, {
927-
mode: mode ?? defaultModeSlug,
930+
mode: taskMode,
928931
task: cline,
929932
})
930933

‎src/core/environment/__tests__/getEnvironmentDetails.spec.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ describe("getEnvironmentDetails", () => {
117117
deref: vi.fn().mockReturnValue(mockProvider),
118118
[Symbol.toStringTag]: "WeakRef",
119119
} as unknown as WeakRef<ClineProvider>,
120+
getTaskMode: vi.fn().mockResolvedValue("code"),
120121
}
121122

122123
// Mock other dependencies.
@@ -464,4 +465,29 @@ describe("getEnvironmentDetails", () => {
464465
const result = await getEnvironmentDetails(mockCline as Task, true)
465466
expect(result).toContain("File listing unavailable: unexpected string rejection")
466467
})
468+
469+
// Regression for issue #1623.
470+
// Before the fix, the Current Mode block read the shared provider mode.
471+
// A child delegated to "architect" mode would report "orchestrator" instead.
472+
it("uses the task-local mode in the Current Mode block, not the provider mode", async () => {
473+
// Provider mode stays "code"; task was delegated to "architect".
474+
mockState.mode = "code"
475+
;(mockCline.getTaskMode as Mock).mockResolvedValue("architect")
476+
;(getFullModeDetails as Mock).mockResolvedValue({
477+
name: "🏗️ Architect",
478+
roleDefinition: "You design software.",
479+
customInstructions: "",
480+
})
481+
482+
const result = await getEnvironmentDetails(mockCline as Task)
483+
484+
expect(result).toContain("<slug>architect</slug>")
485+
expect(result).not.toContain("<slug>code</slug>")
486+
expect(getFullModeDetails).toHaveBeenCalledWith(
487+
"architect",
488+
[],
489+
undefined,
490+
expect.objectContaining({ cwd: mockCwd }),
491+
)
492+
})
467493
})

‎src/core/environment/getEnvironmentDetails.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,16 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
205205

206206
// Add current mode and any mode-specific warnings.
207207
const {
208-
mode,
209208
customModes,
210209
customModePrompts,
211210
experiments = {} as Record<ExperimentId, boolean>,
212211
customInstructions: globalCustomInstructions,
213212
language,
214213
} = state ?? {}
215214

216-
const currentMode = mode ?? defaultModeSlug
215+
// Read the task-local mode, not the shared provider mode.
216+
// A delegated child task may run in a different mode than its parent.
217+
const currentMode = await cline.getTaskMode()
217218

218219
const modeDetails = await getFullModeDetails(currentMode, customModes, customModePrompts, {
219220
cwd: cline.cwd,

0 commit comments

Comments
 (0)