Repository navigation
meta(#531): built-in meta persona + skill authoring tools on /api/agent - #533
Conversation
…api/agent First-party persona + skill authoring tools (product #332, phase 1 #531): - lib/agent/metaTools.ts: createMetaPersonaSkillTools({userId,userPersonas,userSkills}) → meta_persona_* (list/read/create/update_name/update_body/set_default/ clear_default/delete) + meta_skill_* (list/read/create/update_summary/ update_body/delete). Always-on on /api/agent, bound to route userId (confused-deputy; model-passed identity ignored), tenant+user scoped (no existence leak). Author-as-user auto-confirm. Slugs derived in the tool layer when omitted (stores require a slug on create). Bodies to the model only on explicit read, capped (skills SKILL_FETCH_MAX_RETURN_BYTES truncation marker; personas at store cap); writes over-cap rejected (never truncate). Short tool_run one-liners; no secrets. - app/api/agent/route.ts: assemble meta tools always into extraTools; exclude meta_* from the soft-path nonSkillToolCount guard via isMetaToolName. - lib/agent/runAgent.ts resolveSystem: hasMeta → SKILL_META_ONLY_SYSTEM + META_TOOLS_SYSTEM_ADDENDUM. - Tests: lib/agent/metaTools.test.ts (12) — list/read/no-existence-leak, persona+skill round-trips, derived slugs, over-cap rejection, skill read truncation marker, tool-name separation, bound-identity. - Docs: docs/mcp.md (built-in meta MCP section), docs/personas.md, docs/skills.md (search vs authoring split), AGENTS.md row, README pointer. Checks: typecheck, di-gate, full vitest run (1477), next build.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #533
Verdict: CONCERNS
Repo: btipling/invincible
Scope: main ← 6922b8c · 9 files · phase 1 meta authoring tools (#531)
Lenses run: L1, L2, L5, L6, L8 (skip L3: no protocol; L4: no workflow; L7: no UI; L9: tool_run existing)
AGENTS.md read: yes · SECURITY.md personas/skills: non-secret plaintext, no DEK
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Major | L2 | Always-on write/delete with no user-intent gate. meta_*_delete, *_update_body, set_default, create run as soon as the model calls them. Settings is a click; this is a model choice on every /api/agent turn (no flag). Untrusted context (http_get, a skill body, a pasted page) that says “list skills then delete them” / “create persona X and set_default” persists or wipes standing orders. Next session injects the attacker playbook (personaSnapshot / attachedSkills re-resolve). Docs call this “same as Settings” — it is not. |
User: “summarize https://evil”. Model follows page: meta_skill_list → meta_skill_delete each id; or create+set_default a jailbreak persona. |
Require an explicit authoring cue in the user turn, or a confirm tool / host modal, before mutate/delete. At minimum do not auto-run delete / set_default / body overwrite. Locked “auto-confirm” does not make the deputy safe. |
high |
| Major | L5 | No per-user count cap; create is in the model-ended loop. createUserSkill / createUserPersona have no N-row ceiling. resolveAgentMaxSteps() may be null. One turn can insert many 4 MiB skills (store allows each body). List personas is also unbounded (meta_persona_list dumps every row; skills slice to 20). |
“Clone my skill library” / prompt-inject “create 200 skills”. DB + next list/find_skill blow the prompt. |
Cap rows per user (same order as SKILL_FIND_RESULT_MAX / a hard store max). Cap *_list. Reject create past the cap. |
high |
| Nit | L8 | docs/mcp.md heading “Built-in meta MCP” while the same section says it is not MCP. Future agents will wire a real MCP server. |
Drift vs skillTools (“in-process, not MCP”). |
Rename to “Built-in meta tools (meta_*)”. |
med |
| Nit | L8 | Header: tool traces are “short one-liners”. *_read / *_list execute() return the full body/summary; L2 tool_run preview can ship ~100k chars (same class as #527). |
Operator opens the tool card after meta_skill_read. Own non-secret content. |
Say model+preview; L1 one-liner stays. | high |
Checked and not findings:
- L2 IDOR:
userIdclosed over;additionalProperties: false; stores filter tenant+user; missing/foreign delete →not_found(same string). Slugs unique per(tenantId, userId). - L1 403 mask:
isMetaToolNameexcluded fromnonSkillToolCountwithfind_skill/fetch_skill. - L5 write size: store rejects over-cap bodies (16 KiB persona / 4 MiB skill); reads truncate skills at 256 KiB with marker. Tested.
- L6: DI services; no in-body I/O.
hasMeta→ honestSKILL_META_ONLY_SYSTEM.
Residual risk
slugify does not retry with Settings-style _2 suffixes — duplicate derived slug is a hard error. boundSkillBody can split UTF-8 at the byte cap. Bound-identity test only spies list, not delete/update. MCP tools named meta_* are ignored by the 403 counter (fail-safe: more 403s).
Merge guidance
Do not merge until the write path has a real user-intent or confirm gate (or a written product accept of “any turn may wipe/replace standing orders”) and a per-user create/list ceiling. IDOR/403 look mergeable.
What was not attacked
Live Gateway complying with an injected “delete all skills”; two-tenant delete against a real DB (unit doubles only).
… + doc nits Adversarial-review follow-up on PR #533 (concerns addressed; the L2 user-intent gate is WONT-FIX per product decision — see reply + versioning issue). - L5: add per-user create/list ceilings META_USER_PERSONAS_MAX (50) / META_USER_SKILLS_MAX (200). meta_persona_*_create / meta_skill_create reject past the ceiling; meta_persona_list bounds its summary output (meta_skill_list already bounded at SKILL_FIND_RESULT_MAX). - boundSkillBody: back off to a code-point boundary so truncation never splits a multi-byte UTF-8 rune (no lone U+FFFD in model/preview text). - Nits: docs/mcp.md heading 'Built-in meta MCP' -> 'Built-in meta tools (...) - not MCP'; trace vs execute-result width clarified in metaTools header + docs (tool_run trace is a short one-liner; read/list result text is capped but can be large). - Tests: 3 new metaTools cases (persona + skill ceilings with list bound, UTF-8-safe truncation). Full suite 1480 passed / 0 failed.
Response to adversarial review (concerns addressed)Commit L2 — always-on write/delete with no user-intent gate → WONT-FIX (product decision)We are intentionally not adding user gates to the built-in authoring surface. Confirm/modals on every agent write add friction to everything and we don't want that; if you want gate-heavy behavior, Claude Code has plenty. Two things cover this instead:
Authoring stays auto-confirm, same as Settings, and the system addendum already states this honestly. L5 — no per-user count cap / model-ended create loop → FIXEDAdded
Nits — FIXED
Residual
Tests
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #533 (re-run)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: df9d05b on plan/meta-mcp
Lenses run: L1, L2, L5, L6, L8
AGENTS.md read: yes
Prior CONCERNS
| Prior | Status |
|---|---|
| Major L2 always-on write/delete, no user-intent gate | Accepted (product). No confirm/modal. Recovery is #534 (version/rollback, not in this PR) + a future status bar. System addendum still honest. Residual until #534 ships. |
| Major L5 unbounded create / persona list | Fixed in the tool layer. META_USER_PERSONAS_MAX (50) / META_USER_SKILLS_MAX (200); create rejects at ceiling; persona list sliced; skill list still SKILL_FIND_RESULT_MAX (20). Tests added. |
| Nit L8 “meta MCP” heading | Fixed. |
| Nit L8 trace vs execute width | Fixed in header + docs/mcp.md. |
| Residual UTF-8 split | Fixed (back off continuation bytes). Tested. |
Findings (this pass)
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Nit | L5 | Ceiling is tool-layer only (list().length then insert). Store has no N-row limit. Settings can still create past 50/200 (error text even says “edit in Settings”). Two parallel *_create in one step can both pass the check (AI SDK is usually serial). |
User + agent race, or Settings spam, then list/find_skill is large. Agent loop alone cannot. |
Store-side unique/count if you want a hard cap. Not a merge blocker for the stated L5 (model-ended loop). | high |
Checked and not re-opened:
- IDOR / closed-over
userId/not_foundunchanged. isMetaToolNamestill excluded from the 403 soft-path count.- Over-cap body writes still rejected; skill read still marked-truncated.
Residual risk
L2 stands until #534 (and the status bar) exist. A “summarize this URL” turn can still delete or set_default standing orders; rollback is the chosen backstop, and it is not on this branch. Tool-layer count is racy vs Settings. Derived-slug _2 retry still omitted (model can pass an explicit slug).
Merge guidance
OK to merge for #531 if you accept L2 until #534. Do not treat versioning as shipped by this PR.
What was not attacked
Live model complying with injected delete; Settings create past 200 against a real DB.
Summary
Fixes #531 (plan-review → HANDOFF-READY) · Refs #530 (parent epic) · product #332.
Implements Phase 1 of the built-in meta tool surface: first-party persona + skill authoring tools on
/api/agent, bound to the signed-in caller, so the agent can manage its own personas and skills (in addition to Settings). Mirrors the shippedskillTools(find_skill/fetch_skill) pattern as an in-process AI SDKtool()family — not a remote MCP transport, no secrets, no enable flag.meta_persona_*—list/read/create/update_name/update_body/set_default/clear_default/deletemeta_skill_*—list/read/create/update_summary/update_body/deleteWhat changed
lib/agent/metaTools.ts(new) —createMetaPersonaSkillTools({userId, userPersonas, userSkills}), always-on, bound to routeuserId(model-passed identity ignored; tenant+user scoped →not_foundno-existence-leak). Author-as-user, auto-confirm (same as Settings). Slugs derived in the tool layer when omitted (both stores require a slug on create, per plan-review finding). Bodies returned to the model only on explicit read, capped; writes over-cap rejected (never truncated). Tool traces are short one-liners for the existingtool_runpaint.app/api/agent/route.ts— assembles meta tools always intoextraTools; extends the soft-pathnonSkillToolCountguard to also excludemeta_*keys viaisMetaToolName(a config-only meta turn never masks a workspace-absent/grant-deny).lib/agent/runAgent.ts—resolveSystem:hasMeta→SKILL_META_ONLY_SYSTEMbase +META_TOOLS_SYSTEM_ADDENDUM.lib/agent/metaTools.test.ts(12 cases): list/read with no-existence-leak, persona + skill round-trips, derived slugs, over-cap rejection (write), skill-read truncation marker, tool-name separation from read-onlyfind_skill/fetch_skill, bound-identity guard.docs/mcp.md(built-in meta MCP section),docs/personas.md,docs/skills.md(search vs authoring split; supersedes the "agent never edits skills" line),AGENTS.mdrow,README.mdpointer.Verification
npm run typecheck— greennode scripts/di-gate.mjs— green (no in-body I/O construction; tools receive DI services)node_modules/vitest/vitest.mjs run— 1477 passed / 0 failed (111 files). Includes the PR's 12 newmetaTools.test.tscases (route + runAgent tests still green).npm run build— green (Next build;public/harnesspresent, so no token needed)Expected CI delta: full suite
baseline → baseline + 12new test cases.Scope
Backend-only (
/api/agenttool surface + system addendum) + living docs. No schema/data/secret change → Cloud ops N/A. No dual-chat, no new bridge protocol (tool activity flows through the existingtool_run). Phase 2 (#532) sandbox inventory/switch is a separate plan/PR.Do not merge on this skill — stops at merge-ready. Next step: adversarial-review if you want it, then an explicit merge request.