mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 16:35:48 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Sandbox runs must restore their workspace before finalization can succeed. > - Restore diagnostics identify the failed phase and operation. > - A Git integration exit code can still describe several different failures. > - This pull request adds fixed command labels and supported failure classes. > - Operators can distinguish these failures without collecting private Git output. ## Linked Issues or Issue Description Refs #15005. This branch includes merged #15268. It labels that change's locked ref transaction and nested branch probe without changing their behavior. **What happened?** A failed Git integration can report only `git_integration`, `unknown`, and an exit code. That evidence does not identify the failed command. Some Git versions also return exit 1 for both a merge conflict and an invalid object. **Expected behavior** Record a fixed command family and a supported failure class. Keep unknown cases as `unknown`. Exclude command arguments, process output, paths, repository URLs, filenames, and ref names. **Steps to reproduce** The tests create local repositories with conflicting commits, a missing object, and an expected-old ref mismatch. They call the real Git operations and inspect the resulting diagnostic. No hosted workspace or external provider is used. **Paperclip version or commit** Base: `e99854249c`. **Deployment mode** Built from source. The diagnostic applies to sandbox workspace restore. ## What Changed - Label Git integration calls with a closed command enum. Preserve arguments, options, errors, and retry behavior. - Recognize supported object and ref errors and OS permission codes. Require both exit 1 and completed tree output for a merge conflict. - Carry the closed fields through the existing restore receipt, saved adapter result, and Sentry projection. Revalidate saved metadata before projection. - Test nested wrappers, parallel failures, reused errors, handled probes, result settlement, and privacy with the real Sentry SDK. - Document the field contract and its limits. ## Verification - Eight focused suites pass: 295 tests. These cover Git sync, restore diagnostics, result settlement, teardown, sandbox runtime, failure projection, the real Sentry SDK, and native warm-workspace Git history. - `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1 pnpm exec vitest run packages/adapter-utils/src/workspace-restore-diagnostics.test.ts packages/adapter-utils/src/workspace-restore-result.test.ts packages/adapter-utils/src/git-workspace-sync.test.ts packages/adapter-utils/src/workspace-restore-teardown.test.ts packages/adapter-utils/src/sandbox-managed-runtime.test.ts server/src/services/__tests__/run-failure-diagnostics.test.ts server/src/__tests__/run-failure-sentry-real-sdk.test.ts server/src/__tests__/native-workspace-sync-history.test.ts` - Local checks use Node 24.21.0 and the pinned pnpm 9.15.4. The optional Sentry SDK is pinned to the declared 10.71.0 and uses an in-memory transport. - `pnpm -r typecheck` and `pnpm build`: pass on the rebased head. - The full local `pnpm test:run` was stopped before source changes when #15268 merged and required a rebase. It reported three pre-existing company-skill cache test failures on macOS. The same three cases fail on clean bases `2c43b39167` and `e99854249c` and the earlier head with `EACCES` when publishing a read-only cache directory. The relevant test, service, and cache source blobs are identical. No cache changes are included. Later local test groups were not reached. - All 54 checks pass on exact head `8448896bc6`, including the post-ready security scan, with two intentional Storybook skips. Greptile scores this head 5/5 with no review threads. Full Linux CI covers the local groups that were not reached. - Independent review found no blocker. Its diagnostics, Git workspace, and native history suites pass 115/115 on this head. - A source comparison confirms that removing only diagnostic wrappers yields the merged upstream Git integration code exactly, including ref locks, transaction protocol, arguments, options, and retry decisions. - The clean base has stale dependency overrides in its lockfile. Local installation resolved them as the existing PR CI fallback does. No manifest or lockfile change is included. - `git diff --check` and a local secrets/PII review pass. ## Risks A Git version or localized message may not match a known failure form. Such cases remain `unknown`. Up to 16 KiB of stderr and the bounded tree-ID prefix of stdout are inspected only in memory. The saved fields contain enum values only. These diagnostics do not establish workspace recovery or authorize retries. Restore decisions and Git mutations are unchanged. No schema migration is required. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The service does not expose the exact model deployment ID or 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 - [x] All Paperclip CI gates are green - [x] 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>
86 lines
5.4 KiB
TypeScript
86 lines
5.4 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import { applyWorkspaceRestoreFailure, withWorkspaceRestore } from "./workspace-restore-result.js";
|
|
import type { AdapterExecutionResult } from "./types.js";
|
|
import { withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, withWorkspaceRestoreGitCommand } from "./workspace-restore-diagnostics.js";
|
|
|
|
const completed: AdapterExecutionResult = {
|
|
exitCode: 0, signal: null, timedOut: false,
|
|
summary: "The plan is ready.", sessionId: "session", usage: { inputTokens: 4, outputTokens: 9 },
|
|
resultJson: { requestId: "request" },
|
|
};
|
|
const unsafe = new Error("Daytona syncOut refusing tarball link whose target escapes the extraction dir: .claude/skills/paperclip -> /tmp/private-clone/secret-key");
|
|
|
|
describe("workspace restore settlement", () => {
|
|
it("retains only the safe failed restore step alongside the original completed turn", async () => {
|
|
const error = Object.assign(new Error("private-restore-path and command"), { code: 1, stderr: "private-restore-output" });
|
|
const result = await withWorkspaceRestore(async () => completed, () => withWorkspaceRestoreDiagnostics("workspace", () =>
|
|
withWorkspaceRestoreStep("index_reset", async () => { throw error; })));
|
|
expect(result.resultJson).toMatchObject({
|
|
workspaceRestoreFailure: "restore_failed",
|
|
workspaceRestoreDiagnostic: { phase: "workspace", step: "index_reset", errorCode: "unknown", exitCode: 1 },
|
|
executionBeforeRestore: { exitCode: 0, timedOut: false, errorCode: null },
|
|
});
|
|
expect(JSON.stringify(result)).not.toContain("private-restore-");
|
|
});
|
|
|
|
it("persists only bounded Git evidence without replacing the completed model result", async () => {
|
|
const error = Object.assign(new Error("private-git-command /private/workspace"), {
|
|
code: 1, stdout: "a".repeat(40) + "\nprivate-git-file contents", stderr: "private-git-refs",
|
|
});
|
|
const result = await withWorkspaceRestore(async () => completed, () => withWorkspaceRestoreDiagnostics("workspace", () =>
|
|
withWorkspaceRestoreStep("git_integration", () => withWorkspaceRestoreGitCommand("merge_tree", async () => { throw error; }))));
|
|
expect(result).toMatchObject({
|
|
errorCode: "workspace_restore_failed", exitCode: completed.exitCode, summary: completed.summary, usage: completed.usage,
|
|
resultJson: { requestId: "request", workspaceRestoreFailure: "restore_failed",
|
|
workspaceRestoreDiagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 1,
|
|
gitCommand: "merge_tree", gitFailureKind: "merge_conflict" },
|
|
executionBeforeRestore: { exitCode: 0, timedOut: false, errorCode: null } },
|
|
});
|
|
expect(JSON.stringify(result)).not.toContain("private-git-");
|
|
});
|
|
|
|
it("retains the completed result and safe member while excluding the unsafe target", async () => {
|
|
const result = await withWorkspaceRestore(async () => completed, async () => { throw unsafe; });
|
|
expect(result).toMatchObject({
|
|
summary: completed.summary, sessionId: "session", usage: completed.usage,
|
|
errorCode: "workspace_restore_failed", timedOut: false,
|
|
resultJson: { requestId: "request", workspaceRestoreFailure: "restore_unsafe_archive",
|
|
workspaceRestorePath: ".claude/skills/paperclip", finalResponseRecorded: true },
|
|
});
|
|
expect(JSON.stringify(result)).not.toMatch(/private-clone|secret-key|\/tmp/);
|
|
expect(applyWorkspaceRestoreFailure(result)).toBe(result);
|
|
});
|
|
|
|
it("retains the previous execution failure and restore classification", async () => {
|
|
const result = await withWorkspaceRestore(async () => ({ ...completed, exitCode: 2, errorCode: "model_error", errorMessage: "Model request failed." }), async () => { throw unsafe; });
|
|
expect(result.errorMessage).toContain("Model request failed.");
|
|
expect(result.errorMessage).toContain("Workspace restore failed.");
|
|
expect(result.resultJson?.executionBeforeRestore).toMatchObject({ errorCode: "model_error", exitCode: 2 });
|
|
});
|
|
|
|
it("retains a thrown execution error when restore also fails", async () => {
|
|
const result = await withWorkspaceRestore(async () => { throw new Error("Process failed."); }, async () => { throw unsafe; });
|
|
expect(result.errorMessage).toContain("Process failed.");
|
|
expect(result.resultJson).toMatchObject({ finalResponseRecorded: false, executionBeforeRestore: { errorCode: "adapter_failed" } });
|
|
});
|
|
|
|
it("keeps timeout evidence while making restore the terminal failure phase", async () => {
|
|
const result = await withWorkspaceRestore(async () => ({ ...completed, timedOut: true }), async () => { throw unsafe; });
|
|
expect(result.timedOut).toBe(false);
|
|
expect(result.resultJson?.executionBeforeRestore).toMatchObject({ timedOut: true });
|
|
});
|
|
|
|
it("preserves the original result or thrown error after a clean restore", async () => {
|
|
expect(await withWorkspaceRestore(async () => completed, async () => {})).toBe(completed);
|
|
const error = new Error("execution failed");
|
|
await expect(withWorkspaceRestore(async () => { throw error; }, async () => {})).rejects.toBe(error);
|
|
});
|
|
|
|
it.each(["/tmp/private/file", "../escape", "C:\\private\\file"])("omits unsafe member %s", async (member) => {
|
|
const result = await withWorkspaceRestore(async () => completed, async () => {
|
|
throw new Error(`Daytona syncOut refusing tarball link whose target escapes the extraction dir: ${member} -> /secret`);
|
|
});
|
|
expect(result.resultJson?.workspaceRestorePath).toBeUndefined();
|
|
});
|
|
});
|