Skip to content

Feat/issue 1218 agent md context structure#1705

Merged
yinwm merged 7 commits intosipeed:refactor/agentfrom
alexhoshina:feat/issue-1218-agent-md-context-structure
Mar 18, 2026
Merged

Feat/issue 1218 agent md context structure#1705
yinwm merged 7 commits intosipeed:refactor/agentfrom
alexhoshina:feat/issue-1218-agent-md-context-structure

Conversation

@alexhoshina
Copy link
Collaborator

@alexhoshina alexhoshina commented Mar 17, 2026

📝 Description

This PR makes the workspace agent bootstrap fully based on the new AGENT.md structure, while keeping backward compatibility with legacy AGENTS.md workspaces. #1218

What Changed

  • added a structured agent definition loader in pkg/agent
  • parse YAML frontmatter from AGENT.md into a runtime-friendly definition
  • use only the AGENT.md body and resolved SOUL.md content when building the system prompt
  • stop pulling legacy IDENTITY.md into the prompt when a structured AGENT.md workspace is present
  • keep fallback support for legacy AGENTS.md workspaces
  • switch the default workspace scaffold from AGENTS.md / IDENTITY.md / USER.md to AGENT.md + SOUL.md
  • update README workspace layout docs across all translated variants

Behavior Notes

  • AGENT.md frontmatter is parsed for structured metadata, but raw frontmatter is not injected into the prompt
  • structured workspaces track prompt cache invalidation based on AGENT.md, SOUL.md, and memory files
  • legacy workspaces still load through AGENTS.md without breaking existing setups

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

Parse AGENT.md frontmatter into a runtime definition and pair it with SOUL.md while keeping a legacy AGENTS.md fallback for transition.

Refs sipeed#1218
Use AGENT.md and SOUL.md as the structured bootstrap source, ignore IDENTITY.md for structured agents, remove USER.md from the new context flow, and update pkg/agent tests accordingly.

Refs sipeed#1218
Replace the legacy AGENTS.md, IDENTITY.md, and USER.md templates with a structured AGENT.md plus SOUL.md, and update the onboard template test to assert the new generated files.

Refs sipeed#1218
Refresh the documented workspace tree across the README translations so onboarding now points to AGENT.md and SOUL.md instead of the retired AGENTS.md, IDENTITY.md, and USER.md files.

Refs sipeed#1218
@alexhoshina alexhoshina requested a review from yinwm March 17, 2026 15:37
@sipeed-bot sipeed-bot bot added type: enhancement New feature or request domain: agent go Pull requests that update go code labels Mar 17, 2026
@yinwm
Copy link
Collaborator

yinwm commented Mar 17, 2026

PR #1705 Code Review

Overall Assessment

This is a well-designed refactoring that simplifies agent configuration from 3 files to 2. The code quality is good and test coverage is adequate. However, there are a few design issues that need clarification and correction.


🚨 Core Issue: USER.md Positioning

USER.md should be global user information, not agent-level configuration.

~/.picoclaw/                     ← PICOCLAW_HOME (global)
├── config.json                  # Global config
├── USER.md                      # Who the user is, location, preferences (shared by all agents)
│
├── workspace/                   # main agent
│   ├── AGENT.md                # Agent identity definition
│   └── SOUL.md                 # Agent personality
│
├── workspace-sales/             # sales agent
│   ├── AGENT.md
│   └── SOUL.md

Issues:

  1. The old code reads `USER.md` from `workspace/` (the location itself was problematic)
  2. This PR deletes `USER.md` directly without considering fixing its location
  3. If a user has multiple agents, each agent should be able to read the same user information

Suggestions:

  • `USER.md` should be placed in `PICOCLAW_HOME/` directory
  • Code should read from `PICOCLAW_HOME`, not from `workspace`
  • All agents should share the same user information

⚠️ Other Issues

1. Breaking Change Not Clearly Marked

`USER.md`, `IDENTITY.md`, `AGENTS.md` are deleted without:

  • Migration guide
  • Clear Breaking Change label
  • Instructions on how to migrate existing configurations

2. Silent YAML Parsing Errors

In `pkg/agent/definition.go`, `parseAgentFrontmatter` silently returns an empty struct on YAML parsing failure:

```go
if err := yaml.Unmarshal([]byte(frontmatter), &rawFields); err != nil {
return AgentFrontmatter{} // Silent return, user doesn't know config didn't take effect
}
```

Suggest adding at least a debug log to inform users of parsing failures.

3. Unclear `└── ...` in Documentation

The workspace tree in README ends with `└── ...`, which may confuse new users. Suggest either removing it or listing other possible files.


