Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitleaksignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@
# gitleaks' generic-api-key rule fired only because the const identifier ends in
# `KEY` and the namespaced value has mild entropy. Scoped to its original commit.
86e23824a491bc8de59697b62169783f917e63b1:apps/desktop/src/renderer/lib/bannerDismiss.ts:generic-api-key:24

# Synthetic filename used to prove files under `.ade/secrets` cannot be
# imported as proof artifacts. No credential value is present; keep the
# exception scoped to the original PR commit and exact test finding.
631f4066397cd4f98b095d3f7d3a43d0cf758805:apps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.test.ts:generic-api-key:423
7 changes: 7 additions & 0 deletions apps/ade-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,13 @@ ade code
ade code --embedded
ade tests run --lane lane-id --suite unit --wait
ade proof list --arg ownerKind=chat --arg ownerId=session-id
ade proof attach shots/result.png --caption "Checkout complete"
ade proof rm artifact-id
ade proof broken --text # list missing/unimported proof records
ade proof recover artifact-id # re-import when the original capture still exists
ade proof prune # preview broken records; does not delete
ade proof prune --broken # delete every broken proof record
ade proof actions --text # full computer_use_artifacts action inventory
ade ios-sim devices --text
ade --socket ios-sim apps --text
ade --socket ios-sim launch --target target-id --text
Expand Down
66 changes: 50 additions & 16 deletions apps/ade-cli/src/adeRpcServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,27 @@ describe("adeRpcServer", () => {
});
});

// `ade/actions/call` is the only way into `runTool`, and it dispatches off
// READ_ONLY_TOOLS / MUTATION_TOOLS. A tool registered in the inventory but
// absent from both sets is advertised and unreachable — which is how the
// whole proof delete surface (`ade proof rm|prune|recover`) shipped broken.
it("dispatches every registered computer-use mutation tool", async () => {
const { runtime } = createRuntime();
const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
await initialize(handler, { callerId: "chat-1", role: "agent" });

for (const name of [
"delete_computer_use_artifacts",
"prune_broken_computer_use_artifacts",
"recover_computer_use_artifact",
"list_broken_computer_use_artifacts",
]) {
const result = await callTool(handler, name, {});
const serialized = JSON.stringify(result ?? {});
expect(serialized).not.toContain(`Unsupported ADE action: ${name}`);
}
});

