feat(discovery): implement provider-aware discovery and config sync alignment

[lavaman131]

Summary

Implements a provider-aware discovery contract that explicitly defines what each SDK (Claude, OpenCode, Copilot) can discover, how Atomic prepares runtime configuration, and what the UI surfaces as runtime-compatible. This resolves user-visible inconsistencies where skills/agents appear available in UI but are not aligned with provider-native discovery behavior.

Problem Solved: Previously, Atomic had a discovery split where global templates synced to ~/.atomic, UI discovered from broad paths, and provider runtimes used different discovery mechanisms. This caused confusion where skills/agents appeared in the UI but weren't actually available at runtime, especially for Claude sessions.

Key Changes

New Provider Discovery System

• provider-discovery-contract.ts: Single source of truth defining discovery precedence, compatibility modes, and runtime binding strategies
• provider-discovery-plan.ts: Deterministic path resolution engine with platform-aware config home resolution
• provider-discovery-cache.ts: Session-scoped discovery result caching with lifecycle management
• discovery-events.ts: Structured observability and telemetry for discovery operations
• copilot-paths.ts: Platform-aware Copilot config path resolution (fixes ~/.config/.copilot vs ~/.copilot ambiguity)
• path-root-guard.ts: Safety checks for config path validation and traversal prevention

Updated Config Utilities

All config utilities enhanced to accept and propagate discovery plans: claude-config, opencode-config, mcp-config, copilot-manual, copy, atomic-global-config

UI Command Improvements

• agent-commands and skill-commands with discovery plan integration
• definition-integrity validation to prevent malformed command registration
• Platform-aware config-home root scanning
• Runtime compatibility filtering for active provider

SDK Client Updates

Claude, OpenCode, and Copilot clients updated to use discovery plans for runtime config preparation

Chat Command Enhancements

• Discovery plan lifecycle management in startup sequence
• Discovery cache initialization and cleanup hooks
• Improved session isolation for provider-specific discovery state

Documentation & Dependencies

• Updated CLAUDE.md with accurate platform-aware config paths
• Bumped claude-agent-sdk from 0.2.66 to 0.2.68
• Removed stale src config docs
• Added comprehensive research and spec documents

Implementation Highlights

Precedence Resolution (deterministic last-write-wins):

1. atomicBaseline (lowest): ~/.atomic/.claude, ~/.atomic/.opencode, ~/.atomic/.copilot
2. userGlobal: ~/.claude, ~/.config/.opencode, ~/.config/.copilot
3. projectLocal (highest): .claude, .opencode, .github

Platform-Aware Path Resolution:

• Linux/macOS: XDG_CONFIG_HOME (defaults to ~/.config) for OpenCode and Copilot
• Windows: APPDATA for OpenCode and Copilot
• Claude always uses ~/.claude for user-global config (cross-platform)

Compatibility Modes:

• Native: Provider-specific config directories
• Compatibility: Cross-provider support (e.g., Copilot can read .claude and .opencode skills)

Runtime Binding Strategies:

• Claude & OpenCode: mergedConfigDir mode (sets CLAUDE_CONFIG_DIR or OPENCODE_CONFIG_DIR)
• Copilot: manualInjection mode (explicitly passes customAgents, skillDirectories, instructions)

Impact

User Experience:

• Clearer UI: Users see only skills/agents truly available for active provider
• Fewer false "not detected" reports via deterministic discovery
• Consistent cross-platform behavior following platform conventions

Developer Experience:

• Improved observability through discovery events
• Better cross-provider parity with consistent discovery logic
• Safer config operations with path validation guards

Technical Benefits:

• Performance: Session-scoped caching reduces redundant filesystem scans
• Maintainability: Centralized discovery contract eliminates scattered path logic
• Extensibility: Easy to add new providers or adjust precedence

Testing

Comprehensive test coverage (2000+ lines of new/enhanced tests):

Core Discovery Tests (100% coverage):

