mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02: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>
108 lines
5.5 KiB
TypeScript
108 lines
5.5 KiB
TypeScript
import { afterEach, beforeEach, expect, it, vi } from "vitest";
|
|
import { createRunPreparationTiming, measureRunPhase, recordRepositoryObjectCacheDetail, withRunExecutionTiming } from "./execution-preparation-timing.js";
|
|
|
|
const handles = vi.hoisted(() => ({ getTracer: vi.fn(), startActiveSpan: vi.fn() }));
|
|
vi.mock("@opentelemetry/api", async importOriginal => {
|
|
const api = await importOriginal<typeof import("@opentelemetry/api")>();
|
|
return { ...api, trace: { ...api.trace, getTracer: handles.getTracer, getActiveSpan: () => undefined } };
|
|
});
|
|
beforeEach(() => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector.test");
|
|
handles.getTracer.mockReturnValue({ startActiveSpan: handles.startActiveSpan });
|
|
});
|
|
afterEach(() => vi.unstubAllEnvs());
|
|
|
|
it.each(["lookup", "start", "after_callback", "missing_callback", "rejected", "end", "status"])("runs work once and preserves its result or error when tracing fails at %s", async failure => {
|
|
const broken = () => { throw new Error("tracing failed"); };
|
|
handles.getTracer.mockImplementation(failure === "lookup" ? broken : () => ({ startActiveSpan: handles.startActiveSpan }));
|
|
handles.startActiveSpan.mockImplementation((_name, _options, _context, callback) => {
|
|
if (failure === "start") throw new Error("tracer failed before callback");
|
|
if (failure === "missing_callback") return undefined;
|
|
if (failure === "rejected") return Promise.reject(new Error("tracer rejected"));
|
|
const result = callback({ end: failure === "end" ? broken : vi.fn(), setStatus: failure === "status" ? broken : vi.fn() });
|
|
if (failure === "after_callback") throw new Error("tracer failed after callback");
|
|
return result;
|
|
});
|
|
const timing = createRunPreparationTiming("private-run", "native.controller");
|
|
const result = {};
|
|
const work = vi.fn(async () => result);
|
|
await expect(timing("backend_setup", work)).resolves.toBe(result);
|
|
expect(work).toHaveBeenCalledOnce();
|
|
const error = new Error("original private failure");
|
|
const failedWork = vi.fn(async () => { throw error; });
|
|
await expect(timing("assigned_mcp", failedWork)).rejects.toBe(error);
|
|
expect(failedWork).toHaveBeenCalledOnce();
|
|
});
|
|
|
|
it.each([undefined, "", " "])("does not consult tracing for endpoint %s", async endpoint => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint);
|
|
handles.getTracer.mockClear();
|
|
const warn = vi.spyOn(console, "warn");
|
|
const error = vi.spyOn(console, "error");
|
|
try {
|
|
await expect(createRunPreparationTiming("run", "native.controller")("backend_setup", () => 42)).resolves.toBe(42);
|
|
for (const phase of ["execution.usage_startup", "execution.mcp_handoff", "execution.dispatch_admission", "execution.budget_reservation"] as const) {
|
|
const result = {};
|
|
const work = vi.fn(() => {
|
|
recordRepositoryObjectCacheDetail({ outcome: "hit", referenceSupplied: true });
|
|
return result;
|
|
});
|
|
await expect(withRunExecutionTiming("run", () => measureRunPhase(phase, work))).resolves.toBe(result);
|
|
expect(work).toHaveBeenCalledOnce();
|
|
const original = new Error("original startup failure");
|
|
const failed = vi.fn(() => { throw original; });
|
|
await expect(withRunExecutionTiming("run", () => measureRunPhase(phase, failed))).rejects.toBe(original);
|
|
expect(failed).toHaveBeenCalledOnce();
|
|
}
|
|
expect(handles.getTracer).not.toHaveBeenCalled();
|
|
expect(warn).not.toHaveBeenCalled();
|
|
expect(error).not.toHaveBeenCalled();
|
|
} finally {
|
|
warn.mockRestore();
|
|
error.mockRestore();
|
|
}
|
|
});
|
|
|
|
it("keeps cache outcomes and clone references on their owning concurrent phases", async () => {
|
|
const spans: Array<{ name: string; attributes: Record<string, unknown> }> = [];
|
|
handles.startActiveSpan.mockImplementation((name, options, _context, callback) => {
|
|
const record = { name, attributes: { ...options.attributes } };
|
|
spans.push(record);
|
|
return callback({
|
|
end: vi.fn(), setStatus: vi.fn(),
|
|
setAttribute: (key: string, value: unknown) => { record.attributes[key] = value; },
|
|
});
|
|
});
|
|
await Promise.all([
|
|
withRunExecutionTiming("first", () => measureRunPhase("workspace.checkout.cache_lookup", async () => {
|
|
await Promise.resolve();
|
|
recordRepositoryObjectCacheDetail({ outcome: "hit" });
|
|
})),
|
|
withRunExecutionTiming("second", () => measureRunPhase("workspace.checkout.git_clone", async () => {
|
|
await Promise.resolve();
|
|
recordRepositoryObjectCacheDetail({ referenceSupplied: false });
|
|
})),
|
|
]);
|
|
expect(spans.find(span => span.name === "workspace.checkout.cache_lookup")?.attributes).toMatchObject({
|
|
"paperclip.execution.repositoryCache.outcome": "hit",
|
|
});
|
|
expect(spans.find(span => span.name === "workspace.checkout.git_clone")?.attributes).toMatchObject({
|
|
"paperclip.execution.repositoryCache.reference_supplied": false,
|
|
});
|
|
expect(spans.filter(span => span.name === "task.execution_attempt").every(span =>
|
|
Object.keys(span.attributes).every(key => key === "paperclip.task.run.run_id"))).toBe(true);
|
|
});
|
|
|
|
it("does not let cache attribute failures replace the operation result", async () => {
|
|
handles.startActiveSpan.mockImplementation((_name, _options, _context, callback) => callback({
|
|
end: vi.fn(), setStatus: vi.fn(),
|
|
setAttribute: () => { throw new Error("tracer failed"); },
|
|
}));
|
|
const work = vi.fn(() => {
|
|
recordRepositoryObjectCacheDetail({ outcome: "published", referenceSupplied: true });
|
|
return 42;
|
|
});
|
|
await expect(withRunExecutionTiming("run", () => measureRunPhase("workspace.checkout.cache_publish", work))).resolves.toBe(42);
|
|
expect(work).toHaveBeenCalledOnce();
|
|
});
|