Fix workspace review issues and policy check

2026-03-14 14:13:03 -05:00
parent 6e6d67372c
commit dd828e96ad
6 changed files with 123 additions and 407 deletions
--- a/server/src/tests/workspace-runtime.test.ts
+++ b/server/src/tests/workspace-runtime.test.ts
@@ -457,6 +457,8 @@ describe("ensureRuntimeServicesForRun", () => {
    expect(captured.customEnv).toBe("from-adapter");
    expect(captured.port).toMatch(/^\d+$/);
    expect(services[0]?.executionWorkspaceId).toBe("execution-workspace-1");
+    expect(services[0]?.scopeType).toBe("execution_workspace");
+    expect(services[0]?.scopeId).toBe("execution-workspace-1");
  });

  it("stops execution workspace runtime services by executionWorkspaceId", async () => {
@@ -584,4 +586,33 @@ describe("normalizeAdapterManagedRuntimeServices", () => {
    });
    expect(first[0]?.id).toBe(second[0]?.id);
  });
+
+  it("prefers execution workspace ids over cwd for execution-scoped adapter services", () => {
+    const workspace = buildWorkspace("/tmp/project");
+
+    const refs = normalizeAdapterManagedRuntimeServices({
+      adapterType: "openclaw_gateway",
+      runId: "run-1",
+      agent: {
+        id: "agent-1",
+        name: "Gateway Agent",
+        companyId: "company-1",
+      },
+      issue: null,
+      workspace,
+      executionWorkspaceId: "execution-workspace-1",
+      reports: [
+        {
+          serviceName: "preview",
+          scopeType: "execution_workspace",
+        },
+      ],
+    });
+
+    expect(refs[0]).toMatchObject({
+      scopeType: "execution_workspace",
+      scopeId: "execution-workspace-1",
+      executionWorkspaceId: "execution-workspace-1",
+    });
+  });
 });
--- a/server/src/routes/execution-workspaces.ts
+++ b/server/src/routes/execution-workspaces.ts
@@ -54,6 +54,7 @@ export function executionWorkspaceRoutes(db: Db) {
      ...req.body,
      ...(req.body.cleanupEligibleAt ? { cleanupEligibleAt: new Date(req.body.cleanupEligibleAt) } : {}),
    };