✅ What's Done Well

  • Good backward compatibility design (`AGENTS.md` fallback)
  • Comprehensive test coverage
  • Clean commit split
  • `Fields` map in YAML frontmatter preserves extensibility

📋 Suggestions

  1. Reconsider USER.md location - Move to `PICOCLAW_HOME/` as global user configuration
  2. If deleting USER.md is confirmed - Clearly mark Breaking Change in PR description and provide migration guide
  3. Add parsing error logs - Inform users when config parsing fails
  4. Improve documentation - Explain file locations in multi-agent scenarios

▶︎ 中文 Review

整体评价

这是一个设计思路正确的重构,将 agent 配置从 3 个文件精简为 2 个,代码质量良好,测试覆盖到位。但有几个设计问题需要澄清和修正。


🚨 核心问题:USER.md 的定位

USER.md 应该是全局用户信息,而不是 agent 级别的配置。

```
~/.picoclaw/ ← PICOCLAW_HOME(全局)
├── config.json # 全局配置
├── USER.md # 用户是谁、在哪、偏好(所有 agents 共享)

├── workspace/ # main agent
│ ├── AGENT.md # agent 身份定义
│ └── SOUL.md # agent 性格

├── workspace-sales/ # sales agent
│ ├── AGENT.md
│ └── SOUL.md
```

问题:

  1. 旧代码把 `USER.md` 放在 `workspace/` 下读取(位置本身就有问题)
  2. PR 直接删除 `USER.md`,没有考虑修正其位置
  3. 如果用户有多个 agents,每个 agent 都应该能读取到同一个用户信息

建议:

  • `USER.md` 应该放在 `PICOCLAW_HOME/` 目录下
  • 代码从 `PICOCLAW_HOME` 读取,而不是从 `workspace`
  • 所有 agents 共享同一份用户信息

⚠️ 其他问题

1. Breaking Change 未明确标注

`USER.md`、`IDENTITY.md`、`AGENTS.md` 被删除,但没有:

  • 迁移方案
  • 明确标注 Breaking Change
  • 告知用户如何迁移现有配置

2. YAML 解析错误静默

`pkg/agent/definition.go` 中 `parseAgentFrontmatter` 在 YAML 解析失败时静默返回空结构体:

```go
if err := yaml.Unmarshal([]byte(frontmatter), &rawFields); err != nil {
return AgentFrontmatter{} // 静默返回,用户不知道配置没生效
}
```

建议至少加个 debug 日志,让用户知道配置解析失败。

3. 文档中 `└── ...` 不清晰

README 的 workspace tree 结尾用了 `└── ...`,新用户可能困惑。建议要么移除,要么列出其他可能的文件。


✅ 做得好的地方

  • 向后兼容设计得当(`AGENTS.md` fallback)
  • 测试覆盖全面
  • Commit 拆分清晰
  • YAML frontmatter 的 `Fields` 字段保留了扩展性

📋 建议

  1. 重新考虑 USER.md 的位置 - 移到 `PICOCLAW_HOME/` 作为全局用户配置
  2. 如果确定删除 USER.md - 在 PR description 明确标注 Breaking Change,说明迁移方案
  3. 添加解析错误日志 - 让用户知道配置解析失败
  4. 补充文档 - 说明多 agent 场景下各文件的位置关系

Copy link
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #1705 Review - Approved ✅

Summary

This PR introduces a structured AGENT.md format with YAML frontmatter, replacing the previous scattered AGENTS.md and IDENTITY.md files. The new format supports machine-readable configuration (name, model, tools, skills, mcpServers, maxTurns) while preserving the body as the LLM prompt.

Key Strengths

  • Excellent backward compatibility - Falls back to AGENTS.md when AGENT.md is absent
  • Clean separation of machine-readable config and human-readable prompt
  • Comprehensive tests covering frontmatter parsing, fallback logic, and cache invalidation
  • Documentation synced across all language READMEs

Minor Suggestions (non-blocking)

  1. trackedPaths() logic - When using AGENTS.md format, AGENT.md is still included in paths even though it doesn't exist. Consider filtering based on actual source type.

  2. Silent frontmatter failures - Invalid YAML frontmatter returns an empty struct with only a warning log. Users may not realize their config wasn't applied.

  3. Test helper duplication - setupWorkspace and cleanupWorkspace are duplicated across test files. Consider extracting to a shared helper.

  4. README consistency - Some localized READMEs (fr, ja, pt-br, vi) show ... for USER.md while English version shows full description.

Verdict

Approved - Well-designed change with proper backward compatibility and test coverage. The minor issues above can be addressed in follow-up PRs if needed.

Ready to merge. 🚀

@yinwm yinwm merged commit 899558b into sipeed:refactor/agent Mar 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: agent go Pull requests that update go code type: enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants