diff --git a/.docs/index.md b/.docs/index.md index 497109f..01ae75b 100644 --- a/.docs/index.md +++ b/.docs/index.md @@ -12,3 +12,7 @@ title: Documentation Index - [Config Matching](technical/config-matching.md): How agnos resolves glob-aware documentation ignores and rule fragments. - [MCP Env Resolution](technical/mcp-env-resolution.md): How MCP env key declarations are resolved into secret-bearing agent files. - [Skills Reconciliation](technical/skills-reconciliation.md): How declared skill sources are reconciled with materialized skills and lock entries. + +### technical-decisions + +- [Global Skill Store](technical-decisions/global-skill-store.md): Records why Agnos shares verified skill content globally while keeping project materializations independent. diff --git a/.docs/technical-decisions/global-skill-store.md b/.docs/technical-decisions/global-skill-store.md new file mode 100644 index 0000000..b730911 --- /dev/null +++ b/.docs/technical-decisions/global-skill-store.md @@ -0,0 +1,32 @@ +--- +type: Technical Decision +title: Global Skill Store +description: Records why Agnos shares verified skill content globally while keeping project materializations independent. +resource: "" +tags: [skills, storage, caching] +timestamp: 2026-08-13T00:00:00Z +--- + +# Global Skill Store + +## Decision + +Agnos uses one versioned content-addressed skill store per user. Projects import independent materializations from that store and use their lockfiles to select exact content hashes and commits. + +## Rationale + +- Identical locked skills are downloaded and stored once across repositories. +- Hash verification makes shared reuse deterministic and detects accidental corruption. +- Independent project materializations continue working after the store is removed or pruned. +- Exact commit fetches reproduce missing entries after a tracked branch advances. + +## Consequences + +- The global store is part of the current user's trust boundary. +- Cross-project offline reuse requires a committed lockfile. +- Project materialization consumes filesystem metadata and may consume full file data when cloning and hard links are unavailable. +- Machine-specific store placement remains outside project configuration. + +## References + +- Reconciliation behavior: [Skills Reconciliation](../technical/skills-reconciliation.md). diff --git a/.docs/technical/skills-reconciliation.md b/.docs/technical/skills-reconciliation.md index 4d76d69..2245aae 100644 --- a/.docs/technical/skills-reconciliation.md +++ b/.docs/technical/skills-reconciliation.md @@ -27,9 +27,48 @@ timestamp: 2026-07-09T00:00:00Z - The skills domain prunes before installing or updating skills. - A normal `agnos` run prunes before materializing declared skills. - If no skills are declared, pruning still runs so stale materialized skills can be removed. +- Skill reconciliation runs with bounded concurrency while preserving declaration order in its result. +- Concurrent requests for the same Git source are coalesced into one fetch. +- Git checkouts use an isolated per-run workspace under `.agnos/tmp/repos`. +- Repository workspaces are removed at the end of each command or domain run, including when skills are moved, changed, or fail to install. + +## Content-addressed storage + +- Skill content is stored once per user in a global, versioned store selected from the operating system data directory. +- `AGNOS_STORE_DIR` overrides the global store base for installations that need a custom location. +- Every store reuse recomputes the skill hash. Invalid entries are replaced through atomic publication. +- The configured skills route contains an independent project materialization imported with copy-on-write cloning, hard links, or copies. +- Project materializations remain usable if the global store is removed. +- A pinned remote skill can be restored from its lock entry and global stored content without a Git operation. +- Local skill sources are still hashed on each reconciliation so local edits are detected. +- `agnos skills integrity` explicitly hashes project materializations instead of trusting tool-managed state. +- Updates bypass any checkout already staged during the current run before computing and accepting a new hash. +- Store publication never deletes the final hash path, tolerates concurrent publishers, and retries transient Windows filesystem errors. +- Lock updates use temporary files and atomic publication so concurrent readers never observe partial JSON. + +## Reproducible fetches + +- New remote lock entries record the commit SHA returned by the checkout that supplied the accepted content. +- A missing global entry is refetched at the locked commit rather than the current branch head. +- Legacy lock entries without a commit fetch their tracked ref once and are backfilled only when the content hash still matches. + +## Legacy migration + +- Referenced entries from `.agnos/cache/skills` are verified and promoted into the global store. +- Existing project links are replaced with independent materializations before the legacy cache is removed. +- A failed or incomplete migration retains the project cache and reports a warning. + +## Progress reporting + +- Interactive installs show a live completion percentage. +- The progress line reports the declared skill total, content-store reuses, and source fetches. +- A content-store hit increments `reused`. A skill loaded successfully from its declared source increments `fetched`. +- Quiet and non-interactive runs omit the transient progress line. +- Successful interactive and non-interactive installs emit a final 100 percent summary with the same counters. ## References - Skills command surface: [index.ts](../../src/domains/skills/index.ts). - Skills steps: [steps.ts](../../src/domains/skills/steps.ts). - Skills pipeline tests: [skills-pipeline.test.ts](../../test/domains/skills-pipeline.test.ts). +- Global store decision: [Global Skill Store](../technical-decisions/global-skill-store.md). diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..19cf449 --- /dev/null +++ b/.env.example @@ -0,0 +1,20 @@ +# —————————————————————————————————————————————————————————————————————————— +# ——— Agnos: Local CLI configuration + +# Use : Override the user-level content-addressed skill store location. +# Source: Choose a writable directory trusted by the current user. +# Path : Local filesystem > Agnos store +# E.g. : C:\Users\name\AppData\Local\agnos\store +AGNOS_STORE_DIR= + +# Use : Enable diagnostic CLI logging. +# Source: Set locally when troubleshooting Agnos. +# Path : Local shell > Environment +# E.g. : 1 +AGNOS_DEBUG= + +# Use : Override the Model Context Protocol registry endpoint. +# Source: Use the endpoint supplied by the registry operator. +# Path : MCP registry > API endpoint +# E.g. : https://registry.modelcontextprotocol.io +AGNOS_MCP_REGISTRY=https://registry.modelcontextprotocol.io diff --git a/.gitignore b/.gitignore index 0a3c8b3..37c5849 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ scratch/ # ENV .env* +!.env.example # AGENTS .agents/ diff --git a/.prettierignore b/.prettierignore index 0dde962..6690a30 100644 --- a/.prettierignore +++ b/.prettierignore @@ -7,3 +7,4 @@ pnpm-lock.yaml test-output scratch packages/*/dist +.env.example diff --git a/README.md b/README.md index c5d6829..492fb17 100644 --- a/README.md +++ b/README.md @@ -221,7 +221,7 @@ Injects titled sections (by frontmatter `title`) from fragment files into your c ### `skills` -Fetches, pins, verifies, and installs skills into the canonical skills dir (linked per-agent by `agents`). +Fetches, pins, verifies, and installs skills into the canonical skills dir (linked per-agent by `agents`). Install work is concurrent, with live percentage, total, reused, and fetched counters. Each unique locked skill tree is stored once in a user-level content store shared across repositories. Projects receive independent materializations, so removing the store does not break installed skills. Git checkouts are isolated under `.agnos/tmp/repos/` for the current run and always removed afterward. Set `AGNOS_STORE_DIR` to override the operating system data location used by the store. | Subcommand | Args / Flags | Description | | ----------- | --------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | diff --git a/skills/atomic-design/SKILL.md b/skills/atomic-design/SKILL.md new file mode 100644 index 0000000..8d43860 --- /dev/null +++ b/skills/atomic-design/SKILL.md @@ -0,0 +1,131 @@ +--- +name: atomic-design +description: Atomic design methodology with type-filtered slot composition for UI implementation, validation, review, and audits. Use when implementing, validating, reviewing, or auditing frontend/UI code through atomic design principles, component hierarchy, atoms, molecules, organisms, templates, pages, reusable design systems, React-style compound components, named slots, child filtering by type, fixed slot positioning, Storybook or component state coverage, content structure, and page variation resilience. +--- + +# Atomic Design + +Apply atomic design as a UI design-system mental model, not as a rigid build sequence. Organize components by logical responsibility using atomic design, then compose their variable regions through slots. + +Read [Atomic Design Methodology](references/atomic-design-methodology.md) when you need the stage taxonomy, React-oriented placement rules, source notes, or detailed audit prompts. + +Read [Slot-Based Composition](references/slot-based-composition.md) when you need compound component exports, child filtering by type, fixed slot positioning, or review prompts for slot composition. + +## Workflow + +1. Inspect the existing UI architecture before introducing atomic vocabulary. +2. Preserve the repo's naming, routing, styling, testing, and component patterns unless the user explicitly asks for a reorganization. +3. Map the target code to the smallest atomic stage that can own the responsibility. +4. Expose variable regions as slots when a component has stable structure but flexible content. +5. Check the same component in isolation and in its composed page context. +6. Validate with the repo's normal commands and with rendered UI inspection when visual behavior matters. + +## Stage Ownership + +| Stage | Owns | Avoid | +| -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| Atom | Primitive UI elements, design tokens in use, accessible base controls, typographic primitives, icons, inputs, buttons. | Page-specific layout, margins, route data, application workflows. | +| Molecule | Small functional groups of atoms, such as search fields, form rows, nav items, summary badges, or card headers. | Full page sections, global data fetching, unrelated optional regions. | +| Organism | Distinct interface sections composed of atoms, molecules, or other organisms, such as headers, product grids, checkout forms, sidebars, and feature panels. | Route ownership, full-page layout skeletons, hard-coded page-only content. | +| Template | Page-level layout and content structure, including grid regions, slots, skeletons, and constraints for dynamic content. | Final production copy, business-specific records, route side effects. | +| Page | Real representative content, route integration, application state wiring, user-role or data-volume variations, and final resilience checks. | Reusable component internals that belong lower in the hierarchy. | + +## Implementation Rules + +- Prefer the existing component taxonomy. Introduce `atoms`, `molecules`, `organisms`, `templates`, and `pages` folders only when that matches or improves the local system. +- Keep dependencies flowing upward through the hierarchy: atoms should not import molecules, molecules may import atoms, organisms may import molecules and atoms, templates may import organisms and lower stages, and pages may wire all stages together. +- Keep atoms portable. They can expose variants and states, but they must not assume where they sit on a page. +- Compose molecules from atoms to create one focused function. Split a molecule when independent concerns start sharing props, state, or styles. +- Compose organisms as reusable interface sections. They may coordinate child layout but should stay independent enough to work in multiple page contexts. +- Keep templates about structure. They arrange regions and define content constraints without binding final records, permissions, or navigation behavior. +- Use pages to connect templates and components to real app data, routing, representative content, and meaningful variations. +- Centralize design tokens or variables according to the repo's existing style system. Do not duplicate token values inside components. +- Cover meaningful states where the repo supports component examples, stories, screenshots, or interaction tests. + +## Slot-Based Composition Rules + +- Use slots when props like `headerContent`, `footerActions`, `leftIcon`, or `descriptionNode` begin to multiply or when callers need to provide real JSX while the component owns structure. +- Prefer semantic compound slots such as `Card.Header`, `Card.Body`, `Card.Footer`, `Dialog.Title`, or `Toolbar.Action` for reusable molecules and organisms. +- Colocate slot components inside the owning component folder and export them from the root component API. +- Put generic child filtering in a shared `prepareComponentSlots` helper, then wrap it in a colocated hook named like `useSlots`. +- Let slot components import lower-stage components when needed. For example, a molecule slot may import an atom, but an atom slot must not import a molecule. +- Define slot components as function components. Type filtering compares `child.type` with the function reference registered in the slot map. +- Keep slot names semantic, not incidental. Prefer `Title`, `Description`, `Actions`, `Media`, and `Footer` over `Top`, `Left`, or `BlueArea` unless the component is explicitly a layout primitive. +- Filter slot children by component type when the parent owns positioning. The consumer may write slots in a readable order, but the parent renders each recognized slot into its defined region. +- Preserve DOM reading order and accessibility. Do not visually reorder slots in a way that creates a different keyboard or screen-reader order. + +Example organization: + +```text +src/ + components/ + atoms/ + component-a/ + index.tsx + molecules/ + component-b/ + title.tsx + body.tsx + use-component-b-slots.ts + index.tsx +``` + +Example slot hook: + +```ts +import type { ReactNode } from "react"; + +import { prepareComponentSlots } from "../../hooks/prepare-component-slots.js"; +import { Body } from "./body.js"; +import { Title } from "./title.js"; + +const usePreparedComponentBSlots = prepareComponentSlots({ + body: [Body], + title: Title, +}); + +export function useComponentBSlots(children: ReactNode) { + return usePreparedComponentBSlots(children); +} +``` + +Example root export: + +```tsx +import type { ReactNode } from "react"; + +import { Body } from "./body.js"; +import { Title } from "./title.js"; +import { useComponentBSlots } from "./use-component-b-slots.js"; + +export interface ComponentBProps { + children: ReactNode; +} + +export function ComponentBRoot({ children }: ComponentBProps) { + const { body, title } = useComponentBSlots(children); + + return ( +
+ {title} +
{body}
+
+ ); +} + +export const ComponentB = Object.assign(ComponentBRoot, { + Body, + Title, +}); +``` + +## Review And Audit Rules + +- Report misplaced responsibilities as concrete code findings: atom with page layout, molecule doing route work, organism hard-coding one page, template owning production content, or page duplicating component internals. +- Check whether components remain reusable with different content lengths, empty states, disabled states, roles, and data volumes. +- Flag styling that breaks portability, especially margins and positioning buried in low-level atoms. +- Look for duplicate primitives or one-off components that should share an atom or molecule. +- Flag prop-heavy components that should expose named slots or compound subcomponents. +- Flag slots that break atomic dependency direction, hide business logic in reusable component internals, or require consumers to know private child ordering. +- Verify that templates expose content structure and that pages prove the structure with real representative content. +- Treat atomic design as a communication and resilience model. Do not require the exact stage names if the repo uses another clear taxonomy. diff --git a/skills/atomic-design/references/atomic-design-methodology.md b/skills/atomic-design/references/atomic-design-methodology.md new file mode 100644 index 0000000..556406e --- /dev/null +++ b/skills/atomic-design/references/atomic-design-methodology.md @@ -0,0 +1,71 @@ +# Atomic Design Methodology + +## Sources + +- [Atomic Design Methodology](https://atomicdesign.bradfrost.com/chapter-2/) by Brad Frost. +- [Atomic Design and ReactJS](https://danilowoz.com/blog/atomic-design-with-react) by Danilo Woznica. +- [danilowoz/react-atomic-design](https://github.com/danilowoz/react-atomic-design), used as a historical React/Storybook implementation example. + +## Core Model + +Atomic design breaks UI systems into five related stages: atoms, molecules, organisms, templates, and pages. Use the stages concurrently. The point is to move between abstract parts and concrete interfaces, not to build every atom before touching a page. + +Atomic design applies to user interfaces broadly. It is not a CSS architecture, JavaScript architecture, React requirement, or folder-name mandate. Use the local codebase's language when it is clearer, but preserve the hierarchy of responsibility. + +## Stage Guide + +| Stage | Definition | Good implementation signals | Audit failures | +| --------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| Atoms | The smallest functional UI building blocks, such as labels, inputs, buttons, icons, images, typography, colors, and animation primitives. | Accessible defaults, explicit variants, token-driven styles, no knowledge of page placement. | Margins or positioning baked into controls, business records in props, inaccessible states, duplicate primitives. | +| Molecules | Simple groups of atoms working as one functional unit. A label, input, and button can become a search form molecule. | Single responsibility, portable behavior, small prop surface, states documented in isolation. | Too many unrelated options, duplicated atom behavior, direct route or global app coupling. | +| Organisms | More complex sections composed from molecules, atoms, or other organisms. Examples include headers, product grids, forms, and feature sections. | Distinct reusable interface section, clear slots or data contracts, resilient composition. | Hard-coded page copy, page-only layout assumptions, hidden dependencies on a single route. | +| Templates | Page-level layout objects that place components into structure and reveal content constraints. | Grid or slot ownership, skeleton states, character length and media constraints, placeholder or representative structure. | Production records, permissions, data fetching, route side effects, component internals. | +| Pages | Specific instances of templates with real representative content and app integration. | Route wiring, user roles, empty and loaded states, long and short content, real interaction paths. | Repeating lower-level markup, hiding content stress cases, only testing happy-path copy. | + +## React-Oriented Guidance + +Use these rules as architecture guidance, not as framework requirements: + +- Keep shared variables, tokens, and theme primitives centralized. +- Keep atoms free of page-specific margins and positioning. +- Let molecules and organisms compose and arrange their children, but keep their layout portable across contexts. +- Let templates define page grids, slots, and content structure. +- Let pages bind templates to real app state, routing, representative content, and variations. +- Use component examples or Storybook stories to show each meaningful state separately when the repo has that tooling. + +The source repository demonstrates a common structure: + +```text +src/components/ + _settings/ + atoms// + molecules// + organisms// + templates// +``` + +Each sample component colocates implementation, styles, and stories. Borrow the colocated-example idea when it fits the repo, but do not copy its older Flow, Yarn, CSS Modules, or Webpack choices unless the project already uses them. + +## Implementation Prompts + +Use these questions while building: + +- What is the smallest stage that should own this responsibility? +- Will the component work with different content, sizes, themes, and disabled or empty states? +- Is layout owned by the parent level rather than a low-level primitive? +- Does the template show the content structure without taking over page data? +- Does the page prove the design with real representative content and variations? +- Are component examples or tests covering the states users will actually encounter? + +## Review Prompts + +Use these checks while validating, reviewing, or auditing: + +- Are atoms reusable without page context? +- Are molecules focused on one functional grouping? +- Are organisms complete interface sections without owning the route? +- Are templates structural instead of content-specific? +- Are pages connecting real app state without duplicating lower-level component internals? +- Are content stress cases represented, including long text, missing media, empty lists, role differences, and varied item counts? +- Are style tokens reused instead of copied? +- Are states discoverable through tests, stories, screenshots, or examples available in the repo? diff --git a/skills/atomic-design/references/prepareComponentSlots.ts b/skills/atomic-design/references/prepareComponentSlots.ts new file mode 100644 index 0000000..9daaf04 --- /dev/null +++ b/skills/atomic-design/references/prepareComponentSlots.ts @@ -0,0 +1,44 @@ +import { Children, isValidElement, useMemo } from "react"; +import type { ReactElement, ReactNode } from "react"; + +type SlotComponent = (props: never) => ReactNode; +type SlotDefinition = SlotComponent | readonly SlotComponent[]; +type PreparedSlots = Record; + +export function prepareComponentSlots( + definitions: Record, +): (children: ReactNode) => PreparedSlots { + const entries = Object.entries(definitions); + + return function usePreparedComponentSlots(children: ReactNode): PreparedSlots { + return useMemo(() => { + const preparedSlots: PreparedSlots = {}; + + for (const [key, definition] of entries) { + preparedSlots[key] = Array.isArray(definition) ? [] : null; + } + + for (const child of Children.toArray(children)) { + if (!isValidElement(child)) continue; + + const match = entries.find(([, definition]) => { + const slotComponents = Array.isArray(definition) ? definition : [definition]; + return slotComponents.includes(child.type as SlotComponent); + }); + + if (match == null) continue; + + const [key, definition] = match; + + if (Array.isArray(definition)) { + (preparedSlots[key] as ReactElement[]).push(child); + continue; + } + + preparedSlots[key] = child; + } + + return preparedSlots; + }, [children]); + }; +} diff --git a/skills/atomic-design/references/slot-based-composition.md b/skills/atomic-design/references/slot-based-composition.md new file mode 100644 index 0000000..765ef70 --- /dev/null +++ b/skills/atomic-design/references/slot-based-composition.md @@ -0,0 +1,154 @@ +# Slot-Based Composition + +## Sources + +- [Building Component Slots in React](https://sandroroth.com/blog/react-slots/) by Sandro Roth. +- [What is the React Slots pattern?](https://dev.to/neetigyachahar/what-is-the-react-slots-pattern-2ld9) by Neetigya Chahar. +- [Slot-Based APIs in React: Designing Flexible and Composable Components](https://dev.to/talissoncosta/slot-based-apis-in-react-designing-flexible-and-composable-components-7pj) by Talisson Costa. + +## Model + +Slot-based composition gives a component controlled insertion points for caller-provided content. Atomic design decides where a component belongs. Type-filtered slots decide how callers fill the component's stable structure while the parent guarantees positioning. + +Use this pattern when a molecule, organism, or template has named regions that should stay structurally consistent while accepting flexible JSX. Avoid using it as ceremony for components with one simple `children` region. + +## Required Pattern + +- Expose semantic compound slot components from the parent API, such as `Card.Title`, `Card.Description`, and `Card.Actions`. +- Keep slot components colocated with the owning component. +- In the parent root component, inspect direct children and filter recognized slot components by their component type. +- Put generic child filtering logic in a shared `prepareComponentSlots` helper. +- Wrap the shared helper in a colocated component hook named like `useSlots`. +- Define slot components as function components. Type filtering compares `child.type` with the function reference registered in the slot map. +- Render each recognized slot in the parent's fixed structural position. +- Use a single component value for slots that should default to `null`, such as `title: Title`. +- Use an array containing one component value for slots that should default to an empty array, such as `body: [Body]`. +- Document behavior for missing slots, duplicated slots, unknown children, and ordering. + +## Atomic Placement + +- Atoms may expose a default slot through `children` for primitive content. +- Molecules are the usual home for named slots because they group atoms into one small function. +- Organisms may expose larger slots for section regions, actions, filters, media, empty states, or summaries. +- Templates may expose layout slots or render props for page regions, but they must not own final route data. +- Pages fill slots with real content, route wiring, permissions, and state variations. + +## Folder Shape + +Use the owning component folder as the boundary for slot files: + +```text +src/components/ + atoms/ + component-a/ + index.tsx + molecules/ + component-b/ + title.tsx + body.tsx + use-component-b-slots.ts + index.tsx +``` + +The slot file can import lower-stage components: + +```tsx +import type { ReactNode } from "react"; + +import { ComponentA } from "../../atoms/component-a/index.js"; + +export interface TitleProps { + children: ReactNode; +} + +export function Title({ children }: TitleProps) { + return {children}; +} +``` + +The shared helper is declared in [prepareComponentSlots](./prepareComponentSlots.ts). Copy it into the project's shared hooks or utilities area using the project's local file naming and import conventions. + +The colocated component hook wraps the shared helper: + +```tsx +import type { ReactNode } from "react"; + +import { prepareComponentSlots } from "../../hooks/prepare-component-slots.js"; +import { Body } from "./body.js"; +import { Title } from "./title.js"; + +const usePreparedComponentBSlots = prepareComponentSlots({ + body: [Body], + title: Title, +}); + +export function useComponentBSlots(children: ReactNode) { + return usePreparedComponentBSlots(children); +} +``` + +The root file consumes the colocated hook, exports the component, and attaches its slots: + +```tsx +import type { ReactNode } from "react"; + +import { Body } from "./body.js"; +import { Title } from "./title.js"; +import { useComponentBSlots } from "./use-component-b-slots.js"; + +export interface ComponentBProps { + children: ReactNode; +} + +export function ComponentBRoot({ children }: ComponentBProps) { + const { body, title } = useComponentBSlots(children); + + return ( +
+ {title} +
{body}
+
+ ); +} + +export const ComponentB = Object.assign(ComponentBRoot, { + Body, + Title, +}); +``` + +Consumers compose through the parent API: + +```tsx + + Additional body content. + Project status + +``` + +The parent still renders the title and body in the order defined by `ComponentBRoot`. + +With `prepareComponentSlots`, missing single slots resolve to `null`, missing array slots resolve to `[]`, duplicate single slots use the last matching child, duplicate array slots collect every matching child, and unknown children are ignored. + +## Implementation Rules + +- Prefer one composition pattern per component family. +- Keep slot components small and semantic. They should name a region, provide structure, or adapt lower-level components. +- Keep business decisions at pages or feature-level integration points, not inside reusable slot components. +- Preserve accessible DOM order. If the parent normalizes slot regions, its rendered order must be the correct reading and keyboard order. +- When normalizing slots by child type, provide clear behavior for missing, duplicated, unknown, and fallback children. +- Keep child filtering shallow unless the component explicitly documents nested slot support. +- Keep the hook focused on resolution. Rendering and layout stay in the root component. +- Keep the generic helper shared and the component-specific hook colocated with the component. + +## Review Prompts + +- Does the atomic stage own the component's responsibility? +- Does the slot API reduce prop bloat without hiding required structure? +- Are slots named after domain or semantic regions? +- Can consumers omit optional slots without layout breakage? +- Are required slots validated by types, tests, stories, or runtime guards according to local practice? +- Does the implementation preserve accessible DOM order and keyboard flow? +- Are lower-stage imports flowing in the correct direction? +- Is slot resolution implemented once in a hook instead of duplicated across roots, stories, or tests? +- Are examples or stories covering default content, each named slot, missing optional slots, duplicated slots, unknown children, and consumer order that differs from rendered order? diff --git a/src/core/cli.ts b/src/core/cli.ts index c7aacb8..c9e0ddd 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -113,7 +113,11 @@ async function main(): Promise { args: rest, logger: withDomain(ctx.logger, dom.domain), }; - await cmd.run(cmdCtx); + try { + await cmd.run(cmdCtx); + } finally { + await cmdCtx.fetcher.cleanup(); + } return; } diff --git a/src/core/context.ts b/src/core/context.ts index 5aa7473..c4deb88 100644 --- a/src/core/context.ts +++ b/src/core/context.ts @@ -4,6 +4,7 @@ import { createRepoFetcher } from "./resolver.js"; import { createLogger } from "./logger.js"; import { buildPaths, ensureDir } from "./paths.js"; import { readConfigOrDefault } from "./config.js"; +import { resolveGlobalStoreDir } from "./store-path.js"; import type { AgnosConfig, Logger, ResolveContext } from "./types/public.js"; export interface BuildContextOptions { @@ -12,29 +13,32 @@ export interface BuildContextOptions { dryRun?: boolean; logger?: Logger; config?: AgnosConfig; + storeDir?: string; } export async function buildResolveContext(opts: BuildContextOptions): Promise { const config = opts.config ?? (await readConfigOrDefault(path.join(opts.projectRoot, "agnos.json"))); const paths = buildPaths(opts.projectRoot, config); + const logger = opts.logger ?? createLogger(); if (!opts.dryRun) { await ensureDir(paths.agnosRoot); - await ensureDir(paths.cacheDir); + await ensureDir(paths.tempDir); } - const logger = opts.logger ?? createLogger(); const linker = createLinker({ - cacheDir: paths.cacheDir, + probeDir: path.join(paths.tempDir, "link-probes"), logger, copyFallback: opts.copyFallback, }); - const fetcher = createRepoFetcher({ projectRoot: opts.projectRoot, cacheDir: paths.cacheDir }); + const fetcher = createRepoFetcher({ + stagingDir: path.join(paths.tempDir, "repos"), + }); return { projectRoot: opts.projectRoot, configPath: paths.configPath, statePath: paths.statePath, agnosRoot: paths.agnosRoot, - cacheDir: paths.cacheDir, + storeDir: opts.storeDir ?? resolveGlobalStoreDir(), logger, fetcher, linker, diff --git a/src/core/fs/copy-tree.ts b/src/core/fs/copy-tree.ts new file mode 100644 index 0000000..9837d1d --- /dev/null +++ b/src/core/fs/copy-tree.ts @@ -0,0 +1,66 @@ +import { constants } from "node:fs"; +import fs from "node:fs/promises"; +import path from "node:path"; + +export type FileImportMethod = "clone" | "hardlink" | "copy"; + +export interface CopyTreeResult { + methods: Set; +} + +export async function copyRegularTree( + source: string, + destination: string, + importFiles = false, +): Promise { + const methods = new Set(); + await copyDirectory(source, destination, importFiles, methods); + return { methods }; +} + +async function copyDirectory( + source: string, + destination: string, + importFiles: boolean, + methods: Set, +): Promise { + await fs.mkdir(destination, { recursive: true }); + const entries = await fs.readdir(source, { withFileTypes: true }); + await Promise.all( + entries.map(async (entry) => { + const sourcePath = path.join(source, entry.name); + const destinationPath = path.join(destination, entry.name); + if (entry.isDirectory()) { + await copyDirectory(sourcePath, destinationPath, importFiles, methods); + return; + } + if (!entry.isFile()) return; + const method = importFiles + ? await importFile(sourcePath, destinationPath) + : await cloneOrCopyFile(sourcePath, destinationPath); + methods.add(method); + const stat = await fs.stat(sourcePath); + await fs.chmod(destinationPath, stat.mode); + }), + ); +} + +async function importFile(source: string, destination: string): Promise { + try { + await fs.copyFile(source, destination, constants.COPYFILE_FICLONE_FORCE); + return "clone"; + } catch { + try { + await fs.link(source, destination); + return "hardlink"; + } catch { + await fs.copyFile(source, destination); + return "copy"; + } + } +} + +async function cloneOrCopyFile(source: string, destination: string): Promise { + await fs.copyFile(source, destination, constants.COPYFILE_FICLONE); + return "copy"; +} diff --git a/src/core/fs/link.ts b/src/core/fs/link.ts index d98c11a..31357ef 100644 --- a/src/core/fs/link.ts +++ b/src/core/fs/link.ts @@ -4,20 +4,20 @@ import os from "node:os"; import type { LinkKind, Linker, Logger } from "../types/public.js"; interface LinkerOptions { - cacheDir: string; + probeDir: string; logger: Logger; copyFallback?: boolean; } -export function createLinker({ cacheDir, logger, copyFallback }: LinkerOptions): Linker { +export function createLinker({ probeDir, logger, copyFallback }: LinkerOptions): Linker { let cachedFileProbe: boolean | undefined; async function probeFileSymlink(): Promise { if (cachedFileProbe !== undefined) return cachedFileProbe; - await fs.mkdir(cacheDir, { recursive: true }); - const probeDir = await fs.mkdtemp(path.join(cacheDir, "link-probe-")); - const target = path.join(probeDir, "target"); - const link = path.join(probeDir, "link"); + await fs.mkdir(probeDir, { recursive: true }); + const sessionDir = await fs.mkdtemp(path.join(probeDir, "link-probe-")); + const target = path.join(sessionDir, "target"); + const link = path.join(sessionDir, "link"); try { await fs.writeFile(target, ""); await fs.symlink(target, link, "file"); @@ -26,7 +26,7 @@ export function createLinker({ cacheDir, logger, copyFallback }: LinkerOptions): logger.debug(`file symlink probe failed: ${(err as Error).message}`); cachedFileProbe = false; } finally { - await fs.rm(probeDir, { recursive: true, force: true }); + await fs.rm(sessionDir, { recursive: true, force: true }); } return cachedFileProbe; } diff --git a/src/core/index.ts b/src/core/index.ts index 0b7586a..28c5af6 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -22,6 +22,7 @@ export type { Linker, LockFile, Logger, + LoggerProgress, LogInput, LogParts, LogTask, @@ -61,6 +62,8 @@ export { SCHEMA_URL, } from "./config.js"; export { buildResolveContext, workspaceRelativePath } from "./context.js"; +export { resolveGlobalStoreDir } from "./store-path.js"; +export type { GlobalStorePathOptions } from "./store-path.js"; export { loadPlugins, orderedDomains, refToId, resolveAgentByRef } from "./plugin-loader.js"; export type { PluginRegistry, RegisteredAgent, RegisteredDomain } from "./plugin-loader.js"; export { runAll, runOne, runFrom } from "./run.js"; @@ -89,6 +92,8 @@ export type { CommitResolution } from "./commit-resolver.js"; export { findSkillsInRepo, readSkillMeta } from "./skill-discovery.js"; export type { DiscoveredSkill } from "./skill-discovery.js"; export { hashSkillDir } from "./skill-hash.js"; +export { materializeSkill } from "./skill-materialize.js"; +export type { MaterializeSkillResult } from "./skill-materialize.js"; export { prepareSkills } from "./skill-prepare.js"; export type { PrepareResult } from "./skill-prepare.js"; export { diff --git a/src/core/lock.ts b/src/core/lock.ts index 6f1442d..7b21029 100644 --- a/src/core/lock.ts +++ b/src/core/lock.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { randomUUID } from "node:crypto"; import { lockFileSchema } from "./schema.js"; import type { LockFile, SkillLockEntry } from "./types/public.js"; @@ -43,7 +44,46 @@ export async function writeLock(projectRoot: string, lock: LockFile): Promise { + for (let attempt = 0; attempt < 8; attempt += 1) { + try { + await fs.rename(temporary, destination); + return; + } catch (error) { + const current = await fs.readFile(destination, "utf8").catch(() => null); + if (current === contents) return; + if (!isTransient(error) || attempt === 7) { + throw new Error(`failed to publish ${LOCK_FILE}: ${(error as Error).message}`, { + cause: error, + }); + } + await wait(25 * (attempt + 1)); + } + } +} + +function isTransient(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code; + return code === "EACCES" || code === "EBUSY" || code === "EEXIST" || code === "EPERM"; +} + +async function wait(milliseconds: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, milliseconds); + }); } export function upsertSkill(lock: LockFile, key: string, entry: SkillLockEntry): LockFile { diff --git a/src/core/logger.ts b/src/core/logger.ts index 9a68aea..cad8ece 100644 --- a/src/core/logger.ts +++ b/src/core/logger.ts @@ -1,5 +1,12 @@ import colors from "yoctocolors-cjs"; -import type { DomainColor, Logger, LogInput, LogParts, LogTask } from "./types/public.js"; +import type { + DomainColor, + Logger, + LoggerProgress, + LogInput, + LogParts, + LogTask, +} from "./types/public.js"; /** Dim/grey secondary text (e.g. an inline description). No-op off a TTY. */ export function dim(msg: string): string { @@ -85,6 +92,7 @@ function clearActiveLine(): void { } interface SpinnerHandle { + update(parts: LogParts): void; stop(): void; } @@ -102,7 +110,7 @@ function startSpinner( fmt: { domain?: string; color?: DomainColor }, quiet: boolean, ): SpinnerHandle { - if (quiet || !process.stderr.isTTY) return { stop() {} }; + if (quiet || !process.stderr.isTTY) return { update() {}, stop() {} }; if (timer) clearInterval(timer); // supersede any running spinner let message = LEVEL_COLOR[level](parts.message); if (parts.status) message += ` ${colors.dim(colors.italic(parts.status))}`; @@ -112,6 +120,14 @@ function startSpinner( renderActive(); timer = setInterval(renderActive, 80); return { + update(next): void { + if (active !== self) return; + let message = LEVEL_COLOR[level](next.message); + if (next.status) message += ` ${colors.dim(colors.italic(next.status))}`; + self.message = message; + clearActiveLine(); + renderActive(); + }, stop(): void { if (active !== self) return; // a newer spinner took over; leave it alone if (timer) clearInterval(timer); @@ -194,6 +210,17 @@ function makeLogger(state: LoggerState): Logger { }; const logger = { + progress(msg: LogInput): LoggerProgress { + const handle = startSpinner("info", toParts(msg), fmt, suppressed("info")); + return { + update(next): void { + handle.update(toParts(next)); + }, + stop(): void { + handle.stop(); + }, + }; + }, info: make("info"), success: make("success"), warn: make("warn"), @@ -241,6 +268,17 @@ export function indentedLogger(base: Logger, indent: string): Logger { (msg: any): any => (fn as (m: unknown) => unknown)(pad(msg)); return { + progress(msg: LogInput): LoggerProgress { + const handle = base.progress(pad(msg) as LogInput); + return { + update(next): void { + handle.update(pad(next) as LogInput); + }, + stop(): void { + handle.stop(); + }, + }; + }, info: wrap(base.info), success: wrap(base.success), warn: wrap(base.warn), diff --git a/src/core/paths.ts b/src/core/paths.ts index dac4eee..2274fcd 100644 --- a/src/core/paths.ts +++ b/src/core/paths.ts @@ -12,7 +12,7 @@ export interface ProjectPaths { projectRoot: string; configPath: string; agnosRoot: string; - cacheDir: string; + tempDir: string; skillsDir: string; statePath: string; } @@ -24,7 +24,7 @@ export function buildPaths(projectRoot: string, config?: AgnosConfig): ProjectPa projectRoot, configPath: path.join(projectRoot, CONFIG_FILE), agnosRoot, - cacheDir: path.join(agnosRoot, "cache"), + tempDir: path.join(agnosRoot, "tmp"), skillsDir: path.isAbsolute(skillsRel) ? skillsRel : path.join(projectRoot, skillsRel), statePath: path.join(agnosRoot, STATE_FILE), }; diff --git a/src/core/resolver.ts b/src/core/resolver.ts index 17a8a0e..69fbbb3 100644 --- a/src/core/resolver.ts +++ b/src/core/resolver.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { execFile as execFileCb } from "node:child_process"; import { promisify } from "node:util"; import { type GitSource, type LocalSource, type ParsedSource } from "./source.js"; @@ -14,8 +14,8 @@ const DISCOVERY_SUBDIR = "skills"; export interface RepoFetchOptions { /** Optional commit SHA (or branch/tag) to fetch. Defaults to provider default branch. */ ref?: string; - /** Bypass any cached download; force a fresh fetch. */ - noCache?: boolean; + /** Replace any matching checkout already staged during this run. */ + fresh?: boolean; } export interface RepoFetchResult { @@ -23,25 +23,56 @@ export interface RepoFetchResult { path: string; /** Git ref actually fetched (the explicit ref, or the resolved default branch). */ ref?: string; + /** Commit SHA checked out for a Git source. */ + commit?: string; } export interface RepoFetcher { - /** Fetch a repository source to a cache-managed directory and return the root path. */ + /** Fetch a repository source to a transient directory and return the root path. */ fetch(source: ParsedSource, opts?: RepoFetchOptions): Promise; + /** Remove every transient checkout created by this fetcher. */ + cleanup(): Promise; } export interface CreateRepoFetcherOptions { - projectRoot: string; - cacheDir: string; + stagingDir: string; } export function createRepoFetcher(opts: CreateRepoFetcherOptions): RepoFetcher { + const inFlight = new Map>(); + const sessionDir = path.join(opts.stagingDir, randomUUID()); return { async fetch(source, fetchOpts) { if (source.kind === "local") { return { path: source.absolutePath }; } - return fetchGit(source, opts, fetchOpts); + const subdir = source.subPath ?? DISCOVERY_SUBDIR; + const requestedRef = fetchOpts?.ref ?? source.ref ?? "default"; + const key = `${source.canonical}@${requestedRef}@${subdir}@${fetchOpts?.fresh ? "fresh" : "staged"}`; + const pending = inFlight.get(key); + if (pending) return pending; + + const fetch = fetchGit(source, { stagingDir: sessionDir }, fetchOpts); + inFlight.set(key, fetch); + try { + return await fetch; + } finally { + if (inFlight.get(key) === fetch) inFlight.delete(key); + } + }, + async cleanup() { + await Promise.allSettled([...inFlight.values()]); + inFlight.clear(); + try { + await fs.rm(sessionDir, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 50, + }); + } catch { + return; + } }, }; } @@ -61,20 +92,35 @@ async function fetchGit( const explicitRef = opts?.ref ?? source.ref; const ref = explicitRef ?? (await resolveDefaultBranch(source).catch(() => null)) ?? undefined; - // Cache is keyed by repo + ref + subtree so discovery and per-skill installs - // don't clobber each other. - const cacheKey = hashKey(`${source.canonical}@${ref ?? "HEAD"}@${subdir}`); - const destDir = path.join(cfg.cacheDir, "repos", cacheKey); + const destDir = resolveRepoStagingDir(source, cfg.stagingDir, ref); + const stagingKey = path.basename(destDir); - if (!opts?.noCache && (await dirHasFiles(destDir))) { - return { path: destDir, ref }; + if (!opts?.fresh && (await dirHasFiles(destDir))) { + const commit = await resolveCheckoutCommit(destDir); + return { path: destDir, ref, ...(commit ? { commit } : {}) }; } + const parent = path.dirname(destDir); + const temporary = path.join(parent, `.tmp-${stagingKey}-${randomUUID()}`); await fs.rm(destDir, { recursive: true, force: true }); - await fs.mkdir(path.dirname(destDir), { recursive: true }); + await fs.rm(temporary, { recursive: true, force: true }); + await fs.mkdir(parent, { recursive: true }); + + try { + const commit = await sparseClone(buildCloneUrl(source), temporary, ref, subdir); + await fs.rename(temporary, destDir); + return { path: destDir, ref, ...(commit ? { commit } : {}) }; + } catch (error) { + throw new Error(`failed to fetch ${source.canonical}`, { cause: error }); + } finally { + await fs.rm(temporary, { recursive: true, force: true }); + } +} - await sparseClone(buildCloneUrl(source), destDir, ref, subdir); - return { path: destDir, ref }; +function resolveRepoStagingDir(source: GitSource, stagingDir: string, ref?: string): string { + const subdir = source.subPath ?? DISCOVERY_SUBDIR; + const stagingKey = hashKey(`${source.canonical}@${ref ?? "HEAD"}@${subdir}`); + return path.join(stagingDir, stagingKey); } /** @@ -89,7 +135,7 @@ async function sparseClone( dest: string, ref: string | undefined, subdir: string, -): Promise { +): Promise { const cloneArgs = ["clone", "--no-checkout", "--depth", "1", "--filter=blob:none"]; if (ref) cloneArgs.push("--branch", ref); cloneArgs.push(url, dest); @@ -118,6 +164,13 @@ async function sparseClone( ]); await execFile("git", ["-C", dest, "checkout", "FETCH_HEAD"]); } + return resolveCheckoutCommit(dest); +} + +async function resolveCheckoutCommit(directory: string): Promise { + const result = await execFile("git", ["-C", directory, "rev-parse", "HEAD"]); + const commit = result.stdout.trim(); + return commit || undefined; } async function dirHasFiles(p: string): Promise { diff --git a/src/core/run.ts b/src/core/run.ts index 8d9f349..a92d5ad 100644 --- a/src/core/run.ts +++ b/src/core/run.ts @@ -11,7 +11,11 @@ async function runDomain( // Scope the logger to this domain so everything it prints carries the // domain's `[domain]` prefix and color automatically. const scoped: RunContext = { ...ctx, logger: withDomain(ctx.logger, dom.domain) }; - await dom.domain.run(opts, scoped); + try { + await dom.domain.run(opts, scoped); + } finally { + await scoped.fetcher.cleanup?.(); + } } /** diff --git a/src/core/skill-materialize.ts b/src/core/skill-materialize.ts new file mode 100644 index 0000000..78c1df4 --- /dev/null +++ b/src/core/skill-materialize.ts @@ -0,0 +1,69 @@ +import { randomUUID } from "node:crypto"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { copyRegularTree, type CopyTreeResult } from "./fs/copy-tree.js"; + +const SKILL_MARKER = "SKILL.md"; + +export interface MaterializeSkillResult extends CopyTreeResult { + changed: boolean; +} + +export async function materializeSkill( + stored: string, + destination: string, + expectedHash: string, + materializedHash?: string, +): Promise { + if (materializedHash === expectedHash && (await isMaterializedSkill(destination))) { + return { changed: false, methods: new Set() }; + } + + const parent = path.dirname(destination); + const temporary = path.join(parent, `.tmp-${path.basename(destination)}-${randomUUID()}`); + const previous = path.join(parent, `.old-${path.basename(destination)}-${randomUUID()}`); + await fs.mkdir(parent, { recursive: true }); + + try { + const result = await copyRegularTree(stored, temporary, true); + const existing = await fs.lstat(destination).catch(() => null); + if (existing) await retryRename(destination, previous); + try { + await retryRename(temporary, destination); + } catch (error) { + if (existing) await retryRename(previous, destination).catch(() => undefined); + throw error; + } + await fs + .rm(previous, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }) + .catch(() => undefined); + return { changed: true, methods: result.methods }; + } finally { + await fs.rm(temporary, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + } +} + +async function retryRename(source: string, destination: string): Promise { + for (let attempt = 0; attempt < 8; attempt += 1) { + try { + await fs.rename(source, destination); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + const transient = code === "EACCES" || code === "EBUSY" || code === "EPERM"; + if (!transient || attempt === 7) throw error; + await new Promise((resolve) => { + setTimeout(resolve, 25 * (attempt + 1)); + }); + } + } +} + +async function isMaterializedSkill(directory: string): Promise { + const stat = await fs.lstat(directory).catch(() => null); + if (!stat?.isDirectory() || stat.isSymbolicLink()) return false; + return fs + .access(path.join(directory, SKILL_MARKER)) + .then(() => true) + .catch(() => false); +} diff --git a/src/core/skill-prepare.ts b/src/core/skill-prepare.ts index cef7368..bfaaee2 100644 --- a/src/core/skill-prepare.ts +++ b/src/core/skill-prepare.ts @@ -4,6 +4,9 @@ import { buildPaths } from "./paths.js"; import { getSkill, readLock, upsertSkill, writeLock } from "./lock.js"; import { parseCompositeSkillRef } from "./source.js"; import { hashSkillDir } from "./skill-hash.js"; +import { ensureStoredSkill, findStoredSkill } from "./skill-store.js"; +import { materializeSkill } from "./skill-materialize.js"; +import { readState, writeState } from "./state.js"; import type { AgnosConfig, ResolveContext } from "./types/public.js"; const SKILL_MARKER = "SKILL.md"; @@ -25,14 +28,25 @@ export interface PrepareResult { * - missing entry → write it (fresh-clone reproducibility). * - match → proceed. * - mismatch → fail loudly with a clear remediation step. - * 4. Copy `/` to `/` so the canonical - * bytes are on disk before any agent hook runs. + * 4. Store the content by hash and materialize `/` so the + * canonical bytes are available before any agent hook runs. * * Returns a summary so callers can log what was filled vs. verified. */ export async function prepareSkills( config: AgnosConfig, ctx: ResolveContext, +): Promise { + try { + return await prepareSkillsFromSources(config, ctx); + } finally { + await ctx.fetcher.cleanup(); + } +} + +async function prepareSkillsFromSources( + config: AgnosConfig, + ctx: ResolveContext, ): Promise { const result: PrepareResult = { filled: [], verified: [] }; const entries = Object.entries(config.skills?.sources ?? {}); @@ -40,14 +54,17 @@ export async function prepareSkills( const lockBefore = await readLock(ctx.projectRoot); let lock = lockBefore; + let lockDirty = false; + const state = await readState(ctx.statePath); + let stateDirty = false; const skillsDir = buildPaths(ctx.projectRoot, config).skillsDir; if (!ctx.dryRun) await fs.mkdir(skillsDir, { recursive: true }); for (const [name, composite] of entries) { const ref = parseCompositeSkillRef(composite, { projectRoot: ctx.projectRoot }); - const fetched = await ctx.fetcher.fetch(ref.source); - const skillSrc = - ref.source.kind === "git" ? path.join(fetched.path, ref.subPath) : fetched.path; + const existing = getSkill(lock, composite); + const located = await locatePreparedSkill(name, ref, existing, skillsDir, ctx); + const skillSrc = located.path; if (!(await isSkillDir(skillSrc))) { throw new Error( @@ -58,16 +75,20 @@ export async function prepareSkills( } const hash = await hashSkillDir(skillSrc); - const existing = getSkill(lock, composite); - if (!existing) { + if (ref.source.kind === "git" && !located.commit) { + throw new Error(`could not determine the checked-out commit for skill "${name}"`); + } if (ctx.dryRun) { ctx.logger.info(`would: pin ${name} (${composite}) → ${hash.slice(0, 12)}…`); } else { lock = upsertSkill(lock, composite, { computedHash: hash, resolvedAt: new Date().toISOString(), + ...(located.commit ? { resolvedCommit: located.commit } : {}), + ...(located.ref ? { ref: located.ref } : {}), }); + lockDirty = true; ctx.logger.info(`pinned ${name} (${composite}) → ${hash.slice(0, 12)}…`); } result.filled.push(name); @@ -79,24 +100,80 @@ export async function prepareSkills( `Run \`agnos skill update ${name}\` to accept the new content.`, ); } else { + if (ref.source.kind === "git" && located.commit && !existing.resolvedCommit) { + lock = upsertSkill(lock, composite, { ...existing, resolvedCommit: located.commit }); + lockDirty = true; + } result.verified.push(name); } if (!ctx.dryRun) { const dst = path.join(skillsDir, name); - await fs.rm(dst, { recursive: true, force: true }); - await fs.cp(skillSrc, dst, { recursive: true, force: true }); + const stored = await ensureStoredSkill(skillSrc, ctx.storeDir, hash); + await materializeSkill(stored, dst, hash, state.materializedSkills?.[name]); + if (state.materializedSkills?.[name] !== hash) { + state.materializedSkills = { ...(state.materializedSkills ?? {}), [name]: hash }; + stateDirty = true; + } } } // Only write the lock if anything actually changed and we're not in dry-run. - if (!ctx.dryRun && result.filled.length > 0) { + if (!ctx.dryRun && lockDirty) { await writeLock(ctx.projectRoot, lock); } + if (!ctx.dryRun && stateDirty) await writeState(ctx.statePath, state); return result; } +interface LocatedPreparedSkill { + path: string; + ref?: string; + commit?: string; +} + +async function locatePreparedSkill( + name: string, + composite: ReturnType, + existing: ReturnType, + skillsDir: string, + ctx: ResolveContext, +): Promise { + if (composite.source.kind === "local") return { path: composite.source.absolutePath }; + + if (existing) { + const global = await findStoredSkill(ctx.storeDir, existing.computedHash); + if (global && existing.resolvedCommit) { + return { path: global, ref: existing.ref, commit: existing.resolvedCommit }; + } + + const candidates = [ + path.join(skillsDir, name), + path.join(ctx.agnosRoot, "cache", "skills", existing.computedHash), + ]; + for (const candidate of candidates) { + if ((await hashSkillDir(candidate).catch(() => null)) !== existing.computedHash) continue; + const stored = await ensureStoredSkill(candidate, ctx.storeDir, existing.computedHash); + if (existing.resolvedCommit) { + return { path: stored, ref: existing.ref, commit: existing.resolvedCommit }; + } + } + } + + const trackedRef = composite.source.ref ?? existing?.ref; + const checkoutRef = existing?.resolvedCommit ?? trackedRef; + const fetched = await ctx.fetcher.fetch( + composite.source, + checkoutRef ? { ref: checkoutRef } : undefined, + ); + return { + path: path.join(fetched.path, composite.subPath), + ...((trackedRef ?? fetched.ref) ? { ref: trackedRef ?? fetched.ref } : {}), + ...(fetched.commit ? { commit: fetched.commit } : {}), + }; +} + async function isSkillDir(p: string): Promise { try { const s = await fs.stat(p); diff --git a/src/core/skill-store.ts b/src/core/skill-store.ts new file mode 100644 index 0000000..56f56bc --- /dev/null +++ b/src/core/skill-store.ts @@ -0,0 +1,146 @@ +import { randomUUID } from "node:crypto"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { copyRegularTree } from "./fs/copy-tree.js"; +import { hashSkillDir } from "./skill-hash.js"; + +const SKILL_MARKER = "SKILL.md"; +const storeWrites = new Map>(); +const PUBLISH_RETRIES = 8; +const RETRY_DELAY_MS = 25; + +export function resolveStoredSkill(storeDir: string, hash: string): string { + return path.join(storeDir, "skills", hash); +} + +export async function findStoredSkill(storeDir: string, hash: string): Promise { + const stored = resolveStoredSkill(storeDir, hash); + return (await isStoredSkillValid(stored, hash)) ? stored : null; +} + +export async function ensureStoredSkill( + source: string, + storeDir: string, + hash: string, +): Promise { + const stored = resolveStoredSkill(storeDir, hash); + if (path.resolve(source) === path.resolve(stored)) return stored; + + const pending = storeWrites.get(stored); + if (pending) return pending; + + const write = populateStoredSkill(source, stored); + storeWrites.set(stored, write); + try { + return await write; + } finally { + if (storeWrites.get(stored) === write) storeWrites.delete(stored); + } +} + +async function populateStoredSkill(source: string, stored: string): Promise { + if (await isStoredSkillValid(stored, path.basename(stored))) return stored; + + const parent = path.dirname(stored); + const temporary = path.join(parent, `.tmp-${path.basename(stored)}-${randomUUID()}`); + await fs.mkdir(parent, { recursive: true }); + + try { + await retryTransient(async () => { + await copyRegularTree(source, temporary); + }); + const expectedHash = path.basename(stored); + if ((await hashSkillDir(temporary)) !== expectedHash) { + throw new Error(`source content does not match expected hash ${expectedHash}`); + } + return await publishStoredSkill(temporary, stored, expectedHash); + } catch (error) { + throw new Error(`failed to populate skill store at ${stored}: ${formatError(error)}`, { + cause: error, + }); + } finally { + await fs.rm(temporary, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + } +} + +async function publishStoredSkill( + temporary: string, + stored: string, + expectedHash: string, +): Promise { + for (let attempt = 0; attempt < PUBLISH_RETRIES; attempt += 1) { + try { + await fs.rename(temporary, stored); + return stored; + } catch (error) { + if (await isStoredSkillValid(stored, expectedHash)) return stored; + await quarantineInvalidStoreEntry(stored); + if (!isTransient(error) || attempt === PUBLISH_RETRIES - 1) throw error; + await wait(RETRY_DELAY_MS * (attempt + 1)); + } + } + throw new Error(`could not publish ${stored}`); +} + +async function retryTransient(operation: () => Promise): Promise { + for (let attempt = 0; attempt < PUBLISH_RETRIES; attempt += 1) { + try { + await operation(); + return; + } catch (error) { + if (!isTransient(error) || attempt === PUBLISH_RETRIES - 1) throw error; + await wait(RETRY_DELAY_MS * (attempt + 1)); + } + } +} + +function isTransient(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code; + return ( + code === "EACCES" || + code === "EBUSY" || + code === "EEXIST" || + code === "ENOTEMPTY" || + code === "EPERM" + ); +} + +function formatError(error: unknown): string { + const code = (error as NodeJS.ErrnoException).code; + const message = error instanceof Error ? error.message : String(error); + return code ? `${code}: ${message}` : message; +} + +async function wait(milliseconds: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, milliseconds); + }); +} + +async function isSkillDir(directory: string): Promise { + try { + const stat = await fs.stat(directory); + if (!stat.isDirectory()) return false; + await fs.access(path.join(directory, SKILL_MARKER)); + return true; + } catch { + return false; + } +} + +async function isStoredSkillValid(directory: string, expectedHash: string): Promise { + if (!(await isSkillDir(directory))) return false; + return (await hashSkillDir(directory).catch(() => null)) === expectedHash; +} + +async function quarantineInvalidStoreEntry(directory: string): Promise { + const stat = await fs.lstat(directory).catch(() => null); + if (!stat) return; + const quarantine = `${directory}.invalid-${randomUUID()}`; + try { + await fs.rename(directory, quarantine); + await fs.rm(quarantine, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } +} diff --git a/src/core/state.ts b/src/core/state.ts index ff4ef6b..4d40102 100644 --- a/src/core/state.ts +++ b/src/core/state.ts @@ -14,6 +14,8 @@ export interface AgnosState { * `##` sections untouched. */ rulesSections?: Record; + /** Materialized skill name to the content hash last installed by Agnos. */ + materializedSkills?: Record; } const DEFAULT_STATE: AgnosState = { @@ -22,6 +24,7 @@ const DEFAULT_STATE: AgnosState = { initializedDomains: [], importedDomains: {}, rulesSections: {}, + materializedSkills: {}, }; export async function readState(statePath: string): Promise { @@ -91,6 +94,7 @@ function normalize(parsed: Partial): AgnosState { : [], importedDomains: normalizeStringListMap(parsed.importedDomains), rulesSections: normalizeStringListMap(parsed.rulesSections), + materializedSkills: normalizeStringMap(parsed.materializedSkills), }; } @@ -104,3 +108,12 @@ function normalizeStringListMap(value: unknown): Record { } return out; } + +function normalizeStringMap(value: unknown): Record { + if (!value || typeof value !== "object") return {}; + return Object.fromEntries( + Object.entries(value as Record).filter( + (entry): entry is [string, string] => typeof entry[1] === "string", + ), + ); +} diff --git a/src/core/store-path.ts b/src/core/store-path.ts new file mode 100644 index 0000000..cb71f41 --- /dev/null +++ b/src/core/store-path.ts @@ -0,0 +1,41 @@ +import os from "node:os"; +import path from "node:path"; + +export interface GlobalStorePathOptions { + env?: NodeJS.ProcessEnv; + homeDir?: string; + platform?: NodeJS.Platform; +} + +export function resolveGlobalStoreDir(options: GlobalStorePathOptions = {}): string { + const env = options.env ?? process.env; + const homeDir = options.homeDir ?? os.homedir(); + const platform = options.platform ?? process.platform; + const paths = platform === "win32" ? path.win32 : path.posix; + const override = env["AGNOS_STORE_DIR"]; + const baseDir = override + ? paths.resolve(override) + : resolveDefaultStoreBase(env, homeDir, platform, paths); + return paths.join(baseDir, "v1"); +} + +function resolveDefaultStoreBase( + env: NodeJS.ProcessEnv, + homeDir: string, + platform: NodeJS.Platform, + paths: path.PlatformPath, +): string { + if (platform === "win32") { + return paths.join( + env["LOCALAPPDATA"] ?? paths.join(homeDir, "AppData", "Local"), + "agnos", + "store", + ); + } + if (platform === "darwin") return paths.join(homeDir, "Library", "agnos", "store"); + return paths.join( + env["XDG_DATA_HOME"] ?? paths.join(homeDir, ".local", "share"), + "agnos", + "store", + ); +} diff --git a/src/core/types/public.ts b/src/core/types/public.ts index b4b8381..49b0df8 100644 --- a/src/core/types/public.ts +++ b/src/core/types/public.ts @@ -220,7 +220,17 @@ export interface LogTask extends LogParts { done?: LogInput | ((value: T) => LogInput); } +/** A live TTY log line that can be replaced in place until work completes. */ +export interface LoggerProgress { + /** Replace the progress line with current text. */ + update(msg: LogInput): void; + /** Clear the progress line and release its terminal state. */ + stop(): void; +} + export interface Logger { + /** Start a live progress line. It is inert in quiet and non-TTY output. */ + progress(msg: LogInput): LoggerProgress; info(msg: LogTask): Promise; info(msg: LogInput): void; warn(msg: LogTask): Promise; @@ -244,15 +254,16 @@ export interface Linker { /** * Repository fetcher. Materializes a parsed git or local source into a - * cache-managed directory and returns the root path. The result represents + * transient directory and returns the root path. The result represents * the repository root, not a specific skill — domains walk into it to find * what they need (e.g. domain-skills looks under `./skills/*`). */ export interface RepoFetcher { fetch( source: ParsedSourceRef, - opts?: { ref?: string; noCache?: boolean }, - ): Promise<{ path: string; ref?: string }>; + opts?: { ref?: string; fresh?: boolean }, + ): Promise<{ path: string; ref?: string; commit?: string }>; + cleanup(): Promise; } /** @@ -273,7 +284,7 @@ export type ParsedSourceRef = export interface ResolveContext { agnosRoot: string; projectRoot: string; - cacheDir: string; + storeDir: string; configPath: string; statePath: string; logger: Logger; diff --git a/src/domains/skills/concurrency.ts b/src/domains/skills/concurrency.ts new file mode 100644 index 0000000..cfd1e05 --- /dev/null +++ b/src/domains/skills/concurrency.ts @@ -0,0 +1,33 @@ +const SKILL_CONCURRENCY = 8; + +export async function runSkillTasks( + values: T[], + worker: (value: T) => Promise, +): Promise { + const results = new Array(values.length); + let nextIndex = 0; + + async function runWorker(): Promise { + while (nextIndex < values.length) { + const index = nextIndex; + nextIndex += 1; + const value = values[index]; + if (value === undefined) continue; + results[index] = await worker(value); + } + } + + const workers = Array.from({ length: Math.min(SKILL_CONCURRENCY, values.length) }, () => + runWorker(), + ); + const settled = await Promise.allSettled(workers); + const failure = settled.find( + (result): result is PromiseRejectedResult => result.status === "rejected", + ); + if (failure) { + const reason: unknown = failure.reason; + if (reason instanceof Error) throw reason; + throw new Error("skill task failed", { cause: reason }); + } + return results; +} diff --git a/src/domains/skills/index.ts b/src/domains/skills/index.ts index 0c64624..1fb66a7 100644 --- a/src/domains/skills/index.ts +++ b/src/domains/skills/index.ts @@ -155,7 +155,9 @@ async function diagnose( ctx.logger.info("no skills declared"); return; } - const { steps } = await createSkillSteps(config, ctx); + const { steps } = await createSkillSteps(config, ctx, { + verifyMaterialized: which === "integrity", + }); const bad: string[] = []; await ctx.logger.info({ message: `Checking ${count} skill${count === 1 ? "" : "s"}…`, @@ -440,8 +442,10 @@ const commands: Record = { const handle = await createSkillSteps(config, ctx); const res = await runSkillPipeline(sources, handle.steps, ctx.logger); await handle.flush(); - if (res.installed.length > 0) - ctx.logger.success(`installed ${res.installed.length} skill(s)`); + ctx.logger.success({ + message: "Installing skills 100%", + status: `total ${res.progress.total} | reused ${res.progress.reused} | fetched ${res.progress.fetched}`, + }); }, }, prune: { @@ -575,12 +579,11 @@ export const skillsDomain: Domain = { // bucketed and reported as "Skills need to be updated: …" without throwing, // so the overall run continues (§13.1). const handle = await createSkillSteps(config, ctx); - await ctx.logger.info({ - message: `Downloading and installing ${count} skill${count === 1 ? "" : "s"}`, - waitFor: (async () => { - await runSkillPipeline(sources, handle.steps, ctx.logger); - await handle.flush(); - })(), + const result = await runSkillPipeline(sources, handle.steps, ctx.logger); + await handle.flush(); + ctx.logger.success({ + message: "Installing skills 100%", + status: `total ${result.progress.total} | reused ${result.progress.reused} | fetched ${result.progress.fetched}`, }); return undefined; }, diff --git a/src/domains/skills/pipeline.ts b/src/domains/skills/pipeline.ts index ad2200f..568596b 100644 --- a/src/domains/skills/pipeline.ts +++ b/src/domains/skills/pipeline.ts @@ -1,4 +1,5 @@ import type { Logger } from "../../core/index.js"; +import { runSkillTasks } from "./concurrency.js"; /** * Per-skill preparation buckets. A skill lands in exactly one — the pipeline @@ -15,6 +16,10 @@ export interface FetchResult { src?: string; /** Branch/tag actually fetched (git sources) — threaded to `install` for the lock. */ ref?: string; + /** Commit SHA of the fetched repository checkout. */ + commit?: string; + /** Whether the skill came from the content store or its declared source. */ + source?: "reused" | "fetched"; } /** @@ -32,12 +37,30 @@ export interface SkillSteps { /** Does the content hash match the lock? false → "changed". */ integrity(name: string, src: string): Promise; /** Copy into the canonical dir (copy-if-absent-or-changed); pins/backfills the lock. */ - install(name: string, src: string, ref?: string): Promise; + install(name: string, src: string, ref?: string, commit?: string): Promise; } export interface PipelineResult { buckets: Record; installed: string[]; + progress: SkillInstallProgress; +} + +/** Aggregate progress counters for one skill installation pipeline. */ +export interface SkillInstallProgress { + /** Number of declared skills examined by the pipeline. */ + total: number; + /** Number of skills whose pipeline has completed. */ + completed: number; + /** Number restored from the content-addressed skill store. */ + reused: number; + /** Number loaded from their declared source. */ + fetched: number; +} + +interface SkillResult { + bucket?: Bucket; + installed?: string; } /** @@ -55,18 +78,40 @@ export async function runSkillPipeline( const buckets: Record = { moved: [], changed: [] }; const installed: string[] = []; - for (const [name, ref] of Object.entries(sources)) { - const fetched = await steps.fetch(name, ref); - if (!fetched.ok || !fetched.src) { - buckets.moved.push(name); - continue; - } - if (!(await steps.integrity(name, fetched.src))) { - buckets.changed.push(name); - continue; - } - await steps.install(name, fetched.src, fetched.ref); - installed.push(name); + const entries = Object.entries(sources); + const state: SkillInstallProgress = { + total: entries.length, + completed: 0, + reused: 0, + fetched: 0, + }; + const progress = logger.progress(formatSkillProgress(state)); + let results: SkillResult[]; + try { + results = await runSkillTasks(entries, async ([name, ref]) => { + const fetched = await steps.fetch(name, ref); + let result: SkillResult; + if (!fetched.ok || !fetched.src) { + result = { bucket: "moved" }; + } else if (!(await steps.integrity(name, fetched.src))) { + result = { bucket: "changed" }; + } else { + await steps.install(name, fetched.src, fetched.ref, fetched.commit); + result = { installed: name }; + } + recordSkillProgress(state, fetched.source, progress); + return result; + }); + } finally { + progress.stop(); + } + + for (const [index, result] of results.entries()) { + const entry = entries[index]; + if (!entry) continue; + const [name] = entry; + if (result.bucket) buckets[result.bucket].push(name); + if (result.installed) installed.push(result.installed); } const total = buckets.moved.length + buckets.changed.length; @@ -76,5 +121,28 @@ export async function runSkillPipeline( extra: "run: agnos skills update", }); } - return { buckets, installed }; + return { buckets, installed, progress: state }; +} + +function formatSkillProgress(progress: SkillInstallProgress): { + message: string; + status: string; +} { + const percentage = + progress.total === 0 ? 100 : Math.floor((progress.completed / progress.total) * 100); + return { + message: `Installing skills ${percentage}%`, + status: `total ${progress.total} | reused ${progress.reused} | fetched ${progress.fetched}`, + }; +} + +function recordSkillProgress( + progress: SkillInstallProgress, + source: FetchResult["source"], + reporter: { update(input: { message: string; status: string }): void }, +): void { + progress.completed += 1; + if (source === "reused") progress.reused += 1; + else if (source === "fetched") progress.fetched += 1; + reporter.update(formatSkillProgress(progress)); } diff --git a/src/domains/skills/steps.ts b/src/domains/skills/steps.ts index b3f6042..c892cb9 100644 --- a/src/domains/skills/steps.ts +++ b/src/domains/skills/steps.ts @@ -5,14 +5,19 @@ import { buildPaths, getSkill, hashSkillDir, + materializeSkill, parseCompositeSkillRef, readLock, + readState, removeSkill, resolveGitCommit, resolveLocalCommit, upsertSkill, writeLock, + writeState, } from "../../core/index.js"; +import { ensureStoredSkill, findStoredSkill } from "../../core/skill-store.js"; +import { runSkillTasks } from "./concurrency.js"; import type { SkillSteps } from "./pipeline.js"; const SKILL_MARKER = "SKILL.md"; @@ -33,26 +38,38 @@ interface LocateResult { src: string; /** Branch/tag actually fetched (for git sources) — persisted to the lock. */ ref?: string; + /** Commit SHA of the checkout containing the skill. */ + commit?: string; } /** * Locate the fetched skill content for a composite ref (or null if missing). - * `lockedRef` is the branch/tag recorded in the lock: passing it as the explicit - * fetch ref lets `fetchGit` skip the `ls-remote` default-branch lookup and hit - * the cache directly, so warm runs are fully offline. + * An explicit checkout ref may be a locked commit while `trackedRef` remains + * the symbolic branch or tag persisted for freshness checks. */ async function locate( composite: string, ctx: ResolveContext, - lockedRef?: string, + options?: { checkoutRef?: string; trackedRef?: string; fresh?: boolean }, ): Promise { const parsed = parseCompositeSkillRef(composite, { projectRoot: ctx.projectRoot }); const ownRef = parsed.source.kind === "git" ? parsed.source.ref : undefined; - const explicit = ownRef ?? lockedRef; - const fetched = await ctx.fetcher.fetch(parsed.source, explicit ? { ref: explicit } : undefined); + const checkoutRef = options?.checkoutRef ?? ownRef; + const fetchOptions = { + ...(checkoutRef ? { ref: checkoutRef } : {}), + ...(options?.fresh ? { fresh: true } : {}), + }; + const fetched = await ctx.fetcher.fetch( + parsed.source, + Object.keys(fetchOptions).length > 0 ? fetchOptions : undefined, + ); const src = parsed.source.kind === "git" ? path.join(fetched.path, parsed.subPath) : fetched.path; if (!(await isSkillDir(src))) return null; - return { src, ...(fetched.ref ? { ref: fetched.ref } : {}) }; + return { + src, + ...((options?.trackedRef ?? fetched.ref) ? { ref: options?.trackedRef ?? fetched.ref } : {}), + ...(fetched.commit ? { commit: fetched.commit } : {}), + }; } /** Best-effort upstream commit for the ref (undefined on any failure / no network). */ @@ -75,6 +92,10 @@ export interface SkillStepsHandle { flush(): Promise; } +export interface CreateSkillStepsOptions { + verifyMaterialized?: boolean; +} + export interface PruneSkillsResult { removed: string[]; unpinned: string[]; @@ -89,7 +110,9 @@ export async function pruneSkills( const desiredSources = new Set(Object.values(sources)); const skillsDir = buildPaths(ctx.projectRoot, config).skillsDir; let lock = await readLock(ctx.projectRoot); + const state = await readState(ctx.statePath); let dirty = false; + let stateDirty = false; const result: PruneSkillsResult = { removed: [], unpinned: [] }; let children: string[]; @@ -109,6 +132,12 @@ export async function pruneSkills( ctx.logger.info(`would: remove skill "${name}"`); } else { await fs.rm(candidate, { recursive: true, force: true }); + if (state.materializedSkills?.[name]) { + state.materializedSkills = Object.fromEntries( + Object.entries(state.materializedSkills).filter(([skillName]) => skillName !== name), + ); + stateDirty = true; + } } } @@ -124,6 +153,7 @@ export async function pruneSkills( } if (dirty) await writeLock(ctx.projectRoot, lock); + if (stateDirty) await writeState(ctx.statePath, state); return result; } @@ -131,16 +161,20 @@ export async function pruneSkills( * Concrete `SkillSteps` over the real fetcher + lock + content hash. The * `version` step compares the lock's `resolvedCommit` to the upstream HEAD * (treating an absent baseline or a network failure as "current" rather than - * false-alarming); `install` is copy-if-changed and pins new skills. + * false-alarming); `install` imports content-addressed stored skills and pins + * new skills. */ export async function createSkillSteps( config: AgnosConfig, ctx: ResolveContext, + options?: CreateSkillStepsOptions, ): Promise { const sources = config.skills?.sources ?? {}; const skillsDir = buildPaths(ctx.projectRoot, config).skillsDir; let lock = await readLock(ctx.projectRoot); + const state = await readState(ctx.statePath); let dirty = false; + let stateDirty = false; // Hash each fetched source directory at most once per run — `integrity` and // `install` both need the source hash, and the source tree is immutable. @@ -160,12 +194,52 @@ export async function createSkillSteps( }; const steps: SkillSteps = { - async fetch(_name, composite) { + async fetch(name, composite) { try { - const lockedRef = getSkill(lock, composite)?.ref; - const located = await locate(composite, ctx, lockedRef); + const parsed = parseCompositeSkillRef(composite, { projectRoot: ctx.projectRoot }); + const entry = getSkill(lock, composite); + if (parsed.source.kind === "git" && entry) { + let stored = await findStoredSkill(ctx.storeDir, entry.computedHash); + if (!stored) { + const legacyStore = path.join(ctx.agnosRoot, "cache"); + const legacy = await findStoredSkill(legacyStore, entry.computedHash); + if (legacy) stored = await ensureStoredSkill(legacy, ctx.storeDir, entry.computedHash); + } + if (!stored) { + const materialized = path.join(skillsDir, name); + if ((await hashSkillDir(materialized).catch(() => null)) === entry.computedHash) { + stored = await ensureStoredSkill(materialized, ctx.storeDir, entry.computedHash); + } + } + if (stored && entry.resolvedCommit) { + srcHashes.set(stored, entry.computedHash); + const ref = parsed.source.ref ?? entry.ref; + return { + ok: true, + src: stored, + source: "reused", + ...(ref ? { ref } : {}), + ...(entry.resolvedCommit ? { commit: entry.resolvedCommit } : {}), + }; + } + } + + const trackedRef = + parsed.source.kind === "git" ? (parsed.source.ref ?? entry?.ref) : undefined; + const checkoutRef = + parsed.source.kind === "git" ? (entry?.resolvedCommit ?? trackedRef) : undefined; + const located = await locate(composite, ctx, { + ...(checkoutRef ? { checkoutRef } : {}), + ...(trackedRef ? { trackedRef } : {}), + }); return located - ? { ok: true, src: located.src, ...(located.ref ? { ref: located.ref } : {}) } + ? { + ok: true, + src: located.src, + source: "fetched", + ...(located.ref ? { ref: located.ref } : {}), + ...(located.commit ? { commit: located.commit } : {}), + } : { ok: false }; } catch { return { ok: false }; @@ -181,9 +255,10 @@ export async function createSkillSteps( async integrity(name, src) { const entry = getSkill(lock, compositeOf(name)); if (!entry) return true; // unpinned → install will pin it - return (await hashOnce(src)) === entry.computedHash; + const target = options?.verifyMaterialized ? path.join(skillsDir, name) : src; + return (await hashOnce(target)) === entry.computedHash; }, - async install(name, src, ref) { + async install(name, src, ref, commit) { const composite = compositeOf(name); if (ctx.dryRun) { ctx.logger.info(`would: install skill "${name}"`); @@ -191,15 +266,18 @@ export async function createSkillSteps( } const dst = path.join(skillsDir, name); const srcHash = await hashOnce(src); - const dstHash = (await isSkillDir(dst)) ? await hashSkillDir(dst) : null; - if (dstHash !== srcHash) { - await fs.rm(dst, { recursive: true, force: true }); - await fs.mkdir(path.dirname(dst), { recursive: true }); - await fs.cp(src, dst, { recursive: true, force: true }); + const stored = await ensureStoredSkill(src, ctx.storeDir, srcHash); + await materializeSkill(stored, dst, srcHash, state.materializedSkills?.[name]); + if (state.materializedSkills?.[name] !== srcHash) { + state.materializedSkills = { ...(state.materializedSkills ?? {}), [name]: srcHash }; + stateDirty = true; } const existing = getSkill(lock, composite); if (!existing) { - const commit = await resolveCommit(composite, ctx); + const parsed = parseCompositeSkillRef(composite, { projectRoot: ctx.projectRoot }); + if (parsed.source.kind === "git" && !commit) { + throw new Error(`could not determine the checked-out commit for skill "${name}"`); + } lock = upsertSkill(lock, composite, { computedHash: srcHash, resolvedAt: new Date().toISOString(), @@ -207,10 +285,12 @@ export async function createSkillSteps( ...(ref ? { ref } : {}), }); dirty = true; - } else if (ref && !existing.ref) { - // Backfill the tracked ref for legacy lock entries so subsequent runs - // fetch offline (no `ls-remote`). Self-heals after one run. - lock = upsertSkill(lock, composite, { ...existing, ref }); + } else if ((ref && !existing.ref) || (commit && !existing.resolvedCommit)) { + lock = upsertSkill(lock, composite, { + ...existing, + ...(ref ? { ref } : {}), + ...(commit ? { resolvedCommit: commit } : {}), + }); dirty = true; } }, @@ -220,6 +300,10 @@ export async function createSkillSteps( steps, async flush() { if (dirty && !ctx.dryRun) await writeLock(ctx.projectRoot, lock); + if (stateDirty && !ctx.dryRun) await writeState(ctx.statePath, state); + if (!ctx.dryRun) { + await cleanupLegacyCache(ctx, skillsDir, lock, Object.values(sources)); + } }, }; } @@ -235,33 +319,93 @@ export async function updateSkills( ctx: ResolveContext, ): Promise { const sources = config.skills?.sources ?? {}; - const targets = names.length > 0 ? names : Object.keys(sources); + const targets = [...new Set(names.length > 0 ? names : Object.keys(sources))]; const skillsDir = buildPaths(ctx.projectRoot, config).skillsDir; let lock = await readLock(ctx.projectRoot); - const updated: string[] = []; - - for (const name of targets) { + const state = await readState(ctx.statePath); + const updates = await runSkillTasks(targets, async (name) => { const composite = sources[name]; if (!composite) throw new Error(`skill "${name}" is not declared`); - const located = await locate(composite, ctx, getSkill(lock, composite)?.ref); + const parsed = parseCompositeSkillRef(composite, { projectRoot: ctx.projectRoot }); + const trackedRef = + parsed.source.kind === "git" + ? (parsed.source.ref ?? getSkill(lock, composite)?.ref) + : undefined; + const located = await locate(composite, ctx, { + ...(trackedRef ? { checkoutRef: trackedRef, trackedRef } : {}), + fresh: true, + }); if (!located) throw new Error(`skill "${name}" not found at ${composite}`); const hash = await hashSkillDir(located.src); - const commit = await resolveCommit(composite, ctx); - lock = upsertSkill(lock, composite, { - computedHash: hash, - resolvedAt: new Date().toISOString(), - ...(commit ? { resolvedCommit: commit } : {}), - ...(located.ref ? { ref: located.ref } : {}), - }); + const commit = located.commit; + if (parsed.source.kind === "git" && !commit) { + throw new Error(`could not determine the checked-out commit for skill "${name}"`); + } if (!ctx.dryRun) { const dst = path.join(skillsDir, name); - await fs.rm(dst, { recursive: true, force: true }); - await fs.mkdir(path.dirname(dst), { recursive: true }); - await fs.cp(located.src, dst, { recursive: true, force: true }); + const stored = await ensureStoredSkill(located.src, ctx.storeDir, hash); + await materializeSkill(stored, dst, hash); + state.materializedSkills = { ...(state.materializedSkills ?? {}), [name]: hash }; + } + return { name, composite, hash, commit, ref: located.ref }; + }); + + for (const update of updates) { + lock = upsertSkill(lock, update.composite, { + computedHash: update.hash, + resolvedAt: new Date().toISOString(), + ...(update.commit ? { resolvedCommit: update.commit } : {}), + ...(update.ref ? { ref: update.ref } : {}), + }); + } + + if (!ctx.dryRun && updates.length > 0) { + await writeLock(ctx.projectRoot, lock); + await writeState(ctx.statePath, state); + await cleanupLegacyCache(ctx, skillsDir, lock, Object.values(sources)); + } + return updates.map(({ name }) => name); +} + +async function cleanupLegacyCache( + ctx: ResolveContext, + skillsDir: string, + lock: Awaited>, + declaredSources: string[], +): Promise { + const legacyCache = path.join(ctx.agnosRoot, "cache"); + const stat = await fs.lstat(legacyCache).catch(() => null); + if (!stat) return; + + for (const source of declaredSources) { + const entry = getSkill(lock, source); + if (!entry) continue; + if (!(await findStoredSkill(ctx.storeDir, entry.computedHash))) { + ctx.logger.warn("legacy skill cache retained because migration is incomplete"); + return; + } + } + + const children = await fs.readdir(skillsDir).catch(() => []); + for (const child of children) { + const materialized = path.join(skillsDir, child); + const childStat = await fs.lstat(materialized).catch(() => null); + if (!childStat?.isSymbolicLink()) continue; + const target = await fs.realpath(materialized).catch(() => null); + if (target && isInside(target, legacyCache)) { + ctx.logger.warn(`could not remove legacy skill cache because "${child}" still links to it`); + return; } - updated.push(name); } - if (!ctx.dryRun && updated.length > 0) await writeLock(ctx.projectRoot, lock); - return updated; + try { + await fs.rm(legacyCache, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + } catch (error) { + ctx.logger.warn(`could not remove legacy skill cache: ${(error as Error).message}`); + } +} + +function isInside(candidate: string, parent: string): boolean { + const relative = path.relative(parent, candidate); + return relative !== "" && !relative.startsWith("..") && !path.isAbsolute(relative); } diff --git a/test/agents/adapters.test.ts b/test/agents/adapters.test.ts index 6d65675..0affdc0 100644 --- a/test/agents/adapters.test.ts +++ b/test/agents/adapters.test.ts @@ -16,7 +16,7 @@ function ctxFor(root: string): MaterializeContext { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/agents/cleanup.test.ts b/test/agents/cleanup.test.ts index 6d047a8..4f494ff 100644 --- a/test/agents/cleanup.test.ts +++ b/test/agents/cleanup.test.ts @@ -13,7 +13,7 @@ function ctxFor(root: string): MaterializeContext { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/agents/idempotency.test.ts b/test/agents/idempotency.test.ts index 710a503..b2eabb5 100644 --- a/test/agents/idempotency.test.ts +++ b/test/agents/idempotency.test.ts @@ -12,7 +12,7 @@ let tmp: string; const matCtx = (root: string): MaterializeContext => ({ agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/core/context.test.ts b/test/core/context.test.ts new file mode 100644 index 0000000..0b89cc0 --- /dev/null +++ b/test/core/context.test.ts @@ -0,0 +1,25 @@ +import { afterEach, describe, expect, it } from "vitest"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { buildResolveContext } from "../../src/core/context.js"; + +const roots: string[] = []; + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); +}); + +describe("buildResolveContext", () => { + it("uses an injected global store and project-local temporary work", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-context-")); + roots.push(root); + const storeDir = path.join(root, "global-store"); + + const ctx = await buildResolveContext({ projectRoot: root, storeDir }); + + expect(ctx.storeDir).toBe(storeDir); + await expect(fs.access(path.join(root, ".agnos", "tmp"))).resolves.toBeUndefined(); + await ctx.fetcher.cleanup(); + }); +}); diff --git a/test/core/ensure-link.test.ts b/test/core/ensure-link.test.ts index 7f9a696..96e724d 100644 --- a/test/core/ensure-link.test.ts +++ b/test/core/ensure-link.test.ts @@ -16,7 +16,7 @@ describe("ensureLink", () => { function linker() { return createLinker({ - cacheDir: path.join(dir, ".cache"), + probeDir: path.join(dir, ".tmp"), logger: createLogger(), copyFallback: true, }); diff --git a/test/core/init-steps.test.ts b/test/core/init-steps.test.ts index 96b9919..3ae6df7 100644 --- a/test/core/init-steps.test.ts +++ b/test/core/init-steps.test.ts @@ -20,7 +20,7 @@ const recordingLogger = (): Logger => ({ const ctxFor = (): ResolveContext => ({ agnosRoot: tmp, projectRoot: tmp, - cacheDir: path.join(tmp, ".agnos", "cache"), + storeDir: path.join(tmp, "store"), configPath: path.join(tmp, "agnos.json"), statePath: path.join(tmp, ".agnos", "state.json"), logger: recordingLogger(), diff --git a/test/core/link.test.ts b/test/core/link.test.ts index 7b464f6..2b2028e 100644 --- a/test/core/link.test.ts +++ b/test/core/link.test.ts @@ -7,12 +7,12 @@ import { createLogger } from "../../src/core/logger.js"; describe("createLinker", () => { let dir: string; - let cacheDir: string; + let probeDir: string; beforeEach(async () => { dir = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-link-")); - cacheDir = path.join(dir, "cache"); - await fs.mkdir(cacheDir, { recursive: true }); + probeDir = path.join(dir, "tmp"); + await fs.mkdir(probeDir, { recursive: true }); }); afterEach(async () => { @@ -24,7 +24,7 @@ describe("createLinker", () => { await fs.mkdir(target); await fs.writeFile(path.join(target, "marker.txt"), "hello"); - const linker = createLinker({ cacheDir, logger: createLogger() }); + const linker = createLinker({ probeDir, logger: createLogger() }); const linkPath = path.join(dir, "link"); const { kind } = await linker.link(target, linkPath); expect(["symlink", "junction"]).toContain(kind); @@ -36,14 +36,14 @@ describe("createLinker", () => { it("removes existing link before creating a new one (idempotency)", async () => { const target = path.join(dir, "target-dir"); await fs.mkdir(target); - const linker = createLinker({ cacheDir, logger: createLogger() }); + const linker = createLinker({ probeDir, logger: createLogger() }); const linkPath = path.join(dir, "link"); await linker.link(target, linkPath); await linker.link(target, linkPath); // should not throw }); it("canSymlinkDirs always returns true", async () => { - const linker = createLinker({ cacheDir, logger: createLogger() }); + const linker = createLinker({ probeDir, logger: createLogger() }); expect(await linker.canSymlinkDirs()).toBe(true); }); }); diff --git a/test/core/lock.test.ts b/test/core/lock.test.ts index 97b8c04..9248a74 100644 --- a/test/core/lock.test.ts +++ b/test/core/lock.test.ts @@ -69,6 +69,19 @@ describe("lock (per-skill)", () => { expect(raw.indexOf('"a-key"')).toBeLessThan(raw.indexOf('"z-key"')); }); + it("publishes concurrent equivalent writes atomically", async () => { + const root = await tmpRoot(); + const lock = upsertSkill(emptyLock(), "github:foo/bar/skills/pdf", { + computedHash: "a".repeat(64), + resolvedAt: "2026-08-13T00:00:00.000Z", + }); + + await Promise.all(Array.from({ length: 16 }, async () => writeLock(root, lock))); + + expect(await readLock(root)).toEqual(lock); + expect((await fs.readdir(root)).filter((name) => name.endsWith(".tmp"))).toEqual([]); + }); + it("rejects invalid JSON", async () => { const root = await tmpRoot(); await fs.writeFile(path.join(root, "agnos.lock.json"), "not json"); diff --git a/test/core/logger-progress.test.ts b/test/core/logger-progress.test.ts new file mode 100644 index 0000000..bb6d9a6 --- /dev/null +++ b/test/core/logger-progress.test.ts @@ -0,0 +1,38 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createLogger } from "../../src/core/logger.js"; + +const originalIsTTY = Object.getOwnPropertyDescriptor(process.stderr, "isTTY"); + +afterEach(() => { + vi.restoreAllMocks(); + if (originalIsTTY) Object.defineProperty(process.stderr, "isTTY", originalIsTTY); + else Reflect.deleteProperty(process.stderr, "isTTY"); +}); + +describe("logger progress", () => { + it("replaces a live TTY progress line and clears it on completion", () => { + Object.defineProperty(process.stderr, "isTTY", { configurable: true, value: true }); + const writes: string[] = []; + vi.spyOn(process.stderr, "write").mockImplementation((value) => { + writes.push(String(value)); + return true; + }); + const logger = createLogger(); + + const progress = logger.progress({ + message: "Installing skills 0%", + status: "total 4 | reused 0 | fetched 0", + }); + progress.update({ + message: "Installing skills 50%", + status: "total 4 | reused 1 | fetched 1", + }); + progress.stop(); + + const output = writes.join(""); + expect(output).toContain("Installing skills 0%"); + expect(output).toContain("Installing skills 50%"); + expect(output).toContain("total 4 | reused 1 | fetched 1"); + expect(output).toContain("\x1b[?25h"); + }); +}); diff --git a/test/core/resolver-nocache.test.ts b/test/core/resolver.test.ts similarity index 71% rename from test/core/resolver-nocache.test.ts rename to test/core/resolver.test.ts index e67dad2..5197d96 100644 --- a/test/core/resolver-nocache.test.ts +++ b/test/core/resolver.test.ts @@ -30,6 +30,10 @@ function installGitMock(): void { cb(null, { stdout: "ref: refs/heads/main\tHEAD\n", stderr: "" }); return; } + if (gitArgs.includes("rev-parse")) { + cb(null, { stdout: "deadbeef\n", stderr: "" }); + return; + } if (gitArgs[0] === "clone") { // dest is the last positional arg; drop a SKILL.md so the dir is non-empty. const dest = gitArgs[gitArgs.length - 1]!; @@ -60,8 +64,7 @@ describe("createRepoFetcher (sparse git clone)", () => { function fetcher() { return createRepoFetcher({ - projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + stagingDir: path.join(root, ".agnos", "tmp", "repos"), }); } @@ -72,6 +75,7 @@ describe("createRepoFetcher (sparse git clone)", () => { const res = await fetcher().fetch(source); expect(res.ref).toBe("main"); // from the mocked ls-remote symref + expect(res.commit).toBe("deadbeef"); await expect( fs.access(path.join(res.path, "skills", "demo", "SKILL.md")), ).resolves.toBeUndefined(); @@ -95,24 +99,41 @@ describe("createRepoFetcher (sparse git clone)", () => { expect(sparse[sparse.length - 1]).toBe("skills/pdf"); }); - it("reuses the cached clone on a second fetch", async () => { + it("reuses a staged checkout during the same session", async () => { const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); if (source.kind !== "git") throw new Error("expected git source"); + const repoFetcher = fetcher(); - await fetcher().fetch(source); + await repoFetcher.fetch(source); const clonesAfterFirst = calls.filter((c) => c[1] === "clone").length; - await fetcher().fetch(source); + await repoFetcher.fetch(source); const clonesAfterSecond = calls.filter((c) => c[1] === "clone").length; expect(clonesAfterFirst).toBe(1); - expect(clonesAfterSecond).toBe(1); // no re-clone + expect(clonesAfterSecond).toBe(1); }); - it("re-clones when noCache is set", async () => { + it("coalesces concurrent fetches for the same source", async () => { const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); if (source.kind !== "git") throw new Error("expected git source"); + const repoFetcher = fetcher(); - await fetcher().fetch(source); - await fetcher().fetch(source, { noCache: true }); + const [first, second] = await Promise.all([ + repoFetcher.fetch(source), + repoFetcher.fetch(source), + ]); + + expect(first.path).toBe(second.path); + expect(calls.filter((c) => c[1] === "ls-remote")).toHaveLength(1); + expect(calls.filter((c) => c[1] === "clone")).toHaveLength(1); + }); + + it("re-clones when fresh is set", async () => { + const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); + if (source.kind !== "git") throw new Error("expected git source"); + const repoFetcher = fetcher(); + + await repoFetcher.fetch(source); + await repoFetcher.fetch(source, { fresh: true }); expect(calls.filter((c) => c[1] === "clone").length).toBe(2); }); @@ -126,4 +147,18 @@ describe("createRepoFetcher (sparse git clone)", () => { const clone = calls.find((c) => c[1] === "clone")!; expect(clone[clone.indexOf("--branch") + 1]).toBe("canary"); }); + + it("removes every checkout created during the session", async () => { + const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); + if (source.kind !== "git") throw new Error("expected git source"); + const repoFetcher = fetcher(); + + const fetched = await repoFetcher.fetch(source); + await expect(fs.access(fetched.path)).resolves.toBeUndefined(); + + await repoFetcher.cleanup(); + + await expect(fs.access(fetched.path)).rejects.toThrow(); + await expect(fs.access(path.join(root, ".agnos", "cache", "repos"))).rejects.toThrow(); + }); }); diff --git a/test/core/run.test.ts b/test/core/run.test.ts new file mode 100644 index 0000000..e3f4128 --- /dev/null +++ b/test/core/run.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, vi } from "vitest"; +import path from "node:path"; +import { createLogger } from "../../src/core/logger.js"; +import { runOne } from "../../src/core/run.js"; +import type { PluginRegistry } from "../../src/core/plugin-loader.js"; +import type { Domain, RunContext } from "../../src/core/types/public.js"; + +const OPTIONS = { dry: false, once: true, quiet: true, interactive: false }; + +function buildRegistry(domain: Domain): PluginRegistry { + return { + agents: new Map(), + agentsByPackage: new Map(), + domains: new Map([[domain.id, { domain, packageName: "test" }]]), + collisions: [], + }; +} + +function buildContext(cleanup: () => Promise): RunContext { + const root = path.resolve("test-project"); + return { + agnosRoot: path.join(root, ".agnos"), + projectRoot: root, + storeDir: path.join(root, "store"), + configPath: path.join(root, "agnos.json"), + statePath: path.join(root, ".agnos", "state.json"), + logger: createLogger({ quiet: true }), + fetcher: { fetch: vi.fn(), cleanup } as never, + linker: {} as never, + flags: { dry: false, once: true, quiet: true, help: false, init: false, yes: true }, + }; +} + +describe("domain repository staging cleanup", () => { + it("cleans transient checkouts when a domain run fails", async () => { + const cleanup = vi.fn(async () => {}); + const failure = new Error("domain failed"); + const domain: Domain = { + id: "test", + description: "test", + kind: "writer", + priority: 1, + run: async () => { + throw failure; + }, + }; + + await expect( + runOne(buildRegistry(domain), domain.id, OPTIONS, buildContext(cleanup)), + ).rejects.toBe(failure); + expect(cleanup).toHaveBeenCalledOnce(); + }); +}); diff --git a/test/core/skill-materialize.test.ts b/test/core/skill-materialize.test.ts new file mode 100644 index 0000000..7601ae6 --- /dev/null +++ b/test/core/skill-materialize.test.ts @@ -0,0 +1,69 @@ +import { constants } from "node:fs"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { hashSkillDir } from "../../src/core/skill-hash.js"; +import { materializeSkill } from "../../src/core/skill-materialize.js"; + +let root: string; +let stored: string; +let destination: string; +let hash: string; + +beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-materialize-")); + stored = path.join(root, "store", "skill"); + destination = path.join(root, "project", "skill"); + await fs.mkdir(path.join(stored, "nested"), { recursive: true }); + await fs.writeFile(path.join(stored, "SKILL.md"), "# Test\n"); + await fs.writeFile(path.join(stored, "nested", "data.txt"), "data\n"); + hash = await hashSkillDir(stored); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(root, { recursive: true, force: true }); +}); + +describe("materializeSkill", () => { + it("imports an independent project tree and skips an unchanged materialization", async () => { + const first = await materializeSkill(stored, destination, hash); + expect(first.changed).toBe(true); + expect(await hashSkillDir(destination)).toBe(hash); + + await fs.rm(path.join(root, "store"), { recursive: true, force: true }); + expect(await fs.readFile(path.join(destination, "SKILL.md"), "utf8")).toBe("# Test\n"); + + const second = await materializeSkill(destination, destination, hash, hash); + expect(second.changed).toBe(false); + }); + + it("falls back to hard links when forced cloning is unavailable", async () => { + const copyFile = fs.copyFile.bind(fs); + vi.spyOn(fs, "copyFile").mockImplementation(async (source, target, mode) => { + if (mode === constants.COPYFILE_FICLONE_FORCE) throw new Error("clone unavailable"); + await copyFile(source, target, mode); + }); + + const result = await materializeSkill(stored, destination, hash); + + expect(result.methods).toEqual(new Set(["hardlink"])); + expect(await hashSkillDir(destination)).toBe(hash); + }); + + it("falls back to copies when cloning and hard links are unavailable", async () => { + const copyFile = fs.copyFile.bind(fs); + const link = vi.spyOn(fs, "link").mockRejectedValue(new Error("cross-device")); + vi.spyOn(fs, "copyFile").mockImplementation(async (source, target, mode) => { + if (mode === constants.COPYFILE_FICLONE_FORCE) throw new Error("clone unavailable"); + await copyFile(source, target, mode); + }); + + const result = await materializeSkill(stored, destination, hash); + + expect(link).toHaveBeenCalled(); + expect(result.methods).toEqual(new Set(["copy"])); + expect(await hashSkillDir(destination)).toBe(hash); + }); +}); diff --git a/test/core/skill-prepare.test.ts b/test/core/skill-prepare.test.ts index 987ce2f..b48019c 100644 --- a/test/core/skill-prepare.test.ts +++ b/test/core/skill-prepare.test.ts @@ -6,6 +6,7 @@ import { prepareSkills } from "../../src/core/skill-prepare.js"; import { hashSkillDir } from "../../src/core/skill-hash.js"; import { readLock, writeLock, upsertSkill, emptyLock } from "../../src/core/lock.js"; import { createLogger } from "../../src/core/logger.js"; +import { createLinker } from "../../src/core/fs/link.js"; import type { AgnosConfig, ResolveContext } from "../../src/core/types/public.js"; let root: string; @@ -28,22 +29,22 @@ async function seedSkill(rel: string, body = "# Test\n\n"): Promise { } function makeCtx(): ResolveContext { + const storeDir = path.join(root, "store"); + const logger = createLogger({ quiet: true }); return { projectRoot: root, configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), agnosRoot: path.join(root, ".agnos"), - cacheDir: path.join(root, ".agnos", "cache"), - logger: createLogger({ quiet: true }), + storeDir, + logger, // Stub fetcher: every call returns the shared repoCache root. The composite // ref's subPath then locates the actual skill within it. - fetcher: { fetch: async () => ({ path: repoCache }) }, - linker: { - canSymlinkFiles: async () => true, - canSymlinkDirs: async () => true, - link: async () => ({ kind: "symlink" }), - unlink: async () => {}, + fetcher: { + fetch: async () => ({ path: repoCache, ref: "main", commit: "deadbeef" }), + cleanup: async () => {}, }, + linker: createLinker({ probeDir: path.join(root, ".agnos", "tmp"), logger }), }; } diff --git a/test/core/skill-store.test.ts b/test/core/skill-store.test.ts new file mode 100644 index 0000000..471166a --- /dev/null +++ b/test/core/skill-store.test.ts @@ -0,0 +1,82 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { ensureStoredSkill, resolveStoredSkill } from "../../src/core/skill-store.js"; +import { hashSkillDir } from "../../src/core/skill-hash.js"; + +let root: string; + +beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-skill-store-")); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(root, { recursive: true, force: true }); +}); + +describe("skill store publication", () => { + it("retries a transient Windows rename failure without deleting the final path", async () => { + const source = path.join(root, "source"); + const storeDir = path.join(root, "store"); + await fs.mkdir(source, { recursive: true }); + await fs.writeFile(path.join(source, "SKILL.md"), "# Test\n"); + const hash = await hashSkillDir(source); + const stored = resolveStoredSkill(storeDir, hash); + const rename = fs.rename.bind(fs); + let attempts = 0; + const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (from, to) => { + attempts += 1; + if (attempts === 1) { + const error = new Error("file is busy") as NodeJS.ErrnoException; + error.code = "EPERM"; + throw error; + } + await rename(from, to); + }); + const rmSpy = vi.spyOn(fs, "rm"); + + expect(await ensureStoredSkill(source, storeDir, hash)).toBe(stored); + expect(renameSpy).toHaveBeenCalledTimes(2); + expect(rmSpy.mock.calls.some(([target]) => path.resolve(String(target)) === stored)).toBe( + false, + ); + expect(await fs.readFile(path.join(stored, "SKILL.md"), "utf8")).toBe("# Test\n"); + }); + + it("replaces a corrupt entry before reuse", async () => { + const source = path.join(root, "source"); + const storeDir = path.join(root, "store"); + await fs.mkdir(source, { recursive: true }); + await fs.writeFile(path.join(source, "SKILL.md"), "# Valid\n"); + const hash = await hashSkillDir(source); + const stored = resolveStoredSkill(storeDir, hash); + await fs.mkdir(stored, { recursive: true }); + await fs.writeFile(path.join(stored, "SKILL.md"), "# Corrupt\n"); + + await ensureStoredSkill(source, storeDir, hash); + + expect(await fs.readFile(path.join(stored, "SKILL.md"), "utf8")).toBe("# Valid\n"); + }); + + it("does not publish symbolic links excluded from the skill hash", async () => { + const source = path.join(root, "source"); + const external = path.join(root, "external"); + const storeDir = path.join(root, "store"); + await fs.mkdir(source, { recursive: true }); + await fs.mkdir(external, { recursive: true }); + await fs.writeFile(path.join(source, "SKILL.md"), "# Valid\n"); + await fs.writeFile(path.join(external, "secret.txt"), "secret\n"); + await fs.symlink( + external, + path.join(source, "linked"), + process.platform === "win32" ? "junction" : "dir", + ); + const hash = await hashSkillDir(source); + + const stored = await ensureStoredSkill(source, storeDir, hash); + + await expect(fs.access(path.join(stored, "linked"))).rejects.toThrow(); + }); +}); diff --git a/test/core/state.test.ts b/test/core/state.test.ts index 5b0328d..7c0e9f8 100644 --- a/test/core/state.test.ts +++ b/test/core/state.test.ts @@ -36,6 +36,7 @@ describe("state", () => { let s = await readState(statePath); s = markAgentInstalled(s, "claude-code"); s = markDomainInitialized(s, "skills"); + s.materializedSkills = { pdf: "a".repeat(64) }; await writeState(statePath, s); const reloaded = await readState(statePath); expect(reloaded).toEqual(s); diff --git a/test/core/store-path.test.ts b/test/core/store-path.test.ts new file mode 100644 index 0000000..298ab5a --- /dev/null +++ b/test/core/store-path.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import path from "node:path"; +import { resolveGlobalStoreDir } from "../../src/core/store-path.js"; + +describe("resolveGlobalStoreDir", () => { + it("uses AGNOS_STORE_DIR when configured", () => { + expect( + resolveGlobalStoreDir({ env: { AGNOS_STORE_DIR: "/shared/agnos" }, platform: "linux" }), + ).toBe(path.posix.resolve("/shared/agnos", "v1")); + }); + + it("uses the platform user data directory", () => { + expect( + resolveGlobalStoreDir({ + env: { LOCALAPPDATA: "C:\\Users\\test\\AppData\\Local" }, + homeDir: "C:\\Users\\test", + platform: "win32", + }), + ).toBe(path.win32.join("C:\\Users\\test\\AppData\\Local", "agnos", "store", "v1")); + expect(resolveGlobalStoreDir({ env: {}, homeDir: "/Users/test", platform: "darwin" })).toBe( + path.posix.join("/Users/test", "Library", "agnos", "store", "v1"), + ); + expect(resolveGlobalStoreDir({ env: {}, homeDir: "/home/test", platform: "linux" })).toBe( + path.posix.join("/home/test", ".local", "share", "agnos", "store", "v1"), + ); + }); +}); diff --git a/test/core/watch.test.ts b/test/core/watch.test.ts index aa50bef..d50249c 100644 --- a/test/core/watch.test.ts +++ b/test/core/watch.test.ts @@ -14,7 +14,7 @@ function ctxFor(root: string): RunContext { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/docs/compile.test.ts b/test/docs/compile.test.ts index 9a6c43f..481348d 100644 --- a/test/docs/compile.test.ts +++ b/test/docs/compile.test.ts @@ -13,7 +13,7 @@ function ctxFor(root: string): ResolveContext { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/domains/commands.test.ts b/test/domains/commands.test.ts index bae1790..802ba1c 100644 --- a/test/domains/commands.test.ts +++ b/test/domains/commands.test.ts @@ -16,13 +16,13 @@ let capturedLogger: Logger | undefined; const ctxFor = (args: string[], extra: Record = {}): CommandContext => ({ agnosRoot: tmp, projectRoot: tmp, - cacheDir: path.join(tmp, ".agnos", "cache"), + storeDir: path.join(tmp, "store"), configPath: path.join(tmp, "agnos.json"), statePath: path.join(tmp, ".agnos", "state.json"), logger: capturedLogger ?? createLogger({ quiet: true }), // Real fetcher: for `file:` (local) sources it just returns the absolute path, // so skills `add` discovery works without any network access. - fetcher: createRepoFetcher({ projectRoot: tmp, cacheDir: path.join(tmp, ".agnos", "cache") }), + fetcher: createRepoFetcher({ stagingDir: path.join(tmp, ".agnos", "tmp", "repos") }), linker: {} as never, dryRun: false, args, diff --git a/test/domains/empty-slices.test.ts b/test/domains/empty-slices.test.ts index 2db5b75..84fdf8b 100644 --- a/test/domains/empty-slices.test.ts +++ b/test/domains/empty-slices.test.ts @@ -21,7 +21,7 @@ function base(root: string) { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/domains/global-skill-store.test.ts b/test/domains/global-skill-store.test.ts new file mode 100644 index 0000000..7e6cec0 --- /dev/null +++ b/test/domains/global-skill-store.test.ts @@ -0,0 +1,212 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import type { AgnosConfig, ResolveContext } from "../../src/core/index.js"; +import { createLinker, createLogger, hashSkillDir, writeLock } from "../../src/core/index.js"; +import { createSkillSteps } from "../../src/domains/skills/steps.js"; +import { runSkillPipeline } from "../../src/domains/skills/pipeline.js"; + +const COMPOSITE = "github:owner/repo/skills/tool#main"; +const CONFIG: AgnosConfig = { + schemaVersion: 1, + skills: { sources: { tool: COMPOSITE } }, +}; + +let root: string; +let checkout: string; +let storeDir: string; +let hash: string; + +beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-global-store-")); + checkout = path.join(root, "checkout"); + storeDir = path.join(root, "store"); + await fs.mkdir(path.join(checkout, "skills", "tool"), { recursive: true }); + await fs.writeFile(path.join(checkout, "skills", "tool", "SKILL.md"), "# Shared\n"); + hash = await hashSkillDir(path.join(checkout, "skills", "tool")); +}); + +afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); +}); + +function context(projectRoot: string, fetch: ResolveContext["fetcher"]["fetch"]): ResolveContext { + const logger = createLogger({ quiet: true }); + return { + agnosRoot: path.join(projectRoot, ".agnos"), + projectRoot, + storeDir, + configPath: path.join(projectRoot, "agnos.json"), + statePath: path.join(projectRoot, ".agnos", "state.json"), + logger, + fetcher: { fetch, cleanup: async () => {} }, + linker: createLinker({ probeDir: path.join(projectRoot, ".agnos", "tmp"), logger }), + dryRun: false, + }; +} + +async function seedLock( + projectRoot: string, + resolvedCommit: string | null = "deadbeef", +): Promise { + await fs.mkdir(projectRoot, { recursive: true }); + await writeLock(projectRoot, { + version: 1, + skills: { + [COMPOSITE]: { + computedHash: hash, + resolvedAt: "2026-08-13T00:00:00.000Z", + ref: "main", + ...(resolvedCommit ? { resolvedCommit } : {}), + }, + }, + }); +} + +describe("global skill store", () => { + it("shares a locked skill across projects and leaves independent materializations", async () => { + const firstRoot = path.join(root, "first"); + const secondRoot = path.join(root, "second"); + await Promise.all([seedLock(firstRoot), seedLock(secondRoot)]); + const fetch = vi.fn(async (_source, options) => { + expect(options?.ref).toBe("deadbeef"); + return { path: checkout, ref: "deadbeef", commit: "deadbeef" }; + }); + + const first = await createSkillSteps(CONFIG, context(firstRoot, fetch)); + const firstResult = await runSkillPipeline( + CONFIG.skills?.sources ?? {}, + first.steps, + createLogger({ quiet: true }), + ); + await first.flush(); + + const noNetwork = vi.fn(async () => { + throw new Error("network should not be used"); + }); + const second = await createSkillSteps(CONFIG, context(secondRoot, noNetwork)); + const secondResult = await runSkillPipeline( + CONFIG.skills?.sources ?? {}, + second.steps, + createLogger({ quiet: true }), + ); + await second.flush(); + + expect(fetch).toHaveBeenCalledOnce(); + expect(noNetwork).not.toHaveBeenCalled(); + expect(firstResult.progress).toMatchObject({ reused: 0, fetched: 1 }); + expect(secondResult.progress).toMatchObject({ reused: 1, fetched: 0 }); + + await fs.rm(storeDir, { recursive: true, force: true }); + await expect( + fs.readFile(path.join(firstRoot, ".agnos", "skills", "tool", "SKILL.md"), "utf8"), + ).resolves.toBe("# Shared\n"); + await expect( + fs.readFile(path.join(secondRoot, ".agnos", "skills", "tool", "SKILL.md"), "utf8"), + ).resolves.toBe("# Shared\n"); + }); + + it("promotes and removes a valid legacy project cache", async () => { + const projectRoot = path.join(root, "legacy"); + await seedLock(projectRoot); + const legacy = path.join(projectRoot, ".agnos", "cache", "skills", hash); + await fs.cp(path.join(checkout, "skills", "tool"), legacy, { recursive: true }); + const installed = path.join(projectRoot, ".agnos", "skills", "tool"); + await fs.mkdir(path.dirname(installed), { recursive: true }); + await fs.symlink(legacy, installed, process.platform === "win32" ? "junction" : "dir"); + const fetch = vi.fn(async () => { + throw new Error("legacy migration should not fetch"); + }); + + const handle = await createSkillSteps(CONFIG, context(projectRoot, fetch)); + await runSkillPipeline( + CONFIG.skills?.sources ?? {}, + handle.steps, + createLogger({ quiet: true }), + ); + await handle.flush(); + + expect(fetch).not.toHaveBeenCalled(); + expect((await fs.lstat(installed)).isSymbolicLink()).toBe(false); + await expect(fs.access(path.join(projectRoot, ".agnos", "cache"))).rejects.toThrow(); + await expect( + fs.access(path.join(storeDir, "skills", hash, "SKILL.md")), + ).resolves.toBeUndefined(); + }); + + it("fetches and backfills a matching legacy lock entry without a commit", async () => { + const projectRoot = path.join(root, "legacy-lock"); + await seedLock(projectRoot, null); + const fetch = vi.fn(async (_source, options) => { + expect(options?.ref).toBe("main"); + return { path: checkout, ref: "main", commit: "deadbeef" }; + }); + const ctx = context(projectRoot, fetch); + + const handle = await createSkillSteps(CONFIG, ctx); + const result = await runSkillPipeline(CONFIG.skills?.sources ?? {}, handle.steps, ctx.logger); + await handle.flush(); + + const lock = JSON.parse(await fs.readFile(path.join(projectRoot, "agnos.lock.json"), "utf8")); + expect(fetch).toHaveBeenCalledOnce(); + expect(result.installed).toEqual(["tool"]); + expect(lock.skills[COMPOSITE].resolvedCommit).toBe("deadbeef"); + }); + + it("rejects and refetches a corrupt global entry", async () => { + const projectRoot = path.join(root, "corrupt-store"); + await seedLock(projectRoot); + const stored = path.join(storeDir, "skills", hash); + await fs.mkdir(stored, { recursive: true }); + await fs.writeFile(path.join(stored, "SKILL.md"), "# Corrupt\n"); + const fetch = vi.fn(async () => ({ path: checkout, ref: "deadbeef", commit: "deadbeef" })); + const ctx = context(projectRoot, fetch); + + const handle = await createSkillSteps(CONFIG, ctx); + await runSkillPipeline(CONFIG.skills?.sources ?? {}, handle.steps, ctx.logger); + await handle.flush(); + + expect(fetch).toHaveBeenCalledOnce(); + expect(await hashSkillDir(stored)).toBe(hash); + }); + + it("removes an unreferenced legacy cache when a declared source is not pinned", async () => { + const projectRoot = path.join(root, "unpinned-legacy"); + const legacy = path.join(projectRoot, ".agnos", "cache", "skills", "unreferenced"); + await fs.mkdir(legacy, { recursive: true }); + await fs.writeFile(path.join(legacy, "SKILL.md"), "# Unreferenced\n"); + const fetch = vi.fn(async () => { + throw new Error("offline"); + }); + const handle = await createSkillSteps(CONFIG, context(projectRoot, fetch)); + + await runSkillPipeline( + CONFIG.skills?.sources ?? {}, + handle.steps, + createLogger({ quiet: true }), + ); + await handle.flush(); + + await expect(fs.access(path.join(projectRoot, ".agnos", "cache"))).rejects.toThrow(); + }); + + it("retains an unusable legacy cache when migration cannot finish", async () => { + const projectRoot = path.join(root, "failed-legacy"); + await seedLock(projectRoot); + const legacy = path.join(projectRoot, ".agnos", "cache", "skills", hash); + await fs.mkdir(legacy, { recursive: true }); + await fs.writeFile(path.join(legacy, "SKILL.md"), "# Corrupt\n"); + const fetch = vi.fn(async () => { + throw new Error("offline"); + }); + const ctx = context(projectRoot, fetch); + const handle = await createSkillSteps(CONFIG, ctx); + + const result = await runSkillPipeline(CONFIG.skills?.sources ?? {}, handle.steps, ctx.logger); + await handle.flush(); + + expect(result.buckets.moved).toEqual(["tool"]); + await expect(fs.access(path.join(projectRoot, ".agnos", "cache"))).resolves.toBeUndefined(); + }); +}); diff --git a/test/domains/skills-concurrency.test.ts b/test/domains/skills-concurrency.test.ts new file mode 100644 index 0000000..57df3c9 --- /dev/null +++ b/test/domains/skills-concurrency.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { runSkillTasks } from "../../src/domains/skills/concurrency.js"; + +describe("runSkillTasks", () => { + it("waits for active workers before reporting a failure", async () => { + let completed = false; + const run = runSkillTasks(["fail", "finish"], async (value) => { + if (value === "fail") throw new Error("failed"); + await new Promise((resolve) => { + setTimeout(resolve, 10); + }); + completed = true; + return value; + }); + + await expect(run).rejects.toThrow("failed"); + expect(completed).toBe(true); + }); +}); diff --git a/test/domains/skills-pipeline.test.ts b/test/domains/skills-pipeline.test.ts index 60e83fc..b0cc9a9 100644 --- a/test/domains/skills-pipeline.test.ts +++ b/test/domains/skills-pipeline.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import fs from "node:fs/promises"; import path from "node:path"; import os from "node:os"; @@ -8,7 +8,7 @@ import type { LogParts, ResolveContext, } from "../../src/core/index.js"; -import { createLogger } from "../../src/core/index.js"; +import { createLinker, createLogger, hashSkillDir, writeLock } from "../../src/core/index.js"; import { createSkillSteps, pruneSkills, updateSkills } from "../../src/domains/skills/steps.js"; import { runSkillPipeline } from "../../src/domains/skills/pipeline.js"; import skillsDomain from "../../src/domains/skills/index.js"; @@ -16,19 +16,23 @@ import skillsDomain from "../../src/domains/skills/index.js"; let tmp: string; // A fetcher that resolves a local `file:` source to its own directory (no network). -const ctxFor = (): ResolveContext => ({ - agnosRoot: tmp, - projectRoot: tmp, - cacheDir: path.join(tmp, ".agnos", "cache"), - configPath: path.join(tmp, "agnos.json"), - statePath: path.join(tmp, ".agnos", "state.json"), - logger: createLogger({ quiet: true }), - fetcher: { - fetch: async (source: { absolutePath?: string }) => ({ path: source.absolutePath ?? tmp }), - } as never, - linker: {} as never, - dryRun: false, -}); +const ctxFor = (): ResolveContext => { + const storeDir = path.join(tmp, "store"); + const logger = createLogger({ quiet: true }); + return { + agnosRoot: tmp, + projectRoot: tmp, + storeDir, + configPath: path.join(tmp, "agnos.json"), + statePath: path.join(tmp, ".agnos", "state.json"), + logger, + fetcher: { + fetch: async (source: { absolutePath?: string }) => ({ path: source.absolutePath ?? tmp }), + } as never, + linker: createLinker({ probeDir: path.join(tmp, ".agnos", "tmp"), logger }), + dryRun: false, + }; +}; const cfg = (): AgnosConfig => ({ schemaVersion: 1, @@ -79,6 +83,10 @@ describe("skills prep pipeline (steps)", () => { expect(await fs.readFile(path.join(tmp, installed), "utf8")).toContain("My Tool"); const lock = JSON.parse(await fs.readFile(path.join(tmp, "agnos.lock.json"), "utf8")); expect(Object.keys(lock.skills)).toEqual(["file:./skill-src"]); + expect(await fs.lstat(path.join(tmp, ".agnos", "skills", "mytool"))).toMatchObject({}); + expect((await fs.lstat(path.join(tmp, ".agnos", "skills", "mytool"))).isSymbolicLink()).toBe( + false, + ); const h2 = await createSkillSteps(cfg(), ctx); const src = await fetchSrc(h2.steps); @@ -86,6 +94,50 @@ describe("skills prep pipeline (steps)", () => { expect(await h2.steps.version("mytool", src)).toBe(true); // no resolvedCommit baseline }); + it("restores a remote skill directly from the content-addressed store", async () => { + const composite = "github:owner/repo/skills/mytool#main"; + const hash = await hashSkillDir(path.join(tmp, "skill-src")); + const stored = path.join(tmp, "store", "skills", hash); + await fs.cp(path.join(tmp, "skill-src"), stored, { recursive: true }); + await writeLock(tmp, { + version: 1, + skills: { + [composite]: { + computedHash: hash, + resolvedAt: "2026-08-13T00:00:00.000Z", + ref: "main", + resolvedCommit: "deadbeef", + }, + }, + }); + const fetch = vi.fn(async () => { + throw new Error("remote fetch should not run"); + }); + const config: AgnosConfig = { + schemaVersion: 1, + skills: { sources: { mytool: composite } }, + }; + const ctx = { ...ctxFor(), fetcher: { fetch } as never }; + const handle = await createSkillSteps(config, ctx); + const result = await runSkillPipeline(config.skills?.sources ?? {}, handle.steps, ctx.logger); + await handle.flush(); + + expect(result.installed).toEqual(["mytool"]); + expect(fetch).not.toHaveBeenCalled(); + expect( + await fs.readFile(path.join(tmp, ".agnos", "skills", "mytool", "SKILL.md"), "utf8"), + ).toBe("# My Tool\n"); + await expect(fs.access(path.join(tmp, ".agnos", "cache", "repos"))).rejects.toThrow(); + + const materializedMarker = path.join(tmp, ".agnos", "skills", "mytool", "SKILL.md"); + await fs.rm(materializedMarker); + await fs.writeFile(materializedMarker, "# Corrupted\n"); + const verification = await createSkillSteps(config, ctx, { verifyMaterialized: true }); + const fetched = await verification.steps.fetch("mytool", composite); + if (!fetched.src) throw new Error("expected stored skill to resolve"); + expect(await verification.steps.integrity("mytool", fetched.src)).toBe(false); + }); + it("integrity reports changed when content drifts from the lock", async () => { const ctx = ctxFor(); const h = await createSkillSteps(cfg(), ctx); @@ -140,6 +192,21 @@ describe("skills prep pipeline (steps)", () => { }; expect(Object.keys(nextLock.skills)).toEqual(["file:./skill-src"]); }); + + it("prunes a materialized skill without removing stored content", async () => { + const ctx = ctxFor(); + const handle = await createSkillSteps(cfg(), ctx); + await runSkillPipeline(SOURCES, handle.steps, ctx.logger); + await handle.flush(); + const linked = path.join(tmp, ".agnos", "skills", "mytool"); + const lock = JSON.parse(await fs.readFile(path.join(tmp, "agnos.lock.json"), "utf8")); + const stored = path.join(ctx.storeDir, "skills", lock.skills["file:./skill-src"].computedHash); + + await pruneSkills({ schemaVersion: 1, skills: { sources: {} } }, ctx); + + await expect(fs.access(linked)).rejects.toThrow(); + await expect(fs.access(path.join(stored, "SKILL.md"))).resolves.toBeUndefined(); + }); }); describe("skills domain run", () => { diff --git a/test/rules/rules-domain.test.ts b/test/rules/rules-domain.test.ts index cc7fc8c..82ea708 100644 --- a/test/rules/rules-domain.test.ts +++ b/test/rules/rules-domain.test.ts @@ -12,7 +12,7 @@ function ctxFor(root: string): ResolveContext { return { agnosRoot: root, projectRoot: root, - cacheDir: path.join(root, ".agnos", "cache"), + storeDir: path.join(root, "store"), configPath: path.join(root, "agnos.json"), statePath: path.join(root, ".agnos", "state.json"), logger: createLogger({ quiet: true }), diff --git a/test/skills/pipeline.test.ts b/test/skills/pipeline.test.ts index b91bf29..90ff73e 100644 --- a/test/skills/pipeline.test.ts +++ b/test/skills/pipeline.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from "vitest"; import { runSkillPipeline, type SkillSteps } from "../../src/domains/skills/pipeline.js"; import { mergeSkillSources } from "../../src/domains/skills/migrate.js"; import { createLogger } from "../../src/core/index.js"; +import type { LogInput } from "../../src/core/index.js"; const logger = createLogger({ quiet: true }); @@ -47,13 +48,13 @@ describe("runSkillPipeline", () => { it("threads the fetched ref into install", async () => { const installSpy = vi.fn(async () => {}); const s: SkillSteps = { - fetch: async () => ({ ok: true, src: "/src/x", ref: "main" }), + fetch: async () => ({ ok: true, src: "/src/x", ref: "main", commit: "deadbeef" }), version: async () => true, integrity: async () => true, install: installSpy, }; await runSkillPipeline({ x: "r" }, s, logger); - expect(installSpy).toHaveBeenCalledWith("x", "/src/x", "main"); + expect(installSpy).toHaveBeenCalledWith("x", "/src/x", "main", "deadbeef"); }); it("installs everything and reports no buckets when all skills are clean", async () => { @@ -61,6 +62,75 @@ describe("runSkillPipeline", () => { expect(res.installed).toEqual(["a", "b"]); expect(res.buckets).toEqual({ moved: [], changed: [] }); }); + + it("reconciles independent skills concurrently while preserving declaration order", async () => { + let active = 0; + let peak = 0; + const concurrentSteps: SkillSteps = { + async fetch(name) { + active += 1; + peak = Math.max(peak, active); + await Promise.resolve(); + active -= 1; + return { ok: true, src: `/src/${name}` }; + }, + version: async () => true, + integrity: async () => true, + install: async () => {}, + }; + + const res = await runSkillPipeline({ a: "r", b: "r", c: "r" }, concurrentSteps, logger); + + expect(peak).toBeGreaterThan(1); + expect(res.installed).toEqual(["a", "b", "c"]); + }); + + it("reports percentage, total, reused, and fetched counters", async () => { + const updates: LogInput[] = []; + const stop = vi.fn(); + const progressLogger = { + ...logger, + progress(initial: LogInput) { + updates.push(initial); + return { + update(next: LogInput) { + updates.push(next); + }, + stop, + }; + }, + }; + const progressSteps: SkillSteps = { + fetch: async (name) => + name === "missing" + ? { ok: false } + : { + ok: true, + src: `/src/${name}`, + source: name === "cached" ? "reused" : "fetched", + }, + version: async () => true, + integrity: async () => true, + install: async () => {}, + }; + + const result = await runSkillPipeline( + { cached: "a", remote: "b", missing: "c" }, + progressSteps, + progressLogger, + ); + + expect(result.progress).toEqual({ total: 3, completed: 3, reused: 1, fetched: 1 }); + expect(updates[0]).toEqual({ + message: "Installing skills 0%", + status: "total 3 | reused 0 | fetched 0", + }); + expect(updates.at(-1)).toEqual({ + message: "Installing skills 100%", + status: "total 3 | reused 1 | fetched 1", + }); + expect(stop).toHaveBeenCalledOnce(); + }); }); describe("mergeSkillSources (migrate policy)", () => {