From a4de1d30c5ef8620f58cdd851de8df0a4d07f3a4 Mon Sep 17 00:00:00 2001 From: Derek Pearson Date: Sun, 22 Mar 2026 06:22:23 -0400 Subject: [PATCH] fix(skills): address QA round 3 - Fix Windows fd leak: close markerFd before unlinkSync in catch block - Increase pbxproj read limit from 256KB to 1MB (SDKROOT appears after file references/build phases in large projects) - Fix matchPacksForProject docstring: returns catalog order, not sorted - Add legacy dir to skill telemetry (captureAvailableSkills, detectStaleSkills) so pre-migration skills appear in health/telemetry reports --- src/resource-loader.ts | 6 ++++-- src/resources/extensions/gsd/detection.ts | 4 ++-- src/resources/extensions/gsd/skill-catalog.ts | 2 +- src/resources/extensions/gsd/skill-telemetry.ts | 11 +++++++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/resource-loader.ts b/src/resource-loader.ts index a56754121..8d09f13b5 100644 --- a/src/resource-loader.ts +++ b/src/resource-loader.ts @@ -495,10 +495,12 @@ function migrateSkillsToEcosystemDir(agentDir: string): void { // Write migration info to the marker try { writeFileSync(markerFd, `Migrated ${migrated} skill(s) to ${ecosystemDir} on ${new Date().toISOString()}\n`) } catch { /* non-fatal */ } } catch { - // can't create ecosystem dir or read legacy dir — remove marker so we retry next launch + // can't create ecosystem dir or read legacy dir — close fd first (required on Windows + // where unlinkSync fails on open handles), then remove marker so we retry next launch + try { closeSync(markerFd); markerFd = -1 } catch { /* non-fatal */ } try { unlinkSync(markerPath) } catch { /* non-fatal */ } } finally { - try { closeSync(markerFd) } catch { /* non-fatal */ } + if (markerFd !== -1) { try { closeSync(markerFd) } catch { /* non-fatal */ } } } } diff --git a/src/resources/extensions/gsd/detection.ts b/src/resources/extensions/gsd/detection.ts index 4d4f6b27c..08619893d 100644 --- a/src/resources/extensions/gsd/detection.ts +++ b/src/resources/extensions/gsd/detection.ts @@ -377,7 +377,7 @@ const XCODE_SUBDIRS = ["ios", "macos", "app", "apps"] as const; * Returns deduplicated, canonical platform list (e.g. ["iphoneos"]). * * Reading the pbxproj is a lightweight regex scan — no full plist parsing needed. - * We read at most 256 KB per file to keep detection fast. + * We read at most 1 MB per file to keep detection fast. * Searches both the project root and common subdirectories (ios/, macos/, app/). */ function detectXcodePlatforms(basePath: string): XcodePlatform[] { @@ -397,7 +397,7 @@ function detectXcodePlatforms(basePath: string): XcodePlatform[] { if (!entry.isDirectory() || !entry.name.endsWith(".xcodeproj")) continue; const pbxprojPath = join(dir, entry.name, "project.pbxproj"); try { - const content = readBounded(pbxprojPath, 256 * 1024); + const content = readBounded(pbxprojPath, 1024 * 1024); // Match SDKROOT = ; — both quoted and unquoted forms const sdkRe = /SDKROOT\s*=\s*"?([a-z]+)"?\s*;/gi; let m: RegExpExecArray | null; diff --git a/src/resources/extensions/gsd/skill-catalog.ts b/src/resources/extensions/gsd/skill-catalog.ts index 3b5c162e5..3477be097 100644 --- a/src/resources/extensions/gsd/skill-catalog.ts +++ b/src/resources/extensions/gsd/skill-catalog.ts @@ -399,7 +399,7 @@ export const GREENFIELD_STACKS: Array<{ /** * Match project signals to relevant skill packs. - * Returns packs ordered by relevance (language match first, then file match). + * Returns packs in catalog order (not sorted by match type). */ export function matchPacksForProject(signals: ProjectSignals): SkillPack[] { const matched = new Set(); diff --git a/src/resources/extensions/gsd/skill-telemetry.ts b/src/resources/extensions/gsd/skill-telemetry.ts index edf6ff81b..33c427c4c 100644 --- a/src/resources/extensions/gsd/skill-telemetry.ts +++ b/src/resources/extensions/gsd/skill-telemetry.ts @@ -31,7 +31,12 @@ const activelyLoadedSkills = new Set(); */ export function captureAvailableSkills(): void { const skillsDir = join(homedir(), ".agents", "skills"); - availableSkills = listSkillNames(skillsDir); + const legacyDir = join(homedir(), ".gsd", "agent", "skills"); + const names = listSkillNames(skillsDir); + // Include skills still in the legacy directory (pre-migration users) + const legacyNames = listSkillNames(legacyDir); + const all = new Set([...names, ...legacyNames]); + availableSkills = [...all]; activelyLoadedSkills.clear(); } @@ -100,7 +105,9 @@ export function detectStaleSkills( // Check all installed skills, not just those with usage data const skillsDir = join(homedir(), ".agents", "skills"); - const installed = listSkillNames(skillsDir); + const legacyDir = join(homedir(), ".gsd", "agent", "skills"); + const installedSet = new Set([...listSkillNames(skillsDir), ...listSkillNames(legacyDir)]); + const installed = [...installedSet]; for (const skill of installed) { const lastTs = lastUsed.get(skill);