From febc8e9124c452ef634f8a8c29f52060af6225d8 Mon Sep 17 00:00:00 2001 From: codingbo Date: Sat, 26 Sep 2026 22:57:46 +0800 Subject: [PATCH] feat(prompt): snapshot skills catalog per session to preserve prompt cache Closes #5569 --- .../src/content/docs/guides/codex-prompt.md | 29 +++ .../docs/reference/configuration/agents.md | 9 + scripts/test-layout/layout.json | 1 + src/config/diagnostics.ts | 15 +- src/config/schema/config-schema.ts | 2 + src/config/schema/leaf-validators.ts | 7 + src/server/responses/request-prepare.ts | 9 + src/server/responses/skills-snapshot.ts | 211 ++++++++++++++++++ src/types.ts | 2 + src/types/config.ts | 14 ++ structure/config.md | 6 +- structure/transports/responses.md | 2 +- .../config-skills-catalog-refresh.test.ts | 115 ++++++++++ tests/fixtures/test-layout-expected.json | 1 + tests/helpers/responses-core-source.ts | 1 + .../responses-skills-snapshot.test.ts | 113 ++++++++++ 16 files changed, 532 insertions(+), 5 deletions(-) create mode 100644 src/server/responses/skills-snapshot.ts create mode 100644 tests/config/config-skills-catalog-refresh.test.ts create mode 100644 tests/responses/responses-skills-snapshot.test.ts diff --git a/docs-site/src/content/docs/guides/codex-prompt.md b/docs-site/src/content/docs/guides/codex-prompt.md index df6c5a770f8..e31bf030cfd 100644 --- a/docs-site/src/content/docs/guides/codex-prompt.md +++ b/docs-site/src/content/docs/guides/codex-prompt.md @@ -169,6 +169,35 @@ repair writes a backup before it touches anything. Changes apply to newly started sessions. A session already running keeps the prompt settings it started with. +## Keeping the skills catalog stable + +The proxy defaults to `skills.catalog_refresh: "per_session"`: the first +`` catalog received for a conversation is reused on later +requests in that conversation. This keeps skill discovery and `SKILL.md` edits +from changing that part of the upstream prompt cache prefix mid-session. + +To use the catalog supplied by the client on every turn, set this in opencodex's +`$OPENCODEX_HOME/config.json` (normally `~/.opencodex/config.json`), then restart +the proxy: + +```json +{ + "skills": { + "catalog_refresh": "per_turn" + } +} +``` + +The supported values are `"per_session"` (default) and `"per_turn"`. This is a +proxy setting, separate from Codex's `skills.include_instructions` toggle. +Requests without a reliable conversation identity use the catalog supplied by +the client. Snapshots are held in memory and do not survive a proxy restart. +They expire after four hours of inactivity and may be evicted when the bounded +cache fills. An initial catalog block larger than 512 KiB is forwarded without caching. +After expiry or eviction, the next received catalog becomes the new snapshot. +The dashboard's prompt preview still reads the current files; it does not show +the snapshot retained for an ongoing conversation. + ## What this page reads, and what it does not opencodex reads one configuration file — your `config.toml`. Codex resolves its diff --git a/docs-site/src/content/docs/reference/configuration/agents.md b/docs-site/src/content/docs/reference/configuration/agents.md index ac968ee7073..20165746900 100644 --- a/docs-site/src/content/docs/reference/configuration/agents.md +++ b/docs-site/src/content/docs/reference/configuration/agents.md @@ -8,6 +8,15 @@ routes, and limits delegated work. ## Agent fields +### Skills catalog refresh + +`skills.catalog_refresh` accepts `"per_session"` (the default) or `"per_turn"` +in opencodex's `config.json`. Session mode reuses the first received skills +instructions for a conversation, protecting the prompt cache prefix from catalog +changes between turns. Turn mode forwards the client's current catalog. +See [Keeping the skills catalog stable](/guides/codex-prompt/#keeping-the-skills-catalog-stable) +for configuration and snapshot lifetime details. + ### Astra roster upgrade On the first start after upgrading, existing `subagentModels` lists receive diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 404f6269668..db013aac75c 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -168,6 +168,7 @@ "responses-compaction-recovery-policy.test.ts": "responses", "deepseek-artifact-tool-schema.test.ts": "providers", "client-config-export-output-limit.test.ts": "config", + "responses-skills-snapshot.test.ts": "responses", "openai-chat-serialized-tool-call-scaling.test.ts": "adapters/openai", "openai-chat-tool-call-id-remint.test.ts": "adapters/openai", "coding-agent-json-lines-scaling.test.ts": "providers", diff --git a/src/config/diagnostics.ts b/src/config/diagnostics.ts index ca7b899e3d6..d1ab81747c4 100644 --- a/src/config/diagnostics.ts +++ b/src/config/diagnostics.ts @@ -63,6 +63,7 @@ import { runtimeRoleSchema, spendSchema, compactionRoutingSchema, + skillsConfigSchema, } from "./schema/leaf-validators"; export type ConfigDiagnostics = { @@ -594,6 +595,17 @@ export function metricsExportConfigError(value: unknown): string | null { return null; } + +function skillsConfigError(value: unknown): string | null { + const raw = rawConfigRecord(value); + if (!raw || !Object.hasOwn(raw, "skills") || raw.skills === undefined) return null; + const result = skillsConfigSchema.safeParse(raw.skills); + if (result.success) return null; + const issue = result.error.issues[0]; + const field = issue?.path.join("."); + return "schema_invalid: skills" + (field ? "." + field : "") + ": " + (issue?.message ?? "invalid configuration"); +} + export function validateConfigCandidate(value: unknown): { ok: true; config: OcxConfig } | { ok: false; error: string } { const compactionRouting = rawConfigRecord(value)?.compactionRouting; if (compactionRouting !== undefined && !compactionRoutingSchema.safeParse(compactionRouting).success) { @@ -625,7 +637,8 @@ export function validateConfigCandidate(value: unknown): { ok: true; config: Ocx ?? clientRolePairError(value) ?? loopbackListenerPortError(value) ?? managementIngressConfigError(value) - ?? metricsExportConfigError(value); + ?? metricsExportConfigError(value) + ?? skillsConfigError(value); if (boundaryError) return { ok: false, error: boundaryError }; const result = configSchema.safeParse(value); if (result.success) { diff --git a/src/config/schema/config-schema.ts b/src/config/schema/config-schema.ts index 48460d3667e..a855202a29a 100644 --- a/src/config/schema/config-schema.ts +++ b/src/config/schema/config-schema.ts @@ -16,6 +16,7 @@ import { remoteGuiConfigSchema, runtimeRoleSchema, spendSchema, + skillsConfigSchema, configuredCodexPoolAccountIds, apiKeyEntrySchema, asideProfileSyncSchema, @@ -78,6 +79,7 @@ export const configSchema = z.object({ // A malformed privacy block must never be read as "unmask": .catch(undefined) drops it and // emailMaskingEnabled then falls back to masked, which is also what an absent block means. privacy: z.object({ maskEmails: z.boolean().optional() }).strict().optional().catch(undefined), + skills: skillsConfigSchema.optional().catch(undefined), // Malformed hand edits disable this opt-in exporter. Live writes reject them in diagnostics.ts. metricsExport: z.object({ enabled: z.boolean().optional() }).strict().optional().catch(undefined), // Kept raw on purpose: `.catch(undefined)` would turn a mistyped `enabled` into "inherit", diff --git a/src/config/schema/leaf-validators.ts b/src/config/schema/leaf-validators.ts index 449fd54ce13..8de6060f5b2 100644 --- a/src/config/schema/leaf-validators.ts +++ b/src/config/schema/leaf-validators.ts @@ -1055,3 +1055,10 @@ export const spendSchema = z.object({ pool: spendScopeSchema.optional(), retentionDays: z.number().int().min(1).max(365).optional(), }).strict(); + +/** + * Runtime skills catalog configuration (#5569). + */ +export const skillsConfigSchema = z.object({ + catalog_refresh: z.enum(["per_session", "per_turn"]).optional(), +}).strict(); diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 66369040eb3..9edb8630eb3 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -22,6 +22,7 @@ import { reasoningReplayConversationIdFromResponsesRequest, } from "../request-log-conversation"; import { resolveContextPrincipal } from "../auth-cors"; +import { resolveSkillsSnapshotScopeKey, snapshotSkillsCatalogInBody } from "./skills-snapshot"; import { isShadowSourceModel, shadowSourceModelPrefix, @@ -314,6 +315,14 @@ export async function prepareResponsesRequest( ); } + const skillsSnapshotScopeKey = resolveSkillsSnapshotScopeKey({ + req, + config, + admission: options.admission, + promptCacheKeyIsSharedCohort: options.promptCacheKeyIsSharedCohort, + }); + snapshotSkillsCatalogInBody(body, skillsSnapshotScopeKey, config); + let parsed: OcxParsedRequest; let toolBridgeMaps: ReturnType; try { diff --git a/src/server/responses/skills-snapshot.ts b/src/server/responses/skills-snapshot.ts new file mode 100644 index 00000000000..b6c7b057405 --- /dev/null +++ b/src/server/responses/skills-snapshot.ts @@ -0,0 +1,211 @@ +/** + * runtime skills catalog session snapshotting (#5569). + * + * Preserves the Anthropic/LLM prompt cache prefix across turns by freezing + * incoming for the duration of a trustworthy session. + * Gated by config `skills.catalog_refresh`: "per_session" (default) or "per_turn". + * + * Lifecycle & Bounds: + * - 4 hours idle TTL (sliding on access) + * - 1,000 maximum tracked sessions (LRU eviction) + * - 512 KiB maximum per snapshotted skills block + * - 8 MiB global retained byte bound across all sessions + */ +import type { OcxConfig, SkillsCatalogRefresh } from "../../types/config"; +import { resolveContextPrincipal, type DataPlaneAdmission } from "../auth-cors"; +import { + reasoningReplayConversationIdFromResponsesRequest, + sessionIdHeaderFromRequest, +} from "../request-log-conversation"; + +const SKILLS_BLOCK_GLOBAL_REGEX = /([\s\S]*?)<\/skills_instructions>/g; + +/** Maximum distinct sessions tracked in the memory LRU. */ +export const MAX_SNAPSHOT_SESSIONS = 1000; +/** Slide expiry after 4 hours of inactivity. */ +export const SNAPSHOT_TTL_MS = 4 * 60 * 60 * 1000; +/** Bounded byte ceiling per snapshotted skills block (512 KiB). */ +export const MAX_SKILLS_BLOCK_BYTES = 512 * 1024; +/** Global retained byte bound across all tracked sessions (8 MiB). */ +export const MAX_TOTAL_RETAINED_BYTES = 8 * 1024 * 1024; + +interface SnapshotEntry { + skillsBlock: string; // The full ... block + byteLength: number; + lastAccessed: number; +} + +const snapshotCache = new Map(); +let totalRetainedBytes = 0; + +function evictOldestEntry(): boolean { + const oldest = snapshotCache.entries().next().value; + if (!oldest) return false; + const [key, entry] = oldest; + totalRetainedBytes -= entry.byteLength; + snapshotCache.delete(key); + return true; +} + +export function resolveSkillsCatalogRefresh(config: OcxConfig | undefined): SkillsCatalogRefresh { + const configured = config?.skills?.catalog_refresh; + if (configured === "per_turn") return "per_turn"; + return "per_session"; +} + +export interface ResolveSkillsSessionScopeInput { + req: Request; + config: OcxConfig; + admission?: DataPlaneAdmission; + cursorConversationId?: string; + promptCacheKeyIsSharedCohort?: boolean; +} + +/** + * Resolves a trustworthy cache key for skills catalog snapshotting. + * Returns null if no specific, reliable thread/session identity is available, + * or if the identity comes from a shared cohort fallback. + */ +export function resolveSkillsSnapshotScopeKey(input: ResolveSkillsSessionScopeInput): string | null { + if (input.promptCacheKeyIsSharedCohort === true) { + return null; + } + + const parentThread = input.req.headers.get("x-codex-parent-thread-id")?.trim() || undefined; + const ownThreadId = input.req.headers.get("thread-id")?.trim() || undefined; + const cursorId = input.cursorConversationId?.trim() || undefined; + + // When a parent thread is present, subagents/children may share a root session-id header. + // To prevent cross-thread/sibling collapse or parent-level caching, require an explicit + // own thread-id (or cursor id). If parentThread is present without an own child thread, + // bypass snapshotting completely. + if (parentThread) { + const childId = ownThreadId ?? cursorId; + if (!childId || childId === parentThread) { + return null; + } + const qualifiedId = `${parentThread}\u0000${childId}`; + const principal = resolveContextPrincipal(input.req, input.config, input.admission) ?? null; + return JSON.stringify(["skills_catalog_snapshot_v1", principal, qualifiedId]); + } + + // Standalone conversation (no parent thread) + const standaloneId = reasoningReplayConversationIdFromResponsesRequest({ + threadIdHeader: ownThreadId, + cursorConversationId: cursorId, + sessionIdHeader: sessionIdHeaderFromRequest(input.req.headers), + }); + if (!standaloneId) { + return null; + } + const principal = resolveContextPrincipal(input.req, input.config, input.admission) ?? null; + return JSON.stringify(["skills_catalog_snapshot_v1", principal, standaloneId]); +} + +function snapshotOrReplaceInText( + text: string, + scopeKey: string, + now: number, +): string { + if (!text.includes("")) return text; + + return text.replace(SKILLS_BLOCK_GLOBAL_REGEX, (match) => { + const existing = snapshotCache.get(scopeKey); + if (existing) { + // Check TTL on cache hits + if (now - existing.lastAccessed > SNAPSHOT_TTL_MS) { + totalRetainedBytes -= existing.byteLength; + snapshotCache.delete(scopeKey); + } else { + existing.lastAccessed = now; + // Refresh Map order for true LRU behavior + snapshotCache.delete(scopeKey); + snapshotCache.set(scopeKey, existing); + return existing.skillsBlock; + } + } + + // First turn or expired: snapshot incoming block if bounded + const incomingBlock = match; + const blockBytes = Buffer.byteLength(incomingBlock, "utf8"); + if (blockBytes <= MAX_SKILLS_BLOCK_BYTES && blockBytes <= MAX_TOTAL_RETAINED_BYTES) { + // Evict oldest entries until under count ceiling AND under global byte ceiling + while ( + (snapshotCache.size >= MAX_SNAPSHOT_SESSIONS || totalRetainedBytes + blockBytes > MAX_TOTAL_RETAINED_BYTES) + && snapshotCache.size > 0 + ) { + if (!evictOldestEntry()) break; + } + + if (totalRetainedBytes + blockBytes <= MAX_TOTAL_RETAINED_BYTES) { + snapshotCache.set(scopeKey, { + skillsBlock: incomingBlock, + byteLength: blockBytes, + lastAccessed: now, + }); + totalRetainedBytes += blockBytes; + } + } + return match; + }); +} + +/** + * Transforms incoming developer/system prompt contents to reuse the session's + * snapshotted , preserving prefix cache across turns. + * User and assistant messages, as well as tool calls, are never modified. + */ +export function snapshotSkillsCatalogInBody( + body: unknown, + scopeKey: string | null, + config: OcxConfig, + now: number = Date.now(), +): void { + if (!scopeKey) return; + if (resolveSkillsCatalogRefresh(config) === "per_turn") return; + if (!body || typeof body !== "object" || Array.isArray(body)) return; + + const b = body as Record; + + // 1. Check top-level instructions field + if (typeof b.instructions === "string" && b.instructions.includes("")) { + b.instructions = snapshotOrReplaceInText(b.instructions, scopeKey, now); + } + + // 2. Check input array for developer/system messages only + if (Array.isArray(b.input)) { + for (const item of b.input) { + if (!item || typeof item !== "object") continue; + const it = item as Record; + // Restrict message item type: must be undefined or "message", so role-like tool objects are untouched + if (it.type !== undefined && it.type !== "message") continue; + const role = it.role; + // Only developer and system content is inspected/transformed + if (role !== "developer" && role !== "system") continue; + + const content = it.content; + if (typeof content === "string") { + if (content.includes("")) { + it.content = snapshotOrReplaceInText(content, scopeKey, now); + } + } else if (Array.isArray(content)) { + for (const part of content) { + if (!part || typeof part !== "object") continue; + const p = part as Record; + // Restrict text parts to known text / input_text + if (p.type !== "text" && p.type !== "input_text") continue; + if (typeof p.text === "string" && p.text.includes("")) { + p.text = snapshotOrReplaceInText(p.text, scopeKey, now); + } + } + } + } + } +} + +/** Test helpers */ +export function resetSkillsSnapshotCacheForTests(): void { + snapshotCache.clear(); + totalRetainedBytes = 0; +} + diff --git a/src/types.ts b/src/types.ts index ff961d813e4..5281666884d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -77,6 +77,8 @@ export type { OcxConnectedClientId, OcxClientConnectionConfig, OcxConfig, + SkillsCatalogRefresh, + OcxSkillsConfig, OcxAccountPoolRotationStrategy, OcxAccountPoolQuotaWindow, OcxComboCooldownWaitPolicy, diff --git a/src/types/config.ts b/src/types/config.ts index 6e7fa9a48ac..940aa2b06fd 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -362,6 +362,18 @@ export interface OcxConfigRebaseProvenance { deletedTopLevelKeys: string[]; } + +export type SkillsCatalogRefresh = "per_session" | "per_turn"; + +export interface OcxSkillsConfig { + /** + * Refresh policy for the runtime skills catalog (#5569). + * `per_session` (default): snapshots incoming `` on the first turn of a trustworthy session and reuses it across turns to preserve the Anthropic prompt cache. + * `per_turn`: re-derives/passes through incoming skills instructions every turn (previous behavior). + */ + catalog_refresh?: SkillsCatalogRefresh; +} + export type OcxRuntimeRole = "standalone" | "hub" | "client"; export interface OcxHubConfig { @@ -487,6 +499,8 @@ export interface OcxConfig { client?: OcxClientConnectionConfig; /** Operator-facing redaction policy for management and CLI projections. */ privacy?: OcxPrivacyConfig; + /** Runtime skills catalog session snapshotting settings (#5569). */ + skills?: OcxSkillsConfig; /** Opt-in process-local aggregate request metrics on the authenticated management plane. */ metricsExport?: { enabled?: boolean }; /** Opt in to one identical-turn retry when a Responses completion has no text or tool call. */ diff --git a/structure/config.md b/structure/config.md index 98490672149..32037aaaf47 100644 --- a/structure/config.md +++ b/structure/config.md @@ -28,10 +28,10 @@ the [source-owned credential contract](codex-home.md#orca-source-owned-account-i `src/config/schema/compaction-recovery.ts` strictly validates opt-in `compactionRecovery`; invalid disk values disable it with a warning, while candidate writes reject them. The [failure-only contract](transports/responses-failover.md) leaves provider identity, accounts and client compaction unchanged. +`skills.catalog_refresh` in the proxy JSON configuration accepts `per_session` (the runtime default when absent) or `per_turn`. The former retains received skills instructions for a conversation; the latter passes through the current catalog. This is separate from Codex's `skills.include_instructions` TOML switch and does not change the live dashboard probe. See the [Responses snapshot contract](transports/responses.md#responses-httpsse). + Google providers may persist `googleToolSchemaPolicy` as `compatible` or `reject-lossy`. -`ocx provider add --google-tool-schema-policy` is one authoring path and is accepted only when the -effective adapter is `google`. Omission remains absent in `config.json`; the adapter resolves it to -`compatible` in memory. +`ocx provider add --google-tool-schema-policy` is one authoring path and is accepted only when the effective adapter is `google`. Omission remains absent in `config.json`; the adapter resolves it to `compatible` in memory. ### OpenCodex home and live process state diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 8320d296bac..2ecf589bb77 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -11,7 +11,7 @@ Plaintext collaboration restoration treats a null namespace as absent, rejects n When a successful streamed native response has a missing or unrecognized non-JSON content type, the plaintext V2 path confirms a bounded Responses SSE prefix, under the server's `stallTimeoutSec` probe budget, before applying that restoration; an `application/json` body takes the bounded JSON path instead, and an unknown, stalled, or unreadable body retains the fail-closed response. ## Responses HTTP/SSE - +Responses request preparation stabilizes incoming `` under `skills.catalog_refresh`: `per_session` (default) reuses the first received catalog for a conversation; `per_turn` leaves the supplied catalog unchanged. Other instruction sections and user/tool content remain untouched. Requests without a reliable conversation identity bypass snapshots; shared prompt-cache cohorts are not conversation identities. Snapshots are process-local, expire after four idle hours, and use bounded LRU retention; oversized blocks bypass caching. The dashboard's `src/codex/prompt-layers.ts` and `src/codex/prompt-text-probe.ts` continue observing current files for previews and do not own session snapshots. `/v1/responses` is the main Codex-facing endpoint. The server parses Responses input, routes to a provider, lets the selected adapter speak the upstream protocol, then bridges adapter events back to Responses-compatible streaming output. For an opted-in key-auth provider, a hosted-search continuation stays bound to the API-key selection that served the first leg; the contract is the [hosted-search continuation binding](../providers-and-adapters.md#hosted-search-continuation-binding). diff --git a/tests/config/config-skills-catalog-refresh.test.ts b/tests/config/config-skills-catalog-refresh.test.ts new file mode 100644 index 00000000000..7ddac6ce389 --- /dev/null +++ b/tests/config/config-skills-catalog-refresh.test.ts @@ -0,0 +1,115 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + getConfigPath, + getDefaultConfig, + loadConfig, + saveConfig, + validateConfigCandidate, +} from "../../src/config"; +import { resolveSkillsCatalogRefresh } from "../../src/server/responses/skills-snapshot"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +let home = ""; +let previousHome: string | undefined; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + home = mkdtempSync(join(tmpdir(), "ocx-skills-config-")); + process.env.OPENCODEX_HOME = home; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); +}); + +function candidate(skills: unknown) { + return { + ...getDefaultConfig(), + defaultProvider: "xai", + providers: { + xai: { + adapter: "openai-responses", + baseUrl: "https://api.x.ai/v1", + }, + }, + skills, + }; +} + +test("validateConfigCandidate accepts valid skills configuration", () => { + const c1 = validateConfigCandidate(candidate({ catalog_refresh: "per_session" })); + expect(c1.ok).toBe(true); + if (c1.ok) { + expect(c1.config.skills?.catalog_refresh).toBe("per_session"); + } + + const c2 = validateConfigCandidate(candidate({ catalog_refresh: "per_turn" })); + expect(c2.ok).toBe(true); + if (c2.ok) { + expect(c2.config.skills?.catalog_refresh).toBe("per_turn"); + } + + const c3 = validateConfigCandidate(candidate({})); + expect(c3.ok).toBe(true); + + const c4 = validateConfigCandidate(candidate(undefined)); + expect(c4.ok).toBe(true); +}); + +test("validateConfigCandidate explicitly rejects invalid skills configuration", () => { + const badValue = validateConfigCandidate(candidate({ catalog_refresh: "invalid_refresh" })); + expect(badValue.ok).toBe(false); + if (!badValue.ok) { + expect(badValue.error).toContain("schema_invalid: skills.catalog_refresh"); + } + + const extraProp = validateConfigCandidate(candidate({ catalog_refresh: "per_session", extra: 123 })); + expect(extraProp.ok).toBe(false); + if (!extraProp.ok) { + expect(extraProp.error).toContain("schema_invalid: skills"); + } + + const nonObject = validateConfigCandidate(candidate("per_session")); + expect(nonObject.ok).toBe(false); + if (!nonObject.ok) { + expect(nonObject.error).toContain("schema_invalid: skills"); + } +}); + +test("resolveSkillsCatalogRefresh defaults to per_session", () => { + expect(resolveSkillsCatalogRefresh(undefined)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({} as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: {} } as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: { catalog_refresh: "per_session" } } as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: { catalog_refresh: "per_turn" } } as any)).toBe("per_turn"); +}); + +test("skills configuration persists to disk and loads correctly", () => { + const cfg = { + ...getDefaultConfig(), + skills: { catalog_refresh: "per_turn" as const }, + }; + saveConfig(cfg); + + const loaded = loadConfig(); + expect(loaded.skills?.catalog_refresh).toBe("per_turn"); +}); + +test("malformed hand-edited skills in config file degrades gracefully on load", () => { + const configPath = getConfigPath(); + const raw = JSON.stringify({ + ...getDefaultConfig(), + skills: { catalog_refresh: "bad_value" }, + }); + writeFileSync(configPath, raw, "utf8"); + + const loaded = loadConfig(); + expect(loaded.skills).toBeUndefined(); + expect(resolveSkillsCatalogRefresh(loaded)).toBe("per_session"); +}); + diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index e8de7d32f2e..0872cc879f1 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -5,6 +5,7 @@ "responses-compaction-recovery-policy.test.ts": "responses", "deepseek-artifact-tool-schema.test.ts": "providers", "client-config-export-output-limit.test.ts": "config", + "responses-skills-snapshot.test.ts": "responses", "openai-chat-serialized-tool-call-scaling.test.ts": "adapters/openai", "openai-chat-tool-call-id-remint.test.ts": "adapters/openai", "coding-agent-json-lines-scaling.test.ts": "providers", diff --git a/tests/helpers/responses-core-source.ts b/tests/helpers/responses-core-source.ts index 7460b64ada9..f803be42c3e 100644 --- a/tests/helpers/responses-core-source.ts +++ b/tests/helpers/responses-core-source.ts @@ -30,6 +30,7 @@ export const RESPONSES_CORE_MODULES = [ "core-combo.ts", "core-combo-native.ts", "request-prepare.ts", + "skills-snapshot.ts", "shadow-target-availability.ts", "compaction-routing.ts", "compaction-recovery.ts", diff --git a/tests/responses/responses-skills-snapshot.test.ts b/tests/responses/responses-skills-snapshot.test.ts new file mode 100644 index 00000000000..b72b51efe2e --- /dev/null +++ b/tests/responses/responses-skills-snapshot.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { getDefaultConfig } from "../../src/config/proxy-env"; +import { handleResponses } from "../../src/server/responses/core"; +import { resetSkillsSnapshotCacheForTests } from "../../src/server/responses/skills-snapshot"; +import type { OcxConfig } from "../../src/types"; +import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; + +const originalFetch = globalThis.fetch; +let releaseSpendHome: (() => void) | undefined; +const captured: string[] = []; + +beforeEach(() => { + releaseSpendHome = acquireOwnedSpendHome(); + resetSkillsSnapshotCacheForTests(); + captured.length = 0; +}); +afterEach(() => { + releaseSpendHome?.(); + releaseSpendHome = undefined; + globalThis.fetch = originalFetch; + resetSkillsSnapshotCacheForTests(); +}); + +function fixture(adapter: "openai-responses" | "anthropic"): OcxConfig { + globalThis.fetch = (async (_input, init) => { + captured.push(String(init?.body)); + return Response.json(adapter === "anthropic" ? { + id: "msg_skills", type: "message", role: "assistant", model: "fixture-model", + content: [{ type: "text", text: "done" }], stop_reason: "end_turn", + usage: { input_tokens: 10, output_tokens: 1 }, + } : { + id: "resp_skills", status: "completed", + output: [{ type: "message", role: "assistant", content: [{ type: "output_text", text: "done" }] }], + usage: { input_tokens: 10, output_tokens: 1, total_tokens: 11 }, + }); + }) as typeof fetch; + return { + ...getDefaultConfig(), + defaultProvider: "fixture", + providers: { + fixture: { adapter, baseUrl: "https://fixture.test/v1", authMode: "key", apiKey: "fixture-key" }, + }, + }; +} + +async function send(config: OcxConfig, catalog: string, thread?: string, surrounding = "outside", headers: Record = {}) { + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { + "content-type": "application/json", + ...headers, + ...(thread ? { "thread-id": thread, "x-codex-parent-thread-id": "shared-parent" } : {}), + }, + body: JSON.stringify({ + model: "fixture/fixture-model", stream: false, + input: [ + { role: "developer", content: [{ type: "input_text", text: `${surrounding}${catalog}` }] }, + { role: "user", content: "Please help with user-example" }, + ], + }), + }), config, { model: "", provider: "" }); + const text = await response.text(); + expect({ status: response.status, ...(response.status !== 200 ? { text } : {}) }).toEqual({ status: 200 }); + return captured.at(-1)!; +} + +describe("skills catalog snapshots on the Responses request path", () => { + for (const adapter of ["openai-responses", "anthropic"] as const) { + test(`${adapter}: keeps the catalog stable while preserving surrounding instructions and sibling isolation`, async () => { + const config = fixture(adapter); + await send(config, "first-catalog", "child-a"); + const second = await send(config, "edited-catalog", "child-a", "updated-outside"); + expect(second).toContain("first-catalog"); + expect(second).not.toContain("edited-catalog"); + expect(second).toContain("updated-outside"); + expect(second).toContain("user-example"); + const sibling = await send(config, "sibling-catalog", "child-b"); + expect(sibling).toContain("sibling-catalog"); + expect(sibling).not.toContain("first-catalog"); + }); + } + + test("per_turn forwards changed catalogs", async () => { + const config = fixture("anthropic"); + config.skills = { catalog_refresh: "per_turn" }; + await send(config, "first-catalog", "turn-mode"); + expect(await send(config, "edited-catalog", "turn-mode")).toContain("edited-catalog"); + }); + + test("session header aliases reuse the same snapshot", async () => { + const config = fixture("anthropic"); + await send(config, "session-catalog", undefined, "outside", { session_id: "session-a" }); + const next = await send(config, "edited-catalog", undefined, "outside", { "session-id": "session-a" }); + expect(next).toContain("session-catalog"); + expect(next).not.toContain("edited-catalog"); + expect(await send(config, "new-session-catalog", undefined, "outside", { session_id: "session-b" })) + .toContain("new-session-catalog"); + }); + + test("requests without a conversation identity never share catalogs", async () => { + const config = fixture("anthropic"); + await send(config, "first-catalog"); + expect(await send(config, "edited-catalog")).toContain("edited-catalog"); + }); + + test("a parent-only routing identity cannot share sibling catalogs", async () => { + const config = fixture("anthropic"); + const headers = { "x-codex-parent-thread-id": "parent-without-child" }; + await send(config, "first-child-catalog", undefined, "outside", headers); + expect(await send(config, "second-child-catalog", undefined, "outside", headers)) + .toContain("second-child-catalog"); + }); +});