mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-13 23:39:21 +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>
42 lines
2.1 KiB
JavaScript
42 lines
2.1 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { spawnSync } from "node:child_process";
|
|
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
import { fileURLToPath } from "node:url";
|
|
import { test } from "node:test";
|
|
|
|
const loader = fileURLToPath(new URL("./composer-stop-codex-loader.mjs", import.meta.url));
|
|
const resolver = new URL("../../../packages/paperclip-runner/src/drivers/codex/codex-command.ts", import.meta.url).href;
|
|
function launch(env, source = "") {
|
|
return spawnSync(process.execPath, ["--import", loader, "--input-type=module", "-e", source], {
|
|
encoding: "utf8", env: { ...process.env, NODE_ENV: "test", PAPERCLIP_STOP_CODEX_COMMAND: "", ...env },
|
|
});
|
|
}
|
|
|
|
test("pins the fixture even when a bundled Codex dependency is installed", () => {
|
|
const dir = mkdtempSync(join(tmpdir(), "composer-stop-loader-"));
|
|
const command = join(dir, "codex");
|
|
try {
|
|
writeFileSync(command, "#!/bin/sh\nprintf 'composer-stop-fixture'\n", { mode: 0o755 });
|
|
const result = launch({ PAPERCLIP_STOP_CODEX_COMMAND: command }, `
|
|
import { resolveCodexCommand, resolvePinnedCodexCommand } from ${JSON.stringify(resolver)};
|
|
import { execFileSync } from "node:child_process";
|
|
import assert from "node:assert/strict";
|
|
assert.equal(resolveCodexCommand(), ${JSON.stringify(command)});
|
|
assert.equal(resolvePinnedCodexCommand(), ${JSON.stringify(command)});
|
|
process.stdout.write(execFileSync(resolveCodexCommand(), ["--version"]));
|
|
`);
|
|
assert.equal(result.status, 0, result.stderr);
|
|
assert.equal(result.stdout, "composer-stop-fixture");
|
|
} finally { rmSync(dir, { recursive: true, force: true }); }
|
|
});
|
|
|
|
for (const env of [{}, { PAPERCLIP_STOP_CODEX_COMMAND: "codex" }, { NODE_ENV: "production", PAPERCLIP_STOP_CODEX_COMMAND: process.execPath }]) {
|
|
test(`rejects a missing or unsafe fixture configuration: ${JSON.stringify(env)}`, () => {
|
|
const result = launch(env);
|
|
assert.notEqual(result.status, 0);
|
|
assert.match(result.stderr, /Composer Stop requires an absolute fixture command/);
|
|
});
|
|
}
|