Commit Graph
4 Commits
Author SHA1 Message Date
DottaandPaperclip 94e4218310 refactor(heartbeat): extract runtime service coordination (#15980)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Heartbeat runs start workspace services and receive service reports
from adapters.
> - Service context updates and readiness notices are spread through the
executor.
> - These steps share a clear responsibility, while process and lease
ownership already live elsewhere.
> - This pull request puts service coordination in one module.
> - The executor retains the same dispatch and cleanup order.

## Linked Issues or Issue Description

**What existing behavior does this improve?**
Maintenance of heartbeat execution. This is the first of two final
focused extractions after #15973. Related #15653 changes runtime
callback origins; this PR preserves that policy.

**Current behavior**
`heartbeat.ts` has 4,372 lines and 178,459 bytes. It validates service
intents, starts services, persists adapter reports and primary URLs, and
posts readiness notices inline.

**Proposed behavior**
`heartbeat/runtime-services.ts` owns those steps in 212 lines.
`heartbeat.ts` becomes 4,215 lines and 173,657 bytes. Process registries
and leases stay in `workspace-runtime.ts`. The executor still releases
services in its original cleanup path.

**Reason and benefit**
Service coordination has named preparation and result phases. The
execution path is easier to read without changing service lifetime or
moving cleanup policy.

**Breaking changes**
None intended. All 140 named public heartbeat exports remain available,
including `postWorkspaceReadyComment`.

## What Changed

- Move service-intent validation, startup, adapter-result persistence,
and readiness notices into one module.
- Preserve mutable run context, primary URL selection, notice error
handling, and release ordering.
- Add four public-path tests using disposable PostgreSQL, real HTTP
service processes, and a fixture adapter.
- Extend browser recovery acceptance to fetch the actual service URL,
check the readiness notice, and verify listener cleanup.
- Update the module map and acceptance instructions.

## Verification

- Before extraction: all five tests pass, including the four new
characterization cases and existing notice formatting test.
- After extraction: all 17 tests pass across service coordination,
notice formatting, output recording, and runtime authorization.
- Structural comparison confirms that all moved bodies and the
reconstructed executor match the baseline. Module boundary checks pass.
- `pnpm -r typecheck` and `pnpm build` pass.
- Extended isolated browser acceptance: **2/2 pass** (3.7 minutes),
covering actual service readiness, recovery, bounded failure, and
cleanup. At `8e33e2032de681082f128470eb8ab0711b382afe`, all 55
current-head checks pass (two configured skips), including hosted
full-suite tests. Greptile is 5/5 with no findings or review threads.

## Risks

- Service context must be persisted before adapter dispatch and before
run completion. The tests inspect both phases and preserve the first
service URL when adapter reports arrive.
- Notice failures must remain nonfatal. The public-path test checks both
startup and result notices.
- Shared services must survive run cleanup; ephemeral services must stop
after success or failure. Tests use actual processes and HTTP requests.
- The fixture supplies UUID service report IDs, as required by the
database. The existing generated-ID fallback for reports without IDs is
outside this extraction.
- Test adapters and services are local fixtures; these tests make no
live provider calls.

## Model Used

OpenAI Codex, based on GPT-6, assisted with reasoning, code edits, tool
execution, and verification. 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-10-12 10:40:42 -05:00
DottaandPaperclip ce15fe7c73 refactor(heartbeat): extract task context preparation (#15872)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Heartbeat execution prepares task context before it selects an
environment and starts an adapter.
> - This preparation still occupied 319 lines inside the executor after
the earlier extractions.
> - It combines continuation summaries, comments, prompt variants,
communication guidance, and redaction.
> - This pull request moves that responsibility into one typed module
under `services/heartbeat/`.
> - The executor keeps admission, resource ownership, dispatch, and
cleanup ordering together.
> - The result makes the preparation phase easier to find and test
without changing execution behavior.

## Linked Issues or Issue Description

Refs #15825, #15807. This continues the incremental heartbeat extraction
series. A search found no duplicate task-context extraction.

**What existing behavior does this improve?**
Maintenance and review of heartbeat task context preparation.

**Current behavior**
The executor assembles prompts and trust-filtered context inline. This
hides its execution phases among preparation details.

**Proposed behavior**
Call `prepareHeartbeatTaskContext` at the same point in execution.
Preserve statement order, prompt contents, public exports, and the
shared run context object.

## What Changed

- Move task context assembly to
`server/src/services/heartbeat/task-context.ts`. Inputs group task
state, policy, and two existing service methods. Results preserve the
continuation and wake values used by later runtime selection and
dispatch.
- Keep company-scoped loading, quarantined-content filtering, secret
redaction, and revision hashing in their original order. Add no
execution resource acquisition.
- Add four public-path integration tests before the move. Cover prompt
variants and redacted revisions, quarantined summaries/comments, a
mixed-company wake batch, and stale caller fields.
- Extend the real process worker in the opt-in browser suite to check
persisted prompt variants and the description revision before it
completes its task.
- Update the module map and acceptance-test documentation.
- Reduce `heartbeat.ts` from 5,314 lines / 220,784 bytes to 4,989 lines
/ 206,650 bytes. The new module has 376 lines / 15,912 bytes.

## Verification

- Before the move: 11 existing prompt tests and all four new
characterization tests passed against the inline implementation.
- Normalized syntax comparison passed for all 319 moved lines and the
remaining orchestrator, including public exports. Imports and the typed
boundary were reviewed separately.
- `pnpm -r typecheck` passed.
- `pnpm build` passed.
- The clean post-move focused run passed all 26 tests in five files,
with no skips: task-context characterization, task-markdown, Slack
invitation, accepted-plan workspace refresh, and run-preparation.
- The local `pnpm test:run` attempt was interrupted after database
failures while the Mac exhausted its shared-memory slots. All 15
affected native warm-ownership database cases passed in an isolated
rerun after the browser instance stopped; the other 666 cases in that
file were excluded by the test-name filter. The interrupted command is
not a full-suite pass. Full CI passed on
`6bf86cfdc07e7fe34015f7b719218639f4232510`: 55 successful checks and two
optional Storybook checks skipped. Greptile reviewed the latest commit
at 5/5; its one earlier worker-test finding is fixed and resolved.
- Browser E2E passed both scenarios on the final code: persistent
timeouts stop after three runs and survive reload; one timeout recovers
without manual Retry, verifies task-context prompt variants/revision and
preserved workspace files in a real process, then completes through the
run-scoped API. The first recovery attempt exposed an incorrect new test
assertion because the fixture title equals its description. The worker
now checks the description section specifically; the corrected recovery
scenario passed in 55.9 seconds. The original failure trace was
preserved. These tests use a deterministic process adapter and make no
live model calls.

## Risks

This is a behavior-preserving extraction. The main risks are changed
closure bindings, missing return values, or changed preparation order.
Typed inputs, public-path tests, and syntax comparison cover those
risks. The run context is still the same mutable object. The helper
acquires no leases or processes. It introduces no schema, API, or prompt
policy changes.

The first post-move database run skipped cases due to host resource
exhaustion. All 26 focused cases passed on the subsequent clean run.
Browser E2E passed after the worker assertion fix. The full local suite
remains incomplete. All 15 native warm-ownership database cases passed
on their isolated rerun; The full CI test suite passed on the final
commit.

## Model Used

OpenAI Codex, based on GPT-6, with reasoning, repository inspection,
code execution, and test tools. The host does not expose the exact
deployment model ID or context-window size.

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