Address PR #230 review: infra, security, cache correctness, editor bugs, cleanup

Works through the PR #230 review of the visual workflow builder. All bug and
blocker findings across sections 1-6 are fixed with tests; two items (artifact
split, generic list editor) were deliberately deferred/declined with
documented rationale.

Infra: wire deno-runner env into worker + backend pods; build+push the
deno-runner image (CI matrix, oc-build-push.sh, per-instance overlay); run the
graph-workflow jest suite in CI; add linux-arm64 native-binary pins; document
the deno-runner NetworkPolicy DNS rationale; fail fast on missing
PLATFORM_API_KEY.

Security/tenancy: agent workflow tools assert the workflow's group (closes
cross-tenant read+write); getNodeStatuses scopes runId to the lineage;
deleteDynamicNode slug bounded+encoded; conversation resume rejects group
mismatch.

Engine/cache: leaf-scoped cache snapshot + deep-merge restore (no sibling
clobber); best-effort source-node write; P2002 dead-path made best-effort;
honor DynamicNodeVersion.deterministic; version-pin resolution for
name-referenced children; validator accepts source-produced ctx keys; Map
fan-out propagates workflowLineageId; getInputCtx fail-safe 404 on retention +
fail-closed cross-lineage.

Frontend: NodePicker typeable on large graphs; Auto-arrange moves nodes; agent
create->edit route; agent chat no longer stomps unsaved edits; chat-queued
uploads fire; run-drawer duplicate run removed; cache-evicted alert gated on
replay; renameCtxKey rewrites all refs; switch-case edge labels,
error-fallback edges, fresh-Try polling, errorPolicy handle, and literal-input
type-flip fixed.

Housekeeping: remove the unimplemented Join "Any" strategy; gate the dev-only
form-preview route behind import.meta.env.DEV; document the HumanGate
continue-path test skips (test-env hang, not a bug); drop the last explicit
any (deno-runner harness); delete the dead subprocess-harness placeholder.

Cleanup: move benchmark-OCR replay knowledge to an ActivityCatalogEntry
metadata field (engine stays workload-generic); add builderFetch (shared auth
+ cookies + ApiService 401 refresh) and migrate the 9 fetch-based builder
hooks; extract a generic LruTtlCache and consolidate the two backend LRU+TTL
caches; add a shared replaceNode helper + shared artifact-shape detectors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: alex-struk

.github/workflows/deploy-instance.yml

@@ -123,6 +123,12 @@ jobs:
           - service: ches-adapter
             context: "."
             dockerfile: apps/ches-adapter/Dockerfile
+          # Phase 6 dynamic-nodes Deno sandbox. Its Dockerfile COPYs src/ and
+          # deno.json relative to the build context, so the context is the app
+          # dir (not the repo root like the others).
+          - service: deno-runner
+            context: apps/deno-runner
+            dockerfile: apps/deno-runner/Dockerfile
     steps:
       - name: Checkout
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
@@ -265,6 +271,7 @@ jobs:
             --backend-image "${IMAGE_BASE}/backend-services" \
             --frontend-image "${IMAGE_BASE}/frontend" \
             --worker-image "${IMAGE_BASE}/temporal" \
+            --deno-runner-image "${IMAGE_BASE}/deno-runner" \
             --image-tag "${IMAGE_TAG}" \
             --sso-auth-server-url "${SSO_AUTH_SERVER_URL}" \
             --sso-realm "${SSO_REALM}" \

.github/workflows/frontend-qa.yml

@@ -31,6 +31,12 @@ jobs:
         run: npm install --ignore-scripts --no-package-lock
       - name: Build graph-workflow package
         run: npm run build -w packages/graph-workflow
+      # The shared graph-workflow package (validator, cache-hash, ctx-binding)
+      # is consumed by the backend save-path, the temporal worker, and the
+      # canvas. This QA job triggers on packages/** so its jest suite runs here
+      # rather than shipping with zero CI-enforced coverage.
+      - name: Test graph-workflow package
+        run: npm run test -w packages/graph-workflow
       - name: Run Linter
         working-directory: apps/frontend
         run: npm run lint

