mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
fix(recovery): limit the null-host owner fallback to single-host servers and record each use (#19883)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #19868 and #19871 made restart recovery judge a recorded pid only in the PID space that recorded it > - Rows saved before those host columns existed have no recorded host. They kept the previous local dead-pid proof, so in-flight native runs would not be stranded at upgrade > - On an ephemeral (multi-host) server that fallback is unsafe: the row may have been written on another host > - This pull request limits the fallback to single-host servers, records every use, and gives ephemeral servers a working board action > - The benefit is that single-host upgrades still recover on their own, multi-host servers never accept a cross-host proof, and the next release can remove the fallback with evidence ## Linked Issues or Issue Description **Bug description** After #19868 and #19871, a `native_run_finalizations` row with `controller_host_hash = null`, or a run with `process_host_hash = null`, still accepted "the pid is dead here" as proof on every server. That includes an ephemeral server, where the row may come from another host. Nothing recorded how often the fallback was used, so it had no safe removal path. **Expected behavior** - Single-host server: the fallback applies for one release, and each use writes a `legacy_host_unknown_dead_pid_proof` activity-log entry. - Ephemeral server (`PAPERCLIP_EPHEMERAL_SERVER=true`): the row is unprovable, and the board has an action to confirm the previous server stopped. - A mismatched host is always unprovable. Refs #19868, #19871, #19846 ## What Changed - `native-restart-recovery.ts` `evaluateNativeControllerTakeover`: - A coordinator row with `controllerHostHash: null` returns `controller_host_unrecorded` when `ephemeralServerEnabled()` is true. - Otherwise a pid proof from that row carries `legacyHostFallback: true`. - `operator_attested` is accepted as proof (`operator_attested_controller_stopped`). - Receipt-built owners, which have no host key, are unchanged. - New `recordLegacyHostFallbackProof` writes the activity-log entry. It is not a run-log event: run events are progress evidence for restart recovery's crash-loop credit, and the record must not change any recovery decision. It is called from restart recovery, the dead-controller result-lease release, native retirement, and remote warm-session recovery. - New `native-controller-host-recovery.ts`: - `requestNativeControllerStopConfirmation` (board-owned `native_controller_host_unverified` recovery action, idempotent, never displaces another active action). - `confirmNativeControllerStopped`. It verifies the exact recorded controller and stamps `operator_attested` with compare-and-set. It then resolves the action, wakes live-run recovery, and logs `issue.native_controller_stop_confirmed`. - Route `POST /issues/:id/recovery-actions/confirm-controller-stopped`. It is board-only and requires `runtime:confirm_owner_stop`. Generic `restored`/`false_positive` resolution of this cause is refused. A shared `confirmNativeControllerStoppedSchema` validates the body. - UI: `WorkspaceExportRecovery` reuses the "Confirm the previous server stopped" control for this cause, and the generic recovery card hides it. Adds the `issuesApi.confirmControllerStopped` client call. - `recovery/service.ts` orphan reaper: a null `process_host_hash` is judged only when not ephemeral. A successful use records the entry best-effort, after the terminal write. On an ephemeral server the run is left to its existing Stop control. - `ephemeralServerEnabled` moves to a leaf `ephemeral-server-mode.ts` (re-exported by `ephemeral-server-contract.ts`). Importing the contract module from recovery code created an import cycle in the runner child process. - The new route is registered in OpenAPI. - `doc/execution-semantics.md`: documents the host rule and the one-release sunset. ## Verification - `native-restart-recovery.test.ts`: null host on a single-host server is released with the flag. Null host on an ephemeral server is unprovable. A mismatched host is unprovable in both modes. An attested controller is accepted. Receipt owners are unchanged. - New `native-controller-host-recovery.test.ts` (embedded Postgres): - The fallback writes the activity-log entry and no run event. - Ephemeral mode creates exactly one board action. - A wrong run id is refused. - Confirmation stamps the attestation, after which takeover is allowed. The action resolves and the activity is logged. A second confirmation is refused. - `recovery-stale-issue-lock-sweep.test.ts`: on a single-host server a null host is terminalized and records one event. On an ephemeral server it is left alone with no event. The existing audit-failure case still clears the lock. - `WorkspaceExportRecovery.control.test.tsx`: the controller confirmation posts to `confirmControllerStopped` and never to the copyback endpoint. - Locally: these suites plus `remote-warm-session-recovery`, `native-controller-recovery.integration` and `native-startup-cleanup-recovery` (136 passed), and after the CI fixes `native-runner-restart-preparation` and `native-runner-restart-recovery` integration, `openapi-routes` and the suites above (256 passed); the UI recovery suites (52 passed); `node scripts/typecheck-layers.mjs` with no errors in the touched files; `pnpm check:token-gates` clean. ## Risks - Previous-release story: no schema change. Null-host rows saved by earlier releases recover as before on single-host servers, now with an event, and wait for the board on ephemeral servers. The next release removes the fallback; the event count shows whether any rows remain. - On an ephemeral server, a null-host orphaned run process is not reaped automatically. The operator uses Stop. That is the intended "unprovable" outcome. - A board confirmation is an attestation. It applies only to the exact recorded controller (boot id + pid) and only while the host is still unrecorded. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## 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 🤖 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
5a70999306
commit
f439150cb3
21 files changed
+433
-28
No files matched your search
@@ -419,6 +419,8 @@ export const issuesApi = {
|
||||
`/issues/${id}/stalled-review-decision`,
|
||||
data,
|
||||
),
|
||||
confirmControllerStopped: (id: string, data: { actionId: string; runId: string; stopEvidence: string }) =>
|
||||
api.post<{ runId: string; status: "queued" }>(`/issues/${id}/recovery-actions/confirm-controller-stopped`, data),
|
||||
resumeWorkspaceFinalization: (id: string, data: { actionId: string; runId: string; ownerToken: string; controllerAndCopybackStopped: true; stopEvidence: string }) =>
|
||||
api.post<{ runId: string; resultId: string | null; status: "queued" }>(`/issues/${id}/recovery-actions/resume-workspace-finalization`, data),
|
||||
retryWorkspaceExport: (id: string, data: { actionId: string; runId: string; repairNote: string }) =>
|
||||
|
||||
@@ -9,9 +9,9 @@ import { queryKeys } from "../lib/queryKeys";
|
||||
import { writePendingSend } from "../lib/pending-send";
|
||||
import { WorkspaceExportRecovery } from "./WorkspaceExportRecovery";
|
||||
|
||||
const api = vi.hoisted(() => ({ inspect: vi.fn(), recover: vi.fn(), resume: vi.fn() }));
|
||||
const api = vi.hoisted(() => ({ inspect: vi.fn(), recover: vi.fn(), resume: vi.fn(), confirmController: vi.fn() }));
|
||||
vi.mock("../api/issues", () => ({ issuesApi: { recoveryActions: api.inspect, recoverWorkspace: api.recover,
|
||||
resumeWorkspaceFinalization: api.resume } }));
|
||||
resumeWorkspaceFinalization: api.resume, confirmControllerStopped: api.confirmController } }));
|
||||
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
const action = {
|
||||
@@ -473,6 +473,28 @@ describe("governed retained-file recovery", () => {
|
||||
expect(container.querySelector('[data-testid="retained-file-recovery"]')).not.toBeNull();
|
||||
});
|
||||
|
||||
it("confirms a pre-upgrade controller an ephemeral server cannot verify", async () => {
|
||||
api.confirmController.mockResolvedValue({ runId: "run-1", status: "queued" });
|
||||
client.setQueryData(queryKeys.access.currentBoardAccess, { userId: "board-1", source: "session", isInstanceAdmin: false,
|
||||
companyIds: ["company-1"], memberships: [{ companyId: "company-1", membershipRole: "owner", status: "active" }] });
|
||||
const controllerAction = { ...action, cause: "native_controller_host_unverified", ownerType: "board",
|
||||
evidence: { runId: "run-1", controller: { bootId: "boot", pid: 7 } } } as unknown as IssueRecoveryAction;
|
||||
await mount(controllerAction);
|
||||
expect(container.textContent).toContain("before Paperclip stored which host it ran on");
|
||||
await click("Confirm the previous server stopped");
|
||||
const textarea = container.querySelector("textarea")!;
|
||||
await act(async () => {
|
||||
const setter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!;
|
||||
setter.call(textarea, "The old server container was replaced and is not running.");
|
||||
textarea.dispatchEvent(new Event("input", { bubbles: true }));
|
||||
});
|
||||
await click("Confirm and resume");
|
||||
expect(api.confirmController).toHaveBeenCalledExactlyOnceWith("issue-1", { actionId: "action-1", runId: "run-1",
|
||||
stopEvidence: "The old server container was replaced and is not running." });
|
||||
expect(api.resume).not.toHaveBeenCalled();
|
||||
expect(container.textContent).toContain("Resuming the run…");
|
||||
});
|
||||
|
||||
it("shows status only, without the action, once automatic recovery is resuming or for non-administrators", async () => {
|
||||
// The default fixture is an operator: it may resolve other recovery but not attest a controller stop.
|
||||
await mount({ ...ownerAction, evidence: { ...ownerAction.evidence, workspaceOwnerRecovery: { state: "resuming" } } } as IssueRecoveryAction);
|
||||
|
||||
@@ -25,6 +25,14 @@ function ReportRecoveryControl({ owns, onChange }: { owns: boolean; onChange?: (
|
||||
return null;
|
||||
}
|
||||
|
||||
const CONTROLLER_HOST_UNVERIFIED = "native_controller_host_unverified";
|
||||
|
||||
/** Causes whose one action is the board's previous-server stop confirmation;
|
||||
* the generic recovery card must not offer other resolutions for them. */
|
||||
export function isPreviousServerStopCause(cause: string | null | undefined) {
|
||||
return cause === "native_workspace_finalization_owner_unverified" || cause === CONTROLLER_HOST_UNVERIFIED;
|
||||
}
|
||||
|
||||
/** Requests preservation; the existing worker owns stop, copyback and verification. */
|
||||
export function WorkspaceExportRecovery({ action, className, issueId, companyId, showStatus = true, onRecoveryControlChange }: {
|
||||
className?: string; action: IssueRecoveryAction | null; issueId?: string; companyId?: string; showStatus?: boolean;
|
||||
@@ -44,6 +52,19 @@ export function WorkspaceExportRecovery({ action, className, issueId, companyId,
|
||||
action={action} issueId={issueId} companyId={companyId} className={className}
|
||||
onRecoveryControlChange={onRecoveryControlChange} />;
|
||||
}
|
||||
// An ephemeral server cannot judge a pre-upgrade controller row by PID.
|
||||
if (action?.cause === CONTROLLER_HOST_UNVERIFIED && issueId && companyId
|
||||
&& isControllerStopRequired(action, issueId, companyId)) {
|
||||
return <><ReportRecoveryControl owns onChange={onRecoveryControlChange} />
|
||||
<PreviousServerStopConfirmation key={`${action.id}:${String(action.evidence.runId)}`}
|
||||
action={action} issueId={issueId} companyId={companyId} className={className}
|
||||
confirmStop={stopEvidence => issuesApi.confirmControllerStopped(issueId, {
|
||||
actionId: action.id, runId: action.evidence.runId as string, stopEvidence })}
|
||||
copy={{ waiting: "This run’s previous server was recorded before Paperclip stored which host it ran on, so Paperclip can’t verify that it stopped. The run’s saved state is kept.",
|
||||
question: "How did you confirm that the previous server stopped?",
|
||||
placeholder: "For example: the old server container was replaced and is no longer running.",
|
||||
resuming: "Resuming the run…" }} /></>;
|
||||
}
|
||||
const ownerRecovery = action?.cause === "native_workspace_finalization_owner_unverified";
|
||||
// The task's interruption names this wait; the confirmation is its one action.
|
||||
if (ownerRecovery && issueId && companyId && isOwnerStopRequired(action, issueId, companyId)) {
|
||||
@@ -72,6 +93,12 @@ function isRetainedExport(action: IssueRecoveryAction, issueId: string, companyI
|
||||
&& typeof action.evidence.runId === "string" && action.evidence.runId.length > 0;
|
||||
}
|
||||
|
||||
function isControllerStopRequired(action: IssueRecoveryAction, issueId: string, companyId: string) {
|
||||
return action.companyId === companyId && action.sourceIssueId === issueId
|
||||
&& action.ownerType === "board" && ["active", "escalated"].includes(action.status)
|
||||
&& typeof action.evidence.runId === "string" && !action.evidence.controllerStop;
|
||||
}
|
||||
|
||||
function isOwnerStopRequired(action: IssueRecoveryAction, issueId: string, companyId: string) {
|
||||
return action.companyId === companyId && action.sourceIssueId === issueId
|
||||
&& action.ownerType === "board" && ["active", "escalated"].includes(action.status)
|
||||
@@ -89,8 +116,12 @@ function retainedOwnerStopToken(action: IssueRecoveryAction): string | null {
|
||||
|
||||
/** A previous server that can no longer report cannot confirm its own stop.
|
||||
* The board confirms it once; the saved result then finishes without another agent turn. */
|
||||
function PreviousServerStopConfirmation({ action, ownerToken, issueId, companyId, className }: {
|
||||
action: IssueRecoveryAction; ownerToken: string; issueId: string; companyId: string; className?: string;
|
||||
function PreviousServerStopConfirmation({ action, ownerToken, issueId, companyId, className, confirmStop, copy }: {
|
||||
action: IssueRecoveryAction; issueId: string; companyId: string; className?: string;
|
||||
/** Saved-work copyback owner; used when no custom confirmation is given. */
|
||||
ownerToken?: string;
|
||||
confirmStop?: (stopEvidence: string) => Promise<unknown>;
|
||||
copy?: { waiting: string; question: string; placeholder: string; resuming: string };
|
||||
}) {
|
||||
const runId = action.evidence.runId as string;
|
||||
const client = useQueryClient();
|
||||
@@ -101,8 +132,9 @@ function PreviousServerStopConfirmation({ action, ownerToken, issueId, companyId
|
||||
const accessView = useQueryView(access);
|
||||
const canConfirm = (accessView.kind === "ready" || accessView.kind === "stale") && canBoardConfirmOwnerStop(companyId, access.data);
|
||||
const confirm = useMutation({ retry: false, // query-error-ok: an explicit board confirmation; the error is shown below.
|
||||
mutationFn: () => issuesApi.resumeWorkspaceFinalization(issueId, { actionId: action.id, runId, ownerToken,
|
||||
controllerAndCopybackStopped: true, stopEvidence: evidence.trim() }),
|
||||
mutationFn: () => confirmStop ? confirmStop(evidence.trim())
|
||||
: issuesApi.resumeWorkspaceFinalization(issueId, { actionId: action.id, runId, ownerToken: ownerToken!,
|
||||
controllerAndCopybackStopped: true, stopEvidence: evidence.trim() }),
|
||||
onSuccess: () => {
|
||||
setOpen(false);
|
||||
void client.invalidateQueries({ queryKey: queryKeys.issues.detail(issueId) });
|
||||
@@ -111,8 +143,8 @@ function PreviousServerStopConfirmation({ action, ownerToken, issueId, companyId
|
||||
} });
|
||||
const ready = evidence.trim().length >= 20;
|
||||
return <div className={cn("flex flex-col gap-2 text-sm", className)} data-testid="previous-server-stop">
|
||||
<p role="status">{confirm.isSuccess ? "Restoring saved work…"
|
||||
: "The previous server stopped while it was saving this task’s files, and Paperclip can’t verify that it stopped. The agent’s result is saved."}</p>
|
||||
<p role="status">{confirm.isSuccess ? copy?.resuming ?? "Restoring saved work…"
|
||||
: copy?.waiting ?? "The previous server stopped while it was saving this task’s files, and Paperclip can’t verify that it stopped. The agent’s result is saved."}</p>
|
||||
{confirm.isSuccess ? null : !canConfirm
|
||||
? <p className="text-xs text-muted-foreground">{accessView.kind === "loading" || accessView.kind === "reconnecting"
|
||||
? "Checking access…" : "Paperclip is waiting to confirm that the previous server stopped."}</p>
|
||||
@@ -120,9 +152,9 @@ function PreviousServerStopConfirmation({ action, ownerToken, issueId, companyId
|
||||
Confirm the previous server stopped</Button>
|
||||
: <form className="flex flex-col gap-2" onSubmit={event => { event.preventDefault(); if (ready && !confirm.isPending) confirm.mutate(); }}>
|
||||
<label htmlFor={`previous-server-stop-${action.id}`} className="text-xs text-muted-foreground">
|
||||
How did you confirm that the previous server and its file copy stopped?</label>
|
||||
{copy?.question ?? "How did you confirm that the previous server and its file copy stopped?"}</label>
|
||||
<Textarea id={`previous-server-stop-${action.id}`} value={evidence} onChange={event => setEvidence(event.target.value)}
|
||||
placeholder="For example: the old server process exited and was not restarted." maxLength={12000} rows={3} />
|
||||
placeholder={copy?.placeholder ?? "For example: the old server process exited and was not restarted."} maxLength={12000} rows={3} />
|
||||
<div className="flex items-center justify-between gap-2">
|
||||
<Button type="button" variant="ghost" size="sm" disabled={confirm.isPending} onClick={() => setOpen(false)}>Cancel</Button>
|
||||
<Button type="submit" size="sm" disabled={!ready || confirm.isPending}>
|
||||
|
||||
@@ -20,7 +20,7 @@ import { canManageIssuePrivacy } from "../lib/issuePrivacy";
|
||||
import { TextAttachmentContext } from "../context/TextAttachmentContext";
|
||||
import { useTaskPluginPanels, taskPluginPanelActivationKey } from "@/hooks/useTaskPluginPanels";
|
||||
import { useTaskBrowsers, useBrowserArrivals } from "@/hooks/useTaskBrowsers";
|
||||
import { WorkspaceExportRecovery } from "../components/WorkspaceExportRecovery";
|
||||
import { isPreviousServerStopCause, WorkspaceExportRecovery } from "../components/WorkspaceExportRecovery";
|
||||
import { DispositionRecoveryProvider } from "../components/DispositionRecoveryNotice";
|
||||
import { AgentAvatar } from "@/components/AgentAvatar";
|
||||
import { mergeComposerRunSettings, type ComposerRunSettings } from "@/components/task-chat/composer-run-settings";
|
||||
@@ -2690,7 +2690,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({
|
||||
blockerAttention={blockerAttention}
|
||||
successfulRunHandoff={successfulRunHandoff}
|
||||
scheduledRetry={scheduledRetry}
|
||||
recoveryAction={recoveryAction?.cause === "native_workspace_finalization_owner_unverified" ? null : recoveryAction ?? null}
|
||||
recoveryAction={isPreviousServerStopCause(recoveryAction?.cause) ? null : recoveryAction ?? null}
|
||||
onResolveRecoveryAction={onResolveRecoveryAction}
|
||||
onReissueIsolatedRecoveryAction={onReissueIsolatedRecoveryAction}
|
||||
reissueIsolatedRecoveryActionPending={
|
||||
@@ -8564,7 +8564,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS
|
||||
blockerAttention={issue.blockerAttention ?? null}
|
||||
successfulRunHandoff={issue.successfulRunHandoff ?? null}
|
||||
scheduledRetry={issue.scheduledRetry ?? null}
|
||||
recoveryAction={issue.interruption?.responsibility === "system" || issue.activeRecoveryAction?.cause === "native_workspace_finalization_owner_unverified" ? null : issue.activeRecoveryAction ?? null}
|
||||
recoveryAction={issue.interruption?.responsibility === "system" || isPreviousServerStopCause(issue.activeRecoveryAction?.cause) ? null : issue.activeRecoveryAction ?? null}
|
||||
onResolveRecoveryAction={handleResolveRecoveryAction}
|
||||
onReissueIsolatedRecoveryAction={
|
||||
handleReissueIsolatedRecoveryAction
|
||||
|
||||
Reference in new issue
Block a user