diff --git a/docs/ADRs/0024-harness-definitions.md b/docs/ADRs/0024-harness-definitions.md index 775dfc999a..b11ba54bcf 100644 --- a/docs/ADRs/0024-harness-definitions.md +++ b/docs/ADRs/0024-harness-definitions.md @@ -230,12 +230,12 @@ agents/ # Agent definitions (.md, following Claude standard) arch-reviewer.md docs-reviewer.md -skills/ # Skill definitions (SKILL.md, following AgentSkills standard) - triage-coordination/SKILL.md - detect-duplicates/SKILL.md - assess-completeness/SKILL.md - code-implementation/SKILL.md - testing-conventions/SKILL.md +skills/ # Skill directories (each contains SKILL.md + companion files) + triage-coordination/ + detect-duplicates/ + assess-completeness/ + code-implementation/ + testing-conventions/ env/ # Environment files delivered into the sandbox gcp-vertex.env # May contain ${VAR} references expanded at bootstrap diff --git a/docs/ADRs/0038-universal-harness-access.md b/docs/ADRs/0038-universal-harness-access.md index f0cfbcd81b..eee65ad57b 100644 --- a/docs/ADRs/0038-universal-harness-access.md +++ b/docs/ADRs/0038-universal-harness-access.md @@ -69,11 +69,13 @@ Extend every path field in the harness schema to support three forms: 1. **Absolute file path:** `/opt/fullsend/agents/code.md` 2. **Relative file path:** `agents/code.md` (resolved against `.fullsend` base) -3. **HTTP(S) URL:** `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=abc123...` +3. **HTTP(S) URL:** `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=abc123...` (single-file resources) or `https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust#sha256=...` (directory resources like skills) When the runner encounters a URL, it fetches the resource, caches it locally (content-addressed by SHA256), and validates its integrity before use. All referenced resources (skills, policies, scripts, binaries) support the same three forms, creating a uniform resolution model. -**Note on URL immutability:** Example URLs in this ADR use GitHub `raw.githubusercontent.com` URLs with commit SHAs (e.g., `8cd3799...`) to ensure immutability. Branch-based URLs like `https://github.com/fullsend-ai/library/blob/main/agents/code.md` point to mutable content—the branch advances as commits are added. For production use, always use commit-pinned URLs or rely on the mandatory `#sha256=...` integrity hash to detect changes. +**Skill directory model:** Skills are directories containing `SKILL.md` plus optional companion files (`scripts/`, `sub-agents/`, `assets/`). The entire directory tree is uploaded to the sandbox. When referenced via URL, skill URLs point to the directory (not the `SKILL.md` file) and use `github.com/.../tree/...` format. The integrity hash covers the entire directory tree (tree hash). The resolver uses forge APIs (GitHub Contents API, GitLab equivalent) to list directory contents, fetch all files, and reconstruct the directory tree in the local cache. Skills from non-forge HTTPS URLs are rejected because HTTP has no standard directory listing mechanism. Agents and policies remain single-file resources and work with any HTTPS URL. + +**Note on URL immutability:** Example URLs in this ADR use GitHub commit-pinned URLs with commit SHAs (e.g., `8cd3799...`) to ensure immutability. For single-file resources (agents, policies), `raw.githubusercontent.com` URLs are used. For directory resources (skills), `github.com/.../tree/...` URLs are used. Branch-based URLs point to mutable content—the branch advances as commits are added. For production use, always use commit-pinned URLs or rely on the mandatory `#sha256=...` integrity hash to detect changes. **Transitive closure:** A URL-referenced skill that itself references other skills via `dependencies:` in its frontmatter triggers a recursive fetch. The runner builds a complete dependency graph before sandbox creation. Skill-level `policy:` is deferred — the harness-level policy governs sandboxing, and there is no clear runtime semantic for a skill overriding or composing with the harness policy. @@ -87,9 +89,11 @@ Like Option A, but all URLs must include an integrity hash: ```yaml agent: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=abc123... +skills: + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust#sha256=... ``` -The runner verifies the fetched content matches the declared hash before using it. This prevents TOCTOU attacks at the cost of requiring hash management for every remote resource. +The runner verifies the fetched content matches the declared hash before using it. For single-file resources (agents, policies), the hash covers the file content. For directory resources (skills), the hash covers the entire directory tree. This prevents TOCTOU attacks at the cost of requiring hash management for every remote resource. **Trade-offs:** - **Pros:** Eliminates silent substitution attacks. Makes dependency versions explicit. @@ -143,6 +147,7 @@ With the hybrid approach (URL support for declarative resources, local files for ### What changes - **Harness schema:** Declarative resource path fields (`agent`, `policy`, `skills[]`) accept URLs. Executable resource fields (`pre_script`, `post_script`) and configuration files (`host_files[].src`) must be local paths (see "Security implications" section for rationale). +- **Skill resolution model:** Skills referenced via URL point to directories, not individual `SKILL.md` files. The resolver uses forge APIs (GitHub Contents API, GitLab equivalent) to list directory contents, fetch all files, and reconstruct the directory tree in the local cache. Skills from non-forge HTTPS URLs are rejected because HTTP has no standard directory listing mechanism. Agents and policies remain single-file resources and work with any HTTPS URL. - **Resolution logic:** The runner resolves URLs by fetching, caching (content-addressed), and validating before use. - **Transitive closure (Phase 2 feature):** URL-referenced resources can themselves reference other resources via URL, creating a dependency tree. Phase 1 implementation limits URL references to single-level only (harness can reference URL-based resources, but those resources cannot reference additional URLs). Phase 2 adds full transitive resolution with: - **Visited node tracking:** The resolver maintains a set of already-visited URLs. If a URL is encountered twice in the same dependency chain, the resolver returns an error indicating a circular dependency. @@ -201,7 +206,9 @@ See `docs/plans/universal-harness-access.md` for detailed implementation plan. K 3. **Transitive resolver (new package `internal/resolve/`):** Build dependency graph for harnesses, recursively fetch and validate. 4. **Access policy enforcement (`internal/security/`):** Validate fetched resources against org-level and harness-level policies. 5. **Schema extension:** Add `allowed_remote_resources[]` to harness YAML. -6. **CLI flag:** `fullsend run --offline` to disable all network fetches (fail if harness references a URL). +6. **Forge interface extension (`internal/forge/`):** Add `ListDirectoryContents` and `GetFileContentAtRef` to support skill directory listing and file retrieval at specific refs. +7. **Directory cache (`internal/fetch/cache.go`):** Add `CachePutDir`, `CacheGetDir`, and `ComputeTreeHash` for caching directory trees. +8. **CLI flag:** `fullsend run --offline` to disable all network fetches (fail if harness references a URL). ### Differences from traditional package management @@ -229,7 +236,7 @@ To support community sharing and provide a trusted source for harness components Instead, the model applies **uniform security to all remote resources:** -- **All remote resources require hash pinning**, regardless of source. `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=abc123...` and `https://example.com/my-skill.md#sha256=def456...` have the same verification requirements. +- **All remote resources require hash pinning**, regardless of source. `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=abc123...` (single-file) and `https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust#sha256=...` (directory) have the same verification requirements. For directory resources, the hash covers the entire directory tree. - **User-controlled allowlist with sensible defaults.** Organizations configure allowed URL prefixes in `config.yaml`: ```yaml @@ -281,7 +288,7 @@ The following design questions have been resolved as part of this ADR: **Rationale:** Explicit URLs make dependencies auditable and prevent dependency confusion attacks. Version resolution requires a central registry (complexity, availability, trust) or org-level alias files (indirection that obscures actual dependencies). Full URLs are verbose but clear. -**Alternative for ergonomics:** Organizations can use shell aliases or wrapper scripts if they frequently reference the same base URLs. Example: `fullsend run $LIBRARY/harness/rust-linter.yaml#sha256=...` where `LIBRARY=https://raw.githubusercontent.com/fullsend-ai/library/8cd3799...` +**Alternative for ergonomics:** Organizations can use shell aliases or wrapper scripts if they frequently reference the same base URLs. Example: `fullsend run $LIBRARY/harness/rust-linter.yaml#sha256=...` where `LIBRARY=https://github.com/fullsend-ai/library/tree/8cd3799...` #### 5. Offline mode @@ -309,15 +316,23 @@ harnesses: - field: agent url: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/rust.md sha256: def456... + type: file fetched_at: "2026-05-12T10:00:00Z" - field: skills[0] - url: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../skills/cargo-check/SKILL.md - sha256: ghi789... + url: https://github.com/fullsend-ai/library/tree/8cd3799.../skills/cargo-check + sha256: ... + type: directory fetched_at: "2026-05-12T10:00:00Z" + files: + - path: SKILL.md + sha256: abc123... + - path: scripts/check.sh + sha256: def456... transitive_deps: - field: skills[dep0] url: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../policies/rust-sandbox.yaml sha256: jkl012... + type: file fetched_at: "2026-05-12T10:00:00Z" ``` @@ -333,9 +348,9 @@ harnesses: **VCS-specific schemes trade-off:** Structured references like `git+https://` enable automation (dependabot-style updates that understand git semantics), make VCS coupling explicit, and provide stable API via tags/commits. However, they increase complexity (multiple URL parsers, VCS-specific logic) and reduce portability (what if a resource moves from GitHub to GitLab?). -**Future enhancement:** Phase 2/3 could add opt-in support for structured references as an alternative to bare URLs. The implementation plan would translate `github:org/repo/path@ref` to a raw.githubusercontent.com URL with commit SHA lookup, then apply the same fetch/cache/validate logic. Both URL forms would coexist. +**Future enhancement:** Phase 2/3 could add opt-in support for structured references as an alternative to bare URLs. The implementation plan would translate `github:org/repo/path@ref` to the appropriate GitHub URL with commit SHA lookup, then apply the same fetch/cache/validate logic. Both URL forms would coexist. -**Current recommendation:** Use commit-pinned raw.githubusercontent.com URLs (e.g., `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=...`) for GitHub-hosted resources. The commit SHA in the URL path provides immutability at the URL level, and the `#sha256=...` fragment provides content integrity. This achieves the same goals as `git+https://` without requiring VCS-specific logic. +**Current recommendation:** Use commit-pinned URLs for GitHub-hosted resources. For single-file resources (agents, policies), use `raw.githubusercontent.com` URLs (e.g., `https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md#sha256=...`). For directory resources (skills), use `github.com/.../tree/...` URLs (e.g., `https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust#sha256=...`). The commit SHA in the URL path provides immutability at the URL level, and the `#sha256=...` fragment provides content integrity. ## Related Work @@ -349,4 +364,4 @@ The proposed model follows the GitHub Actions approach: URL-based references wit ## Implementation Plan -See `docs/plans/universal-harness-access.md` for full implementation details, security analysis, and migration path. See `docs/plans/universal-harness-access-phase1.md` for the phased PR breakdown (Phase 1 MVP) and `docs/plans/universal-harness-access-phase2.md` for Phase 2 (transitive dependency resolution). +See `docs/plans/universal-harness-access.md` for full implementation details, security analysis, and migration path. See `docs/plans/universal-harness-access-phase1.md` for the phased PR breakdown (Phase 1 MVP), `docs/plans/universal-harness-access-phase2.md` for Phase 2 (transitive dependency resolution), `docs/plans/universal-harness-access-phase3.md` for Phase 3 (lock files), and `docs/plans/universal-harness-access-phase4.md` for Phase 4 (runtime dependency loading). diff --git a/docs/glossary.md b/docs/glossary.md index 4d071fa202..94eaaa5f4a 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -137,7 +137,7 @@ See [architecture.md](architecture.md) and [#101](https://github.com/fullsend-ai ### Skill -A markdown file (optionally with a `scripts/` directory) that gives an agent context and tool authorizations for a specific task. Skills are not general "agent capabilities" — they are concrete, scoped instruction sets. A skill can declare which tools it is authorized to use; when a user or system approves the skill, they implicitly authorize those tools. Skills are assembled by the [harness](#harness) and are the primary mechanism for encoding agent behavior. +A directory containing a `SKILL.md` file and optional companion files (scripts, sub-agents, assets) that gives an agent context and tool authorizations for a specific task. Skills are not general "agent capabilities" — they are concrete, scoped instruction sets. A skill can declare which tools it is authorized to use; when a user or system approves the skill, they implicitly authorize those tools. Skills are assembled by the [harness](#harness) and are the primary mechanism for encoding agent behavior. See [architecture.md](architecture.md) and [codebase-context.md](problems/codebase-context.md). ### Stage diff --git a/docs/guides/user/customizing-agents.md b/docs/guides/user/customizing-agents.md index dee195c03d..4891f96257 100644 --- a/docs/guides/user/customizing-agents.md +++ b/docs/guides/user/customizing-agents.md @@ -160,10 +160,11 @@ To add a custom skill to the code agent's harness: - skills/my-custom-validation # Your custom skill ``` -3. **Add your custom skill file**: +3. **Add your custom skill directory**: ```bash # Create your custom skill - cat > .fullsend/customized/skills/my-custom-validation.md <<'EOF' + mkdir -p .fullsend/customized/skills/my-custom-validation + cat > .fullsend/customized/skills/my-custom-validation/SKILL.md <<'EOF' # My Custom Validation Skill [Your skill content...] @@ -174,7 +175,7 @@ To add a custom skill to the code agent's harness: - Copies upstream defaults to `harness/`, `skills/`, etc. - Copies your `customized/` files on top, **replacing** any files with matching names - The harness loads `harness/code.yaml` (now your customized version) -- Your skill at `skills/my-custom-validation.md` is available +- Your skill at `skills/my-custom-validation/` is available **Important:** You must maintain the full harness structure. You cannot add just a `skills:` field—the entire YAML file must be present and valid. @@ -212,7 +213,7 @@ Each agent role has its own identity, permissions, and purpose: ### Adding a Custom Skill -Create `.fullsend/customized/skills/my-skill.md` in your config repo: +Create `.fullsend/customized/skills/my-skill/SKILL.md` in your config repo: ```markdown # My Custom Skill @@ -224,7 +225,7 @@ Custom domain knowledge for this organization. ... ``` -The skill will be automatically available to all agents that include `skills/my-skill.md` in their harness configuration. +The skill will be automatically available to all agents that include `skills/my-skill/` in their harness configuration. ### Overriding an Agent Definition @@ -280,7 +281,7 @@ runner_env: REPO_DIR: "${GITHUB_WORKSPACE}/target-repo" ``` -Then create your custom skill at `.fullsend/customized/skills/my-custom-linting.md`. +Then create your custom skill at `.fullsend/customized/skills/my-custom-linting/SKILL.md`. ### Per-Repo Overrides @@ -291,7 +292,7 @@ my-repo/ ├── .fullsend/ │ └── customized/ │ ├── agents/code.md # Repo-specific agent instructions -│ ├── skills/repo-skill.md # Repo-specific skill +│ ├── skills/repo-skill/ # Repo-specific skill (contains SKILL.md) │ └── harness/code.yaml # Repo-specific harness config ``` diff --git a/docs/plans/universal-harness-access-phase1.md b/docs/plans/universal-harness-access-phase1.md index a1053ff1ab..7890810247 100644 --- a/docs/plans/universal-harness-access-phase1.md +++ b/docs/plans/universal-harness-access-phase1.md @@ -103,7 +103,7 @@ PRs 1, 2, 4, and 6 have no dependencies and can be developed/merged in parallel. **Modify `internal/harness/harness.go`:** - Add `AllowedRemoteResources []string` with `yaml:"allowed_remote_resources,omitempty"` to `Harness` struct (after existing fields) - Add `ValidateAllowedRemoteResources(orgAllowlist []string) error` — new method (does NOT modify existing `Validate()` to preserve `Load()` behavior). Validates entries are HTTPS URLs with trailing `/`, validates harness entries are subset of org allowlist. -- Add `ValidateResourceTypes() error` — new method. Rejects URLs in executable fields (PreScript, PostScript, ValidationLoop.Script, HostFiles[].Src, APIServers). Requires integrity hash on URLs in declarative fields (Agent, Policy, Skills). Uses `IsURL`/`ParseIntegrityHash` from PR 1. +- Add `ValidateResourceTypes() error` — new method. Rejects URLs in executable fields (PreScript, PostScript, ValidationLoop.Script, HostFiles[].Src, APIServers). Requires integrity hash on URLs in declarative fields (Agent, Policy, Skills). Validates that skill URLs are from supported forges (GitHub, GitLab) since skills are directories that require forge API access. Uses `IsURL`/`ParseIntegrityHash` from PR 1. - Add `MatchesAllowedPrefix(rawURL string) bool` — URL canonicalization, double-encoding rejection, prefix matching against `AllowedRemoteResources` **Modify `internal/config/config.go`:** @@ -142,14 +142,18 @@ PRs 1, 2, 4, and 6 have no dependencies and can be developed/merged in parallel. **Scope:** New package that orchestrates fetch + cache + validation + audit for URL-referenced resources. This is the core logic. **Create `internal/resolve/resolve.go`:** -- `Dependency` struct: URL, LocalPath (cache path), SHA256, FetchedAt, CacheHit -- `ResolveOpts` struct: WorkspaceRoot, FetchPolicy, TraceID, AuditLogPath +- `Dependency` struct: URL, LocalPath (cache path), SHA256, FetchedAt, CacheHit, Type (`"file"` or `"directory"`) +- `ResolveOpts` struct: WorkspaceRoot, FetchPolicy, TraceID, AuditLogPath, ForgeClient (`forge.Client` for skill directory resolution) - `ResolveHarness(ctx, h *harness.Harness, opts) ([]Dependency, error)`: - Modifies the harness in place, replacing URL fields with local cache paths - - For each declarative field (Agent, Policy, Skills): + - For each declarative field (Agent, Policy): - Local path: return as-is - - URL: extract/require integrity hash → validate against `AllowedRemoteResources` → check cache (with re-verification) → if miss and not offline: `fetch.FetchURL` → verify hash → `CachePut` → `AppendFetchAudit` → return cache content path - - Single-level resolution; transitive deps added in Phase 2 (PR 2 of ADR-0038), security scanning deferred + - URL: extract/require integrity hash → validate against `AllowedRemoteResources` → check cache (with re-verification) → if miss and not offline: `fetch.FetchURL` → verify hash → `CachePut` → `AppendFetchAudit` → return cache `content` path + - For Skills (directory resources): + - Local path: return as-is + - URL: extract/require integrity hash → validate against `AllowedRemoteResources` → use `ParseForgeURL` to extract forge components (owner, repo, path, ref) → check directory cache via `CacheGetDir` (with re-verification) → if miss and not offline: call `ForgeClient.ListDirectoryContents` to discover files, fetch each file with `ForgeClient.GetFileContentAtRef`, reconstruct directory tree, verify tree hash, store via `CachePutDir` → `AppendFetchAudit` → return cache `tree/` path + - Non-forge HTTPS URLs for skills are rejected with error: "skill URLs must use a supported forge (GitHub, GitLab)" + - Single-level resolution; transitive deps added in Phase 2, security scanning deferred **Create `internal/resolve/resolve_test.go`:** - Tests using `httptest.NewTLSServer`: local pass-through, URL fetch+cache, cache hit, hash mismatch, URL not in allowlist, missing hash, offline+miss, offline+hit, security scan failure, mixed harness, audit entries @@ -185,9 +189,9 @@ agent: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/c policy: policies/local-policy.yaml skills: - skills/local-skill - - https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../skills/rust/SKILL.md#sha256=def456... + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust#sha256=... allowed_remote_resources: - - https://raw.githubusercontent.com/fullsend-ai/library/ + - https://github.com/fullsend-ai/library/ ``` --- @@ -195,7 +199,7 @@ allowed_remote_resources: ## Future Phases (high-level) ### Phase 2: Transitive dependency resolution (2-3 PRs) -- Parse `dependencies:` field from SKILL.md YAML frontmatter +- Parse `dependencies:` field from SKILL.md YAML frontmatter (read from resolved skill directory, whether local or cached from forge) - Recursive resolution with cycle detection (visited set), depth limit (10), breadth limit (50) - Relative URL resolution for URL-fetched resources (RFC 3986 base URL semantics) diff --git a/docs/plans/universal-harness-access-phase2.md b/docs/plans/universal-harness-access-phase2.md index 389b2868ae..0dd65af7e1 100644 --- a/docs/plans/universal-harness-access-phase2.md +++ b/docs/plans/universal-harness-access-phase2.md @@ -22,7 +22,7 @@ PRs are strictly sequential. Each is independently reviewable and safe to merge ## PR 1: Skill frontmatter parser — extract `dependencies:` from SKILL.md -**Scope:** New package `internal/skill/` with a parser that extracts YAML frontmatter from SKILL.md content (bytes, not files). Pure functions with no callers. Zero risk to existing behavior. +**Scope:** New package `internal/skill/` with a parser that extracts YAML frontmatter from SKILL.md content (bytes, not files). `ParseFrontmatter` reads SKILL.md content from the resolved skill directory (whether local or cached from forge), not from a raw URL response. Pure functions with no callers. Zero risk to existing behavior. **Rationale for new package:** Skill frontmatter parsing is a distinct concern from harness loading (`internal/harness/`) and resource resolution (`internal/resolve/`). Placing it in its own package avoids circular dependencies: `internal/resolve/` will import `internal/skill/`, but `internal/skill/` imports nothing from the resolve or harness packages. @@ -47,8 +47,8 @@ type SkillMeta struct { // --- // name: rust-conventions // dependencies: -// - ../common/cargo-integration/SKILL.md -// - https://github.com/fullsend-ai/skills/security-baseline/SKILL.md#sha256=abc123... +// - ../common/cargo-integration#sha256=... +// - https://github.com/fullsend-ai/skills/tree/8cd3799.../security-baseline#sha256=... // --- func ParseFrontmatter(content []byte) (*SkillMeta, error) ``` @@ -79,7 +79,7 @@ Test cases: ## PR 2: Recursive resolver with cycle detection and depth/breadth limits -**Scope:** Extends `internal/resolve/resolve.go` to recursively resolve transitive dependencies declared in fetched SKILL.md files. Modifies existing resolver internals but does not change the `ResolveHarness` public signature or behavior for harnesses without transitive dependencies. Harnesses with only local paths or single-level URL references continue to work identically. +**Scope:** Extends `internal/resolve/resolve.go` to recursively resolve transitive dependencies declared in fetched SKILL.md files. Transitive skill dependencies resolve to directories (same as direct skill dependencies) and use forge API for listing and fetching. The resolver reads SKILL.md from the cached directory's `tree/` subdirectory. Modifies existing resolver internals but does not change the `ResolveHarness` public signature or behavior for harnesses without transitive dependencies. Harnesses with only local paths or single-level URL references continue to work identically. ### Changes to `internal/resolve/resolve.go` @@ -181,10 +181,11 @@ func resolveTransitiveDeps(ctx context.Context, parentURL string, content []byte Logic: 1. **Depth check:** If `depth+1 > maxDepth`, return error: `"exceeded maximum dependency depth of %d at %s"`. -2. **Parse frontmatter:** Call `skill.ParseFrontmatter(content)`. If error, return wrapped error. If `nil` or no dependencies, return nil (leaf node). -3. **Resolve each dependency reference:** +2. **Read SKILL.md from cached directory:** For directory skills, read `SKILL.md` from the cached directory's `tree/` subdirectory (e.g., `.fullsend-cache/resources/sha256//tree/SKILL.md`). +3. **Parse frontmatter:** Call `skill.ParseFrontmatter(content)`. If error, return wrapped error. If `nil` or no dependencies, return nil (leaf node). +4. **Resolve each dependency reference:** - If the reference is an absolute URL (`harness.IsURL(ref)`): use as-is. - - If the reference is a relative path: resolve relative to `parentURL` using `ResolveRelativeURL(parentURL, ref)` (defined in `relurl.go` below). + - If the reference is a relative path: resolve relative to `parentURL` using `ResolveRelativeURL(parentURL, ref)` (defined in `relurl.go` below). For directory skills, relative paths resolve to sibling directories, not sibling files (e.g., `../common/formatting` resolves to a sibling directory). - Recursively call `resolveURL(ctx, field, resolvedRef, h, opts, state, depth+1, maxDepth, maxResources)`. **Backward compatibility:** For harnesses with no URL-referenced skills, `resolveTransitiveDeps` is never called. For URL-referenced skills whose content has no `dependencies:` frontmatter, `ParseFrontmatter` returns `nil` and the function returns immediately. Phase 1 behavior is preserved exactly. @@ -196,11 +197,11 @@ Logic: // which the containing resource was fetched. // // Examples: -// ResolveRelativeURL("https://github.com/org/skills/rust/SKILL.md", "../common/SKILL.md") -// → "https://github.com/org/skills/common/SKILL.md" +// ResolveRelativeURL("https://github.com/org/skills/tree/abc123/rust", "../common/formatting") +// → "https://github.com/org/skills/tree/abc123/common/formatting" // -// ResolveRelativeURL("https://github.com/org/skills/rust/SKILL.md", "policies/sandbox.yaml") -// → "https://github.com/org/skills/rust/policies/sandbox.yaml" +// ResolveRelativeURL("https://github.com/org/skills/tree/abc123/rust", "policies/sandbox.yaml") +// → "https://github.com/org/skills/tree/abc123/rust/policies/sandbox.yaml" // // Security: The resolved URL is returned as-is. The caller must validate it // against allowed_remote_resources prefixes (which operates on the normalized @@ -219,13 +220,13 @@ This is deliberately simple — the security boundary is enforced by the existin ### New file: `internal/resolve/relurl_test.go` Test cases: -- **Sibling reference:** `../common/SKILL.md` relative to `.../skills/rust/SKILL.md` resolves to `.../skills/common/SKILL.md`. -- **Child reference:** `policies/sandbox.yaml` relative to `.../skills/rust/SKILL.md` resolves to `.../skills/rust/policies/sandbox.yaml`. -- **Absolute URL reference:** `https://other.com/skill.md` is returned unchanged (no resolution against parent). -- **Path traversal:** `../../../../attacker/evil.md` relative to `.../org/skills/rust/SKILL.md` resolves to `https://github.com/attacker/evil.md` (valid URL — the caller's prefix check rejects it). -- **Multiple `..` segments:** `../../other/sub/SKILL.md` resolves correctly. -- **Fragment preservation:** `../common/SKILL.md#sha256=abc123` resolves with the `#sha256=...` fragment intact. Integrity checking depends on the fragment surviving `url.ResolveReference`. -- **Trailing slash handling:** Parent URL without trailing filename component. +- **Sibling directory reference:** `../common/formatting` relative to `.../tree/abc123/skills/rust` resolves to `.../tree/abc123/skills/common/formatting`. +- **Child reference:** `policies/sandbox.yaml` relative to `.../tree/abc123/skills/rust` resolves to `.../tree/abc123/skills/rust/policies/sandbox.yaml`. +- **Absolute URL reference:** `https://github.com/other-org/skills/tree/abc123/python` is returned unchanged (no resolution against parent). +- **Path traversal:** `../../../../attacker/evil` relative to `.../org/skills/tree/abc123/rust` resolves to `https://github.com/attacker/evil` (valid URL — the caller's prefix check rejects it). +- **Multiple `..` segments:** `../../other/sub/formatting` resolves correctly. +- **Fragment preservation:** `../common/formatting#sha256=` resolves with the `#sha256=...` fragment intact. Integrity checking depends on the fragment surviving `url.ResolveReference`. +- **Trailing slash handling:** Parent URL with and without trailing slash. ### Updates to `internal/resolve/resolve_test.go` @@ -243,7 +244,7 @@ New test cases (in addition to existing Phase 1 tests, which remain unchanged): - **Transitive dependency not in allowlist:** Skill A depends on Skill B at a URL outside `allowed_remote_resources`. Verify error contains "not in allowed_remote_resources". - **Transitive dependency hash mismatch:** Skill A depends on Skill B; Skill B's content doesn't match its declared hash. Verify error contains "integrity check failed". - **Mixed local and transitive:** Harness with local skills and one URL skill that has transitive deps. Verify local skills are untouched, URL skill and its transitive deps are all resolved. -- **Relative URL in dependency:** Skill at `https://example.com/skills/rust/SKILL.md` declares dependency `../common/SKILL.md`. Verify resolved to `https://example.com/skills/common/SKILL.md` and fetched. +- **Relative URL in dependency:** Skill directory at `https://github.com/org/skills/tree/abc123/rust` declares dependency `../common/formatting#sha256=...`. Verify resolved to `https://github.com/org/skills/tree/abc123/common/formatting` and fetched as a directory via forge API. **Depends on:** PR 1 (imports `internal/skill`) @@ -331,30 +332,30 @@ agent: agents/code.md policy: policies/code.yaml skills: - skills/local-skill - - https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../skills/rust-conventions/SKILL.md#sha256=abc123... + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust-conventions#sha256=... allowed_remote_resources: - - https://raw.githubusercontent.com/fullsend-ai/library/ + - https://github.com/fullsend-ai/library/ ``` -Where `rust-conventions/SKILL.md` contains: +Where the `rust-conventions` skill directory's `SKILL.md` contains: ```yaml --- name: rust-conventions dependencies: - - ../cargo-integration/SKILL.md#sha256=def456... - - https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../skills/common/formatting/SKILL.md#sha256=ghi789... + - ../cargo-integration#sha256=... + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/common/formatting#sha256=... --- # Rust Conventions skill content... ``` The resolver will: -1. Fetch `rust-conventions/SKILL.md`, verify hash, cache it. -2. Parse its frontmatter, discover 2 transitive skill dependencies. -3. Resolve `../cargo-integration/SKILL.md` relative to the parent URL. -4. Fetch and cache both transitive dependencies (each with hash verification and allowlist checks). -5. Append all resolved cache paths to `h.Skills`. -6. The sandbox upload loop uploads everything. +1. Fetch `rust-conventions` skill directory via forge API (list files, fetch each), verify tree hash, cache under `tree/`. +2. Read `SKILL.md` from the cached `tree/` subdirectory, parse its frontmatter, discover 2 transitive skill dependencies. +3. Resolve `../cargo-integration` relative to the parent URL (sibling directory). +4. Fetch and cache both transitive skill directories (each via forge API with tree hash verification and allowlist checks). +5. Append all resolved cache `tree/` paths to `h.Skills`. +6. The sandbox upload loop uploads all skill directory trees. --- @@ -379,14 +380,14 @@ Maximum total resources defaults to 50 (configurable via `--max-resources`). Thi ### Relative URL path traversal -When a skill at `https://github.com/org/skills/rust/SKILL.md` declares a dependency `../../../../attacker-org/evil/SKILL.md`, RFC 3986 resolution produces `https://github.com/attacker-org/evil/SKILL.md`. This URL passes the domain allowlist check (same domain), but **fails** the `allowed_remote_resources` prefix check: +When a skill directory at `https://github.com/org/skills/tree/abc123/rust` declares a dependency `../../../../attacker-org/evil`, RFC 3986 resolution produces `https://github.com/attacker-org/evil`. This URL passes the domain allowlist check (same domain), but **fails** the `allowed_remote_resources` prefix check: ```yaml allowed_remote_resources: - https://github.com/org/skills/ ``` -The normalized URL `https://github.com/attacker-org/evil/SKILL.md` does not match prefix `https://github.com/org/skills/`. The fetch is rejected. +The normalized URL `https://github.com/attacker-org/evil` does not match prefix `https://github.com/org/skills/`. The fetch is rejected. **Critical:** The prefix check in `MatchingAllowedPrefix` operates on the **normalized** URL (after RFC 3986 `..` resolution), not the raw relative reference. This is already implemented in Phase 1 and applies to transitive dependencies without modification. @@ -396,11 +397,11 @@ Transitive dependencies must satisfy the same `allowed_remote_resources` constra ### Integrity hash requirement -Transitive dependency references in SKILL.md frontmatter must include `#sha256=...` integrity hashes, just like direct references. The existing `ParseIntegrityHash` validation in `resolveURL` enforces this uniformly. A dependency reference without a hash is rejected with a clear error message. +Transitive dependency references in SKILL.md frontmatter must include `#sha256=...` integrity hashes, just like direct references. For skill directory dependencies, the hash is a tree hash covering the entire directory tree. The existing `ParseIntegrityHash` validation in `resolveURL` enforces this uniformly. A dependency reference without a hash is rejected with a clear error message. ### Aggregate fetch latency -With `MaxResources=50` and the existing 30-second per-fetch timeout, the worst-case wall-clock time for a cold resolution is ~25 minutes (50 sequential fetches, each timing out). In practice, most fetches complete in under a second and dependency graphs are shallow, so this is unlikely. A total wall-clock timeout for the entire `ResolveHarness` call is a reasonable future addition but is not included in Phase 2 — the existing per-fetch timeout and breadth limit provide sufficient protection for now. +With `MaxResources=50` and the existing 30-second per-fetch timeout, the worst-case wall-clock time for a cold resolution is significant (50 resources, each potentially involving multiple forge API calls for directory listing and file fetching). In practice, most fetches complete in under a second, skill directories are small, and dependency graphs are shallow, so this is unlikely. A total wall-clock timeout for the entire `ResolveHarness` call is a reasonable future addition but is not included in Phase 2 — the existing per-fetch timeout and breadth limit provide sufficient protection for now. --- @@ -427,8 +428,8 @@ After PR 3 merges, verify Phase 2 end-to-end: 2. **Lint:** `make lint` passes. 3. **Local-only harness (regression):** Run an existing harness with only local paths — no behavioral change from Phase 1. 4. **Single-level URL harness (regression):** Run a harness with URL-referenced skills that have no `dependencies:` frontmatter — same behavior as Phase 1. -5. **Transitive dependency resolution:** Create a test harness referencing a URL-hosted skill whose SKILL.md frontmatter declares `dependencies:` with another URL-hosted skill. Verify both skills are fetched, cached, and uploaded to the sandbox. -6. **Relative URL resolution:** Create a skill that references a dependency via a relative path (`../common/SKILL.md#sha256=...`). Verify the relative reference is resolved against the parent URL and fetched correctly. +5. **Transitive dependency resolution:** Create a test harness referencing a URL-hosted skill directory whose SKILL.md frontmatter declares `dependencies:` with another URL-hosted skill directory. Verify both skill directories are fetched via forge API, cached as directory trees, and uploaded to the sandbox. +6. **Relative URL resolution:** Create a skill directory that references a dependency via a relative path (`../common/formatting#sha256=...`). Verify the relative reference is resolved to a sibling directory against the parent URL and fetched correctly via forge API. 7. **Cycle detection:** Create two skills that reference each other in their `dependencies:`. Verify the resolver fails with a "circular dependency" error. 8. **Depth limit:** Create a chain of skills deeper than 10 levels. Verify the resolver fails with a "exceeded maximum dependency depth" error. Verify `--max-depth 3` lowers the limit. 9. **Breadth limit:** Create a skill that declares more than 50 transitive dependencies. Verify the resolver fails with a "exceeded maximum resource count" error. Verify `--max-resources 5` lowers the limit. diff --git a/docs/plans/universal-harness-access-phase3.md b/docs/plans/universal-harness-access-phase3.md new file mode 100644 index 0000000000..29dc0bdce9 --- /dev/null +++ b/docs/plans/universal-harness-access-phase3.md @@ -0,0 +1,86 @@ +# Implementation Plan: Phase 3 — Lock Files + +## Context + +Phase 3 adds lock files (`.fullsend/lock.yaml`) that pin all resolved remote dependencies for reproducible harness execution. This was implemented alongside Phases 1 and 2. + +## Implementation + +### Lock file package (`internal/lock/lock.go`) + +- `LockFile` struct: version, generated_at, harnesses map +- `HarnessLock`: source, sha256, resolved_at, dependencies +- `DependencyEntry`: field, url, sha256, type, fetched_at, transitive_deps, files + - `type` is `"file"` for agents/policies or `"directory"` for skills + - `files` lists the manifest of files in directory dependencies (skills only) +- `Load(path)`: reads and validates lock file +- `Save(path, lf)`: atomic write with temp-file-then-rename +- `Lookup(harnessName)`: returns entry or nil +- `IsStale(sourceHash)`: checks if harness has changed +- `LookupDep(url)`: depth-first search through dependency tree + +### Lock file schema + +```yaml +# .fullsend/lock.yaml +version: 1 +generated_at: "2026-05-12T14:30:00Z" +harnesses: + code: + source: harness/code.yaml + sha256: abc123... + resolved_at: "2026-05-12T14:30:00Z" + dependencies: + - field: agent + url: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../agents/code.md + sha256: def456... + type: file + fetched_at: "2026-05-12T14:29:55Z" + - field: skills[0] + url: https://github.com/fullsend-ai/library/tree/8cd3799.../skills/cargo-check + sha256: ... + type: directory + fetched_at: "2026-05-12T14:29:56Z" + files: + - path: SKILL.md + sha256: abc123... + - path: scripts/check.sh + sha256: def456... + transitive_deps: + - field: skills[dep0] + url: https://raw.githubusercontent.com/fullsend-ai/library/8cd3799.../policies/rust-sandbox.yaml + sha256: jkl012... + type: file + fetched_at: "2026-05-12T14:29:57Z" +``` + +### CLI lock command (`internal/cli/lock.go`) + +- `fullsend lock --fullsend-dir ` resolves all deps and writes lock file +- `--update` flag forces re-resolution even if entry is current +- Supports `--offline`, `--max-depth`, `--max-resources` flags + +### Lock file resolution (`internal/cli/lock.go:resolveFromLock`) + +- For each pinned dependency, verifies content exists in local cache +- For `type: "file"` entries: uses `CacheGet` and returns `content` path +- For `type: "directory"` entries: uses `CacheGetDir` and returns `tree/` path +- Applies mutations to harness only after all deps are confirmed in cache +- Falls back to normal network resolution on failure + +### Integration in `fullsend run` (`internal/cli/run.go`) + +- Checks for lock file before resolving +- If lock entry exists and is not stale, uses `resolveFromLock` +- If lock entry is stale, warns user to run `fullsend lock` +- Falls back to normal resolution if lock resolution fails + +## Verification + +1. `fullsend lock code --fullsend-dir .fullsend` generates lock file +2. `fullsend run code` uses lock file when available +3. Modifying harness triggers stale warning +4. Missing cache entries produce clear error messages +5. `--update` forces re-resolution +6. Lock file correctly records `type: "directory"` and `files` manifest for skill dependencies +7. Lock file correctly records `type: "file"` for agent and policy dependencies diff --git a/docs/plans/universal-harness-access-phase4.md b/docs/plans/universal-harness-access-phase4.md new file mode 100644 index 0000000000..3533570825 --- /dev/null +++ b/docs/plans/universal-harness-access-phase4.md @@ -0,0 +1,74 @@ +# Implementation Plan: Phase 4 — Runtime Dependency Loading + +## Context + +Phases 1-3 require all dependencies to be declared statically in the harness YAML. Phase 4 adds runtime dependency loading: agents can discover and fetch additional skills during execution based on the specific problem they encounter. + +## Design + +### Harness schema additions + +```yaml +agent: agents/code.md +skills: + - skills/base + - https://github.com/fullsend-ai/library/tree/abc123/skills/rust#sha256=... +allowed_remote_resources: + - https://github.com/fullsend-ai/library/ +allow_runtime_fetch: true # opt-in (default: false) +max_runtime_fetches: 10 # rate limit per agent run +``` + +### In-sandbox fetch binary + +A `fullsend-fetch-skill` binary available inside the sandbox. When the agent runs it: + +1. Agent calls: `fullsend-fetch-skill https://github.com/fullsend-ai/library/tree/abc123/skills/python-linting#sha256=...` +2. Binary sends request to runner over Unix socket +3. Runner validates URL against `allowed_remote_resources` +4. Runner uses forge API to list and fetch the skill directory (skills are directories, requiring `ListDirectoryContents` and `GetFileContentAtRef`) +5. Runner verifies tree hash (hash covers entire directory tree) +6. Runner stores in cache via `CachePutDir` and uploads directory tree to sandbox +7. Binary returns the sandbox-local skill directory path + +### Security constraints + +- Runtime fetch is opt-in per harness (`allow_runtime_fetch: true`) +- All URLs must match `allowed_remote_resources` prefixes +- Integrity hash required on all URLs (tree hash for skill directories) +- Rate limited: `max_runtime_fetches` (default 10) per agent run +- Skills are directories -- requires forge API access (same as static resolution) +- Non-forge HTTPS URLs are rejected for skills (no HTTP directory listing standard) +- All fetched skills pass security scanning pipeline +- Audit log records all runtime fetches with `fetch_type: "runtime"` + +### Implementation steps + +#### PR 1: Runner-side fetch service +- Unix socket listener in the runner process +- Request/response protocol: URL -> local path or error +- Rate limiting enforcement +- Forge API integration for skill directory fetching (reuses Phase 1 forge client) +- Audit logging with `fetch_type: "runtime"` + +#### PR 2: In-sandbox fetch binary +- `fullsend-fetch-skill` binary compiled and uploaded to sandbox during bootstrap +- Connects to Unix socket passed via environment variable +- Reports errors to stderr, success path to stdout +- Returns the sandbox-local skill directory path (not a single file path) + +#### PR 3: Harness schema and CLI integration +- Add `allow_runtime_fetch` and `max_runtime_fetches` to harness schema +- Validation: reject runtime fetch fields if `allowed_remote_resources` is empty +- Socket setup in sandbox provisioning + +## Verification + +1. Agent can fetch a skill directory at runtime matching allowed prefix +2. Fetch of URL outside allowed prefix is rejected +3. Fetch without hash is rejected +4. Rate limit enforcement: 11th fetch fails +5. `allow_runtime_fetch: false` blocks all runtime fetches +6. Audit log records runtime fetches +7. Fetched skill directory structure is preserved in sandbox (SKILL.md plus companion files) +8. Non-forge HTTPS URLs are rejected with clear error message diff --git a/docs/plans/universal-harness-access.md b/docs/plans/universal-harness-access.md index 96445aafb8..669bcf7668 100644 --- a/docs/plans/universal-harness-access.md +++ b/docs/plans/universal-harness-access.md @@ -44,7 +44,7 @@ Resolution logic (`internal/harness/harness.go`): - All paths must resolve within the `.fullsend` directory tree - No network fetches; all resources must exist locally -Skills are directories with a `SKILL.md` file. Policies are OpenShell YAML files. Agent definitions are Markdown files with YAML frontmatter. +Skills are directories containing `SKILL.md` plus optional companion files (`scripts/`, `sub-agents/`, `assets/`). The entire directory tree is uploaded to the sandbox. When referenced via URL, skills require forge API access (e.g., GitHub Contents API) to discover and fetch all files in the directory. Policies are OpenShell YAML files. Agent definitions are Markdown files with YAML frontmatter. ## Proposed Design @@ -63,7 +63,7 @@ Examples (note: `#sha256=...` hash fragments omitted for brevity; all remote URL agent: https://github.com/fullsend-ai/library/agents/code.md policy: policies/local-code-policy.yaml # local override skills: - - https://github.com/fullsend-ai/skills/rust-conventions/SKILL.md + - https://github.com/fullsend-ai/skills/tree/8cd3799.../rust-conventions#sha256=... - skills/org-specific-skill # local skill pre_script: scripts/pre-code.sh # scripts must be local (security) ``` @@ -74,7 +74,7 @@ pre_script: scripts/pre-code.sh # scripts must be local (security) |---------------|----------------|-----------| | Agent definition (`.md`) | ✅ Yes | Declarative; validated by schema | | Policy (`.yaml`) | ✅ Yes | Declarative; validated by schema | -| Skill (`SKILL.md`) | ✅ Yes | Declarative; scanned for injection | +| Skill (directory) | ✅ Yes (forge only) | Directory uploaded as tree; requires forge API | | Schema (`.json`) | ✅ Yes | Declarative; validated before use | | Pre/post scripts (`.sh`) | ❌ No | Executable on host; must be local | | Host files (certs, env) | ❌ No | Configuration; must be local | @@ -98,7 +98,7 @@ When a harness or resource is fetched from a URL, relative paths within that res **Path traversal protection:** URL-based relative paths follow RFC 3986 semantics, including `../` traversal. Example: -A skill at `https://github.com/fullsend-ai/library/skills/rust/SKILL.md` referencing: +A skill directory at `https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust` whose `SKILL.md` references: ```yaml policy: ../../../../attacker-org/evil-repo/policy.yaml @@ -121,16 +121,16 @@ The normalized URL `https://github.com/attacker-org/evil-repo/policy.yaml` does agent: agents/code.md # → https://github.com/fullsend-ai/harnesses/agents/code.md policy: ../policies/code-policy.yaml # → https://github.com/fullsend-ai/policies/code-policy.yaml skills: - - skills/rust-linting/SKILL.md # → https://github.com/fullsend-ai/harnesses/skills/rust-linting/SKILL.md + - skills/rust-linting # → local skill directory (skills/rust-linting/) ``` **Example 2: Skill fetched from URL** ```yaml -# Skill at: https://github.com/fullsend-ai/skills/rust-conventions/SKILL.md +# Skill directory at: https://github.com/fullsend-ai/skills/tree/8cd3799.../rust-conventions +# SKILL.md within that directory contains: --- dependencies: - - ../common/cargo-integration/SKILL.md # → https://github.com/fullsend-ai/skills/common/cargo-integration/SKILL.md -policy: policies/rust-sandbox.yaml # → https://github.com/fullsend-ai/skills/rust-conventions/policies/rust-sandbox.yaml + - ../common/cargo-integration#sha256=... # → sibling directory https://github.com/fullsend-ai/skills/tree/8cd3799.../common/cargo-integration --- ``` @@ -150,12 +150,12 @@ policy: policies/rust-sandbox.yaml # → https://github.com/fullsend-ai/s A URL-referenced skill can itself reference other resources: ```yaml -# https://github.com/fullsend-ai/skills/rust-conventions/SKILL.md +# SKILL.md inside directory https://github.com/fullsend-ai/skills/tree/8cd3799.../rust-conventions --- name: rust-conventions -policy: https://github.com/fullsend-ai/policies/rust-sandbox.yaml dependencies: - - https://github.com/fullsend-ai/skills/cargo-integration/SKILL.md + - ../cargo-integration#sha256=... + - https://github.com/fullsend-ai/skills/tree/8cd3799.../common/formatting#sha256=... --- # skill content ``` @@ -174,9 +174,15 @@ Fetched resources are cached in the repository's workspace using content address ``` .fullsend-cache/resources/ sha256/ - abc123.../ - metadata.json # {url, fetch_time, content_type, headers} - content # the actual fetched content + abc123.../ # single-file resource (agent, policy) + metadata.json # {url, fetch_time, content_type, sha256} + content # the actual fetched content + def456.../ # directory resource (skill) + metadata.json # {url, fetch_time, type: "directory", sha256} + tree/ + SKILL.md + scripts/ + helper.sh ``` **Cache location:** The cache is stored in the repository's workspace (`.fullsend-cache/` directory). In ephemeral CI/CD environments like GitHub Actions, the cache is rebuilt on each run unless the platform's native caching mechanisms (e.g., GitHub Actions cache, GitLab CI cache) are used to persist it across workflow runs. @@ -288,9 +294,9 @@ harness/code.yaml │ └─ (no dependencies) ├─ skills/code-implementation (local) │ └─ (no dependencies) - └─ https://github.com/fullsend-ai/skills/rust-conventions/SKILL.md + └─ https://github.com/fullsend-ai/skills/tree/8cd3799.../rust-conventions ├─ https://github.com/fullsend-ai/policies/rust-sandbox.yaml - └─ https://github.com/fullsend-ai/skills/cargo-integration/SKILL.md + └─ https://github.com/fullsend-ai/skills/tree/8cd3799.../cargo-integration └─ (no dependencies) ``` @@ -299,7 +305,9 @@ Resolution algorithm: 1. Parse the harness YAML to extract all references 2. For each reference: - If local path, validate it exists - - If URL, fetch and cache + - If URL for a single-file resource (agent, policy): fetch via HTTPS, cache as `content` file + - If URL for a directory resource (skill): use forge API (`ListDirectoryContents`, `GetFileContentAtRef`) to discover and fetch all files, cache as `tree/` directory, verify tree hash + - Non-forge HTTPS URLs for skills are rejected (HTTP has no standard directory listing) 3. Parse fetched resources to extract their references 4. Repeat step 2 for new references (depth-first traversal) 5. Detect cycles (if skill A references skill B, and skill B references skill A, reject) @@ -351,7 +359,7 @@ allowed_remote_resources: - https://github.com/fullsend-ai/library/ - https://github.com/myorg/agent-resources/ skills: - - https://github.com/fullsend-ai/library/skills/rust-conventions/SKILL.md + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust-conventions#sha256=... ``` The runner enforces: @@ -372,7 +380,7 @@ allow_runtime_fetch: true max_runtime_fetches: 10 ``` -During execution, the agent can fetch `https://github.com/fullsend-ai/library/skills/python-linting/SKILL.md` because it matches an allowed prefix. The runner validates and caches it. +During execution, the agent can fetch `https://github.com/fullsend-ai/library/tree/8cd3799.../skills/python-linting#sha256=...` because it matches an allowed prefix. The runner uses the forge API to list and fetch the skill directory, validates the tree hash, and caches it. **Audit:** All fetches (static and runtime) are logged: @@ -380,7 +388,7 @@ During execution, the agent can fetch `https://github.com/fullsend-ai/library/sk { "trace_id": "abc123", "fetch_time": "2026-05-07T12:34:56Z", - "url": "https://github.com/fullsend-ai/library/skills/rust-conventions/SKILL.md", + "url": "https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust-conventions", "sha256": "def456...", "fetch_type": "static", // or "runtime" "allowed_by": "allowed_remote_resources[0]" @@ -494,7 +502,7 @@ allowed_remote_resources: - https://github.com/fullsend-ai/library/ - https://github.com/myorg/agent-resources/ skills: - - https://github.com/fullsend-ai/library/skills/rust-conventions/SKILL.md + - https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust-conventions#sha256=... ``` **File:** `internal/harness/harness.go` @@ -916,8 +924,118 @@ func CachePut(workspaceRoot, url string, content []byte) error { return nil } + +// CachePutDir stores a directory tree (skill) in the cache. +// The directory is stored under tree/ within the cache entry. +func CachePutDir(workspaceRoot, url string, files map[string][]byte) error { + treeHash := ComputeTreeHash(files) + dir := CachePath(workspaceRoot, treeHash) + + if err := os.MkdirAll(filepath.Join(dir, "tree"), 0700); err != nil { + return err + } + + for relPath, content := range files { + fullPath := filepath.Join(dir, "tree", relPath) + if err := os.MkdirAll(filepath.Dir(fullPath), 0700); err != nil { + return err + } + if err := os.WriteFile(fullPath, content, 0600); err != nil { + return err + } + } + + entry := CacheEntry{ + URL: url, + FetchTime: time.Now(), + SHA256: treeHash, + Type: "directory", + } + metaData, _ := json.MarshalIndent(entry, "", " ") + return os.WriteFile(filepath.Join(dir, "metadata.json"), metaData, 0600) +} + +// CacheGetDir retrieves a cached directory tree by hash. Returns the tree/ path. +func CacheGetDir(workspaceRoot, hash string) (string, *CacheEntry, error) { + dir := CachePath(workspaceRoot, hash) + treePath := filepath.Join(dir, "tree") + metaPath := filepath.Join(dir, "metadata.json") + + if _, err := os.Stat(metaPath); os.IsNotExist(err) { + return "", nil, nil + } + if _, err := os.Stat(treePath); os.IsNotExist(err) { + return "", nil, nil + } + + metaData, err := os.ReadFile(metaPath) + if err != nil { + return "", nil, err + } + var entry CacheEntry + if err := json.Unmarshal(metaData, &entry); err != nil { + return "", nil, err + } + + return treePath, &entry, nil +} + +// ComputeTreeHash computes a deterministic hash over a directory tree. +// Files are sorted by path, then each file's path and SHA256 are hashed. +func ComputeTreeHash(files map[string][]byte) string { + // Sort file paths for deterministic ordering + paths := make([]string, 0, len(files)) + for p := range files { + paths = append(paths, p) + } + sort.Strings(paths) + + h := sha256.New() + for _, p := range paths { + fmt.Fprintf(h, "%s:%s\n", p, ComputeSHA256(files[p])) + } + return hex.EncodeToString(h.Sum(nil)) +} +``` + +### 4a. Forge Interface Extension for Skill Directories + +**File:** `internal/forge/forge.go` (additions to the forge.Client interface) + +Skills are directories, not single files. To fetch a skill from a forge URL, the resolver must list the directory contents and fetch each file. This requires forge API support. + +```go +// ListDirectoryContents returns the list of files in a directory at a given ref. +// For GitHub, this uses the Trees API or Contents API. +ListDirectoryContents(ctx context.Context, owner, repo, path, ref string) ([]FileEntry, error) + +// GetFileContentAtRef fetches a single file's content at a specific ref. +// For GitHub, this uses the Contents API with a ref parameter. +GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) ([]byte, error) ``` +```go +type FileEntry struct { + Path string // relative path within the directory + SHA string // git blob SHA + Size int64 +} +``` + +**File:** `internal/forge/github/directory.go` (new) + +GitHub implementation using the Contents API (`GET /repos/{owner}/{repo}/contents/{path}?ref={ref}`) for directory listing and file retrieval. + +**File:** `internal/fetch/forgeurl.go` (new) + +```go +// ParseForgeURL extracts forge components (owner, repo, path, ref) from a +// directory URL like https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust +func ParseForgeURL(rawURL string) (host, owner, repo, path, ref string, err error) +``` + +**Why non-forge HTTPS URLs are rejected for skills:** HTTP has no standard mechanism for listing directory contents. A URL like `https://example.com/skills/rust/` might serve an HTML index page, but there is no reliable way to discover all files in the directory. Forge APIs (GitHub Contents API, GitLab Repository Files API) provide structured directory listings. Skills from non-forge URLs are rejected at validation time with a clear error message. + ### 5. Dependency Resolver **File:** `internal/resolve/resolve.go` (new) @@ -1281,12 +1399,19 @@ harnesses: path: policies/local-code-policy.yaml # local paths recorded for completeness sha256: "789abc..." skills: - - url: https://github.com/fullsend-ai/library/skills/rust/SKILL.md - sha256: "123def..." + - url: https://github.com/fullsend-ai/library/tree/8cd3799.../skills/rust + sha256: "..." + type: directory resolved_at: "2026-05-12T14:29:56Z" + files: + - path: SKILL.md + sha256: "abc123..." + - path: scripts/check.sh + sha256: "def456..." transitive_deps: - url: https://github.com/prodsec/agent-skills/security-baseline.md sha256: "456789..." + type: file resolved_at: "2026-05-12T14:29:57Z" ``` @@ -1386,6 +1511,8 @@ If a skill references `policy: rust-sandbox@v2` (a name+version, not a URL), how - **[Implementation Plan — Phase 1](universal-harness-access-phase1.md)** — Phased PR breakdown for Phase 1 (MVP) - **[Implementation Plan — Phase 2](universal-harness-access-phase2.md)** — Phased PR breakdown for Phase 2 (transitive dependency resolution) +- **[Implementation Plan — Phase 3](universal-harness-access-phase3.md)** — Lock files for reproducible harness execution +- **[Implementation Plan — Phase 4](universal-harness-access-phase4.md)** — Runtime dependency loading - **[ADR-0024: Harness Definitions](../ADRs/0024-harness-definitions.md)** — Current harness schema and resolution logic - **[ADR-0022: Output Schema Enforcement](../ADRs/0022-harness-level-output-schema-enforcement.md)** — Security validation of agent output - **[ADR-0017: Credential Isolation](../ADRs/0017-credential-isolation-for-sandboxed-agents.md)** — Sandbox security model diff --git a/internal/cli/lock.go b/internal/cli/lock.go index 045c4a770c..fe706bc536 100644 --- a/internal/cli/lock.go +++ b/internal/cli/lock.go @@ -12,6 +12,8 @@ import ( "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/lock" "github.com/fullsend-ai/fullsend/internal/resolve" @@ -132,12 +134,27 @@ func runLock(ctx context.Context, agentName, fullsendDir string, update bool, rF policy := fetch.DefaultPolicy policy.Offline = rFlags.offline + var forgeClient forge.Client + if h.HasURLSkills() { + if rFlags.forgeClient != nil { + forgeClient = rFlags.forgeClient + } else { + token, err := resolveToken() + if err != nil { + printer.StepFail("Skill URLs require a GitHub token (set GH_TOKEN, GITHUB_TOKEN, or run 'gh auth login')") + return fmt.Errorf("skill URLs require a GitHub token: %w", err) + } + forgeClient = gh.New(token) + } + } + deps, err := resolve.ResolveHarness(ctx, h, resolve.ResolveOpts{ WorkspaceRoot: absFullsendDir, FetchPolicy: policy, AuditLogPath: filepath.Join(absFullsendDir, ".fullsend-cache", "fetch-audit.jsonl"), MaxDepth: rFlags.maxDepth, MaxResources: rFlags.maxResources, + ForgeClient: forgeClient, }) if err != nil { printer.StepFail("Resolution failed") @@ -150,12 +167,29 @@ func runLock(ctx context.Context, agentName, fullsendDir string, update bool, rF now := time.Now().UTC() lockDeps := make([]lock.DependencyEntry, 0, len(deps)) for _, dep := range deps { - lockDeps = append(lockDeps, lock.DependencyEntry{ + entry := lock.DependencyEntry{ Field: dep.Field, URL: dep.URL, SHA256: dep.SHA256, + Type: dep.Type, FetchedAt: dep.FetchedAt, - }) + } + if dep.Type == "directory" { + _, dirEntry, err := fetch.CacheGetDir(absFullsendDir, dep.SHA256) + if err != nil { + return fmt.Errorf("reading cached directory for %s: %w", dep.Field, err) + } + if dirEntry == nil { + return fmt.Errorf("directory %s (%s) was just resolved but is missing from cache", dep.Field, dep.URL) + } + for _, f := range dirEntry.Files { + entry.Files = append(entry.Files, lock.FileEntry{ + Path: f.Path, + SHA256: f.SHA256, + }) + } + } + lockDeps = append(lockDeps, entry) } harnessLock := lock.HarnessLock{ @@ -207,20 +241,36 @@ func resolveFromLock(h *harness.Harness, entry *lock.HarnessLock, workspaceRoot var deps []resolve.Dependency for _, lockDep := range entry.Dependencies { - content, _, err := fetch.CacheGet(workspaceRoot, lockDep.SHA256) - if err != nil { - return nil, fmt.Errorf("cache integrity check failed for %s: %w", lockDep.Field, err) - } - if content == nil { - return nil, fmt.Errorf("dependency %s (%s) is pinned in lock file with sha256=%s but not in cache — run 'fullsend lock' to re-fetch", lockDep.Field, lockDep.URL, lockDep.SHA256) - } + var localPath string - cachePath, err := fetch.CachePath(workspaceRoot, lockDep.SHA256) - if err != nil { - return nil, fmt.Errorf("computing cache path for %s: %w", lockDep.Field, err) + if lockDep.Type == "directory" { + treePath, _, err := fetch.CacheGetDir(workspaceRoot, lockDep.SHA256) + if err != nil { + return nil, fmt.Errorf("dir cache integrity check failed for %s: %w", lockDep.Field, err) + } + if treePath == "" { + return nil, fmt.Errorf("dependency %s (%s) is pinned in lock file with sha256=%s but not in cache — run 'fullsend lock' to re-fetch", lockDep.Field, lockDep.URL, lockDep.SHA256) + } + localPath = treePath + } else { + content, _, err := fetch.CacheGet(workspaceRoot, lockDep.SHA256) + if err != nil { + return nil, fmt.Errorf("cache integrity check failed for %s: %w", lockDep.Field, err) + } + if content == nil { + return nil, fmt.Errorf("dependency %s (%s) is pinned in lock file with sha256=%s but not in cache — run 'fullsend lock' to re-fetch", lockDep.Field, lockDep.URL, lockDep.SHA256) + } + cachePath, err := fetch.CachePath(workspaceRoot, lockDep.SHA256) + if err != nil { + return nil, fmt.Errorf("computing cache path for %s: %w", lockDep.Field, err) + } + localPath = filepath.Join(cachePath, "content") } - localPath := filepath.Join(cachePath, "content") + depType := lockDep.Type + if depType == "" { + depType = "file" + } mutations = append(mutations, mutation{field: lockDep.Field, localPath: localPath}) deps = append(deps, resolve.Dependency{ Field: lockDep.Field, @@ -229,6 +279,7 @@ func resolveFromLock(h *harness.Harness, entry *lock.HarnessLock, workspaceRoot SHA256: lockDep.SHA256, FetchedAt: lockDep.FetchedAt, CacheHit: true, + Type: depType, }) } diff --git a/internal/cli/lock_test.go b/internal/cli/lock_test.go index 4172eac428..94eaefdbc0 100644 --- a/internal/cli/lock_test.go +++ b/internal/cli/lock_test.go @@ -15,8 +15,10 @@ import ( "github.com/stretchr/testify/require" "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/lock" + "github.com/fullsend-ai/fullsend/internal/resolve" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -40,18 +42,17 @@ func newLockTestServer(t *testing.T, contents map[string][]byte) (*httptest.Serv return srv, fetch.NewTestPolicy(tlsCfg, []string{hostname}, []string{port}) } -func setupLockTestDir(t *testing.T, srv *httptest.Server, agentHash, skillHash string) string { +func setupLockTestDir(t *testing.T, srv *httptest.Server, agentHash, policyHash string) string { t.Helper() dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" -skills: - - "%s/skills/rust/SKILL.md#sha256=%s" +policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" -`, srv.URL, agentHash, srv.URL, skillHash, srv.URL) +`, srv.URL, agentHash, srv.URL, policyHash, srv.URL) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), @@ -74,15 +75,15 @@ allowed_remote_resources: func TestRunLock_GeneratesLockFile(t *testing.T) { agentContent := []byte("You are a coding agent.") agentHash := fetch.ComputeSHA256(agentContent) - skillContent := []byte("# Rust skill\nRust development skill.") - skillHash := fetch.ComputeSHA256(skillContent) + policyContent := []byte("sandbox: strict") + policyHash := fetch.ComputeSHA256(policyContent) srv, policy := newLockTestServer(t, map[string][]byte{ - "/agents/code.md": agentContent, - "/skills/rust/SKILL.md": skillContent, + "/agents/code.md": agentContent, + "/policies/sandbox.yaml": policyContent, }) - dir := setupLockTestDir(t, srv, agentHash, skillHash) + dir := setupLockTestDir(t, srv, agentHash, policyHash) fetch.DefaultPolicy = policy defer func() { fetch.DefaultPolicy = fetch.FetchPolicy{} }() @@ -106,9 +107,195 @@ func TestRunLock_GeneratesLockFile(t *testing.T) { assert.Equal(t, fmt.Sprintf("%s/agents/code.md", srv.URL), entry.Dependencies[0].URL) assert.Equal(t, agentHash, entry.Dependencies[0].SHA256) - assert.Equal(t, "skills[0]", entry.Dependencies[1].Field) - assert.Equal(t, fmt.Sprintf("%s/skills/rust/SKILL.md", srv.URL), entry.Dependencies[1].URL) - assert.Equal(t, skillHash, entry.Dependencies[1].SHA256) + assert.Equal(t, "policy", entry.Dependencies[1].Field) + assert.Equal(t, fmt.Sprintf("%s/policies/sandbox.yaml", srv.URL), entry.Dependencies[1].URL) + assert.Equal(t, policyHash, entry.Dependencies[1].SHA256) +} + +func TestRunLock_SkillDirectoryType(t *testing.T) { + agentContent := []byte("You are a coding agent.") + agentHash := fetch.ComputeSHA256(agentContent) + + skillMD := []byte("# Test skill\nA test skill.") + helperSh := []byte("#!/bin/bash\necho hello") + skillFiles := map[string][]byte{ + "SKILL.md": skillMD, + "scripts/helper.sh": helperSh, + } + treeHash := fetch.ComputeTreeHash(skillFiles) + + srv, policy := newLockTestServer(t, map[string][]byte{ + "/agents/code.md": agentContent, + }) + + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + + skillURL := fmt.Sprintf("https://github.com/test-org/test-repo/tree/main/skills/test#sha256=%s", treeHash) + harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +skills: + - "%s" +allowed_remote_resources: + - "%s/" + - "https://github.com/test-org/" +`, srv.URL, agentHash, skillURL, srv.URL) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte(harnessContent), + 0o644, + )) + + orgConfig := fmt.Sprintf(`allowed_remote_resources: + - "%s/" + - "https://github.com/test-org/" +`, srv.URL) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config.yaml"), + []byte(orgConfig), + 0o644, + )) + + fakeClient := forge.NewFakeClient() + fakeClient.DirContents["test-org/test-repo/skills/test@main"] = []forge.DirectoryEntry{ + {Path: "SKILL.md", Type: "file", Size: len(skillMD)}, + {Path: "scripts/helper.sh", Type: "file", Size: len(helperSh)}, + } + fakeClient.FileContentsRef["test-org/test-repo/skills/test/SKILL.md@main"] = skillMD + fakeClient.FileContentsRef["test-org/test-repo/skills/test/scripts/helper.sh@main"] = helperSh + + fetch.DefaultPolicy = policy + defer func() { fetch.DefaultPolicy = fetch.FetchPolicy{} }() + + printer := ui.New(os.Stdout) + err := runLock(context.Background(), "code", dir, false, resolveFlags{forgeClient: fakeClient}, printer) + require.NoError(t, err) + + lockPath := filepath.Join(dir, "lock.yaml") + lf, err := lock.Load(lockPath) + require.NoError(t, err) + require.NotNil(t, lf) + + entry := lf.Lookup("code") + require.NotNil(t, entry) + + var skillDep *lock.DependencyEntry + for i := range entry.Dependencies { + if strings.HasPrefix(entry.Dependencies[i].Field, "skills[") { + skillDep = &entry.Dependencies[i] + break + } + } + require.NotNil(t, skillDep, "should have a skill dependency") + assert.Equal(t, "directory", skillDep.Type) + assert.Equal(t, treeHash, skillDep.SHA256) + require.Len(t, skillDep.Files, 2) + + fileNames := make([]string, len(skillDep.Files)) + for i, f := range skillDep.Files { + fileNames[i] = f.Path + } + assert.Contains(t, fileNames, "SKILL.md") + assert.Contains(t, fileNames, "scripts/helper.sh") +} + +func TestRunLock_SkillDirectoryRoundTrip(t *testing.T) { + agentContent := []byte("You are a coding agent.") + agentHash := fetch.ComputeSHA256(agentContent) + + skillMD := []byte("# Test skill\nA test skill.") + helperSh := []byte("#!/bin/bash\necho hello") + skillFiles := map[string][]byte{ + "SKILL.md": skillMD, + "scripts/helper.sh": helperSh, + } + treeHash := fetch.ComputeTreeHash(skillFiles) + + srv, policy := newLockTestServer(t, map[string][]byte{ + "/agents/code.md": agentContent, + }) + + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + + skillURL := fmt.Sprintf("https://github.com/test-org/test-repo/tree/main/skills/test#sha256=%s", treeHash) + harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +skills: + - "%s" +allowed_remote_resources: + - "%s/" + - "https://github.com/test-org/" +`, srv.URL, agentHash, skillURL, srv.URL) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte(harnessContent), + 0o644, + )) + + orgConfig := fmt.Sprintf(`allowed_remote_resources: + - "%s/" + - "https://github.com/test-org/" +`, srv.URL) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config.yaml"), + []byte(orgConfig), + 0o644, + )) + + fakeClient := forge.NewFakeClient() + fakeClient.DirContents["test-org/test-repo/skills/test@main"] = []forge.DirectoryEntry{ + {Path: "SKILL.md", Type: "file", Size: len(skillMD)}, + {Path: "scripts/helper.sh", Type: "file", Size: len(helperSh)}, + } + fakeClient.FileContentsRef["test-org/test-repo/skills/test/SKILL.md@main"] = skillMD + fakeClient.FileContentsRef["test-org/test-repo/skills/test/scripts/helper.sh@main"] = helperSh + + fetch.DefaultPolicy = policy + defer func() { fetch.DefaultPolicy = fetch.FetchPolicy{} }() + + printer := ui.New(os.Stdout) + + // Step 1: Generate the lock file. + err := runLock(context.Background(), "code", dir, false, resolveFlags{forgeClient: fakeClient}, printer) + require.NoError(t, err) + + lockPath := filepath.Join(dir, "lock.yaml") + lf, err := lock.Load(lockPath) + require.NoError(t, err) + entry := lf.Lookup("code") + require.NotNil(t, entry) + + // Step 2: Reload the harness (runLock mutated it) and resolve from lock. + h2, err := harness.Load(filepath.Join(dir, "harness", "code.yaml")) + require.NoError(t, err) + require.NoError(t, h2.ResolveRelativeTo(dir)) + + deps, err := resolveFromLock(h2, entry, dir, printer) + require.NoError(t, err) + + // Verify the round-trip: agent resolved as file, skill resolved as directory. + require.Len(t, deps, 2) + + var agentDep, skillDep *resolve.Dependency + for i := range deps { + switch { + case deps[i].Field == "agent": + agentDep = &deps[i] + case strings.HasPrefix(deps[i].Field, "skills["): + skillDep = &deps[i] + } + } + require.NotNil(t, agentDep, "should have agent dependency") + require.NotNil(t, skillDep, "should have skill dependency") + + assert.Equal(t, "file", agentDep.Type) + assert.True(t, agentDep.CacheHit) + + assert.Equal(t, "directory", skillDep.Type) + assert.Equal(t, treeHash, skillDep.SHA256) + assert.True(t, skillDep.CacheHit) + assert.True(t, strings.HasSuffix(h2.Skills[0], "/tree"), "skill path should end with /tree, got %s", h2.Skills[0]) } func TestRunLock_NoURLReferences(t *testing.T) { @@ -136,15 +323,15 @@ skills: func TestRunLock_AlreadyUpToDate(t *testing.T) { agentContent := []byte("You are a coding agent.") agentHash := fetch.ComputeSHA256(agentContent) - skillContent := []byte("# Rust skill") - skillHash := fetch.ComputeSHA256(skillContent) + policyContent := []byte("sandbox: strict") + policyHash := fetch.ComputeSHA256(policyContent) srv, policy := newLockTestServer(t, map[string][]byte{ - "/agents/code.md": agentContent, - "/skills/rust/SKILL.md": skillContent, + "/agents/code.md": agentContent, + "/policies/sandbox.yaml": policyContent, }) - dir := setupLockTestDir(t, srv, agentHash, skillHash) + dir := setupLockTestDir(t, srv, agentHash, policyHash) fetch.DefaultPolicy = policy defer func() { fetch.DefaultPolicy = fetch.FetchPolicy{} }() @@ -166,15 +353,15 @@ func TestRunLock_AlreadyUpToDate(t *testing.T) { func TestRunLock_UpdateForceReResolve(t *testing.T) { agentContent := []byte("You are a coding agent.") agentHash := fetch.ComputeSHA256(agentContent) - skillContent := []byte("# Rust skill") - skillHash := fetch.ComputeSHA256(skillContent) + policyContent := []byte("sandbox: strict") + policyHash := fetch.ComputeSHA256(policyContent) srv, policy := newLockTestServer(t, map[string][]byte{ - "/agents/code.md": agentContent, - "/skills/rust/SKILL.md": skillContent, + "/agents/code.md": agentContent, + "/policies/sandbox.yaml": policyContent, }) - dir := setupLockTestDir(t, srv, agentHash, skillHash) + dir := setupLockTestDir(t, srv, agentHash, policyHash) fetch.DefaultPolicy = policy defer func() { fetch.DefaultPolicy = fetch.FetchPolicy{} }() @@ -345,6 +532,80 @@ func TestResolveFromLock_DiamondDependency(t *testing.T) { assert.True(t, strings.HasSuffix(h.Skills[0], "/content")) } +func TestResolveFromLock_DirectoryType(t *testing.T) { + skillMD := []byte("# Skill\nA test skill.") + helperSh := []byte("#!/bin/bash\necho hello") + skillFiles := map[string][]byte{ + "SKILL.md": skillMD, + "scripts/helper.sh": helperSh, + } + treeHash := fetch.ComputeTreeHash(skillFiles) + + root := t.TempDir() + _, err := fetch.CachePutDir(root, "https://github.com/org/repo/tree/main/skills/test", skillFiles) + require.NoError(t, err) + + entry := &lock.HarnessLock{ + Dependencies: []lock.DependencyEntry{ + { + Field: "skills[0]", + URL: "https://github.com/org/repo/tree/main/skills/test", + SHA256: treeHash, + Type: "directory", + Files: []lock.FileEntry{ + {Path: "SKILL.md", SHA256: fetch.ComputeSHA256(skillMD)}, + {Path: "scripts/helper.sh", SHA256: fetch.ComputeSHA256(helperSh)}, + }, + }, + }, + } + + h := &harness.Harness{ + Agent: "agents/code.md", + Skills: []string{"https://github.com/org/repo/tree/main/skills/test#sha256=" + treeHash}, + } + + printer := ui.New(os.Stdout) + deps, err := resolveFromLock(h, entry, root, printer) + require.NoError(t, err) + require.Len(t, deps, 1) + + assert.Equal(t, "directory", deps[0].Type) + assert.Equal(t, treeHash, deps[0].SHA256) + assert.True(t, deps[0].CacheHit) + assert.True(t, strings.HasSuffix(h.Skills[0], "/tree")) +} + +func TestResolveFromLock_EmptyTypeDefaultsToFile(t *testing.T) { + content := []byte("skill content") + hash := fetch.ComputeSHA256(content) + + root := t.TempDir() + require.NoError(t, fetch.CachePut(root, "https://example.com/skills/a", content)) + + entry := &lock.HarnessLock{ + Dependencies: []lock.DependencyEntry{ + { + Field: "skills[0]", + URL: "https://example.com/skills/a", + SHA256: hash, + Type: "", // pre-directory-model lock file + }, + }, + } + + h := &harness.Harness{ + Agent: "agents/code.md", + Skills: []string{"https://example.com/skills/a#sha256=" + hash}, + } + + printer := ui.New(os.Stdout) + deps, err := resolveFromLock(h, entry, root, printer) + require.NoError(t, err) + require.Len(t, deps, 1) + assert.Equal(t, "file", deps[0].Type, "empty Type should default to file for backward compatibility") +} + func TestResolveFromLock_TransitivePolicySkipped(t *testing.T) { policyContent := []byte("transitive policy content") policyHash := fetch.ComputeSHA256(policyContent) diff --git a/internal/cli/run.go b/internal/cli/run.go index 1dcce94897..6cba7a97f2 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -21,6 +21,7 @@ import ( "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/envfile" "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" gh "github.com/fullsend-ai/fullsend/internal/forge/github" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/lock" @@ -53,6 +54,7 @@ type resolveFlags struct { offline bool maxDepth int maxResources int + forgeClient forge.Client // injected by tests; nil means construct from env } // statusOpts holds the optional status notification parameters for a run. @@ -209,6 +211,20 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep policy := fetch.DefaultPolicy policy.Offline = rFlags.offline + var forgeClient forge.Client + if h.HasURLSkills() { + if rFlags.forgeClient != nil { + forgeClient = rFlags.forgeClient + } else { + token, tokenErr := resolveToken() + if tokenErr != nil { + printer.StepFail("Skill URLs require a GitHub token (set GH_TOKEN, GITHUB_TOKEN, or run 'gh auth login')") + return fmt.Errorf("skill URLs require a GitHub token: %w", tokenErr) + } + forgeClient = gh.New(token) + } + } + var resolveErr error deps, resolveErr = resolve.ResolveHarness(ctx, h, resolve.ResolveOpts{ WorkspaceRoot: absFullsendDir, @@ -216,6 +232,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep AuditLogPath: filepath.Join(absFullsendDir, ".fullsend-cache", "fetch-audit.jsonl"), MaxDepth: rFlags.maxDepth, MaxResources: rFlags.maxResources, + ForgeClient: forgeClient, }) if resolveErr != nil { printer.StepFail("Remote resource resolution failed") diff --git a/internal/fetch/cache.go b/internal/fetch/cache.go index 9d332e3c7c..7f7ca9a355 100644 --- a/internal/fetch/cache.go +++ b/internal/fetch/cache.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "time" ) @@ -153,6 +154,181 @@ func validateCachePath(workspaceRoot, dir string) error { return nil } +// DirCacheEntry is metadata for a cached directory resource (e.g., a skill). +type DirCacheEntry struct { + URL string `json:"url"` + FetchTime time.Time `json:"fetch_time"` + SHA256 string `json:"sha256"` // tree hash + Type string `json:"type"` // always "directory" + Files []DirFileEntry `json:"files"` +} + +// DirFileEntry records one file within a cached directory tree. +type DirFileEntry struct { + Path string `json:"path"` + SHA256 string `json:"sha256"` +} + +// ComputeTreeHash computes a deterministic SHA256 hash for a directory tree. +// The hash is SHA256 of the sorted concatenation of "path:sha256(content)\n" +// for all files. This is forge-agnostic and deterministic — any implementation +// can reproduce it from the same file set. +func ComputeTreeHash(files map[string][]byte) string { + entries := make([]string, 0, len(files)) + for path, content := range files { + entries = append(entries, path+":"+ComputeSHA256(content)) + } + sort.Strings(entries) + joined := strings.Join(entries, "\n") + "\n" + return ComputeSHA256([]byte(joined)) +} + +// CachePutDir stores a directory tree in the content-addressed cache. +// files maps relative paths to their content bytes. Returns the computed +// tree hash. The directory is stored under: +// +// /.fullsend-cache/resources/sha256//tree/ +// +// Uses atomic file writes within the tree directory. +func CachePutDir(workspaceRoot, url string, files map[string][]byte) (string, error) { + if len(files) == 0 { + return "", fmt.Errorf("cannot cache empty directory") + } + + treeHash := ComputeTreeHash(files) + dir, err := CachePath(workspaceRoot, treeHash) + if err != nil { + return "", err + } + + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", fmt.Errorf("creating cache directory: %w", err) + } + + if err := validateCachePath(workspaceRoot, dir); err != nil { + return "", err + } + + // Build the tree directory. + treeDir := filepath.Join(dir, "tree") + + // Write each file. + for relPath, content := range files { + fullPath := filepath.Join(treeDir, relPath) + cleanFull := filepath.Clean(fullPath) + cleanTree := filepath.Clean(treeDir) + string(filepath.Separator) + if !strings.HasPrefix(cleanFull, cleanTree) { + return "", fmt.Errorf("path traversal in file path: %s", relPath) + } + fileDir := filepath.Dir(fullPath) + if err := os.MkdirAll(fileDir, 0o700); err != nil { + return "", fmt.Errorf("creating directory for %s: %w", relPath, err) + } + if err := atomicWrite(fileDir, filepath.Base(fullPath), content); err != nil { + return "", fmt.Errorf("writing %s: %w", relPath, err) + } + } + + // Build file manifest for metadata. + fileEntries := make([]DirFileEntry, 0, len(files)) + for relPath, content := range files { + fileEntries = append(fileEntries, DirFileEntry{ + Path: relPath, + SHA256: ComputeSHA256(content), + }) + } + sort.Slice(fileEntries, func(i, j int) bool { + return fileEntries[i].Path < fileEntries[j].Path + }) + + // Write metadata. + entry := DirCacheEntry{ + URL: url, + FetchTime: time.Now().UTC(), + SHA256: treeHash, + Type: "directory", + Files: fileEntries, + } + metadataBytes, err := json.MarshalIndent(entry, "", " ") + if err != nil { + return "", fmt.Errorf("marshaling cache metadata: %w", err) + } + if err := atomicWrite(dir, "metadata.json", metadataBytes); err != nil { + return "", fmt.Errorf("writing cache metadata: %w", err) + } + + return treeHash, nil +} + +// CacheGetDir retrieves a previously cached directory resource by its tree hash. +// Returns ("", nil, nil) on a cache miss. On a hit, returns the path to the +// tree/ subdirectory and the cache metadata. Re-verifies integrity by recomputing +// the tree hash from the cached files. +func CacheGetDir(workspaceRoot, hash string) (string, *DirCacheEntry, error) { + dir, err := CachePath(workspaceRoot, hash) + if err != nil { + return "", nil, err + } + + // Read metadata. + metadataBytes, err := os.ReadFile(filepath.Join(dir, "metadata.json")) + if err != nil { + if os.IsNotExist(err) { + return "", nil, nil // cache miss + } + return "", nil, fmt.Errorf("reading cache metadata: %w", err) + } + + var entry DirCacheEntry + if err := json.Unmarshal(metadataBytes, &entry); err != nil { + return "", nil, fmt.Errorf("unmarshaling cache metadata: %w", err) + } + + if entry.Type != "directory" { + return "", nil, nil // not a directory cache entry + } + + treeDir := filepath.Join(dir, "tree") + if _, err := os.Stat(treeDir); os.IsNotExist(err) { + return "", nil, nil // partial cache entry + } + + if err := validateCachePath(workspaceRoot, dir); err != nil { + return "", nil, err + } + + // Re-verify integrity: walk the tree directory and recompute the tree hash. + files := make(map[string][]byte) + err = filepath.Walk(treeDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + relPath, err := filepath.Rel(treeDir, path) + if err != nil { + return err + } + content, err := os.ReadFile(path) + if err != nil { + return err + } + files[relPath] = content + return nil + }) + if err != nil { + return "", nil, fmt.Errorf("walking cache tree: %w", err) + } + + actualHash := ComputeTreeHash(files) + if actualHash != hash { + return "", nil, fmt.Errorf("cache integrity check failed: expected %s, got %s", hash, actualHash) + } + + return treeDir, &entry, nil +} + // atomicWrite writes data to a temporary file in dir, then renames it to the // final name. This ensures readers never see a partially-written file. func atomicWrite(dir, name string, data []byte) error { diff --git a/internal/fetch/cache_test.go b/internal/fetch/cache_test.go index f790a44804..fb2ff12b8e 100644 --- a/internal/fetch/cache_test.go +++ b/internal/fetch/cache_test.go @@ -205,6 +205,147 @@ func TestCacheSymlinkProtection(t *testing.T) { assert.Nil(t, entry) } +func TestComputeTreeHash(t *testing.T) { + t.Run("Deterministic", func(t *testing.T) { + files := map[string][]byte{ + "a.txt": []byte("alpha"), + "b.txt": []byte("bravo"), + "c.txt": []byte("charlie"), + } + // Compute the hash multiple times — map iteration order varies but hash must be stable. + hash1 := ComputeTreeHash(files) + hash2 := ComputeTreeHash(files) + hash3 := ComputeTreeHash(files) + assert.Equal(t, hash1, hash2) + assert.Equal(t, hash2, hash3) + }) + + t.Run("SingleFile", func(t *testing.T) { + files := map[string][]byte{ + "SKILL.md": []byte("# My Skill"), + } + hash := ComputeTreeHash(files) + assert.Len(t, hash, 64, "should be a 64-char hex SHA256") + }) + + t.Run("DifferentFilesProduceDifferentHashes", func(t *testing.T) { + files1 := map[string][]byte{"a.txt": []byte("hello")} + files2 := map[string][]byte{"a.txt": []byte("world")} + files3 := map[string][]byte{"b.txt": []byte("hello")} + hash1 := ComputeTreeHash(files1) + hash2 := ComputeTreeHash(files2) + hash3 := ComputeTreeHash(files3) + assert.NotEqual(t, hash1, hash2, "different content should produce different hashes") + assert.NotEqual(t, hash1, hash3, "different paths should produce different hashes") + }) + + t.Run("NestedPaths", func(t *testing.T) { + files := map[string][]byte{ + "SKILL.md": []byte("# Skill"), + "scripts/helper.sh": []byte("#!/bin/bash\necho hi"), + "sub-agents/code.md": []byte("# Code agent"), + } + hash := ComputeTreeHash(files) + assert.Len(t, hash, 64) + }) +} + +func TestCachePutDir_CacheGetDir_RoundTrip(t *testing.T) { + root := t.TempDir() + url := "https://github.com/example/repo/tree/main/skills/review" + files := map[string][]byte{ + "SKILL.md": []byte("# Review Skill\nA skill for reviews."), + "scripts/helper.sh": []byte("#!/bin/bash\necho helper"), + "sub-agents/triage.md": []byte("# Triage sub-agent"), + } + + treeHash, err := CachePutDir(root, url, files) + require.NoError(t, err) + assert.Len(t, treeHash, 64) + + treeDir, entry, err := CacheGetDir(root, treeHash) + require.NoError(t, err) + require.NotNil(t, entry) + + // Verify metadata. + assert.Equal(t, url, entry.URL) + assert.Equal(t, treeHash, entry.SHA256) + assert.Equal(t, "directory", entry.Type) + assert.False(t, entry.FetchTime.IsZero()) + assert.Len(t, entry.Files, 3) + + // Files should be sorted by path in metadata. + assert.Equal(t, "SKILL.md", entry.Files[0].Path) + assert.Equal(t, "scripts/helper.sh", entry.Files[1].Path) + assert.Equal(t, "sub-agents/triage.md", entry.Files[2].Path) + + // Verify file content on disk. + for relPath, expectedContent := range files { + got, err := os.ReadFile(filepath.Join(treeDir, relPath)) + require.NoError(t, err, "reading %s", relPath) + assert.Equal(t, expectedContent, got, "content mismatch for %s", relPath) + } +} + +func TestCacheGetDir_Miss(t *testing.T) { + root := t.TempDir() + hash := ComputeSHA256([]byte("nonexistent dir")) + + treeDir, entry, err := CacheGetDir(root, hash) + require.NoError(t, err) + assert.Empty(t, treeDir) + assert.Nil(t, entry) +} + +func TestCacheGetDir_IntegrityVerification(t *testing.T) { + root := t.TempDir() + files := map[string][]byte{ + "SKILL.md": []byte("# Original content"), + } + + treeHash, err := CachePutDir(root, "https://example.com/skill", files) + require.NoError(t, err) + + // Tamper with the cached file. + dir, err := CachePath(root, treeHash) + require.NoError(t, err) + tamperedPath := filepath.Join(dir, "tree", "SKILL.md") + require.NoError(t, os.WriteFile(tamperedPath, []byte("# Tampered!"), 0o600)) + + treeDir, entry, err := CacheGetDir(root, treeHash) + require.Error(t, err) + assert.Contains(t, err.Error(), "cache integrity check failed") + assert.Empty(t, treeDir) + assert.Nil(t, entry) +} + +func TestCachePutDir_NestedDirectories(t *testing.T) { + root := t.TempDir() + files := map[string][]byte{ + "SKILL.md": []byte("# Skill"), + "scripts/helper.sh": []byte("#!/bin/bash\necho hi"), + "sub-agents/review.md": []byte("# Review"), + "sub-agents/deep/nested/file.md": []byte("# Deep nested"), + } + + treeHash, err := CachePutDir(root, "https://example.com/nested-skill", files) + require.NoError(t, err) + + treeDir, entry, err := CacheGetDir(root, treeHash) + require.NoError(t, err) + require.NotNil(t, entry) + + // Verify all files exist with correct content. + for relPath, expectedContent := range files { + got, err := os.ReadFile(filepath.Join(treeDir, relPath)) + require.NoError(t, err, "reading %s", relPath) + assert.Equal(t, expectedContent, got, "content mismatch for %s", relPath) + } + + // Verify metadata has all files. + assert.Len(t, entry.Files, 4) +} + func TestCacheConcurrentPut(t *testing.T) { root := t.TempDir() content := []byte("concurrent content") diff --git a/internal/forge/fake.go b/internal/forge/fake.go index 28b136d5b7..aec624109b 100644 --- a/internal/forge/fake.go +++ b/internal/forge/fake.go @@ -12,12 +12,14 @@ var _ Client = (*FakeClient)(nil) // NewFakeClient returns a FakeClient with all maps initialised. func NewFakeClient() *FakeClient { return &FakeClient{ - FileContents: make(map[string][]byte), - WorkflowRuns: make(map[string]*WorkflowRun), - Secrets: make(map[string]bool), - VariablesExist: make(map[string]bool), - VariableValues: make(map[string]string), - Errors: make(map[string]error), + FileContents: make(map[string][]byte), + WorkflowRuns: make(map[string]*WorkflowRun), + Secrets: make(map[string]bool), + VariablesExist: make(map[string]bool), + VariableValues: make(map[string]string), + Errors: make(map[string]error), + DirContents: make(map[string][]DirectoryEntry), + FileContentsRef: make(map[string][]byte), } } @@ -124,6 +126,12 @@ type FakeClient struct { OrgVariableValues map[string]string // key: "org/name" → value OrgVariableRepoIDs map[string][]int64 // key: "org/name" → repo IDs + // Directory listings for ListDirectoryContents. + DirContents map[string][]DirectoryEntry // key: "owner/repo/path@ref" + + // File contents at specific refs for GetFileContentAtRef. + FileContentsRef map[string][]byte // key: "owner/repo/path@ref" + // Error injection: key is method name, value is error to return. Errors map[string]error @@ -382,6 +390,38 @@ func (f *FakeClient) DeleteFile(_ context.Context, owner, repo, path, message st return nil } +func (f *FakeClient) ListDirectoryContents(_ context.Context, owner, repo, path, ref string, _ bool) ([]DirectoryEntry, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("ListDirectoryContents"); e != nil { + return nil, e + } + + key := fmt.Sprintf("%s/%s/%s@%s", owner, repo, path, ref) + entries, ok := f.DirContents[key] + if !ok { + return nil, fmt.Errorf("%w: %s", ErrNotFound, key) + } + return entries, nil +} + +func (f *FakeClient) GetFileContentAtRef(_ context.Context, owner, repo, path, ref string) ([]byte, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("GetFileContentAtRef"); e != nil { + return nil, e + } + + key := fmt.Sprintf("%s/%s/%s@%s", owner, repo, path, ref) + content, ok := f.FileContentsRef[key] + if !ok { + return nil, fmt.Errorf("%w: %s", ErrNotFound, key) + } + return content, nil +} + func (f *FakeClient) CommitFiles(_ context.Context, owner, repo, message string, files []TreeFile) (bool, error) { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/forge/fake_test.go b/internal/forge/fake_test.go index 844a3ec1fa..42bdf4ac63 100644 --- a/internal/forge/fake_test.go +++ b/internal/forge/fake_test.go @@ -467,6 +467,14 @@ func TestFakeClient_ErrorInjection(t *testing.T) { {"DeleteIssueComment", func(fc *FakeClient) error { return fc.DeleteIssueComment(ctx, "o", "r", 1) }}, + {"ListDirectoryContents", func(fc *FakeClient) error { + _, err := fc.ListDirectoryContents(ctx, "o", "r", "p", "main", false) + return err + }}, + {"GetFileContentAtRef", func(fc *FakeClient) error { + _, err := fc.GetFileContentAtRef(ctx, "o", "r", "p", "main") + return err + }}, } for _, m := range methods { @@ -535,6 +543,8 @@ func TestFakeClient_ThreadSafety(t *testing.T) { _ = fc.SetOrgVariableRepos(ctx, "o", "n", []int64{1, 2}) _, _ = fc.GetOrgVariableRepos(ctx, "o", "n") _ = fc.DeleteIssueComment(ctx, "o", "r", 1) + _, _ = fc.ListDirectoryContents(ctx, "o", "r", "p", "main", false) + _, _ = fc.GetFileContentAtRef(ctx, "o", "r", "p", "main") }(i) } diff --git a/internal/forge/forge.go b/internal/forge/forge.go index a8cc25bcc3..2bb135aba4 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -123,6 +123,13 @@ type TreeFile struct { Mode string // "100644" or "100755" } +// DirectoryEntry represents a file or subdirectory in a repository directory listing. +type DirectoryEntry struct { + Path string // relative path within the listed directory + Type string // "file" or "dir" + Size int // file size in bytes (0 for directories) +} + // Client abstracts all git forge operations. // Implementations exist for GitHub (and eventually GitLab, Forgejo). type Client interface { @@ -161,6 +168,18 @@ type Client interface { GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, error) DeleteFile(ctx context.Context, owner, repo, path, message string) error + // ListDirectoryContents returns all files and subdirectories at the given + // path in a repository at the specified ref (commit SHA, branch, or tag). + // When recursive is true, nested subdirectories are flattened into the + // result with paths relative to the listed directory. + // Returns forge.ErrNotFound if the path does not exist or is not a directory. + ListDirectoryContents(ctx context.Context, owner, repo, path, ref string, recursive bool) ([]DirectoryEntry, error) + + // GetFileContentAtRef retrieves the content of a file at a specific ref + // (commit SHA, branch, or tag). Unlike GetFileContent which reads from + // the default branch, this reads from the specified ref. + GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) ([]byte, error) + // CommitFiles atomically commits multiple files to the repository's // default branch in a single commit. It is idempotent: if all files // already have the expected content and mode, no commit is created diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index 2110cfe798..b048221d02 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -779,6 +779,123 @@ func (c *LiveClient) GetFileContent(ctx context.Context, owner, repo, path strin return data, nil } +// escapePathSegments URL-escapes each segment of a slash-separated path +// individually, preserving the / separators. +func escapePathSegments(p string) string { + segments := strings.Split(p, "/") + for i, s := range segments { + segments[i] = url.PathEscape(s) + } + return strings.Join(segments, "/") +} + +// GetFileContentAtRef retrieves the content of a file at a specific ref +// (commit SHA, branch, or tag). Unlike GetFileContent which reads from +// the default branch, this reads from the specified ref. +func (c *LiveClient) GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + resp, err := c.get(ctx, fmt.Sprintf("/repos/%s/%s/contents/%s?ref=%s", + url.PathEscape(owner), url.PathEscape(repo), escapePathSegments(path), url.QueryEscape(ref))) + if err != nil { + return nil, fmt.Errorf("get file content at ref: %w", err) + } + + var file struct { + Content string `json:"content"` + } + if err := decodeJSON(resp, &file); err != nil { + return nil, fmt.Errorf("decode file content: %w", err) + } + + cleaned := strings.ReplaceAll(strings.ReplaceAll(file.Content, "\n", ""), "\r", "") + data, err := base64.StdEncoding.DecodeString(cleaned) + if err != nil { + return nil, fmt.Errorf("decode base64 content: %w", err) + } + return data, nil +} + +const ( + maxDirDepth = 10 + maxDirAPIcalls = 100 + maxDirFiles = 1000 +) + +// ListDirectoryContents returns all files and subdirectories at the given +// path in a repository at the specified ref. When path points to a directory, +// the GitHub Contents API returns a JSON array of entries. +func (c *LiveClient) ListDirectoryContents(ctx context.Context, owner, repo, path, ref string, recursive bool) ([]forge.DirectoryEntry, error) { + apiCalls := 0 + fileCount := 0 + return c.listDirContents(ctx, owner, repo, path, ref, recursive, 0, &apiCalls, &fileCount) +} + +func (c *LiveClient) listDirContents(ctx context.Context, owner, repo, path, ref string, recursive bool, depth int, apiCalls *int, fileCount *int) ([]forge.DirectoryEntry, error) { + if depth > maxDirDepth { + return nil, fmt.Errorf("directory listing exceeded maximum depth of %d at %s", maxDirDepth, path) + } + if *apiCalls >= maxDirAPIcalls { + return nil, fmt.Errorf("directory listing exceeded maximum of %d API calls", maxDirAPIcalls) + } + *apiCalls++ + + apiPath := fmt.Sprintf("/repos/%s/%s/contents/%s?ref=%s", + url.PathEscape(owner), url.PathEscape(repo), escapePathSegments(path), url.QueryEscape(ref)) + + resp, err := c.get(ctx, apiPath) + if err != nil { + return nil, fmt.Errorf("list directory: %w", err) + } + + var entries []struct { + Name string `json:"name"` + Path string `json:"path"` // full path from repo root + Type string `json:"type"` // "file" or "dir" + Size int `json:"size"` + } + if err := decodeJSON(resp, &entries); err != nil { + return nil, fmt.Errorf("decode directory listing: %w", err) + } + + var result []forge.DirectoryEntry + for _, e := range entries { + var relPath string + if path == "" { + relPath = e.Path + } else { + relPath = strings.TrimPrefix(e.Path, path+"/") + } + + if e.Type == "file" { + if *fileCount >= maxDirFiles { + return nil, fmt.Errorf("directory listing exceeded maximum of %d files", maxDirFiles) + } + *fileCount++ + result = append(result, forge.DirectoryEntry{ + Path: relPath, + Type: "file", + Size: e.Size, + }) + } else if e.Type == "dir" && recursive { + subEntries, err := c.listDirContents(ctx, owner, repo, e.Path, ref, true, depth+1, apiCalls, fileCount) + if err != nil { + return nil, fmt.Errorf("listing subdirectory %s: %w", e.Path, err) + } + for _, sub := range subEntries { + sub.Path = relPath + "/" + sub.Path + result = append(result, sub) + } + } else if e.Type == "dir" { + result = append(result, forge.DirectoryEntry{ + Path: relPath, + Type: "dir", + Size: 0, + }) + } + } + + return result, nil +} + // DeleteFile deletes a file from the repository's default branch. // It first fetches the file to obtain its SHA (required by the GitHub Contents // API), then issues the DELETE. Retries on transient 404/409 errors. diff --git a/internal/forge/url.go b/internal/forge/url.go new file mode 100644 index 0000000000..791c934ddc --- /dev/null +++ b/internal/forge/url.go @@ -0,0 +1,98 @@ +package forge + +import ( + "fmt" + "net/url" + "strings" +) + +// ForgeURLInfo contains the parsed components of a forge URL. +type ForgeURLInfo struct { + Forge string // "github" (future: "gitlab") + Owner string + Repo string + Path string // path within the repo (e.g., "skills/pr-review") + Ref string // commit SHA, tag, or branch name +} + +// ParseForgeURL extracts forge, owner, repo, path, and ref from an HTTPS URL +// pointing to a supported git forge. Returns an error if the URL is not from a +// recognized forge or cannot be parsed. +// +// Any #sha256=... fragment is stripped before parsing — handle integrity hashes +// separately via ParseIntegrityHash. +// +// Accepted GitHub formats: +// +// https://github.com/{owner}/{repo}/tree/{ref}/{path} (directory) +// https://github.com/{owner}/{repo}/blob/{ref}/{path} (file) +func ParseForgeURL(rawURL string) (*ForgeURLInfo, error) { + // Strip fragment (including #sha256=... integrity hashes) before parsing. + if idx := strings.LastIndex(rawURL, "#"); idx != -1 { + rawURL = rawURL[:idx] + } + + u, err := url.Parse(rawURL) + if err != nil { + return nil, fmt.Errorf("invalid URL: %w", err) + } + if u.Scheme != "https" { + return nil, fmt.Errorf("unsupported scheme %q: only https is accepted", u.Scheme) + } + + hostname := u.Hostname() + if !IsSupportedForge(hostname) { + return nil, fmt.Errorf("unsupported forge host %q", hostname) + } + + // Split the path into segments, filtering out empty strings from leading/trailing slashes. + var segments []string + for _, s := range strings.Split(u.Path, "/") { + if s != "" { + segments = append(segments, s) + } + } + + // Need at least 4 segments: owner, repo, type (tree/blob), ref. + if len(segments) < 4 { + return nil, fmt.Errorf("URL path too short: need at least /{owner}/{repo}/{tree|blob}/{ref}") + } + + owner := segments[0] + repo := segments[1] + pathType := segments[2] + ref := segments[3] + + if owner == "" { + return nil, fmt.Errorf("empty owner in URL") + } + if repo == "" { + return nil, fmt.Errorf("empty repo in URL") + } + if ref == "" { + return nil, fmt.Errorf("empty ref in URL") + } + + if pathType != "tree" && pathType != "blob" { + return nil, fmt.Errorf("unsupported path type %q: expected \"tree\" or \"blob\"", pathType) + } + + // Everything after the ref is the path within the repo. + var repoPath string + if len(segments) > 4 { + repoPath = strings.Join(segments[4:], "/") + } + + return &ForgeURLInfo{ + Forge: "github", + Owner: owner, + Repo: repo, + Path: repoPath, + Ref: ref, + }, nil +} + +// IsSupportedForge returns true if the hostname belongs to a recognized forge. +func IsSupportedForge(hostname string) bool { + return hostname == "github.com" +} diff --git a/internal/forge/url_test.go b/internal/forge/url_test.go new file mode 100644 index 0000000000..25e512540d --- /dev/null +++ b/internal/forge/url_test.go @@ -0,0 +1,134 @@ +package forge + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseForgeURL(t *testing.T) { + tests := []struct { + name string + input string + want *ForgeURLInfo + wantErr string + }{ + { + name: "valid GitHub tree URL with commit SHA", + input: "https://github.com/fullsend-ai/library/tree/8cd3799abc/skills/pr-review", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "fullsend-ai", + Repo: "library", + Ref: "8cd3799abc", + Path: "skills/pr-review", + }, + }, + { + name: "valid GitHub blob URL", + input: "https://github.com/fullsend-ai/library/blob/8cd3799abc/agents/code.md", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "fullsend-ai", + Repo: "library", + Ref: "8cd3799abc", + Path: "agents/code.md", + }, + }, + { + name: "URL with sha256 fragment stripped", + input: "https://github.com/fullsend-ai/library/tree/abc123/skills/rust#sha256=def456abcdef0123456789abcdef0123456789abcdef0123456789abcdef01", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "fullsend-ai", + Repo: "library", + Ref: "abc123", + Path: "skills/rust", + }, + }, + { + name: "root path with no path after ref", + input: "https://github.com/fullsend-ai/library/tree/abc123", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "fullsend-ai", + Repo: "library", + Ref: "abc123", + Path: "", + }, + }, + { + name: "non-forge domain", + input: "https://example.com/foo/bar", + wantErr: "unsupported forge host", + }, + { + name: "HTTP not HTTPS", + input: "http://github.com/owner/repo/tree/ref/path", + wantErr: "unsupported scheme", + }, + { + name: "missing type segment", + input: "https://github.com/owner/repo", + wantErr: "URL path too short", + }, + { + name: "invalid type segment", + input: "https://github.com/owner/repo/commits/ref/path", + wantErr: "unsupported path type", + }, + { + name: "deep nested path", + input: "https://github.com/org/repo/tree/main/a/b/c/d", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "org", + Repo: "repo", + Ref: "main", + Path: "a/b/c/d", + }, + }, + { + name: "tag as ref", + input: "https://github.com/org/repo/tree/v1.2.3/skills/foo", + want: &ForgeURLInfo{ + Forge: "github", + Owner: "org", + Repo: "repo", + Ref: "v1.2.3", + Path: "skills/foo", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseForgeURL(tt.input) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestIsSupportedForge(t *testing.T) { + tests := []struct { + name string + hostname string + want bool + }{ + {"github.com", "github.com", true}, + {"gitlab.com not yet supported", "gitlab.com", false}, + {"example.com", "example.com", false}, + {"empty string", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsSupportedForge(tt.hostname)) + }) + } +} diff --git a/internal/harness/harness.go b/internal/harness/harness.go index e17c39e4a8..bf5686a171 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -10,6 +10,8 @@ import ( "strings" "gopkg.in/yaml.v3" + + "github.com/fullsend-ai/fullsend/internal/forge" ) var ( @@ -626,12 +628,27 @@ func (h *Harness) ValidateResourceTypes() error { if _, _, hasHash := ParseIntegrityHash(s); !hasHash { return fmt.Errorf("skills[%d] URL must include #sha256=... integrity hash", i) } + cleanURL, _, _ := ParseIntegrityHash(s) + if _, err := forge.ParseForgeURL(cleanURL); err != nil { + return fmt.Errorf("skills[%d] URL must be hosted on a supported forge (github.com): %w", i, err) + } } } return nil } +// HasURLSkills reports whether any skill field contains a URL. Used to determine +// whether a forge client is needed for resolution. +func (h *Harness) HasURLSkills() bool { + for _, s := range h.Skills { + if IsURL(s) { + return true + } + } + return false +} + // HasURLReferences reports whether any declarative field (agent, policy, skills) // contains a URL. Used to skip remote resource validation and resolution when // the harness references only local paths. diff --git a/internal/lock/lock.go b/internal/lock/lock.go index d79446bccd..57f2705de0 100644 --- a/internal/lock/lock.go +++ b/internal/lock/lock.go @@ -35,10 +35,18 @@ type DependencyEntry struct { Field string `yaml:"field"` URL string `yaml:"url"` SHA256 string `yaml:"sha256"` + Type string `yaml:"type,omitempty"` // "file" or "directory"; empty treated as "file" + Files []FileEntry `yaml:"files,omitempty"` // manifest of files (directory deps only) FetchedAt time.Time `yaml:"fetched_at"` TransitiveDeps []DependencyEntry `yaml:"transitive_deps,omitempty"` } +// FileEntry records one file within a directory dependency. +type FileEntry struct { + Path string `yaml:"path"` + SHA256 string `yaml:"sha256"` +} + const currentVersion = 1 // Load reads a lock file from path. Returns nil (no error) if the file diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 3b03fbef97..9b4bfaca8d 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -3,11 +3,13 @@ package resolve import ( "context" "fmt" + "os" "path/filepath" "strings" "time" "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/skill" ) @@ -25,6 +27,7 @@ type Dependency struct { SHA256 string FetchedAt time.Time CacheHit bool + Type string // "file" or "directory" } // ResolveOpts controls how URL-referenced resources are resolved. @@ -34,6 +37,11 @@ type ResolveOpts struct { TraceID string AuditLogPath string + // ForgeClient is required when the harness contains URL-referenced skills. + // Skills are directories on supported forges; the forge API is used to list + // and fetch all files in the skill directory. + ForgeClient forge.Client + // MaxDepth controls transitive dependency resolution depth. // 0 disables transitive resolution (Phase 1 behavior). // <0 uses DefaultMaxDepth (10). @@ -62,6 +70,11 @@ type resolveState struct { // and h.Skills may grow to include transitively resolved skill dependencies. // Returns the deduplicated list of resolved dependencies. // +// Skills are directories: when a skill field is a URL, the resolver uses the +// forge API (via ForgeClient) to list the directory contents, fetch each file, +// and cache the reconstructed tree. Only URLs pointing to supported forges +// (github.com) are accepted for skills. Agents and policies remain single files. +// // Skills with dependencies: frontmatter are recursively resolved up to // MaxDepth levels. Diamond dependencies are deduplicated; cycles are rejected. // Set MaxDepth to 0 to disable transitive resolution. Negative values use @@ -74,6 +87,7 @@ type resolveState struct { // // The default limits (depth=10, resources=50) bound worst-case resolution. // CI environments with untrusted harnesses should set tighter limits. +// See ADR-0038 for the security model and trust semantics. func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ([]Dependency, error) { maxDepth := opts.MaxDepth if maxDepth < 0 { @@ -95,7 +109,7 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( recurse := maxDepth > 0 if h.Agent != "" && harness.IsURL(h.Agent) { - dep, localPath, err := resolveURL(ctx, "agent", h.Agent, h, opts, state, false, 0) + dep, localPath, err := resolveFileURL(ctx, "agent", h.Agent, h, opts, state) if err != nil { return nil, fmt.Errorf("resolving agent: %w", err) } @@ -104,7 +118,7 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( } if h.Policy != "" && harness.IsURL(h.Policy) { - dep, localPath, err := resolveURL(ctx, "policy", h.Policy, h, opts, state, false, 0) + dep, localPath, err := resolveFileURL(ctx, "policy", h.Policy, h, opts, state) if err != nil { return nil, fmt.Errorf("resolving policy: %w", err) } @@ -114,7 +128,7 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( for i, s := range h.Skills { if harness.IsURL(s) { - dep, localPath, err := resolveURL(ctx, fmt.Sprintf("skills[%d]", i), s, h, opts, state, recurse, 0) + dep, localPath, err := resolveSkillDirURL(ctx, fmt.Sprintf("skills[%d]", i), s, h, opts, state, recurse, 0) if err != nil { return nil, fmt.Errorf("resolving skills[%d]: %w", i, err) } @@ -147,8 +161,10 @@ func (s *resolveState) appendDependency(dep Dependency) { s.deps = append(s.deps, dep) } -func resolveURL(ctx context.Context, field, rawURL string, h *harness.Harness, - opts ResolveOpts, state *resolveState, recurse bool, depth int, +// resolveFileURL fetches a single file from a URL and caches it. +// Used for agents, policies, and other single-file resources. +func resolveFileURL(ctx context.Context, field, rawURL string, h *harness.Harness, + opts ResolveOpts, state *resolveState, ) (Dependency, string, error) { cleanURL, expectedHash, hasHash := harness.ParseIntegrityHash(rawURL) if !hasHash { @@ -164,9 +180,9 @@ func resolveURL(ctx context.Context, field, rawURL string, h *harness.Harness, "%s: URL %s has conflicting integrity hashes: previously resolved with %s, now referenced with %s", field, cleanURL, dep.SHA256, expectedHash) } - copy := dep - copy.Field = field - return copy, dep.LocalPath, nil + depCopy := dep + depCopy.Field = field + return depCopy, dep.LocalPath, nil } if state.inProgress[cleanURL] { return Dependency{}, "", fmt.Errorf("%s: circular dependency detected for %s", field, cleanURL) @@ -232,8 +248,142 @@ func resolveURL(ctx context.Context, field, rawURL string, h *harness.Harness, } } + dep := Dependency{ + Field: field, + URL: cleanURL, + LocalPath: localPath, + SHA256: expectedHash, + FetchedAt: fetchedAt, + CacheHit: cacheHit, + Type: "file", + } + + state.resolved[cleanURL] = dep + + return dep, localPath, nil +} + +// resolveSkillDirURL fetches a skill directory from a supported forge and caches +// the reconstructed directory tree. Skills are always directories containing at +// minimum a SKILL.md file plus optional companion files (scripts/, sub-agents/). +// Only URLs pointing to supported forges are accepted; non-forge HTTPS URLs are +// rejected because HTTP has no standard directory listing mechanism. +func resolveSkillDirURL(ctx context.Context, field, rawURL string, h *harness.Harness, + opts ResolveOpts, state *resolveState, recurse bool, depth int, +) (Dependency, string, error) { + cleanURL, expectedHash, hasHash := harness.ParseIntegrityHash(rawURL) + if !hasHash { + return Dependency{}, "", fmt.Errorf("%s: URL must include #sha256=... integrity hash", field) + } + if !strings.HasPrefix(cleanURL, "https://") { + return Dependency{}, "", fmt.Errorf("%s: URL scheme must be https: %s", field, cleanURL) + } + + if dep, ok := state.resolved[cleanURL]; ok { + if dep.SHA256 != expectedHash { + return Dependency{}, "", fmt.Errorf( + "%s: URL %s has conflicting integrity hashes: previously resolved with %s, now referenced with %s", + field, cleanURL, dep.SHA256, expectedHash) + } + depCopy := dep + depCopy.Field = field + return depCopy, dep.LocalPath, nil + } + if state.inProgress[cleanURL] { + return Dependency{}, "", fmt.Errorf("%s: circular dependency detected for %s", field, cleanURL) + } + if state.resourceCount >= state.maxResources { + return Dependency{}, "", fmt.Errorf("%s: exceeded maximum resource count of %d for %s", field, state.maxResources, cleanURL) + } + + state.inProgress[cleanURL] = true + defer delete(state.inProgress, cleanURL) + state.resourceCount++ + + allowedBy := h.MatchingAllowedPrefix(cleanURL) + if allowedBy == "" { + return Dependency{}, "", fmt.Errorf("%s: URL %q is not in allowed_remote_resources", field, cleanURL) + } + + forgeInfo, err := forge.ParseForgeURL(cleanURL) + if err != nil { + return Dependency{}, "", fmt.Errorf("%s: skill URLs must be hosted on a supported forge: %w", field, err) + } + + treePath, dirEntry, err := fetch.CacheGetDir(opts.WorkspaceRoot, expectedHash) + if err != nil { + return Dependency{}, "", fmt.Errorf("dir cache lookup for %s: %w", field, err) + } + + cacheHit := treePath != "" + fetchedAt := time.Now().UTC() + + if !cacheHit { + if opts.ForgeClient == nil { + return Dependency{}, "", fmt.Errorf("%s: ForgeClient is required to resolve skill URL %s (not cached)", field, cleanURL) + } + if opts.FetchPolicy.Offline { + return Dependency{}, "", fmt.Errorf("fetching %s from %s: offline mode, no cache entry", field, cleanURL) + } + + dirPath := forgeInfo.Path + entries, err := opts.ForgeClient.ListDirectoryContents(ctx, forgeInfo.Owner, forgeInfo.Repo, dirPath, forgeInfo.Ref, true) + if err != nil { + return Dependency{}, "", fmt.Errorf("listing directory for %s at %s: %w", field, cleanURL, err) + } + + files := make(map[string][]byte) + for _, e := range entries { + if e.Type != "file" { + continue + } + var fullPath string + if dirPath == "" { + fullPath = e.Path + } else { + fullPath = dirPath + "/" + e.Path + } + content, err := opts.ForgeClient.GetFileContentAtRef(ctx, forgeInfo.Owner, forgeInfo.Repo, fullPath, forgeInfo.Ref) + if err != nil { + return Dependency{}, "", fmt.Errorf("fetching file %s for %s: %w", e.Path, field, err) + } + files[e.Path] = content + } + + actualHash := fetch.ComputeTreeHash(files) + if actualHash != expectedHash { + return Dependency{}, "", fmt.Errorf("%s: integrity check failed for %s: expected %s, got %s", field, cleanURL, expectedHash, actualHash) + } + + if _, err := fetch.CachePutDir(opts.WorkspaceRoot, cleanURL, files); err != nil { + return Dependency{}, "", fmt.Errorf("caching directory for %s: %w", field, err) + } + + cachePath, err := fetch.CachePath(opts.WorkspaceRoot, expectedHash) + if err != nil { + return Dependency{}, "", fmt.Errorf("computing cache path for %s: %w", field, err) + } + treePath = filepath.Join(cachePath, "tree") + } else { + fetchedAt = dirEntry.FetchTime + } + + if opts.AuditLogPath != "" { + if err := fetch.AppendFetchAudit(opts.AuditLogPath, fetch.FetchAuditEntry{ + TraceID: opts.TraceID, + FetchTime: fetchedAt, + URL: cleanURL, + SHA256: expectedHash, + FetchType: "static", + AllowedBy: allowedBy, + CacheHit: cacheHit, + }); err != nil { + return Dependency{}, "", fmt.Errorf("writing fetch audit log: %w", err) + } + } + if recurse { - if err := resolveTransitiveDeps(ctx, cleanURL, content, h, opts, state, depth+1); err != nil { + if err := resolveSkillTransitiveDeps(ctx, cleanURL, treePath, h, opts, state, depth+1); err != nil { return Dependency{}, "", fmt.Errorf("resolving transitive deps for %s (%s): %w", field, cleanURL, err) } } @@ -241,23 +391,34 @@ func resolveURL(ctx context.Context, field, rawURL string, h *harness.Harness, dep := Dependency{ Field: field, URL: cleanURL, - LocalPath: localPath, + LocalPath: treePath, SHA256: expectedHash, FetchedAt: fetchedAt, CacheHit: cacheHit, + Type: "directory", } state.resolved[cleanURL] = dep - return dep, localPath, nil + return dep, treePath, nil } -// resolveTransitiveDeps parses skill frontmatter and recursively resolves -// declared dependencies. Policy references are fetched as leaf nodes. +// resolveSkillTransitiveDeps reads SKILL.md from a cached skill directory, +// parses its frontmatter, and recursively resolves declared dependencies. +// Skill dependencies are resolved as directories; policy references as files. // depth is the current nesting level (1 for first-level transitive deps). -func resolveTransitiveDeps(ctx context.Context, parentURL string, content []byte, +func resolveSkillTransitiveDeps(ctx context.Context, parentURL, skillDirPath string, h *harness.Harness, opts ResolveOpts, state *resolveState, depth int, ) error { + skillMDPath := filepath.Join(skillDirPath, "SKILL.md") + content, err := os.ReadFile(skillMDPath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("reading SKILL.md from %s: %w", parentURL, err) + } + meta, err := skill.ParseFrontmatter(content) if err != nil { return fmt.Errorf("%s: %w", parentURL, err) @@ -277,7 +438,7 @@ func resolveTransitiveDeps(ctx context.Context, parentURL string, content []byte } field := fmt.Sprintf("skills[%s:dep%d]", parentURL, i) - dep, localPath, err := resolveURL(ctx, field, resolved, h, opts, state, true, depth) + dep, localPath, err := resolveSkillDirURL(ctx, field, resolved, h, opts, state, true, depth) if err != nil { return err } @@ -295,7 +456,7 @@ func resolveTransitiveDeps(ctx context.Context, parentURL string, content []byte } field := fmt.Sprintf("policy[%s]", parentURL) - dep, _, err := resolveURL(ctx, field, resolved, h, opts, state, false, depth) + dep, _, err := resolveFileURL(ctx, field, resolved, h, opts, state) if err != nil { return err } diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index e850cf9090..e9ed2f1058 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -18,9 +18,68 @@ import ( "github.com/stretchr/testify/require" "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/harness" ) +// --- test helpers for forge-based skill resolution --- + +const ( + testForgeOwner = "test-org" + testForgeRepo = "test-repo" + testForgeRef = "main" + testForgeBase = "https://github.com/" + testForgeOwner + "/" + testForgeRepo + "/" +) + +func forgeSkillURL(path, treeHash string) string { + return fmt.Sprintf("https://github.com/%s/%s/tree/%s/%s#sha256=%s", + testForgeOwner, testForgeRepo, testForgeRef, path, treeHash) +} + +func forgeSkillCleanURL(path string) string { + return fmt.Sprintf("https://github.com/%s/%s/tree/%s/%s", + testForgeOwner, testForgeRepo, testForgeRef, path) +} + +// registerSkillDir sets up a skill directory in the FakeClient and returns the tree hash. +func registerSkillDir(fc *forge.FakeClient, path string, files map[string][]byte) string { + treeHash := fetch.ComputeTreeHash(files) + + dirKey := fmt.Sprintf("%s/%s/%s@%s", testForgeOwner, testForgeRepo, path, testForgeRef) + + entries := make([]forge.DirectoryEntry, 0, len(files)) + for relPath, content := range files { + entries = append(entries, forge.DirectoryEntry{ + Path: relPath, + Type: "file", + Size: len(content), + }) + } + + if fc.DirContents == nil { + fc.DirContents = make(map[string][]forge.DirectoryEntry) + } + if fc.FileContentsRef == nil { + fc.FileContentsRef = make(map[string][]byte) + } + + fc.DirContents[dirKey] = entries + for relPath, content := range files { + fileKey := fmt.Sprintf("%s/%s/%s/%s@%s", testForgeOwner, testForgeRepo, path, relPath, testForgeRef) + fc.FileContentsRef[fileKey] = content + } + + return treeHash +} + +// skillFrontmatter returns SKILL.md content with the given YAML frontmatter fields +// and optional body text after the closing delimiter. +func skillFrontmatter(fields, body string) []byte { + return []byte("---\n" + fields + "---\n" + body) +} + +// --- test helpers for HTTP-served single-file resources (agents, policies) --- + func newTestServer(t *testing.T, handler http.Handler) (*httptest.Server, fetch.FetchPolicy) { t.Helper() srv := httptest.NewTLSServer(handler) @@ -35,6 +94,8 @@ func newTestServer(t *testing.T, handler http.Handler) (*httptest.Server, fetch. return srv, fetch.NewTestPolicy(tlsCfg, []string{hostname}, []string{port}) } +// --- Tests --- + func TestResolveHarness_LocalPassThrough(t *testing.T) { h := &harness.Harness{ Agent: "/abs/path/agents/test.md", @@ -77,12 +138,11 @@ func TestResolveHarness_URLFetchAndCache(t *testing.T) { assert.Equal(t, fmt.Sprintf("%s/agents/code.md", srv.URL), deps[0].URL) assert.Equal(t, agentHash, deps[0].SHA256) assert.False(t, deps[0].CacheHit) + assert.Equal(t, "file", deps[0].Type) - // Verify the harness field was replaced with a local path. assert.True(t, strings.HasSuffix(h.Agent, "/content")) assert.False(t, harness.IsURL(h.Agent)) - // Verify the cached file exists and has the right content. got, err := os.ReadFile(h.Agent) require.NoError(t, err) assert.Equal(t, agentContent, got) @@ -93,80 +153,181 @@ func TestResolveHarness_DependencyField(t *testing.T) { agentHash := fetch.ComputeSHA256(agentContent) policyContent := []byte("policy: readonly") policyHash := fetch.ComputeSHA256(policyContent) - skillContent := []byte("# Skill\nA skill.") - skillHash := fetch.ComputeSHA256(skillContent) + skillMD := []byte("# Skill\nA skill.") - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv, fetchPolicy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/agents/code.md": w.Write(agentContent) case "/policies/ro.yaml": w.Write(policyContent) - case "/skills/rust/SKILL.md": - w.Write(skillContent) default: http.NotFound(w, r) } })) + fc := &forge.FakeClient{} + skillHash := registerSkillDir(fc, "skills/rust", map[string][]byte{"SKILL.md": skillMD}) + root := t.TempDir() h := &harness.Harness{ Agent: fmt.Sprintf("%s/agents/code.md#sha256=%s", srv.URL, agentHash), Policy: fmt.Sprintf("%s/policies/ro.yaml#sha256=%s", srv.URL, policyHash), - Skills: []string{fmt.Sprintf("%s/skills/rust/SKILL.md#sha256=%s", srv.URL, skillHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/rust", skillHash)}, + AllowedRemoteResources: []string{srv.URL + "/", testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: root, - FetchPolicy: policy, + FetchPolicy: fetchPolicy, + ForgeClient: fc, }) require.NoError(t, err) require.Len(t, deps, 3) assert.Equal(t, "agent", deps[0].Field) + assert.Equal(t, "file", deps[0].Type) assert.Equal(t, "policy", deps[1].Field) + assert.Equal(t, "file", deps[1].Type) assert.Equal(t, "skills[0]", deps[2].Field) + assert.Equal(t, "directory", deps[2].Type) } -func TestResolveHarness_DiamondDependency(t *testing.T) { - // When the same URL appears as both a transitive dep and a direct skill, - // the resolver deduplicates: the direct reference is removed from skills, - // and the transitive entry is the one kept in the deps list. - sharedContent := []byte("---\ndependencies: []\n---\n# Shared skill") - sharedHash := fetch.ComputeSHA256(sharedContent) - parentContent := []byte(fmt.Sprintf("---\ndependencies:\n - shared.md#sha256=%s\n---\n# Parent", sharedHash)) - parentHash := fetch.ComputeSHA256(parentContent) +func TestResolveHarness_SkillDirFetchAndCache(t *testing.T) { + skillMD := []byte("---\nname: review\n---\n# Code Review skill") + helperSh := []byte("#!/bin/bash\necho hello") - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/parent.md": - w.Write(parentContent) - case "/skills/shared.md": - w.Write(sharedContent) - default: - http.NotFound(w, r) - } - })) + fc := &forge.FakeClient{} + treeHash := registerSkillDir(fc, "skills/review", map[string][]byte{ + "SKILL.md": skillMD, + "scripts/helper.sh": helperSh, + }) root := t.TempDir() - parentURL := fmt.Sprintf("%s/skills/parent.md#sha256=%s", srv.URL, parentHash) - sharedURL := fmt.Sprintf("%s/skills/shared.md#sha256=%s", srv.URL, sharedHash) h := &harness.Harness{ - Agent: "agents/code.md", - Skills: []string{parentURL, sharedURL}, + Skills: []string{forgeSkillURL("skills/review", treeHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + ForgeClient: fc, + }) + require.NoError(t, err) + require.Len(t, deps, 1) + assert.Equal(t, "directory", deps[0].Type) + assert.Equal(t, treeHash, deps[0].SHA256) + assert.False(t, deps[0].CacheHit) + + // Verify h.Skills[0] is a directory path (the tree/ subdirectory). + info, err := os.Stat(h.Skills[0]) + require.NoError(t, err) + assert.True(t, info.IsDir()) + + // Verify SKILL.md is inside the cached directory. + got, err := os.ReadFile(filepath.Join(h.Skills[0], "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, skillMD, got) + + // Verify companion file is inside the cached directory. + got, err = os.ReadFile(filepath.Join(h.Skills[0], "scripts", "helper.sh")) + require.NoError(t, err) + assert.Equal(t, helperSh, got) +} + +func TestResolveHarness_SkillDirCacheHit(t *testing.T) { + skillMD := []byte("# Cached skill") + + fc := &forge.FakeClient{} + files := map[string][]byte{"SKILL.md": skillMD} + treeHash := registerSkillDir(fc, "skills/cached", files) + + root := t.TempDir() + // Pre-populate the directory cache. + _, err := fetch.CachePutDir(root, forgeSkillCleanURL("skills/cached"), files) + require.NoError(t, err) + + h := &harness.Harness{ + Skills: []string{forgeSkillURL("skills/cached", treeHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + ForgeClient: fc, + }) + require.NoError(t, err) + require.Len(t, deps, 1) + assert.True(t, deps[0].CacheHit) +} + +func TestResolveHarness_SkillDirHashMismatch(t *testing.T) { + fc := &forge.FakeClient{} + registerSkillDir(fc, "skills/tampered", map[string][]byte{"SKILL.md": []byte("wrong content")}) + + wrongHash := fetch.ComputeTreeHash(map[string][]byte{"SKILL.md": []byte("expected content")}) + + h := &harness.Harness{ + Skills: []string{forgeSkillURL("skills/tampered", wrongHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + _, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: t.TempDir(), + ForgeClient: fc, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "integrity check failed") +} + +func TestResolveHarness_SkillNonForgeURLRejected(t *testing.T) { + srv, fetchPolicy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("skill content")) + })) + + fakeHash := strings.Repeat("a", 64) + h := &harness.Harness{ + Skills: []string{fmt.Sprintf("%s/skills/review#sha256=%s", srv.URL, fakeHash)}, AllowedRemoteResources: []string{srv.URL + "/"}, } + _, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: t.TempDir(), + FetchPolicy: fetchPolicy, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "supported forge") +} + +func TestResolveHarness_DiamondDependency(t *testing.T) { + fc := &forge.FakeClient{} + + sharedMD := []byte("---\ndependencies: []\n---\n# Shared skill") + sharedHash := registerSkillDir(fc, "skills/shared", map[string][]byte{"SKILL.md": sharedMD}) + + parentMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - shared#sha256=%s\n", sharedHash), + "# Parent skill", + ) + parentHash := registerSkillDir(fc, "skills/parent", map[string][]byte{"SKILL.md": parentMD}) + + root := t.TempDir() + h := &harness.Harness{ + Skills: []string{ + forgeSkillURL("skills/parent", parentHash), + forgeSkillURL("skills/shared", sharedHash), + }, + AllowedRemoteResources: []string{testForgeBase}, + } + deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: root, - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: 5, }) require.NoError(t, err) - // Diamond deduplication: deps has transitive entry only. - sharedCleanURL := fmt.Sprintf("%s/skills/shared.md", srv.URL) + sharedCleanURL := forgeSkillCleanURL("skills/shared") var sharedFields []string for _, d := range deps { if d.URL == sharedCleanURL { @@ -176,7 +337,6 @@ func TestResolveHarness_DiamondDependency(t *testing.T) { require.Len(t, sharedFields, 1) assert.Contains(t, sharedFields[0], "dep0") - // Skills should have parent + shared (direct URL filtered, transitive kept). require.Len(t, h.Skills, 2) } @@ -310,6 +470,48 @@ func TestResolveHarness_OfflineHit(t *testing.T) { assert.Equal(t, agentContent, got) } +func TestResolveHarness_SkillDirOfflineMiss(t *testing.T) { + fc := &forge.FakeClient{} + skillHash := registerSkillDir(fc, "skills/offline", map[string][]byte{"SKILL.md": []byte("# Skill")}) + + h := &harness.Harness{ + Skills: []string{forgeSkillURL("skills/offline", skillHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + _, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: t.TempDir(), + ForgeClient: fc, + FetchPolicy: fetch.FetchPolicy{Offline: true}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "offline") +} + +func TestResolveHarness_SkillDirOfflineHit(t *testing.T) { + fc := &forge.FakeClient{} + files := map[string][]byte{"SKILL.md": []byte("# Cached skill for offline")} + skillHash := registerSkillDir(fc, "skills/offline", files) + + root := t.TempDir() + _, err := fetch.CachePutDir(root, forgeSkillCleanURL("skills/offline"), files) + require.NoError(t, err) + + h := &harness.Harness{ + Skills: []string{forgeSkillURL("skills/offline", skillHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + ForgeClient: fc, + FetchPolicy: fetch.FetchPolicy{Offline: true}, + }) + require.NoError(t, err) + require.Len(t, deps, 1) + assert.True(t, deps[0].CacheHit) +} + func TestResolveHarness_MixedHarness(t *testing.T) { agentContent := []byte("remote agent") agentHash := fetch.ComputeSHA256(agentContent) @@ -381,34 +583,27 @@ func TestResolveHarness_AuditEntries(t *testing.T) { } func TestResolveHarness_MultipleSkills(t *testing.T) { - skill1Content := []byte("skill one content") - skill1Hash := fetch.ComputeSHA256(skill1Content) - skill2Content := []byte("skill two content") - skill2Hash := fetch.ComputeSHA256(skill2Content) + fc := &forge.FakeClient{} + skill1MD := []byte("# Skill one") + skill2MD := []byte("# Skill two") - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/one.md": - w.Write(skill1Content) - case "/skills/two.md": - w.Write(skill2Content) - } - })) + skill1Hash := registerSkillDir(fc, "skills/one", map[string][]byte{"SKILL.md": skill1MD}) + skill2Hash := registerSkillDir(fc, "skills/two", map[string][]byte{"SKILL.md": skill2MD}) root := t.TempDir() h := &harness.Harness{ Agent: "/local/agents/test.md", Skills: []string{ "/local/skills/debug", - fmt.Sprintf("%s/skills/one.md#sha256=%s", srv.URL, skill1Hash), - fmt.Sprintf("%s/skills/two.md#sha256=%s", srv.URL, skill2Hash), + forgeSkillURL("skills/one", skill1Hash), + forgeSkillURL("skills/two", skill2Hash), }, - AllowedRemoteResources: []string{srv.URL + "/"}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: root, - FetchPolicy: policy, + ForgeClient: fc, }) require.NoError(t, err) require.Len(t, deps, 2) @@ -417,13 +612,14 @@ func TestResolveHarness_MultipleSkills(t *testing.T) { assert.False(t, harness.IsURL(h.Skills[1])) assert.False(t, harness.IsURL(h.Skills[2])) - got1, err := os.ReadFile(h.Skills[1]) + // Verify skills resolve to directories with SKILL.md inside. + got1, err := os.ReadFile(filepath.Join(h.Skills[1], "SKILL.md")) require.NoError(t, err) - assert.Equal(t, skill1Content, got1) + assert.Equal(t, skill1MD, got1) - got2, err := os.ReadFile(h.Skills[2]) + got2, err := os.ReadFile(filepath.Join(h.Skills[2], "SKILL.md")) require.NoError(t, err) - assert.Equal(t, skill2Content, got2) + assert.Equal(t, skill2MD, got2) } func TestResolveHarness_PolicyURL(t *testing.T) { @@ -485,48 +681,34 @@ func TestResolveHarness_EmptyFields(t *testing.T) { assert.Empty(t, deps) } -// skillFrontmatter returns SKILL.md content with the given YAML frontmatter fields -// and optional body text after the closing delimiter. -func skillFrontmatter(fields, body string) []byte { - return []byte("---\n" + fields + "---\n" + body) -} - // TestResolveHarness_TransitiveChain verifies A→B→C transitive resolution: -// all three dependencies are fetched and added to h.Skills. +// all three skill directories are fetched and added to h.Skills. func TestResolveHarness_TransitiveChain(t *testing.T) { - cContent := []byte("Skill C content — leaf node") - cHash := fetch.ComputeSHA256(cContent) - - var bContent, aContent []byte + fc := &forge.FakeClient{} - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - case "/skills/c.md": - w.Write(cContent) - } - })) + cMD := []byte("# Skill C — leaf node") + cHash := registerSkillDir(fc, "skills/c", map[string][]byte{"SKILL.md": cMD}) - cURL := fmt.Sprintf("%s/skills/c.md#sha256=%s", srv.URL, cHash) - bContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", cURL), "Skill B content") - bHash := fetch.ComputeSHA256(bContent) + bMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - c#sha256=%s\n", cHash), + "# Skill B", + ) + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A content") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) - aURL := fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash) h := &harness.Harness{ - Skills: []string{aURL}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.NoError(t, err) @@ -537,55 +719,47 @@ func TestResolveHarness_TransitiveChain(t *testing.T) { for _, d := range deps { urls[d.URL] = true } - assert.True(t, urls[srv.URL+"/skills/a.md"]) - assert.True(t, urls[srv.URL+"/skills/b.md"]) - assert.True(t, urls[srv.URL+"/skills/c.md"]) + assert.True(t, urls[forgeSkillCleanURL("skills/a")]) + assert.True(t, urls[forgeSkillCleanURL("skills/b")]) + assert.True(t, urls[forgeSkillCleanURL("skills/c")]) } // TestResolveHarness_DiamondDedup verifies that a diamond graph (A→C, B→C) resolves C // exactly once and produces no duplicate entries in deps or h.Skills. func TestResolveHarness_DiamondDedup(t *testing.T) { - cContent := []byte("Skill C content — shared dep") - cHash := fetch.ComputeSHA256(cContent) + fc := &forge.FakeClient{} - var aContent, bContent []byte - var fetchCount atomic.Int32 + cMD := []byte("# Skill C — shared dep") + cHash := registerSkillDir(fc, "skills/c", map[string][]byte{"SKILL.md": cMD}) - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - case "/skills/c.md": - fetchCount.Add(1) - w.Write(cContent) - } - })) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - c#sha256=%s\n", cHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) - cURL := fmt.Sprintf("%s/skills/c.md#sha256=%s", srv.URL, cHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", cURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) - bContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", cURL), "Skill B") - bHash := fetch.ComputeSHA256(bContent) + bMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - c#sha256=%s\n", cHash), + "# Skill B", + ) + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) h := &harness.Harness{ Skills: []string{ - fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash), - fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash), + forgeSkillURL("skills/a", aHash), + forgeSkillURL("skills/b", bHash), }, - AllowedRemoteResources: []string{srv.URL + "/"}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.NoError(t, err) - assert.Len(t, deps, 3) // C, A, B — each exactly once + assert.Len(t, deps, 3) assert.Len(t, h.Skills, 3) - assert.Equal(t, int32(1), fetchCount.Load()) // C fetched only once urls := make(map[string]bool) for _, d := range deps { @@ -595,81 +769,65 @@ func TestResolveHarness_DiamondDedup(t *testing.T) { } // TestResolveHarness_CycleDetection verifies that A→B→A is rejected with a cycle error. -// The cycle is detected via the inProgress DFS stack before any hash check on the repeat visit. func TestResolveHarness_CycleDetection(t *testing.T) { - // Use a placeholder hash for A in B's dep — cycle is detected before integrity check. - placeholderHash := strings.Repeat("a", 64) - - var aContent, bContent []byte - - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - } - })) + fc := &forge.FakeClient{} - aURL := fmt.Sprintf("%s/skills/a.md", srv.URL) + placeholderHash := strings.Repeat("a", 64) // B references A with a placeholder hash; cycle fires before hash validation. - bContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s#sha256=%s\n", aURL, placeholderHash), "Skill B") - bHash := fetch.ComputeSHA256(bContent) - - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + bMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - a#sha256=%s\n", placeholderHash), + "# Skill B", + ) + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) + + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s#sha256=%s", aURL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.Error(t, err) assert.Contains(t, err.Error(), "circular dependency") } -// TestResolveHarness_MaxDepthExceeded verifies that a chain A→B→C fails when MaxDepth=1, -// allowing one level of transitive resolution (B) but blocking the second (C). +// TestResolveHarness_MaxDepthExceeded verifies that a chain A→B→C fails when MaxDepth=1. func TestResolveHarness_MaxDepthExceeded(t *testing.T) { - cContent := []byte("Skill C — should not be reached") - cHash := fetch.ComputeSHA256(cContent) - - var aContent, bContent []byte + fc := &forge.FakeClient{} - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - case "/skills/c.md": - w.Write(cContent) - } - })) + cMD := []byte("# Skill C — should not be reached") + cHash := registerSkillDir(fc, "skills/c", map[string][]byte{"SKILL.md": cMD}) - cURL := fmt.Sprintf("%s/skills/c.md#sha256=%s", srv.URL, cHash) - bContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", cURL), "Skill B") - bHash := fetch.ComputeSHA256(bContent) + bMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - c#sha256=%s\n", cHash), + "# Skill B", + ) + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: 1, }) require.Error(t, err) @@ -677,35 +835,28 @@ func TestResolveHarness_MaxDepthExceeded(t *testing.T) { } // TestResolveHarness_MaxResourcesExceeded verifies that resolution stops when the -// resource count reaches MaxResources, returning an error on the next fetch attempt. +// resource count reaches MaxResources. func TestResolveHarness_MaxResourcesExceeded(t *testing.T) { - bContent := []byte("Skill B content") - bHash := fetch.ComputeSHA256(bContent) + fc := &forge.FakeClient{} - var aContent []byte - - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - } - })) + bMD := []byte("# Skill B") + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } // MaxResources=1: A consumes the single slot; B is rejected. _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, MaxResources: 1, }) @@ -716,33 +867,26 @@ func TestResolveHarness_MaxResourcesExceeded(t *testing.T) { // TestResolveHarness_TransitiveNotInAllowlist verifies that a transitive dep whose // URL does not match allowed_remote_resources is rejected. func TestResolveHarness_TransitiveNotInAllowlist(t *testing.T) { - bContent := []byte("Skill B content") - bHash := fetch.ComputeSHA256(bContent) + fc := &forge.FakeClient{} - var aContent []byte + bMD := []byte("# Skill B") + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - } - })) - - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - // Only /skills/a.md is allowed; /skills/b.md (the transitive dep) is not. - AllowedRemoteResources: []string{srv.URL + "/skills/a.md"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + // Only skill A's exact path is allowed; skill B (the transitive dep) is not. + AllowedRemoteResources: []string{forgeSkillCleanURL("skills/a")}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.Error(t, err) @@ -750,35 +894,29 @@ func TestResolveHarness_TransitiveNotInAllowlist(t *testing.T) { } // TestResolveHarness_TransitiveHashMismatch verifies that a transitive dep whose -// fetched content does not match the declared SHA256 hash is rejected. +// fetched content does not match the declared tree hash is rejected. func TestResolveHarness_TransitiveHashMismatch(t *testing.T) { - // Declare B with the hash of "expected content" but serve "tampered content". - expectedBContent := []byte("expected B content") - bHash := fetch.ComputeSHA256(expectedBContent) + fc := &forge.FakeClient{} - var aContent []byte + // Register B with content that doesn't match the hash A declares. + registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": []byte("tampered B content")}) - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write([]byte("tampered B content")) - } - })) - - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + // A declares B with the hash of "expected B content". + expectedBHash := fetch.ComputeTreeHash(map[string][]byte{"SKILL.md": []byte("expected B content")}) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", expectedBHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.Error(t, err) @@ -786,35 +924,28 @@ func TestResolveHarness_TransitiveHashMismatch(t *testing.T) { } // TestResolveHarness_TransitiveRelativeURL verifies that a relative dependency reference -// in skill frontmatter is resolved against the parent skill's URL via RFC 3986. +// in skill frontmatter is resolved against the parent skill's URL. func TestResolveHarness_TransitiveRelativeURL(t *testing.T) { - bContent := []byte("Skill B — resolved via relative URL") - bHash := fetch.ComputeSHA256(bContent) - - var aContent []byte + fc := &forge.FakeClient{} - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/common/b.md": - w.Write(bContent) - } - })) + bMD := []byte("# Skill B — resolved via relative URL") + bHash := registerSkillDir(fc, "common/b", map[string][]byte{"SKILL.md": bMD}) - // A is at /skills/a.md; the relative dep "../common/b.md" resolves to /common/b.md. - relDep := fmt.Sprintf("../common/b.md#sha256=%s", bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", relDep), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + // A is at skills/a; the relative dep "../common/b" resolves to common/b. + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - ../common/b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.NoError(t, err) @@ -824,46 +955,45 @@ func TestResolveHarness_TransitiveRelativeURL(t *testing.T) { for _, d := range deps { urls[d.URL] = true } - assert.True(t, urls[srv.URL+"/common/b.md"], "relative URL should resolve to /common/b.md") + assert.True(t, urls[forgeSkillCleanURL("common/b")], "relative URL should resolve to common/b") } // TestResolveHarness_ConflictingHashesForSameURL verifies that two skills declaring the -// same transitive dep URL with different SHA256 hashes is rejected. +// same transitive dep URL with different tree hashes is rejected. func TestResolveHarness_ConflictingHashesForSameURL(t *testing.T) { - dContent := []byte("Skill D content") - dHash := fetch.ComputeSHA256(dContent) + fc := &forge.FakeClient{} + + dMD := []byte("# Skill D") + dHash := registerSkillDir(fc, "skills/d", map[string][]byte{"SKILL.md": dMD}) fakeHash := strings.Repeat("b", 64) - var aContent, bContent []byte + dURL := forgeSkillCleanURL("skills/d") - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - case "/skills/d.md": - w.Write(dContent) - } - })) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - d#sha256=%s\n", dHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) + + bMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - d#sha256=%s\n", fakeHash), + "# Skill B", + ) + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - dURL := srv.URL + "/skills/d.md" - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s#sha256=%s\n", dURL, dHash), "Skill A") - aHash := fetch.ComputeSHA256(aContent) - bContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s#sha256=%s\n", dURL, fakeHash), "Skill B") - bHash := fetch.ComputeSHA256(bContent) + _ = dURL // referenced only to clarify the test setup h := &harness.Harness{ Skills: []string{ - fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash), - fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash), + forgeSkillURL("skills/a", aHash), + forgeSkillURL("skills/b", bHash), }, - AllowedRemoteResources: []string{srv.URL + "/"}, + AllowedRemoteResources: []string{testForgeBase}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.Error(t, err) @@ -871,39 +1001,41 @@ func TestResolveHarness_ConflictingHashesForSameURL(t *testing.T) { } // TestResolveHarness_SkillPolicyLeafNode verifies that a skill-level policy reference -// is fetched and recorded in deps but is NOT appended to h.Skills. +// is fetched as a single file and recorded in deps but is NOT appended to h.Skills. func TestResolveHarness_SkillPolicyLeafNode(t *testing.T) { policyContent := []byte("sandbox: strict") policyHash := fetch.ComputeSHA256(policyContent) - var aContent []byte - - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv, fetchPolicy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) case "/policies/sandbox.yaml": w.Write(policyContent) } })) + fc := &forge.FakeClient{} + policyURL := fmt.Sprintf("%s/policies/sandbox.yaml#sha256=%s", srv.URL, policyHash) - aContent = skillFrontmatter(fmt.Sprintf("policy: %s\n", policyURL), "Skill A content") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("policy: %s\n", policyURL), + "# Skill A content", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase, srv.URL + "/"}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + FetchPolicy: fetchPolicy, + ForgeClient: fc, MaxDepth: -1, }) require.NoError(t, err) assert.Len(t, deps, 2) // skill A + its policy - assert.Len(t, h.Skills, 1) // policy is NOT added to h.Skills + assert.Len(t, h.Skills, 1) depURLs := make(map[string]bool) for _, d := range deps { @@ -919,106 +1051,84 @@ func TestResolveHarness_SkillPolicyLeafNode(t *testing.T) { // TestResolveHarness_ZeroMaxDepthDisablesTransitive verifies that MaxDepth=0 prevents // any transitive dependency resolution even when skills declare dependencies. func TestResolveHarness_ZeroMaxDepthDisablesTransitive(t *testing.T) { - bContent := []byte("Skill B — must not be fetched") - bHash := fetch.ComputeSHA256(bContent) - - var aContent []byte - var bFetched atomic.Int32 + fc := &forge.FakeClient{} - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - bFetched.Add(1) - w.Write(bContent) - } - })) + bMD := []byte("# Skill B — must not be fetched") + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: 0, // disabled }) require.NoError(t, err) assert.Len(t, deps, 1) // only A assert.Len(t, h.Skills, 1) // only A - assert.Equal(t, int32(0), bFetched.Load()) // B never fetched } // TestResolveHarness_MaxDepthDefaultApplied verifies that MaxDepth<0 uses DefaultMaxDepth // and enables transitive resolution. func TestResolveHarness_MaxDepthDefaultApplied(t *testing.T) { - bContent := []byte("Skill B content") - bHash := fetch.ComputeSHA256(bContent) - - var aContent []byte + fc := &forge.FakeClient{} - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - } - })) + bMD := []byte("# Skill B") + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, // uses DefaultMaxDepth }) require.NoError(t, err) assert.Len(t, deps, 2) // A and B both resolved } -// TestResolveHarness_NonHTTPSSchemeRejected verifies that resolveURL rejects URLs whose -// scheme is not https, providing a defense-in-depth check for transitive deps from frontmatter -// that bypass the harness.IsURL guard applied to direct harness fields. +// TestResolveHarness_NonHTTPSSchemeRejected verifies that resolveSkillDirURL rejects URLs +// whose scheme is not https. func TestResolveHarness_NonHTTPSSchemeRejected(t *testing.T) { - bContent := []byte("Skill B content") - bHash := fetch.ComputeSHA256(bContent) + fc := &forge.FakeClient{} - var aContent []byte - - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - } - })) + bHash := fetch.ComputeTreeHash(map[string][]byte{"SKILL.md": []byte("# B")}) // Embed an http:// (non-HTTPS) transitive dep in A's frontmatter. - httpDepURL := fmt.Sprintf("http://example.com/skills/b.md#sha256=%s", bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", httpDepURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + httpDepURL := fmt.Sprintf("http://github.com/%s/%s/tree/%s/skills/b#sha256=%s", + testForgeOwner, testForgeRepo, testForgeRef, bHash) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - %s\n", httpDepURL), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) h := &harness.Harness{ - Skills: []string{fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash)}, - AllowedRemoteResources: []string{srv.URL + "/", "http://example.com/"}, + Skills: []string{forgeSkillURL("skills/a", aHash)}, + AllowedRemoteResources: []string{testForgeBase, "http://github.com/"}, } _, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.Error(t, err) @@ -1026,38 +1136,33 @@ func TestResolveHarness_NonHTTPSSchemeRejected(t *testing.T) { } // TestResolveHarness_DirectAndTransitiveOverlap verifies that a skill appearing both as a -// direct harness skill and as a transitive dep of another skill is deduplicated in h.Skills. +// direct harness skill and as a transitive dep of another skill is deduplicated. func TestResolveHarness_DirectAndTransitiveOverlap(t *testing.T) { - bContent := []byte("Skill B — shared skill") - bHash := fetch.ComputeSHA256(bContent) + fc := &forge.FakeClient{} - var aContent []byte + bMD := []byte("# Skill B — shared skill") + bHash := registerSkillDir(fc, "skills/b", map[string][]byte{"SKILL.md": bMD}) - srv, policy := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/skills/a.md": - w.Write(aContent) - case "/skills/b.md": - w.Write(bContent) - } - })) + aMD := skillFrontmatter( + fmt.Sprintf("dependencies:\n - b#sha256=%s\n", bHash), + "# Skill A", + ) + aHash := registerSkillDir(fc, "skills/a", map[string][]byte{"SKILL.md": aMD}) - bURL := fmt.Sprintf("%s/skills/b.md#sha256=%s", srv.URL, bHash) - aContent = skillFrontmatter(fmt.Sprintf("dependencies:\n - %s\n", bURL), "Skill A") - aHash := fetch.ComputeSHA256(aContent) + bURL := forgeSkillURL("skills/b", bHash) // Both A and B are direct harness skills; A also depends on B transitively. h := &harness.Harness{ Skills: []string{ - fmt.Sprintf("%s/skills/a.md#sha256=%s", srv.URL, aHash), + forgeSkillURL("skills/a", aHash), bURL, }, - AllowedRemoteResources: []string{srv.URL + "/"}, + AllowedRemoteResources: []string{testForgeBase}, } deps, err := ResolveHarness(context.Background(), h, ResolveOpts{ WorkspaceRoot: t.TempDir(), - FetchPolicy: policy, + ForgeClient: fc, MaxDepth: -1, }) require.NoError(t, err) @@ -1071,3 +1176,19 @@ func TestResolveHarness_DirectAndTransitiveOverlap(t *testing.T) { seen[s] = true } } + +// TestResolveHarness_NilForgeClientWithSkillURL verifies that a skill URL without +// a ForgeClient produces a clear error. +func TestResolveHarness_NilForgeClientWithSkillURL(t *testing.T) { + fakeHash := strings.Repeat("a", 64) + h := &harness.Harness{ + Skills: []string{forgeSkillURL("skills/test", fakeHash)}, + AllowedRemoteResources: []string{testForgeBase}, + } + + _, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: t.TempDir(), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "ForgeClient is required") +}