• provider-discovery-contract.test.ts (132 lines)
• provider-discovery-plan.test.ts (219 lines)
• provider-discovery-cache.test.ts (87 lines)
• discovery-events.test.ts (142 lines)
• copilot-paths.test.ts (168 lines)

Integration Tests (Enhanced):

• agent-commands.test.ts (354+ lines added)
• skill-commands.test.ts (428+ lines added)
• chat.test.ts (323+ lines added)
• Plus 9 more enhanced test files

Breaking Changes

None. This is an additive change that improves existing behavior without removing functionality.

Migration Notes

No migration required. The discovery system is backward-compatible and automatically applies to all sessions.

What Changes for Users:

• Skills/agents in UI now accurately reflect runtime availability
• Discovery errors are now observable via structured events
• Platform-specific config paths are correctly documented and applied

What Stays the Same:

• Existing .claude, .opencode, .github configs work unchanged
• SCM skills (gh-, sl-) remain project-local
• SKILL.MD content injection and slash commands unchanged
• Global baseline sync to ~/.atomic continues as before

Related Issues

Fixes user-reported issues where:

• Skills/agents appeared in autocomplete but weren't available at runtime
• Claude sessions didn't detect user-global skills
• Cross-platform path behavior was inconsistent
• Copilot config paths were ambiguous

---

Review Focus Areas:

1. Discovery plan precedence logic in provider-discovery-plan.ts
2. Platform-specific path resolution in copilot-paths.ts
3. Command integrity validation in definition-integrity.ts
4. Discovery cache lifecycle in chat.ts
5. Test coverage for edge cases

[claude]

PR Review: Claude SDK Discovery and Atomic Config Sync

Overall Assessment

This is a substantial PR (7,338 additions, 724 deletions across 49 files) that introduces a provider-aware discovery contract system. The design is well-documented with a comprehensive spec/RFC, and the implementation follows the proposed architecture closely.

---

✅ Strengths

Architecture & Design

• Excellent separation of concerns: The discovery contract (provider-discovery-contract.ts), discovery plan (provider-discovery-plan.ts), and caching (provider-discovery-cache.ts) are cleanly separated
• Strong typing: Comprehensive TypeScript types with as const satisfies patterns for compile-time safety
• Deterministic precedence: Clear tier-based precedence (atomicBaseline → userGlobal → projectLocal) across all providers
• Cross-platform support: Proper handling of XDG_CONFIG_HOME, APPDATA, and platform-specific paths

Security

• Path traversal protection: Good use of path-root-guard.ts with assertPathWithinRoot and assertRealPathWithinRoot for symlink validation in copy.ts:68
• Symlink dereferencing: Safe handling of symlinks by copying target content rather than following potentially malicious links
• Input validation: isValidCommandIdentifier pattern prevents injection via skill/agent names

Test Coverage

• Comprehensive tests for new utilities: provider-discovery-plan.test.ts, provider-discovery-contract.test.ts, discovery-events.test.ts
• Good edge case coverage for path resolution across Windows/Unix platforms
• Test files for copilot paths, opencode config, and command registration

---

⚠️ Areas for Improvement

1. Telemetry Path Redaction Concerns (discovery-events.ts:136-157)

The path redaction logic in redactPathForTelemetry returns <external-path> for paths outside home/project. Consider:

• Logging the redacted path portion (e.g., depth indicator) for debugging without exposing full paths
• Current implementation may make production debugging harder

2. Cache TTL Magic Number (copilot-manual.ts:344)

const CACHE_TTL_MS = 5000;

Consider extracting this to a configurable constant or environment variable for easier tuning without code changes.

3. Silent Error Swallowing (multiple locations)

Several catch {} blocks silently swallow errors:

• skill-commands.ts:473 - directory read failures
• agent-commands.ts:301 - directory read failures
• copilot-manual.ts:329 - agent loading failures

Recommendation: At minimum, emit debug-level discovery events for these failures to aid troubleshooting.

4. Duplicated Code Pattern

buildRuntimeDiscoveryPlanOptions() is duplicated between:

• skill-commands.ts:137-166
• agent-commands.ts:143-172

Recommendation: Extract to a shared utility in definition-integrity.ts or a new shared module.

