mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
## Thinking Path > - Paperclip manages agent work and its execution environments. > - A fresh repository task needs an independent checkout before its runner can start. > - The object cache from #16487 should reduce repeated network transfer. > - The cache kept Git objects but dropped remote-tracking refs during its bare clone. > - Git needs those tips to negotiate which objects it already has. > - This pull request preserves the tips in a versioned mirror cache and records explicit cache outcomes. > - The benefit is less repeated transfer and clearer evidence when a cache does not help. ## Linked Issues or Issue Description Refs #16487 and #16454. No duplicate cache-negotiation fix was found in the public PR search. **What happened?** A valid object cache could make a fresh clone slower. A controlled staging probe took 45.87 seconds and received 184.39 MB even though the reference already held the objects. An earlier probe took 52.33 seconds. An uncached clone took 20.94 seconds. `git clone --bare` copied the pristine checkout's objects, local branches, and tags, but omitted its remote-tracking refs. The source had about 1,200 remote branches. Git could not use the missing tips to negotiate those objects. **Expected behavior** Keep the pristine reference's negotiation tips. Each task must still authenticate online, use the current remote refs, and end with independent Git objects. **Steps to reproduce** 1. Clone a repository with many remote branches into a fresh checkout. 2. Publish a reference with `git clone --bare --no-hardlinks` from that checkout. 3. Clone the remote with `--reference-if-able` and `--dissociate`. 4. Compare with a reference that retains the remote-tracking tips. Use Git Trace2 and count received pack bytes. **Paperclip version or commit** The staging runtime was `27ab902d3db2770ada79c9f42a9c06e1d346f34b`, with Git 2.47.3. The cache implementation was unchanged from #16487. **Deployment mode** Docker on a persistent application volume. The probe used the application file owner and the public Paperclip repository. It did not start agent tasks or alter the application cache. ## What Changed - Publish pristine object caches with `--mirror` to retain remote-tracking tips. - Put cache format version 2 in the key and receipt. The first checkout builds a new generation. Existing generations stay under the current storage limits. - Keep the existing online authentication, current remote refs, credential and company isolation, independent objects, quotas, and fallback behavior. - Add closed Honeycomb cache outcomes and a boolean that states whether a reference was supplied to clone. - Document that a lookup hit or supplied reference does not prove Git avoided transfer. ## Verification - Controlled staging component experiment: a disposable reference copy held exactly the same 210,057 objects as the original. It added 1,187 advertised branch tips whose commits were already in those objects. - Before: 45.87 seconds and 184.39 MB received, one sample in this comparison. - After: 12.20 and 14.58 seconds; median 13.39 seconds. Received pack sizes were 1.99 and 1.95 MB. - All comparison clones used the same current remote HEAD and ended without an alternates file. - The remaining repack took 9.72–9.81 seconds. Checkout file writing took 0.86–0.96 seconds. - The staging experiment restored tips on a disposable cache copy. It did not deploy this patch or measure complete agent latency. - Confirmed both staging scratch locations were empty afterward. The original cache timestamps stayed unchanged. The temporary operator SSH key was revoked and its local files removed. - TypeScript 5.9.3 transpilation reported zero syntax diagnostics across the four changed TypeScript files. `git diff --check` passed. - The new negotiation regression fails with the old `--bare` publisher and passes with the mirror publisher. - Focused Vitest suites: 26 tests passed across repository-object-cache and execution-preparation-timing. These cover independent objects, current refs, unavailable/revoked remotes, scope separation, invalid storage, tracing isolation, and endpoint-off behavior. - The first full focused run passed 24 tests but failed its existing localhost HTTP fixture because the sandbox denied `listen` on `127.0.0.1`. The rerun with local loopback permission passed all 26 tests. - Server semantic typechecking passed with `tsc -p server/tsconfig.json --noEmit` after building the required workspace declarations. Initial attempts lacked generated dependency outputs; the completed dependency build and rerun passed. - A full application build was not run locally. CI passed: 55 checks passed and two optional checks were skipped. Greptile is 5/5 on the current head, with both findings resolved. ## Risks - Version 2 causes one cold cache rebuild. Old cache generations retain their disk usage, and the existing quota can prevent publication. - A cache still needs a full repack to produce an independent task checkout. This PR does not remove that cost. - The component experiment establishes the negotiation effect on staging. It does not establish an end-to-end agent speedup or the under-30-second goal. - The new trace fields remain behind the existing OTLP gate. Their values are closed enums or a boolean. They export no refs, paths, URLs, cache keys, credentials, or error text. ## Model Used OpenAI Codex, GPT-6 family, with code and tool use. The exact serving model ID and context window are 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>
222 lines
12 KiB
TypeScript
222 lines
12 KiB
TypeScript
import { execFile as execute } from "node:child_process";
|
|
import fs from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { pathToFileURL } from "node:url";
|
|
import { promisify } from "node:util";
|
|
import { createServer } from "node:http";
|
|
import { createReadStream } from "node:fs";
|
|
import { afterEach, expect, it, vi } from "vitest";
|
|
import { withDirectoryMergeLock } from "@paperclipai/adapter-utils/workspace-restore-merge";
|
|
import { materializeManagedProjectWorkspace } from "../services/managed-repository-checkout.js";
|
|
import { assertReproducibleBaseGitStorage } from "../services/git-workspace-restoration.js";
|
|
import { readRepositoryObjectCache, resolveRepositoryObjectCache, publishRepositoryObjectCache } from "../services/repository-object-cache.js";
|
|
import * as preparationTiming from "../services/execution-preparation-timing.js";
|
|
|
|
const execFile = promisify(execute);
|
|
const roots: string[] = [];
|
|
afterEach(async () => {
|
|
vi.unstubAllEnvs();
|
|
vi.restoreAllMocks();
|
|
await Promise.all(roots.splice(0).map(root => fs.rm(root, { recursive: true, force: true })));
|
|
});
|
|
async function fixture() {
|
|
const root = await fs.mkdtemp(path.join(os.tmpdir(), "repository-object-cache-")); roots.push(root);
|
|
vi.stubEnv("PAPERCLIP_HOME", path.join(root, "home"));
|
|
const source = path.join(root, "source"); await fs.mkdir(source);
|
|
const git = async (cwd: string, ...args: string[]) => (await execFile("git", ["-C", cwd, ...args])).stdout.trim();
|
|
await git(source, "init", "-b", "main");
|
|
await git(source, "config", "user.name", "Fixture"); await git(source, "config", "user.email", "fixture@example.test");
|
|
await fs.writeFile(path.join(source, "work.txt"), "initial\n");
|
|
await git(source, "add", "."); await git(source, "commit", "-m", "initial");
|
|
const repoUrl = pathToFileURL(source).href;
|
|
const company = path.join(root, "cache", "company");
|
|
const prepare = (name: string) => materializeManagedProjectWorkspace(path.join(root, name), { repoUrl, objectCacheCompanyDirectory: company });
|
|
return { root, source, git, company, repoUrl, prepare };
|
|
}
|
|
|
|
it("retains remote branch tips so a primed clone transfers no existing objects", async () => {
|
|
const f = await fixture();
|
|
await f.git(f.source, "checkout", "-b", "feature");
|
|
await fs.writeFile(path.join(f.source, "feature.txt"), "unmerged feature content\n");
|
|
await f.git(f.source, "add", ".");
|
|
await f.git(f.source, "commit", "-m", "feature");
|
|
const featureHead = await f.git(f.source, "rev-parse", "HEAD");
|
|
await f.git(f.source, "checkout", "main");
|
|
await f.prepare("first");
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
expect(await f.git(cache.directory, "rev-parse", "refs/remotes/origin/feature")).toBe(featureHead);
|
|
|
|
const packTrace = path.join(f.root, "received.pack");
|
|
vi.stubEnv("GIT_TRACE_PACKFILE", packTrace);
|
|
await f.prepare("second");
|
|
expect(await fs.stat(packTrace).then(stat => stat.size, () => 0)).toBe(0);
|
|
const target = path.join(f.root, "second");
|
|
expect(await f.git(target, "rev-parse", "origin/feature")).toBe(featureHead);
|
|
await expect(fs.stat(path.join(target, ".git/objects/info/alternates"))).rejects.toMatchObject({ code: "ENOENT" });
|
|
await fs.rm(f.company, { recursive: true });
|
|
expect(await f.git(target, "show", "origin/feature:feature.txt")).toBe("unmerged feature content");
|
|
await f.git(target, "fsck", "--full");
|
|
});
|
|
|
|
it("uses current remote refs and produces an independent checkout without private cached refs", async () => {
|
|
const f = await fixture();
|
|
await f.git(f.source, "checkout", "-b", "private");
|
|
await fs.writeFile(path.join(f.source, "private.txt"), "private branch bytes");
|
|
await f.git(f.source, "add", "."); await f.git(f.source, "commit", "-m", "private");
|
|
const privateHead = await f.git(f.source, "rev-parse", "HEAD");
|
|
await f.git(f.source, "checkout", "main");
|
|
await f.prepare("first");
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
expect(await readRepositoryObjectCache(cache)).toBe(cache.directory);
|
|
await f.git(f.source, "branch", "-D", "private");
|
|
await fs.writeFile(path.join(f.source, "work.txt"), "current\n");
|
|
await f.git(f.source, "commit", "-am", "current"); await f.git(f.source, "tag", "current-tag");
|
|
const currentHead = await f.git(f.source, "rev-parse", "HEAD");
|
|
await f.prepare("second");
|
|
const target = path.join(f.root, "second");
|
|
expect(await f.git(target, "rev-parse", "HEAD")).toBe(currentHead);
|
|
expect(await f.git(target, "rev-parse", "current-tag")).toBe(currentHead);
|
|
expect(await fs.readFile(path.join(target, "work.txt"), "utf8")).toBe("current\n");
|
|
await expect(f.git(target, "cat-file", "-e", privateHead)).rejects.toThrow();
|
|
await expect(fs.stat(path.join(target, ".git/objects/info/alternates"))).rejects.toMatchObject({ code: "ENOENT" });
|
|
await assertReproducibleBaseGitStorage(target);
|
|
await fs.rm(f.company, { recursive: true });
|
|
await f.git(target, "fsck", "--full");
|
|
expect(await f.git(target, "show", "HEAD~1:work.txt")).toBe("initial");
|
|
});
|
|
|
|
it("never substitutes a cached checkout for an unavailable remote", async () => {
|
|
const f = await fixture(); await f.prepare("first");
|
|
await fs.rename(f.source, `${f.source}-unavailable`);
|
|
await expect(f.prepare("denied")).rejects.toThrow("Failed to prepare managed checkout");
|
|
await expect(fs.stat(path.join(f.root, "denied"))).rejects.toMatchObject({ code: "ENOENT" });
|
|
});
|
|
|
|
it("separates company and current credential scopes", async () => {
|
|
const f = await fixture();
|
|
const auth = { source: "company_secret" as const, secretName: "GITHUB_TOKEN", configArgs: [], env: { TOKEN: "first-token" } };
|
|
const first = await resolveRepositoryObjectCache(f.company, f.repoUrl, auth);
|
|
const other = await resolveRepositoryObjectCache(f.company, f.repoUrl, { ...auth, env: { TOKEN: "new-token" } });
|
|
const company = await resolveRepositoryObjectCache(path.join(f.root, "cache/other-company"), f.repoUrl, auth);
|
|
expect(first!.directory).not.toBe(other!.directory); expect(first!.directory).not.toBe(company!.directory);
|
|
expect(JSON.stringify(first)).not.toContain("first-token");
|
|
});
|
|
|
|
it("skips contended optional cache publication without delaying a fresh checkout", async () => {
|
|
const f = await fixture();
|
|
await resolveRepositoryObjectCache(f.company, f.repoUrl, null);
|
|
await withDirectoryMergeLock(f.company, async () => {
|
|
await f.prepare("first");
|
|
expect(await fs.readdir(f.company)).toEqual([]);
|
|
});
|
|
expect(await fs.readFile(path.join(f.root, "first/work.txt"), "utf8")).toBe("initial\n");
|
|
});
|
|
|
|
it("rebuilds from the remote when the cached pack is corrupt", async () => {
|
|
const f = await fixture(); await f.prepare("first");
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
await f.git(cache.directory, "gc", "--prune=now");
|
|
const packs = (await fs.readdir(path.join(cache.directory, "objects/pack"))).filter(entry => entry.endsWith(".pack"));
|
|
expect(packs.length).toBeGreaterThan(0);
|
|
for (const entry of packs) {
|
|
const file = path.join(cache.directory, "objects/pack", entry);
|
|
await fs.chmod(file, 0o600);
|
|
await fs.writeFile(file, "corrupted");
|
|
}
|
|
await f.git(path.join(f.root, "first"), "fsck", "--full");
|
|
await f.prepare("second");
|
|
expect(await fs.readFile(path.join(f.root, "second/work.txt"), "utf8")).toBe("initial\n");
|
|
});
|
|
|
|
it("still authenticates to the HTTP remote when a matching object cache exists", async () => {
|
|
const f = await fixture();
|
|
const remote = path.join(f.root, "remote.git");
|
|
await execFile("git", ["clone", "--bare", f.source, remote]);
|
|
await f.git(remote, "update-server-info");
|
|
let revoked = false, rejected = 0;
|
|
const server = createServer(async (request, response) => {
|
|
if (revoked || request.headers.authorization !== "Bearer fixture-token") {
|
|
rejected++; response.writeHead(403); response.end(); return;
|
|
}
|
|
const pathname = new URL(request.url!, "http://localhost").pathname;
|
|
if (!/^\/[a-zA-Z0-9/._-]+$/.test(pathname) || pathname.includes("..")) { response.writeHead(404); response.end(); return; }
|
|
const file = path.join(remote, pathname);
|
|
if (!await fs.stat(file).then(stat => stat.isFile(), () => false)) { response.writeHead(404); response.end(); return; }
|
|
response.setHeader("Content-Type", "text/plain");
|
|
createReadStream(file).pipe(response);
|
|
});
|
|
await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
|
|
try {
|
|
const address = server.address() as { port: number };
|
|
const repoUrl = `http://127.0.0.1:${address.port}`;
|
|
const resolveGitAuth = vi.fn(async () => ({ source: "company_secret" as const, secretName: "fixture", configArgs: [], env: {
|
|
GIT_CONFIG_COUNT: "1", GIT_CONFIG_KEY_0: "http.extraHeader", GIT_CONFIG_VALUE_0: "Authorization: Bearer fixture-token",
|
|
} }));
|
|
const prepare = (name: string) => materializeManagedProjectWorkspace(path.join(f.root, name), {
|
|
repoUrl, objectCacheCompanyDirectory: f.company, resolveGitAuth,
|
|
});
|
|
await prepare("authorized");
|
|
expect(await fs.readdir(f.company)).toHaveLength(1);
|
|
revoked = true;
|
|
await expect(prepare("revoked")).rejects.toThrow("403");
|
|
expect(rejected).toBeGreaterThan(0); expect(resolveGitAuth.mock.calls.length).toBeGreaterThanOrEqual(2);
|
|
await expect(fs.stat(path.join(f.root, "revoked"))).rejects.toMatchObject({ code: "ENOENT" });
|
|
expect(await fs.readFile(path.join(f.root, "authorized/.git/config"), "utf8")).not.toContain("fixture-token");
|
|
} finally { await new Promise<void>((resolve, reject) => server.close(error => error ? reject(error) : resolve())); }
|
|
});
|
|
|
|
it("does not follow a cache directory symlink or grow an over-capacity company", async () => {
|
|
const f = await fixture();
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
await fs.symlink(f.source, cache.directory);
|
|
expect(await readRepositoryObjectCache(cache)).toBeNull();
|
|
await fs.unlink(cache.directory);
|
|
for (let i = 0; i < 64; i++) await fs.mkdir(path.join(f.company, `retained-${i}`));
|
|
await publishRepositoryObjectCache(cache, f.source);
|
|
expect(await fs.readdir(f.company)).toHaveLength(64);
|
|
await expect(fs.stat(cache.directory)).rejects.toMatchObject({ code: "ENOENT" });
|
|
});
|
|
|
|
it("skips oversized repositories without creating a cache generation", async () => {
|
|
const f = await fixture();
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
const oversized = path.join(f.source, ".git/oversized");
|
|
await fs.writeFile(oversized, ""); await fs.truncate(oversized, 2 * 1024 ** 3 + 1);
|
|
const detail = vi.spyOn(preparationTiming, "recordRepositoryObjectCacheDetail");
|
|
await publishRepositoryObjectCache(cache, f.source);
|
|
expect(detail).toHaveBeenLastCalledWith({ outcome: "capacity" });
|
|
expect(await fs.readdir(f.company)).toEqual([]);
|
|
expect(await f.git(f.source, "show", "HEAD:work.txt")).toBe("initial");
|
|
});
|
|
|
|
it("reports a symlink found during storage scanning as an invalid cache skip", async () => {
|
|
const f = await fixture();
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
await fs.symlink(f.source, path.join(f.source, ".git/unexpected-link"));
|
|
const detail = vi.spyOn(preparationTiming, "recordRepositoryObjectCacheDetail");
|
|
await publishRepositoryObjectCache(cache, f.source);
|
|
expect(detail).toHaveBeenLastCalledWith({ outcome: "invalid" });
|
|
expect(await fs.readdir(f.company)).toEqual([]);
|
|
});
|
|
|
|
it.each(["https://user:secret@github.com/fixture/repo", "https://github.com/fixture/repo?token=secret", "https://github.com/fixture/repo#secret"])("does not persist credential-bearing URLs: %s", async (repoUrl) => {
|
|
const f = await fixture();
|
|
expect(await resolveRepositoryObjectCache(f.company, repoUrl, null)).toBeNull();
|
|
await expect(fs.stat(f.company)).rejects.toMatchObject({ code: "ENOENT" });
|
|
});
|
|
|
|
it("does not initialize or publish through a replaced cache parent", async () => {
|
|
const f = await fixture();
|
|
const other = path.join(f.root, "other"); await fs.mkdir(other);
|
|
await fs.symlink(other, path.dirname(f.company));
|
|
expect(await resolveRepositoryObjectCache(f.company, f.repoUrl, null)).toBeNull();
|
|
expect(await fs.readdir(other)).toEqual([]);
|
|
await fs.unlink(path.dirname(f.company));
|
|
const cache = (await resolveRepositoryObjectCache(f.company, f.repoUrl, null))!;
|
|
await fs.rename(path.dirname(f.company), path.join(f.root, "retained"));
|
|
await fs.mkdir(path.join(other, "company")); await fs.symlink(other, path.dirname(f.company));
|
|
await publishRepositoryObjectCache(cache, f.source);
|
|
expect(await fs.readdir(path.join(other, "company"))).toEqual([]);
|
|
});
|