Commit Graph
2 Commits
Author SHA1 Message Date
DottaandPaperclip 07fee85eaf refactor(heartbeat): extract workspace preparation (#15807)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The heartbeat service prepares the workspace and dispatches each
agent run.
> - After the runtime extraction, `heartbeat.ts` still had 6,417 lines.
> - Workspace setup occupied one contiguous 857-line block inside
`executeRun`.
> - This pull request moves that block into one workspace preparation
module.
> - The executor keeps dispatch gates and terminal cleanup. Reviewers
can inspect workspace setup as one phase.

## Linked Issues or Issue Description

Refs #15803 and #15578. This continues the heartbeat refactor series.
The search found no open duplicate workspace preparation extraction.

**What existing behavior does this improve?**

It makes heartbeat workspace preparation easier to maintain. It does not
change the workspace policy or execution behavior.

**Current behavior**

`executeRun` contains workspace resolution, freshness decisions,
provisioning, persistence, issue binding, environment acquisition, and
realization inline.

**Proposed behavior**

`prepareHeartbeatWorkspace` owns that sequence in
`server/src/services/heartbeat/workspace-preparation.ts`. `heartbeat.ts`
calls it at the same point and uses its returned workspace, lease,
target, and metadata.

**Reason and benefit**

The main file falls from 6,417 to 5,597 lines. The workspace phase has a
typed boundary and direct regression tests.

**Breaking changes**

None. There are no schema, API, feature-flag, or execution-policy
changes.

## What Changed

- Extract the 857-line workspace preparation block without changing its
operations or their order.
- Preserve native recovery bindings, workspace persistence failure
cleanup, issue rebinding after realization, and the cloned restore
lease.
- Keep environment selection, dispatch admission, terminalization, and
final lease release in `heartbeat.ts`.
- Add 12 boundary tests with disposable PostgreSQL, real Git
repositories, and real local environment leases.
- Make the existing Stop and plan-only recovery tests await executor
completion instead of short polling deadlines. Assert exactly one
continuation for the specific plan-only source run.
- Keep server-only Node preload hooks out of the deterministic process
adapter used by Stop browser acceptance.
- Refresh workspace recovery browser acceptance for the current task
composer, and give its disposable test-drive an explicit 10-second
snapshot deadline. Production snapshot settings stay unchanged.
- Document the workspace preparation boundary in `doc/DEVELOPING.md`.

## Verification

- AST comparison: all 857 moved lines match the original block. The
remaining module body differs only at the extraction call.
- `pnpm build`: passed.
- `pnpm -r typecheck`: passed.
- Focused regression coverage: 651 distinct cases across eight suites
passed across targeted runs, including all 12 new boundary tests. The
complete 400-test recovery suite passed together on the final commit.
- Workspace bootstrap browser acceptance: both journeys passed (4.2
minutes), including preserved source edits, ignored-file exclusion,
bounded retries, and persistence across reload.
- Stop/queue browser acceptance: both process and native runner journeys
passed (2.2 minutes), covering queueing, Stop, subtree pause/cancel,
resume, and unrelated-task isolation.
- [Full repository
CI](https://github.com/paperclipai/paperclip/actions/runs/38062408589):
all 47 jobs passed, including the general and serialized test suites,
eight browser shards, build, typecheck, native runner checks, and canary
dry run.
- The local `pnpm test:run` was interrupted to prioritize the recovery
regression; the complete repository suite was verified in CI.
- Greptile: 5/5 on `c28e698b885cf5fd12f36f7b4052e7a2e6f879d7`, with no
open review threads.

## Risks

- This code writes durable workspace bindings and acquires environment
leases. A changed operation order could break recovery or cleanup after
partial setup. The extraction preserves the original statements and
exception boundaries.
- The new tests cover creation, reuse, immutable recovery bindings,
persistence failure, acquisition and realization failure, controller
ownership loss, isolation, execution policy, and lease snapshot
independence.
- Browser acceptance uses real application services with deterministic
local workers and a fake Codex protocol provider. It does not qualify
live cloud providers.

## Model Used

OpenAI GPT-6 through Codex, with code editing, shell execution, and test
tools. The exact serving model identifier and context-window size are
not exposed in this session.

## 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-10-10 10:30:17 -05:00
DottaandPaperclip f4cdc7b231 fix: recover transient workspace bootstrap scans (#13481)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The control plane prepares task workspaces before it starts an
agent.
> - Workspace preparation reads Git state so it can preserve edits and
exclude private files.
> - A failed scan was treated as a non-Git folder and lost its actual
failure code.
> - The resulting generic setup failure could not recover, even when the
cause was temporary.
> - This pull request keeps the cause and uses the existing bounded
retry schedule before provider startup.
> - Tasks can recover without human intervention, while permanent
failures and exhausted retries stop with useful guidance.

## Linked Issues or Issue Description

**What happened?**

A Git scan error during managed repository preparation became
`Configured repository folder is not a Git checkout`, followed by
generic `setup_failed`. The agent never started. Generic recovery could
not distinguish a temporary timeout from a bad workspace configuration.

**Expected behavior**

Keep the closed scan error code. Retry temporary timeouts and queue
saturation under the existing shared budget. Preserve edits, exclusions,
ownership, and pause gates. Stop permanent failures and exhausted
retries with a specific explanation. Do not replay historical generic
setup failures.

**Steps to reproduce**

1. Configure a task project with a local Git source that must be copied
into its managed repositories.
2. Make the ignored-file scan exceed its timeout before the agent
starts.
3. Before this fix, the snapshot returns null and the run ends as
non-retryable `setup_failed`.
4. Use the disposable browser fixture in
`tests/e2e/workspace-bootstrap/README.md` to inject real timeouts and
test the full recovery path.

**Paperclip version or commit**

Reproduced against `4510bf7c9e2fcbeb043445850928b5dcb79908ca`.

**Deployment mode**

Built from source. The defect is in core workspace setup, not a specific
model provider.

Related work: Refs #13442 (managed repository preparation), Refs #11572
(bounded Git scheduler), Refs #12997 (separate adapter startup retry
work), Refs #13469 (separate terminal-workspace scan performance work).

## What Changed

- Return the non-Git fallback only for repository discovery. Propagate
failed scans of a confirmed repository.
- Replace full ignored status output with an ignored-only directory
listing. Preserve NUL-delimited paths and exclusions.
- Preserve typed, sanitized scan errors through workspace preparation
and persist pre-provider failure details.
- Retry only timeouts and queue saturation, using the existing durable
two-retry budget and issue gates. Prevent generic recovery from adding
another budget.
- Show workspace-specific failure copy and actionable exhausted-recovery
notices.
- Add red-green unit tests, real-database restart and retry-boundary
tests, and opt-in browser acceptance fixtures with real Git subprocess
timeouts.
- Document the recovery contract and browser verification procedure.

## Verification

- Red: injected scan failures returned null instead of rejecting; setup
lost the timeout code; task-thread and recovery notices had generic
copy.
- Green: 119 focused adapter/backend tests, 20 recovery-boundary tests,
and 136 task-thread tests.
- `pnpm -r typecheck` — passed.
- `pnpm build` — passed on the final production code.
- `pnpm check:token-gates` — passed.
- The initial local `pnpm test:run` overlapped source edits and was
interrupted after two late-added assertions saw pre-fix behavior; it is
not counted as a green full run. A fresh final-head run passed all 229
tests across the six affected adapter/backend/UI suites. The clean
latest-head CI full test matrix passed: all five general-server shards,
all five serialized-server shards, and all three general-workspace
shards.
- Latest-head CI also passed all three browser shards and their
aggregate gate, typecheck and release registry, build, runner
verification, canary dry run, policy, Docker context integrity, and
security gates. Greptile: 5/5, with the review thread resolved.
- Browser: created a task in a disposable instance. A real Git timeout
scheduled recovery, the next run completed through the run-scoped API
without manual Retry, and Done survived reload. The deterministic
process worker checked preserved source edits and excluded private
files; no model calls were made.
- `WORKSPACE_BOOTSTRAP_TEST_URL=<disposable-instance-url> pnpm exec
playwright test --config
tests/e2e/workspace-bootstrap/playwright.config.ts` — 2 passed (3.6
minutes). The persistent case made exactly three failed attempts, never
started the worker, showed the cause-specific notice, stayed stopped for
another scheduler tick, and retained Blocked after reload.
- Extra red-green coverage: 50 recovery tests passed after fixing an
exhausted-bootstrap classification that incorrectly implied unknown
provider actions. Missing or uncertain evidence still retains the safety
hold.
- Verified the documented Git executable override during repository
seeding.

## Risks

- A confirmed repository scan failure now fails closed instead of
falling back to directory sync. This prevents unfiltered copying but
makes previously hidden errors visible.
- Temporary host problems can create up to two additional setup
attempts, 30 seconds apart. Permanent scan errors do not auto-retry.
Generic recovery cannot reset this budget.
- The durable retry path still enforces ownership, pause, and work
eligibility. Integration tests cover restart, duplicate promotion,
pause, exhaustion, and non-retryable categories.
- No schema migration, new runtime setting, new retry budget, production
deployment, or historical task replay.

## Model Used

OpenAI Codex, GPT-5-based coding agent, with reasoning, repository
tools, shell execution, and browser testing. The exact deployment model
ID and context-window size are not exposed in this session.

## 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-09-15 12:37:21 -05:00