+    let workspace = existing;
    let cleanupWarnings: string[] = [];

    if (req.body.status === "archived" && existing.status !== "archived") {
@@ -73,52 +74,85 @@ export function executionWorkspaceRoutes(db: Db) {
        return;
      }

-      await stopRuntimeServicesForExecutionWorkspace({
-        db,
-        executionWorkspaceId: existing.id,
-        workspaceCwd: existing.cwd,
+      const closedAt = new Date();
+      const archivedWorkspace = await svc.update(id, {
+        ...patch,
+        status: "archived",
+        closedAt,
+        cleanupReason: null,
      });
-      const projectWorkspace = existing.projectWorkspaceId
-        ? await db
-            .select({
-              cwd: projectWorkspaces.cwd,
-              cleanupCommand: projectWorkspaces.cleanupCommand,
-            })
-            .from(projectWorkspaces)
-            .where(
-              and(
-                eq(projectWorkspaces.id, existing.projectWorkspaceId),
-                eq(projectWorkspaces.companyId, existing.companyId),
-              ),
-            )
-            .then((rows) => rows[0] ?? null)
-        : null;
-      const projectPolicy = existing.projectId
-        ? await db
-            .select({
-              executionWorkspacePolicy: projects.executionWorkspacePolicy,
-            })
-            .from(projects)
-            .where(and(eq(projects.id, existing.projectId), eq(projects.companyId, existing.companyId)))
-            .then((rows) => parseProjectExecutionWorkspacePolicy(rows[0]?.executionWorkspacePolicy))
-        : null;
-      const cleanupResult = await cleanupExecutionWorkspaceArtifacts({
-        workspace: existing,
-        projectWorkspace,
-        teardownCommand: projectPolicy?.workspaceStrategy?.teardownCommand ?? null,
-      });
-      cleanupWarnings = cleanupResult.warnings;
-      patch.closedAt = new Date();
-      patch.cleanupReason = cleanupWarnings.length > 0 ? cleanupWarnings.join(" | ") : null;
-      if (!cleanupResult.cleaned) {
-        patch.status = "cleanup_failed";
+      if (!archivedWorkspace) {
+        res.status(404).json({ error: "Execution workspace not found" });
+        return;
      }
-    }
+      workspace = archivedWorkspace;

-    const workspace = await svc.update(id, patch);
-    if (!workspace) {
-      res.status(404).json({ error: "Execution workspace not found" });
-      return;
+      try {
+        await stopRuntimeServicesForExecutionWorkspace({
+          db,
+          executionWorkspaceId: existing.id,
+          workspaceCwd: existing.cwd,
+        });
+        const projectWorkspace = existing.projectWorkspaceId
+          ? await db
+              .select({
+                cwd: projectWorkspaces.cwd,
+                cleanupCommand: projectWorkspaces.cleanupCommand,
+              })
+              .from(projectWorkspaces)
+              .where(
+                and(
+                  eq(projectWorkspaces.id, existing.projectWorkspaceId),
+                  eq(projectWorkspaces.companyId, existing.companyId),
+                ),
+              )
+              .then((rows) => rows[0] ?? null)
+          : null;
+        const projectPolicy = existing.projectId
+          ? await db
+              .select({
+                executionWorkspacePolicy: projects.executionWorkspacePolicy,
+              })
+              .from(projects)
+              .where(and(eq(projects.id, existing.projectId), eq(projects.companyId, existing.companyId)))
+              .then((rows) => parseProjectExecutionWorkspacePolicy(rows[0]?.executionWorkspacePolicy))
+          : null;
+        const cleanupResult = await cleanupExecutionWorkspaceArtifacts({
+          workspace: existing,
+          projectWorkspace,
+          teardownCommand: projectPolicy?.workspaceStrategy?.teardownCommand ?? null,
+        });
+        cleanupWarnings = cleanupResult.warnings;
+        const cleanupPatch: Record<string, unknown> = {
+          closedAt,
+          cleanupReason: cleanupWarnings.length > 0 ? cleanupWarnings.join(" | ") : null,
+        };
+        if (!cleanupResult.cleaned) {
+          cleanupPatch.status = "cleanup_failed";
+        }
+        if (cleanupResult.warnings.length > 0 || !cleanupResult.cleaned) {
+          workspace = (await svc.update(id, cleanupPatch)) ?? workspace;
+        }
+      } catch (error) {
+        const failureReason = error instanceof Error ? error.message : String(error);
+        workspace =
+          (await svc.update(id, {
+            status: "cleanup_failed",
+            closedAt,
+            cleanupReason: failureReason,
+          })) ?? workspace;
+        res.status(500).json({
+          error: `Failed to archive execution workspace: ${failureReason}`,
+        });
+        return;
+      }
+    } else {
+      const updatedWorkspace = await svc.update(id, patch);
+      if (!updatedWorkspace) {
+        res.status(404).json({ error: "Execution workspace not found" });
+        return;
+      }
+      workspace = updatedWorkspace;
    }
    const actor = getActorInfo(req);
    await logActivity(db, {
--- a/server/src/services/execution-workspace-policy.ts
+++ b/server/src/services/execution-workspace-policy.ts
@@ -34,6 +34,7 @@ export function parseProjectExecutionWorkspacePolicy(raw: unknown): ProjectExecu
  const parsed = parseObject(raw);
  if (Object.keys(parsed).length === 0) return null;
  const enabled = typeof parsed.enabled === "boolean" ? parsed.enabled : false;
+  const workspaceStrategy = parseExecutionWorkspaceStrategy(parsed.workspaceStrategy);
  const defaultMode = asString(parsed.defaultMode, "");
  const defaultProjectWorkspaceId =
    typeof parsed.defaultProjectWorkspaceId === "string" ? parsed.defaultProjectWorkspaceId : undefined;
@@ -57,9 +58,7 @@ export function parseProjectExecutionWorkspacePolicy(raw: unknown): ProjectExecu
    ...(normalizedDefaultMode ? { defaultMode: normalizedDefaultMode } : {}),
    ...(allowIssueOverride !== undefined ? { allowIssueOverride } : {}),
    ...(defaultProjectWorkspaceId ? { defaultProjectWorkspaceId } : {}),
-    ...(parseExecutionWorkspaceStrategy(parsed.workspaceStrategy)
-      ? { workspaceStrategy: parseExecutionWorkspaceStrategy(parsed.workspaceStrategy) }
-      : {}),
+    ...(workspaceStrategy ? { workspaceStrategy } : {}),
    ...(parsed.workspaceRuntime && typeof parsed.workspaceRuntime === "object" && !Array.isArray(parsed.workspaceRuntime)
      ? { workspaceRuntime: { ...(parsed.workspaceRuntime as Record<string, unknown>) } }
      : {}),
@@ -81,6 +80,7 @@ export function parseProjectExecutionWorkspacePolicy(raw: unknown): ProjectExecu
 export function parseIssueExecutionWorkspaceSettings(raw: unknown): IssueExecutionWorkspaceSettings | null {
  const parsed = parseObject(raw);
  if (Object.keys(parsed).length === 0) return null;
+  const workspaceStrategy = parseExecutionWorkspaceStrategy(parsed.workspaceStrategy);
  const mode = asString(parsed.mode, "");
  const normalizedMode = (() => {
    if (
@@ -101,9 +101,7 @@ export function parseIssueExecutionWorkspaceSettings(raw: unknown): IssueExecuti
    ...(normalizedMode
      ? { mode: normalizedMode as IssueExecutionWorkspaceSettings["mode"] }
      : {}),
-    ...(parseExecutionWorkspaceStrategy(parsed.workspaceStrategy)
-      ? { workspaceStrategy: parseExecutionWorkspaceStrategy(parsed.workspaceStrategy) }
-      : {}),
+    ...(workspaceStrategy ? { workspaceStrategy } : {}),
    ...(parsed.workspaceRuntime && typeof parsed.workspaceRuntime === "object" && !Array.isArray(parsed.workspaceRuntime)
      ? { workspaceRuntime: { ...(parsed.workspaceRuntime as Record<string, unknown>) } }
      : {}),
--- a/server/src/services/workspace-runtime.ts
+++ b/server/src/services/workspace-runtime.ts
@@ -644,6 +644,7 @@ function buildTemplateData(input: {
 function resolveServiceScopeId(input: {
  service: Record<string, unknown>;
  workspace: RealizedExecutionWorkspace;
+  executionWorkspaceId?: string | null;
  issue: ExecutionWorkspaceIssueRef | null;
  runId: string;
  agent: ExecutionWorkspaceAgentRef;
@@ -659,7 +660,9 @@ function resolveServiceScopeId(input: {
      ? scopeTypeRaw
      : "run";
  if (scopeType === "project_workspace") return { scopeType, scopeId: input.workspace.workspaceId ?? input.workspace.projectId };
-  if (scopeType === "execution_workspace") return { scopeType, scopeId: input.workspace.cwd };
+  if (scopeType === "execution_workspace") {
+    return { scopeType, scopeId: input.executionWorkspaceId ?? input.workspace.cwd };
+  }
  if (scopeType === "agent") return { scopeType, scopeId: input.agent.id };
  return { scopeType: "run" as const, scopeId: input.runId };
 }
@@ -780,7 +783,7 @@ export function normalizeAdapterManagedRuntimeServices(input: {
      (scopeType === "project_workspace"
        ? input.workspace.workspaceId
        : scopeType === "execution_workspace"
-          ? input.workspace.cwd
+          ? input.executionWorkspaceId ?? input.workspace.cwd
          : scopeType === "agent"
            ? input.agent.id
            : input.runId) ??
@@ -1043,6 +1046,7 @@ export async function ensureRuntimeServicesForRun(input: {
      const { scopeType, scopeId } = resolveServiceScopeId({
        service,
        workspace: input.workspace,
+        executionWorkspaceId: input.executionWorkspaceId,
        issue: input.issue,
        runId: input.runId,
        agent: input.agent,