mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-17 10:06:42 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The task list and the chat views show a Live badge and a Working shimmer for an issue that has an active run. > - A finished task kept the Live badge and the Working shimmer after the run ended and the sandbox stopped. > - The user interface reads run liveness from the `heartbeat_runs.status` row. The run finalizer writes the terminal status in a step that is separate from the agent `status=done` update. When the sandbox or the run process stops between the two steps, `heartbeat_runs.status` stays `running` forever. > - A run row that stays `running` makes a finished task look perpetually Live, and the user interface has no guard for an issue that already reached a terminal status. > - This pull request closes the invariant "environment lease released implies the run is terminal" on the server, and adds a user interface guard that suppresses live state for a terminal issue. > - The benefit is that a finished task stops showing Live and Working, both at the source (the run row) and at the surface (the badge and the shimmer). ## Linked Issues or Issue Description **Bug description** - A completed task kept the Live badge and the Working shimmer after its run ended and the sandbox was torn down. **Steps to reproduce** - Run an agent task to completion. Let the sandbox tear down while the run finalizer is between the `status=done` update and the terminal run-status write. - Open the task list or the chat view for the finished task. **Expected behavior** - A finished task shows no Live badge and no Working shimmer. **Actual behavior (before this change)** - The finished task showed the Live badge and the Working shimmer because its `heartbeat_runs.status` row stayed `running`. This pull request supersedes the two separate pull requests #10954 (frontend) and #10955 (backend). It carries all of their changes for the same race. ## What Changed Server: - Run teardown terminalizes a still-running or still-queued run before it releases the environment lease. It writes `succeeded` when the issue already reached `done`, `cancelled` when the issue is `cancelled`, and `interrupted` otherwise. It never overwrites a status that another path already made terminal. - The recovery stale-lock sweep terminalizes an orphaned running run to `interrupted` after it confirms the process and the sandbox are both gone. It requires recorded process metadata, so it never terminalizes a live run, a queued run, or a scheduled retry. - Each terminal transition writes a run event. - The stale-lock sweep continues and clears the lock when the audit write fails. It logs the failure loudly. - New server tests cover both invariants. User interface: - A shared guard suppresses the Live badge and the Working shimmer when the issue status is terminal. - The guard keeps non-terminal `queued` and `running` issues live. - The guard prefers the newest issue live-status snapshot. - New user interface tests cover the guard and the snapshot preference. ## Verification Server: - `pnpm --filter @paperclipai/plugin-sdk ensure-build-deps && tsc --noEmit` in `server/` — 0 errors. - `pnpm --filter server test heartbeat-run-lease-release-terminalization.test.ts recovery-stale-issue-lock-sweep.test.ts` — 12 tests pass. User interface: - `pnpm --filter @paperclipai/ui typecheck` — 0 errors. - `pnpm exec vitest run ui/src/lib/liveIssueIds.test.ts ui/src/lib/issue-chat-messages.test.ts` — 40 tests pass. ## Risks - Low risk. The server change only forces a still-live run row to a terminal status when the lease releases or when the recovery sweep confirms the process is dead. It never overwrites an existing terminal status, and it guards the recovery path with process metadata to avoid terminalizing a live run. - The user interface change is additive. The guard only suppresses live state for a terminal issue and keeps queued and running issues live. - No database migration. No change to any external endpoint. ## Model Used - Claude Opus 4.8 (`claude-opus-4-8`), extended thinking, 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 - [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>
117 lines
4.3 KiB
TypeScript
117 lines
4.3 KiB
TypeScript
import type { IssueStatus } from "@paperclipai/shared";
|
|
import type { LiveRunForIssue } from "../api/heartbeats";
|
|
|
|
function isLiveRunStatus(status: string): boolean {
|
|
return status === "queued" || status === "running";
|
|
}
|
|
|
|
const TERMINAL_ISSUE_STATUSES = new Set<IssueStatus>(["done", "cancelled"]);
|
|
|
|
export function isTerminalIssueStatus(status: string | null | undefined): status is IssueStatus {
|
|
return TERMINAL_ISSUE_STATUSES.has(status as IssueStatus);
|
|
}
|
|
|
|
export function isLiveIssueRun(
|
|
run: Pick<LiveRunForIssue, "status">,
|
|
issueStatus?: string | null,
|
|
): boolean {
|
|
return isLiveRunStatus(run.status) && !isTerminalIssueStatus(issueStatus);
|
|
}
|
|
|
|
export interface LiveIssueStatusNode {
|
|
id: string;
|
|
status: IssueStatus | string;
|
|
updatedAt?: Date | string | number | null;
|
|
}
|
|
|
|
function collectIssueStatusById(issues: readonly LiveIssueStatusNode[] | null | undefined): Map<string, string> {
|
|
const snapshotByIssueId = new Map<string, { status: string; updatedAtMs: number | null }>();
|
|
for (const issue of issues ?? []) {
|
|
const candidate = {
|
|
status: issue.status,
|
|
updatedAtMs: issueUpdatedAtMs(issue.updatedAt),
|
|
};
|
|
const existing = snapshotByIssueId.get(issue.id);
|
|
if (!existing || shouldReplaceIssueStatusSnapshot(existing, candidate)) snapshotByIssueId.set(issue.id, candidate);
|
|
}
|
|
return new Map([...snapshotByIssueId].map(([issueId, snapshot]) => [issueId, snapshot.status]));
|
|
}
|
|
|
|
function issueUpdatedAtMs(updatedAt: LiveIssueStatusNode["updatedAt"]): number | null {
|
|
if (updatedAt === null || updatedAt === undefined) return null;
|
|
const timestamp = updatedAt instanceof Date ? updatedAt.getTime() : new Date(updatedAt).getTime();
|
|
return Number.isFinite(timestamp) ? timestamp : null;
|
|
}
|
|
|
|
function shouldReplaceIssueStatusSnapshot(
|
|
existing: { status: string; updatedAtMs: number | null },
|
|
candidate: { status: string; updatedAtMs: number | null },
|
|
): boolean {
|
|
if (candidate.updatedAtMs !== null && existing.updatedAtMs !== null) {
|
|
if (candidate.updatedAtMs !== existing.updatedAtMs) return candidate.updatedAtMs > existing.updatedAtMs;
|
|
} else if (candidate.updatedAtMs !== null) {
|
|
return true;
|
|
} else if (existing.updatedAtMs !== null) {
|
|
return false;
|
|
}
|
|
|
|
return !isTerminalIssueStatus(existing.status) && isTerminalIssueStatus(candidate.status);
|
|
}
|
|
|
|
export function collectLiveIssueIds(
|
|
liveRuns: readonly LiveRunForIssue[] | null | undefined,
|
|
issues?: readonly LiveIssueStatusNode[] | null,
|
|
): Set<string> {
|
|
const ids = new Set<string>();
|
|
const statusByIssueId = collectIssueStatusById(issues);
|
|
for (const run of liveRuns ?? []) {
|
|
if (run.issueId && isLiveIssueRun(run, statusByIssueId.get(run.issueId))) ids.add(run.issueId);
|
|
}
|
|
return ids;
|
|
}
|
|
|
|
/**
|
|
* Minimal tree node shape needed to roll live descendants up to their ancestors.
|
|
* Both list and inbox issue objects satisfy this.
|
|
*/
|
|
export interface SubtreeLiveNode {
|
|
id: string;
|
|
parentId: string | null;
|
|
}
|
|
|
|
/**
|
|
* Derive, for every issue in the already-loaded tree, how many of its
|
|
* descendants currently have their own live (queued/running) run.
|
|
*
|
|
* The count is strictly over descendants — an issue's own live run never
|
|
* contributes to its own entry. Ancestors are walked through the loaded set
|
|
* via `parentId`, so descendants that are not loaded are simply not counted.
|
|
*
|
|
* Pair with {@link collectLiveIssueIds}: keep `Live` for `liveIssueIds.has(id)`
|
|
* (own run) and render the distinct "n live below" treatment only when an
|
|
* issue is not itself live but has a positive subtree-live count.
|
|
*/
|
|
export function collectSubtreeLiveCounts(
|
|
issues: readonly SubtreeLiveNode[] | null | undefined,
|
|
liveIssueIds: ReadonlySet<string>,
|
|
): Map<string, number> {
|
|
const counts = new Map<string, number>();
|
|
if (!issues || issues.length === 0 || liveIssueIds.size === 0) return counts;
|
|
|
|
const parentById = new Map<string, string | null>();
|
|
for (const issue of issues) parentById.set(issue.id, issue.parentId);
|
|
|
|
for (const liveId of liveIssueIds) {
|
|
// Only roll up live issues that belong to the loaded tree.
|
|
if (!parentById.has(liveId)) continue;
|
|
const seen = new Set<string>([liveId]);
|
|
let parentId = parentById.get(liveId) ?? null;
|
|
while (parentId && !seen.has(parentId)) {
|
|
seen.add(parentId);
|
|
counts.set(parentId, (counts.get(parentId) ?? 0) + 1);
|
|
parentId = parentById.get(parentId) ?? null;
|
|
}
|
|
}
|
|
return counts;
|
|
}
|