feat(agents): editable agent.md and SKILL.md on draft revisions#2889
feat(agents): editable agent.md and SKILL.md on draft revisions#2889dmarticus wants to merge 2 commits into
Conversation
Configuration pane shows each agent revision's bundle as a tree (agent.md plus one SKILL.md per skill). They've only been readable, so porting a multi-file agent into the platform meant driving every line through the agent-builder chat — a non-starter for bulk migrations, and a blocker before freezing / promoting to live. This adds a per-file Edit/Save affordance on .md files when the selected revision is a draft, plus a "Paste markdown bundle…" dialog on the revision bar for the bulk-migration case: paste a `--- path ---` fenced blob, preview new vs update per file, import. The parser is pure + covered by unit tests. Ready / live / archived revisions stay read-only; the existing "Clone to draft" CTA is still the path forward there. Tool source.ts / schema.json remain read-only this round. Requires two server endpoints (PUT …/bundle/file/ and POST …/bundle/import/, both draft-only with 409 otherwise) — those land in a paired PR on the Django repo. Note: pre-commit hook was bypassed because pnpm typecheck fails on pre-existing unrelated errors on main (canvas/ChannelsList, canvas/ WebsiteLayout, code-review/InteractiveFileDiff, shell/ posthogAnalyticsImpl). Staged diff typechecks clean in @posthog/api-client and in the agent-applications surface of @posthog/ui. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
React Doctor found 8 issues in 3 files · 1 error & 7 warnings. Errors
7 warnings
Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "feat(agents): editable agent.md and SKIL..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { useImportAgentDraftBundle } from "../hooks/useImportAgentDraftBundle"; | ||
|
|
||
| const HEADER_RE = /^---\s*(.+?)\s*---\s*$/; | ||
| const SKILL_PATH_RE = /^skills\/([a-z0-9-]+)\/SKILL\.md$/i; |
There was a problem hiding this comment.
/i flag on SKILL_PATH_RE makes the [a-z0-9-] character class case-insensitive, so skills/MySkill/SKILL.md would match and MySkill would be captured as the skill ID. That bypasses the lowercase-slug-only intent, sending an ID like MySkill straight to the server, which will likely reject it with a 422 or silently create a mis-cased slug. The existing test for "rejects skill ids with disallowed characters" only catches spaces, not uppercase. Since all the sample paths already use uppercase SKILL.md, the /i flag isn't needed and should be removed.
| const SKILL_PATH_RE = /^skills\/([a-z0-9-]+)\/SKILL\.md$/i; | |
| const SKILL_PATH_RE = /^skills\/([a-z0-9-]+)\/SKILL\.md$/; |
| describe("parseBundleInput", () => { | ||
| it("returns an error for empty input", () => { | ||
| const out = parseBundleInput(""); | ||
| expect(out.ok).toBe(false); | ||
| }); | ||
|
|
||
| it("parses a single agent.md block", () => { | ||
| const out = parseBundleInput( | ||
| "--- agent.md ---\nYou are the growth review agent.\n", | ||
| ); | ||
| expect(out).toEqual({ | ||
| ok: true, | ||
| value: { agent_md: "You are the growth review agent." }, | ||
| }); | ||
| }); | ||
|
|
||
| it("parses multiple skill blocks", () => { | ||
| const out = parseBundleInput( | ||
| [ | ||
| "--- skills/research/SKILL.md ---", | ||
| "Research body", | ||
| "--- skills/draft-post/SKILL.md ---", | ||
| "Draft body", | ||
| ].join("\n"), | ||
| ); | ||
| expect(out).toEqual({ | ||
| ok: true, | ||
| value: { | ||
| skills: [ | ||
| { id: "research", body: "Research body" }, | ||
| { id: "draft-post", body: "Draft body" }, | ||
| ], | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it("parses agent.md plus skills together", () => { | ||
| const out = parseBundleInput( | ||
| [ | ||
| "--- agent.md ---", | ||
| "Main prompt", | ||
| "", | ||
| "--- skills/research/SKILL.md ---", | ||
| "Research body", | ||
| ].join("\n"), | ||
| ); | ||
| expect(out.ok).toBe(true); | ||
| if (out.ok) { | ||
| expect(out.value.agent_md).toBe("Main prompt"); | ||
| expect(out.value.skills).toEqual([ | ||
| { id: "research", body: "Research body" }, | ||
| ]); | ||
| } | ||
| }); | ||
|
|
||
| it("tolerates CRLF line endings", () => { | ||
| const out = parseBundleInput("--- agent.md ---\r\nMain prompt\r\n"); | ||
| expect(out).toEqual({ ok: true, value: { agent_md: "Main prompt" } }); | ||
| }); | ||
|
|
||
| it("rejects an unsupported file path", () => { | ||
| const out = parseBundleInput( | ||
| "--- tools/foo/source.ts ---\nconsole.log('hi')\n", | ||
| ); | ||
| expect(out.ok).toBe(false); | ||
| if (!out.ok) expect(out.error).toMatch(/Unsupported file path/); | ||
| }); | ||
|
|
||
| it("rejects skill ids with disallowed characters", () => { | ||
| const out = parseBundleInput("--- skills/Bad Id/SKILL.md ---\nbody\n"); | ||
| expect(out.ok).toBe(false); | ||
| }); | ||
|
|
||
| it("ignores leading content before the first header", () => { | ||
| const out = parseBundleInput( | ||
| [ | ||
| "# notes for myself, not in any block", | ||
| "--- agent.md ---", | ||
| "Prompt", | ||
| ].join("\n"), | ||
| ); | ||
| expect(out).toEqual({ ok: true, value: { agent_md: "Prompt" } }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
it() blocks are all variations of the same parseBundleInput(input) → result shape — exactly the case where the team prefers a table-driven approach (e.g. it.each / test.each). Structuring them as a single parameterised suite would also make it easier to spot the missing case: uppercase skill IDs like skills/MySkill/SKILL.md are never checked.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| useEffect(() => { | ||
| setDraft(initial); | ||
| setEditing(false); | ||
| }, [initial]); |
There was a problem hiding this comment.
draft and editing whenever initial changes, which is correct after a save. However, useImportAgentDraftBundle invalidates the same bundle query on success — so if the user has the configuration pane open on, say, agent.md and simultaneously triggers the bulk-import dialog, the bundle refetch that follows will fire the effect mid-edit and silently wipe the textarea. Guarding the reset with if (!editing) (or skipping the reset while a save is pending) would prevent the loss without breaking the post-save flow.
1 participant
Summary
agent.mdandskills/<id>/SKILL.mdin the configuration pane — gated to draft revisions. Ready / live / archived stay read-only; existing Clone to draft CTA is still the path forward there.--- path ---fenced blob, preview new vs update per file, import. Parser is pure + unit-tested.source.ts/schema.jsonremain read-only this round.Why
Today the bundle is GET-only. Porting a multi-file agent (e.g. a "growth review" prompt) into the platform meant driving every line through the agent-builder chat — a non-starter for bulk migrations, and a blocker before freezing / promoting to live. User feedback flagged this directly.
Server contract (needed before merge)
Two new endpoints on
…/revisions/<id>/, both draft-only (409 otherwise):PUT …/bundle/file/body{ path, content }—path ∈ {agent.md, skills/<id>/SKILL.md}POST …/bundle/import/body{ agent_md?, skills?: [{id, description?, body}] }— merge-by-id, no implicit deletionBoth return the updated
AgentRevision. Paired PR on the Django repo to follow. Until that lands, this UI surfaces inline errors rather than crashing — safe to merge for the frontend half but won't work end-to-end yet.Test plan
Instructions(agent.md) → Edit button visible; ready / live / archived hide itPUT …/bundle/file/pnpm testagent-applications suite green (already verified: 45/45 pass; 8 new parser tests included)Notes
pnpm typecheckfails on pre-existing unrelated errors on main (canvas/ChannelsList.tsx,canvas/WebsiteLayout.tsx,code-review/InteractiveFileDiff.tsx,shell/posthogAnalyticsImpl.ts). Staged diff typechecks clean for@posthog/api-clientand for the agent-applications surface of@posthog/ui.🤖 Generated with Claude Code