mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 11:01:23 +01:00
fix(server): preserve sessions across cosmetic sandbox updates (#19349)
## Thinking Path > - Paperclip manages agent work and saves provider sessions between turns. > - Session and workspace compatibility include the selected environment revision. > - The managed sandbox reconciler also updates names, descriptions, and stock metadata. > - Those cosmetic updates advanced the revision even when the execution configuration stayed the same. > - A later turn therefore discarded an otherwise compatible saved session. > - This pull request keeps that revision stable for cosmetic reconciliation and retains resets for real configuration changes. ## Linked Issues or Issue Description Related: #18943 and #18552. **What happened?** A managed sandbox rename or description update made a later task turn start a fresh provider session. The driver, provider, snapshot, and agent configuration could all remain unchanged. Both session and workspace freshness use the environment's `updatedAt` as its configuration revision. **Expected behavior** Cosmetic managed reconciliation preserves compatible task sessions. Actual sandbox configuration or availability changes retain the normal replacement checks. **Steps to reproduce** 1. Create a managed sandbox and save a task-session fingerprint for it. 2. Reconcile the same sandbox configuration with a new name, description, or managed metadata. 3. Evaluate the saved session against that environment. Before this fix, the decision resets the session and reports the environment category. **Paperclip version or commit** Reproduced on master `d3713d827f70ddba63a323e57c562384dbd648ba`. **Deployment mode** Managed sandbox server. The regression uses an isolated PostgreSQL database and the actual reconciler and session-freshness resolver. ## What Changed - Keep the existing revision for an active managed sandbox when its execution configuration stays equal. - Continue advancing the revision when configuration or availability changes. - Retain activity records and managed-stock binding updates for cosmetic reconciliation. - Add a regression that checks session reuse, audit history, and rejection after a real snapshot change. - Document the freshness contract. ## Verification - The new PostgreSQL regression failed before the production change: it reported `reset: true` and changed category `environment`. - All 53 environment-service cases pass after the fix. - All 190 session-freshness and assembled heartbeat session-preparation cases pass. - Full local typecheck passes. The full local Vitest run remains active, with its local build queued afterward. Initial typecheck setup failures were retained and resolved by repairing generated SDK links; no source workaround was added. - [Full current-head CI](https://github.com/paperclipai/paperclip/actions/runs/42114984839) passes, including the sharded test suites and build: 55 successful checks, 3 skipped, and 1 neutral. - Greptile and Ponytail both reviewed this commit at 5/5 with no findings. No review threads remain unresolved. - Deployed verification remains pending normal delivery. No pinned release or manual rollout is required. ## Risks - Cosmetic managed reconciliation keeps `environments.updatedAt` unchanged, so it does not reorder that environment by this timestamp. Activity records and managed-resource bindings still record the update. - The comparison uses canonical configuration hashes, so JSON property order cannot hide a real change or create false drift. - Existing fingerprint formats and saved session records remain compatible. Real configuration and availability changes still advance the revision. ## Model Used OpenAI GPT-6 through Codex desktop, with reasoning and terminal/browser tools. The host does not expose a more specific deployment suffix 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>
This commit is contained in:
1 parent
da3084a05d
commit
15249a689d
3 files changed
+45
-1
No files matched your search
@@ -896,6 +896,11 @@ remote configuration. An unchanged follow-up reuses that configuration without
|
||||
reporting strategy drift. Real strategy, source, mode and environment changes
|
||||
retain their existing replacement checks.
|
||||
|
||||
Managed sandbox reconciliation preserves the environment revision used by session
|
||||
and workspace freshness checks when only its name, description, or managed metadata
|
||||
changes. It still records the update in the activity log. Configuration or
|
||||
availability changes advance that revision and retain normal replacement checks.
|
||||
|
||||
Older remote Git workspaces can retain the temporary `project_primary`
|
||||
fingerprint. That fingerprint alone cannot distinguish the old preparation bug
|
||||
from a genuinely configured `project_primary` strategy; the remote workspace row
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { environmentService } from "../services/environments.ts";
|
||||
import { attachPaperclipSessionMetadataToSessionParams, buildEffectiveRunSessionConfigMetadata, resolveTaskSessionConfigFreshness } from "../services/heartbeat/workspaces.js";
|
||||
|
||||
const externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL;
|
||||
if (externalTestDatabaseUrl && !new URL(externalTestDatabaseUrl).pathname.endsWith("_test")) {
|
||||
@@ -1290,6 +1291,42 @@ describeEmbeddedPostgres("environmentService leases", () => {
|
||||
expect((await svc.ensureManagedSandboxEnvironment({ ...input, name: "Paperclip Sandbox" })).action).toBe("unchanged");
|
||||
});
|
||||
|
||||
it("preserves task sessions across cosmetic managed sandbox reconciliation", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const input = { companyId, name: "Managed computer", description: "Original description", provider: "daytona",
|
||||
config: { snapshot: "snapshot-a" }, platformFullyManaged: true };
|
||||
const created = (await svc.ensureManagedSandboxEnvironment(input)).environment;
|
||||
const revision = new Date("2026-01-01T00:00:00.000Z");
|
||||
await db.update(environments).set({ updatedAt: revision }).where(eq(environments.id, created.id));
|
||||
const metadata = (environment: typeof created) => buildEffectiveRunSessionConfigMetadata({
|
||||
adapterType: "paperclip_runner", effectiveAdapterConfig: { model: "model-a" },
|
||||
agentRuntimeConfig: {}, issueOverrides: null, workspaceConfig: null,
|
||||
environment: { selectedEnvironmentId: environment.id, selectedEnvironment: {
|
||||
id: environment.id, driver: environment.driver, config: environment.config,
|
||||
configRevisionAt: new Date(environment.updatedAt).toISOString(),
|
||||
} },
|
||||
environmentEnv: environment.envVars, projectEnv: null, routineEnv: null, runtimeSkills: [],
|
||||
});
|
||||
const before = await metadata({ ...created, updatedAt: revision });
|
||||
const taskSessionParams = attachPaperclipSessionMetadataToSessionParams({ sessionId: "saved-session" }, "model-a", before);
|
||||
const reconciled = await svc.ensureManagedSandboxEnvironment({ ...input, name: "Managed sandbox",
|
||||
description: "New description", extraMetadata: { platformLabel: "new label" } });
|
||||
expect(reconciled.action).toBe("updated");
|
||||
const freshness = resolveTaskSessionConfigFreshness({ hasTaskSession: true, configuredModel: "model-a",
|
||||
taskSessionParams, configMetadata: await metadata(reconciled.environment) });
|
||||
expect(freshness).toMatchObject({ reset: false, changedCategories: [] });
|
||||
expect(reconciled.environment.updatedAt).toEqual(revision);
|
||||
expect(await db.select().from(activityLog).where(and(eq(activityLog.companyId, companyId),
|
||||
eq(activityLog.action, "environment.managed_stock_updated")))).toHaveLength(1);
|
||||
const changed = await svc.ensureManagedSandboxEnvironment({ ...input, name: "Managed sandbox",
|
||||
description: "New description", extraMetadata: { platformLabel: "new label" }, config: { snapshot: "snapshot-b" } });
|
||||
expect(resolveTaskSessionConfigFreshness({ hasTaskSession: true, configuredModel: "model-a",
|
||||
taskSessionParams, configMetadata: await metadata(changed.environment) })).toMatchObject({
|
||||
reset: true, changedCategories: ["environment"],
|
||||
});
|
||||
expect(new Date(changed.environment.updatedAt).getTime()).toBeGreaterThan(revision.getTime());
|
||||
});
|
||||
|
||||
it("ensures and refreshes a managed sandbox environment for an arbitrary provider", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const createdResult = await svc.ensureManagedSandboxEnvironment({
|
||||
|
||||
@@ -758,7 +758,9 @@ export function environmentService(db: Db) {
|
||||
mergeManagedEnvironmentMetadata(row.metadata, desiredMetadata, keys),
|
||||
),
|
||||
status: "active",
|
||||
updatedAt: new Date(),
|
||||
updatedAt: row.status === "active" && stockHash(row.config) === stockHash(desiredConfig)
|
||||
? row.updatedAt
|
||||
: new Date(),
|
||||
})
|
||||
.where(eq(environments.id, row.id))
|
||||
.returning()
|
||||
|
||||
Reference in new issue
Block a user