From 99efe7a80854a0227dfb64b1dbab8faffe1614d6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:39:22 +0000 Subject: [PATCH 1/2] fix(cli): Stop caching events and traces for 24 hours The fallback "immutable" tier matched any `/events//`, `/trace//`, or `/trace-items//` URL and was checked before the `/issues/` volatile pattern, so `/issues//events/latest/` was cached for a day. Event payloads embed mutable fields (next/previous event IDs, user reports, release data), `latest`/`oldest`/`recommended` resolve to different events over time, and full traces gain spans (getsentry/sentry#128818). Replace it with a 5-minute `detail` tier limited to exact path shapes: organization event details without `averageColumn` and project trace-item details without `debug`. Issue events, project event details, and full traces move to the 60-second volatile tier, which is now checked first. Server cache headers still take precedence. Existing on-disk entries are re-checked against the new tier on read, so no migration is needed. Dropping `immutableMinTimeToLive` keeps the http-cache-semantics default (24 hours) for server `immutable` responses. Task group 2 of the unify-sentry-api-layer OpenSpec change (#1669). Co-Authored-By: BYK --- packages/cli/src/lib/response-cache.ts | 107 ++++++++++++------ .../test/lib/response-cache.property.test.ts | 58 ++++++++-- packages/cli/test/lib/response-cache.test.ts | 63 ++++++++++- 3 files changed, 185 insertions(+), 43 deletions(-) diff --git a/packages/cli/src/lib/response-cache.ts b/packages/cli/src/lib/response-cache.ts index 6019c1e62..9bef2d49d 100644 --- a/packages/cli/src/lib/response-cache.ts +++ b/packages/cli/src/lib/response-cache.ts @@ -45,49 +45,79 @@ const log = logger.withTag("response-cache"); /** * TTL tier classification for URLs. * - * - `immutable`: data that never changes once created (events, traces) + * - `detail`: single event or trace-item lookups whose response does not + * change once written (organization event details, project trace-item + * details). Still bounded: deletion and access revocation only take + * effect when the entry expires. * - `stable`: data that changes infrequently (orgs, projects, teams) - * - `volatile`: data that changes often (issue lists, log lists) + * - `volatile`: data that changes often (issues and their events, project + * event details, full traces, log and span queries) * - `no-cache`: never cache (polling endpoints like autofix state) + * + * No tier treats a response as immutable. Event payloads embed mutable + * fields (`nextEventID`, user reports, release data), `latest`/`oldest`/ + * `recommended` resolve to different events over time, and traces gain + * spans as they arrive (getsentry/sentry#128818). */ -type TtlTier = "immutable" | "stable" | "volatile" | "no-cache"; +type TtlTier = "detail" | "stable" | "volatile" | "no-cache"; /** Fallback TTL durations by tier (milliseconds). `no-cache` uses 0 as a sentinel. */ const FALLBACK_TTL_MS: Record = { - immutable: 24 * 60 * 60 * 1000, // 24 hours — events and traces never change + detail: 5 * 60 * 1000, // 5 minutes stable: 5 * 60 * 1000, // 5 minutes volatile: 60 * 1000, // 60 seconds "no-cache": 0, }; /** - * URL patterns grouped by TTL tier. + * Path patterns for the volatile tier. * - * Checked in tier priority order (no-cache → immutable → volatile). - * "stable" has no patterns — it is the default fallback when nothing else matches. + * Checked before `detail`, so a URL that matches both is cached for the + * shorter time. */ -const URL_TIER_REGEXPS: Readonly> = { - // Polling endpoints where state changes rapidly - "no-cache": [/\/(?:autofix|root-cause)\//], - // Specific resources by ID (events, traces, span details) — never change once created - immutable: [ - /\/events\/[^/?]+\/?(?:\?|$)/, - /\/trace\/[0-9a-f]{32}\//, - /\/trace-items\/[0-9a-f]+\//, - ], - // Issue endpoints (lists AND detail views), dataset queries, trace-logs - volatile: [/\/issues\//, /[?&]dataset=(?:logs|spans)/, /\/trace-logs\//], - // Default fallback — no patterns needed - stable: [], -}; +const VOLATILE_PATH_REGEXPS: readonly RegExp[] = [ + // Issue endpoints (lists, detail views, and issue events such as + // `/issues/{id}/events/latest/`) + /\/issues\//, + // Trace-scoped logs + /\/trace-logs\//, + // Full traces and their sub-resources; spans keep arriving + /\/trace\/[0-9a-f]{32}\//, + // Project event details embed `nextEventID`/`previousEventID` and user + // reports, and reprocessing can change them for the same ID + /\/projects\/[^/]+\/[^/]+\/events\/[^/]+\/$/, +]; + +/** Query parameters that make a request volatile regardless of path. */ +const VOLATILE_QUERY_REGEXP = /[?&]dataset=(?:logs|spans)/; -/** Tier check order — stable is the default and has no patterns to check. */ -const TIER_CHECK_ORDER: readonly TtlTier[] = [ - "no-cache", - "immutable", - "volatile", +/** + * Exact path shapes for the detail tier, with the query parameters that + * disqualify them. + */ +const DETAIL_ROUTES: readonly { + path: RegExp; + excludedParams: readonly string[]; +}[] = [ + { + // OrganizationEventDetailsEndpoint: `{project}:{event_id}`. + // `averageColumn` adds live aggregates to the response. + path: /\/organizations\/[^/]+\/events\/[^/:]+:[^/]+\/$/, + excludedParams: ["averageColumn"], + }, + { + // ProjectTraceItemDetailsEndpoint. `debug` adds request-time internals. + path: /\/projects\/[^/]+\/[^/]+\/trace-items\/[^/]+\/$/, + excludedParams: ["debug"], + }, ]; +/** Polling endpoints where state changes rapidly. */ +const NO_CACHE_REGEXP = /\/(?:autofix|root-cause)\//; + +/** Base for parsing relative URLs; only the path and query are used. */ +const RELATIVE_URL_BASE = "http://localhost"; + /** * Classify a URL into a TTL tier for fallback caching. * @@ -96,11 +126,25 @@ const TIER_CHECK_ORDER: readonly TtlTier[] = [ * @internal Exported for testing */ export function classifyUrl(url: string): TtlTier { - for (const tier of TIER_CHECK_ORDER) { - for (const pattern of URL_TIER_REGEXPS[tier]) { - if (pattern.test(url)) { - return tier; - } + if (NO_CACHE_REGEXP.test(url)) { + return "no-cache"; + } + if (VOLATILE_QUERY_REGEXP.test(url)) { + return "volatile"; + } + if (!URL.canParse(url, RELATIVE_URL_BASE)) { + return "volatile"; + } + const { pathname, searchParams } = new URL(url, RELATIVE_URL_BASE); + if (VOLATILE_PATH_REGEXPS.some((pattern) => pattern.test(pathname))) { + return "volatile"; + } + for (const route of DETAIL_ROUTES) { + if ( + route.path.test(pathname) && + !route.excludedParams.some((name) => searchParams.has(name)) + ) { + return "detail"; } } return "stable"; @@ -189,7 +233,6 @@ type CacheEntry = { const POLICY_OPTIONS: CachePolicy.Options = { shared: false, cacheHeuristic: 0.1, - immutableMinTimeToLive: FALLBACK_TTL_MS.immutable, }; /** Maximum number of cache files to retain */ diff --git a/packages/cli/test/lib/response-cache.property.test.ts b/packages/cli/test/lib/response-cache.property.test.ts index 6f605c63c..0346a66a2 100644 --- a/packages/cli/test/lib/response-cache.property.test.ts +++ b/packages/cli/test/lib/response-cache.property.test.ts @@ -164,32 +164,70 @@ describe("property: classifyUrl", () => { fcAssert( property(sentryUrlArb, (url) => { const tier = classifyUrl(url); - expect(["immutable", "stable", "volatile", "no-cache"]).toContain(tier); + expect(["detail", "stable", "volatile", "no-cache"]).toContain(tier); }), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("event detail URLs are immutable", () => { + test("organization event details are detail", () => { + const urls = [ + "https://us.sentry.io/api/0/organizations/myorg/events/myproject:abc123/", + "https://sentry.io/api/0/organizations/org/events/42:deadbeef/?field=title", + ]; + for (const url of urls) { + expect(classifyUrl(url)).toBe("detail"); + } + }); + + test("organization event details with averageColumn are stable", () => { + const url = + "https://us.sentry.io/api/0/organizations/org/events/proj:abc123/?averageColumn=span.duration"; + expect(classifyUrl(url)).toBe("stable"); + }); + + test("project trace-item details are detail", () => { + const url = + "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?trace_id=abc&item_type=spans"; + expect(classifyUrl(url)).toBe("detail"); + }); + + test("project trace-item details with debug are stable", () => { + const url = + "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?item_type=spans&debug=1"; + expect(classifyUrl(url)).toBe("stable"); + }); + + test("project event details are volatile", () => { const urls = [ "https://us.sentry.io/api/0/projects/myorg/myproject/events/abc123/", "https://sentry.io/api/0/projects/org/proj/events/deadbeef/?full=true", ]; for (const url of urls) { - expect(classifyUrl(url)).toBe("immutable"); + expect(classifyUrl(url)).toBe("volatile"); + } + }); + + test("issue events are volatile, including latest/oldest/recommended", () => { + for (const eventId of ["latest", "oldest", "recommended", "deadbeef"]) { + const url = `https://us.sentry.io/api/0/organizations/org/issues/123/events/${eventId}/`; + expect(classifyUrl(url)).toBe("volatile"); } }); - test("trace URLs with 32-char hex IDs are immutable", () => { + test("full trace URLs with 32-char hex IDs are volatile", () => { const traceId = "a".repeat(32); - const url = `https://us.sentry.io/api/0/organizations/myorg/trace/${traceId}/`; - expect(classifyUrl(url)).toBe("immutable"); + const url = `https://us.sentry.io/api/0/organizations/myorg/trace/${traceId}/?limit=100`; + expect(classifyUrl(url)).toBe("volatile"); }); - test("trace-items (span detail) URLs are immutable", () => { - const url = - "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?trace_id=abc&item_type=spans"; - expect(classifyUrl(url)).toBe("immutable"); + test("relative URLs are classified by path", () => { + expect(classifyUrl("/api/0/organizations/org/events/proj:abc/")).toBe( + "detail", + ); + expect( + classifyUrl("/api/0/organizations/org/issues/1/events/latest/"), + ).toBe("volatile"); }); test("issue URLs are volatile (lists and detail views)", () => { diff --git a/packages/cli/test/lib/response-cache.test.ts b/packages/cli/test/lib/response-cache.test.ts index 24c0a8369..23022c861 100644 --- a/packages/cli/test/lib/response-cache.test.ts +++ b/packages/cli/test/lib/response-cache.test.ts @@ -7,7 +7,7 @@ import { readdir } from "node:fs/promises"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { setAuthToken } from "../../src/lib/db/auth.js"; import { buildCacheKey, @@ -452,6 +452,67 @@ describe("no-cache tier", () => { }); }); +// --------------------------------------------------------------------------- +// Fallback TTL tiers +// --------------------------------------------------------------------------- + +describe("fallback TTL tiers", () => { + const MINUTE = 60 * 1000; + + beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2026-01-01T00:00:00Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + async function isCachedAfter(url: string, elapsedMs: number) { + await storeCachedResponse(TEST_METHOD, url, {}, mockResponse(TEST_BODY)); + vi.setSystemTime(Date.now() + elapsedMs); + return (await getCachedResponse(TEST_METHOD, url, {})) !== undefined; + } + + test("organization event details are reused for up to 5 minutes", async () => { + const url = + "https://us.sentry.io/api/0/organizations/myorg/events/proj:abc123/"; + expect(await isCachedAfter(url, 4 * MINUTE)).toBe(true); + vi.setSystemTime(Date.now() + 2 * MINUTE); + expect(await getCachedResponse(TEST_METHOD, url, {})).toBeUndefined(); + }); + + test("an issue's latest event expires after 60 seconds", async () => { + const url = + "https://us.sentry.io/api/0/organizations/myorg/issues/123/events/latest/"; + expect(await isCachedAfter(url, 2 * MINUTE)).toBe(false); + }); + + test("project event details expire after 60 seconds", async () => { + const url = + "https://us.sentry.io/api/0/projects/myorg/myproject/events/abc123/"; + expect(await isCachedAfter(url, 2 * MINUTE)).toBe(false); + }); + + test("full traces expire after 60 seconds", async () => { + const url = `https://us.sentry.io/api/0/organizations/myorg/trace/${"a".repeat(32)}/`; + expect(await isCachedAfter(url, 2 * MINUTE)).toBe(false); + }); + + test("server max-age still wins over the fallback tier", async () => { + const url = + "https://us.sentry.io/api/0/organizations/myorg/issues/123/events/latest/"; + await storeCachedResponse( + TEST_METHOD, + url, + {}, + mockResponse(TEST_BODY, 200, { "cache-control": "private, max-age=300" }), + ); + vi.setSystemTime(Date.now() + 4 * MINUTE); + expect(await getCachedResponse(TEST_METHOD, url, {})).toBeDefined(); + }); +}); + // --------------------------------------------------------------------------- // File structure // --------------------------------------------------------------------------- From 31c5de5aaaa5dcc0de91c23c7f59f2a55fa1d97d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:53:43 +0000 Subject: [PATCH 2/2] ref(cli): Fold the detail cache tier into stable `detail` had the same 5-minute TTL as `stable` and only existed to mark future R2 eligibility on the hosted MCP, which belongs to that store's own allowlist. Dropping it removes the route table and URL parsing; organization event details and trace-item details now fall through to `stable`. Add a TODO to classify by generated `@sentry/api` operation metadata (getsentry/sentry-api-schema#81, #102) instead of URL regexes. --- packages/cli/src/lib/response-cache.ts | 106 ++++++------------ .../test/lib/response-cache.property.test.ts | 34 +----- 2 files changed, 40 insertions(+), 100 deletions(-) diff --git a/packages/cli/src/lib/response-cache.ts b/packages/cli/src/lib/response-cache.ts index 9bef2d49d..2ada16ce9 100644 --- a/packages/cli/src/lib/response-cache.ts +++ b/packages/cli/src/lib/response-cache.ts @@ -45,11 +45,8 @@ const log = logger.withTag("response-cache"); /** * TTL tier classification for URLs. * - * - `detail`: single event or trace-item lookups whose response does not - * change once written (organization event details, project trace-item - * details). Still bounded: deletion and access revocation only take - * effect when the entry expires. - * - `stable`: data that changes infrequently (orgs, projects, teams) + * - `stable`: data that changes infrequently (orgs, projects, teams, + * organization event details, trace-item details) * - `volatile`: data that changes often (issues and their events, project * event details, full traces, log and span queries) * - `no-cache`: never cache (polling endpoints like autofix state) @@ -59,64 +56,47 @@ const log = logger.withTag("response-cache"); * `recommended` resolve to different events over time, and traces gain * spans as they arrive (getsentry/sentry#128818). */ -type TtlTier = "detail" | "stable" | "volatile" | "no-cache"; +type TtlTier = "stable" | "volatile" | "no-cache"; /** Fallback TTL durations by tier (milliseconds). `no-cache` uses 0 as a sentinel. */ const FALLBACK_TTL_MS: Record = { - detail: 5 * 60 * 1000, // 5 minutes stable: 5 * 60 * 1000, // 5 minutes volatile: 60 * 1000, // 60 seconds "no-cache": 0, }; /** - * Path patterns for the volatile tier. + * URL patterns grouped by TTL tier. * - * Checked before `detail`, so a URL that matches both is cached for the - * shorter time. + * Checked in tier priority order (no-cache → volatile). + * "stable" has no patterns — it is the default fallback when nothing else matches. + * + * TODO: Classify by generated operation metadata from `@sentry/api` instead + * of URL regexes, once it ships per-operation metadata + * (getsentry/sentry-api-schema#81) and a route-keyed operation map + * (getsentry/sentry-api-schema#102). */ -const VOLATILE_PATH_REGEXPS: readonly RegExp[] = [ - // Issue endpoints (lists, detail views, and issue events such as - // `/issues/{id}/events/latest/`) - /\/issues\//, - // Trace-scoped logs - /\/trace-logs\//, - // Full traces and their sub-resources; spans keep arriving - /\/trace\/[0-9a-f]{32}\//, - // Project event details embed `nextEventID`/`previousEventID` and user - // reports, and reprocessing can change them for the same ID - /\/projects\/[^/]+\/[^/]+\/events\/[^/]+\/$/, -]; - -/** Query parameters that make a request volatile regardless of path. */ -const VOLATILE_QUERY_REGEXP = /[?&]dataset=(?:logs|spans)/; +const URL_TIER_REGEXPS: Readonly> = { + // Polling endpoints where state changes rapidly + "no-cache": [/\/(?:autofix|root-cause)\//], + volatile: [ + // Issue endpoints: lists, detail views, and issue events such as + // `/issues/{id}/events/latest/` + /\/issues\//, + /[?&]dataset=(?:logs|spans)/, + /\/trace-logs\//, + // Full traces gain spans as they arrive + /\/trace\/[0-9a-f]{32}\//, + // Project event details embed `nextEventID`/`previousEventID` and user + // reports, and reprocessing can change them for the same ID + /\/projects\/[^/]+\/[^/]+\/events\/[^/?]+\/?(?:\?|$)/, + ], + // Default fallback — no patterns needed + stable: [], +}; -/** - * Exact path shapes for the detail tier, with the query parameters that - * disqualify them. - */ -const DETAIL_ROUTES: readonly { - path: RegExp; - excludedParams: readonly string[]; -}[] = [ - { - // OrganizationEventDetailsEndpoint: `{project}:{event_id}`. - // `averageColumn` adds live aggregates to the response. - path: /\/organizations\/[^/]+\/events\/[^/:]+:[^/]+\/$/, - excludedParams: ["averageColumn"], - }, - { - // ProjectTraceItemDetailsEndpoint. `debug` adds request-time internals. - path: /\/projects\/[^/]+\/[^/]+\/trace-items\/[^/]+\/$/, - excludedParams: ["debug"], - }, -]; - -/** Polling endpoints where state changes rapidly. */ -const NO_CACHE_REGEXP = /\/(?:autofix|root-cause)\//; - -/** Base for parsing relative URLs; only the path and query are used. */ -const RELATIVE_URL_BASE = "http://localhost"; +/** Tier check order — stable is the default and has no patterns to check. */ +const TIER_CHECK_ORDER: readonly TtlTier[] = ["no-cache", "volatile"]; /** * Classify a URL into a TTL tier for fallback caching. @@ -126,25 +106,11 @@ const RELATIVE_URL_BASE = "http://localhost"; * @internal Exported for testing */ export function classifyUrl(url: string): TtlTier { - if (NO_CACHE_REGEXP.test(url)) { - return "no-cache"; - } - if (VOLATILE_QUERY_REGEXP.test(url)) { - return "volatile"; - } - if (!URL.canParse(url, RELATIVE_URL_BASE)) { - return "volatile"; - } - const { pathname, searchParams } = new URL(url, RELATIVE_URL_BASE); - if (VOLATILE_PATH_REGEXPS.some((pattern) => pattern.test(pathname))) { - return "volatile"; - } - for (const route of DETAIL_ROUTES) { - if ( - route.path.test(pathname) && - !route.excludedParams.some((name) => searchParams.has(name)) - ) { - return "detail"; + for (const tier of TIER_CHECK_ORDER) { + for (const pattern of URL_TIER_REGEXPS[tier]) { + if (pattern.test(url)) { + return tier; + } } } return "stable"; diff --git a/packages/cli/test/lib/response-cache.property.test.ts b/packages/cli/test/lib/response-cache.property.test.ts index 0346a66a2..c649b35a4 100644 --- a/packages/cli/test/lib/response-cache.property.test.ts +++ b/packages/cli/test/lib/response-cache.property.test.ts @@ -164,40 +164,23 @@ describe("property: classifyUrl", () => { fcAssert( property(sentryUrlArb, (url) => { const tier = classifyUrl(url); - expect(["detail", "stable", "volatile", "no-cache"]).toContain(tier); + expect(["stable", "volatile", "no-cache"]).toContain(tier); }), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("organization event details are detail", () => { + test("organization event and trace-item details are stable", () => { const urls = [ "https://us.sentry.io/api/0/organizations/myorg/events/myproject:abc123/", "https://sentry.io/api/0/organizations/org/events/42:deadbeef/?field=title", + "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?trace_id=abc&item_type=spans", ]; for (const url of urls) { - expect(classifyUrl(url)).toBe("detail"); + expect(classifyUrl(url)).toBe("stable"); } }); - test("organization event details with averageColumn are stable", () => { - const url = - "https://us.sentry.io/api/0/organizations/org/events/proj:abc123/?averageColumn=span.duration"; - expect(classifyUrl(url)).toBe("stable"); - }); - - test("project trace-item details are detail", () => { - const url = - "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?trace_id=abc&item_type=spans"; - expect(classifyUrl(url)).toBe("detail"); - }); - - test("project trace-item details with debug are stable", () => { - const url = - "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?item_type=spans&debug=1"; - expect(classifyUrl(url)).toBe("stable"); - }); - test("project event details are volatile", () => { const urls = [ "https://us.sentry.io/api/0/projects/myorg/myproject/events/abc123/", @@ -221,15 +204,6 @@ describe("property: classifyUrl", () => { expect(classifyUrl(url)).toBe("volatile"); }); - test("relative URLs are classified by path", () => { - expect(classifyUrl("/api/0/organizations/org/events/proj:abc/")).toBe( - "detail", - ); - expect( - classifyUrl("/api/0/organizations/org/issues/1/events/latest/"), - ).toBe("volatile"); - }); - test("issue URLs are volatile (lists and detail views)", () => { const urls = [ "https://us.sentry.io/api/0/projects/org/proj/issues/",