Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 20 additions & 9 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import {
validateLaunchProfilePermissionMode,
type TrackedCliLaunchCommand,
} from "../../../../desktop/src/shared/cliLaunch";
import { parseDeeplink, type ParseError } from "../../../../desktop/src/shared/deeplinks";
import { normalizePrCreationStrategy } from "../../../../desktop/src/shared/prStrategy";
import type { createAgentChatService } from "../../../../desktop/src/main/services/chat/agentChatService";
import type { createCtoStateService } from "../../../../desktop/src/main/services/cto/ctoStateService";
Expand Down Expand Up @@ -234,6 +235,21 @@ function asTrimmedString(value: unknown): string | null {
return typeof value === "string" && value.trim().length > 0 ? value.trim() : null;
}

function formatDeeplinkParseError(error: ParseError): string {
switch (error.kind) {
case "malformed":
return error.reason;
case "unsupported_scheme":
return `unsupported scheme '${error.scheme}'`;
case "unsupported_host":
return `unsupported host '${error.host}'`;
case "unknown_type":
return `unknown type '${error.type}'`;
case "empty":
return error.kind;
}
}

function asOptionalBoolean(value: unknown): boolean | undefined {
return typeof value === "boolean" ? value : undefined;
}
Expand Down Expand Up @@ -2576,22 +2592,17 @@ export function createSyncRemoteCommandService(args: SyncRemoteCommandServiceArg
if (!url) {
throw new Error("deeplinks.open requires a url.");
}
let parsed: URL;
try {
parsed = new URL(url);
} catch {
throw new Error("deeplinks.open requires a valid URL.");
}
if (parsed.protocol !== "ade:") {
throw new Error("deeplinks.open only supports ade:// URLs.");
const parsedDeeplink = parseDeeplink(url);
if (!parsedDeeplink.ok) {
throw new Error(`Invalid deeplink: ${formatDeeplinkParseError(parsedDeeplink.error)}`);
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
if (!args.dispatchDeeplinkUrl) {
return {
ok: false,
message: "Desktop navigation is unavailable in this runtime.",
};
}
return await args.dispatchDeeplinkUrl(parsed.toString());
return await args.dispatchDeeplinkUrl(url);
});
register("prs.getDetail", { viewerAllowed: true }, async (payload) => args.prService.getDetail(requirePrId(payload, "prs.getDetail")));
register("prs.getStatus", { viewerAllowed: true }, async (payload) => args.prService.getStatus(requirePrId(payload, "prs.getStatus")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,122 @@ describe("local runtime connection pool", () => {
expect(pool.getStatus().connectionState).toBe("idle");
});

it("clears stale client and project state when an app-owned runtime exits", async () => {
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-runtime-exit-"));
const cliPath = path.join(tempDir, "cli.cjs");
const socketPath = path.join(tempDir, "ade.sock");
const originalAdeCliJs = process.env.ADE_CLI_JS;
fs.writeFileSync(cliPath, "setTimeout(() => process.exit(0), 50);\n", "utf8");
process.env.ADE_CLI_JS = cliPath;

const pool = new LocalRuntimeConnectionPool("1.2.3", {
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
} as never, { disableSync: true });
const rootPath = path.resolve("/repo");
const client = { close: vi.fn() };
let child: ChildProcess | null = null;

try {
child = (pool as unknown as {
spawnRuntime: (path: string) => ChildProcess;
}).spawnRuntime(socketPath);
(pool as unknown as { connection: Promise<unknown>; activeClient: unknown }).connection = Promise.resolve({
client,
child,
socketPath,
});
(pool as unknown as { activeClient: unknown }).activeClient = client;
(pool as unknown as { projectsByRoot: Map<string, unknown> }).projectsByRoot.set(rootPath, {
projectId: "project-1",
rootPath,
displayName: "repo",
addedAt: 1,
lastOpenedAt: 1,
gitOriginUrl: null,
});

expect(pool.getStatus().connectionState).toBe("connected");

await new Promise<void>((resolve, reject) => {
child?.once("exit", () => resolve());
child?.once("error", reject);
});

expect(pool.getStatus().connectionState).toBe("idle");
expect((pool as unknown as { projectsByRoot: Map<string, unknown> }).projectsByRoot.size).toBe(0);
expect(client.close).toHaveBeenCalledTimes(1);
} finally {
if (child && !child.killed) child.kill();
if (originalAdeCliJs === undefined) delete process.env.ADE_CLI_JS;
else process.env.ADE_CLI_JS = originalAdeCliJs;
removeTempDir(tempDir);
}
});

it("does not clear a replacement connection when a superseded owned runtime exits", async () => {
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-runtime-superseded-"));
const oldCliPath = path.join(tempDir, "old-cli.cjs");
const replacementCliPath = path.join(tempDir, "replacement-cli.cjs");
const socketPath = path.join(tempDir, "ade.sock");
const replacementSocketPath = path.join(tempDir, "ade-replacement.sock");
const originalAdeCliJs = process.env.ADE_CLI_JS;
fs.writeFileSync(oldCliPath, "setTimeout(() => process.exit(0), 100);\n", "utf8");
fs.writeFileSync(replacementCliPath, "setInterval(() => {}, 1000);\n", "utf8");

const pool = new LocalRuntimeConnectionPool("1.2.3", {
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
} as never, { disableSync: true });
const rootPath = path.resolve("/repo");
const replacementClient = { close: vi.fn() };
let oldChild: ChildProcess | null = null;
let replacementChild: ChildProcess | null = null;

try {
const spawnRuntime = (pool as unknown as {
spawnRuntime: (path: string) => ChildProcess;
}).spawnRuntime.bind(pool);
process.env.ADE_CLI_JS = oldCliPath;
oldChild = spawnRuntime(socketPath);
process.env.ADE_CLI_JS = replacementCliPath;
replacementChild = spawnRuntime(replacementSocketPath);
(pool as unknown as { connection: Promise<unknown>; activeClient: unknown }).connection = Promise.resolve({
client: replacementClient,
child: replacementChild,
socketPath: replacementSocketPath,
});
(pool as unknown as { activeClient: unknown }).activeClient = replacementClient;
(pool as unknown as { projectsByRoot: Map<string, unknown> }).projectsByRoot.set(rootPath, {
projectId: "project-1",
rootPath,
displayName: "repo",
addedAt: 1,
lastOpenedAt: 1,
gitOriginUrl: null,
});

await new Promise<void>((resolve, reject) => {
oldChild?.once("exit", () => resolve());
oldChild?.once("error", reject);
});

expect(pool.getStatus().connectionState).toBe("connected");
expect((pool as unknown as { projectsByRoot: Map<string, unknown> }).projectsByRoot.size).toBe(1);
expect(replacementClient.close).not.toHaveBeenCalled();
} finally {
if (oldChild && !oldChild.killed) oldChild.kill();
if (replacementChild && !replacementChild.killed) replacementChild.kill();
if (originalAdeCliJs === undefined) delete process.env.ADE_CLI_JS;
else process.env.ADE_CLI_JS = originalAdeCliJs;
removeTempDir(tempDir);
}
});

it("normalizes local action registry entries from runtime action names", async () => {
const pool = new LocalRuntimeConnectionPool("1.2.3", {
debug: vi.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ function serviceHealthState(
export class LocalRuntimeConnectionPool {
private connection: Promise<LocalRuntimeConnection> | null = null;
private activeClient: RuntimeRpcClient | null = null;
private ownedRuntimeChild: ChildProcess | null = null;
private readonly projectsByRoot = new Map<string, RemoteRuntimeProjectRecord>();
private serviceInstallStatus: LocalRuntimeStatus["serviceInstall"] = {
state: "not_attempted",
Expand Down Expand Up @@ -792,6 +793,7 @@ export class LocalRuntimeConnectionPool {
const pending = this.connection;
this.connection = null;
this.activeClient = null;
this.ownedRuntimeChild = null;
this.projectsByRoot.clear();
void pending?.then((entry) => {
try { entry.client.close(); } catch {}
Expand Down Expand Up @@ -828,6 +830,7 @@ export class LocalRuntimeConnectionPool {
private async tryConnect(socketPath: string): Promise<LocalRuntimeConnection | null> {
try {
const client = await this.connectClient(socketPath);
this.ownedRuntimeChild = null;
return { client, child: null, socketPath };
} catch (error) {
if (error instanceof LocalRuntimeCompatibilityError) {
Expand Down Expand Up @@ -868,6 +871,7 @@ export class LocalRuntimeConnectionPool {
});
try {
const client = await this.connectClient(socketPath);
this.ownedRuntimeChild = null;
return { client, child: null, socketPath };
} catch (error) {
if (error instanceof LocalRuntimeCompatibilityError) {
Expand Down Expand Up @@ -975,6 +979,7 @@ export class LocalRuntimeConnectionPool {
stdio: ["ignore", "pipe", "pipe"],
detached: false,
});
this.ownedRuntimeChild = child;
const outputBase = {
logger: this.logger,
socketPath,
Expand All @@ -991,15 +996,24 @@ export class LocalRuntimeConnectionPool {
child.once("close", () => {
flushOutput();
});
const clearCurrentChildState = (): void => {
if (this.ownedRuntimeChild !== child) return;
const client = this.activeClient;
this.ownedRuntimeChild = null;
this.connection = null;
this.activeClient = null;
this.projectsByRoot.clear();
if (client) closeRuntimeClient(client);
};
Comment thread
greptile-apps[bot] marked this conversation as resolved.
child.once("exit", (code, signal) => {
flushOutput();
this.logger.warn("local_runtime.exited", { code, signal, pid: outputBase.pid, socketPath });
this.connection = null;
clearCurrentChildState();
});
child.once("error", (error) => {
flushOutput();
this.logger.warn("local_runtime.spawn_failed", { error: error.message, pid: outputBase.pid, socketPath });
this.connection = null;
clearCurrentChildState();
});
return child;
}
Expand Down
25 changes: 25 additions & 0 deletions apps/desktop/src/main/services/prs/prService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,31 @@ describe("prService.createLaneFromPrBranch", () => {
expect(laneService.importBranch).not.toHaveBeenCalled();
});

it("blocks when the deeplink repo does not match the active project origin", async () => {
const githubService = makeBranchPrGithubService({
getRepoOrThrow: vi.fn(async () => REPO),
apiRequest: vi.fn(),
});
const laneService = {
...makeLaneService([primaryLane]),
importBranch: vi.fn(),
} as any;
const db = makeMockDb();
installPullRequestRowStore(db);
const { service } = buildService({ db, githubService, laneService });

const result = await serviceWithPrBranchActions(service).preflightCreateLaneFromPrBranch({
repoOwner: "other-owner",
repoName: "other-repo",
githubPrNumber: 404,
});

expect(preflightDisposition(result.preflight)).toBe("blocked");
expect(result.preflight.blockingConflict?.code).toBe("project_repo_mismatch");
expect(githubService.apiRequest).not.toHaveBeenCalled();
expect(laneService.importBranch).not.toHaveBeenCalled();
});

it("blocks when another ADE lane already owns the PR head branch", async () => {
const branchOwner = makeFakeLane({
id: "lane-branch-owner",
Expand Down
41 changes: 41 additions & 0 deletions apps/desktop/src/main/services/prs/prService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2209,6 +2209,47 @@ export function createPrService({
args: CreateLaneFromPrBranchArgs,
): Promise<CreateLaneFromPrBranchPreflight> => {
const { repo, prNumber } = await resolveCreateLaneFromPrBranchLocator(args);
let projectRepo: GitHubRepoRef | null = null;
try {
projectRepo = await githubService.getRepoOrThrow();
} catch {
// Projects without a detectable GitHub origin keep the legacy preflight path;
// remote branch checks below still prevent importing a branch that origin lacks.
projectRepo = null;
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
const finishEarly = (
block: CreateLaneFromPrBranchBlock,
): CreateLaneFromPrBranchPreflight => ({
repoOwner: repo.owner,
repoName: repo.name,
githubPrNumber: prNumber,
githubUrl: `https://github.com/${repo.owner}/${repo.name}/pull/${prNumber}`,
title: `PR #${prNumber}`,
headBranch: null,
headSha: null,
headRepoOwner: null,
headRepoName: null,
remoteBranch: null,
importBranchRef: null,
targetLaneName: `PR #${prNumber}`,
baseBranch: null,
canCreate: false,
status: "blocked",
blockingConflict: block,
blockingConflicts: [block],
});
if (
projectRepo
&& (
projectRepo.owner.toLowerCase() !== repo.owner.toLowerCase()
|| projectRepo.name.toLowerCase() !== repo.name.toLowerCase()
)
) {
return finishEarly(createLaneFromPrBranchBlock(
"project_repo_mismatch",
`This project's GitHub remote is ${projectRepo.owner}/${projectRepo.name}, but the request targets ${repo.owner}/${repo.name}. Open the matching project before creating a lane from this PR.`,
));
}
const pr = await fetchPr(repo, prNumber);
const githubPrNumber = Number(pr?.number) || prNumber;
const title = asString(pr?.title) || `PR #${githubPrNumber}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ const IOS_REMOTE_COMMAND_ACTIONS = [
"prs.pipelineSettings.delete",
"prs.pathToMerge.start",
"prs.pathToMerge.stop",
"deeplinks.open",
] satisfies SyncRemoteCommandAction[];

const IOS_FILE_REQUEST_ACTIONS = [
Expand Down Expand Up @@ -1162,6 +1163,60 @@ describe("createSyncRemoteCommandService", () => {
});
});

// ---------------------------------------------------------------
// execute: deeplink commands
// ---------------------------------------------------------------

describe("execute — deeplink commands", () => {
function createServiceWithDeeplinkDispatch(
dispatchDeeplinkUrl?: (url: string) => Promise<{ ok: boolean; message?: string }>,
): ReturnType<typeof createSyncRemoteCommandService> {
return createSyncRemoteCommandService({
laneService,
prService,
issueInventoryService,
queueLandingService,
ptyService,
sessionService,
fileService,
gitService,
diffService,
agentChatService,
workerAgentService,
conflictService,
processService,
logger: createLogger() as any,
dispatchDeeplinkUrl,
});
}

it("deeplinks.open accepts https://ade.app/open URLs and dispatches the original URL", async () => {
const dispatchDeeplinkUrl = vi.fn(async () => ({ ok: true }));
const withDispatch = createServiceWithDeeplinkDispatch(dispatchDeeplinkUrl);
const url = "https://ade.app/open?type=pr&repo=arul28/ADE&number=383";

const result = await withDispatch.execute(makePayload("deeplinks.open", { url }));

expect(dispatchDeeplinkUrl).toHaveBeenCalledWith(url);
expect(result).toEqual({ ok: true });
});

it("deeplinks.open rejects unsupported URLs before dispatching", async () => {
const dispatchDeeplinkUrl = vi.fn(async () => ({ ok: true }));
const withDispatch = createServiceWithDeeplinkDispatch(dispatchDeeplinkUrl);

await expect(withDispatch.execute(makePayload("deeplinks.open", {
url: "https://example.com/open?type=pr&repo=arul28/ADE&number=383",
}))).rejects.toThrow("Invalid deeplink: unsupported host 'example.com'");

await expect(withDispatch.execute(makePayload("deeplinks.open", {
url: "ftp://ade.app/open?type=pr&repo=arul28/ADE&number=383",
}))).rejects.toThrow("Invalid deeplink: unsupported scheme 'ftp'");

expect(dispatchDeeplinkUrl).not.toHaveBeenCalled();
});
});

// ---------------------------------------------------------------
// execute: git commands
// ---------------------------------------------------------------
Expand Down
Loading
Loading