diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java index 778a04e169..9b9fca4133 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java @@ -21,6 +21,7 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RequestPart; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.request.RequestContextHolder; @@ -51,6 +52,7 @@ import stirling.software.common.util.TempFile; import stirling.software.common.util.TempFileManager; import stirling.software.proprietary.audit.AuditContext; import stirling.software.proprietary.policy.asset.PolicyAssetCleaner; +import stirling.software.proprietary.policy.asset.PolicyAssetResolver; import stirling.software.proprietary.policy.config.PolicyAccessGuard; import stirling.software.proprietary.policy.config.PolicyManagementAuthority; import stirling.software.proprietary.policy.engine.PolicyRunHandle; @@ -106,6 +108,7 @@ public class PolicyController { private final PolicyTriggerManager policyTriggerManager; private final PolicyOverviewService policyOverviewService; private final PolicyAssetCleaner assetCleaner; + private final PolicyAssetResolver assetResolver; private final ProcessedLedger processedLedger; private final List policyTriggers; private final ApplicationProperties applicationProperties; @@ -125,12 +128,13 @@ public class PolicyController { + " endpoint and download outputs via /api/v1/general/files/{id}.") public ResponseEntity> run( @RequestPart("json") PipelineDefinition definition, + @RequestParam(value = "policyId", required = false) String policyId, @Valid @ModelAttribute PolicyRunFiles files) throws IOException { stampPolicyAudit(definition); requireRunnable(definition); validateAdHocRun(definition); - PolicyInputs inputs = toInputs(files); + PolicyInputs inputs = resolveStoredAssets(policyId, toInputs(files)); PolicyRunHandle handle = policyRunner.runAdHoc(definition, inputs, PolicyProgressListener.NOOP); recordEditorDocs(inputs); @@ -146,12 +150,13 @@ public class PolicyController { + " 'cancelled', or 'waiting' event carrying the final run view.") public SseEmitter runStream( @RequestPart("json") PipelineDefinition definition, + @RequestParam(value = "policyId", required = false) String policyId, @Valid @ModelAttribute PolicyRunFiles files) throws IOException { stampPolicyAudit(definition); requireRunnable(definition); validateAdHocRun(definition); - PolicyInputs inputs = toInputs(files); + PolicyInputs inputs = resolveStoredAssets(policyId, toInputs(files)); SseEmitter emitter = new SseEmitter(applicationProperties.getPolicies().getStreamTimeoutMs()); @@ -438,10 +443,7 @@ public class PolicyController { * folder sources/outputs is enforced separately by {@link PolicyValidator} at validation time. */ private void requirePolicyEditingAllowed() { - if (!applicationProperties.getSecurity().isEnableLogin()) { - return; - } - if (!policyManagementAuthority.canEditPolicies()) { + if (!policyEditingAllowed()) { throw new ResponseStatusException( HttpStatus.FORBIDDEN, "Policies may only be created or modified by a team leader"); @@ -467,6 +469,15 @@ public class PolicyController { } } + /** + * Whether the caller may create/modify policies (a team leader, or any operator when login is + * off). + */ + private boolean policyEditingAllowed() { + return !applicationProperties.getSecurity().isEnableLogin() + || policyManagementAuthority.canEditPolicies(); + } + @GetMapping @Operation( summary = "List policies", @@ -672,6 +683,25 @@ public class PolicyController { inputs.primary().size()); } + /** + * Resolve a test run's stored {@code asset:} bindings from the saved policy the builder is + * editing, so their bytes need not be re-uploaded. Scoped to that policy (the resolver loads + * only the assets it references, in its own team) and gated to policy editors - the same + * authority that can read asset bytes - so a member can't rebind a policy's stored asset into + * an ad-hoc step to read it back. A blank id (an unsaved pipeline has no stored bindings) or an + * inaccessible policy leaves the run-supplied inputs untouched. + */ + private PolicyInputs resolveStoredAssets(String policyId, PolicyInputs inputs) { + if (policyId == null || policyId.isBlank() || !policyEditingAllowed()) { + return inputs; + } + return policyStore + .get(policyId) + .filter(policyAccessGuard::canAccess) + .map(policy -> assetResolver.resolve(policy, inputs)) + .orElse(inputs); + } + /** * Turn the typed run files into engine {@link PolicyInputs}: the primary documents plus the * named supporting-file store, where each asset's {@code key} is the name a step references diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java index 2fa675597c..36f5cc221b 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java @@ -83,6 +83,8 @@ class PolicyControllerTest { @Mock private stirling.software.proprietary.policy.asset.PolicyAssetCleaner assetCleaner; + @Mock private stirling.software.proprietary.policy.asset.PolicyAssetResolver assetResolver; + @Mock private ProcessedLedger processedLedger; @Mock private TempFileManager tempFileManager; @@ -115,6 +117,7 @@ class PolicyControllerTest { policyTriggerManager, policyOverviewService, assetCleaner, + assetResolver, processedLedger, policyTriggers, applicationProperties, @@ -232,7 +235,7 @@ class PolicyControllerTest { .thenReturn(handle("run-1")); ResponseEntity> response = - controller.run(definitionWithStep(), new PolicyRunFiles()); + controller.run(definitionWithStep(), null, new PolicyRunFiles()); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.ACCEPTED); assertThat(response.getBody().getJobId()).isEqualTo("run-1"); @@ -245,7 +248,7 @@ class PolicyControllerTest { .thenReturn(handle("run-1")); when(sourceAccessGuard.currentTeamId()).thenReturn(3L); - controller.run(definitionWithStep(), new PolicyRunFiles()); + controller.run(definitionWithStep(), null, new PolicyRunFiles()); verify(docCounter).record(EditorSource.counterKey(3L), 0L); } @@ -255,7 +258,7 @@ class PolicyControllerTest { void rejectsEmptyPipeline() { PipelineDefinition empty = new PipelineDefinition("pipe", List.of(), List.of()); - assertThatThrownBy(() -> controller.run(empty, new PolicyRunFiles())) + assertThatThrownBy(() -> controller.run(empty, null, new PolicyRunFiles())) .isInstanceOf(ResponseStatusException.class) .satisfies( e -> @@ -277,7 +280,7 @@ class PolicyControllerTest { .when(policyValidator) .validateOutput(any()); - assertThatThrownBy(() -> controller.run(definition, new PolicyRunFiles())) + assertThatThrownBy(() -> controller.run(definition, null, new PolicyRunFiles())) .isInstanceOf(ResponseStatusException.class) .satisfies( e -> @@ -285,6 +288,38 @@ class PolicyControllerTest { .isEqualTo(HttpStatus.BAD_REQUEST)); verify(policyRunner, never()).runAdHoc(any(), any(), any()); } + + @Test + @DisplayName("resolves stored assets from the supplied policy when the caller may edit it") + void resolvesStoredAssetsForEditor() throws Exception { + applicationProperties.getSecurity().setEnableLogin(false); // editing allowed + Policy p = policy("pol-1", 1L); + when(policyStore.get("pol-1")).thenReturn(Optional.of(p)); + when(policyAccessGuard.canAccess(p)).thenReturn(true); + when(assetResolver.resolve(eq(p), any())).thenAnswer(inv -> inv.getArgument(1)); + when(policyRunner.runAdHoc(any(), any(), eq(PolicyProgressListener.NOOP))) + .thenReturn(handle("run-1")); + + controller.run(definitionWithStep(), "pol-1", new PolicyRunFiles()); + + verify(assetResolver).resolve(eq(p), any()); + } + + @Test + @DisplayName("does not resolve a policy's stored assets for a caller who cannot edit it") + void skipsStoredAssetsForNonEditor() throws Exception { + // Gating asset resolution to editors keeps a member from rebinding a policy's stored + // asset into an ad-hoc step to read it back. + applicationProperties.getSecurity().setEnableLogin(true); + when(policyManagementAuthority.canEditPolicies()).thenReturn(false); + when(policyRunner.runAdHoc(any(), any(), eq(PolicyProgressListener.NOOP))) + .thenReturn(handle("run-1")); + + controller.run(definitionWithStep(), "pol-1", new PolicyRunFiles()); + + verify(assetResolver, never()).resolve(any(), any()); + verify(policyStore, never()).get(any()); + } } @Nested @@ -296,7 +331,8 @@ class PolicyControllerTest { void returnsEmitter() throws Exception { when(policyRunner.runAdHoc(any(), any(), any())).thenReturn(handle("run-2")); - SseEmitter emitter = controller.runStream(definitionWithStep(), new PolicyRunFiles()); + SseEmitter emitter = + controller.runStream(definitionWithStep(), null, new PolicyRunFiles()); assertThat(emitter).isNotNull(); } @@ -306,7 +342,7 @@ class PolicyControllerTest { void rejectsEmpty() { PipelineDefinition empty = new PipelineDefinition("pipe", List.of(), List.of()); - assertThatThrownBy(() -> controller.runStream(empty, new PolicyRunFiles())) + assertThatThrownBy(() -> controller.runStream(empty, null, new PolicyRunFiles())) .isInstanceOf(ResponseStatusException.class); } } diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 12ad26ceec..01db314031 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -7705,7 +7705,6 @@ moreActions = "More actions" needsConfiguring = "Needs setting up" needsDestination = "No destination chosen" needsSource = "No source chosen" -needsUpload = "Needs an uploaded file" noToolMatches = "No tools match your search." pause = "Pause" rename = "Rename pipeline" @@ -7713,11 +7712,11 @@ searchTools = "Search tools" sendToSystem = "Send to another system" stepsIncompatible = "These steps can't run on what their prior step produces: {{tools}}." stepsNeedSetup = "These steps still need setting up before saving: {{tools}}." +supportingFiles = "Supporting files" testRun = "Test with a file" unknownStep = "Unrecognized operation, kept as-is." unsavedBody = "You have unsaved changes. Save them before leaving, or discard them?" unsavedTitle = "Unsaved changes" -uploadUnsupported = "Uploaded files aren't supported in pipelines yet, so these steps can't be saved: {{tools}}." usesDefaults = "Runs with default settings" viewDefinition = "View definition" @@ -7730,7 +7729,6 @@ saveHeading = "To save your changes:" schedule = "Set how often it runs" setup = "Finish setting up: {{tools}}" source = "Choose an input source" -upload = "Remove steps that need an uploaded file: {{tools}}" [portal.pipelines.builder.diagnostic] fan-in = "Combines every incoming file" diff --git a/frontend/editor/scripts/generate-tool-api-types.mts b/frontend/editor/scripts/generate-tool-api-types.mts index d5990096af..38eb7fe7b6 100644 --- a/frontend/editor/scripts/generate-tool-api-types.mts +++ b/frontend/editor/scripts/generate-tool-api-types.mts @@ -28,10 +28,11 @@ const ALLOWED_PATH_PREFIXES = [ "/api/v1/integration/", ]; -// File plumbing, not user parameters: `fileInput` is the uploaded document and -// `fileId` a server-side handle. Stripped from every generated request model. -// Named file fields (stampImage, attachments, ...) are real parameters and kept. -const BASE_FILE_FIELDS = new Set(["fileInput", "fileId"]); +// File plumbing, not user parameters: `fileInput` and `file` are the uploaded primary document +// (endpoints use one name or the other - `file` is never a second, supporting upload) and `fileId` +// a server-side handle. Stripped from every generated request model. Named supporting-file fields +// (stampImage, attachments, ...) are real parameters and kept. +const BASE_FILE_FIELDS = new Set(["fileInput", "file", "fileId"]); // The shared "upload a file or provide a file ID" wrapper schema and its two // branches. An endpoint whose body is exactly this has no parameters, so it must @@ -73,6 +74,19 @@ function isObject(value: unknown): value is Json { return typeof value === "object" && value !== null && !Array.isArray(value); } +/** A single file upload: `type: string, format: binary` (a Java MultipartFile param). */ +function isBinaryField(schema: unknown): schema is Json { + return ( + isObject(schema) && schema.type === "string" && schema.format === "binary" + ); +} + +/** A multi file upload: an array of binary items (some specs also flag the array itself binary). */ +function isBinaryArrayField(schema: unknown): schema is Json { + if (!isObject(schema) || schema.type !== "array") return false; + return schema.format === "binary" || isBinaryField(schema.items); +} + /** * Recursively sort object keys so the output is byte-stable regardless of the * key ordering springdoc happens to emit. @@ -358,6 +372,9 @@ async function main(): Promise { const usedClassNames = new Set(); const pendingComponents = new Set(); const skipped: string[] = []; + // Named file fields (as File uploads) per model, so a caller can tell a file param from a scalar + // string param - which `format: binary` -> `string` would otherwise erase. + const fileFieldsByClass: Record = {}; for (const path of Object.keys(paths).sort()) { if ( @@ -408,7 +425,32 @@ async function main(): Promise { const query = queryParameters(pathItem); // Body wins over query on a name collision. const properties: Json = { ...query.props, ...bodyProps }; + // `file` is stripped as a primary-document alias (see BASE_FILE_FIELDS). That only holds while + // no endpoint uses `file` as a *supporting* upload beside a primary `fileInput`; if one ever + // does, blanket-stripping would silently drop it. Fail generation so the assumption is fixed + // here rather than shipping a lost file. + if ("file" in properties && "fileInput" in properties) { + throw new Error( + `${path} has both 'fileInput' and 'file' uploads. 'file' is stripped as a primary-document` + + " alias, which would drop it as a supporting file. Rename the supporting param or revise" + + " BASE_FILE_FIELDS handling in this generator.", + ); + } for (const field of BASE_FILE_FIELDS) delete properties[field]; + // Type each named file upload as File/File[] (not the `string` a binary format yields) via + // json-schema-to-typescript's `tsType` override, and record it. Base file fields are already + // stripped, so what remains is the real supporting-file params. + const fileFields: string[] = []; + for (const [name, prop] of Object.entries(properties)) { + if (isBinaryField(prop)) { + prop.tsType = "File"; + fileFields.push(name); + } else if (isBinaryArrayField(prop)) { + prop.tsType = "File[]"; + fileFields.push(name); + } + } + fileFieldsByClass[className] = fileFields; modelSchema.properties = properties; const required = new Set(computeRequired(modelSchema, properties)); for (const name of query.required) { @@ -464,6 +506,7 @@ async function main(): Promise { await compileAndWrite( tools, definitions, + fileFieldsByClass, outputPath, values.check ?? false, skipped, @@ -473,6 +516,7 @@ async function main(): Promise { async function compileAndWrite( tools: DiscoveredTool[], definitions: Record, + fileFieldsByClass: Record, outputPath: string, check: boolean, skipped: string[], @@ -525,6 +569,15 @@ async function compileAndWrite( const endpointList = tools .map((t) => ` ${JSON.stringify(t.path)},`) .join("\n"); + // Endpoints that take supporting files, mapped to those file params' names. Only endpoints with at + // least one are listed, so membership answers "does this tool take extra files". + const fileFieldEntries = tools + .filter((t) => (fileFieldsByClass[t.className] ?? []).length > 0) + .map( + (t) => + ` ${JSON.stringify(t.path)}: ${JSON.stringify(fileFieldsByClass[t.className])},`, + ) + .join("\n"); const footer = [ "/** Endpoint path for a generated tool operation (the operation identity across languages). */", @@ -536,6 +589,9 @@ async function compileAndWrite( "/** Every generated tool endpoint, for iteration. */", `export const TOOL_ENDPOINTS = [\n${endpointList}\n] as const satisfies readonly ToolEndpoint[];`, "", + "/** The supporting-file parameters each endpoint accepts beyond its primary fileInput, by name. */", + `export const TOOL_FILE_FIELDS = {\n${fileFieldEntries}\n} as const satisfies Partial<\n Record\n>;`, + "", "/** Union of every generated tool request model. */", `export type ToolApiRequest = ToolApiParams[ToolEndpoint];`, ].join("\n"); diff --git a/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.test.ts b/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.test.ts index ba8c0bc2be..f3443dfb51 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.test.ts @@ -41,12 +41,13 @@ describe("objectToFormData", () => { }); test("expands arrays into repeated fields", () => { - const request: ToolApiParams["/api/v1/misc/add-attachments"] = { - attachments: ["a.png", "b.png", "c.png"], + const request: ToolApiParams["/api/v1/misc/ocr-pdf"] = { + ocrType: "Normal", + languages: ["eng", "fra", "deu"], }; const formData = objectToFormData(request); - expect(formData.getAll("attachments")).toEqual(["a.png", "b.png", "c.png"]); + expect(formData.getAll("languages")).toEqual(["eng", "fra", "deu"]); }); test("throws on a non-primitive field value rather than dropping it", () => { @@ -70,6 +71,18 @@ describe("objectToFormData", () => { expect(formData.get("optimizeLevel")).toBe("5"); }); + test("sends a File-valued model field as a file part, not stringified", () => { + const stamp = new File(["s"], "stamp.png", { type: "image/png" }); + const request: ToolApiParams["/api/v1/misc/add-stamp"] = { + stampType: "image", + stampImage: stamp, + }; + const formData = objectToFormData(request); + + expect(formData.get("stampImage")).toBe(stamp); + expect(formData.get("stampType")).toBe("image"); + }); + test("appends multiple files under the same field name", () => { const files = [ new File(["1"], "a.pdf", { type: "application/pdf" }), diff --git a/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.ts b/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.ts index 9c6f5d9e4b..81663fbf69 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolApiMapping.ts @@ -47,26 +47,28 @@ function appendPrimitive( formData.append(key, value); } else if (typeof value === "number" || typeof value === "boolean") { formData.append(key, `${value}`); + } else if (typeof Blob !== "undefined" && value instanceof Blob) { + // A File upload (models type binary params as File): send it as the file part, not stringified. + formData.append(key, value); } else { - // A non-primitive here means a mapper produced a value the backend cannot - // receive as a form field. Fail loudly rather than silently drop it: - // structured fields must be JSON-encoded in the mapper, and Files passed via - // the `files` argument. + // Any other non-primitive means a mapper produced a value the backend cannot receive as a form + // field. Fail loudly rather than silently drop it: structured fields must be JSON-encoded first. throw new Error( `objectToFormData: field "${key}" has an unsupported value of type ` + - `"${typeof value}"; expected a string, number, or boolean.`, + `"${typeof value}"; expected a string, number, boolean, or File.`, ); } } /** * Serialize a backend request model (the output of a `toApiParams` function) - * into multipart FormData: primitives become string fields, arrays become - * repeated fields, and `undefined`/`null` are omitted. Files are appended - * separately via `files`, keeping file plumbing out of the parameter mapper. + * into multipart FormData: primitives become string fields, `File` values become + * file parts, arrays become repeated fields, and `undefined`/`null` are omitted. + * Extra files may still be passed via `files` (the primary `fileInput`, or a + * field the mapper doesn't carry). * - * Throws if a field holds a non-primitive value, since that cannot be sent as a - * form field: structured fields must be JSON-encoded by the mapper. + * Throws if a field holds any other non-primitive value, since that cannot be + * sent as a form field: structured fields must be JSON-encoded by the mapper. */ export function objectToFormData( params: ToolApiRequest, diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts index 5be4831907..0f7184f848 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts @@ -10,11 +10,14 @@ import { asRegistryConfig, ToolType, } from "@app/hooks/tools/shared/toolOperationTypes"; +import { objectToFormData } from "@app/hooks/tools/shared/toolApiMapping"; import { + activeFileFields, deserializeToolStep, + extractStepFiles, getExecutableTools, serializeToolStep, - stepRequiresUpload, + stepNeedsConfiguring, type WorkingToolStep, } from "@app/hooks/tools/shared/toolAutomation"; import { compressOperationConfig } from "@app/hooks/tools/compress/useCompressOperation"; @@ -28,6 +31,10 @@ import { addPasswordOperationConfig } from "@app/hooks/tools/addPassword/useAddP import { changePermissionsOperationConfig } from "@app/hooks/tools/changePermissions/useChangePermissionsOperation"; import { convertOperationConfig } from "@app/hooks/tools/convert/useConvertOperation"; import { defaultParameters as convertDefaults } from "@app/hooks/tools/convert/useConvertParameters"; +import { overlayPdfsOperationConfig } from "@app/hooks/tools/overlayPdfs/useOverlayPdfsOperation"; +import { defaultParameters as overlayDefaults } from "@app/hooks/tools/overlayPdfs/useOverlayPdfsParameters"; +import { certSignOperationConfig } from "@app/hooks/tools/certSign/useCertSignOperation"; +import { defaultParameters as certSignDefaults } from "@app/hooks/tools/certSign/useCertSignParameters"; function entry(over: Partial): ToolRegistryEntry { return { @@ -419,18 +426,167 @@ describe("convert (format-routed custom tool)", () => { }); }); -describe("stepRequiresUpload", () => { - const step = (params: Record): WorkingToolStep => ({ - toolId: "compress" as ToolId, - operation: "/api/v1/misc/compress-pdf", - params, +describe("supporting files", () => { + const fileRegistry: Partial = { + overlayPdfs: entry({ + name: "Overlay", + automationSettings: NoopSettings, + operationConfig: asRegistryConfig(overlayPdfsOperationConfig), + }), + certSign: entry({ + name: "Cert sign", + automationSettings: NoopSettings, + operationConfig: asRegistryConfig(certSignOperationConfig), + }), + }; + + const overlayStep = ( + params: Record, + fileParameters?: Record, + ): WorkingToolStep => ({ + toolId: "overlayPdfs" as ToolId, + operation: "/api/v1/general/overlay-pdfs", + params: { ...overlayDefaults, ...params }, support: "editable", + fileParameters, }); - test("detects a File (or list of Files) among the parameters", () => { - const image = new File(["x"], "logo.png", { type: "image/png" }); - expect(stepRequiresUpload(step({ level: 5 }))).toBe(false); - expect(stepRequiresUpload(step({ watermarkImage: image }))).toBe(true); - expect(stepRequiresUpload(step({ attachments: [image] }))).toBe(true); + const certStep = ( + params: Record, + fileParameters?: Record, + ): WorkingToolStep => ({ + toolId: "certSign" as ToolId, + operation: "/api/v1/security/cert-sign", + params: { ...certSignDefaults, signMode: "MANUAL", ...params }, + support: "editable", + fileParameters, + }); + + test("extractStepFiles groups fresh picks by their backend file field", () => { + const a = new File(["1"], "a.pdf", { type: "application/pdf" }); + const b = new File(["2"], "b.pdf", { type: "application/pdf" }); + expect( + extractStepFiles(overlayStep({ overlayFiles: [a, b] }), fileRegistry), + ).toEqual({ overlayFiles: [a, b] }); + }); + + test("extractStepFiles respects a tool's file selection (certSign by certType)", () => { + const p12 = new File(["k"], "key.p12"); + expect( + extractStepFiles( + certStep({ certType: "PKCS12", p12File: p12 }), + fileRegistry, + ), + ).toEqual({ p12File: [p12] }); + }); + + test("serialize/deserialize round-trips fileParameters", () => { + const step = certStep({ certType: "PKCS12" }, { p12File: "asset:abc" }); + const api = serializeToolStep(step, fileRegistry); + expect(api.fileParameters).toEqual({ p12File: "asset:abc" }); + expect(deserializeToolStep(api, fileRegistry).fileParameters).toEqual({ + p12File: "asset:abc", + }); + }); + + test("stepNeedsConfiguring: a stored binding satisfies the file requirement", () => { + expect( + stepNeedsConfiguring( + certStep({ certType: "PKCS12" }, { p12File: "asset:abc" }), + fileRegistry, + ), + ).toBe(false); + // Without the binding the keystore is still owed. + expect( + stepNeedsConfiguring(certStep({ certType: "PKCS12" }), fileRegistry), + ).toBe(true); + }); + + test("activeFileFields drops a stored binding the tool no longer emits", () => { + // Still PKCS12: the p12File binding is what the tool sends. + expect( + activeFileFields( + certStep({ certType: "PKCS12" }, { p12File: "asset:abc" }), + fileRegistry, + ), + ).toEqual(["p12File"]); + // Switched to PEM: certSign wants privateKeyFile/certFile, so the p12File binding is stale. + expect( + activeFileFields( + certStep({ certType: "PEM" }, { p12File: "asset:abc" }), + fileRegistry, + ), + ).toEqual([]); + }); + + test("activeFileFields is null (not empty) when the tool can't be probed", () => { + // A buildFormData that throws can't be probed; returning null (vs []) tells callers to keep the + // step's stored bindings rather than drop them and let the server GC the assets. + const config = asRegistryConfig<{ signingCert?: File }>({ + toolType: ToolType.singleFile, + operationType: "certSign", + endpoint: "/api/v1/security/cert-sign", + defaultParameters: {}, + buildFormData: () => { + throw new Error("cannot build"); + }, + }); + const registry: Partial = { + certSign: entry({ name: "Boom", operationConfig: config }), + }; + const step: WorkingToolStep = { + toolId: "certSign" as ToolId, + operation: "/api/v1/security/cert-sign", + params: {}, + support: "editable", + fileParameters: { certFile: "asset:x" }, + }; + expect(activeFileFields(step, registry)).toBeNull(); + }); + + test("the overlay sentinel is sized to the binding's asset count", () => { + // Two ids -> two files, matching two counts, so FixedRepeat validation passes. + const step = overlayStep( + { overlayMode: "FixedRepeatOverlay", counts: [1, 2] }, + { overlayFiles: "asset:one,two" }, + ); + expect(activeFileFields(step, fileRegistry)).toEqual(["overlayFiles"]); + expect(stepNeedsConfiguring(step, fileRegistry)).toBe(false); + }); + + test("a rename override binds a backend field to a differently-named param", () => { + // The cert-sign endpoint's `certFile` is held by a frontend param named `signingCert`. + const config = asRegistryConfig<{ signingCert?: File }>({ + toolType: ToolType.singleFile, + operationType: "certSign", + endpoint: "/api/v1/security/cert-sign", + defaultParameters: {}, + validateParams: (p) => p.signingCert !== undefined, + // Sends the File under the backend field `certFile`, like real tools do via objectToFormData + // (which sends a param's File or File[] under a named field, iterating arrays). + buildFormData: (p, file) => + objectToFormData({}, { fileInput: file, certFile: p.signingCert }), + fileParamOverrides: [{ field: "certFile", param: "signingCert" }], + }); + const registry: Partial = { + certSign: entry({ name: "Sign", operationConfig: config }), + }; + const step = ( + fileParameters?: Record, + ): WorkingToolStep => ({ + toolId: "certSign" as ToolId, + operation: "/api/v1/security/cert-sign", + params: {}, + support: "editable", + fileParameters, + }); + // The stored binding is keyed by the backend field, but satisfies the frontend param on reload. + expect(stepNeedsConfiguring(step({ certFile: "asset:x" }), registry)).toBe( + false, + ); + expect(stepNeedsConfiguring(step(), registry)).toBe(true); + expect(activeFileFields(step({ certFile: "asset:x" }), registry)).toEqual([ + "certFile", + ]); }); }); diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts index 0485791f15..a9368ccdb7 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts @@ -18,11 +18,13 @@ import { type ToolRegistryEntry, } from "@app/data/toolsTaxonomy"; import { type ToolId } from "@app/types/toolId"; +import { TOOL_FILE_FIELDS } from "@app/types/toolApiTypes"; import { isToolEndpoint, type ToolEndpoint, } from "@app/hooks/tools/shared/toolApiMapping"; import { + ToolType, type ErasedToolParams, type RegistryToolOperationConfig, } from "@app/hooks/tools/shared/toolOperationTypes"; @@ -62,6 +64,12 @@ export interface ExecutableTool { export interface ToolApiStep { operation: string; parameters: Record; + /** + * Supporting-file bindings: a backend file field (e.g. `stampImage`, `overlayFiles`) mapped to + * `asset:[,]` (stored supporting files) or a run-supplied key. Absent when the step needs + * no supporting file. Mirrors the wire {@code PipelineStep.fileParameters}. + */ + fileParameters?: SupportingFileBindings; } /** A step being edited in a UI that maps to a known tool: parameters are in the tool's frontend shape. */ @@ -70,6 +78,12 @@ export interface KnownToolStep { operation: ToolEndpoint; params: ErasedToolParams; support: ToolStepSupport; + /** + * Stored supporting-file bindings carried from a saved step (field -> `asset:`), so an edit + * round-trips them without the user re-picking. A field the user re-picks lands in `params` as a + * File and takes precedence on save. + */ + fileParameters?: SupportingFileBindings; } /** A stored step whose endpoint maps to no known tool: preserved verbatim, not editable. */ @@ -78,6 +92,8 @@ export interface UnknownToolStep { operation: string; params: ErasedToolParams; support: "unknown"; + /** Supporting-file bindings preserved verbatim, so an unknown step's files round-trip untouched. */ + fileParameters?: SupportingFileBindings; } /** A step being edited in a UI, discriminated by whether its endpoint maps to a known tool. */ @@ -135,12 +151,176 @@ function isFileValue(value: unknown): boolean { } /** - * True if any of a step's parameters is an uploaded file (or list of files). Such a step cannot be - * saved into a stored pipeline yet: the file bytes are not persisted with the policy, so a later - * (e.g. scheduled) run would have nothing to send for that named file field. + * A stored supporting-file id, as returned by the asset store. */ -export function stepRequiresUpload(step: WorkingToolStep): boolean { - return Object.values(step.params).some(isFileValue); +declare const ASSET_ID_BRAND: unique symbol; +export type AssetId = string & { readonly [ASSET_ID_BRAND]: never }; + +/** + * A step's supporting-file bindings: each backend file field (e.g. `stampImage`) mapped to its file. + * A value of `asset:[,]` names stored assets loaded at run time; any other value is a key for + * a file supplied with the run itself. + */ +export type SupportingFileBindings = Record; + +/** + * The `fileParameters` binding format shared with the backend (see PolicyAssetRefs). This module owns + * the frontend side of the step contract, so the format lives here and the builder/settings reuse it. + */ +export const ASSET_REF_PREFIX = "asset:"; + +/** A `fileParameters` value binding one tool file field to the given stored asset ids. */ +export function assetRef(ids: readonly AssetId[]): string { + return ASSET_REF_PREFIX + ids.join(","); +} + +/** The stored asset ids inside a binding value, or none when it isn't an `asset:` ref. */ +export function assetRefIds(binding: string): AssetId[] { + if (!binding.startsWith(ASSET_REF_PREFIX)) return []; + return binding + .slice(ASSET_REF_PREFIX.length) + .split(",") + .map((id) => id.trim()) + .filter(Boolean) as AssetId[]; +} + +/** A throwaway primary document for probing a tool's buildFormData; never sent anywhere. */ +function dummyPrimaryFile(): File { + return new File([], "input.pdf", { type: "application/pdf" }); +} + +/** + * Run a tool's buildFormData so we can read the request it would produce. + * Returns null when File is unavailable or buildFormData throws. + */ +function probeFormData( + config: RegistryToolOperationConfig, + params: ErasedToolParams, +): FormData | null { + if (typeof File === "undefined") return null; + const dummy = dummyPrimaryFile(); + try { + switch (config.toolType) { + case ToolType.singleFile: + return config.buildFormData(params, dummy); + case ToolType.multiFile: + return config.buildFormData(params, [dummy]); + default: + return null; + } + } catch { + return null; + } +} + +/** Defaults merged under the step's params - the shape a tool's mappers and buildFormData expect. */ +function mergedStepParams( + step: WorkingToolStep, + config: RegistryToolOperationConfig, +): ErasedToolParams { + return { ...(config.defaultParameters ?? {}), ...step.params }; +} + +/** The backend file fields an endpoint accepts, from the generated spec-sourced table. */ +function backendFileFields(operation: string): readonly string[] { + return ( + (TOOL_FILE_FIELDS as Partial>)[ + operation + ] ?? [] + ); +} + +/** + * Each backend file field the step's endpoint accepts (from {@link TOOL_FILE_FIELDS}), mapped to the + * tool param that holds it - the same name unless the tool declared a rename override. + */ +function fileFieldMappings( + operation: string, + config: RegistryToolOperationConfig, +): { field: string; param: string }[] { + // The override's erased type collapses `param` to `never`; restore the real runtime shape. + const overrides = (config.fileParamOverrides ?? []) as readonly { + field: string; + param: string; + }[]; + const paramByField = new Map(overrides.map((o) => [o.field, o.param])); + return backendFileFields(operation).map((field) => ({ + field, + param: paramByField.get(field) ?? field, + })); +} + +/** + * The step's params with a stand-in File array injected for each stored binding whose param has no + * fresh pick, so a tool's buildFormData/validateParams sees the supporting file as present. Stored + * bindings are keyed by the backend field (from {@link TOOL_FILE_FIELDS}), so each field finds its + * binding and the sentinel lands on its param - the two coincide unless the tool declared a rename + * override. The array is sized to the binding's asset count (overlay validates count == file count). + * Sentinels are empty and live only in this local object - never written back to step.params, so they + * can never be uploaded. + */ +function withStoredFileSentinels( + step: WorkingToolStep, + config: RegistryToolOperationConfig, +): ErasedToolParams { + const merged = mergedStepParams(step, config); + const bindings = step.fileParameters; + if (!bindings || typeof File === "undefined") return merged; + for (const { param, field } of fileFieldMappings(step.operation, config)) { + const binding = bindings[field]; + if (binding == null || isFileValue(merged[param])) continue; // unbound, or a fresh pick stands in + const count = Math.max(1, assetRefIds(binding).length); + merged[param] = Array.from({ length: count }, () => new File([], "stored")); + } + return merged; +} + +/** + * The fresh File picks on a step, grouped by the backend file field its buildFormData sends them + * under (excluding the primary `fileInput`). buildFormData is the source of truth for the field name + * and for tool-specific selection (certSign picks files by certType), so probing it - rather than + * scanning params - keeps the field mapping correct. These are the files to upload on save. + */ +export function extractStepFiles( + step: WorkingToolStep, + registry: Partial, +): Record { + if (step.toolId === null) return {}; + const config = registry[step.toolId]?.operationConfig; + if (!config) return {}; + const formData = probeFormData(config, mergedStepParams(step, config)); + if (!formData) return {}; + const files: Record = {}; + formData.forEach((value, key) => { + if (key !== "fileInput" && value instanceof File) { + (files[key] ??= []).push(value); + } + }); + return files; +} + +/** + * The backend file fields this step actually uses right now, per its own buildFormData: fresh picks + * plus any stored binding the tool still emits (a stale one - e.g. a PKCS12 keystore after switching + * to PEM - is dropped, because buildFormData no longer sends it). Drives the stored-file chips, the + * save-time binding set, and the test run. + */ +export function activeFileFields( + step: WorkingToolStep, + registry: Partial, +): string[] | null { + if (step.toolId === null) { + return step.fileParameters ? Object.keys(step.fileParameters) : []; + } + const config = registry[step.toolId]?.operationConfig; + if (!config) return null; + const formData = probeFormData(config, withStoredFileSentinels(step, config)); + if (!formData) return null; + const fields = new Set(); + formData.forEach((value, key) => { + if (key !== "fileInput" && value instanceof File) fields.add(key); + }); + return [...fields]; } /** @@ -158,9 +338,10 @@ export function stepNeedsConfiguring( ): boolean { if (step.toolId === null) return false; const config = registry[step.toolId]?.operationConfig; - if (!config?.validateParams) return false; - const merged = { ...(config.defaultParameters ?? {}), ...step.params }; - return !config.validateParams(merged); + if (!config || !config.validateParams) return false; + // Stored supporting files satisfy their field just as a fresh pick would, so validate against the + // sentinel-injected params rather than the bare ones (which drop the file on reload). + return !config.validateParams(withStoredFileSentinels(step, config)); } /** @@ -240,14 +421,27 @@ export function serializeToolStep( step.toolId !== null ? registry[step.toolId]?.operationConfig : undefined; if (!config) { // Unmapped step (unknown endpoint on edit): round-trip it unchanged. - return { operation: step.operation, parameters: step.params }; + return withFileParameters( + { operation: step.operation, parameters: step.params }, + step, + ); } const merged = { ...(config.defaultParameters ?? {}), ...step.params }; const operation = resolveEndpoint(config, merged) ?? step.operation; const parameters = config.toApiParams ? (config.toApiParams(merged) as Record) : {}; - return { operation, parameters }; + return withFileParameters({ operation, parameters }, step); +} + +/** Attach the step's supporting-file bindings to a serialized step, omitting the field when empty. */ +function withFileParameters( + serialized: ToolApiStep, + step: WorkingToolStep, +): ToolApiStep { + const bindings = step.fileParameters; + if (!bindings || Object.keys(bindings).length === 0) return serialized; + return { ...serialized, fileParameters: bindings }; } /** @@ -308,6 +502,7 @@ function unmappedStep(step: ToolApiStep): UnknownToolStep { operation: step.operation, params: { ...step.parameters }, support: "unknown", + fileParameters: step.fileParameters, }; } @@ -345,5 +540,11 @@ export function deserializeToolStep( resolveEndpoint(config, params) ?? (isToolEndpoint(step.operation) ? step.operation : undefined); if (operation === undefined) return unmappedStep(step); - return { toolId, operation, params, support: classifyToolStepSupport(entry) }; + return { + toolId, + operation, + params, + support: classifyToolStepSupport(entry), + fileParameters: step.fileParameters, + }; } diff --git a/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts b/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts index d6dbf3b355..d01d897002 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts @@ -3,7 +3,11 @@ import { StirlingFile } from "@app/types/fileContext"; import type { ResponseHandler } from "@app/utils/toolResponseProcessor"; import { ToolId } from "@app/types/toolId"; import type { ProcessingProgress } from "@app/hooks/tools/shared/useToolState"; -import type { ToolApiParams, ToolEndpoint } from "@app/types/toolApiTypes"; +import { + TOOL_FILE_FIELDS, + type ToolApiParams, + type ToolEndpoint, +} from "@app/types/toolApiTypes"; export type { ProcessingProgress, ResponseHandler }; @@ -45,6 +49,39 @@ export interface CustomProcessorResult { consumedAllInputs?: boolean; } +/** + * The parameter keys that carry a supporting file - a `File` or `File[]` value the tool sends + * beyond its primary document. Derived from the tool's own parameter type, so a file field can only + * ever be declared against a param that genuinely holds a file. + */ +export type FileParamKey = { + [K in keyof TParams]-?: NonNullable extends File | File[] + ? K + : never; +}[keyof TParams] & + string; + +/** + * The backend multipart file fields an endpoint accepts, from the generated {@link TOOL_FILE_FIELDS} + * (which the spec derives from the Java MultipartFile params). `never` for an endpoint that takes no + * supporting files. This is what makes a rename override's `field` a checked name, not a free string. + */ +export type BackendFileField = + TEndpoint extends keyof typeof TOOL_FILE_FIELDS + ? (typeof TOOL_FILE_FIELDS)[TEndpoint][number] + : never; + +/** + * A remap for the rare case where a tool's frontend file param has a different name from the backend + * field it is sent under. Both sides are checked: `field` must be one of the endpoint's generated + * backend file fields, and `param` a real file param of the tool. Same-name fields need no entry - + * they are derived from {@link TOOL_FILE_FIELDS} directly. + */ +export interface FileParamOverride { + field: BackendFileField; + param: FileParamKey; +} + /** * Configuration for tool operations defining processing behavior and API integration. * @@ -79,6 +116,14 @@ interface BaseToolOperationConfig { /** Default parameter values for automation */ defaultParameters?: TParams; + /** + * Rename overrides for supporting-file params. The set of a tool's file fields is derived from the + * generated {@link TOOL_FILE_FIELDS} (spec-sourced), keyed by the backend field name; declare an + * override only when a backend field maps to a differently-named frontend param, so a step composer + * can bind the stored file to the right param. Omitted by the common case where field == param. + */ + fileParamOverrides?: readonly FileParamOverride[]; + /** * Whether these parameters are complete enough to run. The same predicate a tool gives * `useBaseParameters` as its `validateFn`, so the Run button in the editor and anything composing diff --git a/frontend/editor/src/core/types/toolApiTypes.ts b/frontend/editor/src/core/types/toolApiTypes.ts index bf498eb450..b2bdf7fb4b 100644 --- a/frontend/editor/src/core/types/toolApiTypes.ts +++ b/frontend/editor/src/core/types/toolApiTypes.ts @@ -7,7 +7,7 @@ export interface AddAttachmentRequest { /** * The image file to be overlaid onto the PDF. */ - attachments: string[]; + attachments: File[]; /** * Convert the resulting PDF to PDF/A-3b format after adding attachments */ @@ -148,7 +148,7 @@ export interface AddStampRequest { * The rotation of the stamp in degrees */ rotation?: number; - stampImage?: string; + stampImage?: File; /** * The stamp text */ @@ -187,7 +187,7 @@ export interface AddWatermarkRequest { * The rotation of the watermark in degrees */ rotation?: number; - watermarkImage?: string; + watermarkImage?: File; /** * The watermark text */ @@ -525,9 +525,7 @@ export interface FlattenRequest { */ renderDpi?: number; } -export interface GeneralExtractBookmarksRequest { - file: string; -} +export type GeneralExtractBookmarksRequest = Record; export type GeneralFile = Record; export type GeneralPdfToSinglePageRequest = Record; export type GeneralRemoveImagePdfRequest = Record; @@ -788,7 +786,7 @@ export interface OverlayImageRequest { * Whether to overlay the image onto every page of the PDF. */ everyPage?: boolean; - imageFile: string; + imageFile: File; /** * The x-coordinate at which to place the top-left corner of the image. */ @@ -806,7 +804,7 @@ export interface OverlayPdfsRequest { /** * An array of PDF files to be used as overlays on the base PDF. The order in these files is applied based on the selected mode. */ - overlayFiles: string[]; + overlayFiles: File[]; /** * The mode of overlaying: 'SequentialOverlay' for sequential application, 'InterleavedOverlay' for round-robin application, 'FixedRepeatOverlay' for fixed repetition based on provided counts */ @@ -1276,7 +1274,6 @@ export interface ScannerEffectRequest { yellowish?: boolean; } export interface SecurityCertSignSessionsRequest { - file: string; request?: WorkflowCreationRequest; } export interface WorkflowCreationRequest { @@ -1291,8 +1288,8 @@ export interface WorkflowCreationRequest { } export interface SecurityCertSignValidateCertificateRequest { certType: string; - jksFile?: string; - p12File?: string; + jksFile?: File; + p12File?: File; password?: string; } export type SecurityGetInfoOnPdfRequest = Record; @@ -1302,7 +1299,7 @@ export interface SignPDFWithCertRequest { * The alias of the certificate to sign with. Required for WINDOWS_STORE and recommended for PKCS11 tokens holding multiple certificates. */ alias?: string; - certFile?: string; + certFile?: File; /** * The type of the digital certificate. WINDOWS_STORE and PKCS11 are hardware-backed and only available in the desktop app. */ @@ -1314,7 +1311,7 @@ export interface SignPDFWithCertRequest { | "SERVER" | "WINDOWS_STORE" | "PKCS11"; - jksFile?: string; + jksFile?: File; /** * The location where the PDF is signed */ @@ -1323,7 +1320,7 @@ export interface SignPDFWithCertRequest { * The name of the signer */ name?: string; - p12File?: string; + p12File?: File; /** * The page number where the signature should be visible. This is required if showSignature is set to true */ @@ -1340,7 +1337,7 @@ export interface SignPDFWithCertRequest { * Optional PKCS#11 slot index. When omitted the first slot with a token is used. */ pkcs11Slot?: number; - privateKeyFile?: string; + privateKeyFile?: File; /** * The reason for signing the PDF */ @@ -1355,7 +1352,7 @@ export interface SignPDFWithCertRequest { showSignature?: boolean; } export interface SignatureValidationRequest { - certFile?: string; + certFile?: File; } export interface SplitPagesRequest { /** @@ -1741,5 +1738,22 @@ export const TOOL_ENDPOINTS = [ "/api/v1/security/verify-pdf", ] as const satisfies readonly ToolEndpoint[]; +/** The supporting-file parameters each endpoint accepts beyond its primary fileInput, by name. */ +export const TOOL_FILE_FIELDS = { + "/api/v1/general/overlay-pdfs": ["overlayFiles"], + "/api/v1/misc/add-attachments": ["attachments"], + "/api/v1/misc/add-image": ["imageFile"], + "/api/v1/misc/add-stamp": ["stampImage"], + "/api/v1/security/add-watermark": ["watermarkImage"], + "/api/v1/security/cert-sign": [ + "privateKeyFile", + "certFile", + "p12File", + "jksFile", + ], + "/api/v1/security/cert-sign/validate-certificate": ["p12File", "jksFile"], + "/api/v1/security/validate-signature": ["certFile"], +} as const satisfies Partial>; + /** Union of every generated tool request model. */ export type ToolApiRequest = ToolApiParams[ToolEndpoint]; diff --git a/frontend/editor/src/portal/api/pipelineAssets.ts b/frontend/editor/src/portal/api/pipelineAssets.ts new file mode 100644 index 0000000000..b68c95fda5 --- /dev/null +++ b/frontend/editor/src/portal/api/pipelineAssets.ts @@ -0,0 +1,41 @@ +import { apiClient } from "@portal/api/http"; +import { type AssetId } from "@app/hooks/tools/shared/toolAutomation"; + +export { type AssetId }; + +/** + * Stored supporting files for pipeline steps (backend PolicyAssetController). + * + * A pipeline step that needs more than the document stream - a signing + * certificate, a watermark/stamp image, overlay PDFs, attachments - references + * its file by id from the step's `fileParameters` as `asset:`. The bytes are + * uploaded here first (the save-time validator rejects a policy that binds an + * asset id that doesn't yet exist), then a triggered or scheduled run loads the + * file server-side without anyone re-supplying it. Assets are team-scoped exactly + * like the policies that reference them, and unreferenced uploads are cleaned up + * server-side, so the builder never has to delete what a cancelled edit left. + */ + +/** Metadata for one stored supporting file. Mirrors the Java `PolicyAsset` record. */ +export interface PolicyAsset { + id: AssetId; + fileName: string; + contentType: string | null; + size: number; + createdAt: number; +} + +/** POST /api/v1/policies/assets: store a supporting file, returning its metadata (with the id). */ +export async function uploadPipelineAsset(file: File): Promise { + const form = new FormData(); + form.append("file", file); + return apiClient.local.multipart( + "/api/v1/policies/assets", + form, + ); +} + +/** GET /api/v1/policies/assets: the team's stored supporting files (metadata only). */ +export async function listPipelineAssets(): Promise { + return apiClient.local.json("/api/v1/policies/assets"); +} diff --git a/frontend/editor/src/portal/api/pipelines.ts b/frontend/editor/src/portal/api/pipelines.ts index 50bdc173c4..e441fcdac8 100644 --- a/frontend/editor/src/portal/api/pipelines.ts +++ b/frontend/editor/src/portal/api/pipelines.ts @@ -1,5 +1,8 @@ import { apiClient } from "@portal/api/http"; -import { type ToolApiStep } from "@app/hooks/tools/shared/toolAutomation"; +import { + type SupportingFileBindings, + type ToolApiStep, +} from "@app/hooks/tools/shared/toolAutomation"; /** * Pipelines service layer: the backend contract. @@ -15,7 +18,7 @@ import { type ToolApiStep } from "@app/hooks/tools/shared/toolAutomation"; export interface PipelineStep { operation: string; parameters: Record; - fileParameters?: Record; + fileParameters?: SupportingFileBindings; } /** When a policy input fires automatically. `type` keys a trigger bean (e.g. "schedule"). */ @@ -217,14 +220,28 @@ export interface TestRunDefinition { output: OutputSpec; } +/** + * A fresh, in-memory supporting file sent inline with a test run, bound to the run key a test step's + * `fileParameters` references. Only unsaved picks ride along here; a stored file keeps its + * `asset:` binding, which the backend resolves from the saved policy (see `runPipelineTest`). + */ +export interface TestRunAsset { + key: string; + file: File; +} + /** * POST /api/v1/policies/run: run a definition against one uploaded file now. The builder's test * path - callers force an inline output so nothing reaches the pipeline's real destination, and - * the pipeline need not be saved first. + * the pipeline need not be saved first. Fresh supporting files travel as keyed `assets[i]` parts; + * a stored file keeps its `asset:` binding, and `policyId` lets the backend resolve it from that + * saved policy (so its bytes need not be re-sent). */ export async function runPipelineTest( definition: TestRunDefinition, file: File, + assets: TestRunAsset[] = [], + policyId?: string, ): Promise<{ runId: string }> { const form = new FormData(); form.append( @@ -232,6 +249,11 @@ export async function runPipelineTest( new Blob([JSON.stringify(definition)], { type: "application/json" }), ); form.append("fileInput", file); + if (policyId) form.append("policyId", policyId); + assets.forEach((asset, i) => { + form.append(`assets[${i}].key`, asset.key); + form.append(`assets[${i}].file`, asset.file); + }); // The POST returns the identifier as `jobId`, but it is the same run id every other endpoint // (fetchRun, fetchRunOutput) calls `runId`; normalise to that here so callers see one name. const res = await apiClient.local.multipart<{ jobId: string }>( diff --git a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.css b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.css new file mode 100644 index 0000000000..46bb72a736 --- /dev/null +++ b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.css @@ -0,0 +1,17 @@ +.portal-step-settings__files { + display: flex; + flex-direction: column; + gap: 0.375rem; + margin-bottom: 0.75rem; +} + +.portal-step-settings__files-label { + font-size: 0.75rem; + color: var(--c-text-muted); +} + +.portal-step-settings__files-chips { + display: flex; + flex-wrap: wrap; + gap: 0.375rem; +} diff --git a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.stories.tsx b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.stories.tsx index d2bd6bc6f2..823d0ae4fe 100644 --- a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.stories.tsx +++ b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.stories.tsx @@ -52,6 +52,8 @@ const meta = { step: editableStep, registry, onChange: () => {}, + assetNames: {}, + onClearBinding: () => {}, }, } satisfies Meta; export default meta; diff --git a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx index 55a52b496e..0b15b7233a 100644 --- a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx +++ b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.test.tsx @@ -116,6 +116,8 @@ describe("PipelineStepSettings", () => { step={step} registry={registry} onChange={() => {}} + assetNames={{}} + onClearBinding={() => {}} /> , ), @@ -131,6 +133,8 @@ describe("PipelineStepSettings", () => { step={convertStep} registry={convertRegistry} onChange={() => {}} + assetNames={{}} + onClearBinding={() => {}} /> , ), @@ -146,6 +150,8 @@ describe("PipelineStepSettings", () => { step={changeMetadataStep} registry={changeMetadataRegistry} onChange={() => {}} + assetNames={{}} + onClearBinding={() => {}} /> , ), @@ -161,6 +167,8 @@ describe("PipelineStepSettings", () => { step={overlayStep} registry={overlayRegistry} onChange={() => {}} + assetNames={{}} + onClearBinding={() => {}} /> , ), @@ -205,6 +213,8 @@ describe("PipelineStepSettings", () => { typeof update === "function" ? update(prev) : update, ) } + assetNames={{}} + onClearBinding={() => {}} /> {JSON.stringify(params)} diff --git a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.tsx b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.tsx index 58fb2a96fa..3d17dbf652 100644 --- a/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.tsx +++ b/frontend/editor/src/portal/components/pipelines/PipelineStepSettings.tsx @@ -1,15 +1,22 @@ import { Suspense } from "react"; import { useTranslation } from "react-i18next"; -import { Banner } from "@app/ui"; +import InsertDriveFileOutlinedIcon from "@mui/icons-material/InsertDriveFileOutlined"; +import { Banner, Chip } from "@app/ui"; import { PreferencesProvider } from "@app/contexts/PreferencesContext"; import { SidebarProvider } from "@app/contexts/SidebarContext"; import { type ToolRegistry } from "@app/data/toolsTaxonomy"; import { type ErasedToolParams } from "@app/hooks/tools/shared/toolOperationTypes"; -import { type WorkingToolStep } from "@app/hooks/tools/shared/toolAutomation"; +import { + activeFileFields, + assetRefIds, + extractStepFiles, + type WorkingToolStep, +} from "@app/hooks/tools/shared/toolAutomation"; import { PolicyExternalApiConfig } from "@portal/components/policies/PolicyExternalApiConfig"; import { isIntegrationStep } from "@portal/components/pipelines/integrationStep"; import type { ExternalApiStepParams } from "@portal/components/policies/stepOperations"; +import "@portal/components/pipelines/PipelineStepSettings.css"; /** * A params update: the next params outright, or a merge from the latest params. Settings UIs fire @@ -25,17 +32,61 @@ interface PipelineStepSettingsProps { step: WorkingToolStep; registry: Partial; onChange: (update: ParamsUpdate) => void; + /** Stored asset id -> file name, for labelling the supporting-file chips on a reopened pipeline. */ + assetNames: Record; + /** Drop a field's stored supporting-file binding (the user re-picks a file if the step still needs one). */ + onClearBinding: (field: string) => void; +} + +/** One reopened supporting file shown as a chip: the field it binds and the stored file name(s). */ +interface StoredFileChip { + field: string; + label: string; +} + +/** + * The supporting files this step is reusing from a previous save: an active binding whose field has + * no fresh pick (a fresh pick shows in the tool's own file picker instead). Labelled by the resolved + * asset name so the user sees "using cert.pfx" rather than an empty picker. + */ +function storedFileChips( + step: WorkingToolStep, + registry: Partial, + assetNames: Record, +): StoredFileChip[] { + const bindings = step.fileParameters; + if (!bindings) return []; + // A null active set means the tool couldn't be probed; show every stored binding rather than hide + // the user's files (mirrors the save path, which keeps them too). + const active = activeFileFields(step, registry); + const activeSet = active === null ? null : new Set(active); + const fresh = extractStepFiles(step, registry); + return Object.entries(bindings) + .filter( + ([field]) => + (activeSet === null || activeSet.has(field)) && !fresh[field], + ) + .map(([field, binding]) => ({ + field, + label: + assetRefIds(binding) + .map((id) => assetNames[id] ?? id) + .join(", ") || binding, + })); } /** * Renders the parameter editor for one pipeline step, chosen by the tool's capability: * the tool's own settings UI when editable, an explanatory note when it has no parameters, - * or a "not supported yet" fallback for tools not yet migrated to the mapper seam. + * or a "not supported yet" fallback for tools not yet migrated to the mapper seam. Reopened + * supporting files appear as removable chips above the tool's own settings. */ export function PipelineStepSettings({ step, registry, onChange, + assetNames, + onClearBinding, }: PipelineStepSettingsProps) { // Hooks first: selecting a different step re-renders this same instance, so an early return // above useTranslation would change the hook count between renders and crash. @@ -52,41 +103,70 @@ export function PipelineStepSettings({ ); } - if (step.support === "noSettings") { - return ( - - ); - } + const chips = storedFileChips(step, registry, assetNames); - const entry = step.toolId ? registry[step.toolId] : undefined; - const Settings = - step.support === "editable" ? entry?.automationSettings : null; - - if (!Settings) { + function toolBody() { + if (step.support === "noSettings") { + return ( + + ); + } + const entry = step.toolId ? registry[step.toolId] : undefined; + const Settings = + step.support === "editable" ? entry?.automationSettings : null; + if (!Settings) { + return ( + + ); + } return ( - + + + + + onChange((prev) => ({ ...prev, [key]: value })) + } + disabled={false} + /> + + + ); } return ( - - - - - onChange((prev) => ({ ...prev, [key]: value })) - } - disabled={false} - /> - - - + <> + {chips.length > 0 && ( +
+ + {t("portal.pipelines.builder.supportingFiles")} + +
+ {chips.map((chip) => ( + + } + onRemove={() => onClearBinding(chip.field)} + > + {chip.label} + + ))} +
+
+ )} + {toolBody()} + ); } diff --git a/frontend/editor/src/portal/mocks/handlers/pipelines.ts b/frontend/editor/src/portal/mocks/handlers/pipelines.ts index ed7cec1626..c5505f38ff 100644 --- a/frontend/editor/src/portal/mocks/handlers/pipelines.ts +++ b/frontend/editor/src/portal/mocks/handlers/pipelines.ts @@ -116,6 +116,21 @@ function nextId(): string { return `plc_${Date.now().toString(36)}_${idCounter}`; } +/** Stored supporting files a step binds as `asset:` (PolicyAssetController), for mock mode. */ +interface StoredAsset { + id: string; + fileName: string; + contentType: string | null; + size: number; + createdAt: number; +} +let assetStore: StoredAsset[] = []; +let assetCounter = 0; +function nextAssetId(): string { + assetCounter += 1; + return `ast_${Date.now().toString(36)}_${assetCounter}`; +} + function deriveStatus(policy: StoredPolicy): PipelineStatus { return policy.enabled ? "active" : "paused"; } @@ -197,6 +212,33 @@ export const pipelinesHandlers = [ ]); }), + // Supporting files. Registered before the `/policies/:id` matcher so "assets" isn't read as an id. + http.get("/api/v1/policies/assets", async () => { + await delay(80); + return HttpResponse.json(assetStore); + }), + + http.post("/api/v1/policies/assets", async ({ request }) => { + const form = await request.formData(); + const file = form.get("file"); + if (!(file instanceof File)) { + return HttpResponse.json( + { detail: "Uploaded file is empty" }, + { status: 400 }, + ); + } + await delay(120); + const asset: StoredAsset = { + id: nextAssetId(), + fileName: file.name || "asset", + contentType: file.type || null, + size: file.size, + createdAt: Date.now(), + }; + assetStore = [...assetStore, asset]; + return HttpResponse.json(asset); + }), + // Run status: the mock completes runs immediately, so polling resolves at once. http.get("/api/v1/policies/run/:runId", async ({ params }) => { await delay(120); diff --git a/frontend/editor/src/portal/views/PipelineBuilder.test.tsx b/frontend/editor/src/portal/views/PipelineBuilder.test.tsx index 52fb5d516c..2737411c74 100644 --- a/frontend/editor/src/portal/views/PipelineBuilder.test.tsx +++ b/frontend/editor/src/portal/views/PipelineBuilder.test.tsx @@ -43,6 +43,13 @@ vi.mock("@portal/api/pipelines", () => ({ fetchRun: (runId: string) => fetchRun(runId), })); +const uploadPipelineAsset = vi.fn(); +const listPipelineAssets = vi.fn(); +vi.mock("@portal/api/pipelineAssets", () => ({ + uploadPipelineAsset: (file: File) => uploadPipelineAsset(file), + listPipelineAssets: () => listPipelineAssets(), +})); + const fetchSources = vi.fn(); vi.mock("@portal/api/sources", () => ({ fetchSources: () => fetchSources(), @@ -149,8 +156,21 @@ vi.mock("@app/contexts/ToolRegistryContext", () => { toolType: 0, endpoint: "/api/v1/misc/compress-pdf", defaultParameters: {}, - buildFormData: () => new FormData(), - toApiParams: (params: Record) => ({ ...params }), + // Sends the supporting file under a named field, like a real file tool, so the upload path + // has a field to bind. The scalar mapper drops the File (files never ride in parameters). + buildFormData: (params: Record, file: File | File[]) => { + const fd = new FormData(); + fd.append("fileInput", Array.isArray(file) ? file[0] : file); + if (params.watermarkImage instanceof File) { + fd.append("watermarkImage", params.watermarkImage); + } + return fd; + }, + toApiParams: (params: Record) => { + const scalars = { ...params }; + delete scalars.watermarkImage; + return scalars; + }, fromApiParams: (params: Record) => ({ ...params }), }, } as unknown as ToolRegistryEntry; @@ -203,10 +223,32 @@ vi.mock("@app/contexts/ToolRegistryContext", () => { fromApiParams: (params: Record) => ({ ...params }), }, } as unknown as ToolRegistryEntry; + // A tool whose buildFormData throws, so it can't be probed: exercises the "activeFileFields is + // null" path where a reopened step's stored binding must be kept, not dropped. + const sign = { + name: "Sign", + icon: null, + component: null, + description: "", + categoryId: "recommendedTools", + subcategoryId: "general", + operationConfig: { + operationType: "certSign", + toolType: 0, + endpoint: "/api/v1/security/cert-sign", + defaultParameters: {}, + buildFormData: () => { + throw new Error("cannot build"); + }, + toApiParams: (params: Record) => ({ ...params }), + fromApiParams: (params: Record) => ({ ...params }), + }, + } as unknown as ToolRegistryEntry; const allTools = { compress, extractImages, ocr, + sign, } as unknown as ToolRegistryCatalog["allTools"]; const catalog: ToolRegistryCatalog = { regularTools: allTools, @@ -288,6 +330,16 @@ describe("PipelineBuilder", () => { fetchS3Connections.mockReset(); fetchS3Connections.mockResolvedValue([]); createIntegration.mockReset(); + uploadPipelineAsset.mockReset(); + uploadPipelineAsset.mockResolvedValue({ + id: "ast-1", + fileName: "logo.png", + contentType: "image/png", + size: 1, + createdAt: 0, + }); + listPipelineAssets.mockReset(); + listPipelineAssets.mockResolvedValue([]); }); // The settings of a node are reached by selecting it in the graph, so every helper below opens @@ -798,7 +850,7 @@ describe("PipelineBuilder", () => { ).toBeInTheDocument(); }); - it("blocks saving a step that needs an uploaded file", async () => { + it("uploads a step's supporting file and saves it as an asset binding", async () => { renderBuilder("/processor/pipelines/new"); fireEvent.change( @@ -810,15 +862,65 @@ describe("PipelineBuilder", () => { }, ); await addTool("Compress"); - // The tool's settings upload a file, which a stored pipeline can't persist yet. + // The tool's settings attach a supporting file. fireEvent.click(await screen.findByText("upload logo")); - expect( - await screen.findByText("portal.pipelines.builder.uploadUnsupported"), - ).toBeInTheDocument(); - expect( - screen.getByText("portal.pipelines.composer.create").closest("button"), - ).toBeDisabled(); + await pickInputSource("Claims intake"); + await pickDestination(); + + fireEvent.click(screen.getByText("portal.pipelines.composer.create")); + + // The file is uploaded to the asset store first, then the policy is saved binding that asset. + await waitFor(() => expect(uploadPipelineAsset).toHaveBeenCalledTimes(1)); + expect(uploadPipelineAsset.mock.calls[0][0]).toBeInstanceOf(File); + await waitFor(() => expect(savePipeline).toHaveBeenCalledTimes(1)); + expect(savePipeline).toHaveBeenCalledWith( + expect.objectContaining({ + steps: [ + expect.objectContaining({ + operation: "/api/v1/misc/compress-pdf", + fileParameters: { watermarkImage: "asset:ast-1" }, + }), + ], + }), + ); + }); + + it("keeps a step's stored file binding on save when the tool can't be probed", async () => { + // buildFormData throws for `sign`, so activeFileFields is null. The stored binding must survive + // the save unchanged - dropping it would let the server GC the user's uploaded file - and no + // re-upload should happen. + fetchPipeline.mockResolvedValue({ + id: "plc-sign", + name: "Signed", + enabled: true, + inputs: [{ sourceId: "src-in", trigger: null }], + steps: [ + { + operation: "/api/v1/security/cert-sign", + parameters: {}, + fileParameters: { certFile: "asset:x" }, + }, + ], + output: { type: "inline", options: {} }, + outputIds: ["src-1"], + }); + renderBuilder("/processor/pipelines/plc-sign"); + + fireEvent.click(await screen.findByText("portal.pipelines.composer.save")); + + await waitFor(() => expect(savePipeline).toHaveBeenCalledTimes(1)); + expect(savePipeline).toHaveBeenCalledWith( + expect.objectContaining({ + steps: [ + expect.objectContaining({ + operation: "/api/v1/security/cert-sign", + fileParameters: { certFile: "asset:x" }, + }), + ], + }), + ); + expect(uploadPipelineAsset).not.toHaveBeenCalled(); }); it("blocks saving an integration step with no account chosen", async () => { diff --git a/frontend/editor/src/portal/views/PipelineBuilder.tsx b/frontend/editor/src/portal/views/PipelineBuilder.tsx index d5092f5c88..975a8e5cfe 100644 --- a/frontend/editor/src/portal/views/PipelineBuilder.tsx +++ b/frontend/editor/src/portal/views/PipelineBuilder.tsx @@ -17,14 +17,17 @@ import { } from "@app/ui"; import { useToolRegistry } from "@app/contexts/ToolRegistryContext"; import { + activeFileFields, + assetRef, deserializeToolStep, + extractStepFiles, getExecutableTools, newWorkingToolStep, serializeToolStep, stepNeedsConfiguring, - stepRequiresUpload, updateWorkingStepParams, type ExecutableTool, + type SupportingFileBindings, type WorkingToolStep, } from "@app/hooks/tools/shared/toolAutomation"; import { @@ -48,13 +51,20 @@ import { runPipelineTest, savePipeline, triggerPipeline, + type PipelineStep, type Policy, type PolicyRunView, type RunOutputFile, + type TestRunAsset, type TriggerConfig, type TriggerInfo, type TriggerOutcome, } from "@portal/api/pipelines"; +import { + listPipelineAssets, + uploadPipelineAsset, + type PolicyAsset, +} from "@portal/api/pipelineAssets"; import { clearProcessedHistory } from "@portal/api/policies"; import { DestinationPicker } from "@portal/components/pipelines/DestinationPicker"; import { availableOutputModes } from "@portal/components/pipelines/outputModes"; @@ -207,6 +217,17 @@ export function PipelineBuilder() { [allTools], ); + // Stored supporting files from earlier saves, so a reopened step can label its bindings by name. + const assetsState = useAsync( + async () => await listPipelineAssets(), + [], + ); + const assetNames = useMemo(() => { + const map: Record = {}; + for (const asset of assetsState.data ?? []) map[asset.id] = asset.fileName; + return map; + }, [assetsState.data]); + const policyState = useAsync( async () => (id ? await fetchPipeline(id) : null), [id], @@ -486,6 +507,21 @@ export function PipelineBuilder() { ); } + /** Drop a step's stored supporting-file binding for one field (the chip's remove action). */ + function clearStepBinding(index: number, field: string) { + setSteps((current) => + current.map((step, i) => { + if (i !== index || !step.fileParameters) return step; + const next = { ...step.fileParameters }; + delete next[field]; + return { + ...step, + fileParameters: Object.keys(next).length > 0 ? next : undefined, + }; + }), + ); + } + function stepLabel(step: WorkingToolStep): string { // An integration step's endpoint is the same for every vendor, so the raw path would read // "External api call" for all of them. Name it by the operation instead. @@ -512,11 +548,6 @@ export function PipelineBuilder() { return step.toolId ? allTools[step.toolId]?.icon : undefined; } - // Steps whose params carry an uploaded file can't be saved: the bytes aren't persisted with the - // policy, so a later run would send null for that field (see stepRequiresUpload). - const uploadStepLabels = steps.filter(stepRequiresUpload).map(stepLabel); - const hasUploadSteps = uploadStepLabels.length > 0; - // A step still missing a choice - an integration with no operation or account, a tool whose // mandatory parameters are unset - would fail at run time with a raw backend rejection, so block // saving on it here where the fix is one click away. @@ -601,11 +632,30 @@ export function PipelineBuilder() { // seeding, so leaving the builder can prompt to save or discard. `enabled` is deliberately left // out: in edit it is toggled and persisted at once (never an unsaved edit), and in create it is // chosen at submit - so it can never be the thing that makes the form dirty. + // Per-step dirty signature: the serialized step plus a stable identity (name/size/mtime) of its + // fresh file picks - a raw File JSON-stringifies to `{}`, so serializeToolStep (which excludes + // Files) can't see a file added or swapped. Memoized on the steps because it probes each tool's + // buildFormData; without this it would re-run for every step on any render (e.g. each keystroke in + // the name field). Stored bindings are covered by the serialized step. + const stepSnapshot = useMemo( + () => + steps.map((step) => { + const files: Record = {}; + for (const [field, picks] of Object.entries( + extractStepFiles(step, allTools), + )) { + files[field] = picks.map( + (file) => `${file.name}:${file.size}:${file.lastModified}`, + ); + } + return { step: serializeToolStep(step, allTools), files }; + }), + [steps, allTools], + ); const snapshot = JSON.stringify({ name: name.trim(), input, - steps: steps.map((step) => serializeToolStep(step, allTools)), - uploads: steps.map(stepRequiresUpload), + steps: stepSnapshot, outputIds: [...outputIds].sort(), }); const baseline = useRef(null); @@ -639,12 +689,6 @@ export function PipelineBuilder() { tools: unconfiguredStepLabels.join(", "), }), ); - if (hasUploadSteps) - blockers.push( - t("portal.pipelines.builder.blocker.upload", { - tools: uploadStepLabels.join(", "), - }), - ); if (hasIncompatibleSteps) blockers.push( t("portal.pipelines.builder.blocker.incompatible", { @@ -667,23 +711,84 @@ export function PipelineBuilder() { else navigate(destination); } + /** + * The active supporting-file fields of a step, each paired with its fresh in-memory pick(s) and its + * stored `asset:` binding (either may be absent). The single source both saving and test-running + * read, so the two agree on which fields are active and how a binding is chosen; they differ only in + * how a fresh pick is emitted - uploaded as an asset vs. sent inline. + */ + function stepFileFields( + step: WorkingToolStep, + ): { field: string; fresh: File[] | null; stored: string | null }[] { + const fresh = extractStepFiles(step, allTools); + const stored = step.fileParameters ?? {}; + const fields = activeFileFields(step, allTools) ?? Object.keys(stored); + return fields.map((field) => ({ + field, + fresh: fresh[field] ?? null, + stored: stored[field] ?? null, + })); + } + + /** A wire step, attaching fileParameters only when it has any. */ + function toWireStep( + operation: string, + parameters: Record, + bindings: SupportingFileBindings, + ): PipelineStep { + return Object.keys(bindings).length > 0 + ? { operation, parameters, fileParameters: bindings } + : { operation, parameters }; + } + + /** + * The wire steps for saving: scalar params from serialization, plus supporting-file bindings. A + * fresh pick is uploaded to the asset store (the save-time validator rejects a policy that binds an + * asset id that doesn't yet exist); a stored binding the tool still uses is kept when the user + * didn't replace it. Uploads run in parallel; any abandoned by a later failure are GC'd server-side. + */ + async function serializeStepsForSave(): Promise { + return Promise.all( + steps.map(async (step) => { + const { operation, parameters } = serializeToolStep(step, allTools); + const entries = await Promise.all( + stepFileFields(step).map(async ({ field, fresh, stored }) => { + if (fresh?.length) { + const ids = await Promise.all( + fresh.map((file) => + uploadPipelineAsset(file).then((a) => a.id), + ), + ); + return [field, assetRef(ids)] as const; + } + return stored ? ([field, stored] as const) : null; + }), + ); + const bindings: SupportingFileBindings = Object.fromEntries( + entries.filter((e): e is readonly [string, string] => e !== null), + ); + return toWireStep(operation, parameters, bindings); + }), + ); + } + async function save(destination: string, enabledOverride?: boolean) { if (!canSave) return; setSubmitting(true); setError(null); - const policy: Policy = { - id: policyState.data?.id ?? undefined, - name: name.trim(), - enabled: enabledOverride ?? enabled, - // The wire shape stays a list; canSave guarantees the one input has a source. - inputs: [{ sourceId: input.sourceId, trigger: buildTriggerFor(input) }], - steps: steps.map((step) => serializeToolStep(step, allTools)), - // Destinations are the referenced saved sources; the inline output field is - // preserved as-is (e.g. an editor policy's membership metadata) or defaults to inline. - output: policyState.data?.output ?? { type: "inline", options: {} }, - outputIds, - }; try { + const policy: Policy = { + id: policyState.data?.id ?? undefined, + name: name.trim(), + enabled: enabledOverride ?? enabled, + // The wire shape stays a list; canSave guarantees the one input has a source. + inputs: [{ sourceId: input.sourceId, trigger: buildTriggerFor(input) }], + steps: await serializeStepsForSave(), + // Destinations are the referenced saved sources; the inline output field is + // preserved as-is (e.g. an editor policy's membership metadata) or defaults to inline. + output: policyState.data?.output ?? { type: "inline", options: {} }, + outputIds, + }; await savePipeline(policy); await invalidatePipelines(); navigate(destination); @@ -741,6 +846,32 @@ export function PipelineBuilder() { return null; } + /** + * The steps + inline supporting files for a test run. A fresh (in-memory) pick rides along as a + * keyed `assets[i]` under a per-step run key; a stored file keeps its `asset:` binding, which + * the backend resolves from the pipeline's saved policy (passed as policyId) - no re-fetch needed. + */ + function buildTestSteps(): { steps: PipelineStep[]; assets: TestRunAsset[] } { + const assets: TestRunAsset[] = []; + const outSteps = steps.map((step, i) => { + const { operation, parameters } = serializeToolStep(step, allTools); + const bindings: SupportingFileBindings = {}; + for (const { field, fresh, stored } of stepFileFields(step)) { + if (fresh?.length) { + // In-memory pick: inline the bytes under a run key. + const key = `s${i}_${field}`; + bindings[field] = key; + for (const file of fresh) assets.push({ key, file }); + } else if (stored) { + // Already an asset: keep its ref for the backend to resolve from the saved policy. + bindings[field] = stored; + } + } + return toWireStep(operation, parameters, bindings); + }); + return { steps: outSteps, assets }; + } + /** * Run the steps as they stand against one uploaded file. Output is forced inline so nothing * reaches the pipeline's real destination, and the pipeline need not be saved first - this is @@ -752,13 +883,17 @@ export function PipelineBuilder() { setTestRun(null); setRunResult(null); try { + const { steps: testSteps, assets } = buildTestSteps(); const { runId } = await runPipelineTest( { name: name.trim() || t("portal.pipelines.builder.testRun"), - steps: steps.map((step) => serializeToolStep(step, allTools)), + steps: testSteps, output: { type: "inline", options: {} }, }, file, + assets, + // Lets the backend resolve any stored `asset:` refs from this saved policy. + policyState.data?.id, ); const final = await awaitRun(runId, (view) => { if (mounted.current) setTestRun(view); @@ -934,8 +1069,6 @@ export function PipelineBuilder() { return t("portal.pipelines.builder.chooseAccount"); return undefined; } - if (stepRequiresUpload(step)) - return t("portal.pipelines.builder.needsUpload"); if (stepNeedsConfiguring(step, allTools)) return t("portal.pipelines.builder.needsConfiguring"); return undefined; @@ -1120,6 +1253,8 @@ export function PipelineBuilder() { step={selectedStep} registry={allTools} onChange={(params) => updateStepParams(chosenSteps[0], params)} + assetNames={assetNames} + onClearBinding={(field) => clearStepBinding(chosenSteps[0], field)} /> ); } @@ -1165,14 +1300,6 @@ export function PipelineBuilder() { {runResult && ( )} - {hasUploadSteps && ( - - )} {hasUnconfiguredSteps && (