mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-08 00:54:38 +02:00
## Thinking Path > - Paperclip manages agent work in isolated execution workspaces. > - A workspace depends on a valid database seed before it can run. > - Deferred seed failures were hidden behind a successful provision status. > - The seed restore also had two possible owners for the embedded PostgreSQL process. > - That allowed the target database to stop while the restore was still running. > - This pull request makes seed failures visible and gives the seed process sole lifecycle ownership. > - The benefit is that workspace provisioning reports the real result and does not stop its own target database. ## Linked Issues or Issue Description Related: #11684 **What happened?** Initial worktree provisioning could report success before its deferred database seed completed. The seed restore could also reuse a target embedded PostgreSQL process with another shutdown owner. This could stop the target database during the restore. **Expected behavior** Workspace status must show a failed deferred seed as a failure. The seed restore must own the target embedded PostgreSQL process until restore, migration, and validation finish. **Steps to reproduce** 1. Provision a worktree with deferred database seeding. 2. Make the seed manifest end in a failed state while the command exits with code 0. 3. Observe that the provision status remains successful on `master`. 4. Start a seed restore against an already-running target embedded PostgreSQL process. 5. Observe that another lifecycle owner can stop the target during restore. **Paperclip version or commit** `51a843e135` **Deployment mode** Local dev with execution workspaces and embedded PostgreSQL. ## What Changed - Add a first-class `workspace_seed` operation for deferred database seeds. - Require terminal, verified seed evidence before the seed operation succeeds. - Surface the seed phase and failure metadata in workspace status and UI state. - Give the seed process exclusive lifecycle ownership of the target embedded PostgreSQL process. - Suppress imported embedded-Postgres exit hooks without removing existing host listeners. - Record a credential-safe shutdown diagnostic in failed seed manifests. ## Verification - The original deferred-seed commit passed 4 server tests, 24 workspace-status UI tests, shared/server/UI typechecks, and the UI token gate. - The original PostgreSQL-lifecycle commit passed 3 lifecycle tests, 3 ownership/diagnostic tests, 1 real embedded-Postgres seed integration, and the affected package typechecks. - No local tests were rerun after the clean cherry-pick because the operator requested the shortest landing path. - Review the automatic PR checks for the clean `origin/master` replay. ## Risks - A live target database now causes an early error instead of being reused. The error includes recovery guidance. - Workspace consumers must handle the new `workspace_seed` operation type. Shared types and UI state handling are updated in this pull request. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5, high-reasoning mode, with repository, shell, and GitHub tool use. The runtime does not expose a more specific deployment suffix or context-window value. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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>
78 lines
2.4 KiB
TypeScript
78 lines
2.4 KiB
TypeScript
import { EventEmitter } from "node:events";
|
|
import { describe, expect, it, vi } from "vitest";
|
|
import { loadWithoutEmbeddedPostgresExitHooks } from "./embedded-postgres-lifecycle.js";
|
|
|
|
describe("loadWithoutEmbeddedPostgresExitHooks", () => {
|
|
it("removes every eager exit hook from the real embedded-postgres import", async () => {
|
|
const eventNames = [
|
|
"exit",
|
|
"beforeExit",
|
|
"SIGHUP",
|
|
"SIGINT",
|
|
"SIGTERM",
|
|
"SIGBREAK",
|
|
"message",
|
|
];
|
|
const before = new Map(eventNames.map((eventName) => [
|
|
eventName,
|
|
process.rawListeners(eventName),
|
|
]));
|
|
const moduleName = "embedded-postgres";
|
|
|
|
await loadWithoutEmbeddedPostgresExitHooks(() => import(moduleName));
|
|
|
|
for (const eventName of eventNames) {
|
|
expect(process.rawListeners(eventName)).toEqual(before.get(eventName));
|
|
}
|
|
});
|
|
|
|
it("removes dependency exit hooks while preserving existing listeners", async () => {
|
|
const target = new EventEmitter();
|
|
const existingSignalListener = vi.fn();
|
|
const existingExitListener = vi.fn();
|
|
target.on("SIGTERM", existingSignalListener);
|
|
target.on("exit", existingExitListener);
|
|
|
|
const dependencyListener = vi.fn();
|
|
const loaded = await loadWithoutEmbeddedPostgresExitHooks(
|
|
async () => {
|
|
for (const eventName of [
|
|
"exit",
|
|
"beforeExit",
|
|
"SIGHUP",
|
|
"SIGINT",
|
|
"SIGTERM",
|
|
"SIGBREAK",
|
|
"message",
|
|
]) {
|
|
target.on(eventName, dependencyListener);
|
|
}
|
|
return { default: class EmbeddedPostgres {} };
|
|
},
|
|
target,
|
|
);
|
|
|
|
expect(loaded.default.name).toBe("EmbeddedPostgres");
|
|
expect(target.rawListeners("SIGTERM")).toEqual([existingSignalListener]);
|
|
expect(target.rawListeners("exit")).toEqual([existingExitListener]);
|
|
for (const eventName of ["beforeExit", "SIGHUP", "SIGINT", "SIGBREAK", "message"]) {
|
|
expect(target.rawListeners(eventName)).toEqual([]);
|
|
}
|
|
});
|
|
|
|
it("cleans up listeners even when the import fails", async () => {
|
|
const target = new EventEmitter();
|
|
const dependencyListener = vi.fn();
|
|
|
|
await expect(loadWithoutEmbeddedPostgresExitHooks(
|
|
async () => {
|
|
target.on("SIGTERM", dependencyListener);
|
|
throw new Error("import failed");
|
|
},
|
|
target,
|
|
)).rejects.toThrow("import failed");
|
|
|
|
expect(target.rawListeners("SIGTERM")).toEqual([]);
|
|
});
|
|
});
|