mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
## Thinking Path > - Paperclip must preserve saved agent work while proving earlier execution stopped. > - A deliberate task Retry can observe a newer failure while an older preparation still holds execution. > - That older preparation can also have a finalized empty object log despite never reaching the provider. > - Binding only the latest run and requiring every log field to be absent keeps valid recovery deferred. > - This change follows the exact persisted predecessor chain and binds current Retry authority to eligible holds. > - It recognizes only an exact finalized empty-log receipt within the existing strict preparation proof. > - One fresh user turn can then start with saved work and the existing ownership, stop and accounting checks. ## Linked Issues or Issue Description Refs #18989, #18860 and #18997. The first waits for predecessor cleanup during automatic dispatch. The second selects the committed native session owner. The third retains admitted Retry authority when no source recovery action exists. None passes current board Retry intent to a held failed predecessor. A search of open repairs found no duplicate. ROADMAP.md was checked; this is a recovery bug fix. **What happened?** A failed native preparation retains a legacy run row and an inherited session. Its remote workspace is later saved and verified. An automatic successor also fails during preparation. Board Retry observes that newer run, but the effective hold belongs to its predecessor. The Retry does not bind the predecessor's recovery generation. The native historical-source guard then keeps the request deferred. **Expected behavior** Carry the deliberate Retry through its verified same-company, same-task and same-agent persisted predecessor chain. Start one fresh user turn after every source passes current execution, stop and saved-work checks. Keep incomplete or unrelated proof held. Preserve prior work and recorded action outcomes. **Steps to reproduce** 1. Fail native preparation with an inherited session and a remote workspace. 2. Finish its real workspace recovery and stop receipts. 3. Save a newer failed automatic successor with `retryOfRunId` bound to that source. 4. Submit ordinary task Retry for the newer run. 5. Before the change, no successor is admitted. The request remains deferred behind the older hold. **Paperclip version or commit** The regression fails on master `039e0dad2c51c80251e1302a19c5566e039c357a`, which includes #18989. **Deployment mode** Source checkout on macOS arm64 with real PostgreSQL, HTTP task Retry and the assembled heartbeat service. The remote filesystem boundary uses temporary files. Hosted rollout and an original-task Retry are not yet verified. This PR incorporates the reviewed empty-log follow-up #19021, preserving its commits and regression controls. Both parts are required for hosted preparation recovery, so the combined revision is qualified and deployed together. ## What Changed - Accept a finalized zero-byte `s3_segments` log only when its reference binds the exact company, agent and run, its digest matches empty content, compression is false, and no output progress or nonempty excerpt exists. Add a real-recorder HTTP regression and 14 negative controls. Unknown or nonempty logs stay held. - Read a complete persisted Retry chain in one recursive query, limited to 32 runs. Reject missing, cyclic, newer, unfinished, cross-company, cross-task and cross-agent sources. Transcript hints grant no authority. - Bind eligible historical recovery records to the current Retry intent. Keep complete or running recovery budgets. Leave successful ancestors and mismatched recovery-source records unchanged. - Recheck that same chain when selecting an ancestor's fresh-user strategy. - Admit only a held ancestor through the shared native preparation proof. Keep the closed preparation failure codes, no-provider state, profile, input, result, output and accounting checks. Treat inherited session input as prior context and accept the exact bounded host start marker. Verify current ownership, saved work and stop receipts before forcing a fresh session. - Add HTTP and PostgreSQL regressions and document the rule. - Give the HTTP Retry nudge and explicit test worker the same real heartbeat facade and provider boundary. Establish the revoked catalog receipt before either can dispatch. ## Verification - Combined integration head `91f021f233151192f69dd7a035d083abcc67c47c` requires fresh complete CI and review; qualification is in progress. - Repair validation: all **six** recovered preparation provider-boundary cases pass. The catalog cases still require the exact access refusal, zero provider inputs and zero provider turns. The success case still checks preserved file bytes and one native turn. - [Previous exact-head CI](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785508136) failed the catalog case with `remote_workspace_transport_unsupported`. The route created a default heartbeat that could race the injected worker and allocate outside the controlled transport. The repaired test shares its real heartbeat across both dispatchers and holds the agent start lock until the revoked catalog receipt is committed. It tracks and drains both Retry nudges during teardown. It changes no production access check or expected refusal. - The first broad local reproduction hit a full local disk. It passed 201 cases and failed 86, including explicit manifest free-space errors. Generated build caches were removed from these isolated checkouts. The owning run passed 670/671 cases. Its one failure was the saved-Files loader refusing insufficient temporary disk space. After cleanup, that unchanged test passes in isolation. The final shared-facade and start-lock version passes all six dispatch cases. Both legacy continuation suites pass all 29 cases. The final fixture teardown also retires deferred input before removing recovery proof, so global queue sweeps cannot revisit earlier cases. All seven dispatch and saved-Files controls pass together after that change. No disk reserve, assertion or timeout changed. The initial local typecheck also stopped on `ENOSPC` while materializing provider assets. Server typecheck now passes after cleanup. No test timeout or reserve was lowered. - Current-head CI is complete: 52 successful checks, three expected skips, and a successful security status. Both required gates (`ci / e2e` and `ci / verify`) passed on `0adc2e38103533dd45bcc56f957ac019642c57b1`. [Full CI evidence](https://github.com/paperclipai/paperclip/actions/runs/41898305521). Greptile scored final head `0adc2e38103533dd45bcc56f957ac019642c57b1` 5/5 with no blocking findings or unresolved threads. The earlier verification below describes the previous head. - RED on unchanged master `039e0dad2c`: the certified-preparation sibling passes, while the inherited-session case admits zero successors instead of one. A second isolated RED on latest base `8f06bd4e17`, including #18997, uses the exact final three-node test fixture. The HTTP route returns 202, then the test observes zero successors instead of one. All baseline production files remain unchanged. The initial dependency/setup failures are separate from these behavioral failures. - The positive cases cover the failed-failed-successful chain. They keep the completed result unchanged, verify the original saved file and validate fresh-session continuation authority. Repeated HTTP input and concurrent resume calls keep one successor. - PostgreSQL controls accept a complete 32-run chain. A 33-run chain and a newer-created predecessor return no usable ancestry and do not bind the older recovery record. - Safety controls cover company, task and agent scope, conflicting canonical bindings, missing and cyclic edges, transcript hints, live execution and controllers, unknown stop, failed saving, unsafe writer state, Stop, actor and generation changes, independent holds and mismatched recovery records. - Ancestor controls retain provider-event, native-input, session-checkpoint, controller-recovery, result, process, runtime, accounting and unknown-result guards. Host start and warning prefixes use exact origins, sequence and payloads. A fifth extra event is rejected. - The first complete owning run found 22 introduced admission failures. The initial historical-native allowance also applied to exact-source Retry. It admitted contradictory preparation evidence. The fix now requires a canonical ancestor and the shared strict startup proof. The original direct-source assertions remain unchanged. - The next focused run passed 82 cases, including both catalogue-warning controls. One new result fixture lacked the database's required owner tuple. Two existing controls reached their original 30-second limit. The fixture now uses the real native task and completion-contract owner tuple. A controlled nine-case run passes the result case, both unchanged timed-out controls, three failure-code controls and all three positives. The earlier timeout causes remain undetermined. All failed-run logs are retained. - Complete owning suites passed on previous head `137a8eb3febf41052e112fc3c4ffe1abb92bcab7`: all 675 tests across workspace recovery, task Retry, explicit continuation and legacy continuation. The command exits successfully with normal teardown. This includes every earlier introduced admission failure and the two unchanged timeout controls. Its base includes the merged #18997 authority repair and #18982 rolling-copy packaging repair. The rebase retained both test imports at their one adjacent-import conflict. - Node 24.19.0 and pnpm 9.15.4. Module boundaries and `git diff --check` pass. The initial PostgreSQL start lacked package library links after install with scripts disabled. The documented hydration restored the isolated dependency; that setup failure was not a behavioral red. - Previous-head [full typecheck](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785506215) and [full build](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785506094) passed. Earlier attempts received synchronized runner shutdown signals and a lost-runner annotation. The shutdown initiator remains unknown. A CI owner used the documented hosted-runner failover for attempt three and restored the original routing value after capture. That attempt exposed the catalog failure above. Source workflows and check requirements remain unchanged. The current head now has green CI, Greptile 5/5, no unresolved review threads, and no merge conflicts. Hosted rollout and the original task are not yet verified. - Combined recovery proof: 24 of 24 cases passed on this exact head in 43.13 seconds, with normal teardown and unchanged assertions and limits. They cover the real recorder, all 14 empty-output rejection controls, ancestor and direct Retry, saved messages, the configured native provider, and verified stop. The targeted TypeScript graph also passed; the preceding integration step passed 95 safety/provider/message cases and all 49 task Retry cases. - The incorporated #19021 was independently red/green with the real recorder: unchanged predecessor code returned HTTP 202 but admitted zero successors. Its 18 focused cases, all 690 owning recovery cases, full typecheck/build, 1,904 local chat cases, complete CI and Greptile 5/5 passed before integration. The unchanged CLI startup lane passed one justified retry; no hosted acceptance is inferred from those results. - Additional local qualification of the unchanged workspace fixture alongside the native bootstrap fixture passed 288 of 289 cases. One manual-admission HTTP case ended with `socket hang up` after 451 ms, without an assertion or HTTP status. That unchanged case passed once in isolation at the original 40-second limit, with normal teardown, and passed in exact `0adc` CI in 17.2 seconds. The earlier disconnect cause remains unknown; no assertion or timeout changed. - An earlier CI attempt failed the newly merged native ingress fixture because its shared Node command failed strict permission qualification. The current base includes #19020's owned executable fixture repair; no production permission check was relaxed. - Integration with current master had one documentation conflict. An automatic test merge also duplicated an app declaration; that syntax failure was retained and fixed by reusing the configured real HTTP app. Both new message-state assertions remain intact. Workflows and lockfile retain master's contents. ## Risks - A damaged or overlong chain remains held. The helper does not truncate it and grant partial authority. - The new query runs only for a deliberate Retry or its ancestor strategy check. It adds no background polling, table, credential or provider policy. - A fresh intent is not stop or file-recovery proof. The existing owners retain those checks. Prior effects remain fenced, and no old result is replayed. - Local tests do not prove hosted rollout. Live acceptance must use the served fixed revision and the task's original saved work. ## Model Used OpenAI GPT-6 through Codex for the original change. OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, for the merge and CI repair. Both used repository tools and PostgreSQL verification. Context-window limits were not exposed by the harness. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing>
44 lines
2.9 KiB
TypeScript
44 lines
2.9 KiB
TypeScript
import { and, asc, eq } from "drizzle-orm";
|
|
import { heartbeatRunEvents, heartbeatRuns, type Db } from "@paperclipai/db";
|
|
import { isUuidLike } from "@paperclipai/shared";
|
|
|
|
type Source = Pick<typeof heartbeatRuns.$inferSelect, "id" | "companyId" | "agentId" | "error">;
|
|
|
|
export async function readFailedPreparationSettlement(db: Db, source: Source, allowRunStarted = false): Promise<{
|
|
unsupportedHarnessWarning: { message: string | null; revision: string } | null;
|
|
} | null> {
|
|
if (typeof source.error !== "string" || !source.error.length) return null;
|
|
const events = await db.select().from(heartbeatRunEvents).where(and(
|
|
eq(heartbeatRunEvents.companyId, source.companyId), eq(heartbeatRunEvents.runId, source.id),
|
|
)).orderBy(asc(heartbeatRunEvents.seq)).limit(allowRunStarted ? 5 : 4);
|
|
const hostEvent = (event: typeof heartbeatRunEvents.$inferSelect) => event.agentId === source.agentId
|
|
&& event.stream === "system" && event.sourceInstanceId === null && event.sourceEventId === null
|
|
&& event.sourceSeq === null && event.sourcePayloadSha256 === null && event.protocolSchemaVersion === null;
|
|
let offset = 0;
|
|
let unsupportedHarnessWarning: { message: string | null; revision: string } | null = null;
|
|
const warning = events[0];
|
|
if (warning?.eventType === "agent_observability.unavailable") {
|
|
if (!hostEvent(warning) || warning.seq !== 1 || warning.level !== "warn"
|
|
|| warning.payload?.reason !== "unsupported_harness" || typeof warning.payload.revision !== "string" || !isUuidLike(warning.payload.revision)
|
|
|| Object.keys(warning.payload).some(key => !["reason", "revision"].includes(key))) return null;
|
|
unsupportedHarnessWarning = { message: warning.message, revision: warning.payload.revision };
|
|
offset = 1;
|
|
}
|
|
const started = events[offset];
|
|
if (allowRunStarted && started?.eventType === "lifecycle" && started.message === "run started") {
|
|
if (!hostEvent(started) || started.seq !== 1 + offset || started.level !== "info" || started.payload !== null) return null;
|
|
offset++;
|
|
}
|
|
const [error, measured] = events.slice(offset);
|
|
if (events.length !== 2 + offset || !error || !measured || !hostEvent(error) || !hostEvent(measured)
|
|
|| error.seq !== 1 + offset || error.eventType !== "error" || error.level !== "error"
|
|
|| error.message !== source.error || error.payload !== null
|
|
|| measured.seq !== 2 + offset || measured.eventType !== "lifecycle" || measured.level !== "info" || measured.message !== "task.run.measured"
|
|
|| measured.payload?.phase !== "task.run.measured" || measured.payload.outcome !== "failed"
|
|
|| measured.payload.durableSettlement !== true
|
|
|| Object.keys(measured.payload).some(key => !["phase", "outcome", "durationMs", "durableSettlement"].includes(key))
|
|
|| typeof measured.payload.durationMs !== "number"
|
|
|| !Number.isFinite(measured.payload.durationMs) || measured.payload.durationMs < 0) return null;
|
|
return { unsupportedHarnessWarning };
|
|
}
|