From 22a93686043f4cbd4fcf81f0be5e1fe9df814307 Mon Sep 17 00:00:00 2001 From: dymux <91779374+putramkti@users.noreply.github.com> Date: Tue, 1 Sep 2026 09:32:20 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=84=20fix:=20Refresh=20Stale=20Skill?= =?UTF-8?q?=20File=20References=20(#15413)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): check skill version in codeEnvRef cache-hit path When a SKILL.md body is edited, `updateSkill` bumps `Skill.version` but does not clear the `codeEnvRef` on bundled skill files. The cache-hit path in `primeSkillFiles` reused those stale refs without comparing versions, causing Code API to return 403 ("Unauthorized file reference") because the session key no longer matched. Add a version check to the cache-hit guard: a ref whose version does not match the current skill version is treated as a cache miss, forcing a fresh upload under the new session key. Closes #15275 * fix(agents): apply version check to aggregate cache path in primeInvokedSkills The previous commit added a version check to the per-skill cache-hit path in primeSkillFiles. Codex review pointed out that primeInvokedSkills has its own aggregate cache-hit branch that returns cached refs before delegating to primeSkillFiles. That branch checked ref presence and session activity but not ref.version against skill.version, so a stale ref from an older skill revision would still bypass the guard. Add the same version comparison to the aggregate filter and include the skill object in the allResolved entries so the version is accessible. Addresses Codex review on #15413 * fix(agents): type skill cache version checks --------- Co-authored-by: dymux Co-authored-by: Danny Avila --- packages/api/src/agents/skillFiles.spec.ts | 80 ++++++++++++++++++++++ packages/api/src/agents/skillFiles.ts | 26 +++++-- 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/packages/api/src/agents/skillFiles.spec.ts b/packages/api/src/agents/skillFiles.spec.ts index 429da21f32..37d1f604b7 100644 --- a/packages/api/src/agents/skillFiles.spec.ts +++ b/packages/api/src/agents/skillFiles.spec.ts @@ -542,6 +542,48 @@ describe('primeInvokedSkills — execute_code capability gate', () => { }, ]); }); + + it('falls through to per-skill upload when codeEnvRef version is stale', async () => { + const listSkillFiles = jest.fn().mockResolvedValue([ + { + relativePath: 'references/style.md', + filename: 'style.md', + filepath: '/storage/brand-guidelines/references/style.md', + source: 's3', + bytes: 256, + codeEnvRef: { + kind: 'skill', + id: SKILL_ID.toString(), + storage_session_id: 'session-stale', + file_id: 'file-stale', + version: SKILL_VERSION - 1, // stale: one version behind + }, + }, + ]); + const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({ + storage_session_id: 'session-fresh', + files: [{ fileId: 'file-fresh', filename: 'skills/brand-guidelines/references/style.md' }], + }); + const getStrategyFunctions = jest.fn().mockReturnValue({ + getDownloadStream: jest.fn().mockResolvedValue(Readable.from(Buffer.from('style'))), + }); + const getSessionInfo = jest.fn(); + const deps = makeDeps({ + codeEnvAvailable: true, + listSkillFiles, + batchUploadCodeEnvFiles, + getStrategyFunctions, + getSessionInfo, + checkIfActive: jest.fn().mockReturnValue(true), + }); + + const result = await primeInvokedSkills(deps); + + expect(getSessionInfo).not.toHaveBeenCalled(); + expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1); + const codeSession = result.initialSessions?.get('execute_code'); + expect(codeSession?.files?.[0]?.version).toBe(SKILL_VERSION); + }); }); describe('primeInvokedSkillsForProfiles', () => { @@ -822,6 +864,44 @@ describe('primeSkillFiles — resource identity propagation', () => { ]); }); + it('reuploads cached refs from an older skill version', async () => { + const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({ + storage_session_id: 'session-fresh', + files: [ + { fileId: 'file-fresh', filename: 'skills/brand-guidelines/references/style.md' }, + { fileId: 'skill-md', filename: 'skills/brand-guidelines/SKILL.md' }, + ], + }); + const getSessionInfo = jest.fn(); + const deps = makeSkillFilesDeps({ + skillFiles: [ + { + relativePath: 'references/style.md', + filename: 'style.md', + filepath: '/storage/brand-guidelines/references/style.md', + source: 's3', + bytes: 256, + codeEnvRef: { + kind: 'skill', + id: SKILL_ID.toString(), + storage_session_id: 'session-stale', + file_id: 'file-stale', + version: SKILL_VERSION - 1, + }, + }, + ], + batchUploadCodeEnvFiles, + getSessionInfo, + checkIfActive: jest.fn().mockReturnValue(true), + }); + + const result = await primeSkillFiles(deps); + + expect(getSessionInfo).not.toHaveBeenCalled(); + expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1); + expect(result?.files[0]).toEqual(expect.objectContaining({ version: SKILL_VERSION })); + }); + it('reuploads a cached ref that belongs to the other execution profile', async () => { const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({ storage_session_id: 'stateful-session', diff --git a/packages/api/src/agents/skillFiles.ts b/packages/api/src/agents/skillFiles.ts index 5eb0645d98..27bef398b1 100644 --- a/packages/api/src/agents/skillFiles.ts +++ b/packages/api/src/agents/skillFiles.ts @@ -137,6 +137,14 @@ const uploadSlots = createConcurrencyLimiter(SKILL_UPLOAD_CONCURRENCY); const inflightPrimes = new Map>(); type SkillUploadFiles = Array<{ stream: NodeJS.ReadableStream; filename: string }>; +type SkillCodeEnvRef = Extract; + +function isCurrentSkillRef( + ref: CodeEnvRef | undefined, + skillVersion: number, +): ref is SkillCodeEnvRef { + return ref?.kind === 'skill' && ref.version === skillVersion; +} function getRetryAfterMs(error: unknown): number | null { if (!isAxiosError(error) || error.response?.status !== 429) { @@ -427,12 +435,14 @@ async function executePrimeSkillFiles( * previous prime. Check freshness against codeapi for every distinct * storage session; if all are still active, reuse without * re-uploading. The skill version is part of the ref — when the - * skill is edited, the upsert clears the ref and forces a fresh - * upload on the next prime. */ + * skill version has been bumped (e.g. by a SKILL.md edit), stale + * refs are treated as cache misses and the files are re-uploaded + * under the new version's session key. */ if (getSessionInfo && checkIfActive && skillFiles.length > 0) { - const allHaveRefs = skillFiles.every( - (sf) => getCodeEnvRefForProfile(sf, executionRouteKey) !== undefined, - ); + const allHaveRefs = skillFiles.every((sf) => { + const ref = getCodeEnvRefForProfile(sf, executionRouteKey); + return isCurrentSkillRef(ref, skill.version); + }); if (allHaveRefs) { const refsBySession = new Map(); for (const sf of skillFiles) { @@ -753,12 +763,16 @@ export async function primeInvokedSkills( if (!inspectStoredSkillFileContent && deps.getSessionInfo && deps.checkIfActive) { const allResolved = fileListResults.flatMap((r) => r.files.map((f) => ({ + skill: r.skill, skillName: r.skill.name, file: f, ref: getCodeEnvRefForProfile(f, executionRouteKey), })), ); - const resolvedWithRef = allResolved.filter((x) => x.ref !== undefined); + const resolvedWithRef = allResolved.filter( + (entry): entry is typeof entry & { ref: SkillCodeEnvRef } => + isCurrentSkillRef(entry.ref, entry.skill.version), + ); // Only use cache when ALL files have refs (no partial persistence) if (resolvedWithRef.length > 0 && resolvedWithRef.length === allResolved.length) {