Repository navigation
fix(subagents): keep agent profile fields this app does not own - #794
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
readStoredFrontmatter() can still allow overwriting a file with an unparseable frontmatter block (dropping unmanaged keys), which reintroduces data-loss risk in a key path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens subagent profile editing so Pi Web no longer drops frontmatter fields it doesn’t own when saving shared agent profile files, preserving compatibility with other runtimes (notably pi-subagents).
Changes:
- Preserve “unmanaged” frontmatter keys on save, while still updating the subset of keys Pi Web owns.
- Round-trip foreign
ext:tool selectors intools:instead of stripping them during Pi Web’s built-in tool filtering. - Add parsing/saving behavior for
skills/extensionsalias flags (including whitelist-preservation) plus targeted regression tests and documentation.
File summaries
| File | Description |
|---|---|
lib/subagents.ts |
Preserves unmanaged frontmatter keys, carries ext: selectors through tools:, and supports alias-based flag fallback/sync for skills/extensions. |
lib/subagents.test.mjs |
Adds regression tests covering unknown-key round-trip, alias seeding/whitelist preservation, and alias fallback behavior. |
AGENTS.md |
Documents which profile frontmatter keys Pi Web manages vs. must preserve for other runtimes. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+191
to
+198
| function readStoredFrontmatter(filePath: string): Record<string, unknown> { | ||
| if (!existsSync(filePath)) return {}; | ||
| try { | ||
| return parseFrontmatter(readFileSync(filePath, "utf8")).data ?? {}; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } |
Comment on lines
+505
to
+506
| const file = join(cwd, ".pi", "agents", "fresh.md"); | ||
| saveProjectSubagentProfile(cwd, profile({ name: "fresh", loadSkills: true, loadExtensions: true })); |
Saving a profile from the Agents editor rebuilt the whole frontmatter from the 11 keys this app knows, silently dropping everything the other runtime on those same files reads: `name`, `skills`, `extensions`, `exclude_extensions`, `allowed_subagents`, `disallowed_tools`, and the `ext:<name>` selectors inside `tools:`. In production that turned an orchestrator profile into one that could no longer spawn anything. - saveSubagentProfile round-trips unknown frontmatter keys and carries foreign `ext:` selectors through `tools:` - the `skills` / `extensions` spellings pi-subagents reads are seeded on the first save and kept in step while they are booleans; a hand-authored whitelist such as `extensions: pi-advisor-flow` is never rewritten - parsing falls back to those aliases when `load_skills` / `load_extensions` are absent, so the editor reflects what the file actually declares - AGENTS.md documents the ownership split Tests: lib/subagents.test.mjs (+3 cases: unknown-key round-trip, alias seeding with whitelist preservation, alias-based flag fallback). npm test 985 pass, tsc --noEmit and eslint clean.
agegr
force-pushed
the
fix/agent-profile-frontmatter
branch
from
September 11, 2026 14:46
cb10f23 to
a3925c2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Saving a profile from the Agents editor rebuilt the whole frontmatter from the 11 keys Pi Web knows, so every key the other runtime on those same files reads was dropped:
name,skills,extensions,exclude_extensions,allowed_subagents,disallowed_tools, and theext:<name>selectors insidetools:. In production that turned an orchestrator profile into one that could no longer spawn anything, and silently flipped two profiles from "advisor off" to "advisor on".What changed
saveSubagentProfileround-trips frontmatter keys this app does not own, and carries foreignext:selectors throughtools:.skills/extensionsspellingspi-subagentsreads are seeded on the first save and kept in step while they are booleans; a hand-authored whitelist such asextensions: pi-advisor-flowis never rewritten, because the UI has no way to express it.load_skills/load_extensionsare absent, so the editor reflects the state the file actually declares.AGENTS.mddocuments which keys this app manages and which it must carry through.Tests
lib/subagents.test.mjs— three new cases: unknown-key round-trip on save, alias seeding with whitelist preservation, and alias-based flag fallback.npm test976 pass,tsc --noEmitandeslintclean.Found while running Pi Web against a set of agent definitions shared with the
pi-subagentsextension: those files are the contract between the two, and only one of them was writing them.