Skip to content

Conversation

@payneio
Copy link

@payneio payneio commented Oct 30, 2025

This pull request introduces improvements to resource discovery and context handling in the Amplifier CLI, as well as a fix for cache key generation for git-sourced modules. The most important changes focus on making resource directories more discoverable after package installation, ensuring profile markdown is added to context, and preventing cache collisions for modules with subdirectories.

Resource discovery improvements:

  • Resource directories such as profiles, context, agents, scenario-tools, and modules are now symlinked from package subdirectories to the collection root during installation, making them easier to discover and use.

Context handling enhancements:

  • When processing profile mentions, the profile's own markdown body is now added as a system message to the context before handling any referenced files, improving downstream context availability.

Module resolution and caching fixes:

  • The cache key for git-sourced modules now includes the subdirectory (if specified), preventing cache collisions when resolving different subdirectories from the same repo and ref. The cache validation and return logic have also been updated to reflect this change.

Discussion:

Problem Discovery

Initial symptom: The amplifier-collection-issues tool failed to load with the error:
Failed to load module 'tool-issue': Module 'tool-issue' found at /home/payne/.amplifier/module-cache/6dc3a016d9d1/main but failed to load

The error was confusing because:

  • The collection installed successfully
  • The module files were present in the cache
  • But the module loader couldn't find/load them properly

Root Cause Analysis

Through investigation, we discovered two related bugs in GitSource's resolve() method:

Bug 1: Cache Key Collision

Problem: The cache key only used url@ref, ignoring the subdirectory component:

Original (buggy)

cache_key = hashlib.sha256(f"{self.url}@{self.ref}".encode()).hexdigest()[:12]

Why this breaks: When installing from the same repo:

Both got the same cache key (6dc3a016d9d1), causing the module to overwrite the collection or vice versa.

Bug 2: Incorrect Path Appending

Problem: After uv installed the package, code incorrectly appended the subdirectory path:

Original (buggy)

final_path = cache_path / self.subdirectory if self.subdirectory else cache_path
return final_path

Why this breaks: When using uv pip install with #subdirectory=path, uv installs the package content FROM the subdirectory TO the target directory directly. It doesn't recreate the subdirectory
structure in the target.

So the code was looking for:
~/.amplifier/module-cache/6dc3a016d9d1/amplifier_collection_issues/modules/tool-issue/

But uv had actually installed to:
~/.amplifier/module-cache/6dc3a016d9d1/

Solution Design

We chose a minimal, mechanism-focused fix aligned with kernel philosophy:

Fix 1: Include Subdirectory in Cache Key

Fixed

cache_key_input = f"{self.url}@{self.ref}"
if self.subdirectory:
cache_key_input += f"#{self.subdirectory}"
cache_key = hashlib.sha256(cache_key_input.encode()).hexdigest()[:12]

Why: Each unique combination of repo+ref+subdirectory now gets its own cache directory. No more collisions.

Result:

  • Collection cache: 6dc3a016d9d1 (no subdirectory)
  • Module cache: cafdd90c9518 (includes subdirectory in hash)
  • ✅ Now unique!

Fix 2: Remove Path Appending

Fixed

return cache_path # Don't append subdirectory - uv already installed to cache_path

Why: Respect uv's behavior. When #subdirectory= is specified, uv installs the package content FROM that subdirectory TO the target directly.

Added comments to make the uv behavior explicit:

Note: uv installs the package content FROM subdirectory TO cache_path directly

It does NOT create the subdirectory structure in the target

Why This Solution

  1. Respects Existing Mechanisms: Doesn't change the caching approach, just fixes the key generation
  2. Minimal Change: Two targeted fixes to the actual bugs, no architectural changes
  3. Follows uv Conventions: Works with how uv's #subdirectory= syntax actually behaves
  4. Kernel Philosophy Aligned:
    - Fixes mechanism (cache key generation, path resolution)
    - Doesn't add policy (doesn't change what gets cached or when)
    - Maintains backward compatibility (existing non-subdirectory modules unaffected)
  5. Verifiable: After the fix:
    - amplifier run --profile amplifier-collection-issues:issue-aware "list issues" ✅ worked
    - Tool loaded successfully and listed 30 issues
    - Both collection and module in cache with unique keys

Alternative Approaches Considered

Why not use separate cache directories for collections vs modules?

  • Would require policy decision about categorization
  • Adds complexity
  • The hash collision was the real bug, not the cache structure

Why not change how uv is called?

  • uv's #subdirectory= behavior is correct and documented
  • Our code assumptions were wrong, not uv's behavior

Why not add more complex path resolution logic?

  • Violates ruthless simplicity
  • The bug was incorrect assumptions, not missing features

Impact

  • Fixes: Collection module loading from git sources with subdirectories
  • Enables: Proper isolation of collections and their modules in cache
  • Preserves: Existing caching behavior for sources without subdirectories
  • Aligns: With uv's documented subdirectory handling

This was a textbook "fix the mechanism" change - we identified incorrect assumptions in the cache key generation and path resolution, fixed them to match actual tool behavior (uv), and
restored correct operation without adding complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant