mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 16:51:28 +02:00
fix: expire unused bases with generated repository ignore metadata (#16695)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Finished tasks can release Git caches that Paperclip can restore
later.
> - Nested repository preparation adds an ignore entry to the base
repository.
> - Base cleanup currently treats this generated entry as custom
metadata.
> - This pull request accepts the exact generated entry and keeps other
metadata protected.
> - Unused private bases can then expire with their workspaces and
return on demand.
## Linked Issues or Issue Description
Refs #16691. Related #15404 and #14443 change repository preparation or
sync, not base expiration.
**What happened?**
A finished workspace with a nested project repository expires, but its
unused private base remains with `custom_git_metadata`. Normal
preparation added `/.paperclip-repositories/` to the stock Git exclude
file.
**Expected behavior**
Cleanup removes a verified, unused base with this generated entry.
Custom metadata still prevents base deletion.
**Steps to reproduce**
Create an isolated workspace with a nested repository through normal
preparation. Finish the task, wait for the retention period, and run
expiration. The workspace disappears but its private base remains.
**Paperclip version or commit**
790b729086.
## What Changed
- Accept stock `info/exclude` bytes or stock bytes plus the exact single
entry produced by Paperclip.
- Verify base deletion and restoration of both repository commits and
settings after upstream advances.
- Preserve custom patterns, comments, extra bytes, duplicate entries,
and symlinks.
## Verification
- Captured a failing lifecycle baseline: the workspace expired but the
base remained with `custom_git_metadata`.
- Both new real Git/PostgreSQL lifecycle cases pass. Restoration returns
main and nested commit A after upstream moves to B, preserves settings
permissions, and permits a subsequent commit.
- Six focused metadata controls pass. The retained baseline fixture
passes the storage check without edits to its files.
- Server typecheck passes. Full local build passes. Required CI is
tracked on this PR.
- Independent review found no actionable findings. Staging reclamation
and product reopening remain deployment acceptance checks.
## Risks
This expands a deletion allowlist. Matching is byte-exact and limited to
the generated suffix. Other ownership, authenticated fetch, consumer,
file, and Git history checks remain required. No schema or retention
changes.
## Model Used
OpenAI Codex (GPT-6), with code execution and independent agent review.
## 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
- [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
2befcd9a0a
commit
1678aae862
3 files changed
+98
-5
No files matched your search
@@ -0,0 +1,54 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { assertReproducibleBaseGitStorage } from "../services/git-workspace-restoration.js";
|
||||
import { ensureManagedRepositoriesIgnored } from "../services/managed-repository-checkout.js";
|
||||
|
||||
describe("reconstructable base Git metadata", () => {
|
||||
let root: string;
|
||||
let checkout: string;
|
||||
let exclude: string;
|
||||
beforeEach(async () => {
|
||||
root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-base-storage-"));
|
||||
const source = path.join(root, "source");
|
||||
checkout = path.join(root, "checkout");
|
||||
await fs.mkdir(source);
|
||||
const git = (cwd: string, ...args: string[]) => execFileSync("git", ["-C", cwd, "-c", "user.name=Fixture", "-c", "user.email=fixture@example.test",
|
||||
"-c", "commit.gpgsign=false", ...args], { stdio: "pipe", env: { ...process.env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: os.devNull } });
|
||||
git(source, "init", "--initial-branch=main");
|
||||
await fs.writeFile(path.join(source, "marker.txt"), "fixture\n");
|
||||
git(source, "add", "."); git(source, "commit", "-m", "Fixture");
|
||||
git(root, "clone", source, checkout);
|
||||
exclude = path.join(checkout, ".git", "info", "exclude");
|
||||
await ensureManagedRepositoriesIgnored(checkout);
|
||||
});
|
||||
afterEach(async () => { await fs.rm(root, { recursive: true, force: true }); });
|
||||
|
||||
it("accepts the ignore entry produced by normal repository preparation", async () => {
|
||||
await expect(assertReproducibleBaseGitStorage(checkout)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["custom pattern", Buffer.from("private-output/\n")],
|
||||
["custom comment", Buffer.from("# Retain my local metadata\n")],
|
||||
["nonstandard bytes", Buffer.from([0xff, 0x00])],
|
||||
["duplicate generated entry", Buffer.from("\n/.paperclip-repositories/\n")],
|
||||
])("preserves a %s beyond the generated ignore entry", async (_name, bytes) => {
|
||||
await fs.appendFile(exclude, bytes);
|
||||
const original = await fs.readFile(exclude);
|
||||
await expect(assertReproducibleBaseGitStorage(checkout)).rejects.toThrow("custom_git_metadata");
|
||||
expect(await fs.readFile(exclude)).toEqual(original);
|
||||
});
|
||||
|
||||
it("preserves a symlink even when its target contains the generated ignore entry", async () => {
|
||||
const target = path.join(root, "original-exclude");
|
||||
await fs.rename(exclude, target);
|
||||
await fs.symlink(target, exclude);
|
||||
const original = await fs.readFile(target);
|
||||
await expect(assertReproducibleBaseGitStorage(checkout)).rejects.toThrow("custom_git_metadata");
|
||||
expect((await fs.lstat(exclude)).isSymbolicLink()).toBe(true);
|
||||
expect(await fs.readFile(target)).toEqual(original);
|
||||
});
|
||||
});
|
||||
@@ -47,7 +47,7 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => {
|
||||
await db.$client.end({ timeout: 1 }); await database.cleanup(); await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function fixture(managed = false, nested = false) {
|
||||
async function fixture(managed = false, nested = false, withProject = managed) {
|
||||
const companyId = randomUUID(), issueId = randomUUID(), workspaceId = randomUUID();
|
||||
const location = path.join(root, companyId), source = path.join(location, "author"), remote = path.join(location, "remote.git");
|
||||
await fs.mkdir(source, { recursive: true });
|
||||
@@ -60,7 +60,7 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => {
|
||||
await db.insert(companies).values({ id: companyId, name: "Git expiration fixture", issuePrefix: `GC${companyId.slice(0, 6)}` });
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Finished task", status: "done", completedAt: new Date(Date.now() - 48 * 3_600_000) });
|
||||
const repoUrl = pathToFileURL(remote).href;
|
||||
const projectId = managed ? randomUUID() : null;
|
||||
const projectId = withProject ? randomUUID() : null;
|
||||
if (projectId) await db.insert(projects).values({ id: projectId, companyId, name: "Fixture project" });
|
||||
const baseCwd = managed
|
||||
? (await materializeManagedProjectWorkspace(resolveManagedProjectWorkspaceDir({ companyId, projectId: projectId!, repoName: "remote" }), { repoUrl })).cwd
|
||||
@@ -149,8 +149,8 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => {
|
||||
expect(await fs.readFile(settingsPath, "utf8")).toBe("later task settings\n");
|
||||
});
|
||||
|
||||
async function nestedFixture() {
|
||||
const f = await fixture(true);
|
||||
async function nestedFixture(managed = true) {
|
||||
const f = await fixture(managed, !managed, true);
|
||||
const nestedSource = path.join(path.dirname(f.source), "nested-author");
|
||||
const nestedRemote = path.join(path.dirname(f.source), "nested.git");
|
||||
await fs.mkdir(nestedSource);
|
||||
@@ -174,6 +174,40 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => {
|
||||
return { ...f, nestedSource, nestedRemote, nestedA, preparation, nested: nested!, settings, settingsPath, nestedSettingsPath };
|
||||
}
|
||||
|
||||
it("expires a nested workspace and its unused private base with the generated ignore entry", async () => {
|
||||
const f = await nestedFixture(false);
|
||||
await f.advance();
|
||||
await fs.writeFile(path.join(f.nestedSource, "nested.txt"), "nested B\n");
|
||||
git(f.nestedSource, "add", "."); git(f.nestedSource, "commit", "-m", "Nested B");
|
||||
git(f.nestedSource, "push", f.nestedRemote, "main");
|
||||
const result = await f.expire();
|
||||
expect(result).toMatchObject({ expired: true, baseExpired: true, skipReason: null });
|
||||
expect(result.reclaimedBytes).toBeGreaterThan(0);
|
||||
expect(await exists(f.cwd)).toBe(false);
|
||||
expect(await exists(f.baseCwd)).toBe(false);
|
||||
expect(await f.reopen()).toMatchObject({ ok: true, reopened: true });
|
||||
expect(git(f.cwd, "rev-parse", "HEAD")).toBe(f.commitA);
|
||||
expect(git(f.nested.cwd, "rev-parse", "HEAD")).toBe(f.nestedA);
|
||||
expect(await fs.readFile(f.settingsPath)).toEqual(f.settings);
|
||||
expect((await fs.stat(f.settingsPath)).mode & 0o777).toBe(0o600);
|
||||
expect(await fs.readFile(f.nestedSettingsPath)).toEqual(f.settings);
|
||||
expect((await fs.stat(f.nestedSettingsPath)).mode & 0o777).toBe(0o640);
|
||||
await fs.writeFile(path.join(f.nested.cwd, "continued.txt"), "continued after base restoration\n");
|
||||
git(f.nested.cwd, "add", "."); git(f.nested.cwd, "commit", "-m", "Continued after base restoration");
|
||||
expect(git(f.nested.cwd, "rev-parse", "HEAD")).not.toBe(f.nestedA);
|
||||
});
|
||||
|
||||
it("retains custom base ignore patterns after expiring the nested workspace", async () => {
|
||||
const f = await nestedFixture(false);
|
||||
const exclude = path.join(f.baseCwd, ".git", "info", "exclude");
|
||||
await fs.appendFile(exclude, "private-output/\n");
|
||||
const original = await fs.readFile(exclude);
|
||||
expect(await f.expire()).toMatchObject({ expired: true, baseExpired: false, skipReason: "custom_git_metadata" });
|
||||
expect(await exists(f.cwd)).toBe(false);
|
||||
expect(await exists(f.baseCwd)).toBe(true);
|
||||
expect(await fs.readFile(exclude)).toEqual(original);
|
||||
});
|
||||
|
||||
async function crashWithExpirationLocks(f: Awaited<ReturnType<typeof nestedFixture>>, beforePublication = false) {
|
||||
expect(await f.expire({ beforeDelete: async () => { throw new Error("stop_before_locks"); } }))
|
||||
.toMatchObject({ expired: false, skipReason: "stop_before_locks" });
|
||||
|
||||
@@ -156,7 +156,12 @@ export async function assertReproducibleBaseGitStorage(cwd: string, signal?: Abo
|
||||
for (const name of await fs.readdir(path.join(root, directory))) {
|
||||
const file = `${directory}/${name}`;
|
||||
const entry = await fs.lstat(path.join(root, file));
|
||||
if (!entry.isFile() || !templates.get(file)?.equals(await fs.readFile(path.join(root, file)))) throw new Error("custom_git_metadata");
|
||||
if (!entry.isFile()) throw new Error("custom_git_metadata");
|
||||
const actual = await fs.readFile(path.join(root, file));
|
||||
const template = templates.get(file);
|
||||
const managedExclude = file === "info/exclude" && template
|
||||
? Buffer.concat([template, Buffer.from(`\n/${PROJECT_REPOSITORIES_DIR}/\n`)]) : null;
|
||||
if (!template?.equals(actual) && !managedExclude?.equals(actual)) throw new Error("custom_git_metadata");
|
||||
}
|
||||
}
|
||||
if (!templates.get("description")?.equals(await fs.readFile(path.join(root, "description")))) throw new Error("custom_git_metadata");
|
||||
|
||||
Reference in new issue
Block a user