5. Type Safety in Frontmatter Parsing

In parseSkillFile and parseAgentInfoWithIssues, frontmatter values are checked with in operator but not type-narrowed:

if ("name" in fm) {
  if (typeof fm.name \!== "string" || fm.name.trim().length === 0) {

Consider using a validation library like Zod for frontmatter schema validation to improve maintainability.

---

🐛 Potential Issues

1. Race Condition in Session Cache (provider-discovery-cache.ts:105-117)

The session state initialization has a potential race condition:

if (\!providerDiscoverySessionState) {
  providerDiscoverySessionState = createProviderDiscoverySessionState(projectRoot);
}

In concurrent async contexts, this could lead to duplicate initialization. Consider using a singleton pattern with lazy initialization guard.

2. Memory Leak in Invalidator Set (provider-discovery-cache.ts:14)

const cacheInvalidators = new Set<DiscoveryCacheInvalidator>();

If invalidators are registered but never unregistered (the returned cleanup function not called), this set will grow. Consider using WeakRef or documenting cleanup requirements.

3. Unbounded Cache Growth (copilot-manual.ts:339-343)

const agentCache = new Map<string, { agents: CopilotAgent[]; timestamp: number }>();
const skillDirectoryCache = new Map<...>();

These caches are never pruned beyond TTL checks. If many different project roots are processed over time, memory usage will grow. Consider:

• Adding LRU eviction
• Setting max cache size

---

📊 Performance Considerations

Positives:

• Parallel file loading with Promise.all and Promise.allSettled in copilot-manual.ts
• Session-scoped caching prevents redundant filesystem operations
• Deduplication of discovery paths prevents scanning same directory twice

Suggestions:

• Consider lazy loading of discovery plans - currently all provider plans are built even when only one is needed (createAllProviderDiscoveryPlans)
• The discoverSkillFiles function performs synchronous existsSync calls in a loop - consider batching these as async checks

---

📝 Documentation & Code Quality

Good:

• Comprehensive JSDoc comments on public APIs
• Clear module headers explaining purpose (e.g., skill-commands.ts:1-22)
• Well-structured RFC/spec document in /specs/

Suggestions:

• Consider adding ARCHITECTURE.md explaining the discovery system
• The spec references [R1]-[R6] but these references should be included in the document or linked

---

✅ Security Checklist

• No hardcoded secrets
• Path traversal protection implemented
• Input validation on command identifiers
• Symlink handling is safe
• No SQL/command injection vectors identified

---

Summary

This is a well-designed PR that addresses a real architectural need. The provider discovery contract provides a solid foundation for cross-provider compatibility. The main areas needing attention are:

1. Critical: Fix potential race condition in session cache initialization
2. Important: Add cache size limits to prevent memory growth
3. Nice-to-have: Extract duplicated buildRuntimeDiscoveryPlanOptions function
4. Nice-to-have: Add debug logging for silently swallowed errors

Recommendation: Approve with minor revisions for the cache issues.

[claude]

PR Review: Provider-Aware Discovery and Config Sync Alignment

Overall, this is a well-designed PR that introduces a robust provider discovery system. The architecture is clean and the implementation is thorough. Here's my detailed feedback:

---

✅ Code Quality and Best Practices

Strengths:

• Excellent modular design - Clear separation of concerns between contract definitions, plan building, caching, and events
• Strong type safety - Good use of TypeScript with specific types (no any or unknown)
• Immutability patterns - Good use of readonly types for arrays and maps
• Clear documentation - Well-documented module headers explaining purpose and discovery paths

Issues to Address:

1. Code Duplication - The isPathWithinRoot function is duplicated in multiple files:

   • src/ui/commands/skill-commands.ts:189
   • src/ui/commands/agent-commands.ts:200
   • src/ui/commands/definition-integrity.ts:42
   • src/utils/path-root-guard.ts:9

   Recommendation: Consolidate to a single export from path-root-guard.ts and import elsewhere.

2. Duplicate utility function - buildRuntimeDiscoveryPlanOptions is duplicated in both skill-commands.ts:137 and agent-commands.ts:143. Extract to a shared utility.

3. Long functions - discoverAndRegisterDiskSkills spans ~150 lines. Consider breaking into smaller helper functions for better maintainability.

---

⚠️ Potential Bugs or Issues

1. Silent error suppression in cache invalidation (provider-discovery-cache.ts:73-78):

   try {
     invalidator();
   } catch {
     // Cache invalidation is best-effort by design.
   }

   While intentional, consider at least logging these errors in debug mode for diagnostics.

2. Global mutable state - providerDiscoverySessionState in provider-discovery-cache.ts:15 is module-level mutable state. While this works for the current single-threaded use case, document this assumption explicitly to prevent future issues.

3. Missing null check - In copilot-paths.ts:42-48, normalizeOptionalPath handles null | undefined, but the logic could be simplified:

   function normalizeOptionalPath(pathValue: string | null | undefined): string | null {
     if (typeof pathValue !== "string") return null;
     const trimmed = pathValue.trim();
     return trimmed.length > 0 ? trimmed : null;
   }

   This is correct but could use pathValue?.trim() || null for brevity.

---

🚀 Performance Considerations

Strengths:

• Good session-level caching strategy in provider-discovery-cache.ts
• Efficient Set-based deduplication for paths
• Fingerprint-based cache invalidation is smart

Minor Concerns:

1. Repeated path resolution - Multiple resolve() calls for the same paths in discovery loops. Consider caching resolved paths within a single discovery operation.

2. Batch optimization opportunity - collectDefinitionDiscoveryMatches iterates all plans × all roots for each definition. For bulk operations (many skills/agents), a pre-indexed lookup structure could help.

---

🔒 Security Considerations

Strengths:

• Good path traversal protection via path-root-guard.ts with assertRealPathWithinRoot resolving symlinks
• Path redaction in telemetry events (discovery-events.ts:136-157) prevents sensitive path leakage
• Structured event schema (atomic.discovery.event.v1) enables safe logging

Note: The assertRealPathWithinRoot correctly uses realpath to resolve symlinks before validation, which is the right approach for security-sensitive operations.

---

🧪 Test Coverage

Excellent coverage with:

• provider-discovery-contract.test.ts - Contract definitions and precedence rules
• provider-discovery-plan.test.ts - Plan generation with platform variations (Windows APPDATA, XDG_CONFIG_HOME)
• provider-discovery-cache.test.ts - Cache lifecycle
• discovery-events.test.ts - Event emission and redaction
• copilot-paths.test.ts - Platform-specific path resolution
• chat.test.ts - Integration with SCM skills

Suggestions for additional coverage:

1. Add tests for concurrent cache access patterns (even if just documenting expected behavior)
2. Add edge case tests for malformed frontmatter in SKILL.md/agent definitions (corrupt YAML)
3. Consider property-based tests for path resolution logic given the complexity

---

📝 Minor Suggestions

1. Consistent async usage - resolveCopilotUserRoots is async but doesn't await anything. Consider making it sync or documenting why it's async (future-proofing).

2. Constants organization - PROVIDER_IDS and PROVIDER_DISCOVERY_TIERS are well-defined. Consider adding JSDoc explaining the precedence semantics.

3. Spec document location - The spec at specs/claude-sdk-discovery-and-atomic-config-sync-alignment.md is comprehensive. Consider linking it from the code comments for future maintainers.

---

Summary

Verdict: Approve with minor suggestions

This is a solid implementation that significantly improves the UX consistency for provider discovery. The architecture is well-thought-out with proper separation between:

• Contract definitions (what each provider supports)
• Plan building (how to resolve paths at runtime)
• Caching (session-level optimization)
• Events (observability)

The main action items are:

1. Consolidate duplicate isPathWithinRoot implementations (medium priority)
2. Extract shared buildRuntimeDiscoveryPlanOptions (low priority)
3. Add debug logging for swallowed cache invalidation errors (low priority)

Great work on the comprehensive test coverage and documentation! 🎉