Commit Graph
1 Commits
Author SHA1 Message Date
DottaandPaperclip 396b7fc62a fix(runner): scope session diagnostics to the active run recorder (#18944)
## Thinking Path

> - Paperclip manages agents and their work.
> - The native executor can retain a transport between runs.
> - Each run recorder has a shorter lifetime than that transport.
> - The transport kept its first recorder after warm reuse and
completion.
> - A later close could write to the wrong run or a sealed object log.
> - This change binds diagnostics to the active run epoch and retires
that binding at execution exit.
> - The existing recorder still owns accepted-byte draining and honest
incomplete finalization.

## Linked Issues or Issue Description

**What happened?**

A warm native transport retained the first run's diagnostic callback.
Reusing the session for another run did not change that callback. A
later idle close could append to a retired writer and emit a persistence
warning.

**Expected behavior**

Active diagnostics belong to the current company, session and run. A
retired transport callback must not use a completed recorder. Diagnostic
metadata must not delay essential execution cleanup.

**Steps to reproduce**

Run the native warm-session regression with two sequential run owners.
Close the retained transport after both runs finish. The original code
calls the first recorder. The real completion regression also observes
the resulting closed-writer warning after normal lease cleanup.

This is a local source defect with behavioral red/green proof. Repeated
hosted generic persistence warnings motivated the investigation, but
their original exceptions were not retained. This PR does not claim to
identify every such warning or repair an export failure.

Related #18379 queues sanitized performance spans in heartbeat. It does
not own transport diagnostic callback lifetime.

## What Changed

- Bind transport diagnostics to an exact active company/session/run
epoch, including warm reuse.
- Activate a retained channel in the existing session-admission hook
after identity validation and before attachment starts, including an
attachment failure before `onSession`.
- Forward active diagnostics synchronously. Detach their recorder at
execution exit. Keep the original asynchronous metadata semantics.
- Keep idle transport evidence inspectable. Log only a fixed category
and trusted company/run identifiers to the process logger.
- Preserve recorder-owned byte draining, redaction, source coverage and
genuine persistence-failure warnings.
- Cover actual native executor, completion and lease-close boundaries,
including stalled local appends and exact object-log source/hash
preservation.
- Place the retained-workspace test fixture under the existing
published-path contract after merging newer recovery tests from master.
Production validation and native/catalog assertions remain unchanged.

## Verification

- Current head: `c500e4933a85a4c453f66c872fa335108c661b15`. Normal
master merges preserve upstream behavior. The one textual conflict was
in the executor imports.
- The first GitHub run tested merge tree
`a4cb59448a3b5cc2047ac41c691720e6acec6e8d`, which included newer
recovery tests from master. Five cases failed with
`remote_workspace_published_path_invalid`. The same five failures
reproduced on an identical local tree. The fixture now puts retained
files under `.paperclip-workspaces/<workspaceId>` and keeps the target,
lease and authority bound to that path. Production validation and the
native-success/catalog-denial assertions are unchanged.
- On unchanged `c500e493`, the full recovery file passed **244/244**
tests. Existing cold-restore and occupied-root controls passed **4/4**.
Server typecheck, server production build, module boundaries and
whitespace checks passed. An intermediate missing-import setup failure
was corrected and is separate from the five-case behavioral red.
- Before that fixture correction, local `f2fdaddb` passed **1,221
tests** across the five diagnostic, executor, output, completion and
object-store files. Recursive typecheck and full repository build also
passed. Those results are tied to that earlier revision; [fresh
current-head
CI](https://github.com/paperclipai/paperclip/actions/runs/41842043248)
must qualify the final merge.
- Behavioral regressions cover the original closed-writer warning, warm
A-to-B ownership, and an actual Runner attachment exception before
`onSession`. The admission hook gives the attachment diagnostic to B and
leaves A untouched. The attachment fixture uses synthetic event
persistence and outer failure projection; it does not certify a real
database terminal projection.
- The tests preserve synchronous diagnostic admission and asynchronous
metadata recording. A stalled append reaches the existing bounded
finalization result with unknown size/hash. The normal object-log
control preserves canonical bytes, SHA256 and complete source coverage
after lease cleanup.
- An initial local run had **37 PostgreSQL startup failures** because
the Darwin package lacked aliases to its included versioned libraries.
Only isolated `node_modules` aliases were restored. No source or
lockfile changed for this setup issue. A passing intermediate run
crossed a source merge and is excluded from immutable-head proof.
- An earlier combined test run passed 1,216 assertions but exited with a
completion `afterAll` timeout. Its cause remains unproved. Later
isolated 60/60 passes used the original timeout and do not erase that
failure. An earlier repository-wide `pnpm test:run` was interrupted
after its source became obsolete; it is not full-suite or current-head
proof.

Greptile reviewed the exact `c500e493` head at **5/5** with no open
findings. Fresh CI must pass before merge. The native review is queued
behind its separate saved-file recovery; no current-head native
assessment is claimed.

The provider execution loop is a fixture. These tests do not start a
model or prove hosted recovery. Missing accounting receipts remain
pending, not zero cost. Checkout-release invocation is observed; this is
not a separate task-arbiter journey. The original native review
published a valid finding before a separate workspace-export failure.
The finding is fixed; this PR does not claim to fix that export failure
or every hosted persistence warning.

## Risks

Idle process logs intentionally omit raw provider text. The existing
bounded transport evidence retains its own diagnostics. Accepted fs
appends that do not settle retain the existing unknown-size/hash warning
and cannot be certified complete. No new timeout, storage policy, schema
or provider execution is added.

## Model Used

OpenAI `gpt-6.1-sol` through Codex assisted with diagnosis,
implementation, tests and review. The exact context-window size is not
exposed. Tool use and code execution were enabled.

## 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 local tests and recorded passing checks, pending checks
and unresolved failures above
- [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] I will require all Paperclip CI gates to be green before merge
- [x] I will require Greptile 5/5 with no unresolved findings before
merge
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-11-04 08:04:52 -06:00