mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(runner-e2e): find saved slugify source in a workspace project folder (#19471)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The runner E2E everyday `recover-controller` case proves that a task survives a control-plane restart. > - The case waits for an active run with a saved `slugify.py` before it kills the controller. > - The harness looked for `slugify.py` only at the workspace root. > - The task brief asks for `slugify.py` "in the project workspace". It does not ask for the root. ACPX Claude saved it in `slugify-tool/slugify.py`. > - The harness timed out before the restart, so the case did not test restart recovery at all. > - This pull request makes the harness find the saved source in a bounded set of project folders. > - The benefit is that the case reaches its real fault boundary for agents that make a project folder. ## Linked Issues or Issue Description **What happened?** The local cell `everyday-workflows.runner-acpx-claude.local.recover-controller` failed with "Timed out waiting for active run with saved source". The agent finished the task and delivered a correct ZIP. It saved `slugify.py` in `<workspace>/slugify-tool/`. The harness read only `<workspace>/slugify.py`, so it never saw the saved source. It also recorded `saved-source-unavailable-at-final-capture`. The controller restart never happened. **Expected behavior** The harness finds the source that the agent saved in the project workspace. Then it kills the controller while the run is active and grades the recovery. **Steps to reproduce** 1. Run `pnpm test:e2e:runner -- --max-automatic-retries 0 --id everyday-workflows.runner-acpx-claude.local.recover-controller` with a provider that saves the project in a subfolder. 2. See the timeout before the `controller-restarted` timeline entry. **Paperclip version or commit** `master` at `6cafcaf89b` (also reproduced at `d87f6fa4c2`). ## What Changed - Added `readWorkspaceSource` in `tests/runner-e2e/everyday-interruption.ts`. It reads `slugify.py` at the workspace root first. Then it searches subfolders to depth 2, shallowest and alphabetical first. It skips hidden folders, `node_modules`, `__pycache__` and `venv`. - `recordSource` (the stop-redirect and recover-controller boundaries) and the final local source capture in `tests/runner-e2e/everyday-flow.ts` use this helper. - The stop-redirect workspace snapshot (`workspaceFiles`) now includes the same bounded project folders. A stop check therefore still sees edits when the project is below the root. - Added `tests/runner-e2e/everyday-interruption.test.ts`. ## Verification - Red before the fix: `npx vitest run --config tests/runner-e2e/vitest.config.ts everyday-interruption` failed 2 of 4 tests (the nested-folder cases returned `undefined`). - Green after the fix: the same command passes 4 of 4. - `pnpm test:e2e:runner:typecheck` passes. - Live rerun of the cell after the fix (local, native ACPX Claude, `claude-sonnet-5`): the harness observed the active run with saved source, submitted the follow-up, and restarted the controller (`source-saved-before-interruption` with `active: true`, then `controller-restarted`). The original timeout did not occur. - Two valid live reruns after the fix both reached the restart. Both still did not pass. After the restart, the interrupted ACPX turn ended as `native_provider_terminal_failed: Provider turn failed`. In the second rerun the task stayed Blocked with the follow-up comment still queued and a Retry offered. This is a separate product signature. It is not in the scope of this harness fix. A follow-up handles it (see Risks). - One earlier rerun failed before the task started because the new worktree had no built runner binary (`paperclip_runner_binary_missing`). That was local setup only. The runner was then built with `pnpm --filter @paperclipai/paperclip-runner build`. ## Risks - Low risk. The change is test-harness only. - If a workspace has several `slugify.py` files, the harness picks the root copy, else the shallowest copy. A stale copy in another folder could be chosen. The search depth and skip list keep this bounded. - Known remaining failure in the same cell (separate signature): after a hard controller restart, the ACPX runner restore path (`acpx_provider_backend.rs`, `restore_session_if_needed`) closes the in-flight turn with `code: acpx_active_turn_recovery_closed` and no `error` object. The run shows the generic "Provider turn failed". The Codex fix in #15297 gives Codex a recoverable `provider_turn_lost_on_restore` cause and a same-conversation continuation. ACPX does not have this yet. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edit) for diagnosis, the regression test, the fix, and live reruns. ## 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 (not applicable: test-harness-only change) - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
26071353d2
commit
a12fcb8f26
3 files changed
+103
-16
No files matched your search
@@ -22,7 +22,7 @@ import type { MatrixExecution } from "./types.js";
|
||||
import { submitTaskReply } from "./user-actions.js";
|
||||
import { createTaskFromPromptThroughUi } from "./plan-task-ui.js";
|
||||
import { waitForTaskChatRendered, waitForTaskIdentity } from "./continuation-screenshot.js";
|
||||
import { hasPersistedSource, isSavedSourceCheckpoint } from "./everyday-interruption.js";
|
||||
import { hasPersistedSource, isSavedSourceCheckpoint, readWorkspaceSource } from "./everyday-interruption.js";
|
||||
import { collectRunEvents } from "./run-observations.js";
|
||||
import { gradeRemoteRestart, observeRemoteRestartBoundary, projectRemoteRestartEvents, projectRemoteRestartRun, waitForRemoteRestartBoundary, type RemoteRestartEvidence } from "./everyday-remote-restart.js";
|
||||
import { controllerOutageForExecution } from "./controller-outage.js";
|
||||
@@ -222,16 +222,22 @@ export async function runEverydayFlow(input: Input) {
|
||||
parentBlockedObserved: boolean;
|
||||
} | undefined;
|
||||
async function workspaceFiles() {
|
||||
const workspaceRoot = input.workspacePath;
|
||||
// Include project folders so a stop check still sees edits when the agent
|
||||
// saved the project below the workspace root (see readWorkspaceSource).
|
||||
const files: Record<string, string> = {};
|
||||
for (const entry of await readdir(workspaceRoot, {
|
||||
withFileTypes: true,
|
||||
})) {
|
||||
if (entry.isFile() && /\.(py|md|zip)$/.test(entry.name))
|
||||
files[entry.name] = createHash("sha256")
|
||||
.update(await readFile(path.join(workspaceRoot, entry.name)))
|
||||
.digest("hex");
|
||||
}
|
||||
const visit = async (relative: string, depth: number) => {
|
||||
for (const entry of await readdir(path.join(input.workspacePath, relative), { withFileTypes: true })) {
|
||||
const name = path.join(relative, entry.name);
|
||||
if (entry.isFile() && /\.(py|md|zip)$/.test(entry.name))
|
||||
files[name] = createHash("sha256")
|
||||
.update(await readFile(path.join(input.workspacePath, name)))
|
||||
.digest("hex");
|
||||
else if (entry.isDirectory() && depth < 2 && !entry.name.startsWith(".")
|
||||
&& !["node_modules", "__pycache__", "venv"].includes(entry.name))
|
||||
await visit(name, depth + 1);
|
||||
}
|
||||
};
|
||||
await visit("", 0);
|
||||
return files;
|
||||
}
|
||||
async function runnerStopped(run: StoryRun) {
|
||||
@@ -529,14 +535,13 @@ export async function runEverydayFlow(input: Input) {
|
||||
requireActive = false,
|
||||
maxWaitMs = 180_000,
|
||||
) {
|
||||
const filePath = path.join(input.workspacePath, "slugify.py");
|
||||
const bytes = await pollUntil({
|
||||
label: requireActive ? "active run with saved source" : "saved source after interruption",
|
||||
deadlineAt: Math.min(input.deadlineAt, Date.now() + maxWaitMs),
|
||||
intervalMs: 500,
|
||||
load: async () => {
|
||||
await refresh();
|
||||
const source = await readFile(filePath).catch(() => undefined);
|
||||
const source = await readWorkspaceSource(input.workspacePath);
|
||||
return { active: ev.runs.some(isActiveStoryRun), source };
|
||||
},
|
||||
accept: ({ active, source }) =>
|
||||
@@ -1763,11 +1768,10 @@ export async function runEverydayFlow(input: Input) {
|
||||
.map((r) => ({ runId: r.id, code: r.errorCode })),
|
||||
});
|
||||
if (execution.environment.id === "local") {
|
||||
const saved = await readWorkspaceSource(input.workspacePath);
|
||||
try {
|
||||
const source = await readFile(
|
||||
path.join(input.workspacePath, "slugify.py"),
|
||||
"utf8",
|
||||
);
|
||||
if (!saved) throw new Error("No saved slugify.py in the workspace");
|
||||
const source = saved.toString("utf8");
|
||||
await input.evidence("source-after-interruption.json", {
|
||||
body: source,
|
||||
sha256: createHash("sha256").update(source).digest("hex"),
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { readWorkspaceSource } from "./everyday-interruption.js";
|
||||
|
||||
const roots: string[] = [];
|
||||
async function workspace(files: Record<string, string>) {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "everyday-interruption-"));
|
||||
roots.push(root);
|
||||
for (const [name, body] of Object.entries(files)) {
|
||||
await mkdir(path.dirname(path.join(root, name)), { recursive: true });
|
||||
await writeFile(path.join(root, name), body);
|
||||
}
|
||||
return root;
|
||||
}
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
describe("readWorkspaceSource", () => {
|
||||
it("reads slugify.py at the workspace root", async () => {
|
||||
const root = await workspace({ "slugify.py": "root" });
|
||||
expect((await readWorkspaceSource(root))?.toString()).toBe("root");
|
||||
});
|
||||
|
||||
it("finds slugify.py the agent saved in a project folder inside the workspace", async () => {
|
||||
// The brief says "in slugify.py ... Work in the project workspace"; it does not
|
||||
// require the workspace root, and agents legitimately create a project folder.
|
||||
const root = await workspace({ "slugify-tool/slugify.py": "nested", "slugify-tool/README.md": "readme" });
|
||||
expect((await readWorkspaceSource(root))?.toString()).toBe("nested");
|
||||
});
|
||||
|
||||
it("prefers the root copy and ignores caches, hidden folders, and deep paths", async () => {
|
||||
const root = await workspace({ "slugify.py": "root", "a/slugify.py": "nested" });
|
||||
expect((await readWorkspaceSource(root))?.toString()).toBe("root");
|
||||
const ignored = await workspace({
|
||||
".git/slugify.py": "hidden",
|
||||
"node_modules/x/slugify.py": "deps",
|
||||
"a/b/c/slugify.py": "too deep",
|
||||
});
|
||||
expect(await readWorkspaceSource(ignored)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("chooses the shallowest, then alphabetically first, nested copy", async () => {
|
||||
const root = await workspace({ "b/slugify.py": "b", "a/slugify.py": "a", "a/x/slugify.py": "deeper" });
|
||||
expect((await readWorkspaceSource(root))?.toString()).toBe("a");
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,6 @@
|
||||
import { readFile, readdir } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
export function hasPersistedSource(value: Buffer | undefined): boolean {
|
||||
return Boolean(value && value.length > 0);
|
||||
}
|
||||
@@ -9,3 +12,34 @@ export function isSavedSourceCheckpoint(
|
||||
): boolean {
|
||||
return active && hasPersistedSource(value);
|
||||
}
|
||||
|
||||
const SOURCE_FILE = "slugify.py";
|
||||
const SOURCE_SEARCH_DEPTH = 2;
|
||||
const SKIPPED_DIRECTORIES = new Set(["node_modules", "__pycache__", "venv"]);
|
||||
|
||||
/**
|
||||
* Reads the slugify source the agent saved in the task workspace. The brief
|
||||
* asks for `slugify.py` "in the project workspace", not at its root, so agents
|
||||
* may reasonably create a project folder. Prefer the root copy, then the
|
||||
* shallowest (alphabetically first) copy in a bounded set of subfolders.
|
||||
*/
|
||||
export async function readWorkspaceSource(
|
||||
workspaceRoot: string,
|
||||
): Promise<Buffer | undefined> {
|
||||
let level = [workspaceRoot];
|
||||
for (let depth = 0; depth <= SOURCE_SEARCH_DEPTH && level.length; depth += 1) {
|
||||
const next: string[] = [];
|
||||
for (const directory of level) {
|
||||
const source = await readFile(path.join(directory, SOURCE_FILE)).catch(() => undefined);
|
||||
if (source) return source;
|
||||
const entries = await readdir(directory, { withFileTypes: true }).catch(() => []);
|
||||
next.push(...entries
|
||||
.filter((entry) => entry.isDirectory() && !entry.name.startsWith(".") && !SKIPPED_DIRECTORIES.has(entry.name))
|
||||
.map((entry) => entry.name)
|
||||
.sort()
|
||||
.map((name) => path.join(directory, name)));
|
||||
}
|
||||
level = next;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
Reference in new issue
Block a user