дзеркало
https://github.com/l0ng-ai/openidea.git
синхронізовано 2026-11-09 11:15:40 +02:00
fix(deployment): filter servers by selected path coverage (#544)
The server choosers listed every configured server without checking mapping coverage or exclusions, so choosing the wrong one silently skipped the files. - Add a shared eligibility check, available through desktop IPC and host RPC, that never contacts a server. - Filter Upload to, Download from, Compare and Delete choosers by it. A selection lists a server if it covers at least one path, and the rest are reported as skipped. - Explain why the default server cannot take a selection, by file name, instead of doing nothing. Apply the same exclusion rules when a transfer runs. Fixes #538
Цей коміт міститься в:
1 джерело
999eb86408
коміт
ee2585e7e9
14 files changed
+333
-38
No files matched your search
@@ -18,7 +18,8 @@ function openSync(entries: SyncEntry[]) {
|
||||
sync: {
|
||||
server: "s1",
|
||||
serverName: "Staging",
|
||||
path: "/work/app",
|
||||
paths: ["/work/app"],
|
||||
skipped: [],
|
||||
request: null,
|
||||
entries,
|
||||
error: null,
|
||||
|
||||
@@ -78,7 +78,7 @@ function SyncDialog() {
|
||||
const shown = entries?.filter((e) => !sync.hideSame || e.status !== "same") ?? [];
|
||||
const steps = syncSteps(sync.choices);
|
||||
const same = entries?.filter((e) => e.status === "same").length ?? 0;
|
||||
const where = root ? relativePath(root, sync.path) || "/" : sync.path;
|
||||
const where = sync.paths.map((path) => (root ? relativePath(root, path) || "/" : path)).join(", ");
|
||||
|
||||
return (
|
||||
<Dialog
|
||||
@@ -110,6 +110,12 @@ function SyncDialog() {
|
||||
<span className={styles.mono}>{where}</span>
|
||||
{entries && <> · {txt(`${steps.length} of ${entries.length} files will change`)}</>}
|
||||
</div>
|
||||
{!!sync.skipped?.length && (
|
||||
<div className={styles.note}>
|
||||
{txt(`Skipped: ${sync.skipped.length}`)}:{" "}
|
||||
{sync.skipped.map((s) => (root ? relativePath(root, s.path) : s.path)).join(", ")}
|
||||
</div>
|
||||
)}
|
||||
{sync.error && <div className={styles.error}>{sync.error}</div>}
|
||||
{!entries && !sync.error && (
|
||||
<div className={styles.center}>
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { DeploymentServerValid } from "@/platform/remoteSettings";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
actions: [] as { id: string; run: (context: { arg: string[] }) => unknown }[],
|
||||
servers: [] as DeploymentServerValid[],
|
||||
coverage: [] as { server: string; paths: string[]; skipped: { path: string; message: string }[] }[],
|
||||
choose: vi.fn(),
|
||||
notify: vi.fn(),
|
||||
upload: vi.fn(),
|
||||
compare: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/platform/actions", () => ({
|
||||
executeAction: vi.fn(),
|
||||
registerActions: (actions: typeof mocks.actions) => {
|
||||
mocks.actions = actions;
|
||||
return () => {};
|
||||
},
|
||||
}));
|
||||
vi.mock("@/workbench/messages", () => ({ confirm: vi.fn() }));
|
||||
vi.mock("@/platform/openUrl", () => ({ openExternalUrl: vi.fn() }));
|
||||
vi.mock("@/platform/context", () => ({ getContext: () => ({}) }));
|
||||
vi.mock("@/platform/project", () => ({
|
||||
relativePath: (_: string, path: string) => path,
|
||||
useProject: { getState: () => ({ project: { root: "/project" } }) },
|
||||
}));
|
||||
vi.mock("@/platform/workflowSettings", () => ({
|
||||
workflowSettings: () => ({ deployment: { servers: mocks.servers, defaultServer: "one" } }),
|
||||
}));
|
||||
vi.mock("@/workbench/contributions", () => ({
|
||||
notifications: () => ({ notify: mocks.notify }),
|
||||
projectViewSelection: () => [],
|
||||
toolWindowService: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/workbench/inputDialogs", () => ({ chooseItem: mocks.choose }));
|
||||
vi.mock("./ssh", () => ({ withSsh: (_: unknown, run: () => unknown) => run() }));
|
||||
vi.mock("@/platform/ipc", () => ({
|
||||
call: (value: unknown) => value,
|
||||
nextRequestId: () => 1,
|
||||
isCancelled: () => false,
|
||||
commands: {
|
||||
deployCoverage: async () => mocks.coverage,
|
||||
deployUpload: mocks.upload,
|
||||
deployCompare: mocks.compare,
|
||||
},
|
||||
}));
|
||||
import { registerDeploymentActions } from "./actions";
|
||||
import { useDeployment } from "./store";
|
||||
|
||||
async function run(id: string, paths = ["/project/a", "/project/b"]) {
|
||||
mocks.actions.find((action) => action.id === id)!.run({ arg: paths });
|
||||
await vi.waitFor(() =>
|
||||
expect(
|
||||
mocks.choose.mock.calls.length + mocks.notify.mock.calls.length + (useDeployment.getState().sync ? 1 : 0),
|
||||
).toBeGreaterThan(0),
|
||||
);
|
||||
}
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.servers = ["one", "two"].map((id) => ({ id, name: id, kind: "local" }) as DeploymentServerValid);
|
||||
mocks.coverage = [
|
||||
{
|
||||
server: "one",
|
||||
paths: ["/project/a"],
|
||||
skipped: [{ path: "/project/b", message: "Not covered by this server's mappings" }],
|
||||
},
|
||||
{ server: "two", paths: [], skipped: [] },
|
||||
];
|
||||
mocks.choose.mockResolvedValue(null);
|
||||
mocks.upload.mockResolvedValue({ files: 1, skipped: 1, errors: [] });
|
||||
mocks.compare.mockResolvedValue([]);
|
||||
useDeployment.setState({ sync: null });
|
||||
registerDeploymentActions();
|
||||
});
|
||||
describe("deployment eligibility", () => {
|
||||
it.each(["deployment.uploadTo", "deployment.downloadFrom", "deployment.syncWith"])(
|
||||
"filters %s and keeps a single option visible",
|
||||
async (id) => {
|
||||
await run(id);
|
||||
expect(mocks.choose).toHaveBeenCalledWith(expect.objectContaining({ items: ["one"] }));
|
||||
},
|
||||
);
|
||||
it("explains an empty match and links settings", async () => {
|
||||
mocks.coverage[0]!.paths = [];
|
||||
await run("deployment.uploadTo");
|
||||
expect(mocks.choose).not.toHaveBeenCalled();
|
||||
expect(mocks.notify).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
title: "No deployment server covers the selected paths",
|
||||
actions: [expect.objectContaining({ label: "Configure…" })],
|
||||
}),
|
||||
);
|
||||
});
|
||||
it("explains why the default cannot deploy the named file", async () => {
|
||||
mocks.coverage[0]!.paths = [];
|
||||
await run("deployment.uploadToDefault");
|
||||
expect(mocks.upload).not.toHaveBeenCalled();
|
||||
expect(mocks.notify.mock.calls[0]![0].body.props.report.errors[0].path).toBe("/project/b");
|
||||
});
|
||||
it("compares every covered selection and retains skipped paths", async () => {
|
||||
mocks.coverage[0]!.paths.push("/project/c");
|
||||
mocks.choose.mockResolvedValue("one");
|
||||
await run("deployment.syncWith");
|
||||
await vi.waitFor(() => expect(mocks.compare).toHaveBeenCalledTimes(2));
|
||||
expect(useDeployment.getState().sync?.skipped?.[0]?.path).toBe("/project/b");
|
||||
});
|
||||
});
|
||||
@@ -51,22 +51,54 @@ function notifyNoTargets() {
|
||||
});
|
||||
}
|
||||
|
||||
/** Asks which server to use (the only one without asking). */
|
||||
async function pickServer(title: string): Promise<DeploymentServerValid | null> {
|
||||
const servers = deployment().servers;
|
||||
if (!servers.length) {
|
||||
async function serverCoverage(paths: string[]) {
|
||||
try {
|
||||
return await call(commands.deployCoverage(paths));
|
||||
} catch (e) {
|
||||
notifications().notify({
|
||||
level: "error",
|
||||
title: "Cannot check deployment mappings",
|
||||
body: errorText(e),
|
||||
group: GROUP,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function notifyNoCoverage(server?: DeploymentServerValid, skipped?: TransferReport["errors"]) {
|
||||
notifications().notify({
|
||||
level: "warning",
|
||||
title: server
|
||||
? txTemplate(`No selected paths can be deployed to ${serverLabel(server)}`)
|
||||
: "No deployment server covers the selected paths",
|
||||
body: skipped?.length ? <ReportBody report={{ files: 0, skipped: skipped.length, errors: skipped }} /> : undefined,
|
||||
group: GROUP,
|
||||
key: "deployment.noCoverage",
|
||||
actions: [{ label: "Configure…", run: () => void executeAction("deployment.configure") }],
|
||||
});
|
||||
}
|
||||
|
||||
/** Keep the chooser visible even when only one server covers the selection. */
|
||||
async function pickServer(title: string, paths: string[]): Promise<DeploymentServerValid | null> {
|
||||
const configured = deployment().servers;
|
||||
if (!configured.length) {
|
||||
notifyNoServers();
|
||||
return null;
|
||||
}
|
||||
if (servers.length === 1) return servers[0]!;
|
||||
const coverage = await serverCoverage(paths);
|
||||
if (!coverage) return null;
|
||||
const servers = configured.filter((server) => coverage.some((c) => c.server === server.id && c.paths.length));
|
||||
if (!servers.length) {
|
||||
notifyNoCoverage();
|
||||
return null;
|
||||
}
|
||||
const labels = servers.map(serverLabel);
|
||||
const picked = await chooseItem({ title, items: labels });
|
||||
return picked === null ? null : (servers[labels.indexOf(picked)] ?? null);
|
||||
}
|
||||
|
||||
/** The default server; asks when none is marked default and there are several. */
|
||||
async function defaultServer(title: string): Promise<DeploymentServerValid | null> {
|
||||
return resolveServer(deployment(), null) ?? pickServer(title);
|
||||
async function defaultServer(title: string, paths: string[]): Promise<DeploymentServerValid | null> {
|
||||
return resolveServer(deployment(), null) ?? pickServer(title, paths);
|
||||
}
|
||||
|
||||
function ReportBody({ report }: { report: TransferReport }) {
|
||||
@@ -118,6 +150,9 @@ function showFailure(title: string, server: DeploymentServerValid, e: unknown) {
|
||||
|
||||
async function transfer(direction: "upload" | "download", server: DeploymentServerValid | null, paths: string[]) {
|
||||
if (!server) return;
|
||||
const coverage = (await serverCoverage(paths))?.find((c) => c.server === server.id);
|
||||
if (!coverage) return;
|
||||
if (!coverage.paths.length) return notifyNoCoverage(server, coverage.skipped);
|
||||
const request = nextRequestId();
|
||||
try {
|
||||
const report = await withSsh(server.kind === "sftp" ? server.sshConfig : "", () =>
|
||||
@@ -139,8 +174,11 @@ export function downloadPaths(server: DeploymentServerValid, paths: string[]) {
|
||||
}
|
||||
|
||||
async function deleteRemote(paths: string[]) {
|
||||
const server = await defaultServer("Delete from Server");
|
||||
const server = await defaultServer("Delete from Server", paths);
|
||||
if (!server) return;
|
||||
const coverage = (await serverCoverage(paths))?.find((c) => c.server === server.id);
|
||||
if (!coverage) return;
|
||||
if (!coverage.paths.length) return notifyNoCoverage(server, coverage.skipped);
|
||||
const root = projectRoot();
|
||||
const names = paths.map((p) => (root ? relativePath(root, p) || "/" : p));
|
||||
const ok = await confirm({
|
||||
@@ -164,14 +202,18 @@ async function deleteRemote(paths: string[]) {
|
||||
}
|
||||
}
|
||||
|
||||
/** Opens the Sync dialog for `path` and compares it with `server`. */
|
||||
export async function openSync(server: DeploymentServerValid, path: string) {
|
||||
/** Compares every covered selection with `server`. */
|
||||
export async function openSync(server: DeploymentServerValid, paths: string[]) {
|
||||
const coverage = (await serverCoverage(paths))?.find((c) => c.server === server.id);
|
||||
if (!coverage) return;
|
||||
if (!coverage.paths.length) return notifyNoCoverage(server, coverage.skipped);
|
||||
const request = nextRequestId();
|
||||
useDeployment.setState({
|
||||
sync: {
|
||||
server: server.id,
|
||||
serverName: serverLabel(server),
|
||||
path,
|
||||
paths,
|
||||
skipped: coverage.skipped,
|
||||
request,
|
||||
entries: null,
|
||||
error: null,
|
||||
@@ -181,9 +223,15 @@ export async function openSync(server: DeploymentServerValid, path: string) {
|
||||
},
|
||||
});
|
||||
try {
|
||||
const entries = await withSsh(server.kind === "sftp" ? server.sshConfig : "", () =>
|
||||
call(commands.deployCompare(path, server.id, request)),
|
||||
);
|
||||
const groups = await withSsh(server.kind === "sftp" ? server.sshConfig : "", async () => {
|
||||
const entries = [];
|
||||
for (const path of coverage.paths) {
|
||||
if (useDeployment.getState().sync?.request !== request) break;
|
||||
entries.push(...(await call(commands.deployCompare(path, server.id, request))));
|
||||
}
|
||||
return entries;
|
||||
});
|
||||
const entries = [...new Map(groups.map((entry) => [entry.path, entry])).values()];
|
||||
if (useDeployment.getState().sync?.request !== request) return;
|
||||
patchSync({ entries, choices: initialSyncChoices(entries), request: null });
|
||||
} catch (e) {
|
||||
@@ -211,6 +259,7 @@ export async function runSync(steps: { path: string; action: SyncAction }[]) {
|
||||
call(commands.deploySync(steps, server.id, request)),
|
||||
);
|
||||
useDeployment.setState({ sync: null });
|
||||
report.skipped += sync.skipped.length;
|
||||
showReport(report, "sync", server);
|
||||
} catch (e) {
|
||||
if (isCancelled(e)) patchSync({ running: false, request: null });
|
||||
@@ -275,7 +324,7 @@ export function registerDeploymentActions(): () => void {
|
||||
icon: "export",
|
||||
when,
|
||||
keywords: ["deploy", "sftp", "upload", "remote"],
|
||||
run: withTargets(async (paths) => transfer("upload", await defaultServer("Upload to"), paths)),
|
||||
run: withTargets(async (paths) => transfer("upload", await defaultServer("Upload to", paths), paths)),
|
||||
},
|
||||
{
|
||||
id: "deployment.uploadTo",
|
||||
@@ -284,7 +333,7 @@ export function registerDeploymentActions(): () => void {
|
||||
category: CATEGORY,
|
||||
when,
|
||||
keywords: ["deploy", "sftp", "upload", "remote"],
|
||||
run: withTargets(async (paths) => transfer("upload", await pickServer("Upload to"), paths)),
|
||||
run: withTargets(async (paths) => transfer("upload", await pickServer("Upload to", paths), paths)),
|
||||
},
|
||||
{
|
||||
id: "deployment.downloadFromDefault",
|
||||
@@ -293,7 +342,7 @@ export function registerDeploymentActions(): () => void {
|
||||
category: CATEGORY,
|
||||
when,
|
||||
keywords: ["deploy", "sftp", "download", "remote"],
|
||||
run: withTargets(async (paths) => transfer("download", await defaultServer("Download from"), paths)),
|
||||
run: withTargets(async (paths) => transfer("download", await defaultServer("Download from", paths), paths)),
|
||||
},
|
||||
{
|
||||
id: "deployment.downloadFrom",
|
||||
@@ -302,7 +351,7 @@ export function registerDeploymentActions(): () => void {
|
||||
category: CATEGORY,
|
||||
when,
|
||||
keywords: ["deploy", "sftp", "download", "remote"],
|
||||
run: withTargets(async (paths) => transfer("download", await pickServer("Download from"), paths)),
|
||||
run: withTargets(async (paths) => transfer("download", await pickServer("Download from", paths), paths)),
|
||||
},
|
||||
{
|
||||
id: "deployment.syncWith",
|
||||
@@ -315,7 +364,8 @@ export function registerDeploymentActions(): () => void {
|
||||
const paths = actionTargets(arg);
|
||||
const target = paths[0] ?? projectRoot();
|
||||
if (!target) return;
|
||||
void pickServer("Sync with Deployed to").then((server) => server && openSync(server, target));
|
||||
const targets = paths.length ? paths : [target];
|
||||
void pickServer("Sync with Deployed to", targets).then((server) => server && openSync(server, targets));
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/** Session state of the deployment feature: the Sync dialog, the SSH secret prompt and the Remote Host tree. */
|
||||
import { create } from "zustand";
|
||||
import type { RemoteEntry, RequestId, SyncEntry } from "@/platform/ipc";
|
||||
import type { TransferReport, RemoteEntry, RequestId, SyncEntry } from "@/platform/ipc";
|
||||
import type { FocusedPart, SyncChoice } from "./model";
|
||||
|
||||
export const REMOTE_HOST_WINDOW = "remoteHost";
|
||||
@@ -8,8 +8,9 @@ export const REMOTE_HOST_WINDOW = "remoteHost";
|
||||
export interface SyncDialogState {
|
||||
server: string;
|
||||
serverName: string;
|
||||
/** Absolute local path compared (file or folder). */
|
||||
path: string;
|
||||
/** Absolute local paths selected for comparison. */
|
||||
paths: string[];
|
||||
skipped: TransferReport["errors"];
|
||||
request: RequestId | null;
|
||||
entries: SyncEntry[] | null;
|
||||
error: string | null;
|
||||
|
||||
Reference in new issue
Заблокувати користувача