it("caps a session-bound CTO caller and scopes lifecycle actions to its own session", async () => {
await withEnv({ ADE_DEFAULT_ROLE: "cto", ADE_CHAT_SESSION_ID: undefined }, async () => {
const { runtime } = createRuntime();
Expand Down Expand Up @@ -1772,26 +1793,39 @@ describe("adeRpcServer", () => {
);
});

it("rejects computer-use manifests outside the project root", async () => {
it("forwards the caller's root so relative capture paths resolve in the agent's lane worktree", async () => {
const fixture = createRuntime();
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });
const outsideManifest = path.join(path.dirname(fixture.runtime.projectRoot), `ade-artifacts-${Date.now()}.json`);
fs.writeFileSync(outsideManifest, JSON.stringify([{ kind: "screenshot", path: "/tmp/shot.png" }]), "utf8");
const laneRoot = path.join(fixture.runtime.projectRoot, ".ade", "worktrees", "lane-a");

try {
await initialize(handler, { callerId: "chat-session-1", role: "agent" });
const response = await callTool(handler, "ingest_computer_use_artifacts", {
backendStyle: "external_cli",
backendName: "agent-browser",
manifestPath: `../${path.basename(outsideManifest)}`,
});
await initialize(handler, { callerId: "chat-session-1", role: "agent" });
await callTool(handler, "ingest_computer_use_artifacts", {
backendStyle: "manual",
backendName: "ade-cli",
callerRoot: laneRoot,
inputs: [{ kind: "screenshot", title: "Lane proof", path: "shots/proof.png" }],
});

expect(response.isError).toBe(true);
expect(JSON.stringify(response.error ?? response.structuredContent ?? {})).toContain("project root");
expect(fixture.runtime.computerUseArtifactBrokerService.ingest).not.toHaveBeenCalled();
} finally {
fs.rmSync(outsideManifest, { force: true });
}
expect(fixture.runtime.computerUseArtifactBrokerService.ingest).toHaveBeenCalledWith(
expect.objectContaining({ callerRoot: laneRoot }),
);
});

it("rejects a relative caller root, which would resolve differently on each side", async () => {
const fixture = createRuntime();
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });

await initialize(handler, { callerId: "chat-session-1", role: "agent" });
const response = await callTool(handler, "ingest_computer_use_artifacts", {
backendStyle: "manual",
backendName: "ade-cli",
callerRoot: "../elsewhere",
inputs: [{ kind: "screenshot", title: "Proof", path: "shots/proof.png" }],
});

expect(response.isError).toBe(true);
expect(JSON.stringify(response.error ?? response.structuredContent ?? {})).toContain("absolute");
expect(fixture.runtime.computerUseArtifactBrokerService.ingest).not.toHaveBeenCalled();
});


Expand Down
122 changes: 88 additions & 34 deletions apps/ade-cli/src/adeRpcServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
getLocalComputerUseCapabilities,
toProjectArtifactUri,
} from "../../desktop/src/main/services/computerUse/localComputerUse";
import { loadAgentBrowserArtifactPayloadFromFile, parseAgentBrowserArtifactPayload } from "../../desktop/src/main/services/proof/agentBrowserArtifactAdapter";
import {
ADE_ACTION_DOMAIN_NAMES,
type AdeActionDomain,
Expand Down Expand Up @@ -544,7 +543,7 @@ const TOOL_SPECS: ToolSpec[] = [
backendName: { type: "string", minLength: 1 },
toolName: { type: "string" },
command: { type: "string" },
manifestPath: { type: "string" },
callerRoot: { type: "string", description: "Absolute directory that relative input paths are resolved against. Defaults to the agent's workspace root." },
inputs: {
type: "array",
items: {
Expand Down Expand Up @@ -620,6 +619,44 @@ const TOOL_SPECS: ToolSpec[] = [
}
}
},
{
name: "delete_computer_use_artifacts",
description: "Delete stored proof artifacts: removes the database records and the stored file. Idempotent.",
inputSchema: {
type: "object",
additionalProperties: false,
properties: {
artifactId: { type: "string", minLength: 1 },
artifactIds: { type: "array", items: { type: "string", minLength: 1 } },
}
}
},
{
name: "list_broken_computer_use_artifacts",
description: "List proof records whose stored file is missing or was never imported, with the path each can be recovered from when one survives.",
inputSchema: {
type: "object",
additionalProperties: false,
properties: {
limit: { type: "number", minimum: 1, maximum: 2000, default: 200 },
}
}
},
{
name: "prune_broken_computer_use_artifacts",
description: "Delete every proof record whose file is missing or was never imported.",
inputSchema: { type: "object", additionalProperties: false, properties: {} }
},
{
name: "recover_computer_use_artifact",
description: "Re-import a broken proof record's original file when it still exists on disk.",
inputSchema: {
type: "object",
additionalProperties: false,
required: ["artifactId"],
properties: { artifactId: { type: "string", minLength: 1 } }
}
},
{
name: "get_computer_use_backend_status",
description: "Describe external-first computer-use backends available to ADE and the local fallback status.",
Expand Down Expand Up @@ -1396,12 +1433,16 @@ const READ_ONLY_TOOLS = new Set([
"getLinearIssueComments",
"get_environment_info",
"list_computer_use_artifacts",
"list_broken_computer_use_artifacts",
"get_computer_use_backend_status",
]);

const MUTATION_TOOLS = new Set([
"saveMemory",
"create_lane",
"delete_computer_use_artifacts",
"prune_broken_computer_use_artifacts",
"recover_computer_use_artifact",
"run_ade_action",
"start_cli_session",
"send_to_session",
Expand Down Expand Up @@ -4347,39 +4388,20 @@ async function runTool(args: {
if (name === "ingest_computer_use_artifacts") {
const backendStyle = assertComputerUseBackendStyle(toolArgs.backendStyle, "backendStyle");
const backendName = assertNonEmptyString(toolArgs.backendName, "backendName");
const manifestPath = asOptionalTrimmedString(toolArgs.manifestPath);
let inputs = Array.isArray(toolArgs.inputs) ? toolArgs.inputs.map((entry) => safeObject(entry)) : [];
if (manifestPath) {
if (path.isAbsolute(manifestPath)) {
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "manifestPath must be relative to the project root");
}
let resolvedManifest: string;
try {
resolvedManifest = resolvePathWithinRoot(runtime.projectRoot, path.resolve(runtime.projectRoot, manifestPath));
} catch {
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "manifestPath must stay within the project root");
}
inputs = loadAgentBrowserArtifactPayloadFromFile(resolvedManifest).map((entry) => ({
...entry,
metadata: {
...(isRecord(entry.metadata) ? entry.metadata : {}),
manifestPath: resolvedManifest,
},
}));
} else if (backendName === "agent-browser" && inputs.length === 1 && isRecord(inputs[0]?.json)) {
const adapted = parseAgentBrowserArtifactPayload(inputs[0].json);
if (adapted.length > 0) {
inputs = adapted.map((entry) => ({
...entry,
metadata: {
...(isRecord(entry.metadata) ? entry.metadata : {}),
adapter: "agent-browser-json",
},
}));
}
}
const inputs = Array.isArray(toolArgs.inputs) ? toolArgs.inputs.map((entry) => safeObject(entry)) : [];
if (inputs.length === 0) {
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "Provide inputs or manifestPath for computer-use ingestion.");
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "Provide inputs for computer-use ingestion.");
}
// Relative `input.path` values come from an agent whose cwd is its lane
// worktree, not the project root. Prefer an explicit callerRoot, then the
// caller's lane worktree, and only then the project root.
const callerRoot = asOptionalTrimmedString(toolArgs.callerRoot)
?? resolveLaneWorktreePath(
runtime,
asOptionalTrimmedString(toolArgs.laneId) ?? resolveChatSessionLaneId(runtime, session),
);
if (callerRoot && !path.isAbsolute(callerRoot)) {
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "callerRoot must be an absolute path");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
const result = runtime.computerUseArtifactBrokerService.ingest({
backend: {
Expand All @@ -4388,6 +4410,7 @@ async function runTool(args: {
toolName: asOptionalTrimmedString(toolArgs.toolName),
command: asOptionalTrimmedString(toolArgs.command),
},
...(callerRoot ? { callerRoot } : {}),
inputs: inputs.map((entry) => ({
kind: asOptionalTrimmedString(entry.kind),
title: asOptionalTrimmedString(entry.title),
Expand Down Expand Up @@ -4416,6 +4439,37 @@ async function runTool(args: {
};
}

if (name === "delete_computer_use_artifacts") {
const ids = [
...(asOptionalTrimmedString(toolArgs.artifactId) ? [asOptionalTrimmedString(toolArgs.artifactId)!] : []),
...(Array.isArray(toolArgs.artifactIds)
? toolArgs.artifactIds.map((entry) => asOptionalTrimmedString(entry)).filter((entry): entry is string => Boolean(entry))
: []),
];
if (!ids.length) {
throw new JsonRpcError(JsonRpcErrorCode.invalidParams, "Provide artifactId or artifactIds to delete.");
}
return runtime.computerUseArtifactBrokerService.deleteArtifacts({ artifactIds: ids });
}

if (name === "list_broken_computer_use_artifacts") {
return {
broken: runtime.computerUseArtifactBrokerService.listBrokenArtifacts({
limit: asNumber(toolArgs.limit, 200),
}),
};
}

if (name === "prune_broken_computer_use_artifacts") {
return runtime.computerUseArtifactBrokerService.pruneBrokenArtifacts();
}

if (name === "recover_computer_use_artifact") {
return runtime.computerUseArtifactBrokerService.recoverArtifact({
artifactId: assertNonEmptyString(toolArgs.artifactId, "artifactId"),
});
}

if (name === "get_computer_use_backend_status") {
return runtime.computerUseArtifactBrokerService.getBackendStatus();
}
Expand Down
5 changes: 5 additions & 0 deletions apps/ade-cli/src/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,11 @@ export async function createAdeRuntime(args: {
captureAnalytics: (input) => {
productAnalyticsService.capture(input);
},
// Removing proof files from Settings must drop their records too,
// otherwise the drawer keeps listing items whose bytes are gone.
purgeProofRecordsUnder: (removedPath) => {
computerUseArtifactBrokerService.purgeArtifactRecordsUnder(removedPath);
},
});
const budgetCapService = createBudgetCapService({
db,
Expand Down
58 changes: 58 additions & 0 deletions apps/ade-cli/src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5922,6 +5922,64 @@ describe("ADE CLI", () => {
});
});

it("resolves a relative proof attach path against the caller's cwd", () => {
// The agent's cwd is its lane worktree; the runtime storing the artifact
// runs at the project root. Resolving here is what stops the runtime from
// having to guess which tree a bare "shots/proof.png" belongs to.
const plan = buildCliPlan(["proof", "attach", "shots/proof.png"]);
expect(plan.kind).toBe("execute");
if (plan.kind !== "execute") throw new Error("Expected proof attach to produce an execute plan");

const args = plan.steps[0]?.params?.arguments as Record<string, unknown>;
expect(args.callerRoot).toBe(process.cwd());
expect((args.inputs as Array<{ path: string }>)[0]?.path).toBe(
path.resolve(process.cwd(), "shots/proof.png"),
);
});

it("maps proof rm and prune --broken to the delete actions", () => {
const rm = buildCliPlan(["proof", "rm", "artifact-1", "artifact-2"]);
expect(rm.kind).toBe("execute");
if (rm.kind !== "execute") throw new Error("Expected proof rm to produce an execute plan");
expect(rm.steps[0]?.params).toMatchObject({
name: "delete_computer_use_artifacts",
arguments: { artifactIds: ["artifact-1", "artifact-2"] },
});

const prune = buildCliPlan(["proof", "prune", "--broken"]);
expect(prune.kind).toBe("execute");
if (prune.kind !== "execute") throw new Error("Expected proof prune --broken to produce an execute plan");
expect(prune.steps[0]?.params).toMatchObject({
name: "prune_broken_computer_use_artifacts",
});

// Bare `prune` only reports; removal has to be asked for explicitly.
const dryRun = buildCliPlan(["proof", "prune"]);
expect(dryRun.kind).toBe("execute");
if (dryRun.kind !== "execute") throw new Error("Expected bare proof prune to produce an execute plan");
expect(dryRun.steps[0]?.params).toMatchObject({
name: "list_broken_computer_use_artifacts",
});
});

it("maps proof broken and recover to artifact repair actions", () => {
const broken = buildCliPlan(["proof", "broken", "--arg", "limit=25"]);
expect(broken.kind).toBe("execute");
if (broken.kind !== "execute") throw new Error("Expected proof broken to produce an execute plan");
expect(broken.steps[0]?.params).toEqual({
name: "list_broken_computer_use_artifacts",
arguments: { limit: 25 },
});

const recover = buildCliPlan(["proof", "recover", "artifact-1"]);
expect(recover.kind).toBe("execute");
if (recover.kind !== "execute") throw new Error("Expected proof recover to produce an execute plan");
expect(recover.steps[0]?.params).toEqual({
name: "recover_computer_use_artifact",
arguments: { artifactId: "artifact-1" },
});
});

it("rejects invalid --role values", () => {
expect(() => parseCliArgs(["--role", "bogus", "lanes", "list"])).toThrow(
/--role must be one of/,
Expand Down
Loading