mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(server): preserve terminal workspace cursors and retry deadlines (#17505)
## Thinking Path > - Paperclip manages agent work and its execution workspaces. > - Workspace recovery scans candidates and retains deadlines for unfinished work. > - Rounded database timestamps can prevent a scan from advancing. > - Separate clock reads can give one retry obligation different deadlines. > - Repeated inspection must preserve both scan progress and the saved retry time. > - This PR keeps full cursor precision and computes each retry deadline once. > - Recovery can finish its scan and wait until work is actually due. ## Linked Issues or Issue Description **What happened?** Terminal workspace recovery can repeatedly inspect the oldest skipped candidate when its timestamp contains microseconds. Eligible workspaces behind it never reach cleanup. Separately, nested-clone migration and isolated-repository rehome recovery use separate clock reads for their retry gate and advertised deadline. A later inspection can move the advertised deadline backward. **Expected behavior** Each bounded scan advances using the complete timestamp and ID. The retry gate and advertised deadline represent the same saved time, including when the clock advances between calls. **Steps to reproduce** For cursor starvation, place two skipped workspaces with microsecond timestamps before an eligible terminal workspace and use a one-row page. For deadline drift, inject a clock that advances one millisecond per read, run disabled migration or rehome recovery, then inspect it again before the retry time. Stacked on #17497. Related cursor fixes #17479 and #17482 have separate owners. This PR owns workspace recovery cursor and retry precision. ## What Changed - Preserve the database timestamp as text in the private timestamp/ID cursor. - Compute one retry time per migration or rehome outcome and reuse it in the existing maps. - Test the cursor with real PostgreSQL microsecond timestamps and the retry boundary with a deterministic advancing clock. - Support validated disposable external test databases in the affected fixtures, retaining embedded fallback. - Document the evidence in `doc/idle-terminal-workspace-pagination.md`. ## Verification Current test revision: `9d846c84e76c90893ab62a09548c11bbdb627771`, followed by docs-only head `a34dc50de3`, which received both 5/5 reviews and passing CI. This candidate normally integrates landed approval retention `553a004926`. Every runtime and acceptance-test source matches that tested/reviewed tree; only four independent Runner E2E accounting files and the ledger differ. Migration metadata is unchanged. Existing local checks are reused for their unchanged source; fresh final-head CI and reviews are required against master. This remains the terminal-precision duty. - **136 cases pass** in four complete suites: execution workspace service, terminal worker, isolated repository rehome and nested-clone migration. Server typecheck and production build pass with the current Runner vendor. - The preceding `f0f0de1f6f` run also passed all 136 on three newly created, unmigrated PostgreSQL databases. The suites themselves applied migrations. The current run reuses those clean fixtures after integrating the parent's approval wake repair; it is not presented as a fresh-database run. - The regressions retain real PostgreSQL microsecond timestamps and an advancing clock. Before repair, skipped rows starved eligible cleanup and early inspection moved a saved retry by one millisecond. Current assertions preserve exact progress/deadline behavior; no timeout or business policy is weakened. - Earlier external-fixture setup tests failed with missing company/restore-point schema. Their setup repair and red/green record remain in `doc/idle-terminal-workspace-pagination.md`. This PR adds no migration or new queue. - Earlier stack integration receipts and failures remain in the stack ledger and `doc/idle-approved-tool-recovery.md`; they are historical parent evidence, not this head's acceptance. No full local repository suite, deployed suspension, fleet cost or live provider qualification is claimed. Commands: server Vitest for `execution-workspaces-service`, `terminal-workspace-work`, `isolated-repository-rehome` and `nested-clone-migration`; server typecheck/build. Fresh exact-head CI and Greptile/Ponytail 5/5 remain required before merge in dependency order. ## Risks The cursor remains private query state. Retry intervals, eligibility, cleanup authority, and shutdown behavior are unchanged. No schema, queue, external alarm, or additional polling loop is introduced. The tests establish workspace recovery behavior, not host suspension or cost savings. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The exact serving model identifier and context window were not exposed. ## 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>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
553a004926
commit
d23dd19750
6 files changed
+224
-37
No files matched your search
@@ -0,0 +1,103 @@
|
||||
# Terminal workspace scan and retry deadline precision
|
||||
|
||||
Terminal workspace maintenance already uses the shared delivery coordinator and
|
||||
retains deadlines only for outstanding work. Its bounded scan must progress past
|
||||
ineligible rows so that later eligible work can finish and the worker can park.
|
||||
|
||||
The source query orders candidates by `(updated_at, id)`. PostgreSQL timestamps
|
||||
can contain microseconds. The former cursor used a JavaScript Date, which loses
|
||||
that precision. A skipped row could then compare greater than its own rounded
|
||||
cursor. Repeated full pages kept recovery active and prevented later cleanup.
|
||||
|
||||
The query now carries the database timestamp as text alongside each workspace.
|
||||
Only the private scan cursor uses that value. Workspace data, business deadlines,
|
||||
and the frozen rotation boundary keep their existing types and behavior. The
|
||||
cursor comparison uses the same timestamp/ID tuple as the ordering. The source
|
||||
service continues to own cleanup eligibility, authorization, concurrency fences,
|
||||
page limits, abort handling, failed-page recovery, and rotation under churn.
|
||||
No scheduler API, queue, migration, producer signal, or external alarm changes.
|
||||
|
||||
## Verification
|
||||
|
||||
The existing full-page starvation regression now gives its skipped rows actual
|
||||
PostgreSQL microsecond timestamps. Before the repair, the eligible workspace was
|
||||
never archived. After the repair, the full workspace service and terminal worker
|
||||
selection passes all 87 tests, including bounded rotation under continuous churn,
|
||||
changed rows behind the cursor, failed-page recovery, quiet workers, and shutdown.
|
||||
The service suite accepts the repository's validated disposable external
|
||||
PostgreSQL URL. Setup applies pending migrations before opening the test client,
|
||||
then clears fixture tables before the first test as well as after each test. This
|
||||
supports a fresh database and reuse after an interrupted test run. The database
|
||||
connection closes during teardown. Server typechecking passes.
|
||||
|
||||
This fixes recovery progress. It does not establish provider suspension or change
|
||||
which active or unresolved resources prevent instance sleep.
|
||||
|
||||
Workspace-wide typechecking and the production build also pass. Full sharded CI
|
||||
and fresh review remain required before this PR is ready.
|
||||
|
||||
Review follow-up reproduced the missing-schema failure against a newly created,
|
||||
unmigrated disposable database. The setup repair migrates that same database
|
||||
inside the suite; the full workspace-service selection then runs without manual
|
||||
schema preparation. The production cursor implementation is unchanged.
|
||||
|
||||
## Preserve one retry deadline through early inspection
|
||||
|
||||
Nested-clone migration and isolated-source rehoming kept a retry guard and a
|
||||
separate advertised scheduler deadline. Each value called the clock separately.
|
||||
When the clock advanced between those calls, an early inspection copied the
|
||||
earlier guard back into the advertised deadline. CI observed a one-millisecond
|
||||
backward change in an otherwise unchanged retry.
|
||||
|
||||
Each path now computes one retry timestamp and stores that value in all of its
|
||||
retry and scheduling state. The cadence, cleanup eligibility, quiet conditions,
|
||||
rotation, and wake contract do not change. No new timer, queue, or table is added.
|
||||
|
||||
The existing nested-clone regression and the isolated-rehome dry-run case now
|
||||
use a clock that advances one millisecond per read. Both failed before the fix
|
||||
with the same one-millisecond drift. Both exact-deadline assertions remain. The
|
||||
complete migration suites passed all 49 cases afterward, using real PostgreSQL
|
||||
and Git, including preserved files/history and interrupted migration recovery.
|
||||
The complete workspace-service and terminal-worker suites passed another 87
|
||||
cases; server typechecking passed. The two focused pre-fix failures used a name
|
||||
filter intentionally; the reported final 136 cases are four complete suites
|
||||
without skips. Validated external test database hooks keep each migration suite
|
||||
on its own disposable database and retain the existing embedded fixture fallback.
|
||||
|
||||
These are local correctness and scheduling results, not deployed host-sleep or
|
||||
external-wake proof. Remote acceptance must be checked at the published head.
|
||||
|
||||
## External fixture migrations (October 26 review repair)
|
||||
|
||||
Review found that the two migration suites opened external test databases without
|
||||
applying migrations. The previous verification used prepared databases, which
|
||||
hid this setup gap. Both selected retry cases failed against new empty disposable
|
||||
databases with missing `companies` and restore-point tables. Setup now applies
|
||||
pending migrations before creating its test client, matching the workspace
|
||||
service fixture. The embedded path is unchanged.
|
||||
|
||||
All 49 cases in the two complete suites then passed against those same databases,
|
||||
including Git/filesystem migration, recovery, and exact retry-deadline checks.
|
||||
The fixture itself prepared the databases. This repair changes no production
|
||||
recovery code. Evidence: `/tmp/sleep-workspace-fresh-red.log` and
|
||||
`/tmp/sleep-workspace-fresh-green.log`.
|
||||
|
||||
|
||||
## Current parent verification
|
||||
|
||||
Approval-retention parent `3efda33812` is integrated with normal merges. All
|
||||
workspace precision, retry and fixture implementations remain unchanged from the
|
||||
preceding candidate. All 136 cases in the complete workspace-service, terminal
|
||||
worker, isolated-rehome and nested-clone suites pass at `9d846c84e7`; server types
|
||||
and production build pass with the current Runner vendor.
|
||||
|
||||
The preceding `f0f0de1f6f` run also passed all 136 cases with three newly created,
|
||||
unmigrated external databases: the suites themselves applied the current
|
||||
migrations. The `9d846c84e7` run reused those clean fixtures after the parent's
|
||||
approval wake repair. This retains actual proof of fixture self-preparation
|
||||
without mislabeling the reused run as a fresh-database test. No new migration,
|
||||
queue, wake contract or workspace policy change is added by this PR.
|
||||
|
||||
Earlier microsecond starvation, one-millisecond retry drift and missing-schema
|
||||
failures remain documented above. Fresh exact-head CI and both 5/5 reviews remain
|
||||
required in stack order; the parent is not claimed merged.
|
||||
@@ -15,7 +15,7 @@ Do not mark an item done from a timer removal or a green check alone.
|
||||
| 01 | Custom-image setup expiry | Session writes, existing expiry, startup/retry/drain | [#17443](https://github.com/paperclipai/paperclip/pull/17443), merged after CI and passing reviews |
|
||||
| 02 | Remaining chat maintenance | Provider runtime discovery; voice reconciliation; milestones; wake notices; interaction continuations; terminal confirmations; GitHub recovery; Teams maintenance, separately by owner | Pending |
|
||||
| 03 | Run and task recovery | Orphan runs and active leases; shutdown workspace settlement; scheduled retries; queued runs; stranded assignments; resolved dependencies; task watchdogs; silent-run monitoring; stale locks | Pending |
|
||||
| 04 | Decision retention and notifications | Retention deadlines and notification delivery are separate reasons | Approval retention implemented in slice 14; final review/CI pending; other source retention pending; archive delivery [#17475](https://github.com/paperclipai/paperclip/pull/17475) merged after exact-head 5/5 reviews and passing current CI |
|
||||
| 04 | Decision retention and notifications | Retention deadlines and notification delivery are separate reasons | Approval retention [#17497](https://github.com/paperclipai/paperclip/pull/17497) and archive delivery [#17475](https://github.com/paperclipai/paperclip/pull/17475) merged; other source retention pending |
|
||||
| 05 | Decision expiry and interrupted effects | Expiry, effect recovery, continuation recovery | Continuations [#17478](https://github.com/paperclipai/paperclip/pull/17478) merged; expiry [#17479](https://github.com/paperclipai/paperclip/pull/17479) merged; effects [#17482](https://github.com/paperclipai/paperclip/pull/17482) merged; targets [#17486](https://github.com/paperclipai/paperclip/pull/17486) merged |
|
||||
|
||||
|
||||
@@ -772,6 +772,7 @@ files. All runtime and acceptance test sources are byte-identical to its tested
|
||||
remain required after retargeting to master.
|
||||
|
||||
|
||||
|
||||
## Slice 13: Grok routine dispatch
|
||||
|
||||
The one-second idle scan is replaced with a shared-coordinator worker. The existing
|
||||
@@ -809,6 +810,7 @@ its tested/reviewed `679f523d78` tree; the 273-case, type/build and migration
|
||||
receipts retain their original revisions. Migration 0391 and all parent journal
|
||||
entries are unchanged. Fresh final-head CI and reviews required against master.
|
||||
|
||||
|
||||
## Next owner audit: decision retention
|
||||
|
||||
Slice 13 is [#17490](https://github.com/paperclipai/paperclip/pull/17490), head
|
||||
@@ -1026,3 +1028,58 @@ accounting files. Its entire runtime and acceptance sources match the tested
|
||||
and reviewed `3efda33812` tree (315 cases, server types/build). Migration 0392
|
||||
and the parent journal prefix are unchanged. Fresh final-head CI/reviews are
|
||||
required against master. The abandoned-import-spool inventory duty remains.
|
||||
|
||||
Latest slice-14 head is `a29ce265b0c00678ada3f29f533d79f80fe25654`.
|
||||
The precision finding was replied to (4368650634) and its thread resolved.
|
||||
Fresh Greptile review was requested (6390004743). CI run **40136197148** is
|
||||
confirmed in progress on this head; commit review run 40136193928 succeeded.
|
||||
The previous-head CI run 40135489716 succeeded. Do not treat that as evidence
|
||||
for the new head. No local test/build process remains active.
|
||||
|
||||
Next actions: finish fresh #17497 CI/review; reproduce and repair precision
|
||||
paging in the earlier TTL and effect-recovery owners, propagating parent changes
|
||||
through later PRs; qualify the terminal-workspace cursor; then implement the
|
||||
join-request retention audit on this branch. Every original inventory item and
|
||||
remaining retention source remains required. No merge is authorized.
|
||||
|
||||
## Decision and workspace precision follow-up
|
||||
|
||||
Expiry #17479 now preserves the PostgreSQL timestamp in its cursor, head
|
||||
`e89c682ba5`. Its regression failed before repair; all 15 expiry cases and server
|
||||
typechecking pass. Fresh CI is green and Greptile is 5/5 at that head.
|
||||
Effect recovery #17482 has the same verified repair at `cebcd9c427`: its invalid
|
||||
legacy claim regression failed before repair, then all 18 cases and typechecking
|
||||
passed. Both changes have been merged through #17486, #17490, and #17497.
|
||||
The combined 33 expiry/effect cases pass on the latest stack. Fresh dependent
|
||||
CI/review is running. Superseded runs on the Grok/retention heads were cancelled
|
||||
when both base and head updated; replacement workflows exist. Do not rerun or
|
||||
claim those heads green until their current runs finish.
|
||||
|
||||
Approval retention previously passed all checks and fresh 5/5 at `a29ce265b0`.
|
||||
Its new head is `2c5babcabc`, with only parent paging repairs added. Join retention
|
||||
still has no implementation; its branch now includes the current parents.
|
||||
|
||||
The terminal-workspace audit also reproduced starvation using real microsecond
|
||||
source timestamps. The focused branch `codex/sleep-terminal-workspace-precision`
|
||||
preserves cursor precision in the owning query. All 87 workspace service/worker
|
||||
cases and server typechecking pass. See
|
||||
[workspace pagination](../idle-terminal-workspace-pagination.md). Publication and
|
||||
fresh CI/review are next. Join retention remains the next new migration duty.
|
||||
|
||||
|
||||
Terminal workspace precision follow-up #17505 normally integrates approval wake
|
||||
parent `3efda33812`. All 136 cases across four complete workspace/worker/rehome/
|
||||
nested-clone suites, server types and build pass at `9d846c84e7`. The preceding
|
||||
`f0f0de1f6f` run passed the same cases on three freshly created unmigrated fixtures;
|
||||
the suites prepared their own schemas. Current reused-fixture evidence and old
|
||||
starvation/clock/setup failures are distinguished in its owning audit. No new
|
||||
schema or wake mechanism. Fresh head CI and reviews required; parent unmerged.
|
||||
|
||||
|
||||
Slice 14 merged `a26d865c99` → `553a004926` after exact-head Greptile/Ponytail
|
||||
5/5 and all 58 checks passed or were neutral/skipped. Terminal precision #17505
|
||||
normally integrates that landed dependency and four independent Runner E2E
|
||||
accounting files. Its entire runtime and acceptance sources match the tested/
|
||||
reviewed `a34dc50de3` tree (136 cases, server types/build); migration metadata is
|
||||
unchanged. No local checks were repeated on unchanged source. Fresh final-head
|
||||
CI/reviews are required against master; all remaining checklist duties persist.
|
||||
@@ -1,3 +1,4 @@
|
||||
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
|
||||
import * as localServiceSupervisor from "../services/local-service-supervisor.js";
|
||||
import { EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db";
|
||||
import { execFile } from "node:child_process";
|
||||
@@ -11,6 +12,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest
|
||||
import { eq, inArray, sql } from "drizzle-orm";
|
||||
import {
|
||||
activityLog,
|
||||
applyPendingMigrations,
|
||||
agents,
|
||||
companies,
|
||||
createDb,
|
||||
@@ -160,7 +162,8 @@ describe("execution workspace config helpers", () => {
|
||||
});
|
||||
});
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const externalUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_EXECUTION_WORKSPACES_TEST_DATABASE_URL");
|
||||
const embeddedPostgresSupport = externalUrl ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
@@ -258,8 +261,10 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
}>();
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-execution-workspaces-service-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
if (externalUrl) await applyPendingMigrations(externalUrl);
|
||||
else tempDb = await startEmbeddedPostgresTestDatabase("paperclip-execution-workspaces-service-");
|
||||
db = createDb(externalUrl ?? tempDb!.connectionString);
|
||||
await resetDatabase();
|
||||
svc = executionWorkspaceService(db, {
|
||||
resolvePullRequestDetails: vi.fn(async (companyId, reference) =>
|
||||
pullRequestDetailsByKey.get(`${companyId}:${reference.number}`)
|
||||
@@ -272,7 +277,7 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
});
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
|
||||
afterEach(async () => {
|
||||
async function resetDatabase() {
|
||||
await db.delete(workspaceRuntimeServices);
|
||||
await db.delete(activityLog);
|
||||
await db.delete(issueRecoveryActions);
|
||||
@@ -286,6 +291,10 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await resetDatabase();
|
||||
pullRequestDetailsByKey.clear();
|
||||
|
||||
for (const dir of tempDirs) {
|
||||
@@ -295,6 +304,7 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await db?.$client.end({ timeout: 1 });
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
@@ -1124,6 +1134,8 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
.set({ sourceIssueId: openIssueId, updatedAt: new Date(Date.UTC(2020, 0, index + 1)) })
|
||||
.where(eq(executionWorkspaces.id, workspaceId));
|
||||
}
|
||||
await db.execute(sql`update execution_workspaces set updated_at = updated_at + interval '456 microseconds'
|
||||
where id in (${sql.join(skippedWorkspaceIds.map(id => sql`${id}::uuid`), sql`, `)})`);
|
||||
// The eligible workspace is newest, so it sorts after the whole skipped page.
|
||||
const eligible = await seedAncestryTerminalWorkspace({ updatedAt: new Date(Date.UTC(2020, 0, 9)) });
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import { randomUUID } from "node:crypto";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { agents, companies, createDb, executionWorkspaceRestorePoints, executionWorkspaces, heartbeatRuns, issues, projects } from "@paperclipai/db";
|
||||
import { agents, applyPendingMigrations, companies, createDb, executionWorkspaceRestorePoints, executionWorkspaces, heartbeatRuns, issues, projects } from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
import { resolveManagedProjectWorkspaceDir } from "../home-paths.js";
|
||||
import { materializeManagedProjectWorkspace, ensureManagedRepositoriesIgnored } from "../services/managed-repository-checkout.js";
|
||||
@@ -25,17 +25,21 @@ const git = (cwd: string, ...args: string[]) => execFileSync("git", args, { cwd,
|
||||
const exists = (target: string) => fs.lstat(target).then(() => true, () => false);
|
||||
const DAY_MS = 24 * 60 * 60 * 1000;
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
|
||||
|
||||
const external = resolveExternalTestDatabaseUrl("PAPERCLIP_ISOLATED_REHOME_TEST_DATABASE_URL");
|
||||
const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = support.supported ? describe : describe.skip;
|
||||
describeEmbeddedPostgres("isolated repository source re-home", () => {
|
||||
let db: ReturnType<typeof createDb>;
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
||||
let root: string;
|
||||
const previousHome = process.env.PAPERCLIP_HOME;
|
||||
const previousInstance = process.env.PAPERCLIP_INSTANCE_ID;
|
||||
beforeAll(async () => {
|
||||
database = await startEmbeddedPostgresTestDatabase("isolated-repository-rehome-");
|
||||
db = createDb(database.connectionString);
|
||||
if (external) await applyPendingMigrations(external);
|
||||
else database = await startEmbeddedPostgresTestDatabase("isolated-repository-rehome-");
|
||||
db = createDb(external ?? database!.connectionString);
|
||||
root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-isolated-rehome-"));
|
||||
process.env.PAPERCLIP_HOME = path.join(root, "home");
|
||||
process.env.PAPERCLIP_INSTANCE_ID = "default";
|
||||
@@ -50,7 +54,7 @@ describeEmbeddedPostgres("isolated repository source re-home", () => {
|
||||
vi.unstubAllEnvs();
|
||||
if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome;
|
||||
if (previousInstance === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; else process.env.PAPERCLIP_INSTANCE_ID = previousInstance;
|
||||
await db.$client.end({ timeout: 1 }); await database.cleanup(); await fs.rm(root, { recursive: true, force: true });
|
||||
await db.$client.end({ timeout: 1 }); await database?.cleanup(); await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function fixture(options: { sharedBase?: boolean; issueStatus?: "in_progress" | "done" } = {}) {
|
||||
@@ -409,11 +413,16 @@ describeEmbeddedPostgres("isolated repository source re-home", () => {
|
||||
await executionWorkspaceService(db, { gitCacheExpirationEnabled: true }).sweepTerminalWorkspaces();
|
||||
await executionWorkspaceService(db, { isolatedSourceRehomeEnabled: true }).sweepTerminalWorkspaces();
|
||||
await expectUntouched(f, before);
|
||||
const dryRun = executionWorkspaceService(db, { gitCacheExpirationEnabled: true, isolatedSourceRehomeEnabled: true, isolatedSourceRehomeDryRun: true });
|
||||
let tick = Date.now();
|
||||
const dryRun = executionWorkspaceService(db, { gitCacheExpirationEnabled: true, isolatedSourceRehomeEnabled: true,
|
||||
isolatedSourceRehomeDryRun: true, now: () => new Date(tick++) });
|
||||
await dryRun.sweepTerminalWorkspaces();
|
||||
await expectUntouched(f, before);
|
||||
expect(await exists(f.clone)).toBe(true);
|
||||
expect(await dryRun.nextTerminalWorkspaceSweepAt()).toBeGreaterThan(Date.now());
|
||||
const retryAt = await dryRun.nextTerminalWorkspaceSweepAt();
|
||||
expect(retryAt).toBeGreaterThan(Date.now());
|
||||
await dryRun.sweepTerminalWorkspaces();
|
||||
expect(await dryRun.nextTerminalWorkspaceSweepAt()).toBe(retryAt);
|
||||
expect(await rehomeIsolatedRepositoryWorkspace(db, await f.row(), { dryRun: true })).toMatchObject({ outcome: "dry_run", rehomed: false, to: f.target });
|
||||
await expectUntouched(f, before);
|
||||
});
|
||||
|
||||
@@ -6,7 +6,7 @@ import { randomUUID } from "node:crypto";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { agents, companies, createDb, executionWorkspaceRestorePoints, executionWorkspaces, heartbeatRuns, issues, projects, projectWorkspaces } from "@paperclipai/db";
|
||||
import { agents, applyPendingMigrations, companies, createDb, executionWorkspaceRestorePoints, executionWorkspaces, heartbeatRuns, issues, projects, projectWorkspaces } from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
import { materializeManagedProjectWorkspace } from "../services/managed-repository-checkout.js";
|
||||
import { materializeIsolatedRemoteWorkspaceSource } from "../services/isolated-remote-workspace-source.js";
|
||||
@@ -44,17 +44,21 @@ describe("nested clone migration batches", () => {
|
||||
});
|
||||
});
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
|
||||
|
||||
const external = resolveExternalTestDatabaseUrl("PAPERCLIP_NESTED_CLONE_TEST_DATABASE_URL");
|
||||
const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = support.supported ? describe : describe.skip;
|
||||
describeEmbeddedPostgres("nested repository clone migration", () => {
|
||||
let db: ReturnType<typeof createDb>;
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
||||
let root: string;
|
||||
const previousHome = process.env.PAPERCLIP_HOME;
|
||||
const previousInstance = process.env.PAPERCLIP_INSTANCE_ID;
|
||||
beforeAll(async () => {
|
||||
database = await startEmbeddedPostgresTestDatabase("nested-clone-migration-");
|
||||
db = createDb(database.connectionString);
|
||||
if (external) await applyPendingMigrations(external);
|
||||
else database = await startEmbeddedPostgresTestDatabase("nested-clone-migration-");
|
||||
db = createDb(external ?? database!.connectionString);
|
||||
root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-nested-migration-"));
|
||||
process.env.PAPERCLIP_HOME = path.join(root, "home");
|
||||
process.env.PAPERCLIP_INSTANCE_ID = "default";
|
||||
@@ -68,7 +72,7 @@ describeEmbeddedPostgres("nested repository clone migration", () => {
|
||||
vi.unstubAllEnvs();
|
||||
if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome;
|
||||
if (previousInstance === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; else process.env.PAPERCLIP_INSTANCE_ID = previousInstance;
|
||||
await db.$client.end({ timeout: 1 }); await database.cleanup(); await fs.rm(root, { recursive: true, force: true });
|
||||
await db.$client.end({ timeout: 1 }); await database?.cleanup(); await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function repository(location: string, name: string, files: Record<string, string>) {
|
||||
@@ -504,7 +508,10 @@ describeEmbeddedPostgres("nested repository clone migration", () => {
|
||||
const agentId = randomUUID();
|
||||
await db.insert(agents).values({ id: agentId, companyId: f.companyId, name: "Fixture", role: "engineer", adapterType: "process" });
|
||||
const [run] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId, issueId: f.issueId, status: "running" }).returning();
|
||||
const busy = executionWorkspaceService(db, { nestedCloneMigrationEnabled: true });
|
||||
// Make a second clock read observably different without changing the
|
||||
// scheduler's saved business deadline during an early inspection.
|
||||
let tick = Date.now();
|
||||
const busy = executionWorkspaceService(db, { nestedCloneMigrationEnabled: true, now: () => new Date(tick++) });
|
||||
await busy.sweepTerminalWorkspaces();
|
||||
await expectUntouched(f, before);
|
||||
const retryAt = await busy.nextTerminalWorkspaceSweepAt();
|
||||
|
||||
@@ -45,7 +45,7 @@ import { createHash } from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { isDeepStrictEqual, promisify } from "node:util";
|
||||
import { and, asc, desc, eq, gt, inArray, isNull, lte, ne, or, sql } from "drizzle-orm";
|
||||
import { and, asc, desc, eq, inArray, isNull, lte, ne, or, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
executionWorkspaces,
|
||||
@@ -1757,7 +1757,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
// required: without it the query re-selects the oldest ineligible rows every
|
||||
// sweep and starves eligible rows behind them. The cursor resets to the start
|
||||
// when a sweep reaches the end of the candidate set.
|
||||
let terminalSweepCursor: { updatedAt: Date; id: string } | null = null;
|
||||
let terminalSweepCursor: { updatedAt: string; id: string } | null = null;
|
||||
const terminalInspections = new Map<string, { revision: string; issueIds: Set<string>; nextAt: number }>();
|
||||
const terminalWorkspaceDeadlines = new Map<string, number>();
|
||||
const isolatedRehomeDeadlines = new Map<string, number>();
|
||||
@@ -3399,17 +3399,11 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
? lte(executionWorkspaces.updatedAt, boundary)
|
||||
: undefined;
|
||||
const cursorFilter = cursor
|
||||
? or(
|
||||
gt(executionWorkspaces.updatedAt, cursor.updatedAt),
|
||||
and(
|
||||
eq(executionWorkspaces.updatedAt, cursor.updatedAt),
|
||||
gt(executionWorkspaces.id, cursor.id),
|
||||
),
|
||||
)
|
||||
? sql`(${executionWorkspaces.updatedAt}, ${executionWorkspaces.id}) > (${cursor.updatedAt}::timestamptz, ${cursor.id}::uuid)`
|
||||
: undefined;
|
||||
const scanFilter = and(baseCandidateFilter, boundaryFilter, cursorFilter);
|
||||
const candidates = await db
|
||||
.select()
|
||||
.select({ workspace: executionWorkspaces, cursorAt: sql<string>`${executionWorkspaces.updatedAt}::text` })
|
||||
.from(executionWorkspaces)
|
||||
.where(scanFilter)
|
||||
.orderBy(asc(executionWorkspaces.updatedAt), asc(executionWorkspaces.id))
|
||||
@@ -3422,7 +3416,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
terminalSweepBoundary = null;
|
||||
} else {
|
||||
const lastCandidate = candidates[candidates.length - 1]!;
|
||||
terminalSweepCursor = { updatedAt: lastCandidate.updatedAt, id: lastCandidate.id };
|
||||
terminalSweepCursor = { updatedAt: lastCandidate.cursorAt, id: lastCandidate.workspace.id };
|
||||
}
|
||||
const result = {
|
||||
checked: candidates.length,
|
||||
@@ -3470,7 +3464,8 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
"Isolated task directory retention inspected");
|
||||
};
|
||||
|
||||
for (let workspace of candidates) {
|
||||
for (const candidate of candidates) {
|
||||
let workspace = candidate.workspace;
|
||||
signal?.throwIfAborted();
|
||||
// Trash deletion is part of compaction, so a sweep that skips compaction
|
||||
// also leaves leftover trash for a later sweep.
|
||||
@@ -3488,8 +3483,9 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
const [after] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, workspace.id));
|
||||
if (after && (isIsolatedRepositoryRehomeCandidate(after) || hasPendingIsolatedRepositoryRehomeCleanup(after))) {
|
||||
if (isolatedRehomeNotBefore.size >= 4096) isolatedRehomeNotBefore.delete(isolatedRehomeNotBefore.keys().next().value!);
|
||||
isolatedRehomeNotBefore.set(workspace.id, now().getTime() + ISOLATED_REHOME_RETRY_MS);
|
||||
isolatedRehomeDeadlines.set(workspace.id, now().getTime() + ISOLATED_REHOME_RETRY_MS);
|
||||
const retryAt = now().getTime() + ISOLATED_REHOME_RETRY_MS;
|
||||
isolatedRehomeNotBefore.set(workspace.id, retryAt);
|
||||
isolatedRehomeDeadlines.set(workspace.id, retryAt);
|
||||
} else {
|
||||
isolatedRehomeNotBefore.delete(workspace.id);
|
||||
isolatedRehomeDeadlines.delete(workspace.id);
|
||||
@@ -3526,14 +3522,17 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
||||
}
|
||||
const [after] = names.length > 0 ? await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, workspace.id)) : [workspace];
|
||||
if (!dryRun && after && hasPendingNestedCloneMigration(after)) retry = true;
|
||||
// The retry gate and advertised deadline describe one obligation.
|
||||
// An early inspection must not move it by reading the clock again.
|
||||
const retryAt = now().getTime() + ISOLATED_REHOME_RETRY_MS;
|
||||
if (remaining) {
|
||||
if (nestedCloneMigrationNotBefore.size >= 4096) nestedCloneMigrationNotBefore.delete(nestedCloneMigrationNotBefore.keys().next().value!);
|
||||
nestedCloneMigrationNotBefore.set(workspace.id, now().getTime() + ISOLATED_REHOME_RETRY_MS);
|
||||
nestedCloneMigrationNotBefore.set(workspace.id, retryAt);
|
||||
} else nestedCloneMigrationNotBefore.delete(workspace.id);
|
||||
if (retry) {
|
||||
if (nestedCloneMigrationRetryAt.size >= 4096) nestedCloneMigrationRetryAt.delete(nestedCloneMigrationRetryAt.keys().next().value!);
|
||||
nestedCloneMigrationRetryAt.set(workspace.id, now().getTime() + ISOLATED_REHOME_RETRY_MS);
|
||||
nestedCloneMigrationDeadlines.set(workspace.id, now().getTime() + ISOLATED_REHOME_RETRY_MS);
|
||||
nestedCloneMigrationRetryAt.set(workspace.id, retryAt);
|
||||
nestedCloneMigrationDeadlines.set(workspace.id, retryAt);
|
||||
} else {
|
||||
nestedCloneMigrationRetryAt.delete(workspace.id);
|
||||
nestedCloneMigrationDeadlines.delete(workspace.id);
|
||||
|
||||
Reference in new issue
Block a user