skills: managed-tool so the agent can author its own skills (with mid-session hydration)#194
skills: managed-tool so the agent can author its own skills (with mid-session hydration)#194ishaan-berri wants to merge 5 commits into
Conversation
…ydration) The shared, harness-agnostic skill tool spec. Mirrors managed-tools/memory.ts: schemas, descriptions, HTTP handlers — no harness glue. save_skill dedups by exact name match against GET /api/v1/skills, then PATCHes the existing row or POSTs a new one. After save, it detaches-then-attaches the skill to AGENT_ID so the marker stays single-instance in agent.prompt and the next session's hydration picks it up. Finally writes ~/.claude/skills/<slug>/SKILL.md with synthesized frontmatter so the current session can pick it up mid-run without waiting for a restart. slugifySkillName + ensureSkillFrontmatter are intentionally byte-identical copies of src/server/k8s.ts so the mid-session write path lands at the same location the platform would use when hydrating on next boot.
…ete_skill Mirrors memory-tools.ts: builds an in-process MCP server (lap-skills) whose three tools wrap the shared handlers in @lap/managed-tools/skills. Returns null when LAP_BASE_URL/AGENT_ID/LAP_AUTH_TOKEN aren't all set, so local-dev without the platform keeps booting cleanly with the tools simply absent.
Each in-process MCP is independently optional now: a half-configured env that only builds one of the two still gets that one wired in. Tool names from both servers are merged into allowedTools.
Greptile SummaryThis PR adds three agent-facing managed tools —
Confidence Score: 3/5Safe to merge for single-skill agents; agents with two skills whose names reduce to the same ASCII slug will see incorrect mid-session file writes and deletes. The slug collision handling in k8s.ts appends a -2/-3 counter when multiple attached skills share the same base slug, but skills.ts never applies that counter. When two skills collide, writeLocalSkill silently overwrites the first skill's SKILL.md and removeLocalSkill deletes the wrong directory — the stated invariant that the mid-session write lands at the same path as next-session hydration is broken in that case. managed-tools/src/skills.ts — the writeLocalSkill and callDeleteSkill functions both need awareness of the collision-suffix logic from k8s.ts.
|
| Filename | Overview |
|---|---|
| managed-tools/src/skills.ts | Core skill CRUD logic; slug collision handling diverges from k8s.ts, causing mid-session writes and deletes to target the wrong local directory when two skills share a base slug. Also writes input.content to disk instead of the server's returned content. |
| harnesses/claude-agent-sdk/src/skill-tools.ts | Thin SDK adapter wrapping the managed-tools spec; mirrors memory-tools.ts cleanly with no issues. |
| harnesses/claude-agent-sdk/src/server.ts | Registers the new skills MCP server alongside memory; each MCP is independently optional — clean change. |
| managed-tools/package.json | Adds the ./skills subpath export; straightforward. |
| managed-tools/src/index.ts | Re-exports all public symbols from skills.ts; no issues. |
Reviews (1): Last reviewed commit: "harness(claude-agent-sdk): register lap-..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // 3) Write the skill to the running container's skills directory so the | ||
| // current session sees it without restart. Claude Code rescans this | ||
| // directory between turns (no SDK restart needed) — but if the active | ||
| // harness happens to cache its skill list at boot, the file is still | ||
| // written and will pick up on the next session anyway. | ||
| const slug = slugifySkillName(savedSkill.name, savedSkill.skill_id); | ||
| const localPath = writeLocalSkill(slug, savedSkill, input); |
There was a problem hiding this comment.
k8s.ts (buildSkillsJsonForAgent) appends -2, -3, … when two attached skills produce the same base slug. skills.ts calls the bare slugifySkillName without the dedup counter, so the mid-session path is always <base-slug>, never <base-slug-2>.
Concrete failure: the agent has two skills whose names both reduce to the slug "code-review". k8s.ts hydrates them to code-review/ and code-review-2/. When save_skill is called for the second skill it writes to code-review/SKILL.md (the first skill's file), corrupting it. When delete_skill is called for the second skill it removes the code-review/ directory (again the first skill's), leaving code-review-2/ as a ghost on disk.
| // 3) Write the skill to the running container's skills directory so the | |
| // current session sees it without restart. Claude Code rescans this | |
| // directory between turns (no SDK restart needed) — but if the active | |
| // harness happens to cache its skill list at boot, the file is still | |
| // written and will pick up on the next session anyway. | |
| const slug = slugifySkillName(savedSkill.name, savedSkill.skill_id); | |
| const localPath = writeLocalSkill(slug, savedSkill, input); | |
| // 3) Write the skill to the running container's skills directory so the | |
| // current session sees it without restart. Claude Code rescans this | |
| // directory between turns (no SDK restart needed) — but if the active | |
| // harness happens to cache its skill list at boot, the file is still | |
| // written and will pick up on the next session anyway. | |
| // | |
| // NOTE: We don't have access to the full ordered list of attached skills | |
| // here, so we cannot reproduce the collision-suffix logic from k8s.ts. | |
| // The slug written here will match k8s.ts as long as no two attached | |
| // skills share the same base slug. If they do, callers should restart | |
| // the session to get a consistent hydration from the server. | |
| const slug = slugifySkillName(savedSkill.name, savedSkill.skill_id); | |
| const localPath = writeLocalSkill(slug, savedSkill, input); |
| if (row) { | ||
| const slug = slugifySkillName(row.name, row.skill_id); | ||
| removeLocalSkill(slug); | ||
| } |
There was a problem hiding this comment.
delete_skill silently removes the wrong local directory on slug collision
When two attached skills share a base slug and the second was assigned a -2 suffix by k8s.ts, delete_skill computes the unsuffixed slug and removes the first skill's directory instead of the second's. The second skill's SKILL.md remains active in the current session after a supposedly successful delete.
| if (row) { | |
| const slug = slugifySkillName(row.name, row.skill_id); | |
| removeLocalSkill(slug); | |
| } | |
| if (row) { | |
| // Best-effort: remove the directory at the base slug. If a collision | |
| // suffix was assigned by k8s.ts (e.g. "code-review-2"), this will | |
| // miss that directory. A session restart will clear the stale file. | |
| const slug = slugifySkillName(row.name, row.skill_id); | |
| removeLocalSkill(slug); | |
| } |
| const body = ensureSkillFrontmatter(input.content, { | ||
| slug, | ||
| name: saved.name, | ||
| description: saved.description ?? input.description, | ||
| }); |
There was a problem hiding this comment.
input.content instead of the server's authoritative content
After a PATCH, savedSkill.content holds the server's canonical value. Writing input.content to disk means any normalization the server applies is skipped in the local file. In a tight save-then-use loop the in-session version could differ from what the server stores, and the server's version wins at next hydration.
| const body = ensureSkillFrontmatter(input.content, { | |
| slug, | |
| name: saved.name, | |
| description: saved.description ?? input.description, | |
| }); | |
| const body = ensureSkillFrontmatter(saved.content ?? input.content, { | |
| slug, | |
| name: saved.name, | |
| description: saved.description ?? input.description, | |
| }); |
| async function callApi<T = unknown>( | ||
| env: SkillsEnv, | ||
| method: "GET" | "POST" | "PATCH" | "DELETE", | ||
| url: string, | ||
| body?: unknown, | ||
| ): Promise<ApiResponse<T>> { | ||
| try { | ||
| const res = await fetch(url, { | ||
| method, | ||
| headers: { | ||
| Authorization: `Bearer ${env.auth_token}`, | ||
| ...(body !== undefined && { "Content-Type": "application/json" }), | ||
| }, | ||
| ...(body !== undefined && { body: JSON.stringify(body) }), | ||
| }); | ||
| const text = await res.text(); | ||
| const data = text ? (safeJson(text) as T) : null; | ||
| return { ok: res.ok, status: res.status, data }; | ||
| } catch (e) { | ||
| return { | ||
| ok: false, | ||
| status: 0, | ||
| data: null, | ||
| error: e instanceof Error ? e.message : String(e), | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
fetch calls — agent turn can hang indefinitely
None of the callApi calls set an AbortSignal or timeout. If the LAP platform is slow or momentarily unreachable during a save_skill, list_skills, or delete_skill call, the agent turn stalls until the OS-level TCP timeout fires (minutes). Passing signal: AbortSignal.timeout(10_000) to fetch bounds the wait and lets the LLM surface an actionable error instead.
1 participant
What
Adds three tools to the claude-agent-sdk harness:
save_skill,list_skills,delete_skill. Lets the agent codify recurring user feedback as a Claude CodeSKILL.mdand have it active on the very next turn — not just the next session.How it works
save_skill(name, description, content):GET /api/v1/skills, look for an exact name match.PATCH /api/v1/skills/{id}. Else →POST /api/v1/skills. Name match is the dedup key — re-passing the same name updates instead of duplicating.AGENT_IDvia the existing/skillroute. Idempotent: detach is a no-op when not attached; attach writes a fresh<!-- skill:id -->marker. After this, the next container boot will hydrate the skill fromSKILLS_JSONautomatically.~/.claude/skills/<slug>/SKILL.mdwith synthesized YAML frontmatter so the running session sees the skill mid-run.slugifySkillName+ensureSkillFrontmatterare byte-identical copies of the helpers insrc/server/k8s.ts, so the mid-session write lands at the same path the platform would use on next boot.list_skills(query?)—GET /api/v1/skills, optional substring filter on name or description.delete_skill(skill_id)— detach from current agent,DELETEthe row, remove the local SKILL.md directory.File layout
managed-tools/src/skills.ts— schemas, descriptions, handlers, slug/frontmatter helpers. Pure async, harness-agnostic.managed-tools/src/index.ts+package.json— re-exports and./skillssubpath.harnesses/claude-agent-sdk/src/skill-tools.ts— ~85-line adapter that wraps the spec increateSdkMcpServer({ name: 'lap-skills', ... }). Mirrorsmemory-tools.ts1:1.harnesses/claude-agent-sdk/src/server.ts— registers the new MCP next tolap-memory. Each MCP is independently optional now (a half-configured env that only builds one of them still wires that one in).Caveats
claudeCLI re-scanning~/.claude/skills/between turns. If a future harness ends up caching its skill list at boot, the tool's success message says so: "active on next session." The file is still written either way.AGENT_ID. That's the right default — the agent is editing its own behavior. Noattach_to_agent: falseescape hatch yet; can add one if a use case shows up.skill-tools.ts.Test plan
cd managed-tools && npm install && npm run buildsucceedscd harnesses/claude-agent-sdk && npx tsc --noEmitcleansave_skillwith a new name creates a Skill row, attaches it, writes the local SKILL.md, and the skill shows up in available-skills on the next turnsave_skillwith the same name PATCHes the existing row instead of creating a duplicatedelete_skillremoves the row, strips the agent's prompt marker, and deletes the local directory