diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 1048f886509..bfabbd50c6a 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -897,8 +897,14 @@ Permissions / Trust settings. Board operators with `agents:create` can add same-company projects through `POST /api/agents/:id/project-access`; the operation locks the agent, preserves existing permissions and audits added projects. Saving does not retry work. The task offers explicit Retry after the required -project is allowed, and dispatch rechecks authorization. Existing setup failures -with the exact project-boundary denial also receive the explanation on reload. +projects are allowed, and dispatch rechecks authorization. Readiness checks the +task's current organizational project and selected workspace source, including +a valid pending selection. Moving the task to an allowed project does not clear +a retained source's access requirement. Existing setup failures with the exact +project-boundary denial also receive the explanation on reload, without assuming +that the task's current project caused the historical failure. A stale or +unavailable workspace selection stays in the non-retryable failure state; it +does not ask the user to grant project access that cannot repair the selection. Cross-issue writes are contained per heartbeat run. An agent-authored comment may wake the target assignee, including an explicit `resume: true` comment on a diff --git a/maps/feature-map/recovery.md b/maps/feature-map/recovery.md index b7de12dddc4..a6f49522fef 100644 --- a/maps/feature-map/recovery.md +++ b/maps/feature-map/recovery.md @@ -53,6 +53,12 @@ A workspace project denial names the blocked access and links to the agent's project, return to the saved task for an explicit **Retry task** action. Saving permissions alone does not wake the agent. Historical setup denials receive the same notice on reload; unrelated errors and prior assignees do not. +Move a denied task to a project the agent can access while retaining a source +from another project: the notice must stay blocked until the selected source is +also allowed or an authorized workspace selection replaces it. A valid pending +selection is checked for the next admission; a stale or unavailable selection +must show a non-retryable failure without a project-permission repair action. +Project changes alone preserve selected files. Thinking appears immediately after Retry, before the request completes. Technical source/run details remain in deliberate diagnostics. The diff --git a/server/src/__tests__/agent-project-access.test.ts b/server/src/__tests__/agent-project-access.test.ts index 38618a3669c..5b7b26baa34 100644 --- a/server/src/__tests__/agent-project-access.test.ts +++ b/server/src/__tests__/agent-project-access.test.ts @@ -4,12 +4,14 @@ import request from "supertest"; import { and, eq } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { agents, agentWakeupRequests, activityLog, authUsers, companies, companyMemberships, createDb, - heartbeatRuns, issues, projects, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; + executionWorkspaces, heartbeatRuns, issues, projects, projectWorkspaces, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; import { registerAgentProjectAccessRoutes } from "../routes/agent-project-access.js"; import { errorHandler } from "../middleware/error-handler.js"; -import { assertTaskWorkspaceSourceProjectAccess } from "../services/task-workspace-access.js"; +import { assertTaskWorkspaceExecutionAccess, assertTaskWorkspaceSourceProjectAccess } from "../services/task-workspace-access.js"; import { taskInterruptionPresentation } from "../services/task-interruption-presentation.js"; import { ProjectAccessDeniedError } from "../services/project-access-denial.js"; +import { issueService } from "../services/issues.js"; +import { executionWorkspaceService } from "../services/execution-workspaces.js"; import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; describe("additive project access", () => { @@ -43,6 +45,94 @@ describe("additive project access", () => { } const board = { type: "board" as const, source: "local_implicit" as const }; + it("keeps a historical denial blocked after the task moves to an allowed project but retains its source", async () => { + const f = await seed(); + const [source] = await db.insert(projectWorkspaces).values({ companyId: f.companyId, projectId: f.selected[1]!.id, + name: "Original repository", cwd: "/tmp/project-access-original" }).returning(); + await db.update(issues).set({ projectWorkspaceId: source!.id, + workspaceSelection: { version: 1, source: "project", selection: { + kind: "configured_source", projectWorkspaceId: source!.id, mode: "managed_isolated", + } } }).where(eq(issues.id, f.issue.id)); + const moved = await issueService(db).update(f.issue.id, { projectId: f.selected[0]!.id }); + expect(moved).toMatchObject({ projectId: f.selected[0]!.id, projectWorkspaceId: source!.id }); + const actor = { type: "agent" as const, source: "agent_key" as const, agentId: f.agentId, companyId: f.companyId }; + await expect(assertTaskWorkspaceExecutionAccess(db, actor, f.companyId, { issueId: f.issue.id })) + .rejects.toBeInstanceOf(ProjectAccessDeniedError); + expect(await taskInterruptionPresentation(db, moved!)).toMatchObject({ + canRetry: false, userAction: "project_access", projectAccess: { ready: false, projectId: null }, + }); + expect((await request(app(board)).post(`/agents/${f.agentId}/project-access`) + .send({ projectIds: [f.selected[1]!.id] })).status).toBe(200); + expect(await taskInterruptionPresentation(db, moved!)).toMatchObject({ canRetry: true, projectAccess: { ready: true } }); + await expect(assertTaskWorkspaceExecutionAccess(db, actor, f.companyId, { issueId: f.issue.id })).resolves.toBeDefined(); + }); + + it.each(["applied", "pending"] as const)("rechecks the selected source after an %s repair instead of the old denied project", async repair => { + const f = await seed(); + const [source] = await db.insert(projectWorkspaces).values({ companyId: f.companyId, projectId: f.selected[0]!.id, + name: "Allowed repository", cwd: "/tmp/project-access-allowed" }).returning(); + await db.update(heartbeatRuns).set({ errorCode: "project_access_denied", error: null, + resultJson: { projectAccessDenied: { projectId: f.selected[1]!.id } } }).where(eq(heartbeatRuns.id, f.runId)); + const moved = await issueService(db).update(f.issue.id, { projectId: f.selected[0]!.id }); + if (repair === "applied") { + await issueService(db).update(f.issue.id, { projectWorkspaceId: source!.id, workspaceSelectionActor: board }); + } else { + await executionWorkspaceService(db).selectTaskWorkspace({ companyId: f.companyId, issueId: f.issue.id, actor: board, + selection: { kind: "configured_source", projectWorkspaceId: source!.id, mode: "managed_isolated" }, + expectedBindingRevision: 0, requestKey: randomUUID() }); + } + expect(await taskInterruptionPresentation(db, moved!)).toMatchObject({ canRetry: true, projectAccess: { ready: true } }); + // The original denied source was not granted, and presentation creates no execution. + expect((await db.select().from(agents).where(eq(agents.id, f.agentId)))[0]!.permissions).toEqual(f.permissions); + expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); + if (repair === "pending") { + await db.update(issues).set({ workspaceBindingRevision: 2 }).where(eq(issues.id, f.issue.id)); + const stale = await taskInterruptionPresentation(db, moved!); + expect(stale).toMatchObject({ state: "failed", canRetry: false, userAction: null }); + expect(stale).not.toHaveProperty("projectAccess"); + } + }); + + it("keeps a pending existing selection blocked if its workspace is archived before retry", async () => { + const f = await seed(); + const task = await issueService(db).update(f.issue.id, { projectId: f.selected[0]!.id }); + const [workspace] = await db.insert(executionWorkspaces).values({ companyId: f.companyId, + projectId: f.selected[0]!.id, name: "Selected files", cwd: "/tmp/project-access-selected", + mode: "isolated_workspace", strategyType: "git_worktree" }).returning(); + const service = executionWorkspaceService(db); + await service.selectTaskWorkspace({ companyId: f.companyId, issueId: f.issue.id, actor: board, + selection: { kind: "existing", workspaceId: workspace!.id }, expectedBindingRevision: 0, requestKey: randomUUID() }); + expect(await taskInterruptionPresentation(db, task!)).toMatchObject({ canRetry: true, projectAccess: { ready: true } }); + expect(await service.archiveWorkspaceUnderLifecycleLock({ id: workspace!.id, patch: {}, closedAt: new Date() })) + .toMatchObject({ outcome: "archived" }); + await expect(service.applyPendingTaskWorkspaceSelection({ companyId: f.companyId, issueId: f.issue.id, + runId: f.runId, actor: { type: "agent", source: "agent_key", agentId: f.agentId, companyId: f.companyId } })) + .rejects.toThrow("Workspace is unavailable or inaccessible"); + const archived = await taskInterruptionPresentation(db, task!); + expect(archived).toMatchObject({ state: "failed", canRetry: false, userAction: null }); + expect(archived).not.toHaveProperty("projectAccess"); + }); + + it.each(["denied_project", "foreign_company"] as const)("retains the %s gate on a bound workspace even when its source is allowed", async boundary => { + const f = await seed(), foreign = boundary === "foreign_company" ? await seed() : null; + const [source] = await db.insert(projectWorkspaces).values({ companyId: f.companyId, projectId: f.selected[0]!.id, + name: "Allowed source", cwd: "/tmp/project-access-source" }).returning(); + const [workspace] = await db.insert(executionWorkspaces).values({ companyId: foreign?.companyId ?? f.companyId, + projectId: foreign?.selected[0]!.id ?? f.selected[1]!.id, projectWorkspaceId: foreign ? null : source!.id, + name: "Retained files", cwd: "/tmp/project-access-bound", mode: "isolated_workspace", strategyType: "git_worktree" }).returning(); + const [task] = await db.update(issues).set({ projectId: f.selected[0]!.id, projectWorkspaceId: source!.id, + executionWorkspaceId: workspace!.id }).where(eq(issues.id, f.issue.id)).returning(); + const blocked = await taskInterruptionPresentation(db, task!); + if (boundary === "denied_project") { + expect(blocked).toMatchObject({ canRetry: false, + userAction: "project_access", projectAccess: { ready: false, projectId: null } }); + } else { + expect(blocked).toMatchObject({ state: "failed", canRetry: false, userAction: null }); + expect(blocked).not.toHaveProperty("projectAccess"); + } + }); + it("preserves containment, audits concurrent additions, and changes the persisted denial into an explicit retry", async () => { const f = await seed(); const actor = { type: "agent" as const, source: "agent_key" as const, agentId: f.agentId, companyId: f.companyId }; diff --git a/server/src/services/task-interruption-presentation.ts b/server/src/services/task-interruption-presentation.ts index f8dd6664e33..eb489f1ef6d 100644 --- a/server/src/services/task-interruption-presentation.ts +++ b/server/src/services/task-interruption-presentation.ts @@ -6,6 +6,7 @@ import { isAiAuthenticationBlocked } from "./ai-auth-failure.js"; import { readSystemRecovery } from "./system-recovery-state.js"; import { PROJECT_ACCESS_DENIED } from "./project-access-denial.js"; import { authorizationService } from "./authorization.js"; +import { taskWorkspaceProjectRequirements } from "./task-workspace-access.js"; type ProjectionIssue = Pick; type ProjectionRun = Pick; projectAccessAllowed?: boolean; + projectAccessProjectId?: string | null; + workspaceSelectionAvailable?: boolean; project?: Pick; company?: Pick; paused: boolean; @@ -30,7 +33,7 @@ function deniedProjectId(issue: ProjectionIssue, run?: ProjectionRun): string | || (run.errorCode !== PROJECT_ACCESS_DENIED && !historical)) return undefined; const detail = run.resultJson?.projectAccessDenied; return detail && typeof detail === "object" && "projectId" in detail && typeof detail.projectId === "string" - ? detail.projectId : historical ? issue.projectId : null; + ? detail.projectId : null; } /** Presentation never grants authority. Dispatch repeats all gates under its @@ -53,8 +56,11 @@ export function projectTaskInterruption(issue: ProjectionIssue, input: Projectio const deniedProject = deniedProjectId(issue, run); let projectAccess: TaskInterruptionPresentation["projectAccess"]; if (deniedProject !== undefined) { + // Permissions cannot repair a stale or unavailable workspace selection. + if (input.workspaceSelectionAvailable === false) return { ...base, state: "failed" }; const ready = input.projectAccessAllowed === true; - projectAccess = { agentId: run.agentId!, projectId: deniedProject === issue.projectId ? deniedProject : null, ready }; + const projectId = input.projectAccessProjectId === undefined ? deniedProject : input.projectAccessProjectId; + projectAccess = { agentId: run.agentId!, projectId: projectId === issue.projectId ? projectId : null, ready }; if (!ready) return { ...base, responsibility: "user", state: "waiting_for_user", userAction: "project_access", projectAccess }; } @@ -120,13 +126,17 @@ export async function taskInterruptionPresentations(db: Db, companyId: string, i // Recheck current authority, including standard trust, root-issue grants and // project policy. Do not reuse the failed run's frozen policy for a new retry. // Only denied tasks need these reads; deduplicate projects and batch per agent. + // A task's organizational project can differ from its retained source. Old + // failures did not record the denied project, and even typed failures can + // become obsolete after a workspace selection changes. + const deniedIssues = selected.filter(issue => issue.assigneeAgentId && !["done", "cancelled"].includes(issue.status) + && deniedProjectId(issue, runMap.get(issue.id)) !== undefined); + const requirements = await taskWorkspaceProjectRequirements(db, companyId, deniedIssues.map(issue => issue.id)); const deniedByAgent = new Map>(); - for (const issue of selected) { - const projectId = deniedProjectId(issue, runMap.get(issue.id)); - if (!projectId || !issue.assigneeAgentId || ["done", "cancelled"].includes(issue.status)) continue; - const ids = deniedByAgent.get(issue.assigneeAgentId) ?? new Set(); - ids.add(projectId); - deniedByAgent.set(issue.assigneeAgentId, ids); + for (const issue of deniedIssues) { + const ids = deniedByAgent.get(issue.assigneeAgentId!) ?? new Set(); + for (const projectId of requirements.get(issue.id)?.projectIds ?? []) ids.add(projectId); + if (ids.size) deniedByAgent.set(issue.assigneeAgentId!, ids); } const currentAccess = new Map(); const authorization = authorizationService(db); @@ -140,12 +150,18 @@ export async function taskInterruptionPresentations(db: Db, companyId: string, i const agentMap = new Map(agentRows.map(agent => [agent.id, agent])); const projectMap = new Map(projectRows.map(project => [project.id, project])); const pauses = new Set(pauseRows.map(row => row.issueId)), approvalsSet = new Set(approvalRows.map(row => row.issueId)); - for (const issue of selected) result.set(issue.id, projectTaskInterruption(issue, { - run: runMap.get(issue.id), action: actionMap.get(issue.id), wait: waitMap.get(issue.id), - projectAccessAllowed: currentAccess.get(`${issue.assigneeAgentId}:${deniedProjectId(issue, runMap.get(issue.id))}`), - agent: agentMap.get(issue.assigneeAgentId ?? ""), project: projectMap.get(issue.projectId ?? ""), - company: companyRows[0], paused: pauses.has(issue.id), approval: approvalsSet.has(issue.id), - })); + for (const issue of selected) { + const required = requirements.get(issue.id); + const denied = required?.projectIds.filter(projectId => currentAccess.get(`${issue.assigneeAgentId}:${projectId}`) !== true); + result.set(issue.id, projectTaskInterruption(issue, { + run: runMap.get(issue.id), action: actionMap.get(issue.id), wait: waitMap.get(issue.id), + projectAccessAllowed: denied?.length === 0, + projectAccessProjectId: denied?.includes(issue.projectId ?? "") ? issue.projectId : null, + workspaceSelectionAvailable: required?.available ?? false, + agent: agentMap.get(issue.assigneeAgentId ?? ""), project: projectMap.get(issue.projectId ?? ""), + company: companyRows[0], paused: pauses.has(issue.id), approval: approvalsSet.has(issue.id), + })); + } return result; } diff --git a/server/src/services/task-workspace-access.ts b/server/src/services/task-workspace-access.ts index 31d40f79655..0c4599982e1 100644 --- a/server/src/services/task-workspace-access.ts +++ b/server/src/services/task-workspace-access.ts @@ -10,9 +10,59 @@ import { resolveCoreTrustPreset } from "./trust-preset-resolver.js"; import { isDeepStrictEqual } from "node:util"; import { ProjectAccessDeniedError } from "./project-access-denial.js"; import { taskWorkspaceSelectionSchema, type TaskWorkspaceSelection } from "@paperclipai/shared"; +import { taskWorkspaceSelectableCondition } from "./task-workspace-selection.js"; type DbTransaction = Parameters[0]>[0]; +/** Read the project requirements of the next selected workspace, without + * admitting execution or moving files. Batch only the tasks needing a denial + * presentation; this adds at most three bounded reads, not queries per task. */ +export async function taskWorkspaceProjectRequirements(db: Db, companyId: string, issueIds: string[]) { + const result = new Map(); + if (!issueIds.length) return result; + const tasks = await db.select({ id: issues.id, projectId: issues.projectId, + executionWorkspaceId: issues.executionWorkspaceId, projectWorkspaceId: issues.projectWorkspaceId, + workspaceSelection: issues.workspaceSelection, workspacePendingSelection: issues.workspacePendingSelection, + workspaceBindingRevision: issues.workspaceBindingRevision, + }).from(issues).where(and(eq(issues.companyId, companyId), inArray(issues.id, issueIds))); + const selections = tasks.map(task => { + const pending = task.workspacePendingSelection; + const intent = pending?.intent ?? (!task.executionWorkspaceId ? task.workspaceSelection : null); + const parsed = intent ? taskWorkspaceSelectionSchema.safeParse(intent.selection) : null; + const selection = parsed?.success ? parsed.data : null; + const available = (!pending || pending.expectedBindingRevision === task.workspaceBindingRevision) + && (!parsed || parsed.success); + const workspaceId = selection ? selection.kind === "existing" ? selection.workspaceId : null : task.executionWorkspaceId; + return { task, selection, available, workspaceId }; + }); + const workspaceIds = [...new Set(selections.flatMap(row => row.workspaceId ? [row.workspaceId] : []))]; + const workspaces = workspaceIds.length ? await db.select({ id: executionWorkspaces.id, + projectId: executionWorkspaces.projectId, projectWorkspaceId: executionWorkspaces.projectWorkspaceId, + selectable: sql`${taskWorkspaceSelectableCondition()}`, + }).from(executionWorkspaces).where(and(eq(executionWorkspaces.companyId, companyId), inArray(executionWorkspaces.id, workspaceIds))) : []; + const workspaceMap = new Map(workspaces.map(workspace => [workspace.id, workspace])); + const sources = selections.map(row => { + const workspace = row.workspaceId ? workspaceMap.get(row.workspaceId) : undefined; + const sourceId = row.selection + ? row.selection.kind === "configured_source" ? row.selection.projectWorkspaceId : workspace?.projectWorkspaceId + : workspace?.projectWorkspaceId ?? row.task.projectWorkspaceId; + return { ...row, workspace, sourceId }; + }); + const sourceIds = [...new Set(sources.flatMap(row => row.sourceId ? [row.sourceId] : []))]; + const sourceRows = sourceIds.length ? await db.select({ id: projectWorkspaces.id, projectId: projectWorkspaces.projectId }) + .from(projectWorkspaces).where(and(eq(projectWorkspaces.companyId, companyId), inArray(projectWorkspaces.id, sourceIds))) : []; + const sourceMap = new Map(sourceRows.map(source => [source.id, source])); + for (const row of sources) { + const source = row.sourceId ? sourceMap.get(row.sourceId) : undefined; + result.set(row.task.id, { + projectIds: [...new Set([row.task.projectId, row.workspace?.projectId, source?.projectId].filter((id): id is string => Boolean(id)))], + available: row.available && (!row.workspaceId || Boolean(row.workspace)) && (!row.sourceId || Boolean(source)) + && (row.selection?.kind !== "existing" || row.workspace?.selectable === true), + }); + } + return result; +} + export async function assertTaskWorkspaceSourceProjectAccess( db: Db | DbTransaction, actor: AuthorizationActor, companyId: string, projectId: string | null, ) {