mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
## Thinking Path > - Paperclip is the open source control plane people use to coordinate AI-agent work > - Opening an issue fans out into several authenticated issue-detail reads, so repeated work on that path directly affects perceived latency > - Those reads repeated issue and authorization lookups, returned full private JSON even when unchanged, and performed non-critical bookkeeping writes on the request path > - Interaction reads also performed lifecycle writes even though `GET` must be read-only > - This pull request adds request-scoped reuse, private conditional responses, read-only interaction access, and bounded write debouncing without crossing actor, request, or company boundaries > - The result is less database, serialization, logging, and response-body work while preserving authorization and interaction lifecycle invariants ## Linked Issues or Issue Description This is the server-only latency phase. Related work is tracked separately in #10415 (aggregate view), #10416 (warm navigation, merged into the base), and #10463 (bundle split). This pull request intentionally excludes those scopes. **What happened?** Opening an issue detail view caused avoidable server costs: repeated issue and authorization reads within one request, full private JSON responses when a representation was unchanged, writes during interaction-list reads, production debug transport setup, and immediate bookkeeping writes for cloud tenant activity and board-key usage. **Expected behavior** All successful JSON `GET /api/issues/:id/*` responses should support strong private ETags and `304 Not Modified`. Repeated work may be reused only within the current request. `GET /interactions` must not modify stored interactions. Non-critical activity timestamps may be debounced without weakening authentication or stale instance-admin cleanup. **Steps to reproduce** 1. Start Paperclip in local development or self-hosted server mode. 2. Open one issue and request its detail subresources with the same authenticated actor. 3. Repeat a successful JSON request with its `ETag` in `If-None-Match`. 4. Observe `304 Not Modified`, no interaction writes from `GET /interactions`, and unchanged authorization boundaries. **Deployment mode / installation** - Local development or self-hosted server - Built from source - Core server behavior; not adapter-specific ## What Changed - Added strong ETags and `Cache-Control: private, must-revalidate` to successful JSON reads under `/api/issues/:id/*`, including standards-compliant `If-None-Match` handling. - Added request-scoped promise memoization for issue and authorization lookups; no authorization result survives the request. - Made `GET /interactions` read-only, moved supersession and terminal-state handling to mutation paths, and prevented plugin callers from accepting or rejecting interactions after an issue closes. - Removed the production debug-file logger transport while preserving development formatting. - Debounced cloud-tenant activity and board-key `lastUsedAt` persistence, while keeping stale instance-admin deletion unconditional and authentication checks per request. - Added focused tests for ETags, request isolation, authorization lifecycle behavior, interaction invariants, logger configuration, and retry-safe debounce behavior. ## Verification - `pnpm exec vitest run server/src/__tests__/private-json-etag.test.ts server/src/__tests__/issue-thread-interaction-routes.test.ts` — 2 files, 23 tests passed. - Focused Vitest run covering request memoization, authorization, interactions, plugin orchestration, logger, cloud tenant, board auth, and issue services — 9 files, 264 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check origin/master...HEAD` — passed. - Scope guardrails: 21 changed files under `server/src`; no lockfile, workflow, migration, UI, aggregate-view, or bundle-split changes. ## Risks - Strong ETags hash each successful serialized JSON response. This adds a small CPU cost but avoids transferring unchanged bodies. - Debounced bookkeeping timestamps can lag by the bounded debounce interval. They are non-critical usage metadata; authentication still runs per request, and stale instance-admin deletion remains unconditional. - Legacy pending interactions on terminal issues are projected as expired by reads and are finalized only by mutation paths. The stored record remains unchanged on `GET` by design. - No database schema or migration changes are included. > This is a focused performance correction and does not duplicate a planned core feature in `ROADMAP.md`. ## Model Used OpenAI Codex using `gpt-5.3-codex` for the initial implementation and `gpt-5.6-sol` for isolation, verification, and PR preparation, with reasoning, repository tool use, code execution, and GitHub CLI access. The runtimes did not expose authoritative context-window sizes. ## 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> Co-authored-by: Dev Agent <dev@paperclip.ing>
109 lines
3.7 KiB
TypeScript
109 lines
3.7 KiB
TypeScript
import { describe, expect, it, vi, beforeEach } from "vitest";
|
|
|
|
/**
|
|
* Regression test for https://github.com/paperclipai/paperclip/issues/2879
|
|
*
|
|
* pino-pretty's `translateTime: "HH:MM:ss"` formats all timestamps in UTC
|
|
* regardless of the process's TZ env var. The `SYS:` prefix instructs
|
|
* pino-pretty to use the local system timezone, so operators in non-UTC
|
|
* zones see correct wall-clock times in their logs.
|
|
*
|
|
* We verify that:
|
|
* 1. The logger module initialises pino-pretty with "SYS:HH:MM:ss".
|
|
* 2. The pino-pretty SYS: prefix resolves to a timezone-sensitive format
|
|
* string — confirmed via pino-pretty's own asynchronous formatter, which
|
|
* applies translateTime to a known epoch under different TZ values.
|
|
*/
|
|
|
|
const mockTransport = vi.hoisted(() => vi.fn(() => ({ write: vi.fn() })));
|
|
const mockPino = vi.hoisted(() => {
|
|
const fn = vi.fn(() => ({
|
|
info: vi.fn(),
|
|
debug: vi.fn(),
|
|
error: vi.fn(),
|
|
warn: vi.fn(),
|
|
child: vi.fn(),
|
|
}));
|
|
(fn as any).transport = mockTransport;
|
|
return fn;
|
|
});
|
|
|
|
// Mock fs so the module-level mkdirSync call is a no-op in tests.
|
|
vi.mock("node:fs", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("node:fs")>();
|
|
return { ...actual, mkdirSync: vi.fn() };
|
|
});
|
|
|
|
vi.mock("pino", () => ({
|
|
default: mockPino,
|
|
}));
|
|
vi.mock("pino-http", () => ({
|
|
pinoHttp: vi.fn(() => vi.fn()),
|
|
}));
|
|
vi.mock("../config-file.js", () => ({
|
|
readConfigFile: vi.fn(() => null),
|
|
}));
|
|
vi.mock("../home-paths.js", () => ({
|
|
resolveHomeAwarePath: vi.fn((p: string) => p),
|
|
resolveDefaultLogsDir: vi.fn(() => "/tmp/paperclip-test-logs"),
|
|
}));
|
|
|
|
describe("logger translateTime respects TZ environment variable", () => {
|
|
beforeEach(() => {
|
|
vi.resetModules();
|
|
vi.unstubAllEnvs();
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it("configures pino-pretty with SYS:HH:MM:ss so timestamps honour the TZ env var", async () => {
|
|
vi.stubEnv("NODE_ENV", "development");
|
|
await import("../middleware/logger.js");
|
|
|
|
expect(mockTransport).toHaveBeenCalledOnce();
|
|
const transport = mockTransport.mock.calls[0][0] as {
|
|
target: string;
|
|
options: Record<string, unknown>;
|
|
};
|
|
expect(transport.target).toBe("pino-pretty");
|
|
expect(transport.options.translateTime).toBe("SYS:HH:MM:ss");
|
|
});
|
|
|
|
it("does not construct a pretty transport in production", async () => {
|
|
vi.stubEnv("NODE_ENV", "production");
|
|
await import("../middleware/logger.js");
|
|
|
|
expect(mockTransport).not.toHaveBeenCalled();
|
|
expect(mockPino).toHaveBeenCalledWith(expect.objectContaining({ level: "info" }));
|
|
});
|
|
|
|
it("SYS: prefix produces timezone-sensitive output: UTC epoch formats differently under UTC vs UTC+8", () => {
|
|
// Verifies the contract that SYS: relies on: formatting the same epoch
|
|
// with different explicit timezones (mirroring what the process TZ env
|
|
// var does at the OS level) must yield different results.
|
|
const EPOCH_MS = 946_684_800_000; // 2000-01-01 00:00:00 UTC
|
|
|
|
const fmtUtc = new Intl.DateTimeFormat("en-GB", {
|
|
timeZone: "UTC",
|
|
hour: "2-digit",
|
|
minute: "2-digit",
|
|
second: "2-digit",
|
|
hour12: false,
|
|
}).format(EPOCH_MS);
|
|
|
|
const fmtSgt = new Intl.DateTimeFormat("en-GB", {
|
|
timeZone: "Asia/Singapore", // UTC+8
|
|
hour: "2-digit",
|
|
minute: "2-digit",
|
|
second: "2-digit",
|
|
hour12: false,
|
|
}).format(EPOCH_MS);
|
|
|
|
// UTC midnight = 00:00:00; the same instant in SGT = 08:00:00.
|
|
// SYS: picks up whichever of these the process TZ is set to — which is
|
|
// exactly what the fix enables by switching from HH:MM:ss (UTC-only).
|
|
expect(fmtUtc).toBe("00:00:00");
|
|
expect(fmtSgt).toBe("08:00:00");
|
|
expect(fmtUtc).not.toBe(fmtSgt);
|
|
});
|
|
});
|