Skip to content

Commit 79c64be

Browse files
arul28claude
andcommitted
Improve path safety and orchestrator; misc fixes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ca433fc commit 79c64be

10 files changed

Lines changed: 206 additions & 17 deletions

File tree

apps/desktop/src/main/services/chat/agentChatService.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1844,6 +1844,7 @@ describe("createAgentChatService", () => {
18441844
model: "gpt-5.4",
18451845
});
18461846

1847+
const threadsBefore = mockState.codexThreadCounter;
18471848
const turnsBefore = mockState.codexTurnCounter;
18481849
const outsidePath = path.join(process.cwd(), `.ade-agent-chat-outside-${Date.now()}.txt`);
18491850
fs.writeFileSync(outsidePath, "secret", "utf8");
@@ -1856,6 +1857,7 @@ describe("createAgentChatService", () => {
18561857
} finally {
18571858
fs.rmSync(outsidePath, { force: true });
18581859
}
1860+
expect(mockState.codexThreadCounter).toBe(threadsBefore);
18591861
expect(mockState.codexTurnCounter).toBe(turnsBefore);
18601862
});
18611863

@@ -1892,7 +1894,12 @@ describe("createAgentChatService", () => {
18921894
});
18931895

18941896
it("logs attachment read failures and keeps the fallback text generic", async () => {
1895-
const { service, logger } = createService();
1897+
const events: AgentChatEventEnvelope[] = [];
1898+
const { service, logger } = createService({
1899+
onEvent: (event: AgentChatEventEnvelope) => {
1900+
events.push(event);
1901+
},
1902+
});
18961903
const attachmentDir = path.join(tmpRoot, "attachment-dir");
18971904
fs.mkdirSync(attachmentDir, { recursive: true });
18981905
vi.mocked(generateText).mockResolvedValue({ text: "Attachment fallback test" } as any);
@@ -1924,10 +1931,21 @@ describe("createAgentChatService", () => {
19241931
: [];
19251932
const messages = rawMessages;
19261933
const currentUserMessageText = JSON.stringify(messages.at(-1)?.content);
1934+
const userMessageEvent = await waitForEvent(
1935+
events,
1936+
(event): event is AgentChatEventEnvelope & {
1937+
event: Extract<AgentChatEventEnvelope["event"], { type: "user_message" }>;
1938+
} => event.event.type === "user_message",
1939+
);
1940+
const rendererPayload = JSON.stringify(userMessageEvent.event);
19271941

19281942
expect(currentUserMessageText).toContain("Attachment unavailable: attachment-dir");
19291943
expect(currentUserMessageText).not.toContain("Path is not a regular file");
19301944
expect(currentUserMessageText).not.toContain("EISDIR");
1945+
expect(rendererPayload).not.toContain("Path is not a regular file");
1946+
expect(rendererPayload).not.toContain("EISDIR");
1947+
expect(rendererPayload).not.toContain(attachmentDir);
1948+
expect(rendererPayload).not.toContain(tmpRoot);
19311949
expect(logger.warn).toHaveBeenCalledWith(
19321950
"agent_chat.streaming_attachment_unavailable",
19331951
expect.objectContaining({
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import fs from "node:fs";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { afterEach, describe, expect, it, vi } from "vitest";
5+
import { createFileService } from "./fileService";
6+
7+
describe("fileService", () => {
8+
afterEach(() => {
9+
vi.restoreAllMocks();
10+
});
11+
12+
it("preserves non-escape filesystem errors while resolving workspace paths", () => {
13+
const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "ade-file-service-"));
14+
const rootReal = fs.realpathSync(rootPath);
15+
const blockedPath = path.join(rootReal, "blocked");
16+
const permissionError = Object.assign(new Error("permission denied"), { code: "EACCES" as const });
17+
const originalLstatSync = fs.lstatSync.bind(fs);
18+
19+
const laneService = {
20+
resolveWorkspaceById: vi.fn(() => ({
21+
id: "workspace-1",
22+
laneId: "lane-1",
23+
rootPath,
24+
})),
25+
getFilesWorkspaces: vi.fn(() => []),
26+
} as any;
27+
28+
const service = createFileService({ laneService });
29+
const spy = vi.spyOn(fs, "lstatSync").mockImplementation(((filePath: fs.PathLike) => {
30+
if (String(filePath) === blockedPath) {
31+
throw permissionError;
32+
}
33+
return originalLstatSync(filePath);
34+
}) as typeof fs.lstatSync);
35+
36+
try {
37+
expect(() =>
38+
service.readFile({
39+
workspaceId: "workspace-1",
40+
path: "blocked/child.txt",
41+
})
42+
).toThrow(permissionError);
43+
} finally {
44+
spy.mockRestore();
45+
fs.rmSync(rootPath, { recursive: true, force: true });
46+
}
47+
});
48+
});

apps/desktop/src/main/services/files/fileService.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,11 @@ function ensureSafePath(
139139
let absPath: string;
140140
try {
141141
absPath = resolvePathWithinRoot(rootPath, joinedPath, { allowMissing: opts.allowMissing });
142-
} catch {
143-
throw new Error("Refusing to access path outside workspace");
142+
} catch (error) {
143+
if (error instanceof Error && error.message === "Path escapes root") {
144+
throw new Error("Refusing to access path outside workspace");
145+
}
146+
throw error;
144147
}
145148
if (containsDotGit(absPath)) {
146149
throw new Error("Refusing to access .git internals");

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3383,6 +3383,90 @@ describe("aiOrchestratorService", () => {
33833383
}
33843384
});
33853385

3386+
it("keeps coordinator availability interventions open when no live coordinator exists", async () => {
3387+
const fixture = await createFixture();
3388+
try {
3389+
const mission = fixture.missionService.create({
3390+
prompt: "Do not clear coordinator blockers without a live coordinator.",
3391+
laneId: fixture.laneId,
3392+
});
3393+
const started = fixture.orchestratorService.startRun({
3394+
missionId: mission.id,
3395+
steps: [],
3396+
});
3397+
const runId = started.run.id;
3398+
fixture.missionService.update({
3399+
missionId: mission.id,
3400+
status: "in_progress",
3401+
});
3402+
fixture.orchestratorService.addSteps({
3403+
runId,
3404+
steps: [
3405+
{
3406+
stepKey: "manual-check",
3407+
title: "Manual check",
3408+
stepIndex: 0,
3409+
dependencyStepKeys: [],
3410+
executorKind: "manual",
3411+
},
3412+
],
3413+
});
3414+
fixture.orchestratorService.tick({ runId });
3415+
const graph = fixture.orchestratorService.getRunGraph({ runId });
3416+
const readyStep = graph.steps.find((step) => step.stepKey === "manual-check" && step.status === "ready");
3417+
if (!readyStep) throw new Error("Expected manual-check step to be ready");
3418+
const attempt = await fixture.orchestratorService.startAttempt({
3419+
runId,
3420+
stepId: readyStep.id,
3421+
ownerId: "test-owner",
3422+
executorKind: "manual",
3423+
});
3424+
await fixture.orchestratorService.completeAttempt({
3425+
attemptId: attempt.id,
3426+
status: "succeeded",
3427+
});
3428+
const intervention = fixture.missionService.addIntervention({
3429+
missionId: mission.id,
3430+
interventionType: "failed_step",
3431+
title: "Coordinator unavailable",
3432+
body: "Coordinator agent is not available for this run.",
3433+
requestedAction: "Resume after coordinator runtime is healthy.",
3434+
metadata: {
3435+
runId,
3436+
stepId: readyStep.id,
3437+
attemptId: attempt.id,
3438+
reasonCode: "coordinator_unavailable",
3439+
},
3440+
});
3441+
3442+
fixture.aiOrchestratorService.onOrchestratorRuntimeEvent({
3443+
type: "orchestrator-step-updated",
3444+
runId,
3445+
stepId: readyStep.id,
3446+
attemptId: attempt.id,
3447+
at: new Date().toISOString(),
3448+
reason: "attempt_completed",
3449+
} as any);
3450+
3451+
const refreshedMission = fixture.missionService.get(mission.id);
3452+
const refreshedIntervention = refreshedMission?.interventions.find((entry) => entry.id === intervention.id);
3453+
expect(refreshedIntervention?.status).toBe("open");
3454+
3455+
const resolvedEvents = fixture.orchestratorService.listRuntimeEvents({
3456+
runId,
3457+
eventTypes: ["intervention_resolved"],
3458+
limit: 10,
3459+
});
3460+
expect(
3461+
resolvedEvents.some((entry) =>
3462+
String((entry.payload as Record<string, unknown> | null)?.interventionId ?? "") === intervention.id
3463+
)
3464+
).toBe(false);
3465+
} finally {
3466+
fixture.dispose();
3467+
}
3468+
});
3469+
33863470
it("prefers mission launch failures over later coordinator-unavailable interventions in run view", async () => {
33873471
const fixture = await createFixture();
33883472
try {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10178,14 +10178,14 @@ Check all worker statuses and continue managing the mission from here. Read work
1017810178
reason: event.reason,
1017910179
recovered: false
1018010180
});
10181+
} else {
10182+
resolveCoordinatorHealthInterventions({
10183+
runId,
10184+
note: "Coordinator runtime is healthy again; closed stale coordinator availability intervention.",
10185+
resolutionReason: "coordinator_recovered",
10186+
});
1018110187
}
1018210188

10183-
resolveCoordinatorHealthInterventions({
10184-
runId,
10185-
note: "Coordinator runtime is healthy again; closed stale coordinator availability intervention.",
10186-
resolutionReason: "coordinator_recovered",
10187-
});
10188-
1018910189
// Run finalized — coordinator's job is done, shut it down
1019010190
if (event.reason === "finalized") {
1019110191
const terminalRunStatus = getEventGraph().run.status;

apps/desktop/src/main/services/orchestrator/coordinatorTools.test.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ function createTestDeps(args: {
1515
text: string;
1616
priority?: "normal" | "urgent";
1717
}) => Promise<CoordinatorWorkerDeliveryStatus>;
18+
projectRoot?: string;
19+
workspaceRoot?: string;
1820
memoryService?: {
1921
search: (opts: Record<string, unknown>) => Promise<any[]>;
2022
searchAcrossScopeOwners?: (opts: Record<string, unknown>) => Promise<any[]>;
@@ -55,15 +57,40 @@ function createTestDeps(args: {
5557
missionId: "mission-1",
5658
logger,
5759
db: {} as any,
58-
projectRoot: "/tmp",
59-
workspaceRoot: "/tmp/worktree",
60+
projectRoot: args.projectRoot ?? "/tmp",
61+
workspaceRoot: args.workspaceRoot ?? "/tmp/worktree",
6062
onDagMutation: vi.fn(),
6163
sendWorkerMessageToSession: args.sendWorkerMessageToSession,
6264
});
6365

6466
return { tools, orchestratorService };
6567
}
6668

69+
describe("coordinator project context", () => {
70+
it("redacts the workspace root from get_project_context", async () => {
71+
const workspaceRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-coordinator-tools-"));
72+
try {
73+
fs.writeFileSync(path.join(workspaceRoot, "README.md"), "# Test workspace\n", "utf8");
74+
const { tools } = createTestDeps({
75+
graph: { run: { id: "run-1", metadata: {} }, steps: [], attempts: [] },
76+
workspaceRoot,
77+
});
78+
79+
const result = await (tools.get_project_context as any).execute({});
80+
81+
expect(result).toMatchObject({
82+
ok: true,
83+
projectRoot: "./",
84+
projectRootRedacted: true,
85+
rootLabel: "./",
86+
});
87+
expect(JSON.stringify(result)).not.toContain(workspaceRoot);
88+
} finally {
89+
fs.rmSync(workspaceRoot, { recursive: true, force: true });
90+
}
91+
});
92+
});
93+
6794
describe("coordinator memory tools", () => {
6895
it("memory_search queries project memory with mission scope defaults", async () => {
6996
const memoryService = {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6299,9 +6299,12 @@ Format: Lead with the concrete rule or fact, then brief context for WHY. One act
62996299
} catch {
63006300
// Ignore
63016301
}
6302+
const workspaceRootLabel = "./";
63026303
return {
63036304
ok: true,
6304-
projectRoot: workspaceRoot,
6305+
projectRoot: workspaceRootLabel,
6306+
projectRootRedacted: true,
6307+
rootLabel: workspaceRootLabel,
63056308
topLevelEntries: topLevel,
63066309
docs,
63076310
};

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,7 @@ describe("resolvePathWithinRoot", () => {
237237
fs.rmSync(outsideDir, { recursive: true, force: true });
238238
}
239239
});
240-
});
241240

242-
describe("resolvePathWithinRoot", () => {
243241
it("allows a normal child path when intermediate segments do not exist yet", () => {
244242
const root = fs.mkdtempSync(path.join(os.tmpdir(), "ade-utils-root-"));
245243
try {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ function ensureDirectoryChainWithinRoot(
276276

277277
const nextPath = path.join(cursor, segment);
278278
try {
279-
const stat = fs.lstatSync(nextPath);
280-
const resolvedPath = stat.isSymbolicLink() ? realpathExisting(nextPath) : realpathExisting(nextPath);
279+
fs.lstatSync(nextPath);
280+
const resolvedPath = realpathExisting(nextPath);
281281
if (!isPathAlignedWithRoot(rootReal, resolvedPath)) {
282282
throw new Error("Path escapes root");
283283
}

apps/desktop/src/renderer/components/cto/pipeline/WorkflowListSidebar.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,24 @@ export function WorkflowListSidebar({
154154
key={workflow.id}
155155
type="button"
156156
onClick={() => onSelectWorkflow(workflow.id)}
157+
aria-current={isSelected ? "true" : undefined}
158+
aria-pressed={workflow.enabled}
157159
className={cn(
158160
"w-full rounded-lg px-3 py-2.5 text-left transition-all duration-200",
159161
isSelected ? "bg-[rgba(167,139,250,0.08)]" : "hover:bg-white/[0.03]",
160162
)}
161163
style={isSelected ? { border: "1px solid rgba(167,139,250,0.15)" } : { border: "1px solid transparent" }}
162164
>
163165
<div className="flex items-center justify-between gap-2">
164-
<div className="truncate text-xs font-medium text-fg">{workflow.name}</div>
166+
<div className="truncate text-xs font-medium text-fg">
167+
{workflow.name}
168+
<span className="sr-only">
169+
{` ${isSelected ? "selected" : "not selected"} ${workflow.enabled ? "enabled" : "disabled"}`}
170+
</span>
171+
</div>
165172
<div
166173
className="h-2 w-2 rounded-full shrink-0"
174+
aria-hidden="true"
167175
style={{ background: workflow.enabled ? "#34D399" : "#6B7280" }}
168176
/>
169177
</div>

0 commit comments

Comments
 (0)