diff --git a/packages/core/src/skills/skill-curator.test.ts b/packages/core/src/skills/skill-curator.test.ts index dfb3a0762b..4d3af0ad01 100644 --- a/packages/core/src/skills/skill-curator.test.ts +++ b/packages/core/src/skills/skill-curator.test.ts @@ -827,4 +827,53 @@ describe('auto-skill curator', () => { restoreArchivedAutoSkill(projectRoot, 'auto-skill-absent', now), ).rejects.toThrow('Archived auto-skill not found'); }); + + it('clamps a future manifest mtime so the skill remains curatable', async () => { + const now = new Date('2026-07-27T00:00:00.000Z'); + const old = new Date(now.getTime() - 180 * DAY_MS); + const future = new Date(now.getTime() + 10 * 365 * DAY_MS); + const manifest = await writeSkill('auto-skill-future', 'auto-skill', old); + await recordAutoSkillUsage( + projectRoot, + { name: 'future', level: 'project', filePath: manifest }, + old, + ); + // Stamp the manifest far in the future (clock skew, backup restore). + await fs.utimes(manifest, future, future); + + const result = await runAutoSkillCurator(projectRoot, { now }); + + expect(result.archived).toEqual(['auto-skill-future']); + }); + + it('does not double-list a directory present in both live and archived roots', async () => { + const now = new Date('2026-07-27T00:00:00.000Z'); + const old = new Date(now.getTime() - 100 * DAY_MS); + const manifest = await writeSkill('auto-skill-dup', 'auto-skill', old); + await recordAutoSkillUsage( + projectRoot, + { name: 'dup', level: 'project', filePath: manifest }, + old, + ); + await runAutoSkillCurator(projectRoot, { now }); + + // Recreate the same directory name in the live library. + await writeSkill('auto-skill-dup', 'auto-skill', now); + + const status = await getAutoSkillCuratorStatus(projectRoot, now); + + const allNames = [ + ...status.active, + ...status.stale, + ...status.archived, + ].map((entry) => entry.directoryName); + const dupCount = allNames.filter((n) => n === 'auto-skill-dup').length; + expect(dupCount).toBe(1); + expect(status.active.map((e) => e.directoryName)).toContain( + 'auto-skill-dup', + ); + expect(status.archived.map((e) => e.directoryName)).not.toContain( + 'auto-skill-dup', + ); + }); }); diff --git a/packages/core/src/skills/skill-curator.ts b/packages/core/src/skills/skill-curator.ts index 1d6d1ce2fe..af26de5ff8 100644 --- a/packages/core/src/skills/skill-curator.ts +++ b/packages/core/src/skills/skill-curator.ts @@ -72,9 +72,8 @@ async function readRegularFileNoFollow( if (stat.size > maxBytes) { throw new Error(`Auto-skill curator file too large at ${filePath}.`); } - const buffer = Buffer.allocUnsafe(stat.size); - const { bytesRead } = await handle.read(buffer, 0, stat.size, 0); - return { content: buffer.toString('utf8', 0, bytesRead), stat }; + const content = await handle.readFile('utf8'); + return { content, stat }; } finally { await handle.close().catch(() => {}); } @@ -479,9 +478,11 @@ function recordForSkill( function lastActivityMs( skill: ManagedAutoSkill, record: AutoSkillRecord, + nowMs: number, ): number { + const mtimeMs = parseTimestamp(skill.modifiedAt) ?? 0; return Math.max( - parseTimestamp(skill.modifiedAt) ?? 0, + mtimeMs <= nowMs ? mtimeMs : 0, parseTimestamp(record.firstSeenAt) ?? 0, parseTimestamp(record.lastActivityAt) ?? 0, parseTimestamp(record.lastUsedAt) ?? 0, @@ -492,12 +493,15 @@ function entryFor( skill: ManagedAutoSkill, record: AutoSkillRecord, state: AutoSkillState, + nowMs: number, ): AutoSkillCuratorEntry { return { directoryName: skill.directoryName, skillName: skill.skillName, state, - lastActivityAt: new Date(lastActivityMs(skill, record)).toISOString(), + lastActivityAt: new Date( + lastActivityMs(skill, record, nowMs), + ).toISOString(), useCount: record.useCount, pinned: record.pinned, }; @@ -545,14 +549,15 @@ async function runLocked( continue; } if (record.pinned) continue; - const inactivityMs = nowMs - lastActivityMs(scanned, record); + const inactivityMs = nowMs - lastActivityMs(scanned, record, nowMs); if (inactivityMs >= AUTO_SKILL_ARCHIVE_AFTER_MS) { const current = await readManagedSkill( paths.skillsRoot, scanned.directoryName, ); if (!current) continue; - const currentInactivityMs = nowMs - lastActivityMs(current, record); + const currentInactivityMs = + nowMs - lastActivityMs(current, record, nowMs); if (currentInactivityMs < AUTO_SKILL_ARCHIVE_AFTER_MS) { if (currentInactivityMs >= AUTO_SKILL_STALE_AFTER_MS) { if (record.state !== 'stale') { @@ -628,6 +633,7 @@ async function previewRun( archived: [], skippedCollisions: [], }; + const nowMs = now.getTime(); for (const skill of skills) { const existing = state.skills[skill.directoryName]; if (!existing) { @@ -636,7 +642,7 @@ async function previewRun( } const record = recordForSkill(state, skill); if (record.pinned) continue; - const inactivityMs = now.getTime() - lastActivityMs(skill, record); + const inactivityMs = nowMs - lastActivityMs(skill, record, nowMs); if (inactivityMs >= AUTO_SKILL_ARCHIVE_AFTER_MS) { try { await fs.lstat(path.join(paths.archiveRoot, skill.directoryName)); @@ -675,11 +681,21 @@ export async function maybeRunAutoSkillCurator( projectRoot: string, now: Date = new Date(), ): Promise { - return withCuratorLock(projectRoot, async (paths) => { - const state = await readState(paths.statePath); + const paths = getCuratorPaths(projectRoot); + // Fast path: readState is safe unlocked (atomic-rename writes prevent torn + // reads), so most boots return not_due without paying for the lock. + const unlocked = await readState(paths.statePath); + if (unlocked.lastRunAt) { + const lastRunMs = parseTimestamp(unlocked.lastRunAt)!; + if (now.getTime() - lastRunMs < AUTO_SKILL_CURATOR_INTERVAL_MS) { + return { status: 'not_due' }; + } + } + return withCuratorLock(projectRoot, async (lockedPaths) => { + const state = await readState(lockedPaths.statePath); if (!state.lastRunAt) { const nowIso = now.toISOString(); - const skills = await scanManagedSkills(paths.skillsRoot); + const skills = await scanManagedSkills(lockedPaths.skillsRoot); for (const skill of skills) { const existing = state.skills[skill.directoryName]; state.skills[skill.directoryName] = { @@ -699,17 +715,21 @@ export async function maybeRunAutoSkillCurator( }; } state.lastRunAt = nowIso; - await atomicWriteJSON(paths.statePath, state, { + await atomicWriteJSON(lockedPaths.statePath, state, { mode: 0o600, noFollow: true, }); return { status: 'seeded', checked: skills.length }; } + // Re-check under lock: another process may have run while we waited. const lastRunMs = parseTimestamp(state.lastRunAt)!; if (now.getTime() - lastRunMs < AUTO_SKILL_CURATOR_INTERVAL_MS) { return { status: 'not_due' }; } - return { status: 'ran', result: await runLocked(paths, state, now) }; + return { + status: 'ran', + result: await runLocked(lockedPaths, state, now), + }; }); } @@ -791,9 +811,10 @@ export async function getAutoSkillCuratorStatus( stale: [], archived: [], }; + const nowMs = now.getTime(); for (const skill of liveSkills) { const record = recordForSkill(state, skill, now.toISOString()); - const inactivityMs = now.getTime() - lastActivityMs(skill, record); + const inactivityMs = nowMs - lastActivityMs(skill, record, nowMs); const effectiveState: AutoSkillState = record.pinned ? record.state === 'stale' ? 'stale' @@ -801,11 +822,13 @@ export async function getAutoSkillCuratorStatus( : inactivityMs >= AUTO_SKILL_STALE_AFTER_MS ? 'stale' : 'active'; - status[effectiveState].push(entryFor(skill, record, effectiveState)); + status[effectiveState].push(entryFor(skill, record, effectiveState, nowMs)); } + const liveNames = new Set(liveSkills.map((s) => s.directoryName)); for (const skill of archivedSkills) { + if (liveNames.has(skill.directoryName)) continue; const record = recordForSkill(state, skill); - status.archived.push(entryFor(skill, record, 'archived')); + status.archived.push(entryFor(skill, record, 'archived', nowMs)); } return status; }