mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(workspaces): preserve task-only warnings during offline migration (#19144)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Ephemeral web servers must save existing workspaces before volume
removal.
> - Offline migration checks file ownership and recovery authority.
> - The same gate also rejects task assignment and configuration
warnings.
> - Those warnings do not require the old filesystem.
> - This pull request preserves known task-only warnings while it saves
files.
> - File recovery and execution obligations keep their existing
blockers.
## Linked Issues or Issue Description
Refs #18943, #19038, #19138.
**What happened?**
Offline workspace migration refuses tasks with assignment,
configuration, disposition or deliberate-wait warnings. It returns
`workspace_recovery_unresolved` even when the warning has no file or
execution recovery authority.
**Expected behavior**
Save local workspace bytes while retaining the task warning unchanged.
Continue to block unknown recovery kinds, file sources, export failures
and execution recovery work.
**Steps to reproduce**
1. Retain a local workspace on a task with an active assignment or
configuration warning.
2. Stop all app writers through the offline maintenance entry.
3. Attempt workspace migration.
## What Changed
- Admit four known task-only warning kinds during offline file
migration.
- Keep file recovery markers, system recovery, native failure markers,
export-failure causes and unknown kinds blocked unless their exact
legacy source is already preserved.
- Keep the existing live-migration recovery exclusion.
- Recheck warning evidence before publication. Do not resolve warnings
or change their owners.
- Add restoration, rejection and publication-race tests. Update the
migration plan.
## Verification
- 112 focused migration and concurrency tests pass with real PostgreSQL
and filesystem archives.
- The four task-only restoration cases fail against the previous
importer with `workspace_recovery_unresolved`.
- Tests delete the old home, restore its saved bytes and compare each
warning row unchanged.
- File and execution markers, unknown kinds and export failures remain
blocked.
- A task warning that gains file recovery during upload prevents
publication.
- Local server typecheck has no changed-file diagnostics. Stale linked
dependency declarations prevent a clean local whole-server result. Clean
full CI remains required before merge.
- The PR was rebased onto the merged Git preservation fix after
resolving a plan-document conflict. The rebased head passed full PR CI
(41943153422). Current head ba18efc7b5 adds the reviewed deliberate-wait
case and reuses the canonical export-cause set; fresh full CI is
required. The earlier unmergeable head did not have a PR workflow run.
- Serving enrollment and full copied-data acceptance remain separate
gates.
## Risks
- The allowlist must remain narrow. New warning kinds are blocked by
default.
- A known task-only warning with file or execution recovery evidence
still blocks migration.
- This change depends on the existing offline provider fence. It does
not establish that old app writers stopped.
## Model Used
OpenAI GPT-6.1 Codex, exact model ID `gpt-6.1-sol`. Used for reasoning,
code editing, shell tools and test execution. The runtime does not
expose its context-window size.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
19c710fe93
commit
c8013d06ad
3 files changed
+63
-5
No files matched your search
@@ -205,3 +205,12 @@ and rejection at the archive limit before any object or row is published. This
|
||||
finite offline operation adds no idle background work. A resumed rehearsal can
|
||||
measure the fix but cannot replace fresh full-image migration and empty-home
|
||||
recovery proof.
|
||||
|
||||
During offline migration, known task-only assignment, configuration, deliberate-wait and
|
||||
disposition warnings can remain unchanged while the owner saves local files.
|
||||
They do not certify file recovery and are not resolved by migration. An active
|
||||
system recovery, native execution failure marker, declared workspace recovery
|
||||
source, export-failure cause or unknown recovery kind retains the existing
|
||||
blocker unless the importer already records that exact legacy recovery source.
|
||||
Live migration keeps its recovery exclusion. Recheck recovery evidence before
|
||||
publishing the saved workspace.
|
||||
@@ -9,7 +9,7 @@ import { pathToFileURL } from "node:url";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { activityLog, agents, environmentLeases, heartbeatRuns, nativeRunFinalizations, nativeRunResults, completionContracts, applyPendingMigrations, closeRegisteredClients, companies, createDb, executionWorkspaceSnapshots,
|
||||
executionWorkspaceRuntimeLeases, executionWorkspaces, issues, projects, projectWorkspaces, workspaceOperations, workspaceRuntimeServices } from "@paperclipai/db";
|
||||
executionWorkspaceRuntimeLeases, executionWorkspaces, issueRecoveryActions, issues, projects, projectWorkspaces, workspaceOperations, workspaceRuntimeServices } from "@paperclipai/db";
|
||||
import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
||||
import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js";
|
||||
import { createStorageService } from "../storage/service.js";
|
||||
@@ -151,6 +151,45 @@ describe.skipIf(!support.supported)("remote workspace migration", () => {
|
||||
expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run!.id)))[0]).toEqual(run);
|
||||
expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).not.toBeNull();
|
||||
});
|
||||
it.each([["stranded_assigned_issue", "stranded_assigned_issue"], ["configuration_validation", "configuration_incomplete"],
|
||||
["missing_disposition", "successful_run_missing_state"], ["deliberate_wait_without_target", "deliberate_wait_without_target"]])("preserves a task-only %s warning through offline file migration", async (kind, cause) => {
|
||||
const f = await seed();
|
||||
const [action] = await db.insert(issueRecoveryActions).values({ companyId: f.companyId, sourceIssueId: f.issueId,
|
||||
kind, cause, fingerprint: randomUUID(), nextAction: "Keep the existing task warning", evidence: { latestRunStatus: "failed" } }).returning();
|
||||
await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("workspace_recovery_unresolved");
|
||||
vi.stubEnv("PAPERCLIP_EPHEMERAL_MIGRATION", "true");
|
||||
await migrateRemoteWorkspace({ db, storage, ...f, apply: true, writersStopped: true });
|
||||
expect((await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, action!.id)))[0]).toEqual(action);
|
||||
const authority = readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)!;
|
||||
const ref = await getWorkspaceSnapshot(db, { ...f, snapshotId: authority.snapshotId! });
|
||||
const archivePath = path.join(root, "task-warning.pcws"), targetDir = path.join(root, "restored-task-warning");
|
||||
await fs.rm(f.cwd, { recursive: true });
|
||||
await loadWorkspaceSnapshotArchive({ storage, reference: ref!, targetPath: archivePath });
|
||||
await restoreRemoteWorkspaceArchive({ archivePath, targetDir, expectedManifestDigest: ref!.manifestDigest });
|
||||
expect(await fs.readFile(path.join(targetDir, "saved.txt"), "utf8")).toBe("original saved work\n");
|
||||
});
|
||||
it.each(["workspaceRestoreRecovery", "workspaceRecovery", "historicalWorkspaceSource", "systemRecovery", "nativeFailureBlock", "export-cause", "unknown-kind", "workspace_validation", "active_run_watchdog"])("retains unresolved file or execution recovery as an offline blocker (%s)", async mode => {
|
||||
const f = await seed(); vi.stubEnv("PAPERCLIP_EPHEMERAL_MIGRATION", "true");
|
||||
await db.insert(issueRecoveryActions).values({ companyId: f.companyId, sourceIssueId: f.issueId,
|
||||
kind: mode === "unknown-kind" ? "future_recovery" : ["workspace_validation", "active_run_watchdog"].includes(mode) ? mode : "configuration_validation",
|
||||
cause: mode === "export-cause" ? "native_workspace_sync_out_unrecoverable" : mode === "workspace_validation" ? "workspace_validation_failed"
|
||||
: mode === "active_run_watchdog" ? "native_session_interrupted" : "configuration_incomplete",
|
||||
fingerprint: randomUUID(), nextAction: "Preserve unresolved recovery", evidence: ["export-cause", "unknown-kind", "workspace_validation", "active_run_watchdog"].includes(mode) ? {}
|
||||
: { [mode]: mode === "systemRecovery" ? { state: "running" } : { schema: "unverified" } } });
|
||||
await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true, writersStopped: true })).rejects.toThrow("workspace_recovery_unresolved");
|
||||
expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull();
|
||||
});
|
||||
it("rechecks a task warning that becomes file recovery during upload", async () => {
|
||||
const f = await seed(); vi.stubEnv("PAPERCLIP_EPHEMERAL_MIGRATION", "true");
|
||||
const [action] = await db.insert(issueRecoveryActions).values({ companyId: f.companyId, sourceIssueId: f.issueId,
|
||||
kind: "configuration_validation", cause: "configuration_incomplete", fingerprint: randomUUID(), nextAction: "Repair configuration" }).returning();
|
||||
const put = storage.putFile.bind(storage);
|
||||
storage.putFile = async input => { const result = await put(input);
|
||||
await db.update(issueRecoveryActions).set({ evidence: { historicalWorkspaceSource: { schema: "paperclip.historical-workspace-source.v1" } } })
|
||||
.where(eq(issueRecoveryActions.id, action!.id)); return result; };
|
||||
await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true, writersStopped: true })).rejects.toThrow("workspace_recovery_unresolved");
|
||||
expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull();
|
||||
});
|
||||
it("detects overlapping folders reached through symlinked parent directories", async () => {
|
||||
const f = await seed(), alias = path.join(root, "alias"); await fs.symlink(root, alias);
|
||||
await db.insert(executionWorkspaces).values({ companyId: f.companyId, name: "Physical alias", mode: "isolated_workspace",
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "@paperclipai/db";
|
||||
import { readRemoteWorkspaceAuthority, remoteWorkspaceSourceSchema, type RemoteWorkspaceAuthority, type RemoteWorkspaceSource } from "@paperclipai/shared";
|
||||
import { offlineMigrationExecutionWhere } from "./offline-migration-execution.js";
|
||||
import { workspaceExportCauses } from "./workspace-recovery-source.js";
|
||||
import type { StorageService } from "../storage/types.js";
|
||||
import { persistActivity, publishActivity } from "./activity-log.js";
|
||||
import { withExecutionWorkspaceLifecycleLock } from "./execution-workspace-lifecycle-lock.js";
|
||||
@@ -202,13 +203,22 @@ async function inspect(db: ReadDb, input: { companyId: string; workspaceId: stri
|
||||
)).limit(1);
|
||||
if (service) reasons.push("workspace_runtime_service_unsettled");
|
||||
const recoveryIssueIds = [...new Set([workspace.sourceIssueId, ...legacyRecoverySources.map(source => source.issueId)].filter((id): id is string => Boolean(id)))];
|
||||
const recoveries = recoveryIssueIds.length ? await db.select({ id: issueRecoveryActions.id, evidence: issueRecoveryActions.evidence }).from(issueRecoveryActions).where(and(
|
||||
const recoveries = recoveryIssueIds.length ? await db.select({ id: issueRecoveryActions.id, kind: issueRecoveryActions.kind,
|
||||
cause: issueRecoveryActions.cause, evidence: issueRecoveryActions.evidence }).from(issueRecoveryActions).where(and(
|
||||
eq(issueRecoveryActions.companyId, workspace.companyId), inArray(issueRecoveryActions.sourceIssueId, recoveryIssueIds),
|
||||
inArray(issueRecoveryActions.status, ["active", "escalated"]),
|
||||
)) : [];
|
||||
if (recoveries.some(recovery => !input.writersStopped
|
||||
|| !legacyRecoverySources.some(source => source.runId === recovery.evidence?.runId)
|
||||
|| (recovery.evidence?.systemRecovery as { state?: unknown } | undefined)?.state === "running")) reasons.push("workspace_recovery_unresolved");
|
||||
if (recoveries.some(recovery => {
|
||||
if (!input.writersStopped || object(recovery.evidence?.systemRecovery).state === "running") return true;
|
||||
if (legacyRecoverySources.some(source => source.runId === recovery.evidence?.runId)) return false;
|
||||
// These warnings govern task disposition, not file authority. Keep them
|
||||
// unchanged while the offline owner saves the workspace. Unknown recovery
|
||||
// kinds and any declared file source still require explicit preservation.
|
||||
return !["stranded_assigned_issue", "configuration_validation", "missing_disposition", "deliberate_wait_without_target"].includes(recovery.kind)
|
||||
|| workspaceExportCauses.includes(recovery.cause)
|
||||
|| ["workspaceRestoreRecovery", "workspaceRecovery", "historicalWorkspaceSource", "systemRecovery", "nativeFailureBlock"]
|
||||
.some(key => recovery.evidence?.[key] != null);
|
||||
})) reasons.push("workspace_recovery_unresolved");
|
||||
const repositories = await db.select().from(executionWorkspaceRepositories).where(and(eq(executionWorkspaceRepositories.companyId, workspace.companyId),
|
||||
eq(executionWorkspaceRepositories.executionWorkspaceId, workspace.id)));
|
||||
if (repositories.some(repository => repository.state !== "ready")) reasons.push("repository_preparation_unresolved");
|
||||
|
||||
Reference in new issue
Block a user