Skip to content

Conversation

@rysweet
Copy link

@rysweet rysweet commented Jan 17, 2026

Summary

Fixes a bug where agent instructions from .md files were never loaded when spawning sub-sessions via the task tool.

Fixes: microsoft/amplifier#174

Problem

The resolve_agent_path() method existed but was never called in the spawn pipeline. Agents specified via include: only stored their names, not their instruction content from the .md files.

Solution

Add two new methods to the Bundle class:

load_agent_content(name)

  • Parses agent .md files (frontmatter + body)
  • Extracts meta.name and meta.description from frontmatter
  • Sets body as system.instruction
  • Returns None for invalid paths or missing files

resolve_agents()

  • Iterates agents that only have name references (from include:)
  • Loads full content from .md files
  • Uses deep_merge to preserve existing nested config
  • Call after composition when source_base_paths is populated

Security Hardening

  • Rejects agent names containing .. or starting with /
  • Validates resolved paths stay within agents/ directory using path containment check
  • Sanitizes error messages to avoid path disclosure

Tests

Added comprehensive tests:

  • TestBundleAgentLoading (7 tests) - functional tests
  • TestBundleAgentSecurity (3 tests) - path traversal protection

Review Process

This PR went through:

  1. Architecture review (zen-architect) - philosophy compliance ✓
  2. Security review (security-guardian) - found/fixed path traversal issue ✓
  3. Test coverage review (test-coverage) - found/fixed yaml.YAMLError handling ✓

🤖 Generated with Amplifier

Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com

Fixes a bug where agent instructions from .md files were never loaded when
spawning sub-sessions via the task tool. The resolve_agent_path() method
existed but was never called in the spawn pipeline.

Changes:
- Add load_agent_content(name) method to parse agent .md files (frontmatter + body)
- Add resolve_agents() method to pre-populate agent configs from files
- Add path traversal protection to prevent directory escape attacks
- Add comprehensive tests for agent loading and security

The load_agent_content method:
- Parses frontmatter and body using existing parse_frontmatter()
- Extracts meta.name and meta.description from frontmatter
- Sets body as system.instruction
- Returns None for invalid paths or missing files

The resolve_agents method:
- Iterates agents that only have name references (from include:)
- Loads full content from .md files
- Uses deep_merge to preserve existing nested config

Security hardening:
- Rejects agent names containing '..' or starting with '/'
- Validates resolved paths stay within agents/ directory
- Sanitizes error messages to avoid path disclosure

Fixes: microsoft/amplifier#174

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
rysweet added a commit to rysweet/amplihack that referenced this pull request Jan 17, 2026
WORKAROUND for microsoft/amplifier#174 where agent instructions from .md
files are never loaded in the spawn pipeline.

Changes:
- Modified session-start hook to populate agent configs from agents/*.md
  files at session start
- Added parse_frontmatter() and load_agent_content() helper functions
- Added populate_agent_configs() that iterates agents dir and updates
  coordinator.config['agents'] with loaded content
- Updated bundle.md comments to reference issue and PR

The workaround:
1. On session:start, the hook reads agents/*.md files
2. For each agent that only has a name (no system.instruction), loads
   the content from the .md file
3. Updates the coordinator config so the task tool can find the content

REMOVE when microsoft/amplifier-foundation#30 is merged.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@rysweet
Copy link
Author

rysweet commented Jan 17, 2026

Cross-reference: Once this PR is merged, there's cleanup work tracked in rysweet/amplihack#1957 to remove the workaround we implemented there.

rysweet added a commit to rysweet/amplihack that referenced this pull request Jan 17, 2026
Adds amplihack as an Amplifier bundle with:

## Features
- Lock mode and auto-workflow for continuous work
- Guide agent for feature discovery
- 9 workflow recipes (default-workflow, ux-workflow, security-workflow, etc.)
- Session start/stop hooks with Neo4j integration

## Test Fixes
- Fixed pytest.ini configuration (section header, pythonpath)
- Resolved package name conflicts (neo4j → neo4j_integration)
- Added conditional imports for optional dependencies
- Skipped tests for unimplemented functions (tracked in #1967, #1968)

## Known Issues
- Includes workaround for agent instruction loading bug (cleanup tracked in #1957)
- Depends on microsoft/amplifier-foundation#30 for proper fix

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant