Skip to content

Commit bcd3638

Browse files
committed
ship: iteration 2 fix CI and review comments
1 parent b060e4b commit bcd3638

5 files changed

Lines changed: 115 additions & 24 deletions

File tree

apps/ade-cli/src/adeRpcServer.test.ts

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,14 @@ async function withEnv<T>(vars: Record<string, string | undefined>, fn: () => Pr
946946
}
947947
}
948948

949+
function createFakePathExecutable(dir: string, name: string): string {
950+
fs.mkdirSync(dir, { recursive: true });
951+
const executablePath = path.join(dir, process.platform === "win32" ? `${name}.cmd` : name);
952+
fs.writeFileSync(executablePath, process.platform === "win32" ? "@echo off\r\n" : "#!/bin/sh\n");
953+
if (process.platform !== "win32") fs.chmodSync(executablePath, 0o755);
954+
return executablePath;
955+
}
956+
949957
describe("adeRpcServer", () => {
950958
it("treats requested privileged roles as external without trusted env identity", async () => {
951959
const { runtime } = createRuntime();
@@ -1805,15 +1813,19 @@ describe("adeRpcServer", () => {
18051813

18061814
it("routes spawn_agent to lane-scoped tracked pty sessions", async () => {
18071815
const fixture = createRuntime();
1816+
const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-cli-spawn-bin-"));
1817+
const claudePath = createFakePathExecutable(binDir, "claude");
18081818
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });
18091819

1810-
await initialize(handler, { role: "orchestrator" });
1811-
const response = await callTool(handler, "spawn_agent", {
1812-
laneId: "lane-1",
1813-
provider: "claude",
1814-
model: "claude-sonnet-4-6",
1815-
prompt: "Implement API wiring",
1816-
title: "Orchestrator Spawn"
1820+
const response = await withEnv({ PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}` }, async () => {
1821+
await initialize(handler, { role: "orchestrator" });
1822+
return await callTool(handler, "spawn_agent", {
1823+
laneId: "lane-1",
1824+
provider: "claude",
1825+
model: "claude-sonnet-4-6",
1826+
prompt: "Implement API wiring",
1827+
title: "Orchestrator Spawn"
1828+
});
18171829
});
18181830

18191831
expect(response?.isError).toBeUndefined();
@@ -1824,7 +1836,7 @@ describe("adeRpcServer", () => {
18241836
rows: 36,
18251837
tracked: true,
18261838
toolType: "claude-orchestrated",
1827-
command: "claude",
1839+
command: claudePath,
18281840
args: expect.arrayContaining(["--model", "claude-sonnet-4-6", "--permission-mode", "default", "Implement API wiring"]),
18291841
env: expect.objectContaining({
18301842
ADE_DEFAULT_ROLE: "agent",
@@ -1841,17 +1853,21 @@ describe("adeRpcServer", () => {
18411853
it("starts spawn_agent without writing an attached ADE server config", async () => {
18421854
const fixture = createRuntime();
18431855
fixture.runtime.workspaceRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-cli-spawn-workspace-"));
1856+
const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-cli-spawn-bin-"));
1857+
const claudePath = createFakePathExecutable(binDir, "claude");
18441858
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });
18451859

1846-
await initialize(handler, { role: "orchestrator", runId: "run-from-identity" });
1847-
const response = await callTool(handler, "spawn_agent", {
1848-
laneId: "lane-1",
1849-
provider: "claude",
1850-
model: "claude-sonnet-4-6",
1851-
prompt: "Implement API wiring",
1852-
title: "Orchestrator Spawn",
1853-
runId: "run-1",
1854-
attemptId: "attempt-workspace-roots"
1860+
const response = await withEnv({ PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}` }, async () => {
1861+
await initialize(handler, { role: "orchestrator", runId: "run-from-identity" });
1862+
return await callTool(handler, "spawn_agent", {
1863+
laneId: "lane-1",
1864+
provider: "claude",
1865+
model: "claude-sonnet-4-6",
1866+
prompt: "Implement API wiring",
1867+
title: "Orchestrator Spawn",
1868+
runId: "run-1",
1869+
attemptId: "attempt-workspace-roots"
1870+
});
18551871
});
18561872

18571873
expect(response?.isError).toBeUndefined();
@@ -1860,7 +1876,7 @@ describe("adeRpcServer", () => {
18601876
expect(response.structuredContent.startupCommand).toContain("ADE_ATTEMPT_ID=attempt-workspace-roots");
18611877
expect(fixture.runtime.ptyService.create).toHaveBeenCalledWith(
18621878
expect.objectContaining({
1863-
command: "claude",
1879+
command: claudePath,
18641880
env: expect.objectContaining({
18651881
ADE_RUN_ID: "run-1",
18661882
ADE_ATTEMPT_ID: "attempt-workspace-roots",
@@ -1870,6 +1886,32 @@ describe("adeRpcServer", () => {
18701886
);
18711887
});
18721888

1889+
it("keeps spawn_agent on shell startup when the provider executable cannot be resolved", async () => {
1890+
const fixture = createRuntime();
1891+
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });
1892+
1893+
const response = await withEnv({ PATH: fs.mkdtempSync(path.join(os.tmpdir(), "ade-cli-empty-path-")) }, async () => {
1894+
await initialize(handler, { role: "orchestrator" });
1895+
return await callTool(handler, "spawn_agent", {
1896+
laneId: "lane-1",
1897+
provider: "claude",
1898+
prompt: "Implement API wiring",
1899+
});
1900+
});
1901+
1902+
expect(response?.isError).toBeUndefined();
1903+
expect(fixture.runtime.ptyService.create).toHaveBeenCalledWith(
1904+
expect.not.objectContaining({
1905+
command: expect.any(String),
1906+
})
1907+
);
1908+
expect(fixture.runtime.ptyService.create).toHaveBeenCalledWith(
1909+
expect.objectContaining({
1910+
startupCommand: expect.stringContaining("claude"),
1911+
})
1912+
);
1913+
});
1914+
18731915
it("rejects config-toml permission mode for Claude spawn_agent sessions", async () => {
18741916
const fixture = createRuntime();
18751917
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });

apps/ade-cli/src/adeRpcServer.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,26 @@ const DEFAULT_PTY_ROWS = 36;
8383

8484
const RESOURCE_MIME_JSON = "application/json";
8585

86+
function resolveExecutableOnPath(command: string, env: NodeJS.ProcessEnv = process.env): string | null {
87+
const trimmed = command.trim();
88+
if (!trimmed) return null;
89+
const lookup = process.platform === "win32"
90+
? { command: "where.exe", args: [trimmed] }
91+
: { command: env.SHELL?.trim() || "/bin/sh", args: ["-lc", `command -v ${shellEscapeArg(trimmed)}`] };
92+
const result = spawnSync(lookup.command, lookup.args, {
93+
encoding: "utf8",
94+
env,
95+
stdio: ["ignore", "pipe", "ignore"],
96+
});
97+
if (result.status !== 0 || typeof result.stdout !== "string") return null;
98+
const first = result.stdout
99+
.split(/\r?\n/)
100+
.map((line) => line.trim())
101+
.find(Boolean);
102+
if (!first) return null;
103+
return path.isAbsolute(first) ? first : null;
104+
}
105+
86106
const TOOL_SPECS: ToolSpec[] = [
87107
{
88108
name: "spawn_agent",
@@ -5943,6 +5963,7 @@ async function runTool(args: {
59435963
const startupCommand = envPrefixParts.length > 0
59445964
? `${envPrefixParts.join(" ")} ${commandPreviewParts.join(" ")}`
59455965
: commandPreviewParts.join(" ");
5966+
const providerExecutable = resolveExecutableOnPath(provider);
59465967

59475968
const created = await runtime.ptyService.create({
59485969
laneId,
@@ -5951,8 +5972,7 @@ async function runTool(args: {
59515972
title,
59525973
tracked: true,
59535974
toolType: `${provider}-orchestrated`,
5954-
command: provider,
5955-
args: commandArgs,
5975+
...(providerExecutable ? { command: providerExecutable, args: commandArgs } : {}),
59565976
env: workerEnv,
59575977
startupCommand
59585978
});

apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4265,6 +4265,7 @@ Check all worker statuses and continue managing the mission from here. Read work
42654265
skippedBlockedRuns += 1;
42664266
continue;
42674267
}
4268+
let ownsHealthSweepRun = false;
42684269
if (activeHealthSweepRuns.has(run.id)) {
42694270
// Interval/startup sweeps are opportunistic; manual/chat/status sweeps should wait briefly
42704271
// so explicit health checks don't get dropped due to an in-flight background sweep.
@@ -4273,9 +4274,17 @@ Check all worker statuses and continue managing the mission from here. Read work
42734274
while (activeHealthSweepRuns.has(run.id) && Date.now() < deadline) {
42744275
await new Promise((resolve) => setTimeout(resolve, 25));
42754276
}
4276-
if (activeHealthSweepRuns.has(run.id)) continue;
4277+
if (activeHealthSweepRuns.has(run.id)) {
4278+
logger.debug("ai_orchestrator.health_sweep_explicit_overlap", {
4279+
runId: run.id,
4280+
reason
4281+
});
4282+
}
4283+
}
4284+
if (!activeHealthSweepRuns.has(run.id)) {
4285+
activeHealthSweepRuns.add(run.id);
4286+
ownsHealthSweepRun = true;
42774287
}
4278-
activeHealthSweepRuns.add(run.id);
42794288
try {
42804289
if (disposed) break;
42814290
sweeps += 1;
@@ -4589,7 +4598,7 @@ Check all worker statuses and continue managing the mission from here. Read work
45894598
error: error instanceof Error ? error.message : String(error)
45904599
});
45914600
} finally {
4592-
activeHealthSweepRuns.delete(run.id);
4601+
if (ownsHealthSweepRun) activeHealthSweepRuns.delete(run.id);
45934602
}
45944603
}
45954604

apps/desktop/src/main/services/shared/processExecution.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@ describe("processExecution", () => {
3131

3232
it("quotes cmd arguments consistently", () => {
3333
expect(quoteWindowsCmdArg("C:\\Program Files\\tool.cmd")).toBe('"C:\\Program Files\\tool.cmd"');
34+
expect(quoteWindowsCmdArg("C:\\Program Files\\")).toBe('"C:\\Program Files\\\\"');
3435
expect(quoteWindowsCmdArg("100% done")).toBe('"100%% done"');
3536
expect(quoteWindowsCmdArg('say "hi"')).toBe('"say ""hi"""');
37+
expect(quoteWindowsCmdArg('C:\\path\\"quoted"')).toBe('"C:\\path\\\\\"\"quoted\"\"\"');
3638
});
3739

3840
it("wraps Windows shim invocations with ComSpec", () => {

apps/desktop/src/main/services/shared/processExecution.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,25 @@ export function processOutputToString(value: Buffer | string | null | undefined)
2020
}
2121

2222
export function quoteWindowsCmdArg(value: string): string {
23-
return `"${value.replace(/"/g, "\"\"").replace(/%/g, "%%")}"`;
23+
let quoted = "\"";
24+
let backslashes = 0;
25+
for (const char of value.replace(/%/g, "%%")) {
26+
if (char === "\\") {
27+
backslashes += 1;
28+
continue;
29+
}
30+
if (char === "\"") {
31+
quoted += "\\".repeat(backslashes * 2);
32+
quoted += "\"\"";
33+
} else {
34+
quoted += "\\".repeat(backslashes);
35+
quoted += char;
36+
}
37+
backslashes = 0;
38+
}
39+
quoted += "\\".repeat(backslashes * 2);
40+
quoted += "\"";
41+
return quoted;
2442
}
2543

2644
export function shouldUseWindowsCmdWrapper(command: string, platform: NodeJS.Platform = process.platform): boolean {

0 commit comments

Comments
 (0)