mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-13 13:38:01 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Heartbeat prepares, dispatches, and completes each agent run. > - After the runtime-selection extraction, the main service still had 7,295 lines. > - Provider dispatch and workspace finalization formed one large block with shared cleanup rules. > - This pull request moves that block into a typed runtime-execution module. > - The service now has 6,417 lines. Readers can follow admission and outer cleanup without reading the provider execution details. ## Linked Issues or Issue Description Follow-up to #15746. I searched open heartbeat PRs and issues. I found no competing runtime-execution extraction. **What existing behavior does this improve?** The organization and test coverage of native and legacy heartbeat execution. **Current behavior** The heartbeat service contains a 912-line block for provider dispatch, usage capture, workspace finalization, and instruction cleanup. **Proposed behavior** Move that block to `server/src/services/heartbeat/runtime-execution.ts`. Keep the atomic dispatch gate, admission, terminal run status, and outer lease cleanup in the service. Preserve run behavior. **Reason and benefit** This gives runtime execution a named boundary and direct tests. It removes 878 lines from the main service in one bounded change. **Breaking changes** None intended. There are no schema, API, or policy changes. ## What Changed - Add `executeHeartbeatRuntime` for prepared native and legacy execution, usage capture, workspace finalization, and gateway/instruction cleanup. - Return an explicit dispatch result. Update outer ownership state through immediate callbacks, including before exceptions. Read instruction state through live getters. - Move the two internal recovery error classes with their existing behavior. The caller imports the same classes for `instanceof` checks. - Add 15 database-backed execution tests. Keep the existing heartbeat regression suites. - Repair the native Stop acceptance fixture: installed Codex now takes precedence over `PATH`. A test-only loader pins the fake provider. An isolated home and dummy key keep readiness offline. Add four loader tests. - Document the module boundary and fixture setup. ## Verification - Passed: eight focused heartbeat suites (483 distinct tests, including 15 new execution tests). The boundary suite passed again after adding the failed-restore retention case from review. - Passed: `node --test tests/e2e/fixtures/composer-stop-codex-loader.test.mjs` (four tests). - Passed: `pnpm -r typecheck` and `pnpm build`. - Passed: AST comparison of all 912 moved lines after the explicit boundary substitutions. SQL template text, recovery class bodies, public exports, and the rest of the executor match the baseline. - Passed: browser Stop journeys for legacy process and native Paperclip Runner (two tests). The native fixture log proves provider `turn/start` before Stop and `turn/interrupt` after Stop. - Passed: all three ACP continuation browser journeys (same-session queued input, no replay of an interrupted write, and a new direction after composer Stop). Five relevant browser journeys passed in total. - Passed: the full CI test groups, including all server, chat, workspace, serialized-server, runner, and browser shards. - Local full-suite limitation: `pnpm test:run` first hit the macOS shared-memory segment limit while starting embedded PostgreSQL. After releasing only this task’s unused acceptance-server segments, the retry progressed without reported failures. I stopped that duplicate run after the full CI test groups passed. - Passed: all 56 PR checks/statuses on `8694c116e8d43ed8ac11bc4c58623ea3af34a675`. Greptile is 5/5 with no unresolved review threads. ## Risks - Cleanup depends on state written before an error. The module reports dispatch, retention, and recovery ownership immediately. Tests cover early returns and exceptional cleanup. - Instruction callbacks can replace saved metadata during execution. Live getters preserve those changes. - Native acceptance uses a real server, database, browser, and Rust runner with a deterministic provider. It does not verify a live provider service. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and repository/tool access. The exact serving model ID and context-window limit were 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>
52 lines
2.0 KiB
TypeScript
52 lines
2.0 KiB
TypeScript
import fs from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { defineConfig } from "@playwright/test";
|
|
import base from "./playwright.config";
|
|
|
|
// Opt-in native coverage uses real runnerd with the repo's deterministic Codex
|
|
// protocol fixture. Never let this suite fall through to a logged-in real Codex.
|
|
const fixture = process.env.PAPERCLIP_STOP_FAKE_CODEX;
|
|
const fixtureDir = fs.mkdtempSync(
|
|
path.join(os.tmpdir(), "composer-stop-provider-"),
|
|
);
|
|
const logPath =
|
|
process.env.PAPERCLIP_STOP_CODEX_LOG ??
|
|
path.join(fixtureDir, "codex-calls.log");
|
|
process.env.PAPERCLIP_STOP_CODEX_LOG = logPath;
|
|
if (fixture) {
|
|
if (!path.isAbsolute(fixture) || !fs.existsSync(fixture))
|
|
throw new Error(
|
|
"PAPERCLIP_STOP_FAKE_CODEX must name the built fake-codex-app-server binary",
|
|
);
|
|
fs.mkdirSync(path.join(fixtureDir, "codex-home"));
|
|
const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`;
|
|
fs.writeFileSync(
|
|
path.join(fixtureDir, "codex"),
|
|
`#!/bin/sh\nexec ${quote(fixture)} --state-file ${quote(fixtureDir)}/state-$$.json --hold-turn --call-log ${quote(logPath)} "$@"\n`,
|
|
{ mode: 0o755 },
|
|
);
|
|
}
|
|
const server = base.webServer as Exclude<typeof base.webServer, unknown[]>;
|
|
export default defineConfig({
|
|
...base,
|
|
testMatch: "composer-stop.spec.ts",
|
|
timeout: 90_000,
|
|
webServer: {
|
|
...server,
|
|
env: {
|
|
...server?.env,
|
|
HEARTBEAT_SCHEDULER_INTERVAL_MS: "10000",
|
|
PATH: `${fixtureDir}${path.delimiter}${process.env.PATH ?? ""}`,
|
|
...(fixture ? {
|
|
// Provider selection prefers the installed dependency over PATH.
|
|
// Scope this override and its empty credential home to the test server.
|
|
PAPERCLIP_STOP_CODEX_COMMAND: path.join(fixtureDir, "codex"),
|
|
CODEX_HOME: path.join(fixtureDir, "codex-home"),
|
|
OPENAI_API_KEY: "composer-stop-fixture-key",
|
|
NODE_OPTIONS: `${server?.env?.NODE_OPTIONS ?? ""} --import=${path.resolve(import.meta.dirname, "fixtures/composer-stop-codex-loader.mjs")}`,
|
|
} : {}),
|
|
},
|
|
},
|
|
});
|