From adad3007bcb08303eecb99a1c8d7fd7c24520a3f Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 02:46:29 +0000 Subject: [PATCH 1/3] Add SkillServer native sub-agent sync --- Directory.Packages.props | 2 +- Netclaw.slnx | 1 + docs/runbooks/subagents.md | 17 +- .../.openspec.yaml | 2 + .../skillserver-native-sidecar-sync/design.md | 141 +++++++ .../proposal.md | 69 ++++ .../specs/netclaw-subagents/spec.md | 49 +++ .../skillserver-native-sidecar-sync/spec.md | 117 ++++++ .../skillserver-native-sidecar-sync/tasks.md | 63 ++++ .../FileSubAgentDefinitionLoaderTests.cs | 134 +++++++ .../FileSubAgentDefinitionLoader.cs | 95 ++++- src/Netclaw.Configuration/NetclawPaths.cs | 8 + .../Netclaw.Daemon.IntegrationTests.csproj | 26 ++ ...killServerNativeSidecarIntegrationTests.cs | 203 ++++++++++ .../ServerFeedSkillSyncServiceTests.cs | 299 ++++++++++++++- .../Services/SkillSyncHelpersTests.cs | 44 +++ src/Netclaw.Daemon/Properties/AssemblyInfo.cs | 1 + .../Services/ServerFeedSkillSyncService.cs | 349 +++++++++++++++--- .../Services/SkillSyncHelpers.cs | 106 ++++++ src/Netclaw.Tests.Utilities/AssemblyInfo.cs | 1 + 20 files changed, 1664 insertions(+), 63 deletions(-) create mode 100644 openspec/changes/skillserver-native-sidecar-sync/.openspec.yaml create mode 100644 openspec/changes/skillserver-native-sidecar-sync/design.md create mode 100644 openspec/changes/skillserver-native-sidecar-sync/proposal.md create mode 100644 openspec/changes/skillserver-native-sidecar-sync/specs/netclaw-subagents/spec.md create mode 100644 openspec/changes/skillserver-native-sidecar-sync/specs/skillserver-native-sidecar-sync/spec.md create mode 100644 openspec/changes/skillserver-native-sidecar-sync/tasks.md create mode 100644 src/Netclaw.Daemon.IntegrationTests/Netclaw.Daemon.IntegrationTests.csproj create mode 100644 src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index e67d7603a..62083b5f2 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -63,7 +63,7 @@ - + diff --git a/Netclaw.slnx b/Netclaw.slnx index 9263541f5..a7cf41e75 100644 --- a/Netclaw.slnx +++ b/Netclaw.slnx @@ -15,6 +15,7 @@ + diff --git a/docs/runbooks/subagents.md b/docs/runbooks/subagents.md index 3ae7f7188..d29f54063 100644 --- a/docs/runbooks/subagents.md +++ b/docs/runbooks/subagents.md @@ -150,6 +150,13 @@ skill system uses and the de facto format used by Claude Code and OpenCode. One file per agent. No JSON sidecar. The filename is a convenience for humans; the authoritative agent name comes from the `name` field in the frontmatter. +SkillServer feed sync can also install managed subagent definitions under +`~/.netclaw/agents/.server-feeds//.md`. Those files are +owned by the server-feed sync process: edit local user-authored agents in the +top-level `~/.netclaw/agents/*.md` namespace instead. If a top-level local agent +and a managed feed agent declare the same `name`, the local definition wins and +the managed one is skipped with a warning. + ### Frontmatter fields ```markdown @@ -194,15 +201,15 @@ written. ### Loader behavior (fail loud) On the next turn or subagent lookup, `FileSubAgentDefinitionLoader` rescans -`~/.netclaw/agents/*.md` and logs a specific warning for every file it rejects. -A rejection does not stop the scan — other valid files in the same directory -still load. Rejection -reasons: +top-level `~/.netclaw/agents/*.md` files first, then managed server-feed files +under `~/.netclaw/agents/.server-feeds/*/*.md`. It logs a specific warning for +every file it rejects. A rejection does not stop the scan — other valid files in +the same directory still load. Rejection reasons: - Missing or unparseable YAML frontmatter - Missing required field (`name` or `description`) - Empty body (system prompt) -- Duplicate `name` across files (the alphabetically-first file wins) +- Duplicate `name` across files (top-level local files win over managed feed files; managed feed duplicates use configured feed order) Non-`.md` files in the agents directory (`stray.json`, `README.txt`, etc.) are ignored at the glob layer and never logged. diff --git a/openspec/changes/skillserver-native-sidecar-sync/.openspec.yaml b/openspec/changes/skillserver-native-sidecar-sync/.openspec.yaml new file mode 100644 index 000000000..d6b53dee5 --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-30 diff --git a/openspec/changes/skillserver-native-sidecar-sync/design.md b/openspec/changes/skillserver-native-sidecar-sync/design.md new file mode 100644 index 000000000..48efd1dd6 --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/design.md @@ -0,0 +1,141 @@ +## Context + +NetClaw already syncs private SkillServer feeds through `ServerFeedSkillSyncService` using the Cloudflare Agent Skills RFC index. That path is intentionally skill-only and writes managed skills under `~/.netclaw/skills/.server-feeds//` before rebuilding the skill registry. + +SkillServer now exposes a native manifest sidecar at `/manifest.json` with native resource traversal and artifact download APIs. The native sidecar is needed for sub-agents because the RFC skill feed cannot represent them. NetClaw's current sub-agent loader only scans top-level `~/.netclaw/agents/*.md`, so a managed server-feed namespace must be introduced without letting feeds overwrite operator-authored local agents. + +Actor boundary note: this change stays in daemon background sync and configuration loading. It updates files on disk and the in-memory sub-agent registry, but it does not add new actor messages, session journal events, or persistence schema. Sub-agent execution behavior remains under the existing `SubAgentActor` contract after definitions are loaded. + +## Goals / Non-Goals + +**Goals:** + +- Preserve RFC skill sync as the primary and authoritative skill path. +- Feature-detect native `/manifest.json` per configured SkillServer feed. +- Sync native `agent-md` sub-agent artifacts into a NetClaw-owned managed namespace. +- Verify SHA-256 digests before writing managed sub-agent files. +- Keep local user-authored sub-agents authoritative on name conflicts. +- Prune only managed server-feed sub-agents after a confirmed successful native sync. +- Keep previous managed files during native sidecar outages, malformed responses, timeouts, or verification failures. + +**Non-Goals:** + +- Replace RFC skill sync with native skill sync. +- Add native sync for non-sub-agent resources in this MVP. +- Add new feed configuration knobs unless implementation proves they are required. +- Let SkillServer manifest data prescribe local filesystem paths. +- Add manifest signature verification. + +## Decisions + +### D1. RFC index fetch remains the feed reachability gate + +For each enabled feed, NetClaw first fetches the RFC skill index with the existing client path. If that fetch times out or fails, the service skips both skill updates and native sidecar sync for that feed. If the RFC index fetch succeeds, even with zero skills, NetClaw may attempt optional native sidecar detection. + +Rationale: + +- Preserves the current RFC-first mental model and failure behavior. +- Avoids treating native manifest success as a replacement for RFC skill sync. +- Still allows sub-agent-only feeds when the server is reachable and the RFC endpoint responds with an empty index. + +Alternative considered: + +- Fetch native sidecar even when RFC fetch fails. Rejected because it creates two competing feed reachability models and makes pruning safety harder to reason about during partial outages. + +### D2. Native sidecar sync is fail-soft and optional + +Missing `/manifest.json`, 404s, malformed native manifests, unsupported native manifest shapes, and native traversal failures are logged and treated as sidecar sync failures only. The existing RFC skill sync result remains valid, and existing managed sub-agent files are left untouched. + +Rationale: + +- Existing SkillServer feeds and non-SkillServer RFC feeds should continue to work unchanged. +- Native sidecar deployment can roll out independently from NetClaw client support. + +Alternative considered: + +- Fail the entire feed sync when native sidecar sync fails. Rejected because sub-agent distribution is additive and should not break skill updates. + +### D3. NetClaw owns all managed local paths + +Server-synced sub-agents are written under `~/.netclaw/agents/.server-feeds//.md`. NetClaw derives the filename from the logical sub-agent name after validating it is a safe file segment. Manifest-provided paths are ignored for local storage. + +Rationale: + +- Prevents path traversal and server-controlled writes outside the managed namespace. +- Mirrors the existing managed server-feed skill namespace. +- Makes pruning scope precise. + +Alternative considered: + +- Allow the native manifest to carry local target paths. Rejected because it gives remote feed content too much authority over the operator's filesystem. + +### D4. Downloaded sub-agent files must verify and self-identify + +NetClaw downloads the native `agent-md` artifact for each advertised sub-agent, verifies the expected SHA-256 digest, parses the markdown frontmatter, and requires the frontmatter `name` to match the advertised manifest name before replacing the managed file. + +Rationale: + +- Digest verification protects against corrupted or wrong artifacts. +- Frontmatter identity validation prevents a feed from advertising one agent name while delivering another. +- Reusing the existing markdown parser keeps format behavior aligned with local sub-agent authoring. + +Alternative considered: + +- Trust manifest metadata without parsing the downloaded file before write. Rejected because the runtime loads the markdown file, so the file's own frontmatter is the authoritative execution input. + +### D5. Local user-authored agents take precedence over managed feed agents + +The loader scans top-level `~/.netclaw/agents/*.md` as user-owned definitions first, then scans managed server-feed directories. If a managed feed agent has the same logical name as a local user-owned definition, the local definition is registered and the managed one is skipped with a diagnostic. If multiple managed feeds publish the same name, the configured feed order determines the winner and later duplicates are skipped with diagnostics. + +Rationale: + +- Protects operator intent and local customization. +- Keeps conflict behavior deterministic. +- Keeps managed feed files available on disk for audit and future conflict resolution without exposing the shadowed definition at runtime. + +Alternative considered: + +- Let the most recently synced managed feed override local files. Rejected because it would make remote feeds capable of changing local operator behavior unexpectedly. + +### D6. Pruning is a post-success managed-only operation + +Each feed tracks its managed sub-agent sync state separately from user-authored files. After native sidecar traversal and all advertised sub-agent artifact operations for that feed complete successfully, NetClaw prunes managed files and state entries no longer advertised by that feed. If any native sub-agent download, verification, parse, or write fails, the sync is partial and no managed sub-agent pruning occurs for that feed. + +Rationale: + +- Prevents transient partial failures from deleting still-useful managed agents. +- Keeps destructive behavior confined to the feed-owned managed namespace. + +Alternative considered: + +- Prune based on whatever subset was successfully downloaded. Rejected because one failed artifact could incorrectly remove other managed agents during an outage or server bug. + +## Risks / Trade-offs + +- [Risk] The existing sub-agent loader fingerprint only covers top-level files. Mitigation: include managed feed files in the fingerprint so runtime refresh sees server-synced changes. +- [Risk] Managed feed conflicts can be confusing when local definitions win. Mitigation: emit explicit diagnostics with local path, feed name, and shadowed managed path. +- [Risk] Feed names may contain unsafe path characters. Mitigation: reuse existing feed directory behavior only if safe; otherwise add a shared safe-segment helper before writing managed agent paths. +- [Risk] Native sidecar sync adds network calls to startup feed sync. Mitigation: reuse per-feed timeout bounds and keep sidecar failures fail-soft. +- [Risk] Partial native sync may write some updated agents while retaining stale ones and skipping prune. Mitigation: log partial sync status and retry on the next scheduled sync. + +## Migration Plan + +1. Upgrade NetClaw's `Netclaw.SkillClient` dependency to the published prerelease containing native manifest APIs. +2. Add NetClaw path helpers for managed server-feed sub-agent directories and sync-state path. +3. Extend `ServerFeedSkillSyncService` with optional native sidecar discovery after successful RFC index fetch. +4. Implement verified native sub-agent download, validation, atomic managed-file replacement, state updates, and safe pruning. +5. Extend `FileSubAgentDefinitionLoader` to scan local top-level agents first and managed feed agents second with deterministic conflict diagnostics. +6. Add targeted tests for sidecar absence, sidecar success, digest failure, partial sync no-prune, local precedence, and managed prune behavior. +7. Update docs if operator-facing feed/sub-agent sync behavior needs to be documented. + +Rollback: + +- Disable or remove the native sidecar branch from `ServerFeedSkillSyncService`; existing RFC skill sync remains intact. +- Managed sub-agent files under `~/.netclaw/agents/.server-feeds/` can remain on disk but will no longer be refreshed or loaded if the loader change is also reverted. +- No journal or database rollback is required. + +## Open Questions + +- Should shadowed managed sub-agents appear in diagnostic tooling beyond daemon logs? +- Should stale managed sub-agent files be kept for audit instead of deleted when pruned? +- Should `SkillSync.Enabled = false` also disable managed sub-agent server-feed loading, even if files already exist on disk? diff --git a/openspec/changes/skillserver-native-sidecar-sync/proposal.md b/openspec/changes/skillserver-native-sidecar-sync/proposal.md new file mode 100644 index 000000000..bf48d037d --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/proposal.md @@ -0,0 +1,69 @@ +## Why + +SkillServer now exposes native manifest endpoints for resources that the Cloudflare Agent Skills RFC feed cannot represent, especially sub-agents. NetClaw currently consumes only the RFC skill index, so private SkillServer feeds can distribute skills but cannot safely distribute companion sub-agent definitions needed by `metadata.subagent` routing. + +## What Changes + +- Keep RFC skill sync as the authoritative primary path for skills. +- Feature-detect each configured SkillServer feed's optional native `/manifest.json` sidecar after RFC sync remains available. +- Sync native sub-agent definitions from the sidecar into a feed-owned managed namespace under `~/.netclaw/agents/.server-feeds//`. +- Verify downloaded sub-agent artifacts by SHA-256 before writing them to disk. +- Preserve existing local user-authored sub-agent files and give them precedence on name conflicts. +- Prune only managed server-synced sub-agent files, and only after a confirmed successful native sidecar sync. +- Keep previous managed sub-agent files when the native sidecar is unavailable, malformed, times out, or fails artifact verification. + +## Capabilities + +### New Capabilities + +- `skillserver-native-sidecar-sync`: Defines optional native manifest sidecar discovery, native-only resource sync, managed storage, digest verification, and safe pruning semantics for SkillServer feeds. + +### Modified Capabilities + +- `netclaw-subagents`: Add managed server-feed sub-agent discovery, local-user precedence, and conflict diagnostics to the sub-agent loading contract. + +## Impact + +### Affected code and systems + +- `ServerFeedSkillSyncService` will continue using the RFC index for skills and add optional native manifest traversal for sub-agents. +- `Netclaw.SkillClient` package consumption will move to the prerelease client version that contains native manifest and sub-agent artifact APIs. +- `NetclawPaths` will need a managed sub-agent feed namespace alongside the existing user-authored `AgentsDirectory`. +- `FileSubAgentDefinitionLoader` and `SubAgentDefinitionRegistry` will need deterministic loading and conflict handling across user-authored and managed feed files. +- Daemon tests will need coverage for native sidecar success, unavailable sidecar fallback, digest failure, user precedence, and prune safety. + +### APIs and behavior + +- No public user-facing CLI or config breaking change is intended. +- Existing SkillServer feeds without `/manifest.json` continue to sync skills exactly as today. +- Existing local sub-agent files remain user-owned and are never overwritten or deleted by server-feed sync. + +### Security and operational impact + +- Server manifest metadata SHALL NOT prescribe local filesystem paths; NetClaw maps names to its own managed namespace. +- Artifact digests are verified before writes; failed verification keeps the previous managed file, if any. +- Sub-agent sync is fail-soft relative to skill sync so a native sidecar outage cannot remove existing skills or agents. +- Conflict diagnostics must make it clear when a server-managed sub-agent is shadowed by a local user-authored definition. + +### In scope for MVP + +- Optional `/manifest.json` feature detection. +- Native sub-agent traversal and `agent-md` artifact download. +- SHA-256 verification and atomic managed-file replacement. +- Managed sub-agent load support and local precedence. +- Safe managed sub-agent pruning after successful sync. +- Unit tests and targeted daemon/configuration tests for the new sync behavior. + +### Out of scope for MVP + +- Replacing RFC skill sync with native skill sync. +- Syncing non-sub-agent native resources. +- Letting SkillServer choose NetClaw local paths. +- Overwriting or deleting user-authored local sub-agents. +- Signature verification for native manifests beyond the existing digest verification requirement. + +### Source PRDs + +- `PRD-001` (MVP runtime determinism and reliability) +- `PRD-002` (security envelope and fail-closed/default-deny posture) +- `PRD-004` (operator configuration and local filesystem ownership) diff --git a/openspec/changes/skillserver-native-sidecar-sync/specs/netclaw-subagents/spec.md b/openspec/changes/skillserver-native-sidecar-sync/specs/netclaw-subagents/spec.md new file mode 100644 index 000000000..effc35413 --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/specs/netclaw-subagents/spec.md @@ -0,0 +1,49 @@ +## ADDED Requirements + +### Requirement: Managed server-feed sub-agent discovery + +The system SHALL load sub-agent definitions from user-authored top-level files under `~/.netclaw/agents/*.md` and from managed server-feed files under `~/.netclaw/agents/.server-feeds//*.md`. User-authored top-level sub-agents SHALL take precedence over managed server-feed sub-agents with the same logical name. Shadowed managed sub-agents SHALL NOT be exposed through sub-agent discovery, `spawn_agent`, or routed skill execution. + +#### Scenario: Managed sub-agent is loaded when no local conflict exists + +- **GIVEN** `~/.netclaw/agents/.server-feeds/team/code-reviewer.md` declares `name: code-reviewer` +- **AND** no top-level local sub-agent declares `name: code-reviewer` +- **WHEN** the sub-agent loader refreshes definitions +- **THEN** `code-reviewer` is registered as an available sub-agent according to its frontmatter visibility + +#### Scenario: Local sub-agent shadows managed sub-agent + +- **GIVEN** `~/.netclaw/agents/code-reviewer.md` declares `name: code-reviewer` +- **AND** `~/.netclaw/agents/.server-feeds/team/code-reviewer.md` also declares `name: code-reviewer` +- **WHEN** the sub-agent loader refreshes definitions +- **THEN** the top-level local `code-reviewer` definition is registered +- **AND** the managed feed `code-reviewer` definition is skipped +- **AND** NetClaw emits a diagnostic identifying the shadowed managed definition + +#### Scenario: Shadowed managed sub-agent cannot be spawned by routed skill + +- **GIVEN** a top-level local sub-agent shadows a managed server-feed sub-agent with the same name +- **WHEN** a skill routes execution through `metadata.subagent` using that name +- **THEN** routed execution resolves to the registered local sub-agent definition +- **AND** the shadowed managed definition is not used + +#### Scenario: Managed feed conflicts are deterministic + +- **GIVEN** two configured server feeds both provide a managed sub-agent named `reviewer` +- **WHEN** the sub-agent loader refreshes definitions +- **THEN** NetClaw registers only one `reviewer` definition using deterministic configured feed order +- **AND** skips later managed duplicates with diagnostics + +#### Scenario: Managed file changes refresh the registry + +- **GIVEN** a managed sub-agent file changes under `~/.netclaw/agents/.server-feeds/team/` +- **WHEN** the sub-agent loader checks for changes +- **THEN** the loader detects the managed file change +- **AND** refreshes the sub-agent registry snapshot + +#### Scenario: Missing managed namespace does not block local loading + +- **GIVEN** `~/.netclaw/agents/.server-feeds/` does not exist +- **AND** top-level local sub-agent files exist under `~/.netclaw/agents/` +- **WHEN** the sub-agent loader refreshes definitions +- **THEN** local sub-agent loading continues without requiring the managed namespace to exist diff --git a/openspec/changes/skillserver-native-sidecar-sync/specs/skillserver-native-sidecar-sync/spec.md b/openspec/changes/skillserver-native-sidecar-sync/specs/skillserver-native-sidecar-sync/spec.md new file mode 100644 index 000000000..eb1ee4d94 --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/specs/skillserver-native-sidecar-sync/spec.md @@ -0,0 +1,117 @@ +## ADDED Requirements + +### Requirement: RFC skill sync remains primary with optional native sidecar + +For each enabled SkillServer feed, the system SHALL keep using the Cloudflare Agent Skills RFC index as the primary skill sync source. After a successful RFC index fetch for a feed, including a successful empty index, the system MAY feature-detect that feed's native `/manifest.json` sidecar for native-only resources. Native sidecar absence or failure SHALL NOT cause RFC skill sync for that feed to fail. + +#### Scenario: Feed without native sidecar still syncs skills + +- **GIVEN** an enabled server feed exposes a valid RFC skill index +- **AND** the feed does not expose `/manifest.json` +- **WHEN** server feed sync runs +- **THEN** NetClaw syncs skills from the RFC index using existing behavior +- **AND** logs or records that native sidecar sync was unavailable +- **AND** leaves existing managed sub-agent files for that feed unchanged + +#### Scenario: RFC fetch failure skips native sidecar sync + +- **GIVEN** an enabled server feed times out or fails while fetching the RFC skill index +- **WHEN** server feed sync runs +- **THEN** NetClaw does not attempt native sidecar sync for that feed +- **AND** keeps existing on-disk skills and managed sub-agents for that feed + +#### Scenario: Empty RFC index can still use native sidecar + +- **GIVEN** an enabled server feed returns a successful RFC index with zero skills +- **AND** the feed exposes a valid native sidecar with sub-agents +- **WHEN** server feed sync runs +- **THEN** NetClaw does not create or prune RFC skills for that empty index beyond existing safe behavior +- **AND** may sync native sub-agents from the sidecar + +### Requirement: Native sub-agent artifacts sync to a managed namespace + +When a feed's native sidecar advertises sub-agent resources, the system SHALL traverse native sub-agent versions, select the `agent-md` artifact, download it, verify its SHA-256 digest, and write it only to a NetClaw-owned managed path under `~/.netclaw/agents/.server-feeds//.md`. Server-provided manifest metadata SHALL NOT control local filesystem paths. + +#### Scenario: Verified sub-agent artifact is written atomically + +- **GIVEN** a native sidecar advertises sub-agent `code-reviewer` with an `agent-md` artifact and expected SHA-256 digest +- **AND** the downloaded artifact content hashes to the expected digest +- **AND** the artifact frontmatter declares `name: code-reviewer` +- **WHEN** native sidecar sync processes the sub-agent +- **THEN** NetClaw writes the file to `~/.netclaw/agents/.server-feeds//code-reviewer.md` +- **AND** replaces any previous managed file atomically +- **AND** records sync state for the managed sub-agent + +#### Scenario: Server path metadata is ignored + +- **GIVEN** a native sidecar artifact includes metadata that resembles an absolute path or relative traversal path +- **WHEN** native sidecar sync processes the artifact +- **THEN** NetClaw ignores that metadata for local storage +- **AND** derives the managed target path only from the configured feed name and validated sub-agent name + +#### Scenario: Digest mismatch keeps previous managed file + +- **GIVEN** a native sidecar advertises sub-agent `code-reviewer` +- **AND** a previous managed file already exists for `code-reviewer` +- **WHEN** the downloaded artifact hash does not match the expected SHA-256 digest +- **THEN** NetClaw rejects the downloaded artifact +- **AND** keeps the previous managed file unchanged +- **AND** treats the feed's native sidecar sync as partial for pruning purposes + +#### Scenario: Artifact name mismatch is rejected + +- **GIVEN** a native sidecar advertises sub-agent `code-reviewer` +- **AND** the downloaded artifact frontmatter declares `name: other-agent` +- **WHEN** native sidecar sync processes the artifact +- **THEN** NetClaw rejects the downloaded artifact +- **AND** does not replace the managed `code-reviewer.md` file + +### Requirement: Native sidecar failure preserves managed sub-agents + +The system SHALL treat native sidecar failures as non-destructive. If native manifest fetch, traversal, artifact download, digest verification, parsing, validation, or managed write fails for a feed, NetClaw SHALL keep existing managed sub-agent files for that feed and SHALL NOT prune removed sub-agents during that sync attempt. + +#### Scenario: Malformed native manifest does not prune managed sub-agents + +- **GIVEN** managed sub-agent files already exist for a feed +- **WHEN** `/manifest.json` is present but malformed or unsupported +- **THEN** NetClaw logs the native sidecar failure +- **AND** leaves all managed sub-agent files for that feed unchanged +- **AND** does not prune managed sub-agent sync state for that feed + +#### Scenario: One failed artifact prevents pruning + +- **GIVEN** a native sidecar advertises sub-agents `alpha` and `beta` +- **AND** `alpha` downloads and verifies successfully +- **AND** `beta` fails download or verification +- **WHEN** native sidecar sync completes for the feed +- **THEN** NetClaw may keep the successfully synced `alpha` managed file +- **AND** keeps any previous managed `beta` file unchanged +- **AND** skips pruning for that feed because the sync was partial + +### Requirement: Managed sub-agent pruning is successful-sync only + +After a native sidecar sync for a feed completes successfully for all advertised sub-agents, the system SHALL remove only managed sub-agent files and sync-state entries for that same feed that are no longer advertised by the sidecar. The system SHALL NOT remove user-authored sub-agents or managed sub-agents belonging to other feeds. + +#### Scenario: Removed managed sub-agent is pruned after successful sync + +- **GIVEN** the managed namespace for feed `team` contains `old-agent.md` from a previous successful sync +- **AND** the feed's current native sidecar successfully syncs all advertised sub-agents +- **AND** the sidecar no longer advertises `old-agent` +- **WHEN** native sidecar sync completes +- **THEN** NetClaw removes `~/.netclaw/agents/.server-feeds/team/old-agent.md` +- **AND** removes `old-agent` from that feed's managed sub-agent sync state + +#### Scenario: User-authored local sub-agent is never pruned + +- **GIVEN** `~/.netclaw/agents/code-reviewer.md` exists as a user-authored local sub-agent +- **AND** a native sidecar sync for feed `team` completes successfully +- **WHEN** NetClaw prunes removed managed sub-agents for feed `team` +- **THEN** `~/.netclaw/agents/code-reviewer.md` remains unchanged +- **AND** pruning is limited to `~/.netclaw/agents/.server-feeds/team/` + +#### Scenario: Other feed managed sub-agent is never pruned + +- **GIVEN** `~/.netclaw/agents/.server-feeds/team-a/reviewer.md` exists +- **AND** `~/.netclaw/agents/.server-feeds/team-b/reviewer.md` exists +- **WHEN** native sidecar sync for feed `team-a` completes and prunes removed entries +- **THEN** NetClaw does not remove or modify files under `~/.netclaw/agents/.server-feeds/team-b/` diff --git a/openspec/changes/skillserver-native-sidecar-sync/tasks.md b/openspec/changes/skillserver-native-sidecar-sync/tasks.md new file mode 100644 index 000000000..3b97b6898 --- /dev/null +++ b/openspec/changes/skillserver-native-sidecar-sync/tasks.md @@ -0,0 +1,63 @@ +## 1. Dependency and API readiness + +- [x] 1.1 Confirm SkillServer prerelease `0.4.0-beta.1` is published with native manifest and sub-agent artifact APIs. +- [x] 1.2 Update NetClaw package references to consume `Netclaw.SkillClient` `0.4.0-beta.1` or the final approved version for implementation. +- [x] 1.3 Confirm SkillClient API names for native manifest fetch, sub-agent traversal, `agent-md` selection, and verified artifact download. +- [x] 1.4 Keep RFC skill sync behavior unchanged before adding native sidecar logic. + +## 2. Managed path and state model + +- [x] 2.1 Add `NetclawPaths` helpers for `~/.netclaw/agents/.server-feeds/`, per-feed managed agent directories, and per-feed managed agent sync state. +- [x] 2.2 Add or reuse sync-state models for managed sub-agent name, version, SHA-256, and sync timestamp tracking. +- [x] 2.3 Add safe segment validation for feed names and sub-agent filenames before writing managed agent paths. +- [x] 2.4 Add atomic single-file replace helper for managed sub-agent writes. + +## 3. Native sidecar sync implementation + +- [x] 3.1 Refactor `ServerFeedSkillSyncService.SyncFeedAsync` so a successful RFC fetch with an empty index can still proceed to optional native sidecar sync. +- [x] 3.2 Add optional `/manifest.json` feature detection using `SkillServerClient` native manifest APIs after successful RFC index fetch. +- [x] 3.3 Treat missing, unavailable, malformed, or unsupported native sidecars as fail-soft outcomes that preserve existing managed sub-agents. +- [x] 3.4 Traverse native sub-agent resources and select the `agent-md` artifact for each advertised sub-agent version. +- [x] 3.5 Download each `agent-md` artifact with feed timeout and API key behavior consistent with RFC skill downloads. +- [x] 3.6 Verify SHA-256 digest before parsing or writing the downloaded artifact. +- [x] 3.7 Parse downloaded sub-agent frontmatter and require `name` to match the advertised manifest sub-agent name. +- [x] 3.8 Write verified sub-agents only under the managed per-feed namespace and update per-feed sub-agent sync state. +- [x] 3.9 Skip managed sub-agent pruning for any partial native sidecar sync failure. +- [x] 3.10 Prune only stale managed sub-agent files and state entries after a fully successful native sidecar sync for that feed. + +## 4. Sub-agent loader and conflict behavior + +- [x] 4.1 Extend `FileSubAgentDefinitionLoader` to include managed server-feed files in its fingerprint and load snapshot. +- [x] 4.2 Load user-authored top-level `~/.netclaw/agents/*.md` files before managed server-feed files. +- [x] 4.3 Preserve local user-authored precedence when a managed feed sub-agent has the same logical name. +- [x] 4.4 Implement deterministic managed-feed duplicate handling using configured feed order or another documented stable order. +- [x] 4.5 Emit diagnostics for shadowed managed sub-agents without exposing shadowed definitions through discovery, `spawn_agent`, or routed skill execution. + +## 5. Tests + +- [x] 5.1 Add daemon sync tests for feed without native sidecar preserving RFC skill sync behavior. +- [x] 5.2 Add daemon sync tests for successful native sub-agent download, digest verification, identity validation, managed write, and state update. +- [x] 5.3 Add daemon sync tests for native sidecar fetch failure, malformed manifest, digest mismatch, and artifact name mismatch preserving previous managed files. +- [x] 5.4 Add daemon sync tests proving partial native sidecar sync skips pruning. +- [x] 5.5 Add daemon sync tests proving successful native sidecar sync prunes only stale files in the same managed feed namespace. +- [x] 5.6 Add configuration loader tests proving managed server-feed agents load when no local conflict exists. +- [x] 5.7 Add configuration loader tests proving local user-authored agents shadow managed feed agents and routed lookups resolve to the local definition. +- [x] 5.8 Add configuration loader tests proving managed feed duplicate handling is deterministic and diagnostic. + +## 6. Documentation and verification + +- [x] 6.1 Update operator/developer documentation for SkillServer feeds and managed sub-agent sync if an appropriate docs page exists. +- [x] 6.2 Run targeted NetClaw daemon/configuration tests for skill sync and sub-agent loading. +- [x] 6.3 Run `openspec validate "skillserver-native-sidecar-sync"` and fix any artifact or spec issues. +- [x] 6.4 Run `dotnet build -c Release` after implementation. +- [x] 6.5 Run `dotnet test -c Release` or the agreed targeted subset plus any required full-suite follow-up. +- [x] 6.6 Run `dotnet slopwatch analyze` and resolve new violations. + +## 7. Docker-backed integration spike + +- [x] 7.1 Add or run a Testcontainers-based spike that starts a real `ghcr.io/netclaw-dev/skillserver:0.4.0-beta.1` container. +- [x] 7.2 Seed the real SkillServer instance through its HTTP API or CLI with a real skill and real sub-agent artifact. +- [x] 7.3 Configure NetClaw server-feed sync against the container endpoint and verify the RFC skill path plus native sub-agent sidecar path end-to-end. +- [x] 7.4 Verify managed sub-agent files land under `~/.netclaw/agents/.server-feeds//` and local user-authored sub-agents still win conflicts. +- [x] 7.5 Confirmed the spike does not exercise actual sub-agent execution, so no inference call to `https://spark2.testlab.petabridge.net/` was required. +- [x] 7.6 Keep the Docker-backed spike self-skipping or opt-in on hosts without Docker or required inference credentials. diff --git a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs index dd7167119..5d70fde21 100644 --- a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs +++ b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs @@ -37,6 +37,15 @@ private string WriteAgent(string fileName, string content) return path; } + private string WriteManagedAgent(string feedName, string fileName, string content) + { + var dir = _paths.ServerFeedAgentDirectory(feedName); + Directory.CreateDirectory(dir); + var path = Path.Combine(dir, fileName); + File.WriteAllText(path, content); + return path; + } + [Fact] public void LoadAll_logs_warning_when_agents_directory_is_missing() { @@ -433,6 +442,131 @@ public void RefreshIfChanged_detects_deletes_and_returns_empty_snapshot() Assert.Empty(refreshed); } + [Fact] + public void LoadAll_loads_managed_server_feed_agents_when_no_local_conflict_exists() + { + WriteManagedAgent("team", "code-reviewer.md", """ + --- + name: code-reviewer + description: Managed reviewer + tools: [file_read] + --- + + Managed body. + """); + + var results = _loader.LoadAll(); + + var profile = Assert.Single(results); + Assert.Equal("code-reviewer", profile.Name); + Assert.Equal("Managed reviewer", profile.Description); + Assert.Contains("Managed body.", profile.SystemPrompt); + } + + [Fact] + public void LoadAll_local_agent_shadows_managed_server_feed_agent() + { + WriteAgent("code-reviewer.md", """ + --- + name: code-reviewer + description: Local reviewer + tools: [file_read] + --- + + Local body. + """); + WriteManagedAgent("team", "code-reviewer.md", """ + --- + name: code-reviewer + description: Managed reviewer + tools: [web_search] + --- + + Managed body. + """); + + var results = _loader.LoadAll(); + + var profile = Assert.Single(results); + Assert.Equal("Local reviewer", profile.Description); + Assert.Contains("Local body.", profile.SystemPrompt); + Assert.Contains(_logger.Warnings, w => + w.Contains(".server-feeds", StringComparison.Ordinal) + && w.Contains("duplicate name", StringComparison.OrdinalIgnoreCase) + && w.Contains("code-reviewer", StringComparison.Ordinal)); + } + + [Fact] + public void LoadAll_uses_configured_feed_order_for_managed_duplicates() + { + WriteManagedAgent("team-b", "reviewer.md", """ + --- + name: reviewer + description: Team B reviewer + --- + + Team B body. + """); + WriteManagedAgent("team-a", "reviewer.md", """ + --- + name: reviewer + description: Team A reviewer + --- + + Team A body. + """); + + var feedsConfig = new SkillFeedsConfig + { + Feeds = + [ + new SkillFeedSource { Name = "team-b" }, + new SkillFeedSource { Name = "team-a" } + ] + }; + var logger = new ListLogger(); + var loader = new FileSubAgentDefinitionLoader(_paths, logger, feedsConfig); + + var results = loader.LoadAll(); + + var profile = Assert.Single(results); + Assert.Equal("Team B reviewer", profile.Description); + Assert.Contains(logger.Warnings, w => + w.Contains("team-a", StringComparison.Ordinal) + && w.Contains("duplicate name", StringComparison.OrdinalIgnoreCase) + && w.Contains("reviewer", StringComparison.Ordinal)); + } + + [Fact] + public void RefreshIfChanged_detects_managed_server_feed_agent_edits() + { + var path = WriteManagedAgent("team", "managed.md", """ + --- + name: managed + description: First managed description + --- + + First managed body. + """); + + var first = _loader.LoadAll(); + Assert.Equal("First managed description", Assert.Single(first).Description); + + File.WriteAllText(path, """ + --- + name: managed + description: Updated managed description + --- + + Updated managed body. + """); + + Assert.True(_loader.RefreshIfChanged(out var refreshed)); + var updated = Assert.Single(refreshed); + Assert.Equal("Updated managed description", updated.Description); + Assert.Contains("Updated managed body.", updated.SystemPrompt); + } + [Fact] public void SyncInto_replaces_registry_profiles_when_disk_changes() { diff --git a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs index 6db9845b9..7ea90b312 100644 --- a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs +++ b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs @@ -17,15 +17,23 @@ namespace Netclaw.Configuration; public sealed class FileSubAgentDefinitionLoader { private sealed record LoadSnapshot(string Fingerprint, IReadOnlyList Profiles); + private sealed record AgentDefinitionFile(string Path, string? FeedName); private readonly string _agentsDirectory; + private readonly string _serverFeedAgentsDirectory; + private readonly SkillFeedsConfig? _feedsConfig; private readonly ILogger _logger; private readonly object _snapshotGate = new(); private LoadSnapshot? _lastSnapshot; - public FileSubAgentDefinitionLoader(NetclawPaths paths, ILogger logger) + public FileSubAgentDefinitionLoader( + NetclawPaths paths, + ILogger logger, + SkillFeedsConfig? feedsConfig = null) { _agentsDirectory = paths.AgentsDirectory; + _serverFeedAgentsDirectory = paths.ServerFeedAgentsDirectory; + _feedsConfig = feedsConfig; _logger = logger; } @@ -101,8 +109,8 @@ private IReadOnlyList LoadProfilesFromDisk() return []; } - var files = Directory.GetFiles(_agentsDirectory, "*.md"); - if (files.Length == 0) + var files = EnumerateAgentDefinitionFiles(); + if (files.Count == 0) { _logger.LogWarning("No agent definition files found in {Path}", _agentsDirectory); return []; @@ -111,18 +119,15 @@ private IReadOnlyList LoadProfilesFromDisk() var results = new List(); var seenNames = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var filePath in files.OrderBy(p => p, StringComparer.Ordinal)) + foreach (var file in files) { - var profile = TryParse(filePath); + var profile = TryParse(file.Path); if (profile is null) continue; if (!seenNames.Add(profile.Name)) { - _logger.LogWarning( - "Agent definition at {Path} declares duplicate name '{Name}' — skipping", - filePath, - profile.Name); + LogDuplicate(file, profile.Name); continue; } @@ -143,10 +148,10 @@ private string ComputeDirectoryFingerprint() // Length is part of the fingerprint so rapid edits within a single mtime tick // still register as a change. mtime alone is unreliable on low-resolution filesystems // and during fast successive writes. - var files = Directory.GetFiles(_agentsDirectory, "*.md") - .OrderBy(p => p, StringComparer.Ordinal) - .Select(path => + var files = EnumerateAgentDefinitionFiles() + .Select(file => { + var path = file.Path; var info = new FileInfo(path); return $"{path}|{info.Length}|{info.LastWriteTimeUtc.Ticks}"; }); @@ -154,6 +159,72 @@ private string ComputeDirectoryFingerprint() return string.Join(";", files); } + private IReadOnlyList EnumerateAgentDefinitionFiles() + { + var files = new List(); + if (!Directory.Exists(_agentsDirectory)) + return files; + + files.AddRange(Directory.GetFiles(_agentsDirectory, "*.md", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal) + .Select(path => new AgentDefinitionFile(path, FeedName: null))); + + if (!Directory.Exists(_serverFeedAgentsDirectory)) + return files; + + foreach (var feedName in EnumerateManagedFeedNames()) + { + var feedDir = Path.Combine(_serverFeedAgentsDirectory, feedName); + if (!Directory.Exists(feedDir)) + continue; + + files.AddRange(Directory.GetFiles(feedDir, "*.md", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal) + .Select(path => new AgentDefinitionFile(path, feedName))); + } + + return files; + } + + private IEnumerable EnumerateManagedFeedNames() + { + var seen = new HashSet(StringComparer.Ordinal); + if (_feedsConfig is not null) + { + foreach (var feed in _feedsConfig.Feeds.Where(f => f.Enabled)) + { + if (seen.Add(feed.Name)) + yield return feed.Name; + } + } + + foreach (var dir in Directory.GetDirectories(_serverFeedAgentsDirectory) + .OrderBy(p => p, StringComparer.Ordinal)) + { + var feedName = Path.GetFileName(dir); + if (!string.IsNullOrEmpty(feedName) && seen.Add(feedName)) + yield return feedName; + } + } + + private void LogDuplicate(AgentDefinitionFile file, string name) + { + if (file.FeedName is null) + { + _logger.LogWarning( + "Agent definition at {Path} declares duplicate name '{Name}' — skipping", + file.Path, + name); + return; + } + + _logger.LogWarning( + "Managed agent definition at {Path} from feed '{FeedName}' declares duplicate name '{Name}' — skipping; earlier definition wins", + file.Path, + file.FeedName, + name); + } + private SubAgentProfile? TryParse(string filePath) { string content; diff --git a/src/Netclaw.Configuration/NetclawPaths.cs b/src/Netclaw.Configuration/NetclawPaths.cs index 7a254ab64..1909ac3d1 100644 --- a/src/Netclaw.Configuration/NetclawPaths.cs +++ b/src/Netclaw.Configuration/NetclawPaths.cs @@ -62,6 +62,13 @@ public string ServerFeedSyncStatePath(string feedName) // ── Agent definitions directory ── public string AgentsDirectory => Path.Combine(BasePath, "agents"); + public string ServerFeedAgentsDirectory => Path.Combine(AgentsDirectory, ".server-feeds"); + + public string ServerFeedAgentDirectory(string feedName) + => Path.Combine(ServerFeedAgentsDirectory, feedName); + + public string ServerFeedAgentSyncStatePath(string feedName) + => Path.Combine(ServerFeedAgentDirectory(feedName), ".sync-state.json"); // ── Project workspaces ── /// @@ -165,6 +172,7 @@ private IEnumerable StandardDirectories() yield return LogsDirectory; yield return SessionLogsDirectory; yield return AgentsDirectory; + yield return ServerFeedAgentsDirectory; yield return SessionsDirectory; yield return BinDirectory; yield return KeysDirectory; diff --git a/src/Netclaw.Daemon.IntegrationTests/Netclaw.Daemon.IntegrationTests.csproj b/src/Netclaw.Daemon.IntegrationTests/Netclaw.Daemon.IntegrationTests.csproj new file mode 100644 index 000000000..29219acf1 --- /dev/null +++ b/src/Netclaw.Daemon.IntegrationTests/Netclaw.Daemon.IntegrationTests.csproj @@ -0,0 +1,26 @@ + + + + net10.0 + enable + enable + false + true + + + + + + + + + + + + + + + + + + diff --git a/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs new file mode 100644 index 000000000..1d5c6812a --- /dev/null +++ b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs @@ -0,0 +1,203 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Net.Http.Headers; +using System.Text; +using DotNet.Testcontainers.Builders; +using DotNet.Testcontainers.Containers; +using Microsoft.Extensions.Logging.Abstractions; +using Netclaw.Actors.Skills; +using Netclaw.Configuration; +using Netclaw.Daemon.Services; +using Netclaw.Security.Skills; +using Netclaw.SkillClient; +using Netclaw.Tests.Utilities; +using Xunit; + +namespace Netclaw.Daemon.IntegrationTests; + +/// +/// Opt-in spike against a real SkillServer container. This catches drift between +/// SkillServer's native sidecar wire format and NetClaw's sync adapter. +/// +[Trait("Category", "Integration")] +public sealed class SkillServerNativeSidecarIntegrationTests : IAsyncLifetime +{ + private const string Image = "ghcr.io/netclaw-dev/skillserver:0.4.0-beta.1"; + private const string ApiKey = "sk-test-native-sidecar-sync"; + private const int ContainerPort = 8080; + private const int HostPort = 18080; + private const string ServerUrl = "http://localhost:18080"; + + private IContainer? _container; + private string? _skipReason; + + public async ValueTask InitializeAsync() + { + var optIn = Environment.GetEnvironmentVariable("NETCLAW_RUN_SKILLSERVER_INTEGRATION_TESTS"); + if (!string.Equals(optIn, "1", StringComparison.Ordinal)) + { + _skipReason = "SkillServer integration test is opt-in; set NETCLAW_RUN_SKILLSERVER_INTEGRATION_TESTS=1 to run."; + return; + } + + IContainer? container = null; + try + { + container = new ContainerBuilder(Image) + .WithPortBinding(HostPort, ContainerPort) + .WithEnvironment("SKILLSERVER__DATAPATH", "/tmp/skillserver-data") + .WithEnvironment("SKILLSERVER__BASEURL", ServerUrl) + .WithEnvironment("SKILLSERVER__APIKEY", ApiKey) + .WithEnvironment("ASPNETCORE_URLS", "http://+:8080") + .WithWaitStrategy(Wait.ForUnixContainer().UntilHttpRequestIsSucceeded(r => + r.ForPort(ContainerPort).ForPath("/health"))) + .Build(); + + await container.StartAsync(); + } + catch (Exception ex) when (IsDockerOrPortUnavailable(ex)) + { + if (container is not null) + await container.DisposeAsync(); + + _skipReason = $"Docker or fixed host port {HostPort} is unavailable; SkillServer integration test skipped. ({ex.GetType().Name}: {ex.Message})"; + return; + } + + _container = container; + } + + public async ValueTask DisposeAsync() + { + if (_container is not null) + await _container.DisposeAsync(); + } + + [Fact] + public async Task Syncs_skill_and_subagent_from_real_skillserver_container() + { + if (_skipReason is not null) + { + Assert.Skip(_skipReason); + return; + } + + await SeedSkillServerAsync(); + + using var dir = new DisposableTempDir(); + var paths = new NetclawPaths(dir.Path); + paths.EnsureDirectoriesExist(); + + var feedsConfig = new SkillFeedsConfig + { + SyncIntervalMinutes = 0, + Feeds = + [ + new SkillFeedSource + { + Name = "real-skillserver", + Url = ServerUrl, + ApiKey = new SensitiveString(ApiKey), + TimeoutSeconds = 30 + } + ] + }; + var service = new ServerFeedSkillSyncService( + feedsConfig, + paths, + new SkillRegistry(), + new SkillIndexContextLayer(), + TimeProvider.System, + new NoOpSkillContentScanner(), + NullLogger.Instance, + []); + + await service.SyncOnceAsync(CancellationToken.None); + + var skillPath = Path.Combine(paths.ServerFeedDirectory("real-skillserver"), "review-code", "SKILL.md"); + Assert.True(File.Exists(skillPath)); + Assert.Contains("metadata:", File.ReadAllText(skillPath), StringComparison.Ordinal); + + var agentPath = Path.Combine( + paths.ServerFeedAgentDirectory("real-skillserver"), + "code-reviewer.md"); + Assert.True(File.Exists(agentPath)); + Assert.Contains("You review code for concrete risks.", File.ReadAllText(agentPath), StringComparison.Ordinal); + + var loader = new FileSubAgentDefinitionLoader( + paths, + NullLogger.Instance, + feedsConfig); + var registry = new SubAgentDefinitionRegistry(); + Assert.True(loader.SyncInto(registry)); + Assert.NotNull(registry.TryGetByName("code-reviewer")); + } + + private static async Task SeedSkillServerAsync() + { + using var client = new SkillServerClient(ServerUrl, ApiKey); + + var skillContent = """ + --- + name: review-code + description: Route code review work to the managed reviewer sub-agent. + metadata: + subagent: code-reviewer + --- + + # Review Code + + Use the `code-reviewer` sub-agent for bounded code review tasks. + """; + await using var skillStream = TextStream(skillContent); + await client.UploadSkillIfNotExistsAsync( + "review-code", + "1.0.0", + skillStream, + [], + category: null, + CancellationToken.None); + + var subAgentContent = """ + --- + name: code-reviewer + description: Reviews code for concrete risks and regressions. + tools: [file_read] + visibility: user-facing + --- + + You review code for concrete risks. Report findings first, with file and line references when available. + """; + await using var subAgentStream = TextStream(subAgentContent); + await client.UploadSubAgentIfNotExistsAsync( + "code-reviewer", + "1.0.0", + subAgentStream, + CancellationToken.None); + } + + private static MemoryStream TextStream(string content) + => new(Encoding.UTF8.GetBytes(content)); + + private static bool IsDockerOrPortUnavailable(Exception ex) + { + for (var current = ex; current is not null; current = current.InnerException) + { + if (current.GetType().Name.Contains("Docker", StringComparison.Ordinal)) + return true; + + var msg = current.Message ?? ""; + if (msg.Contains("Docker", StringComparison.OrdinalIgnoreCase) + || msg.Contains("named pipe", StringComparison.OrdinalIgnoreCase) + || msg.Contains("/var/run/docker.sock", StringComparison.OrdinalIgnoreCase) + || msg.Contains("port is already allocated", StringComparison.OrdinalIgnoreCase) + || msg.Contains($":{HostPort}", StringComparison.OrdinalIgnoreCase)) + return true; + } + + return false; + } +} diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs index 2429ff8c6..3f9a85a3b 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -4,13 +4,17 @@ // // ----------------------------------------------------------------------- using System.IO.Compression; +using System.Net; +using System.Net.Http.Headers; using System.Text; +using System.Text.Json; using Microsoft.Extensions.Logging.Abstractions; using Netclaw.Actors.Skills; using Netclaw.Configuration; using Netclaw.Configuration.Feeds; using Netclaw.Daemon.Services; using Netclaw.Security.Skills; +using Netclaw.SkillClient; using Netclaw.Tests.Utilities; using Xunit; @@ -18,10 +22,12 @@ namespace Netclaw.Daemon.Tests.Services; public sealed class ServerFeedSkillSyncServiceTests : IDisposable { + private const string BaseUrl = "https://skillserver.test/"; + private readonly DisposableTempDir _dir = new(); private readonly NetclawPaths _paths; private readonly SkillRegistry _skillRegistry = new(); - private readonly SkillIndexContextLayer _indexLayer = new(); + private readonly SkillIndexContextLayer _skillIndexLayer = new(); public ServerFeedSkillSyncServiceTests() { @@ -29,6 +35,8 @@ public ServerFeedSkillSyncServiceTests() _paths.EnsureDirectoriesExist(); } + public void Dispose() => _dir.Dispose(); + [Fact] public async Task ExtractArchiveAsync_AllowsArbitraryResourcesAndPreservesExecutableMode() { @@ -90,19 +98,304 @@ public async Task ExtractArchiveAsync_RejectsTraversalEntries() Assert.Null(files); } - public void Dispose() => _dir.Dispose(); + [Fact] + public async Task SyncOnce_syncs_native_subagent_from_sidecar_after_empty_rfc_index() + { + var agentContent = AgentMarkdown("code-reviewer", "Managed reviewer", "Review code carefully."); + var digest = SkillSyncHelpers.ComputeSha256(agentContent); + + var handler = new FakeHttpMessageHandler(); + AddEmptyRfcIndex(handler); + AddNativeSubAgentResponses(handler, "code-reviewer", "1.0.0", agentContent, digest); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + var agentPath = Path.Combine(_paths.ServerFeedAgentDirectory("team"), "code-reviewer.md"); + Assert.True(File.Exists(agentPath)); + Assert.Equal(agentContent, File.ReadAllText(agentPath)); + + var state = ReadAgentSyncState(); + Assert.Equal("1.0.0", state.Skills["code-reviewer"].Version); + Assert.Equal(digest, state.Skills["code-reviewer"].Sha256); + } + + [Fact] + public async Task SyncOnce_missing_native_sidecar_preserves_rfc_skill_sync() + { + var skillContent = "---\nname: feed-skill\ndescription: Feed skill\n---\n\n# Feed Skill\n"; + var digest = SkillSyncHelpers.ComputeSha256(skillContent); + + var handler = new FakeHttpMessageHandler(); + handler.AddStringResponse( + BaseUrl + ".well-known/agent-skills/index.json", + $$""" + { + "skills": [ + { + "name": "feed-skill", + "type": "skill", + "description": "Feed skill", + "url": "{{BaseUrl}}skills/feed-skill/1.0.0/SKILL.md", + "digest": "sha256:{{digest}}", + "version": "1.0.0" + } + ] + } + """, + "application/json"); + handler.AddStringResponse(BaseUrl + "skills/feed-skill/1.0.0/SKILL.md", skillContent, "text/markdown"); + handler.AddErrorResponse(BaseUrl + "manifest.json", HttpStatusCode.NotFound); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + var skillPath = Path.Combine(_paths.ServerFeedDirectory("team"), "feed-skill", "SKILL.md"); + Assert.True(File.Exists(skillPath)); + Assert.Equal(skillContent, File.ReadAllText(skillPath)); + Assert.False(Directory.Exists(_paths.ServerFeedAgentDirectory("team"))); + } + + [Fact] + public async Task SyncOnce_digest_failure_keeps_existing_managed_subagents_and_skips_prune() + { + var agentDir = _paths.ServerFeedAgentDirectory("team"); + Directory.CreateDirectory(agentDir); + var oldContent = AgentMarkdown("code-reviewer", "Old reviewer", "Old body."); + File.WriteAllText(Path.Combine(agentDir, "code-reviewer.md"), oldContent); + File.WriteAllText(Path.Combine(agentDir, "stale-agent.md"), AgentMarkdown("stale-agent", "Stale", "Stale body.")); + + SkillSyncHelpers.WriteSyncState(_paths.ServerFeedAgentSyncStatePath("team"), new SkillSyncState + { + Skills = + { + ["code-reviewer"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = SkillSyncHelpers.ComputeSha256(oldContent) + }, + ["stale-agent"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = "stale" + } + } + }); + + var expectedContent = AgentMarkdown("code-reviewer", "New reviewer", "New body."); + var deliveredContent = AgentMarkdown("code-reviewer", "Tampered reviewer", "Tampered body."); + var expectedDigest = SkillSyncHelpers.ComputeSha256(expectedContent); + + var handler = new FakeHttpMessageHandler(); + AddEmptyRfcIndex(handler); + AddNativeSubAgentResponses(handler, "code-reviewer", "1.0.0", deliveredContent, expectedDigest); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + Assert.Equal(oldContent, File.ReadAllText(Path.Combine(agentDir, "code-reviewer.md"))); + Assert.True(File.Exists(Path.Combine(agentDir, "stale-agent.md"))); + + var state = ReadAgentSyncState(); + Assert.True(state.Skills.ContainsKey("stale-agent")); + Assert.Equal("0.9.0", state.Skills["code-reviewer"].Version); + } + + [Fact] + public async Task SyncOnce_successful_sidecar_prunes_only_removed_managed_subagents() + { + var agentDir = _paths.ServerFeedAgentDirectory("team"); + Directory.CreateDirectory(agentDir); + File.WriteAllText(Path.Combine(agentDir, "stale-agent.md"), AgentMarkdown("stale-agent", "Stale", "Stale body.")); + File.WriteAllText(Path.Combine(_paths.AgentsDirectory, "stale-agent.md"), + AgentMarkdown("stale-agent", "Local stale", "Local must survive.")); + + SkillSyncHelpers.WriteSyncState(_paths.ServerFeedAgentSyncStatePath("team"), new SkillSyncState + { + Skills = + { + ["stale-agent"] = new SyncedSkillState { Version = "0.9.0", Sha256 = "stale" } + } + }); + + var agentContent = AgentMarkdown("code-reviewer", "Managed reviewer", "Review code carefully."); + var digest = SkillSyncHelpers.ComputeSha256(agentContent); + + var handler = new FakeHttpMessageHandler(); + AddEmptyRfcIndex(handler); + AddNativeSubAgentResponses(handler, "code-reviewer", "1.0.0", agentContent, digest); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + Assert.False(File.Exists(Path.Combine(agentDir, "stale-agent.md"))); + Assert.True(File.Exists(Path.Combine(_paths.AgentsDirectory, "stale-agent.md"))); + + var state = ReadAgentSyncState(); + Assert.Equal(["code-reviewer"], state.Skills.Keys.OrderBy(k => k)); + } private ServerFeedSkillSyncService CreateService(ISkillContentScanner? scanner = null) => new( new SkillFeedsConfig(), _paths, _skillRegistry, - _indexLayer, + _skillIndexLayer, TimeProvider.System, scanner ?? new NoOpSkillContentScanner(), NullLogger.Instance, []); + private ServerFeedSkillSyncService CreateService(FakeHttpMessageHandler handler) + { + var feedsConfig = new SkillFeedsConfig + { + SyncIntervalMinutes = 0, + Feeds = [new SkillFeedSource { Name = "team", Url = BaseUrl, TimeoutSeconds = 30 }] + }; + + return new ServerFeedSkillSyncService( + feedsConfig, + _paths, + _skillRegistry, + _skillIndexLayer, + TimeProvider.System, + new NoOpSkillContentScanner(), + NullLogger.Instance, + [], + feed => + { + var client = new HttpClient(handler) + { + BaseAddress = new Uri(feed.Url.TrimEnd('/') + "/") + }; + if (feed.ApiKey is { Value: { Length: > 0 } apiKey }) + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiKey); + return new SkillServerClient(client); + }); + } + + private SkillSyncState ReadAgentSyncState() + { + var json = File.ReadAllText(_paths.ServerFeedAgentSyncStatePath("team")); + return JsonSerializer.Deserialize(json)!; + } + + private static void AddEmptyRfcIndex(FakeHttpMessageHandler handler) + { + handler.AddStringResponse( + BaseUrl + ".well-known/agent-skills/index.json", + """ + { + "skills": [] + } + """, + "application/json"); + } + + private static void AddNativeSubAgentResponses( + FakeHttpMessageHandler handler, + string name, + string version, + string artifactContent, + string expectedDigest) + { + handler.AddStringResponse( + BaseUrl + "manifest.json", + """ + { + "$schema": "https://schemas.netclaw.dev/skillserver/native-manifest/v1.json", + "generatedAt": "2026-06-30T00:00:00Z", + "links": { + "self": { "href": "manifest.json" }, + "rfcSkills": { "href": ".well-known/agent-skills/index.json" }, + "skills": { "href": "manifest/skills/index.json" }, + "subagents": { "href": "manifest/subagents/index.json" } + } + } + """, + "application/json"); + handler.AddStringResponse( + BaseUrl + "manifest/subagents/index.json", + """ + { + "kind": "subagent-collection-index", + "links": { "self": { "href": "manifest/subagents/index.json" } }, + "pages": [ + { "range": "a-z", "href": "manifest/subagents/pages/a-z.json" } + ] + } + """, + "application/json"); + handler.AddStringResponse( + BaseUrl + "manifest/subagents/pages/a-z.json", + $$""" + { + "kind": "subagent-collection-page", + "range": "a-z", + "links": { "self": { "href": "manifest/subagents/pages/a-z.json" } }, + "items": [ + { + "name": "{{name}}", + "latestVersion": "{{version}}", + "versionRange": { "min": "{{version}}", "max": "{{version}}", "count": 1 }, + "href": "manifest/subagents/{{name}}/index.json" + } + ] + } + """, + "application/json"); + handler.AddStringResponse( + BaseUrl + $"manifest/subagents/{name}/index.json", + $$""" + { + "kind": "subagent-identity-index", + "name": "{{name}}", + "latestVersion": "{{version}}", + "links": { "self": { "href": "manifest/subagents/{{name}}/index.json" } }, + "versions": [ + { + "version": "{{version}}", + "publishedAt": "2026-06-30T00:00:00Z", + "digest": "sha256:{{expectedDigest}}", + "href": "manifest/subagents/{{name}}/versions/{{version}}.json" + } + ] + } + """, + "application/json"); + handler.AddStringResponse( + BaseUrl + $"manifest/subagents/{name}/versions/{version}.json", + $$""" + { + "kind": "subagent-version-detail", + "name": "{{name}}", + "version": "{{version}}", + "type": "agent-md", + "description": "Test sub-agent", + "url": "{{BaseUrl}}subagents/{{name}}/{{version}}/agent.md", + "digest": "sha256:{{expectedDigest}}", + "links": { "self": { "href": "manifest/subagents/{{name}}/versions/{{version}}.json" } } + } + """, + "application/json"); + handler.AddStringResponse( + BaseUrl + $"subagents/{name}/{version}/agent.md", + artifactContent, + "text/markdown"); + } + + private static string AgentMarkdown(string name, string description, string body) + => $""" + --- + name: {name} + description: {description} + tools: [file_read] + --- + + {body} + """; + private static byte[] BuildArchive(params (string Path, byte[] Content, int UnixMode)[] entries) { using var stream = new MemoryStream(); diff --git a/src/Netclaw.Daemon.Tests/Services/SkillSyncHelpersTests.cs b/src/Netclaw.Daemon.Tests/Services/SkillSyncHelpersTests.cs index 0ca0c73d9..9ff1cc8cd 100644 --- a/src/Netclaw.Daemon.Tests/Services/SkillSyncHelpersTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/SkillSyncHelpersTests.cs @@ -143,4 +143,48 @@ public void PruneRemovedSkills_is_a_no_op_for_an_empty_index() Assert.True(Directory.Exists(Path.Combine(feedDir, "skill-a"))); Assert.Single(syncState.Skills); } + + [Fact] + public void PruneRemovedSubAgents_drops_stale_managed_files_and_state_only() + { + var feedDir = _dir.Path; + File.WriteAllText(Path.Combine(feedDir, "agent-a.md"), "a"); + File.WriteAllText(Path.Combine(feedDir, "agent-b.md"), "b"); + File.WriteAllText(Path.Combine(feedDir, "notes.txt"), "not managed"); + Directory.CreateDirectory(Path.Combine(feedDir, ".staging")); + + var syncState = new SkillSyncState + { + Skills = + { + ["agent-a"] = State(), + ["agent-b"] = State(), + ["agent-c"] = State(), + } + }; + + var changed = SkillSyncHelpers.PruneRemovedSubAgents( + feedDir, ["agent-a"], syncState, NullLogger.Instance); + + Assert.True(changed); + Assert.True(File.Exists(Path.Combine(feedDir, "agent-a.md"))); + Assert.False(File.Exists(Path.Combine(feedDir, "agent-b.md"))); + Assert.True(File.Exists(Path.Combine(feedDir, "notes.txt"))); + Assert.True(Directory.Exists(Path.Combine(feedDir, ".staging"))); + Assert.Equal(["agent-a"], syncState.Skills.Keys.OrderBy(k => k)); + } + + [Theory] + [InlineData("code-reviewer", true)] + [InlineData("agent123", true)] + [InlineData("CodeReviewer", false)] + [InlineData("../agent", false)] + [InlineData("agent.md", false)] + [InlineData("", false)] + public void IsSafeManagedFileStem_accepts_only_lowercase_alphanumeric_hyphen( + string value, + bool expected) + { + Assert.Equal(expected, SkillSyncHelpers.IsSafeManagedFileStem(value)); + } } diff --git a/src/Netclaw.Daemon/Properties/AssemblyInfo.cs b/src/Netclaw.Daemon/Properties/AssemblyInfo.cs index 33cd5d5ca..30c4ae9db 100644 --- a/src/Netclaw.Daemon/Properties/AssemblyInfo.cs +++ b/src/Netclaw.Daemon/Properties/AssemblyInfo.cs @@ -6,4 +6,5 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Netclaw.Actors.Tests")] +[assembly: InternalsVisibleTo("Netclaw.Daemon.IntegrationTests")] [assembly: InternalsVisibleTo("Netclaw.Daemon.Tests")] diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 8e488fea8..6b054adc0 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -4,8 +4,8 @@ // // ----------------------------------------------------------------------- using System.IO.Compression; -using System.Net.Http.Headers; using System.Text; +using System.Text.Json; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Netclaw.Actors.Skills; @@ -37,6 +37,7 @@ internal sealed class ServerFeedSkillSyncService : BackgroundService private readonly ISkillContentScanner _scanner; private readonly ILogger _logger; private readonly IReadOnlyList _externalSources; + private readonly Func _clientFactory; // Random jitter (0–5 min) so multiple daemon instances don't all poll at once private readonly TimeSpan _initialJitter; @@ -50,6 +51,29 @@ public ServerFeedSkillSyncService( ISkillContentScanner scanner, ILogger logger, IReadOnlyList externalSources) + : this( + feedsConfig, + paths, + skillRegistry, + skillIndexLayer, + timeProvider, + scanner, + logger, + externalSources, + CreateSkillServerClient) + { + } + + internal ServerFeedSkillSyncService( + SkillFeedsConfig feedsConfig, + NetclawPaths paths, + SkillRegistry skillRegistry, + SkillIndexContextLayer skillIndexLayer, + TimeProvider timeProvider, + ISkillContentScanner scanner, + ILogger logger, + IReadOnlyList externalSources, + Func clientFactory) { _feedsConfig = feedsConfig; _paths = paths; @@ -59,6 +83,7 @@ public ServerFeedSkillSyncService( _scanner = scanner; _logger = logger; _externalSources = externalSources; + _clientFactory = clientFactory; _initialJitter = TimeSpan.FromSeconds(Random.Shared.Next(0, 300)); } @@ -104,6 +129,9 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } } + internal Task SyncOnceAsync(CancellationToken cancellationToken) + => SyncAllFeedsAsync(cancellationToken); + private async Task SyncAllFeedsAsync(CancellationToken cancellationToken) { foreach (var feed in _feedsConfig.Feeds.Where(f => f.Enabled)) @@ -144,12 +172,12 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell var now = _timeProvider.GetUtcNow(); RfcSkillIndex? index; + using var client = _clientFactory(feed); using (var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)) { cts.CancelAfter(TimeSpan.FromSeconds(feed.TimeoutSeconds)); try { - using var client = new SkillServerClient(feed.Url, feed.ApiKey?.Value); index = await client.GetRfcIndexAsync(cts.Token); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) @@ -168,20 +196,25 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell } } - if (index is null || index.Skills.Count == 0) + if (index is null) { - _logger.LogDebug("Server feed '{FeedName}' returned empty index", feed.Name); + _logger.LogDebug("Server feed '{FeedName}' returned no RFC index", feed.Name); return; } - _logger.LogDebug( - "Fetched RFC index from server feed '{FeedName}' ({SkillCount} skills)", - feed.Name, index.Skills.Count); + if (index.Skills.Count == 0) + { + _logger.LogDebug("Server feed '{FeedName}' returned empty index", feed.Name); + } + else + { + _logger.LogDebug( + "Fetched RFC index from server feed '{FeedName}' ({SkillCount} skills)", + feed.Name, index.Skills.Count); + } var updated = false; - using var httpClient = CreateHttpClientForFeed(feed); - foreach (var entry in index.Skills) { var digestHex = NormalizeDigest(entry.Digest); @@ -200,7 +233,7 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell if (string.Equals(entry.Type, ArchiveType, StringComparison.OrdinalIgnoreCase)) { var archiveBytes = await DownloadAndVerifyBytesAsync( - httpClient, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); + client, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); if (archiveBytes is null) continue; @@ -211,7 +244,7 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell else { var mainContent = await DownloadAndVerifyAsync( - httpClient, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); + client, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); if (mainContent is null) continue; @@ -246,7 +279,7 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell var resourceDigest = NormalizeDigest(resource.Digest); var fileContent = await DownloadAndVerifyAsync( - httpClient, resource.Url, resourceDigest, + client, resource.Url, resourceDigest, $"{entry.Name}/{resource.Path}", feed.TimeoutSeconds, cancellationToken); if (fileContent is null) { @@ -296,12 +329,15 @@ await SkillSyncHelpers.ReplaceSkillDirectoryAsync( } } - // Reverse pass: drop skills the server no longer advertises. This is - // only reached with a confirmed, non-empty index (see the early returns - // above), so a transient outage or empty response never triggers a prune. - var serverSkillNames = index.Skills.Select(e => e.Name).ToList(); - if (SkillSyncHelpers.PruneRemovedSkills(feedDir, serverSkillNames, syncState, _logger)) - updated = true; + if (index.Skills.Count > 0) + { + // Reverse pass: drop skills the server no longer advertises. This is + // only reached with a confirmed, non-empty index, so a transient outage + // or empty response never triggers a skill prune. + var serverSkillNames = index.Skills.Select(e => e.Name).ToList(); + if (SkillSyncHelpers.PruneRemovedSkills(feedDir, serverSkillNames, syncState, _logger)) + updated = true; + } if (updated) { @@ -309,28 +345,254 @@ await SkillSyncHelpers.ReplaceSkillDirectoryAsync( SkillSyncHelpers.WriteSyncState( _paths.ServerFeedSyncStatePath(feed.Name), syncState); } + + await SyncNativeSubAgentsAsync(feed, client, now, cancellationToken); + } + + private async Task SyncNativeSubAgentsAsync( + SkillFeedSource feed, + SkillServerClient client, + DateTimeOffset now, + CancellationToken cancellationToken) + { + if (!SkillSyncHelpers.IsSafeManagedFileStem(feed.Name)) + { + _logger.LogWarning( + "Skipping native sub-agent sync for feed '{FeedName}': unsafe managed feed directory name", + feed.Name); + return; + } + + var feedDir = _paths.ServerFeedAgentDirectory(feed.Name); + var syncState = SkillSyncHelpers.ReadSyncState( + _paths.ServerFeedAgentSyncStatePath(feed.Name), _logger); + + NativeSubAgentCollectionIndex? subAgentIndex; + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(TimeSpan.FromSeconds(feed.TimeoutSeconds)); + + var manifest = await client.GetManifestAsync(cts.Token); + if (manifest?.Links?.SubAgents is not { Href.Length: > 0 } subAgentsLink) + { + _logger.LogDebug( + "Server feed '{FeedName}' native sidecar is unavailable or does not advertise sub-agents", + feed.Name); + return; + } + + subAgentIndex = await client.GetNativeSubAgentIndexAsync(subAgentsLink, cts.Token); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + _logger.LogWarning( + "Server feed '{FeedName}' native sidecar fetch timed out — keeping managed sub-agents", + feed.Name); + return; + } + catch (HttpRequestException ex) + { + _logger.LogDebug( + "Server feed '{FeedName}' native sidecar fetch failed: {Message} — keeping managed sub-agents", + feed.Name, ex.Message); + return; + } + catch (JsonException ex) + { + _logger.LogWarning(ex, + "Server feed '{FeedName}' native sidecar is malformed — keeping managed sub-agents", + feed.Name); + return; + } + + if (subAgentIndex is null) + return; + + var advertisedNames = new List(); + var changed = false; + var fullySuccessful = true; + + foreach (var pageLink in subAgentIndex.Pages) + { + NativeSubAgentCollectionPage? page; + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(TimeSpan.FromSeconds(feed.TimeoutSeconds)); + page = await client.GetNativeSubAgentPageAsync(pageLink, cts.Token); + } + catch (Exception ex) when (ex is not OperationCanceledException || !cancellationToken.IsCancellationRequested) + { + _logger.LogWarning(ex, + "Failed to fetch native sub-agent page {Href} from feed '{FeedName}' — keeping existing managed sub-agents", + pageLink.Href, feed.Name); + fullySuccessful = false; + continue; + } + + if (page is null) + { + fullySuccessful = false; + continue; + } + + foreach (var item in page.Items) + { + advertisedNames.Add(item.Name); + var result = await SyncNativeSubAgentAsync( + feed, client, feedDir, item, syncState, now, cancellationToken); + changed |= result.Changed; + fullySuccessful &= result.Success; + } + } + + if (fullySuccessful + && SkillSyncHelpers.PruneRemovedSubAgents(feedDir, advertisedNames, syncState, _logger)) + { + changed = true; + } + + if (changed) + { + syncState.LastSyncUtc = now; + SkillSyncHelpers.WriteSyncState( + _paths.ServerFeedAgentSyncStatePath(feed.Name), syncState); + } } - private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) + private async Task SyncNativeSubAgentAsync( + SkillFeedSource feed, + SkillServerClient client, + string feedDir, + NativeSubAgentPageItem item, + SkillSyncState syncState, + DateTimeOffset now, + CancellationToken cancellationToken) { - var client = new HttpClient(); - client.DefaultRequestHeaders.TryAddWithoutValidation("User-Agent", NetclawUserAgent.Value); - client.DefaultRequestHeaders.TryAddWithoutValidation( - NetclawUserAgent.ComponentHeader, "skill-feed"); - if (feed.ApiKey is { Value: { } apiKey } && !string.IsNullOrWhiteSpace(apiKey)) + if (!SkillSyncHelpers.IsSafeManagedFileStem(item.Name)) { - client.DefaultRequestHeaders.Authorization = - new AuthenticationHeaderValue("Bearer", apiKey); + _logger.LogWarning( + "Rejected native sub-agent '{AgentName}' from feed '{FeedName}': unsafe managed file name", + item.Name, feed.Name); + return NativeSubAgentSyncResult.Failed; } - return client; + + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(TimeSpan.FromSeconds(feed.TimeoutSeconds)); + + var identity = await client.GetNativeSubAgentIdentityAsync(item, cts.Token); + var versionLink = SelectSubAgentVersion(item, identity); + if (versionLink is null) + { + _logger.LogWarning( + "Native sub-agent '{AgentName}' from feed '{FeedName}' has no latest version link", + item.Name, feed.Name); + return NativeSubAgentSyncResult.Failed; + } + + var detail = await client.GetNativeSubAgentVersionAsync(versionLink, cts.Token); + if (detail is null) + return NativeSubAgentSyncResult.Failed; + + if (!string.Equals(detail.Type, SubAgentArtifactTypes.AgentMd, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning( + "Native sub-agent '{AgentName}' from feed '{FeedName}' has unsupported artifact type '{ArtifactType}'", + item.Name, feed.Name, detail.Type); + return NativeSubAgentSyncResult.Failed; + } + + if (!string.Equals(detail.Name, item.Name, StringComparison.Ordinal)) + { + _logger.LogWarning( + "Native sub-agent '{AgentName}' from feed '{FeedName}' returned mismatched detail name '{DetailName}'", + item.Name, feed.Name, detail.Name); + return NativeSubAgentSyncResult.Failed; + } + + var digestHex = NormalizeDigest(detail.Digest); + var targetPath = Path.Combine(feedDir, $"{item.Name}.md"); + if (syncState.Skills.TryGetValue(item.Name, out var existing) + && existing.Version == detail.Version + && string.Equals(existing.Sha256, digestHex, StringComparison.OrdinalIgnoreCase) + && File.Exists(targetPath)) + { + return NativeSubAgentSyncResult.Unchanged; + } + + await using var stream = new MemoryStream(); + var download = await client.DownloadNativeSubAgentArtifactAsync(detail, stream, cts.Token); + if (!string.Equals(NormalizeDigest(download.Digest), digestHex, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning( + "Native sub-agent '{AgentName}' from feed '{FeedName}' verified unexpected digest {ActualDigest}", + item.Name, feed.Name, download.Digest); + return NativeSubAgentSyncResult.Failed; + } + + var content = Encoding.UTF8.GetString(stream.ToArray()); + var frontmatter = SubAgentMarkdownParser.ExtractFrontmatter(content); + if (!string.Equals(frontmatter?.Name, item.Name, StringComparison.Ordinal)) + { + _logger.LogWarning( + "Rejected native sub-agent '{AgentName}' from feed '{FeedName}': artifact frontmatter name was '{FrontmatterName}'", + item.Name, feed.Name, frontmatter?.Name ?? ""); + return NativeSubAgentSyncResult.Failed; + } + + await SkillSyncHelpers.ReplaceTextFileAsync( + feedDir, $"{item.Name}.md", content, cancellationToken); + + syncState.Skills[item.Name] = new SyncedSkillState + { + Version = detail.Version, + Sha256 = digestHex, + SyncedAtUtc = now + }; + + _logger.LogInformation( + "Synced sub-agent '{AgentName}' v{Version} from feed '{FeedName}'", + item.Name, detail.Version, feed.Name); + return NativeSubAgentSyncResult.Updated; + } + catch (Exception ex) when (ex is not OperationCanceledException || !cancellationToken.IsCancellationRequested) + { + _logger.LogWarning(ex, + "Failed to sync native sub-agent '{AgentName}' from feed '{FeedName}' — keeping existing version", + item.Name, feed.Name); + return NativeSubAgentSyncResult.Failed; + } + } + + private static NativeSubAgentVersionLink? SelectSubAgentVersion( + NativeSubAgentPageItem item, + NativeSubAgentIdentityIndex? identity) + { + if (identity is null) + return null; + + var latestVersion = string.IsNullOrWhiteSpace(item.LatestVersion) + ? identity.LatestVersion + : item.LatestVersion; + if (string.IsNullOrWhiteSpace(latestVersion)) + return identity.Versions.FirstOrDefault(); + + return identity.Versions.FirstOrDefault(v => + string.Equals(v.Version, latestVersion, StringComparison.Ordinal)); } + private static SkillServerClient CreateSkillServerClient(SkillFeedSource feed) + => new(feed.Url, feed.ApiKey?.Value); + private async Task DownloadAndVerifyAsync( - HttpClient httpClient, string url, string expectedSha256Hex, string label, + SkillServerClient client, string url, string expectedSha256Hex, string label, int timeoutSeconds, CancellationToken cancellationToken) { var contentBytes = await DownloadAndVerifyBytesAsync( - httpClient, url, expectedSha256Hex, label, timeoutSeconds, cancellationToken); + client, url, expectedSha256Hex, label, timeoutSeconds, cancellationToken); if (contentBytes is null) return null; @@ -346,7 +608,7 @@ private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) } private async Task DownloadAndVerifyBytesAsync( - HttpClient httpClient, string url, string expectedSha256Hex, string label, + SkillServerClient client, string url, string expectedSha256Hex, string label, int timeoutSeconds, CancellationToken cancellationToken) { try @@ -354,18 +616,9 @@ private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); cts.CancelAfter(TimeSpan.FromSeconds(timeoutSeconds)); - var content = await httpClient.GetByteArrayAsync(url, cts.Token); - - var hash = SkillSyncHelpers.ComputeSha256(content); - if (!string.Equals(hash, expectedSha256Hex, StringComparison.OrdinalIgnoreCase)) - { - _logger.LogWarning( - "SHA-256 mismatch for {Label}: expected {Expected}, got {Actual}", - label, expectedSha256Hex, hash); - return null; - } - - return content; + var bytes = await client.DownloadVerifiedArtifactBytesAsync( + url, expectedSha256Hex, cts.Token); + return bytes; } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { @@ -377,6 +630,11 @@ private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) _logger.LogWarning("Download failed for {Label}: {Message}", label, ex.Message); return null; } + catch (InvalidDataException ex) + { + _logger.LogWarning("Download failed verification for {Label}: {Message}", label, ex.Message); + return null; + } } internal async Task?> ExtractArchiveAsync( @@ -580,4 +838,11 @@ internal static string NormalizeDigest(string digest) return digest[7..]; return digest; } + + private readonly record struct NativeSubAgentSyncResult(bool Success, bool Changed) + { + public static NativeSubAgentSyncResult Failed => new(false, false); + public static NativeSubAgentSyncResult Unchanged => new(true, false); + public static NativeSubAgentSyncResult Updated => new(true, true); + } } diff --git a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs index adb4b75ac..e5c0d8f79 100644 --- a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs +++ b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs @@ -52,6 +52,22 @@ internal static string ComputeSha256(byte[] content) : normalized; } + internal static bool IsSafeManagedFileStem(string value) + { + if (string.IsNullOrWhiteSpace(value)) + return false; + + foreach (var c in value) + { + if ((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-') + continue; + + return false; + } + + return true; + } + internal static SkillSyncState ReadSyncState(string path, ILogger logger) { if (!File.Exists(path)) @@ -163,6 +179,52 @@ internal static bool PruneRemovedSkills( return changed; } + internal static bool PruneRemovedSubAgents( + string feedDir, + IReadOnlyCollection serverAgentNames, + SkillSyncState syncState, + ILogger logger) + { + var present = new HashSet(serverAgentNames, StringComparer.Ordinal); + var changed = false; + + var staleStateKeys = syncState.Skills.Keys + .Where(name => !present.Contains(name)) + .ToList(); + foreach (var name in staleStateKeys) + { + syncState.Skills.Remove(name); + changed = true; + } + + if (!Directory.Exists(feedDir)) + return changed; + + foreach (var file in Directory.GetFiles(feedDir, "*.md")) + { + var agentName = Path.GetFileNameWithoutExtension(file); + if (string.IsNullOrEmpty(agentName) || present.Contains(agentName)) + continue; + + try + { + File.Delete(file); + logger.LogInformation( + "Pruned removed sub-agent '{AgentName}' from feed directory {FeedDir}", + agentName, feedDir); + changed = true; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, + "Failed to prune removed sub-agent '{AgentName}' from {FeedDir} — leaving in place", + agentName, feedDir); + } + } + + return changed; + } + internal static async Task ReplaceSkillDirectoryAsync( string parentDirectory, string skillName, @@ -213,6 +275,50 @@ internal static async Task ReplaceSkillDirectoryAsync( Directory.Delete(backupDir, recursive: true); } } + + internal static async Task ReplaceTextFileAsync( + string parentDirectory, + string fileName, + string content, + CancellationToken cancellationToken) + { + Directory.CreateDirectory(parentDirectory); + + var stagingRoot = Path.Combine(parentDirectory, ".staging"); + Directory.CreateDirectory(stagingRoot); + + var targetPath = Path.Combine(parentDirectory, fileName); + var stagingPath = Path.Combine(stagingRoot, $"{fileName}-{Guid.NewGuid():N}.tmp"); + var backupPath = Path.Combine(stagingRoot, $"{fileName}-backup-{Guid.NewGuid():N}.tmp"); + + try + { + await File.WriteAllTextAsync(stagingPath, content, cancellationToken); + + if (File.Exists(targetPath)) + File.Move(targetPath, backupPath); + + File.Move(stagingPath, targetPath); + + if (File.Exists(backupPath)) + File.Delete(backupPath); + } + catch + { + if (!File.Exists(targetPath) && File.Exists(backupPath)) + File.Move(backupPath, targetPath); + + throw; + } + finally + { + if (File.Exists(stagingPath)) + File.Delete(stagingPath); + + if (File.Exists(backupPath) && !File.Exists(targetPath)) + File.Delete(backupPath); + } + } } internal sealed record DownloadedSkillFile(string RelativePath, byte[] Content, int? UnixMode = null) diff --git a/src/Netclaw.Tests.Utilities/AssemblyInfo.cs b/src/Netclaw.Tests.Utilities/AssemblyInfo.cs index ee9d6e4f1..61798166a 100644 --- a/src/Netclaw.Tests.Utilities/AssemblyInfo.cs +++ b/src/Netclaw.Tests.Utilities/AssemblyInfo.cs @@ -8,5 +8,6 @@ [assembly: InternalsVisibleTo("Netclaw.Actors.Tests")] [assembly: InternalsVisibleTo("Netclaw.Cli.Tests")] [assembly: InternalsVisibleTo("Netclaw.Configuration.Tests")] +[assembly: InternalsVisibleTo("Netclaw.Daemon.IntegrationTests")] [assembly: InternalsVisibleTo("Netclaw.Daemon.Tests")] [assembly: InternalsVisibleTo("Netclaw.Search.Tests")] From 3ff1a0a902abdd6e11e2a0630638991816a576ac Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 19:53:24 +0000 Subject: [PATCH 2/3] Harden native sub-agent sync --- .../FileSubAgentDefinitionLoaderTests.cs | 44 ++++++++++++++++ .../FileSubAgentDefinitionLoader.cs | 37 ++++++++------ .../ServerFeedSkillSyncServiceTests.cs | 51 +++++++++++++++++++ .../Services/ServerFeedSkillSyncService.cs | 14 +++-- 4 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs index 5d70fde21..612cdc74d 100644 --- a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs +++ b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs @@ -537,6 +537,50 @@ Team A body. && w.Contains("reviewer", StringComparison.Ordinal)); } + [Fact] + public void LoadAll_with_feed_config_ignores_disabled_and_removed_managed_feeds() + { + WriteManagedAgent("enabled", "enabled-agent.md", """ + --- + name: enabled-agent + description: Enabled managed agent + --- + + Enabled body. + """); + WriteManagedAgent("disabled", "disabled-agent.md", """ + --- + name: disabled-agent + description: Disabled managed agent + --- + + Disabled body. + """); + WriteManagedAgent("removed", "removed-agent.md", """ + --- + name: removed-agent + description: Removed managed agent + --- + + Removed body. + """); + + var feedsConfig = new SkillFeedsConfig + { + Feeds = + { + new SkillFeedSource { Name = "enabled", Enabled = true }, + new SkillFeedSource { Name = "disabled", Enabled = false } + } + }; + var loader = new FileSubAgentDefinitionLoader(_paths, NullLogger.Instance, feedsConfig); + + var results = loader.LoadAll(); + + var profile = Assert.Single(results); + Assert.Equal("enabled-agent", profile.Name); + } + [Fact] public void RefreshIfChanged_detects_managed_server_feed_agent_edits() { diff --git a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs index 7ea90b312..6695de069 100644 --- a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs +++ b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs @@ -196,6 +196,8 @@ private IEnumerable EnumerateManagedFeedNames() if (seen.Add(feed.Name)) yield return feed.Name; } + + yield break; } foreach (var dir in Directory.GetDirectories(_serverFeedAgentsDirectory) @@ -238,35 +240,40 @@ private void LogDuplicate(AgentDefinitionFile file, string name) return null; } + return TryParseDefinition(filePath, content, _logger); + } + + public static SubAgentProfile? TryParseDefinition(string sourcePath, string content, ILogger logger) + { var frontmatter = SubAgentMarkdownParser.ExtractFrontmatter(content); if (frontmatter is null) { - _logger.LogWarning( + logger.LogWarning( "Agent definition at {Path} has missing or unparseable YAML frontmatter — skipping", - filePath); + sourcePath); return null; } if (string.IsNullOrWhiteSpace(frontmatter.Name)) { - _logger.LogWarning("Agent definition at {Path} has no 'name' in frontmatter — skipping", filePath); + logger.LogWarning("Agent definition at {Path} has no 'name' in frontmatter — skipping", sourcePath); return null; } if (string.IsNullOrWhiteSpace(frontmatter.Description)) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has no 'description' in frontmatter — skipping", - frontmatter.Name, filePath); + frontmatter.Name, sourcePath); return null; } var systemPrompt = SubAgentMarkdownParser.ExtractBody(content); if (string.IsNullOrWhiteSpace(systemPrompt)) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has an empty system prompt body — skipping", - frontmatter.Name, filePath); + frontmatter.Name, sourcePath); return null; } @@ -276,17 +283,17 @@ private void LogDuplicate(AgentDefinitionFile file, string name) if (!TryParseModelRole(frontmatter.ModelRole, out var modelRole)) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has invalid modelRole '{Value}' (expected Main or Compaction) — skipping", - frontmatter.Name, filePath, frontmatter.ModelRole); + frontmatter.Name, sourcePath, frontmatter.ModelRole); return null; } if (!TryParseVisibility(frontmatter.Visibility, out var visibility)) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has invalid visibility '{Value}' (expected user-facing or internal) — skipping", - frontmatter.Name, filePath, frontmatter.Visibility); + frontmatter.Name, sourcePath, frontmatter.Visibility); return null; } @@ -296,17 +303,17 @@ private void LogDuplicate(AgentDefinitionFile file, string name) // documented ceiling — the frontmatter path bypasses schema validation. if (frontmatter.TimeoutSeconds is { } timeoutSeconds && timeoutSeconds is < 5 or > 600) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has invalid timeoutSeconds {Value} (expected 5–600) — skipping", - frontmatter.Name, filePath, timeoutSeconds); + frontmatter.Name, sourcePath, timeoutSeconds); return null; } if (frontmatter.PrefillTimeoutSeconds is { } prefillSeconds && prefillSeconds is < 5 or > 3600) { - _logger.LogWarning( + logger.LogWarning( "Agent '{Name}' at {Path} has invalid prefillTimeoutSeconds {Value} (expected 5–3600) — skipping", - frontmatter.Name, filePath, prefillSeconds); + frontmatter.Name, sourcePath, prefillSeconds); return null; } diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs index 3f9a85a3b..8f6ee6c05 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -201,6 +201,57 @@ public async Task SyncOnce_digest_failure_keeps_existing_managed_subagents_and_s Assert.Equal("0.9.0", state.Skills["code-reviewer"].Version); } + [Fact] + public async Task SyncOnce_invalid_native_artifact_keeps_existing_managed_subagents_and_skips_prune() + { + var agentDir = _paths.ServerFeedAgentDirectory("team"); + Directory.CreateDirectory(agentDir); + var oldContent = AgentMarkdown("code-reviewer", "Old reviewer", "Old body."); + File.WriteAllText(Path.Combine(agentDir, "code-reviewer.md"), oldContent); + File.WriteAllText(Path.Combine(agentDir, "stale-agent.md"), AgentMarkdown("stale-agent", "Stale", "Stale body.")); + + SkillSyncHelpers.WriteSyncState(_paths.ServerFeedAgentSyncStatePath("team"), new SkillSyncState + { + Skills = + { + ["code-reviewer"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = SkillSyncHelpers.ComputeSha256(oldContent) + }, + ["stale-agent"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = "stale" + } + } + }); + + var invalidContent = """ + --- + name: code-reviewer + tools: [file_read] + --- + + Missing a description, so the runtime loader would reject this artifact. + """; + var digest = SkillSyncHelpers.ComputeSha256(invalidContent); + + var handler = new FakeHttpMessageHandler(); + AddEmptyRfcIndex(handler); + AddNativeSubAgentResponses(handler, "code-reviewer", "1.0.0", invalidContent, digest); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + Assert.Equal(oldContent, File.ReadAllText(Path.Combine(agentDir, "code-reviewer.md"))); + Assert.True(File.Exists(Path.Combine(agentDir, "stale-agent.md"))); + + var state = ReadAgentSyncState(); + Assert.True(state.Skills.ContainsKey("stale-agent")); + Assert.Equal("0.9.0", state.Skills["code-reviewer"].Version); + } + [Fact] public async Task SyncOnce_successful_sidecar_prunes_only_removed_managed_subagents() { diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 6b054adc0..04b1a053e 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -514,7 +514,8 @@ private async Task SyncNativeSubAgentAsync( } var digestHex = NormalizeDigest(detail.Digest); - var targetPath = Path.Combine(feedDir, $"{item.Name}.md"); + var targetFileName = $"{item.Name}.md"; + var targetPath = Path.Combine(feedDir, targetFileName); if (syncState.Skills.TryGetValue(item.Name, out var existing) && existing.Version == detail.Version && string.Equals(existing.Sha256, digestHex, StringComparison.OrdinalIgnoreCase) @@ -534,17 +535,20 @@ private async Task SyncNativeSubAgentAsync( } var content = Encoding.UTF8.GetString(stream.ToArray()); - var frontmatter = SubAgentMarkdownParser.ExtractFrontmatter(content); - if (!string.Equals(frontmatter?.Name, item.Name, StringComparison.Ordinal)) + var profile = FileSubAgentDefinitionLoader.TryParseDefinition(targetPath, content, _logger); + if (profile is null) + return NativeSubAgentSyncResult.Failed; + + if (!string.Equals(profile.Name, item.Name, StringComparison.Ordinal)) { _logger.LogWarning( "Rejected native sub-agent '{AgentName}' from feed '{FeedName}': artifact frontmatter name was '{FrontmatterName}'", - item.Name, feed.Name, frontmatter?.Name ?? ""); + item.Name, feed.Name, profile.Name); return NativeSubAgentSyncResult.Failed; } await SkillSyncHelpers.ReplaceTextFileAsync( - feedDir, $"{item.Name}.md", content, cancellationToken); + feedDir, targetFileName, content, cancellationToken); syncState.Skills[item.Name] = new SyncedSkillState { From 56d4de28137abcc7af6915d87b7af03adc811fb8 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 3 Jul 2026 02:22:23 +0000 Subject: [PATCH 3/3] Harden native sub-agent artifact handling --- .../FileSubAgentDefinitionLoaderTests.cs | 43 ++++++++++++ .../FileSubAgentDefinitionLoader.cs | 24 +++++++ .../Schemas/netclaw-config.v1.schema.json | 1 + .../ServerFeedSkillSyncServiceTests.cs | 65 ++++++++++++++++++- .../Services/ServerFeedSkillSyncService.cs | 19 +++++- .../Services/SkillSyncHelpers.cs | 9 ++- .../FakeHttpMessageHandler.cs | 9 +++ 7 files changed, 164 insertions(+), 6 deletions(-) diff --git a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs index 612cdc74d..275e26cc0 100644 --- a/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs +++ b/src/Netclaw.Configuration.Tests/FileSubAgentDefinitionLoaderTests.cs @@ -581,6 +581,49 @@ Removed body. Assert.Equal("enabled-agent", profile.Name); } + [Fact] + public void LoadAll_with_feed_config_ignores_unsafe_managed_feed_names() + { + var escapedDir = Path.GetFullPath(Path.Combine(_paths.ServerFeedAgentsDirectory, "..", "escaped")); + Directory.CreateDirectory(escapedDir); + File.WriteAllText(Path.Combine(escapedDir, "escaped-agent.md"), """ + --- + name: escaped-agent + description: Escaped managed agent + --- + + Escaped body. + """); + + WriteManagedAgent("safe-feed", "safe-agent.md", """ + --- + name: safe-agent + description: Safe managed agent + --- + + Safe body. + """); + + var feedsConfig = new SkillFeedsConfig + { + Feeds = + { + new SkillFeedSource { Name = "../escaped", Enabled = true }, + new SkillFeedSource { Name = "safe-feed", Enabled = true } + } + }; + var logger = new ListLogger(); + var loader = new FileSubAgentDefinitionLoader(_paths, logger, feedsConfig); + + var results = loader.LoadAll(); + + var profile = Assert.Single(results); + Assert.Equal("safe-agent", profile.Name); + Assert.Contains(logger.Warnings, w => + w.Contains("../escaped", StringComparison.Ordinal) + && w.Contains("safe feed name", StringComparison.OrdinalIgnoreCase)); + } + [Fact] public void RefreshIfChanged_detects_managed_server_feed_agent_edits() { diff --git a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs index 6695de069..e7f7f37bf 100644 --- a/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs +++ b/src/Netclaw.Configuration/FileSubAgentDefinitionLoader.cs @@ -193,6 +193,14 @@ private IEnumerable EnumerateManagedFeedNames() { foreach (var feed in _feedsConfig.Feeds.Where(f => f.Enabled)) { + if (!IsSafeManagedFeedName(feed.Name)) + { + _logger.LogWarning( + "Ignoring managed sub-agent feed '{FeedName}' because it is not a safe feed name", + feed.Name); + continue; + } + if (seen.Add(feed.Name)) yield return feed.Name; } @@ -209,6 +217,22 @@ private IEnumerable EnumerateManagedFeedNames() } } + private static bool IsSafeManagedFeedName(string value) + { + if (string.IsNullOrWhiteSpace(value)) + return false; + + foreach (var c in value) + { + if ((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-') + continue; + + return false; + } + + return true; + } + private void LogDuplicate(AgentDefinitionFile file, string name) { if (file.FeedName is null) diff --git a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json index 2a65ed2d6..d21c04573 100644 --- a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json +++ b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json @@ -465,6 +465,7 @@ "properties": { "Name": { "type": "string", + "pattern": "^[a-z0-9-]+$", "description": "Unique feed identifier (used as directory name). Lowercase alphanumeric and hyphens." }, "Url": { diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs index 8f6ee6c05..b2a910026 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -101,7 +101,7 @@ public async Task ExtractArchiveAsync_RejectsTraversalEntries() [Fact] public async Task SyncOnce_syncs_native_subagent_from_sidecar_after_empty_rfc_index() { - var agentContent = AgentMarkdown("code-reviewer", "Managed reviewer", "Review code carefully."); + var agentContent = AgentMarkdown("code-reviewer", "Managed reviewer", "Review code carefully. 请仔细审查代码。"); var digest = SkillSyncHelpers.ComputeSha256(agentContent); var handler = new FakeHttpMessageHandler(); @@ -114,6 +114,7 @@ public async Task SyncOnce_syncs_native_subagent_from_sidecar_after_empty_rfc_in var agentPath = Path.Combine(_paths.ServerFeedAgentDirectory("team"), "code-reviewer.md"); Assert.True(File.Exists(agentPath)); Assert.Equal(agentContent, File.ReadAllText(agentPath)); + Assert.Equal(Encoding.UTF8.GetBytes(agentContent), await File.ReadAllBytesAsync(agentPath, TestContext.Current.CancellationToken)); var state = ReadAgentSyncState(); Assert.Equal("1.0.0", state.Skills["code-reviewer"].Version); @@ -252,6 +253,58 @@ public async Task SyncOnce_invalid_native_artifact_keeps_existing_managed_subage Assert.Equal("0.9.0", state.Skills["code-reviewer"].Version); } + [Fact] + public async Task SyncOnce_invalid_utf8_native_artifact_keeps_existing_managed_subagents_and_skips_prune() + { + var agentDir = _paths.ServerFeedAgentDirectory("team"); + Directory.CreateDirectory(agentDir); + var oldContent = AgentMarkdown("code-reviewer", "Old reviewer", "Old body."); + File.WriteAllText(Path.Combine(agentDir, "code-reviewer.md"), oldContent); + File.WriteAllText(Path.Combine(agentDir, "stale-agent.md"), AgentMarkdown("stale-agent", "Stale", "Stale body.")); + + SkillSyncHelpers.WriteSyncState(_paths.ServerFeedAgentSyncStatePath("team"), new SkillSyncState + { + Skills = + { + ["code-reviewer"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = SkillSyncHelpers.ComputeSha256(oldContent) + }, + ["stale-agent"] = new SyncedSkillState + { + Version = "0.9.0", + Sha256 = "stale" + } + } + }); + + var validPrefix = Encoding.UTF8.GetBytes(""" + --- + name: code-reviewer + description: Managed reviewer + --- + + Review code carefully. + """); + var invalidContent = validPrefix.Concat([byte.MaxValue]).ToArray(); + var digest = SkillSyncHelpers.ComputeSha256(invalidContent); + + var handler = new FakeHttpMessageHandler(); + AddEmptyRfcIndex(handler); + AddNativeSubAgentResponses(handler, "code-reviewer", "1.0.0", invalidContent, digest); + + var service = CreateService(handler); + await service.SyncOnceAsync(CancellationToken.None); + + Assert.Equal(oldContent, File.ReadAllText(Path.Combine(agentDir, "code-reviewer.md"))); + Assert.True(File.Exists(Path.Combine(agentDir, "stale-agent.md"))); + + var state = ReadAgentSyncState(); + Assert.True(state.Skills.ContainsKey("stale-agent")); + Assert.Equal("0.9.0", state.Skills["code-reviewer"].Version); + } + [Fact] public async Task SyncOnce_successful_sidecar_prunes_only_removed_managed_subagents() { @@ -350,6 +403,14 @@ private static void AddNativeSubAgentResponses( string version, string artifactContent, string expectedDigest) + => AddNativeSubAgentResponses(handler, name, version, Encoding.UTF8.GetBytes(artifactContent), expectedDigest); + + private static void AddNativeSubAgentResponses( + FakeHttpMessageHandler handler, + string name, + string version, + byte[] artifactContent, + string expectedDigest) { handler.AddStringResponse( BaseUrl + "manifest.json", @@ -430,7 +491,7 @@ private static void AddNativeSubAgentResponses( } """, "application/json"); - handler.AddStringResponse( + handler.AddByteResponse( BaseUrl + $"subagents/{name}/{version}/agent.md", artifactContent, "text/markdown"); diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 04b1a053e..8b2026b8a 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -534,7 +534,20 @@ private async Task SyncNativeSubAgentAsync( return NativeSubAgentSyncResult.Failed; } - var content = Encoding.UTF8.GetString(stream.ToArray()); + var contentBytes = stream.ToArray(); + string content; + try + { + content = StrictUtf8.GetString(contentBytes); + } + catch (DecoderFallbackException ex) + { + _logger.LogWarning( + "Rejected native sub-agent '{AgentName}' from feed '{FeedName}': agent.md is not valid UTF-8: {Message}", + item.Name, feed.Name, ex.Message); + return NativeSubAgentSyncResult.Failed; + } + var profile = FileSubAgentDefinitionLoader.TryParseDefinition(targetPath, content, _logger); if (profile is null) return NativeSubAgentSyncResult.Failed; @@ -547,8 +560,8 @@ private async Task SyncNativeSubAgentAsync( return NativeSubAgentSyncResult.Failed; } - await SkillSyncHelpers.ReplaceTextFileAsync( - feedDir, targetFileName, content, cancellationToken); + await SkillSyncHelpers.ReplaceFileAsync( + feedDir, targetFileName, contentBytes, cancellationToken); syncState.Skills[item.Name] = new SyncedSkillState { diff --git a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs index e5c0d8f79..fe69986e6 100644 --- a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs +++ b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs @@ -281,6 +281,13 @@ internal static async Task ReplaceTextFileAsync( string fileName, string content, CancellationToken cancellationToken) + => await ReplaceFileAsync(parentDirectory, fileName, Encoding.UTF8.GetBytes(content), cancellationToken); + + internal static async Task ReplaceFileAsync( + string parentDirectory, + string fileName, + byte[] content, + CancellationToken cancellationToken) { Directory.CreateDirectory(parentDirectory); @@ -293,7 +300,7 @@ internal static async Task ReplaceTextFileAsync( try { - await File.WriteAllTextAsync(stagingPath, content, cancellationToken); + await File.WriteAllBytesAsync(stagingPath, content, cancellationToken); if (File.Exists(targetPath)) File.Move(targetPath, backupPath); diff --git a/src/Netclaw.Tests.Utilities/FakeHttpMessageHandler.cs b/src/Netclaw.Tests.Utilities/FakeHttpMessageHandler.cs index a5758c471..6764856a5 100644 --- a/src/Netclaw.Tests.Utilities/FakeHttpMessageHandler.cs +++ b/src/Netclaw.Tests.Utilities/FakeHttpMessageHandler.cs @@ -30,6 +30,15 @@ public void AddStringResponse(string url, string content, string contentType = " Content = new StringContent(content, Encoding.UTF8, contentType) }; + public void AddByteResponse(string url, byte[] content, string contentType = "application/octet-stream") + => _routes[url] = _ => new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new ByteArrayContent(content) + { + Headers = { ContentType = new(contentType) } + } + }; + public void AddResponse(string url, HttpStatusCode status, string content, string contentType) => _routes[url] = _ => new HttpResponseMessage(status) {