From 51cd6da223d011021c44b2c4533d3f89e9921f3b Mon Sep 17 00:00:00 2001 From: KDH Date: Wed, 2 Sep 2026 15:51:02 +0900 Subject: [PATCH 1/3] perf(coding-agent): load skill files concurrently loadSkillsFromDirInternal read every SKILL.md sequentially with readFileSync; with many skills (and slow disks) that serializes disk access during startup. Collect the file work in traversal order, then run the IO-bound reads with Promise.all while preserving skill/diagnostic order exactly. loadSkills and loadSkillsFromDir become async; extendResources, updateSkillsFromPaths and the app-server skills loader await them. Existing skills suite adapted to the async API (27 cases) plus one new canonical-path dedupe case; the imagegen skill-gating test awaits loadSkills. --- packages/coding-agent/changes.md | 8 ++ .../coding-agent/src/core/agent-session.ts | 2 +- .../coding-agent/src/core/resource-loader.ts | 10 +- packages/coding-agent/src/core/skills.ts | 65 +++++---- .../src/modes/app-server/server/skills.ts | 4 +- .../test/imagegen-skill-gating.test.ts | 2 +- packages/coding-agent/test/skills.test.ts | 127 +++++++++++------- 7 files changed, 130 insertions(+), 88 deletions(-) diff --git a/packages/coding-agent/changes.md b/packages/coding-agent/changes.md index 43416f6ff7..5e0d11f7a3 100644 --- a/packages/coding-agent/changes.md +++ b/packages/coding-agent/changes.md @@ -1,5 +1,13 @@ # Local fork changes +## 2026-09-02 - Load skill files concurrently at startup + +- `loadSkills` / `loadSkillsFromDir` are now async and read skill files concurrently while preserving skill and diagnostic order exactly. `resource-loader`'s `extendResources` / `updateSkillsFromPaths` and the app-server skills loader await them; startup with many skills no longer serializes disk reads. + +## 2026-09-02 - Keep the startup indicator through the interactive-mode import + +- The startup loading indicator now stays visible through the dynamic import of the interactive-mode module graph (the largest cold-start cost after resource loading) instead of stopping before it, which left a blank terminal that looked frozen. + ## 2026-09-01 - Acknowledge RPC abort before quiesce - The RPC `abort` command now acknowledges immediately after dispatching the abort signal, while observing quiesce failures through the existing `rpc_error` event path. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 5e5f1fafa5..edc0542bf4 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -6637,7 +6637,7 @@ export class AgentSession { hookPaths: this.buildExtensionResourcePaths(hookPaths), }; - this._resourceLoader.extendResources(extensionPaths); + await this._resourceLoader.extendResources(extensionPaths); if (skillPaths.length > 0 || promptPaths.length > 0 || themePaths.length > 0) { this._baseSystemPrompt = this._rebuildSystemPrompt(this.getActiveToolNames()); this.agent.state.systemPrompt = this._baseSystemPrompt; diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 558960c3e0..6f9762576e 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -77,7 +77,7 @@ export interface ResourceLoader { getAppendSystemPrompt(): string[]; getLoadedHookSources?(): LoadedHookSources; getAppendSystemPromptSources(): Array<{ path: string }>; - extendResources(paths: ResourceExtensionPaths): void; + extendResources(paths: ResourceExtensionPaths): Promise; reload(options?: ResourceLoaderReloadOptions): Promise; } @@ -520,7 +520,7 @@ export class DefaultResourceLoader implements ResourceLoader { return this.appendSystemPromptSourcePaths.map((path) => ({ path })); } - extendResources(paths: ResourceExtensionPaths): void { + async extendResources(paths: ResourceExtensionPaths): Promise { const skillPaths = this.normalizeExtensionPaths(paths.skillPaths ?? []); const promptPaths = this.normalizeExtensionPaths(paths.promptPaths ?? []); const themePaths = this.normalizeExtensionPaths(paths.themePaths ?? []); @@ -541,7 +541,7 @@ export class DefaultResourceLoader implements ResourceLoader { this.lastSkillPaths, skillPaths.map((entry) => entry.path), ); - this.updateSkillsFromPaths(this.lastSkillPaths, this.resourceMetadataByPath); + await this.updateSkillsFromPaths(this.lastSkillPaths, this.resourceMetadataByPath); } if (promptPaths.length > 0) { @@ -953,12 +953,12 @@ export class DefaultResourceLoader implements ResourceLoader { }; } - private updateSkillsFromPaths(skillPaths: string[], metadataByPath?: Map): void { + private async updateSkillsFromPaths(skillPaths: string[], metadataByPath?: Map): Promise { let skillsResult: { skills: Skill[]; diagnostics: ResourceDiagnostic[] }; if (this.noSkills && skillPaths.length === 0) { skillsResult = { skills: [], diagnostics: [] }; } else { - skillsResult = loadSkills({ + skillsResult = await loadSkills({ cwd: this.cwd, agentDir: this.agentDir, skillPaths, diff --git a/packages/coding-agent/src/core/skills.ts b/packages/coding-agent/src/core/skills.ts index 102b51fa6b..23fa5db85b 100644 --- a/packages/coding-agent/src/core/skills.ts +++ b/packages/coding-agent/src/core/skills.ts @@ -1,4 +1,5 @@ -import { existsSync, readdirSync, readFileSync, statSync } from "fs"; +import { existsSync, readdirSync, statSync } from "fs"; +import { readFile } from "fs/promises"; import ignore from "ignore"; import { basename, dirname, join, relative, resolve, sep } from "path"; import { CONFIG_DIR_NAME, getAgentDir } from "../config.ts"; @@ -165,7 +166,7 @@ function createSkillSourceInfo(filePath: string, baseDir: string, source: string * - otherwise, load direct .md children in the root * - recurse into subdirectories to find SKILL.md */ -export function loadSkillsFromDir(options: LoadSkillsFromDirOptions): LoadSkillsResult { +export async function loadSkillsFromDir(options: LoadSkillsFromDirOptions): Promise { const { dir, source } = options; return loadSkillsFromDirInternal(dir, source, true); } @@ -176,12 +177,12 @@ function loadSkillsFromDirInternal( includeRootFiles: boolean, ignoreMatcher?: IgnoreMatcher, rootDir?: string, -): LoadSkillsResult { +): Promise { const skills: Skill[] = []; const diagnostics: ResourceDiagnostic[] = []; if (!existsSync(dir)) { - return { skills, diagnostics }; + return Promise.resolve({ skills, diagnostics }); } const root = rootDir ?? dir; @@ -212,14 +213,17 @@ function loadSkillsFromDirInternal( continue; } - const result = loadSkillFromFile(fullPath, source); - if (result.skill) { - skills.push(result.skill); - } - diagnostics.push(...result.diagnostics); - return { skills, diagnostics }; + // A root SKILL.md is the whole skill root: do not recurse. + return loadSkillFromFile(fullPath, source).then((result) => ({ + skills: result.skill ? [result.skill] : [], + diagnostics: result.diagnostics, + })); } + // Collect the work in traversal order, then run the (IO-bound) file reads + // concurrently so a large skill tree does not serialize disk access. Order + // and diagnostics stay identical to the sequential walk. + const pending: Array> = []; for (const entry of entries) { if (entry.name.startsWith(".")) { continue; @@ -253,9 +257,7 @@ function loadSkillsFromDirInternal( } if (isDirectory) { - const subResult = loadSkillsFromDirInternal(fullPath, source, false, ig, root); - skills.push(...subResult.skills); - diagnostics.push(...subResult.diagnostics); + pending.push(loadSkillsFromDirInternal(fullPath, source, false, ig, root)); continue; } @@ -263,27 +265,34 @@ function loadSkillsFromDirInternal( continue; } - const result = loadSkillFromFile(fullPath, source); - if (result.skill) { - skills.push(result.skill); - } - diagnostics.push(...result.diagnostics); + pending.push(loadSkillFromFile(fullPath, source).then((result) => ({ + skills: result.skill ? [result.skill] : [], + diagnostics: result.diagnostics, + }))); } - } catch {} - return { skills, diagnostics }; + return Promise.all(pending).then((results) => { + for (const result of results) { + skills.push(...result.skills); + diagnostics.push(...result.diagnostics); + } + return { skills, diagnostics }; + }); + } catch { + return Promise.resolve({ skills, diagnostics }); + } } -function loadSkillFromFile( +async function loadSkillFromFile( filePath: string, source: string, -): { skill: Skill | null; diagnostics: ResourceDiagnostic[] } { +): Promise<{ skill: Skill | null; diagnostics: ResourceDiagnostic[] }> { const diagnostics: ResourceDiagnostic[] = []; const isDeclaredSkill = basename(filePath) === "SKILL.md"; let rawContent: string; try { - rawContent = readFileSync(filePath, "utf-8"); + rawContent = await readFile(filePath, "utf-8"); } catch (error) { const message = error instanceof Error ? error.message : "failed to read skill file"; diagnostics.push({ type: "warning", message, path: filePath }); @@ -404,7 +413,7 @@ export interface LoadSkillsOptions { * Load skills from all configured locations. * Returns skills and any validation diagnostics. */ -export function loadSkills(options: LoadSkillsOptions): LoadSkillsResult { +export async function loadSkills(options: LoadSkillsOptions): Promise { const { agentDir, skillPaths, includeDefaults } = options; // Resolve agentDir - if not provided, use default from config @@ -448,8 +457,8 @@ export function loadSkills(options: LoadSkillsOptions): LoadSkillsResult { } if (includeDefaults) { - addSkills(loadSkillsFromDirInternal(join(resolvedAgentDir, "skills"), "user", true)); - addSkills(loadSkillsFromDirInternal(resolve(resolvedCwd, CONFIG_DIR_NAME, "skills"), "project", true)); + addSkills(await loadSkillsFromDirInternal(join(resolvedAgentDir, "skills"), "user", true)); + addSkills(await loadSkillsFromDirInternal(resolve(resolvedCwd, CONFIG_DIR_NAME, "skills"), "project", true)); } const userSkillsDir = join(resolvedAgentDir, "skills"); @@ -483,9 +492,9 @@ export function loadSkills(options: LoadSkillsOptions): LoadSkillsResult { const stats = statSync(resolvedPath); const source = getSource(resolvedPath); if (stats.isDirectory()) { - addSkills(loadSkillsFromDirInternal(resolvedPath, source, true)); + addSkills(await loadSkillsFromDirInternal(resolvedPath, source, true)); } else if (stats.isFile() && resolvedPath.endsWith(".md")) { - const result = loadSkillFromFile(resolvedPath, source); + const result = await loadSkillFromFile(resolvedPath, source); if (result.skill) { addSkills({ skills: [result.skill], diagnostics: result.diagnostics }); } else { diff --git a/packages/coding-agent/src/modes/app-server/server/skills.ts b/packages/coding-agent/src/modes/app-server/server/skills.ts index dee7d8e785..2afe6609fb 100644 --- a/packages/coding-agent/src/modes/app-server/server/skills.ts +++ b/packages/coding-agent/src/modes/app-server/server/skills.ts @@ -131,11 +131,11 @@ function findLoadedLoader( async function createLoader(cwd: string, options: SkillsListEntryOptions): Promise { if (options.resourceLoaderFactory) return options.resourceLoaderFactory(cwd); - let loaded = loadSkills({ cwd, agentDir: options.agentDir, skillPaths: [], includeDefaults: true }); + let loaded = await loadSkills({ cwd, agentDir: options.agentDir, skillPaths: [], includeDefaults: true }); return { getSkills: () => loaded, reload: async () => { - loaded = loadSkills({ cwd, agentDir: options.agentDir, skillPaths: [], includeDefaults: true }); + loaded = await loadSkills({ cwd, agentDir: options.agentDir, skillPaths: [], includeDefaults: true }); }, }; } diff --git a/packages/coding-agent/test/imagegen-skill-gating.test.ts b/packages/coding-agent/test/imagegen-skill-gating.test.ts index ec8729c507..25a5635977 100644 --- a/packages/coding-agent/test/imagegen-skill-gating.test.ts +++ b/packages/coding-agent/test/imagegen-skill-gating.test.ts @@ -74,7 +74,7 @@ describe("imagegen skill contribution", () => { const resources = await runner.emitResourcesDiscover(cwd, "startup"); const skillPaths = resources.skillPaths.map((entry) => entry.path); - const loaded = loadSkills({ cwd, agentDir: join(cwd, "agent"), skillPaths, includeDefaults: false }); + const loaded = await loadSkills({ cwd, agentDir: join(cwd, "agent"), skillPaths, includeDefaults: false }); const prompt = await runner.emitBeforeAgentStart("draw an image", undefined, "base", { cwd }); expect(skillPaths).toHaveLength(1); diff --git a/packages/coding-agent/test/skills.test.ts b/packages/coding-agent/test/skills.test.ts index 8da42bd7a7..f41392a402 100644 --- a/packages/coding-agent/test/skills.test.ts +++ b/packages/coding-agent/test/skills.test.ts @@ -1,4 +1,5 @@ -import { homedir } from "os"; +import { mkdtempSync, mkdirSync, writeFileSync } from "fs"; +import { homedir, tmpdir } from "os"; import { join, resolve } from "path"; import { describe, expect, it } from "vitest"; import type { ResourceDiagnostic } from "../src/core/diagnostics.ts"; @@ -28,8 +29,8 @@ function createTestSkill(options: { describe("skills", () => { describe("loadSkillsFromDir", () => { - it("should load a valid skill", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should load a valid skill", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "valid-skill"), source: "test", }); @@ -41,8 +42,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should allow names that don't match parent directory", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should allow names that don't match parent directory", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "name-mismatch"), source: "test", }); @@ -54,8 +55,8 @@ describe("skills", () => { ).toBe(false); }); - it("should warn when name contains invalid characters", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should warn when name contains invalid characters", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "invalid-name-chars"), source: "test", }); @@ -64,8 +65,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("invalid characters"))).toBe(true); }); - it("should warn when name exceeds 64 characters", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should warn when name exceeds 64 characters", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "long-name"), source: "test", }); @@ -74,8 +75,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("exceeds 64 characters"))).toBe(true); }); - it("should warn and skip skill when description is missing", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should warn and skip skill when description is missing", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "missing-description"), source: "test", }); @@ -84,8 +85,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("description is required"))).toBe(true); }); - it("should ignore unknown frontmatter fields", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should ignore unknown frontmatter fields", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "unknown-field"), source: "test", }); @@ -94,8 +95,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should load nested skills recursively", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should load nested skills recursively", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "nested"), source: "test", }); @@ -105,8 +106,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should prefer a directory's root SKILL.md over nested SKILL.md files", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should prefer a directory's root SKILL.md over nested SKILL.md files", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "root-skill-preferred"), source: "test", }); @@ -117,8 +118,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should skip files without frontmatter", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should skip files without frontmatter", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "no-frontmatter"), source: "test", }); @@ -128,8 +129,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("description is required"))).toBe(true); }); - it("should warn and skip skill when YAML frontmatter is invalid", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should warn and skip skill when YAML frontmatter is invalid", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "invalid-yaml"), source: "test", }); @@ -138,8 +139,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("at line"))).toBe(true); }); - it("should preserve multiline descriptions from YAML", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should preserve multiline descriptions from YAML", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "multiline-description"), source: "test", }); @@ -150,8 +151,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should warn when name contains consecutive hyphens", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should warn when name contains consecutive hyphens", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "consecutive-hyphens"), source: "test", }); @@ -160,8 +161,8 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("consecutive hyphens"))).toBe(true); }); - it("should load all skills from fixture directory", () => { - const { skills } = loadSkillsFromDir({ + it("should load all skills from fixture directory", async () => { + const { skills } = await loadSkillsFromDir({ dir: fixturesDir, source: "test", }); @@ -172,8 +173,8 @@ describe("skills", () => { expect(skills.length).toBeGreaterThanOrEqual(6); }); - it("should return empty for non-existent directory", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should return empty for non-existent directory", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: "/non/existent/path", source: "test", }); @@ -182,11 +183,11 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should use parent directory name when name not in frontmatter", () => { + it("should use parent directory name when name not in frontmatter", async () => { // The no-frontmatter fixture has no name in frontmatter, so it should use "no-frontmatter" // But it also has no description, so it won't load // Let's test with a valid skill that relies on directory name - const { skills } = loadSkillsFromDir({ + const { skills } = await loadSkillsFromDir({ dir: join(fixturesDir, "valid-skill"), source: "test", }); @@ -195,8 +196,8 @@ describe("skills", () => { expect(skills[0].name).toBe("valid-skill"); }); - it("should parse disable-model-invocation frontmatter field", () => { - const { skills, diagnostics } = loadSkillsFromDir({ + it("should parse disable-model-invocation frontmatter field", async () => { + const { skills, diagnostics } = await loadSkillsFromDir({ dir: join(fixturesDir, "disable-model-invocation"), source: "test", }); @@ -210,8 +211,8 @@ describe("skills", () => { ); }); - it("should default disableModelInvocation to false when not specified", () => { - const { skills } = loadSkillsFromDir({ + it("should default disableModelInvocation to false when not specified", async () => { + const { skills } = await loadSkillsFromDir({ dir: join(fixturesDir, "valid-skill"), source: "test", }); @@ -222,12 +223,12 @@ describe("skills", () => { }); describe("formatSkillsForPrompt", () => { - it("should return empty string for no skills", () => { + it("should return empty string for no skills", async () => { const result = formatSkillsForPrompt([]); expect(result).toBe(""); }); - it("should format skills as XML", () => { + it("should format skills as XML", async () => { const skills: Skill[] = [ createTestSkill({ name: "test-skill", @@ -247,7 +248,7 @@ describe("skills", () => { expect(result).toContain("/path/to/skill/SKILL.md"); }); - it("should escape XML special characters", () => { + it("should escape XML special characters", async () => { const skills: Skill[] = [ createTestSkill({ name: "test-skill", @@ -264,7 +265,7 @@ describe("skills", () => { expect(result).toContain(""characters""); }); - it("should format multiple skills", () => { + it("should format multiple skills", async () => { const skills: Skill[] = [ createTestSkill({ name: "skill-one", @@ -287,7 +288,7 @@ describe("skills", () => { expect((result.match(//g) || []).length).toBe(2); }); - it("should exclude skills with disableModelInvocation from prompt", () => { + it("should exclude skills with disableModelInvocation from prompt", async () => { const skills: Skill[] = [ createTestSkill({ name: "visible-skill", @@ -311,7 +312,7 @@ describe("skills", () => { expect((result.match(//g) || []).length).toBe(1); }); - it("should return empty string when all skills have disableModelInvocation", () => { + it("should return empty string when all skills have disableModelInvocation", async () => { const skills: Skill[] = [ createTestSkill({ name: "hidden-skill", @@ -331,8 +332,8 @@ describe("skills", () => { const emptyAgentDir = resolve(__dirname, "fixtures/empty-agent"); const emptyCwd = resolve(__dirname, "fixtures/empty-cwd"); - it("should load from explicit skillPaths", () => { - const { skills, diagnostics } = loadSkills({ + it("should load from explicit skillPaths", async () => { + const { skills, diagnostics } = await loadSkills({ agentDir: emptyAgentDir, cwd: emptyCwd, skillPaths: [join(fixturesDir, "valid-skill")], @@ -343,8 +344,8 @@ describe("skills", () => { expect(diagnostics).toHaveLength(0); }); - it("should warn when skill path does not exist", () => { - const { skills, diagnostics } = loadSkills({ + it("should warn when skill path does not exist", async () => { + const { skills, diagnostics } = await loadSkills({ agentDir: emptyAgentDir, cwd: emptyCwd, skillPaths: ["/non/existent/path"], @@ -354,15 +355,15 @@ describe("skills", () => { expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("does not exist"))).toBe(true); }); - it("should expand ~ in skillPaths", () => { + it("should expand ~ in skillPaths", async () => { const homeSkillsDir = join(homedir(), ".senpi/agent/skills"); - const { skills: withTilde } = loadSkills({ + const { skills: withTilde } = await loadSkills({ agentDir: emptyAgentDir, cwd: emptyCwd, skillPaths: ["~/.senpi/agent/skills"], includeDefaults: true, }); - const { skills: withoutTilde } = loadSkills({ + const { skills: withoutTilde } = await loadSkills({ agentDir: emptyAgentDir, cwd: emptyCwd, skillPaths: [homeSkillsDir], @@ -373,14 +374,14 @@ describe("skills", () => { }); describe("collision handling", () => { - it("should detect name collisions and keep first skill", () => { + it("should detect name collisions and keep first skill", async () => { // Load from first directory - const first = loadSkillsFromDir({ + const first = await loadSkillsFromDir({ dir: join(collisionFixturesDir, "first"), source: "first", }); - const second = loadSkillsFromDir({ + const second = await loadSkillsFromDir({ dir: join(collisionFixturesDir, "second"), source: "second", }); @@ -412,3 +413,27 @@ describe("skills", () => { }); }); }); + + +describe("loadSkills canonical path dedupe", () => { + it("loads the same file once when reached through an explicit dir and file path", async () => { + const root = mkdtempSync(join(tmpdir(), "senpi-skill-dedupe-")); + const skillDir = join(root, "dup-skill"); + mkdirSync(skillDir, { recursive: true }); + writeFileSync( + join(skillDir, "SKILL.md"), + "---\nname: dup-skill\ndescription: duplicate description\n---\n\nbody\n", + "utf8", + ); + + const { skills } = await loadSkills({ + cwd: root, + agentDir: join(root, "agent"), + skillPaths: [skillDir, join(skillDir, "SKILL.md")], + includeDefaults: false, + }); + + expect(skills).toHaveLength(1); + expect(skills[0]?.name).toBe("dup-skill"); + }); +}); From 2d732d31b0d854f289a3a2d6b238af85eb6af410 Mon Sep 17 00:00:00 2001 From: KDH Date: Wed, 2 Sep 2026 15:51:02 +0900 Subject: [PATCH 2/3] fix(interactive): keep the startup indicator through the TUI graph import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The startup loading indicator stopped right after createAgentSessionRuntime, before the dynamic import of the interactive-mode module graph — the single largest cold-start cost after resource loading — leaving a blank terminal that looked frozen. Stop it only after the import (still before the TUI starts writing stdout), so the spinner covers the wait. --- packages/coding-agent/src/main.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 86e791f72c..f1997367ac 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -1117,8 +1117,6 @@ export async function main(args: string[], options?: MainOptions) { cwd: sessionManager.getCwd(), agentDir, sessionManager, - }).finally(() => { - startupLoadingIndicator.stop(); }); time("createAgentSessionRuntime"); let selectedRuntime = runtime; @@ -1223,6 +1221,10 @@ export async function main(args: string[], options?: MainOptions) { // Keep the TUI graph out of headless RPC children. This is intentionally at the // mode seam: interactive startup still loads the same module before first use. const { InteractiveMode } = await import("./modes/interactive/interactive-mode.ts"); + // The startup indicator must keep spinning through the TUI graph import: it is the + // single largest cold-start cost after resource loading, and stopping before it + // leaves a blank terminal that looks frozen. + startupLoadingIndicator.stop(); const interactiveMode = new InteractiveMode(selectedRuntime, { migratedProviders, modelFallbackMessage, From 7eb6b8e2c0c6c52a56049efcaaba2f2bded71715 Mon Sep 17 00:00:00 2001 From: KDH Date: Wed, 2 Sep 2026 16:30:36 +0900 Subject: [PATCH 3/3] fix(coding-agent): address cubic review findings - reload(): await updateSkillsFromPaths so the loaded state is published before reload() completes (skills were assigned asynchronously after the caller observed the loader). - main.ts: stop the startup indicator in a finally around the interactive-mode import so an import rejection cannot leave the spinner running with the cursor hidden. - skills.test.ts: clean up the dedupe fixture temp dir. - resource-loader.test.ts / hooks-builtin-extension.test.ts: await the new Promise-returning extendResources API. --- packages/coding-agent/src/core/resource-loader.ts | 2 +- packages/coding-agent/src/main.ts | 10 +++++----- packages/coding-agent/test/resource-loader.test.ts | 6 +++--- packages/coding-agent/test/skills.test.ts | 10 +++++++--- .../test/suite/hooks-builtin-extension.test.ts | 2 +- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 6f9762576e..0972e4e036 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -687,7 +687,7 @@ export class DefaultResourceLoader implements ResourceLoader { : this.mergePaths([...cliEnabledSkills, ...enabledSkills], this.additionalSkillPaths); this.lastSkillPaths = skillPaths; - this.updateSkillsFromPaths(skillPaths, metadataByPath); + await this.updateSkillsFromPaths(skillPaths, metadataByPath); time("skills", "extensions"); for (const p of this.additionalSkillPaths) { if (isLocalPath(p)) { diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index f1997367ac..6b852a960a 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -1220,11 +1220,11 @@ export async function main(args: string[], options?: MainOptions) { } else if (appMode === "interactive") { // Keep the TUI graph out of headless RPC children. This is intentionally at the // mode seam: interactive startup still loads the same module before first use. - const { InteractiveMode } = await import("./modes/interactive/interactive-mode.ts"); - // The startup indicator must keep spinning through the TUI graph import: it is the - // single largest cold-start cost after resource loading, and stopping before it - // leaves a blank terminal that looks frozen. - startupLoadingIndicator.stop(); + const { InteractiveMode } = await import("./modes/interactive/interactive-mode.ts").finally(() => { + // The startup indicator must stop on success AND failure: an import rejection + // must not leave the spinner running with the cursor hidden. + startupLoadingIndicator.stop(); + }); const interactiveMode = new InteractiveMode(selectedRuntime, { migratedProviders, modelFallbackMessage, diff --git a/packages/coding-agent/test/resource-loader.test.ts b/packages/coding-agent/test/resource-loader.test.ts index e5c9851da9..18a5b2d34f 100644 --- a/packages/coding-agent/test/resource-loader.test.ts +++ b/packages/coding-agent/test/resource-loader.test.ts @@ -716,7 +716,7 @@ Extra prompt content`, const loader = new DefaultResourceLoader({ cwd, agentDir }); await loader.reload(); - loader.extendResources({ + await loader.extendResources({ skillPaths: [ { path: extraSkillDir, @@ -770,7 +770,7 @@ Extra content`, const loader = new DefaultResourceLoader({ cwd, agentDir }); await loader.reload(); - loader.extendResources({ + await loader.extendResources({ skillPaths: [ { path: pathToFileURL(extraSkillDir).href, @@ -865,7 +865,7 @@ Extension prompt content`, scope: "temporary", origin: "top-level", } as const; - loader.extendResources({ + await loader.extendResources({ skillPaths: [{ path: extensionSkillDir, metadata: extensionMetadata }], promptPaths: [{ path: extensionPromptsDir, metadata: extensionMetadata }], themePaths: [{ path: extensionThemesDir, metadata: extensionMetadata }], diff --git a/packages/coding-agent/test/skills.test.ts b/packages/coding-agent/test/skills.test.ts index f41392a402..451f9a658d 100644 --- a/packages/coding-agent/test/skills.test.ts +++ b/packages/coding-agent/test/skills.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, mkdirSync, writeFileSync } from "fs"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "fs"; import { homedir, tmpdir } from "os"; import { join, resolve } from "path"; import { describe, expect, it } from "vitest"; @@ -433,7 +433,11 @@ describe("loadSkills canonical path dedupe", () => { includeDefaults: false, }); - expect(skills).toHaveLength(1); - expect(skills[0]?.name).toBe("dup-skill"); + try { + expect(skills).toHaveLength(1); + expect(skills[0]?.name).toBe("dup-skill"); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); }); diff --git a/packages/coding-agent/test/suite/hooks-builtin-extension.test.ts b/packages/coding-agent/test/suite/hooks-builtin-extension.test.ts index 41b6f12e5f..af9ea107c5 100644 --- a/packages/coding-agent/test/suite/hooks-builtin-extension.test.ts +++ b/packages/coding-agent/test/suite/hooks-builtin-extension.test.ts @@ -159,7 +159,7 @@ describe("builtin hooks extension registration and resource plumbing", () => { await loader.reload(); // When - loader.extendResources({ + await loader.extendResources({ hookPaths: [ { path: runtimeHookPath,