mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-14 08:26:46 +02:00
codex/codex-runner-default
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> |
||
|
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> |
||
|
DottaandPaperclip
|
30e338fba6 |
refactor(heartbeat): extract environment selection (#15825)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Heartbeat execution selects an environment before it prepares a workspace and starts an agent. > - Workspace preparation now has its own module, but its selection policy still occupies the executor. > - Environment defaults, recovery bindings, chat isolation, and workspace contention must retain their order. > - This pull request moves that selection phase behind one typed function. > - The executor keeps dispatch gates and lease cleanup. Tests cover the public path and the new boundary. ## Linked Issues or Issue Description Refs #15807. This continues the heartbeat extraction series after workspace preparation. **Current behavior** The 5,599-line heartbeat service contains a 301-line environment selection phase inside run execution. **Proposed behavior** Move that phase to `heartbeat/environment-selection.ts`. Keep the same policy, statement order, errors, and public exports. **Reason and benefit** Readers can find selection policy in one module and follow the executor's phases without reading each policy branch. ## What Changed - Add `selectHeartbeatEnvironment` with typed run, task, policy, configuration, and service inputs. Return the values needed by configuration and workspace preparation. - Preserve native recovery bindings, environment precedence, managed sandbox enforcement, Kubernetes provisioning, native chat isolation, and shared workspace contention. - Reduce `heartbeat.ts` from 5,599 lines / 234,599 bytes to 5,314 lines / 220,784 bytes. The new module is 402 lines / 18,215 bytes. - Add six public-path characterization cases before moving production code and six boundary cases. Repair plugin-test teardown to drain active execution and call the database cleanup helper. - Assert the selected local environment in the browser fixture's real worker and document the new boundary. ## Verification - Before the move: 71 tests passed, including all six new public-path cases. Three plugin tests skipped when the host could not start their database probe. - After the move: all 112 focused tests passed across eight files, with no skips. These cover environment policy, local and plugin execution, contention, recovery bindings, workspace preparation, chat isolation, and low-trust directories. - Passed `pnpm -r typecheck` and `pnpm build`. - A normalized TypeScript AST comparison confirms the 301 moved lines and remaining executor statements are unchanged, apart from the new call and imports. - Browser acceptance: the task UI recovered automatically and the worker verified its selected local environment, preserved uncommitted files, and completed the task. The persistent-failure case passed in isolation: three total runs, then a blocked task that remained blocked after reload. - The first persistent-failure browser attempt missed the retry banner within five seconds, despite a persisted scheduled retry. Its unchanged isolated rerun passed. The trace is retained; the intermittent UI failure is not yet attributed to a root cause. No assertion or timeout was relaxed. - Greptile reviewed `20953dd164decb565bc8d85081aad31a23fd33e5`: 5/5, no actionable findings. - Hosted CI passed on `20953dd164decb565bc8d85081aad31a23fd33e5`: 52 successful checks and two intentionally skipped Storybook checks, with no pending or failing checks. This includes all general and serialized test groups, Runner verification, eight browser shards, typecheck, and build. - The duplicate local `pnpm test:run` was interrupted after hosted CI completed. It is not reported as a local full-suite pass. Local completed checks are the 112 focused tests, typecheck, build, and browser cases described above. ## Risks - This selection phase enforces execution restrictions. A wrong input or import could select the wrong environment or change retry behavior. Public-path tests cover refusal, redirect, contention, and dispatch; boundary tests cover recovery and isolation. - The helper can provision a managed environment row and add concurrency notes to the supplied task context. It acquires no execution lease. The executor retains its catch/finally boundaries and resource ownership. - Kubernetes selection is tested with real database state and bootstrap settings. These checks do not claim execution against a live Kubernetes cluster or hosted sandbox provider. - No schema, migration, API contract, or intended behavior change. ## Model Used OpenAI Codex (GPT-6), with repository inspection, code editing, compiler checks, database integration tests, and browser automation. The exact serving 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> |
||
|
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> |