mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 21:49:18 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On restart, `/api/health` reports `starting` until startup recovery finishes, and deploy tooling waits for `ok` > - After heartbeat recovery, readiness also waits for housekeeping that every scheduler tick repeats and that serving requests does not need > - #16330 added OpenTelemetry spans for the main startup phases; native recovery and orphan reaping are the slow ones, and both run the native finalization reconciliation pass, which has no inner breakdown > - This pull request stops readiness waiting on that housekeeping, and records the phases of slow reconciliation passes as spans > - The benefit is faster readiness on every restart, and traces that show where a slow reconciliation pass spends its time ## Linked Issues or Issue Description Builds on #16330, which records `controller.startup.<phase>` spans. Related to #16328. **What existing behavior does this improve?** Server readiness time after a restart, and diagnosis of slow native recovery. **Current behavior** - After heartbeat recovery, readiness waits for the custom-image setup cleanup, tool connection health (remote MCP checks), decision expiry, the adapter and setup-token login reapers, environment lease cleanup (provider calls) and the retention sweep. Each also runs on every scheduler tick. On one instance they added 11–28 s to every measured restart. A throw in any of them fails startup. - On the same instance, native recovery took from 33 s to 11.5 min and orphan reaping from 30 s to 4 min across restarts in one hour. Both run the native finalization reconciliation pass. The #16330 spans time each step as a whole, but not the phases inside that pass. **Proposed behavior** - The housekeeping steps run after readiness. The scheduler interval still starts after them. - With an OTLP endpoint, a reconciliation pass of at least one second records a span with one child span per phase. **Reason and benefit** Readiness depends only on what serving requests and run recovery need. Slow reconciliation passes become attributable to a phase in the existing traces, without adding trace volume for the periodic pass. **Breaking changes** None. A failed housekeeping step no longer fails startup; it is logged and the next step runs. ## What Changed - `server/src/index.ts`: moves the housekeeping steps into `runStartupHousekeeping`, which runs after readiness under `trackHeartbeatSchedulerWork` (idle sleep and shutdown wait for it). Each step catches and logs its own failure (`startup housekeeping step failed`). The heartbeat scheduler interval starts when housekeeping finishes, unless the scheduler was stopped, so the reapers still finish before the first timer tick. - New `server/src/services/native-runtime/native-reconciliation-timing.ts`: records the phase boundaries of one pass. When the pass took at least one second and an OTLP endpoint is configured, it emits a `controller.native_reconciliation` root span (`paperclip.native-reconciliation`) with child spans `legacy_workspace_exports`, `obsolete_policy_reviews`, `automatic_completion_reviews`, `candidate_scan` and `candidates`, using the recorded start and end times. It records `scoped` and the candidate count only. A failed pass marks the pass span and the running phase. Tracing failures are swallowed. - `native-finalization-reconciler.ts`: the exported function wraps the pass in that timing and finishes it in `finally`, so failed passes are recorded; the original error is rethrown. - `doc/execution-semantics.md`: what readiness waits for, and the housekeeping order. `doc/observability.md`: the reconciliation spans. ## Verification - `pnpm exec vitest run src/__tests__/server-startup-feedback-export.test.ts src/services/native-runtime/native-reconciliation-timing.test.ts src/services/native-runtime/native-finalization-reconciler-timing.test.ts src/startup-recovery-timing.test.ts` (in `server/`): 52 passed. New tests: - Readiness is `ready` while a housekeeping step hangs, and the heartbeat interval starts only after the step settles. - A failed housekeeping step is logged, and the scheduler still starts after the remaining steps. - Span timing: nothing for a fast pass or without an endpoint; a slow pass emits the pass span and one child per phase with the recorded times; a failed pass marks the running phase and the pass; a failing tracer does not throw. - The reconciler records its phases and a failed outcome when a pass throws, and rethrows. - Three existing startup tests settle housekeeping before they read the scheduler interval or the setup cleanup. - `pnpm exec vitest run src/services/native-runtime`: 4,399 passed. - `pnpm typecheck` in `server/`: passed. ## Risks - Housekeeping now runs concurrently with the first requests and with `waitForExternalAdapters`. None of these steps is needed to serve requests, and each one also runs on every scheduler tick. - Readiness is reported earlier; the heartbeat timer starts at the same point relative to housekeeping as before. - The new spans exist only with an OTLP endpoint and only for passes of at least one second. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code with tool use (shell, file edits, test runs). ## 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
112 lines
4.5 KiB
TypeScript
112 lines
4.5 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
const handles = vi.hoisted(() => ({
|
|
startSpan: vi.fn(),
|
|
getTracer: vi.fn(),
|
|
setSpan: vi.fn((_context: unknown, span: unknown) => ({ parent: span })),
|
|
}));
|
|
vi.mock("@opentelemetry/api", () => ({
|
|
ROOT_CONTEXT: "root",
|
|
SpanStatusCode: { ERROR: 2 },
|
|
trace: { getTracer: handles.getTracer, setSpan: handles.setSpan },
|
|
}));
|
|
|
|
const { createNativeReconciliationPassTiming } = await import("./native-reconciliation-timing.js");
|
|
|
|
function clock() {
|
|
let at = 10_000;
|
|
return { now: () => at, advance: (ms: number) => { at += ms; } };
|
|
}
|
|
|
|
function spanStub(name: string) {
|
|
return { name, end: vi.fn(), setStatus: vi.fn() };
|
|
}
|
|
|
|
beforeEach(() => {
|
|
vi.unstubAllEnvs();
|
|
vi.clearAllMocks();
|
|
handles.getTracer.mockReturnValue({ startSpan: handles.startSpan });
|
|
handles.startSpan.mockImplementation((name: string) => spanStub(name));
|
|
});
|
|
afterEach(() => vi.unstubAllEnvs());
|
|
|
|
describe("native reconciliation pass timing", () => {
|
|
it("records nothing for a fast pass", () => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector.test");
|
|
const c = clock();
|
|
const timing = createNativeReconciliationPassTiming({ now: c.now });
|
|
timing.phase("candidate_scan");
|
|
c.advance(999);
|
|
timing.finish({ outcome: "ok", scoped: false });
|
|
expect(handles.getTracer).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("records nothing without an OTLP endpoint", () => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "");
|
|
const c = clock();
|
|
const timing = createNativeReconciliationPassTiming({ now: c.now });
|
|
timing.phase("candidate_scan");
|
|
c.advance(5_000);
|
|
timing.finish({ outcome: "ok", scoped: false });
|
|
expect(handles.getTracer).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("emits a pass span with one child span per phase for a slow pass", () => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector.test");
|
|
const c = clock();
|
|
const timing = createNativeReconciliationPassTiming({ now: c.now });
|
|
timing.phase("legacy_workspace_exports");
|
|
c.advance(100);
|
|
timing.phase("candidate_scan");
|
|
c.advance(300);
|
|
timing.candidates(187);
|
|
timing.phase("candidates");
|
|
c.advance(2_000);
|
|
timing.finish({ outcome: "ok", scoped: false });
|
|
|
|
const calls = handles.startSpan.mock.calls;
|
|
expect(calls[0]).toEqual([
|
|
"controller.native_reconciliation",
|
|
{ startTime: 10_000, attributes: { "paperclip.native_reconciliation.scoped": false, "paperclip.native_reconciliation.candidates": 187 } },
|
|
"root",
|
|
]);
|
|
const pass = handles.startSpan.mock.results[0]!.value;
|
|
expect(calls.slice(1).map(([name, options, context]) => [name, options.startTime, context])).toEqual([
|
|
["controller.native_reconciliation.legacy_workspace_exports", 10_000, { parent: pass }],
|
|
["controller.native_reconciliation.candidate_scan", 10_100, { parent: pass }],
|
|
["controller.native_reconciliation.candidates", 10_400, { parent: pass }],
|
|
]);
|
|
const children = handles.startSpan.mock.results.slice(1).map(result => result.value);
|
|
expect(children.map(span => span.end.mock.calls[0]?.[0])).toEqual([10_100, 10_400, 12_400]);
|
|
expect(pass.end).toHaveBeenCalledWith(12_400);
|
|
expect(pass.setStatus).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("marks the failed phase and the pass when a slow pass fails", () => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector.test");
|
|
const c = clock();
|
|
const timing = createNativeReconciliationPassTiming({ now: c.now });
|
|
timing.phase("obsolete_policy_reviews");
|
|
c.advance(10);
|
|
timing.phase("automatic_completion_reviews");
|
|
c.advance(2_000);
|
|
timing.finish({ outcome: "failed", scoped: true });
|
|
|
|
const [pass, reviews, completion] = handles.startSpan.mock.results.map(result => result.value);
|
|
expect(pass.setStatus).toHaveBeenCalledWith({ code: 2 });
|
|
expect(reviews.setStatus).not.toHaveBeenCalled();
|
|
expect(completion.name).toBe("controller.native_reconciliation.automatic_completion_reviews");
|
|
expect(completion.setStatus).toHaveBeenCalledWith({ code: 2 });
|
|
});
|
|
|
|
it("never throws when tracing fails", () => {
|
|
vi.stubEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector.test");
|
|
handles.getTracer.mockImplementation(() => { throw new Error("tracer unavailable"); });
|
|
const c = clock();
|
|
const timing = createNativeReconciliationPassTiming({ now: c.now });
|
|
timing.phase("candidates");
|
|
c.advance(5_000);
|
|
expect(() => timing.finish({ outcome: "ok", scoped: false })).not.toThrow();
|
|
});
|
|
});
|