Commit Graph

5 Commits

Author SHA1 Message Date
Nicky Leach b2752b21d5
fix(adapter-utils): allow process sessions without birthtime (#12451)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Remote agent adapters use a process-session wrapper inside a
sandbox.
> - Some supported sandbox filesystems do not report inode creation
time.
> - The wrapper rejects a zero creation time before it launches the
agent.
> - This pull request accepts that filesystem shape and keeps the
existing change-time probe.
> - The benefit is that valid sandbox runs can start without a
birth-time field.

## Linked Issues or Issue Description

**What happened?**

The remote process-session wrapper exited before it launched the agent
when the sandbox filesystem reported `birthtimeMs` as zero.

**Expected behavior**

The wrapper must start on a filesystem that does not report inode
creation time.

**Steps to reproduce**

1. Start the remote process-session wrapper.
2. Make `lstat()` report a zero `birthtimeMs` for its session directory.
3. Observe that the pre-fix wrapper terminates before the child process
starts.

**Paperclip version or commit**

Reproduced from `66e1c0df8b23cb8354b36dd446d9548dc4389191`.

**Deployment mode**

Self-hosted server with a remote sandbox runtime.

## What Changed

- Allow a zero reported creation time for process-session directories.
- Keep the probe that rejects a creation time copied from change time.
- Add a regression test that launches and stops a session with zero
birth time.

## Verification

- `npx vitest run
packages/adapter-utils/src/execution-target-stdin-race.test.ts`
- `pnpm --filter @paperclipai/adapter-utils typecheck`

## Risks

A filesystem without creation time can reduce the precision of
sandbox-local path-swap detection. This change does not change host-file
or Paperclip API authority. A follow-up will review that larger security
posture alignment.

> I checked `ROADMAP.md`. This is a focused compatibility bug fix for
the existing sandbox-agent roadmap area.

## Model Used

OpenAI Codex — GPT-5.6. The exact deployment suffix and context-window
size are not exposed. The model used reasoning, shell tools, 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>
2026-08-28 10:33:22 -07:00
Nicky Leach e628cf35da
fix(adapter-utils): harden the wrapper birth-time probe with exclusive create and identity-aware cleanup (#12248)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Adapter utilities run process sessions in local and remote
environments.
> - The remote process-session wrapper uses a probe file to verify
directory creation time.
> - A peer could pre-create the probe path or replace it before cleanup.
> - This pull request uses exclusive create and file-descriptor identity
checks to protect the probe.
> - The benefit is safer cleanup and fail-closed behavior at the sandbox
boundary.

## Linked Issues or Issue Description

**What existing behavior does this improve?**

The remote process-session wrapper creates and removes a birth-time
probe file. The old path-based flow did not prove that the wrapper
created the path or that the path still named the same file.

**Current behavior**

A sandbox peer can race with the probe path. The peer can pre-create a
symbolic link or replace the probe before cleanup. The wrapper can then
inspect or remove an object that it did not create.

**Proposed behavior**

The wrapper creates the probe with exclusive create. It reads `(dev,
ino, ctimeMs)` from the open file descriptor. It removes the path only
when a final identity read matches the created file.

**Reason and benefit**

This change prevents symlink-following during creation and avoids
removal of a peer's replacement object. The wrapper still fails closed
when it cannot prove a real creation time.

**Breaking changes**

None. The wrapper keeps its existing fail-closed capture behavior.

## What Changed

- Create the birth-time probe with `fs.open(path, "wx")`.
- Read probe identity with `fstat` from the open descriptor.
- Remove the probe only after a matching final identity read.
- Add focused race tests for ordinary cleanup and file, directory, and
symbolic-link replacement.

## Verification

- Run `pnpm --filter @paperclipai/adapter-utils exec tsc --noEmit`.
- Run the focused suite
`packages/adapter-utils/src/execution-target-stdin-race.test.ts`.
- Confirm that the focused suite passes all 33 tests.

## Risks

The change affects shared wrapper source for local and remote process
sessions. An identity read or cleanup failure leaves the probe in place
and stops capture. The focused tests cover the new race paths.

## Model Used

OpenAI Codex, GPT-5, tool use and code execution. The model reviewed and
prepared this pull request from the supplied implementation and test
results.

## 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>
2026-08-26 17:51:36 -07:00
Nicky Leach 4277ecbb2e
fix(adapter-utils): terminate the remote process-session wrapper deterministically on bridge stop (#12244)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Adapter utilities run remote process-session wrappers for sandbox
work.
> - A wrapper can outlive its host run when the host removes its session
directory during shutdown.
> - A failed directory read can look like an empty queue, so the wrapper
can poll forever.
> - This pull request adds an explicit shutdown acknowledgement and
fail-closed identity checks.
> - The benefit is deterministic wrapper cleanup without killing an
unrelated session.

## Linked Issues or Issue Description

Refs: #11916

**What happened?**

Remote process-session wrappers could remain after a host run ended. The
host could remove the session directory before the wrapper read the
shutdown marker. The wrapper then treated directory errors as an empty
queue and continued to poll.

**Expected behavior**

The host must receive an explicit shutdown acknowledgement before it
treats the wrapper as stopped. The wrapper must stop when its session
identity becomes invalid or untrusted.

**Steps to reproduce**

1. Start a remote process-session wrapper.
2. Stop the bridge while the wrapper polls its session directory.
3. Remove the session directory during the poll.
4. Observe that the wrapper must terminate with its child.

**Paperclip version or commit**

`7cfbd1ecbe4a40261ba51fed07f624524352ada2`

**Deployment mode**

Built from source with the adapter-utils test suite.

## What Changed

- Add a shutdown control file and wait for a bounded `shutdownAck`
before session cleanup.
- Require `shutdownAck` as proof of host-side shutdown.
- Capture and verify session and stdin directory identity before each
poll.
- Terminate and latch the wrapper on missing, changed, linked,
non-directory, or untrusted paths.
- Reject unusable creation times and treat all identity-check `lstat`
errors as terminal.
- Add focused regression coverage for shutdown races and identity
failures.

## Verification

- `npx vitest run
packages/adapter-utils/src/execution-target-stdin-race.test.ts` passes.
- The full execution-target set passes: 175 tests across three files.
- The `packages/adapter-utils` typecheck passes with `tsc --noEmit`.
- CI will run on this pull request.
- Greptile will review the pull request.

## Risks

- A platform with unreliable directory creation times can stop a wrapper
earlier than before. This fail-closed result prevents an orphan.
- A transient identity-check error now stops the wrapper. This favors
cleanup over continued polling when the session identity cannot be
trusted.
- Session cleanup remains unconditional after the bounded
acknowledgement wait.

## Model Used

OpenAI Codex — GPT-5. Context window size is not exposed in this run.
The model used tool calls 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>
2026-08-26 15:12:49 -07:00
Nicky Leach ed8075b535
fix(adapter-utils): order stdin file writes in the sandbox process-session bridge (#11406)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent adapters can run through a sandbox process-session bridge
> - The bridge writes streamed standard input to files before the remote
process reads them
> - Concurrent file writes can make a later chunk visible before an
earlier chunk
> - The remote process can then parse a tail fragment and wait forever
for the missing head
> - This pull request serializes host writes and makes an unexpected
file gap a loud error
> - The benefit is ordered input with a bounded failure path for sandbox
ACP sessions

## Linked Issues or Issue Description

No public GitHub issue exists for this change. The description below
follows `.github/ISSUE_TEMPLATE/bug_report.yml`.

**What happened?**

A sandbox ACP process-session bridge could stall after its handshake
when a run sent a large prompt. The host sent one un-awaited file write
for each standard input chunk. A small later chunk could finish before a
large earlier chunk. The remote poller then sent the tail bytes first.
The agent parser raised an error on the tail fragment, and the head
bytes stayed buffered without a newline.

**Expected behavior**

The bridge must expose standard input files in sequence. The remote
poller must report a clear error when an earlier file remains missing
beyond the retry budget.

**Steps to reproduce**

1. Start an ACP session through a sandbox process-session bridge.
2. Send a prompt that produces multiple standard input file chunks.
3. Delay finalization of an earlier chunk while a later chunk completes.
4. Observe that the remote parser can receive the later chunk first and
the session can stop without a clear error.

**Paperclip version or commit**

The change targets the current `master` branch at the submitted commit.

**Deployment mode**

The bug affects sandbox execution.

**Agent adapter(s) involved**

The failure affects the ACP process-session bridge.

**Database mode**

Not database-related.

**Additional context**

The fix keeps the existing per-file atomic write behavior. It adds
ordering at the host write boundary and a bounded ordering check in the
shared wrapper poll tail.

## What Changed

- Add a per-session promise chain for host standard input file writes.
- Keep a failed write from blocking later chain entries.
- Track the next expected sequence number in the shared wrapper poll
tail.
- Hold later files while an earlier file is missing within the existing
retry budget.
- Emit a loud error and advance after the retry budget expires.
- Add regression tests for host ordering, gap holding, and the loud
error path.

## Verification

- `npx vitest run
packages/adapter-utils/src/execution-target-stdin-race.test.ts` — 9
tests passed.
- `npx vitest run
packages/adapter-utils/src/execution-target-sandbox.test.ts` — 43 tests
passed.
- `pnpm --filter @paperclipai/adapter-utils typecheck` — clean.
- With the source fix reverted, the 3 new tests fail and the 6 original
tests pass.
- CI must pass on the pull request before merge.

## Risks

Low risk.

- The host now serializes writes for each session, which can reduce
write parallelism.
- A failed write still emits one error and destroys the socket, as
before.
- The wrapper can emit a loud error after the existing retry budget when
a file gap persists.
- The change does not alter the atomic per-file write behavior.

## Model Used

OpenAI GPT-5, model ID `gpt-5`, with tool use and code execution. The
context window and internal reasoning details are not disclosed.

## 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>
2026-08-14 18:45:31 -07:00
Nicky Leach 8f478242f1
fix(adapter-utils): close sandbox stdin file race with atomic write and fault-tolerant poller (#11235)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent adapters use execution targets to exchange input and output
with sandbox processes
> - The sandbox input path can expose partial files, and the poller can
delete or stop on invalid input
> - These timing windows can lose agent input without a clear error
> - This pull request makes host writes atomic and makes the poller
retry invalid files before it drops them
> - The benefit is reliable sandbox input delivery with visible failure
after bounded retries

## Linked Issues or Issue Description

Closes #10874

## What Changed

- Decode host input into a temporary file, then rename it onto the final
JSON path.
- Apply the same atomic write pattern to the filesystem client.
- Parse each input file before deletion.
- Retry parse failures and drop a file after the bounded retry limit
with an error event.
- Add regression tests for empty, partial, and permanently malformed
input files.

## Verification

- `pnpm exec vitest run
packages/adapter-utils/src/execution-target-stdin-race.test.ts` passes
5/5 tests.
- Related sandbox callback, execution target, and sandbox execution
suites pass 71/71 tests.
- TypeScript checks pass for the changed files.
- The regression suite fails on the old code and passes on this change.

## Risks

- Low risk. The change affects sandbox input file handling and adds
bounded retry behavior.
- A permanently malformed file now creates an error event after the
retry limit.

> This bug fix does not add a core feature, so a roadmap change is not
needed.

## Model Used

Codex, OpenAI GPT-5, current agent runtime, large context window, tool
use and code review support.

## 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>
2026-08-11 16:05:41 -07:00