fix: harden workflow console contracts
This commit is contained in:
@@ -75,7 +75,9 @@ describe("ConsoleShell", () => {
|
||||
);
|
||||
|
||||
const skipLink = screen.getByRole("link", { name: "Skip to main content" });
|
||||
const main = screen.getByRole("main", { name: "Console workspace" });
|
||||
expect(skipLink).toHaveAttribute("href", "#console-workspace-main");
|
||||
expect(main).toHaveAttribute("tabindex", "-1");
|
||||
|
||||
await userEvent.tab();
|
||||
|
||||
|
||||
@@ -56,7 +56,7 @@ export const ConsoleShell = ({
|
||||
</li>
|
||||
</ul>
|
||||
</nav>
|
||||
<main id="console-workspace-main" aria-label="Console workspace">
|
||||
<main id="console-workspace-main" aria-label="Console workspace" tabIndex={-1}>
|
||||
{children}
|
||||
</main>
|
||||
<aside aria-label="Operation evidence" className="console-workspace__evidence">
|
||||
|
||||
@@ -50,7 +50,7 @@ describe("CapabilityClient", () => {
|
||||
|
||||
await client.list({
|
||||
query: "document",
|
||||
sourceId: "local.lda_docs",
|
||||
sourceId: " local.lda_docs ",
|
||||
limit: 50,
|
||||
});
|
||||
|
||||
@@ -65,7 +65,7 @@ describe("CapabilityClient", () => {
|
||||
const readExecutor = executor();
|
||||
const client = createCapabilityClient(readExecutor);
|
||||
|
||||
await client.inspect("local.lda_docs.read_documents");
|
||||
await client.inspect(" local.lda_docs.read_documents ");
|
||||
|
||||
expect(readExecutor.run).toHaveBeenCalledWith(
|
||||
"workflow.capabilities.inspect",
|
||||
|
||||
@@ -27,7 +27,7 @@ export const createCapabilityClient = (
|
||||
list: (input) => {
|
||||
const params: Record<string, string | number> = {};
|
||||
if (input.query !== undefined) params.query = input.query;
|
||||
if (input.sourceId !== undefined) params.source_id = input.sourceId;
|
||||
if (input.sourceId !== undefined) params.source_id = input.sourceId.trim();
|
||||
if (input.cursor !== undefined) params.cursor = input.cursor;
|
||||
if (input.limit !== undefined) params.limit = input.limit;
|
||||
return executor.run(
|
||||
@@ -38,7 +38,8 @@ export const createCapabilityClient = (
|
||||
},
|
||||
|
||||
inspect: (qualifiedName) => {
|
||||
if (!qualifiedName.trim()) {
|
||||
const normalizedQualifiedName = qualifiedName.trim();
|
||||
if (!normalizedQualifiedName) {
|
||||
return Promise.reject(
|
||||
invalidInput(
|
||||
"workflow.capabilities.inspect",
|
||||
@@ -48,7 +49,7 @@ export const createCapabilityClient = (
|
||||
}
|
||||
return executor.run(
|
||||
"workflow.capabilities.inspect",
|
||||
{ qualified_name: qualifiedName },
|
||||
{ qualified_name: normalizedQualifiedName },
|
||||
decodeCapabilityDetail,
|
||||
);
|
||||
},
|
||||
|
||||
@@ -27,7 +27,7 @@ describe("DraftWorkspaceClient", () => {
|
||||
const readExecutor = executor();
|
||||
const client = createDraftWorkspaceClient(readExecutor);
|
||||
|
||||
await client.load("draft-report");
|
||||
await client.load(" draft-report ");
|
||||
|
||||
expect(readExecutor.run).toHaveBeenCalledWith(
|
||||
"workflow.draft_workspaces.get",
|
||||
|
||||
@@ -27,7 +27,8 @@ export const createDraftWorkspaceClient = (
|
||||
),
|
||||
|
||||
load: (workspaceId) => {
|
||||
if (!workspaceId.trim()) {
|
||||
const normalizedWorkspaceId = workspaceId.trim();
|
||||
if (!normalizedWorkspaceId) {
|
||||
return Promise.reject(
|
||||
invalidInput(
|
||||
"workflow.draft_workspaces.get",
|
||||
@@ -37,7 +38,7 @@ export const createDraftWorkspaceClient = (
|
||||
}
|
||||
return executor.run(
|
||||
"workflow.draft_workspaces.get",
|
||||
{ workspace_id: workspaceId, include_draft: true },
|
||||
{ workspace_id: normalizedWorkspaceId, include_draft: true },
|
||||
decodeDraftWorkspace,
|
||||
);
|
||||
},
|
||||
|
||||
@@ -15,8 +15,9 @@ export class ConsoleClientError extends Error {
|
||||
readonly kind: ConsoleClientErrorKind,
|
||||
readonly operation: OperationName,
|
||||
message: string,
|
||||
options?: ErrorOptions,
|
||||
) {
|
||||
super(message);
|
||||
super(message, options);
|
||||
Object.setPrototypeOf(this, new.target.prototype);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,10 +33,10 @@ describe("lifecycle clients", () => {
|
||||
const readExecutor = executor();
|
||||
const { artifacts, deployments, runs } = createLifecycleClients(readExecutor);
|
||||
|
||||
await artifacts.inspect("report", 2);
|
||||
await deployments.inspect("report.default");
|
||||
await deployments.validate("report.default");
|
||||
await runs.inspect("run_123");
|
||||
await artifacts.inspect(" report ", 2);
|
||||
await deployments.inspect(" report.default ");
|
||||
await deployments.validate(" report.default ");
|
||||
await runs.inspect(" run_123 ");
|
||||
|
||||
expect(readExecutor.run).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
@@ -68,7 +68,7 @@ describe("lifecycle clients", () => {
|
||||
const readExecutor = executor();
|
||||
const { runs } = createLifecycleClients(readExecutor);
|
||||
|
||||
await runs.trace("run_123", 50, 50);
|
||||
await runs.trace(" run_123 ", 50, 50);
|
||||
|
||||
expect(readExecutor.run).toHaveBeenCalledWith(
|
||||
"workflow.runs.trace",
|
||||
|
||||
@@ -53,8 +53,10 @@ const requireIdentifier = (
|
||||
operation: OperationName,
|
||||
value: string,
|
||||
label: string,
|
||||
): void => {
|
||||
if (!value.trim()) throw invalidInput(operation, `${label} must not be blank`);
|
||||
): string => {
|
||||
const normalizedValue = value.trim();
|
||||
if (!normalizedValue) throw invalidInput(operation, `${label} must not be blank`);
|
||||
return normalizedValue;
|
||||
};
|
||||
|
||||
const requirePositiveInteger = (
|
||||
@@ -93,7 +95,7 @@ export const createLifecycleClients = (executor: ConsoleReadExecutor): Lifecycle
|
||||
return executor.run("workflow.artifacts.list", params, decodeArtifactList);
|
||||
},
|
||||
inspect: async (artifactId, version) => {
|
||||
requireIdentifier(
|
||||
const normalizedArtifactId = requireIdentifier(
|
||||
"workflow.artifacts.inspect",
|
||||
artifactId,
|
||||
"artifact id",
|
||||
@@ -101,7 +103,7 @@ export const createLifecycleClients = (executor: ConsoleReadExecutor): Lifecycle
|
||||
requirePositiveInteger("workflow.artifacts.inspect", version, "version");
|
||||
return executor.run(
|
||||
"workflow.artifacts.inspect",
|
||||
{ artifact_id: artifactId, version },
|
||||
{ artifact_id: normalizedArtifactId, version },
|
||||
decodeArtifactDetail,
|
||||
);
|
||||
},
|
||||
@@ -111,26 +113,26 @@ export const createLifecycleClients = (executor: ConsoleReadExecutor): Lifecycle
|
||||
list: () =>
|
||||
executor.run("workflow.deployments.list", {}, decodeDeploymentList),
|
||||
inspect: async (deploymentId) => {
|
||||
requireIdentifier(
|
||||
const normalizedDeploymentId = requireIdentifier(
|
||||
"workflow.deployments.inspect",
|
||||
deploymentId,
|
||||
"deployment id",
|
||||
);
|
||||
return executor.run(
|
||||
"workflow.deployments.inspect",
|
||||
{ deployment_id: deploymentId },
|
||||
{ deployment_id: normalizedDeploymentId },
|
||||
decodeDeploymentDetail,
|
||||
);
|
||||
},
|
||||
validate: async (deploymentId) => {
|
||||
requireIdentifier(
|
||||
const normalizedDeploymentId = requireIdentifier(
|
||||
"workflow.deployments.validate",
|
||||
deploymentId,
|
||||
"deployment id",
|
||||
);
|
||||
return executor.run(
|
||||
"workflow.deployments.validate",
|
||||
{ deployment_id: deploymentId },
|
||||
{ deployment_id: normalizedDeploymentId },
|
||||
decodeDeploymentValidation,
|
||||
);
|
||||
},
|
||||
@@ -144,19 +146,27 @@ export const createLifecycleClients = (executor: ConsoleReadExecutor): Lifecycle
|
||||
return executor.run("workflow.runs.list", params, decodeRunList);
|
||||
},
|
||||
inspect: async (runId) => {
|
||||
requireIdentifier("workflow.runs.inspect", runId, "run id");
|
||||
const normalizedRunId = requireIdentifier(
|
||||
"workflow.runs.inspect",
|
||||
runId,
|
||||
"run id",
|
||||
);
|
||||
return executor.run(
|
||||
"workflow.runs.inspect",
|
||||
{ run_id: runId },
|
||||
{ run_id: normalizedRunId },
|
||||
decodeRunDetail,
|
||||
);
|
||||
},
|
||||
trace: async (runId, start, limit) => {
|
||||
requireIdentifier("workflow.runs.trace", runId, "run id");
|
||||
const normalizedRunId = requireIdentifier(
|
||||
"workflow.runs.trace",
|
||||
runId,
|
||||
"run id",
|
||||
);
|
||||
requireTraceRange("workflow.runs.trace", start, limit);
|
||||
return executor.run(
|
||||
"workflow.runs.trace",
|
||||
{ run_id: runId, trace_range: { start, limit } },
|
||||
{ run_id: normalizedRunId, trace_range: { start, limit } },
|
||||
decodeTracePage,
|
||||
);
|
||||
},
|
||||
|
||||
@@ -75,6 +75,21 @@ describe("ConsoleReadExecutor", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("maps unknown browser operations to operation errors", async () => {
|
||||
const executor = createConsoleReadExecutor({
|
||||
target: "http://console.test/rpc",
|
||||
recordEvidence: vi.fn(),
|
||||
invoke: vi.fn(async () => failure("unknown_operation")),
|
||||
});
|
||||
|
||||
await expect(
|
||||
executor.run("workflow.capabilities.list", {}, (value) => value),
|
||||
).rejects.toMatchObject({
|
||||
kind: "operation",
|
||||
operation: "workflow.capabilities.list",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a successful response for a different requested operation", async () => {
|
||||
const recordEvidence = vi.fn();
|
||||
const decode = vi.fn((value: unknown) => value);
|
||||
@@ -104,6 +119,7 @@ describe("ConsoleReadExecutor", () => {
|
||||
|
||||
it("turns decoder failures into decode errors", async () => {
|
||||
const recordEvidence = vi.fn();
|
||||
const cause = new Error("invalid capability page");
|
||||
const executor = createConsoleReadExecutor({
|
||||
target: "http://console.test/rpc",
|
||||
recordEvidence,
|
||||
@@ -112,14 +128,55 @@ describe("ConsoleReadExecutor", () => {
|
||||
|
||||
await expect(
|
||||
executor.run("workflow.capabilities.list", {}, () => {
|
||||
throw new Error("invalid capability page");
|
||||
throw cause;
|
||||
}),
|
||||
).rejects.toMatchObject({ kind: "decode" });
|
||||
).rejects.toMatchObject({ kind: "decode", cause });
|
||||
expect(recordEvidence).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("maps rejected invocations to transport errors", async () => {
|
||||
const recordEvidence = vi.fn();
|
||||
const cause = new Error("fetch failed");
|
||||
const executor = createConsoleReadExecutor({
|
||||
target: "http://console.test/rpc",
|
||||
recordEvidence,
|
||||
invoke: vi.fn(async () => {
|
||||
throw cause;
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(
|
||||
executor.run("workflow.capabilities.list", {}, (value) => value),
|
||||
).rejects.toMatchObject({ kind: "transport", cause });
|
||||
expect(recordEvidence).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("measures duration for failures before operation metadata exists", async () => {
|
||||
const now = vi
|
||||
.spyOn(performance, "now")
|
||||
.mockReturnValueOnce(100)
|
||||
.mockReturnValueOnce(137);
|
||||
const recordEvidence = vi.fn();
|
||||
const executor = createConsoleReadExecutor({
|
||||
target: "http://console.test/rpc",
|
||||
recordEvidence,
|
||||
invoke: vi.fn(async () => failure("upstream_unreachable")),
|
||||
});
|
||||
|
||||
await expect(
|
||||
executor.run("workflow.capabilities.list", {}, (value) => value),
|
||||
).rejects.toMatchObject({ kind: "connection" });
|
||||
|
||||
expect(recordEvidence.mock.calls[0]?.[0]?.durationMs).toBe(37);
|
||||
now.mockRestore();
|
||||
});
|
||||
|
||||
it("measures duration for rejected invocations before operation metadata exists", async () => {
|
||||
const now = vi
|
||||
.spyOn(performance, "now")
|
||||
.mockReturnValueOnce(200)
|
||||
.mockReturnValueOnce(249);
|
||||
const recordEvidence = vi.fn();
|
||||
const executor = createConsoleReadExecutor({
|
||||
target: "http://console.test/rpc",
|
||||
recordEvidence,
|
||||
@@ -131,7 +188,9 @@ describe("ConsoleReadExecutor", () => {
|
||||
await expect(
|
||||
executor.run("workflow.capabilities.list", {}, (value) => value),
|
||||
).rejects.toMatchObject({ kind: "transport" });
|
||||
expect(recordEvidence).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(recordEvidence.mock.calls[0]?.[0]?.durationMs).toBe(49);
|
||||
now.mockRestore();
|
||||
});
|
||||
|
||||
it.each([
|
||||
|
||||
@@ -28,7 +28,7 @@ const errorKindForCode = (code: string): ConsoleClientErrorKind => {
|
||||
case "upstream_unreachable":
|
||||
return "connection";
|
||||
case "unknown_operation":
|
||||
return "permission";
|
||||
return "operation";
|
||||
case "rpc_decode_error":
|
||||
return "decode";
|
||||
default:
|
||||
@@ -85,6 +85,9 @@ export const createConsoleReadExecutor = (options: {
|
||||
params: unknown,
|
||||
decode: (value: unknown) => T,
|
||||
): Promise<T> {
|
||||
const startedAt = performance.now();
|
||||
const durationSinceStart = (): number =>
|
||||
Math.max(0, Math.round(performance.now() - startedAt));
|
||||
let response: RpcResponse;
|
||||
try {
|
||||
response = await invoke(operation, options.target, params);
|
||||
@@ -95,12 +98,13 @@ export const createConsoleReadExecutor = (options: {
|
||||
"unavailable: operation failed before CLI metadata",
|
||||
null,
|
||||
null,
|
||||
0,
|
||||
durationSinceStart(),
|
||||
);
|
||||
throw new ConsoleClientError(
|
||||
clientErrorKindForInvocation(error),
|
||||
operation,
|
||||
errorMessage(error),
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -111,7 +115,7 @@ export const createConsoleReadExecutor = (options: {
|
||||
"unavailable: operation failed before CLI metadata",
|
||||
response.exchange.request,
|
||||
response.exchange.response,
|
||||
0,
|
||||
durationSinceStart(),
|
||||
);
|
||||
throw new ConsoleClientError(
|
||||
errorKindForCode(response.error.code),
|
||||
@@ -152,6 +156,7 @@ export const createConsoleReadExecutor = (options: {
|
||||
"decode",
|
||||
operation,
|
||||
errorMessage(error),
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
},
|
||||
|
||||
@@ -117,6 +117,27 @@ describe("LifecycleRoute", () => {
|
||||
expect(currentController.selectArtifact).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not decode a literal percent sequence twice", async () => {
|
||||
const currentController = controller();
|
||||
mockUseLifecycleExplorer.mockReturnValue(currentController);
|
||||
|
||||
renderRoute("/console/runs/run%2525complete");
|
||||
|
||||
await waitFor(() => {
|
||||
expect(currentController.selectRun).toHaveBeenCalledWith("run%25complete");
|
||||
});
|
||||
});
|
||||
|
||||
it("does not select an artifact with a nonnumeric version route", async () => {
|
||||
const currentController = controller();
|
||||
mockUseLifecycleExplorer.mockReturnValue(currentController);
|
||||
|
||||
renderRoute("/console/artifacts/report/not-a-version");
|
||||
|
||||
await screen.findByRole("heading", { name: "Artifacts", level: 1 });
|
||||
expect(currentController.selectArtifact).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("suppresses stale detail synchronously until it matches the URL identity", () => {
|
||||
const currentController = controller({
|
||||
...emptyState(),
|
||||
|
||||
@@ -24,14 +24,6 @@ const artifactPathFor = (artifactKey: string): string => {
|
||||
return `/console/artifacts/${encodeURIComponent(artifactKey.slice(0, separator))}/${encodeURIComponent(artifactKey.slice(separator + 1))}`;
|
||||
};
|
||||
|
||||
const decodeRouteParam = (value: string): string => {
|
||||
try {
|
||||
return decodeURIComponent(value);
|
||||
} catch {
|
||||
return value;
|
||||
}
|
||||
};
|
||||
|
||||
const visibleStateForRoute = (
|
||||
state: LifecycleState,
|
||||
kind: LifecycleRouteKind,
|
||||
@@ -93,16 +85,21 @@ export const LifecycleRoute = ({ kind }: LifecycleRouteProps) => {
|
||||
selectRun,
|
||||
} = controller;
|
||||
|
||||
const artifactVersion = params.version === undefined ? null : Number(params.version);
|
||||
const artifactIdentity =
|
||||
kind === "artifact" && params.artifactId && params.version
|
||||
? `${decodeRouteParam(params.artifactId)}@${decodeRouteParam(params.version)}`
|
||||
kind === "artifact" &&
|
||||
params.artifactId &&
|
||||
artifactVersion !== null &&
|
||||
Number.isSafeInteger(artifactVersion) &&
|
||||
artifactVersion >= 0
|
||||
? `${params.artifactId}@${artifactVersion}`
|
||||
: null;
|
||||
const deploymentIdentity =
|
||||
kind === "deployment" && params.deploymentId
|
||||
? decodeRouteParam(params.deploymentId)
|
||||
? params.deploymentId
|
||||
: null;
|
||||
const runIdentity =
|
||||
kind === "run" && params.runId ? decodeRouteParam(params.runId) : null;
|
||||
kind === "run" && params.runId ? params.runId : null;
|
||||
const visibleState = useMemo(
|
||||
() => visibleStateForRoute(
|
||||
state,
|
||||
|
||||
Reference in New Issue
Block a user