apps/backend-services/src/agent/agent.service.startchat.spec.ts

@@ -1,4 +1,4 @@
-import { ForbiddenException } from "@nestjs/common";
+import { ForbiddenException, NotFoundException } from "@nestjs/common";
 import type { OnFinishEvent, StreamTextResult, ToolSet } from "ai";
 
 // Mock the AI SDK boundary so startChat's orchestration is unit-testable
@@ -75,6 +75,9 @@ function makeHarness(opts: {
           id: "conv-1",
           workflowId: null,
           title: "existing",
+          // Must match baseInput.groupId — startChat rejects a resume whose
+          // stored group differs from the request's group.
+          groupId: "g1",
         }
       : opts.conversation;
 
@@ -160,6 +163,30 @@ describe("AgentService.startChat — per-conversation budget (ITEM 26)", () => {
   });
 });
 
+describe("AgentService.startChat — resume group scoping (SECURITY §2.4)", () => {
+  it("rejects resuming a conversation whose stored group differs from the request group", async () => {
+    const { service, chatRepository } = makeHarness({
+      conversation: {
+        id: "conv-1",
+        workflowId: "wf-in-group-B",
+        title: "existing",
+        groupId: "group-B",
+      },
+    });
+
+    await expect(
+      service.startChat({
+        ...baseInput,
+        // Request presents group A ("g1") but the conversation belongs to B.
+        messages: [userMsg("hi")],
+      } as never),
+    ).rejects.toBeInstanceOf(NotFoundException);
+
+    // The mismatch is caught before any message is persisted.
+    expect(chatRepository.createMessage).not.toHaveBeenCalled();
+  });
+});
+
 describe("AgentService.startChat — onFinish persistence (ITEM 23 + 26)", () => {
   it("persists full assistant parts (tool calls included) and totals tokens", async () => {
     const { service, chatRepository } = makeHarness({});
@@ -251,7 +278,12 @@ describe("AgentService.startChat — abort cleanup (ITEM 24/25)", () => {
 describe("AgentService.startChat — ctx binding via onWorkflowCreated (ITEM 25)", () => {
   it("binds the conversation workflowId when the agent's onWorkflowCreated hook fires", async () => {
     const { service, setWorkflowId } = makeHarness({
-      conversation: { id: "conv-1", workflowId: null, title: "t" },
+      conversation: {
+        id: "conv-1",
+        workflowId: null,
+        title: "t",
+        groupId: "g1",
+      },
     });
     await service.startChat({
       ...baseInput,
@@ -269,7 +301,12 @@ describe("AgentService.startChat — ctx binding via onWorkflowCreated (ITEM 25)
 
   it("does not rebind when the conversation already has a workflowId", async () => {
     const { service, setWorkflowId } = makeHarness({
-      conversation: { id: "conv-1", workflowId: "wf-existing", title: "t" },
+      conversation: {
+        id: "conv-1",
+        workflowId: "wf-existing",
+        title: "t",
+        groupId: "g1",
+      },
     });
     await service.startChat({
       ...baseInput,

apps/backend-services/src/agent/agent.service.ts

@@ -80,6 +80,16 @@ export class AgentService {
       throw new NotFoundException("Conversation not found");
     }
 
+    // A conversation is bound to the group it was created in. Resuming it
+    // under a different group (a multi-group user presenting group A while the
+    // stored conversation — and its bound workflowId — belong to group B)
+    // would build the tool context against a mismatched group. Reject rather
+    // than operate cross-group; NotFound (not Forbidden) matches the id-probing
+    // convention used elsewhere.
+    if (conversation !== null && conversation.groupId !== input.groupId) {
+      throw new NotFoundException("Conversation not found");
+    }
+
     if (conversation === null) {
       conversation = await this.chatRepository.createConversation({
         workflowId: input.workflowId,

apps/backend-services/src/agent/tools.spec.ts

@@ -67,6 +67,11 @@ function makeCtx(overrides: Partial<AgentToolContext> = {}): {
     description: null,
     slug: "wf",
     version: 3,
+    // Tenancy: the agent tools assert the fetched workflow belongs to
+    // ctx.groupId. Default the mock to the same group so the happy-path
+    // tests pass; the cross-group test overrides getWorkflow to return a
+    // foreign group.
+    groupId: "group-1",
     config: state.config,
   }));
   const updateWorkflow = jest.fn(
@@ -299,6 +304,74 @@ describe("wrapToolData (ITEM 27 delimiting + ITEM 26 truncation)", () => {
   });
 });
 
+// ── Tenancy: agent tools must not read/write cross-group workflows ───
+describe("agent tools enforce group ownership (SECURITY §2.1)", () => {
+  function makeForeignCtx() {
+    // getWorkflow resolves a workflow in a DIFFERENT group than ctx.groupId —
+    // simulating a model-supplied workflowId (reachable via prompt injection)
+    // that points at another tenant's row.
+    const foreignConfig = emptyConfig();
+    const getWorkflow = jest.fn(async () => ({
+      id: "victim-wf",
+      name: "Victim",
+      description: null,
+      slug: "victim",
+      version: 1,
+      groupId: "group-OTHER",
+      config: foreignConfig,
+    }));
+    const updateWorkflow = jest.fn(async () => ({
+      id: "victim-wf",
+      name: "Victim",
+    }));
+    const ctx = {
+      actorId: "actor-1",
+      groupId: "group-1",
+      workflowId: null,
+      apiKey: "key-1",
+      backendBaseUrl: "http://backend",
+      workflowService: { getWorkflow, updateWorkflow } as unknown,
+      dynamicNodesService: {
+        getMergedCatalogForGroup: jest.fn(async () => []),
+      } as unknown,
+    } as unknown as AgentToolContext;
+    return { ctx, getWorkflow, updateWorkflow };
+  }
+
+  it("getWorkflow throws NotFound for a cross-group workflow id", async () => {
+    const { ctx, getWorkflow } = makeForeignCtx();
+    const tools = createAgentTools(ctx);
+    await expect(
+      exec(tools, "getWorkflow", { workflowId: "victim-wf" }),
+    ).rejects.toMatchObject({ status: 404 });
+    expect(getWorkflow).toHaveBeenCalledWith("victim-wf", "actor-1");
+  });
+
+  it("a write tool refuses a cross-group workflow and never calls updateWorkflow", async () => {
+    const { ctx, updateWorkflow } = makeForeignCtx();
+    const tools = createAgentTools(ctx);
+    await expect(
+      exec(tools, "updateWorkflowMetadata", {
+        workflowId: "victim-wf",
+        name: "hijacked",
+      }),
+    ).rejects.toMatchObject({ status: 404 });
+    expect(updateWorkflow).not.toHaveBeenCalled();
+  });
+
+  it("addNode (read+write path) refuses a cross-group workflow", async () => {
+    const { ctx, updateWorkflow } = makeForeignCtx();
+    const tools = createAgentTools(ctx);
+    await expect(
+      exec(tools, "addNode", {
+        workflowId: "victim-wf",
+        node: { id: "n1", type: "file.prepare" },
+      }),
+    ).rejects.toMatchObject({ status: 404 });
+    expect(updateWorkflow).not.toHaveBeenCalled();
+  });
+});
+
 // ── ITEM 25 — tools.ts write / validation / retry logic ─────────────
 
 describe("connectNodes validation (ITEM 25)", () => {
@@ -499,6 +572,35 @@ describe("publishDynamicNode 409 → PUT republish (ITEM 25)", () => {
   });
 });
 
+describe("deleteDynamicNode slug bounding (SECURITY §2.3)", () => {
+  it("rejects a path-traversal slug without issuing any self-call", async () => {
+    const { ctx, internalFetchMock } = makeCtx();
+    const tools = createAgentTools(ctx);
+    const result = await exec<{ ok: boolean; error?: { code: string } }>(
+      tools,
+      "deleteDynamicNode",
+      { slug: "../workflows/victim-wf-id" },
+    );
+    expect(result.ok).toBe(false);
+    expect(result.error?.code).toBe("invalid-slug");
+    expect(internalFetchMock).not.toHaveBeenCalled();
+  });
+
+  it("issues a DELETE to the slug-scoped endpoint for a valid slug", async () => {
+    const { ctx, internalFetchMock } = makeCtx();
+    internalFetchMock.mockResolvedValueOnce(fetchResponse(200, { ok: true }));
+    const tools = createAgentTools(ctx);
+    const result = await exec<{ ok: boolean }>(tools, "deleteDynamicNode", {
+      slug: "my-node",
+    });
+    expect(result.ok).toBe(true);
+    const url = internalFetchMock.mock.calls[0][0] as string;
+    const init = internalFetchMock.mock.calls[0][1] as RequestInit;
+    expect(url).toContain("/api/dynamic-nodes/my-node");
+    expect(init.method).toBe("DELETE");
+  });
+});
+
 describe("createWorkflow binds the conversation (ITEM 25)", () => {
   it("invokes onWorkflowCreated with the new id", async () => {
     const onWorkflowCreated = jest.fn();

apps/backend-services/src/agent/tools.ts

@@ -8,6 +8,7 @@ import {
 // bridge instantiates pathologically (TS2589, ~15M instantiations/tool, OOM on
 // a full type-check) against Zod's v3 API; the v4 API resolves in ~23k
 // instantiations. The runtime API used here is identical across v3/v4.
+import { NotFoundException } from "@nestjs/common";
 import { type ToolSet, tool } from "ai";
 import { z } from "zod/v4";
 import type { DynamicNodesService } from "@/dynamic-nodes/dynamic-nodes.service";
@@ -16,7 +17,10 @@ import type {
   GraphNode,
   GraphWorkflowConfig,
 } from "@/workflow/graph-workflow-types";
-import type { WorkflowService } from "@/workflow/workflow.service";
+import type {
+  WorkflowInfo,
+  WorkflowService,
+} from "@/workflow/workflow.service";
 
 /**
  * Default cap on the number of characters of a single tool result that
@@ -27,6 +31,17 @@ import type { WorkflowService } from "@/workflow/workflow.service";
  */
 export const DEFAULT_MAX_TOOL_RESULT_CHARS = 20000;
 
+/**
+ * Canonical dynamic-node slug grammar: a lowercase letter followed by
+ * lowercase alphanumerics/hyphens. Matches the `@name` capture the publish
+ * path is bounded by. Used to reject model-supplied slugs BEFORE they are
+ * interpolated into a self-call path — an unbounded slug like
+ * `../workflows/<id>` would path-normalise into `DELETE /api/workflows/<id>`,
+ * converting a soft-delete tool into an arbitrary same-privilege DELETE
+ * (reachable via prompt injection).
+ */
+const DYNAMIC_NODE_SLUG_RE = /^[a-z][a-z0-9-]*$/;
+
 /**
  * Visible marker bracketing tool-result content returned to the model.
  * The system prompt instructs the model to treat anything between these
@@ -151,11 +166,40 @@ function ensureNonNullWorkflowId(
   return resolved;
 }
 
+/**
+ * Fetch a workflow and enforce that it belongs to the agent's bound group.
+ *
+ * The REST `WorkflowController` guards every endpoint with
+ * `identityCanAccessGroup`; the agent tools call `WorkflowService` directly,
+ * so tenancy MUST be re-asserted here. `getWorkflow` does a bare
+ * `findUnique({ where: { id } })` with no group filter (by design — the
+ * controller checks the returned `groupId` against the caller's possibly-many
+ * groups). The agent, by contrast, operates in exactly one group
+ * (`ctx.groupId`), so any workflow it reads or writes must live in that group.
+ *
+ * The tool schema accepts a model-supplied `workflowId`, reachable via prompt
+ * injection from untrusted document/OCR text surfaced through the read tools —
+ * so this is the security boundary, not a convenience. On mismatch we throw
+ * `NotFoundException` (not Forbidden) so a cross-group id is indistinguishable
+ * from a missing one and can't be used to probe existence, matching the
+ * service's own 404-vs-403 convention.
+ */
+async function fetchWorkflowInGroup(
+  ctx: AgentToolContext,
+  workflowId: string,
+): Promise<WorkflowInfo> {
+  const wf = await ctx.workflowService.getWorkflow(workflowId, ctx.actorId);
+  if (wf.groupId !== ctx.groupId) {
+    throw new NotFoundException(`Workflow not found: ${workflowId}`);
+  }
+  return wf;
+}
+
 async function readWorkflow(
   ctx: AgentToolContext,
   workflowId: string,
 ): Promise<GraphWorkflowConfig> {
-  const wf = await ctx.workflowService.getWorkflow(workflowId, ctx.actorId);
+  const wf = await fetchWorkflowInGroup(ctx, workflowId);
   return wf.config;
 }
 
@@ -180,6 +224,12 @@ async function writeWorkflow(
   workflowId: string,
   config: GraphWorkflowConfig,
 ): Promise<void> {
+  // Assert group ownership BEFORE writing: updateWorkflow re-validates the
+  // config against the *target* row's group_id, so without this a cross-group
+  // write would validate and succeed. Callers that first read via
+  // readWorkflow are already guarded, but the assertion is cheap and keeps
+  // writeWorkflow safe on its own.
+  await fetchWorkflowInGroup(ctx, workflowId);
   const resolved = resolveConfigForPersist(config);
   await ctx.workflowService.updateWorkflow(workflowId, ctx.actorId, {
     config: resolved,
@@ -274,7 +324,7 @@ export function createAgentTools(ctx: AgentToolContext): ToolSet {
       }),
       execute: async ({ workflowId }) => {
         const id = ensureNonNullWorkflowId(ctx, workflowId);
-        const wf = await ctx.workflowService.getWorkflow(id, ctx.actorId);
+        const wf = await fetchWorkflowInGroup(ctx, id);
         // Workflow name/description/node params are user-controlled; wrap
         // as DATA + size-cap before it enters the model context.
         return {
@@ -359,6 +409,9 @@ export function createAgentTools(ctx: AgentToolContext): ToolSet {
       }),
       execute: async ({ workflowId, name, description }) => {
         const id = ensureNonNullWorkflowId(ctx, workflowId);
+        // Assert group ownership before the metadata write (same tenancy
+        // boundary as the graph-edit tools).
+        await fetchWorkflowInGroup(ctx, id);
         const patch: { name?: string; description?: string } = {};
         if (name !== undefined) patch.name = name;
         if (description !== undefined) patch.description = description;
@@ -711,7 +764,7 @@ export function createAgentTools(ctx: AgentToolContext): ToolSet {
             const slug = parsed[1];
             const putResult = await internalFetch(
               ctx,
-              `/api/dynamic-nodes/${slug}`,
+              `/api/dynamic-nodes/${encodeURIComponent(slug)}`,
               { method: "PUT", body: JSON.stringify({ script }) },
             );
             return putResult.ok
@@ -727,9 +780,24 @@ export function createAgentTools(ctx: AgentToolContext): ToolSet {
       description: "Soft-delete a published dynamic node by slug.",
       inputSchema: z.object({ slug: z.string() }),
       execute: async ({ slug }) => {
-        const result = await internalFetch(ctx, `/api/dynamic-nodes/${slug}`, {
-          method: "DELETE",
-        });
+        // Reject anything that isn't a bare dynamic-node slug before it reaches
+        // the self-call path — otherwise `slug='../workflows/<id>'` normalises
+        // to a DELETE against an arbitrary endpoint.
+        if (!DYNAMIC_NODE_SLUG_RE.test(slug)) {
+          return {
+            ok: false,
+            error: {
+              code: "invalid-slug",
+              message:
+                "slug must match /^[a-z][a-z0-9-]*$/ (a bare dynamic-node slug).",
+            },
+          };
+        }
+        const result = await internalFetch(
+          ctx,
+          `/api/dynamic-nodes/${encodeURIComponent(slug)}`,
+          { method: "DELETE" },
+        );
         return result.ok
           ? { ok: true, ...(result.body as object) }
           : { ok: false, error: result.body };