From b0c63389aad22762e7d2fb35245d8a9190e4df87 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sat, 7 Nov 2026 00:38:09 +0100 Subject: [PATCH 1/2] test: Cover a Deployment Skill File Removed at Runtime --- .../e2e-removed-file-skill/SKILL.md | 9 ++ .../e2e-removed-file-skill/a-first.txt | 1 + .../e2e-removed-file-skill/b-removed.txt | 1 + .../e2e-removed-file-skill/z-last.txt | 1 + .../deployment-skill-removed-file.spec.ts | 96 +++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 e2e/fixtures/deployment-skills/e2e-removed-file-skill/SKILL.md create mode 100644 e2e/fixtures/deployment-skills/e2e-removed-file-skill/a-first.txt create mode 100644 e2e/fixtures/deployment-skills/e2e-removed-file-skill/b-removed.txt create mode 100644 e2e/fixtures/deployment-skills/e2e-removed-file-skill/z-last.txt create mode 100644 e2e/specs/mock/scenarios/deployment-skill-removed-file.spec.ts diff --git a/e2e/fixtures/deployment-skills/e2e-removed-file-skill/SKILL.md b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/SKILL.md new file mode 100644 index 00000000000..ee08aa51490 --- /dev/null +++ b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/SKILL.md @@ -0,0 +1,9 @@ +--- +name: e2e-removed-file-skill +description: Use this deployment skill to verify priming survives a bundled file removed at runtime. +user-invocable: true +--- + +# e2e-removed-file-skill + +Bundled files sort around the one the scenario removes while the server is running. diff --git a/e2e/fixtures/deployment-skills/e2e-removed-file-skill/a-first.txt b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/a-first.txt new file mode 100644 index 00000000000..39c9fe01e00 --- /dev/null +++ b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/a-first.txt @@ -0,0 +1 @@ +first bundled file diff --git a/e2e/fixtures/deployment-skills/e2e-removed-file-skill/b-removed.txt b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/b-removed.txt new file mode 100644 index 00000000000..6c346b7ff0a --- /dev/null +++ b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/b-removed.txt @@ -0,0 +1 @@ +removed while the server runs diff --git a/e2e/fixtures/deployment-skills/e2e-removed-file-skill/z-last.txt b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/z-last.txt new file mode 100644 index 00000000000..ed7506d8c7b --- /dev/null +++ b/e2e/fixtures/deployment-skills/e2e-removed-file-skill/z-last.txt @@ -0,0 +1 @@ +last bundled file diff --git a/e2e/specs/mock/scenarios/deployment-skill-removed-file.spec.ts b/e2e/specs/mock/scenarios/deployment-skill-removed-file.spec.ts new file mode 100644 index 00000000000..71ff52abd9f --- /dev/null +++ b/e2e/specs/mock/scenarios/deployment-skill-removed-file.spec.ts @@ -0,0 +1,96 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test } from '@playwright/test'; +import type { Page } from '@playwright/test'; +import { + MOCK_ENDPOINTS, + NEW_CHAT_PATH, + enableCodeInterpreter, + enableSkills, + getAccessToken, + getCodeProvisionedUploads, + messagesView, + requestJson, + selectMockEndpoint, + sendMessage, +} from '../helpers'; + +/** + * The deployment skill registry is built once at boot, so a bundled file removed from a running + * instance stays registered. Its stream used to fail only after the batch upload had started, + * which aborted the whole request and kept every later file out of the sandbox. + */ +const SKILL_NAME = 'e2e-removed-file-skill'; +const REMOVED_FILE = 'b-removed.txt'; +const REMOVED_CONTENT = 'removed while the server runs\n'; +const REMOVED_PATH = path.resolve( + __dirname, + '../../../fixtures/deployment-skills', + SKILL_NAME, + REMOVED_FILE, +); + +async function withRemovedFile(run: () => Promise): Promise { + await fs.promises.rm(REMOVED_PATH, { force: true }); + try { + await run(); + } finally { + await fs.promises.writeFile(REMOVED_PATH, REMOVED_CONTENT); + } +} + +/** The fake code server records the base name of each uploaded file. */ +async function uploadedSkillFiles(page: Page, from: number): Promise { + const uploads = await getCodeProvisionedUploads(page); + return uploads + .slice(from) + .filter((upload) => upload.kind === 'skill') + .map((upload) => upload.filename); +} + +test('a deployment skill still primes its remaining files after one is removed from disk @scenario:deployment-skill-primes-without-removed-file', async ({ + page, +}) => { + test.setTimeout(120_000); + await withRemovedFile(async () => { + await page.goto(NEW_CHAT_PATH, { timeout: 15_000 }); + await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); + await enableSkills(page); + await enableCodeInterpreter(page); + const before = (await getCodeProvisionedUploads(page)).length; + + const response = await sendMessage(page, `E2E_INVOKE_SKILL:${SKILL_NAME}`); + expect(response.ok()).toBeTruthy(); + await expect( + messagesView(page).getByText(`E2E skill tool assertion passed: ${SKILL_NAME}`), + ).toBeVisible({ timeout: 30_000 }); + + await expect + .poll(() => uploadedSkillFiles(page, before), { timeout: 15_000 }) + .toEqual(expect.arrayContaining(['SKILL.md', 'a-first.txt', 'z-last.txt'])); + expect(await uploadedSkillFiles(page, before)).not.toContain(REMOVED_FILE); + }); +}); + +test('downloading a removed deployment skill file answers with an error that names no path @scenario:deployment-skill-removed-file-download-fails-cleanly', async ({ + page, +}) => { + await withRemovedFile(async () => { + await page.goto(NEW_CHAT_PATH, { timeout: 15_000 }); + const token = await getAccessToken(page); + const list = await requestJson<{ skills: Array<{ _id: string; name: string }> }>(page, { + path: `/api/skills?search=${SKILL_NAME}&limit=10`, + token, + }); + const skill = list.skills.find((item) => item.name === SKILL_NAME); + expect(skill).toBeDefined(); + + const response = await page.request.get( + `/api/skills/${skill?._id}/files/${REMOVED_FILE}?raw=true`, + { headers: { Authorization: `Bearer ${token}` } }, + ); + + expect(response.status()).toBe(500); + expect(await response.json()).toEqual({ error: 'Error downloading skill file' }); + }); +}); From 87c7611b938ea2b8e2bfc23dce5322bd8c828734 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sat, 7 Nov 2026 00:46:53 +0100 Subject: [PATCH 2/2] fix: Name the Skipped Skill File in the Console Log The default console format prints the message and request context but drops metadata, so a relativePath passed as metadata only reached the JSON file log. --- packages/api/src/agents/skillFiles.spec.ts | 8 ++++---- packages/api/src/agents/skillFiles.ts | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/api/src/agents/skillFiles.spec.ts b/packages/api/src/agents/skillFiles.spec.ts index e38560b0098..dc56f2d75d9 100644 --- a/packages/api/src/agents/skillFiles.spec.ts +++ b/packages/api/src/agents/skillFiles.spec.ts @@ -973,10 +973,10 @@ describe('primeSkillFiles — resource identity propagation', () => { ]); expect(result?.storage_session_id).toBe('session-fresh'); expect(logger.error).toHaveBeenCalledTimes(1); - expect(logger.error).toHaveBeenCalledWith('[primeSkillFiles] Failed to get stream:', { - relativePath: 'masters/missing.dotx', - type: 'Error', - }); + expect(logger.error).toHaveBeenCalledWith( + '[primeSkillFiles] Failed to get stream for "masters/missing.dotx":', + { type: 'Error' }, + ); }); it('fresh-upload path: emits resource_id=skill._id, kind=skill, version on each file', async () => { diff --git a/packages/api/src/agents/skillFiles.ts b/packages/api/src/agents/skillFiles.ts index e81c671aa49..d5b5d8a1f6c 100644 --- a/packages/api/src/agents/skillFiles.ts +++ b/packages/api/src/agents/skillFiles.ts @@ -248,10 +248,11 @@ async function collectSkillUploadFiles( if (result.status === 'fulfilled' && result.value) { filesToUpload.push(result.value); } else if (result.status === 'rejected') { - logger.error('[primeSkillFiles] Failed to get stream:', { - relativePath: skillFiles[index].relativePath, - ...getSafeErrorMetadata(result.reason), - }); + /* The path belongs in the message: the default console format drops metadata. */ + logger.error( + `[primeSkillFiles] Failed to get stream for "${skillFiles[index].relativePath}":`, + getSafeErrorMetadata(result.reason), + ); } }