mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 19:35:04 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The runner E2E suite verifies complete agent tasks against real providers. > - A Codex Plan test finished in 76 seconds, but its Playwright process stayed alive for 25 more minutes. > - The launcher accepted the saved passing result after its watchdog killed the process. > - The existing Plan limits also allowed much more time than recent successful runs need. > - This pull request adds a bounded result-to-exit check and safe process evidence. > - It also reduces the Plan limits while it keeps large headroom over measured success times. > - The benefit is faster diagnosis and no false green result after a teardown stall. ## Linked Issues or Issue Description **Pre-submission checklist** I searched open pull requests for runner E2E timeout and Playwright cleanup changes. I found no duplicate. The problem reproduces on `master`. **What happened?** The local Codex Plan cell completed its test in 76 seconds. Playwright then stayed alive for about 25 minutes. The launcher watchdog killed it after 26.5 minutes, but the launcher still accepted the saved passing result. **Expected behavior** The launcher must stop a process that stays alive after all results exist. It must report a cleanup failure instead of a pass. Plan tests must also use limits that match measured successful runs. **Steps to reproduce** 1. Run `core-compatibility.runner-codex.local.plan-revise-accept`. 2. Observe a valid result and the Playwright pass output. 3. Observe that the process can stay alive until the old launcher watchdog stops it. **Paperclip version or commit** The evidence came from `bcc6fe7a442dae74ab0321ad472f7536ffa58f04` in [Actions run 33963318820](https://github.com/paperclipai/paperclip/actions/runs/33963318820). ## What Changed - Reduce the Plan attempt limit from 20 to 8 minutes for local execution. - Reduce the Plan attempt limit from 35 to 12 minutes for Daytona execution. - Stop Playwright after it stays alive for 120 seconds after every result exists. - Record only allowlisted process kinds in the stall diagnostic. - Validate process identities before cleanup and retain continuously live process groups through member replacement. - Treat watchdog, post-result, cleanup, and nonzero-exit conflicts as cleanup failures. - Keep interactive `--ui` and `--debug` sessions exempt from the result-to-exit check. ## Verification - Prettier completed for all changed files. - `git diff --check` passed. - Static review confirmed the timeout derivation and cleanup boundaries. - An independent review found no blocking issue in the final patch. - I did not run local tests, builds, or type checks because this workstation must use the lightweight workflow. - GitHub CI and the exact paid Codex Plan cell will verify this commit. ## Risks The main risk is a false cleanup failure when Playwright needs more than 120 seconds after it writes all results. The allowance is separate from the task limit. Interactive modes are exempt. The diagnostic does not print command arguments or environment values. ## Model Used OpenAI Codex with GPT-5.6, reasoning, tool use, and code execution. ## 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 - [ ] 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
143 lines
4.3 KiB
TypeScript
143 lines
4.3 KiB
TypeScript
export interface ProcessObservation {
|
|
pid: number;
|
|
parentPid: number;
|
|
processGroupId: number;
|
|
started: string;
|
|
kind: string;
|
|
}
|
|
|
|
export interface ObservedProcessGroup {
|
|
processGroupId: number;
|
|
depth: number;
|
|
members: Array<Pick<ProcessObservation, "pid" | "started">>;
|
|
}
|
|
|
|
export interface ObservedProcessTreeMember {
|
|
process: ProcessObservation;
|
|
depth: number;
|
|
}
|
|
|
|
export function observeDescendantProcessTree(
|
|
table: readonly ProcessObservation[],
|
|
rootPid: number,
|
|
) {
|
|
const byPid = new Map(table.map((candidate) => [candidate.pid, candidate]));
|
|
const byParent = new Map<number, ProcessObservation[]>();
|
|
for (const candidate of table) {
|
|
const children = byParent.get(candidate.parentPid) ?? [];
|
|
children.push(candidate);
|
|
byParent.set(candidate.parentPid, children);
|
|
}
|
|
const observed = new Map<number, ObservedProcessTreeMember>();
|
|
const pending = [{ pid: rootPid, depth: 0 }];
|
|
while (pending.length > 0) {
|
|
const next = pending.shift()!;
|
|
if (observed.has(next.pid)) continue;
|
|
const candidate = byPid.get(next.pid);
|
|
if (!candidate) continue;
|
|
observed.set(next.pid, { process: candidate, depth: next.depth });
|
|
for (const child of byParent.get(next.pid) ?? []) {
|
|
pending.push({ pid: child.pid, depth: next.depth + 1 });
|
|
}
|
|
}
|
|
const groupsById = new Map<number, ObservedProcessGroup>();
|
|
for (const { process: candidate, depth } of observed.values()) {
|
|
const group = groupsById.get(candidate.processGroupId) ?? {
|
|
processGroupId: candidate.processGroupId,
|
|
depth,
|
|
members: [],
|
|
};
|
|
group.depth = Math.max(group.depth, depth);
|
|
group.members.push({ pid: candidate.pid, started: candidate.started });
|
|
groupsById.set(candidate.processGroupId, group);
|
|
}
|
|
return {
|
|
members: [...observed.values()].sort(
|
|
(left, right) => left.depth - right.depth,
|
|
),
|
|
groups: [...groupsById.values()].sort(
|
|
(left, right) => right.depth - left.depth,
|
|
),
|
|
};
|
|
}
|
|
|
|
export function safeProcessGroupTerminationOrder(input: {
|
|
rootProcessGroupId: number;
|
|
currentProcessGroupId: number | null;
|
|
groups: readonly ObservedProcessGroup[];
|
|
}) {
|
|
if (input.currentProcessGroupId === null) return [];
|
|
const safe = new Map<number, number>();
|
|
for (const group of input.groups) {
|
|
if (
|
|
group.processGroupId <= 1 ||
|
|
group.processGroupId === input.currentProcessGroupId
|
|
) {
|
|
continue;
|
|
}
|
|
safe.set(
|
|
group.processGroupId,
|
|
Math.max(safe.get(group.processGroupId) ?? -1, group.depth),
|
|
);
|
|
}
|
|
if (
|
|
input.rootProcessGroupId > 1 &&
|
|
input.rootProcessGroupId !== input.currentProcessGroupId &&
|
|
input.groups.some(
|
|
(group) => group.processGroupId === input.rootProcessGroupId,
|
|
)
|
|
) {
|
|
safe.set(input.rootProcessGroupId, -1);
|
|
}
|
|
return [...safe.entries()]
|
|
.sort((left, right) => right[1] - left[1])
|
|
.map(([processGroupId]) => processGroupId);
|
|
}
|
|
|
|
export function revalidateObservedProcessGroups(
|
|
groups: readonly ObservedProcessGroup[],
|
|
table: readonly ProcessObservation[],
|
|
) {
|
|
const byPid = new Map(table.map((candidate) => [candidate.pid, candidate]));
|
|
return groups.filter((group) =>
|
|
group.members.some((member) => {
|
|
const candidate = byPid.get(member.pid);
|
|
return (
|
|
candidate?.processGroupId === group.processGroupId &&
|
|
candidate.started === member.started
|
|
);
|
|
}),
|
|
);
|
|
}
|
|
|
|
/**
|
|
* Refresh groups only after their original members have been revalidated.
|
|
* A process may replace itself or fork a final cleanup helper after SIGTERM.
|
|
* Retain that continuously live group until one poll observes it as empty.
|
|
*/
|
|
export function refreshContinuouslyLiveProcessGroups(
|
|
groups: readonly ObservedProcessGroup[],
|
|
table: readonly ProcessObservation[],
|
|
) {
|
|
const membersByGroup = new Map<number, ProcessObservation[]>();
|
|
for (const candidate of table) {
|
|
const members = membersByGroup.get(candidate.processGroupId) ?? [];
|
|
members.push(candidate);
|
|
membersByGroup.set(candidate.processGroupId, members);
|
|
}
|
|
return groups.flatMap((group) => {
|
|
const members = membersByGroup.get(group.processGroupId);
|
|
return members
|
|
? [
|
|
{
|
|
...group,
|
|
members: members.map((candidate) => ({
|
|
pid: candidate.pid,
|
|
started: candidate.started,
|
|
})),
|
|
},
|
|
]
|
|
: [];
|
|
});
|
|
}
|