feat: persist skills as message parts like AGENTS.md (#23748)

## Summary Skills are now discovered once on the first turn (or when the workspace agent changes) and persisted as `skill` message parts alongside `context-file` parts. On subsequent turns, the skill index is reconstructed from persisted parts instead of re-dialing the workspace agent. This makes skills consistent with the AGENTS.md pattern and is groundwork for a future `/context` endpoint that surfaces loaded workspace context to the frontend. ## Changes - Add `skill` `ChatMessagePartType` with `SkillName` and `SkillDescription` fields - Extend `persistInstructionFiles` to also discover and persist skills as parts - Add `skillsFromParts()` to reconstruct skill index from persisted parts on subsequent turns - Update `runChat()` to use `skillsFromParts` instead of re-dialing workspace for skills - Frontend: handle new `skill` part type (skip rendering, hide metadata-only messages) ## Before / After | | AGENTS.md | Skills | |---|---|---| | **Before** | Persist as `context-file` parts, reconstruct from parts | In-memory `skillsCache` only, re-dial workspace on cache miss | | **After** | Persist as `context-file` parts, reconstruct from parts | Persist as `skill` parts, reconstruct from parts | The in-memory `skillsCache` remains for `read_skill`/`read_skill_file` tool calls that need full skill bodies on demand. <details><summary>Design context</summary> This is the first step toward a unified workspace context representation. Currently: - Context files are persisted as message parts (works) - Skills were only in-memory (inconsistent) - Workspace MCP servers are cached in-memory (future work) Persisting skills as parts means a future `/context` endpoint can query both context files and skills from the same message parts in the DB, without depending on ephemeral server-side caches. </details>
2026-09-24 15:04:27 +08:00 · 2026-03-29 21:48:17 -04:00
parent f7aa46c4ba
commit 4d2b0a2f82
9 changed files with 309 additions and 115 deletions
@@ -125,10 +125,6 @@ type Server struct {
 	// keyed by chat ID and invalidated when the agent changes.
 	workspaceMCPToolsCache sync.Map // uuid.UUID -> *cachedWorkspaceMCPTools

-	// skillsCache caches discovered skill metadata per chat so
-	// we avoid re-scanning .agents/skills/ on every turn.
-	skillsCache sync.Map // uuid.UUID -> *cachedSkills
-
 	usageTracker *workspacestats.UsageTracker
 	clock        quartz.Clock

@@ -181,69 +177,21 @@ type cachedWorkspaceMCPTools struct {
 	tools   []workspacesdk.MCPToolInfo
 }

-// cachedSkills stores discovered skill metadata from a workspace
-// agent, keyed by the agent ID that provided them.
-type cachedSkills struct {
-	agentID uuid.UUID
-	skills  []chattool.SkillMeta
-}
-
-// discoverWorkspaceSkills returns cached skill metadata for a chat
-// or discovers them fresh using the provided agent connection. The
-// result is cached per chat+agent so subsequent turns skip the
-// filesystem scan.
-func (p *Server) discoverWorkspaceSkills(
-	ctx context.Context,
-	chatID uuid.UUID,
-	agent database.WorkspaceAgent,
-	conn workspacesdk.AgentConn,
-	logger slog.Logger,
-) []chattool.SkillMeta {
-	// Check cache first.
-	if cached, ok := p.skillsCache.Load(chatID); ok {
-		if entry, ok2 := cached.(*cachedSkills); ok2 {
-			if entry.agentID == agent.ID {
-				return entry.skills
-			}
-		}
-	}
-
-	dir := agent.ExpandedDirectory
-	if dir == "" {
-		dir = agent.Directory
-	}
-	discovered, err := chattool.DiscoverSkills(ctx, conn, dir)
-	if err != nil {
-		logger.Warn(ctx, "failed to discover skills",
-			slog.Error(err))
-		return nil
-	}
-	// Cache the result. Unlike MCP tools, an empty skills
-	// list is a valid stable state (the workspace simply has
-	// no skills), so we always cache.
-	p.skillsCache.Store(chatID, &cachedSkills{
-		agentID: agent.ID,
-		skills:  discovered,
-	})
-	return discovered
-}
-
-// loadCachedWorkspaceContext checks the MCP tools and skills caches
-// for the given chat and agent. Returns non-nil tools when the MCP
-// cache hits, which signals the caller to skip the slow discovery
-// path. Skills may also be populated from the skills cache.
+// loadCachedWorkspaceContext checks the MCP tools cache for the
+// given chat and agent. Returns non-nil tools when the cache hits,
+// which signals the caller to skip the slow MCP discovery path.
 func (p *Server) loadCachedWorkspaceContext(
 	chatID uuid.UUID,
 	agent database.WorkspaceAgent,
 	getConn func(context.Context) (workspacesdk.AgentConn, error),
-) ([]fantasy.AgentTool, []chattool.SkillMeta) {
+) []fantasy.AgentTool {
 	cached, ok := p.workspaceMCPToolsCache.Load(chatID)
 	if !ok {
-		return nil, nil
+		return nil
 	}
 	entry, ok := cached.(*cachedWorkspaceMCPTools)
 	if !ok || entry.agentID != agent.ID {
-		return nil, nil
+		return nil
 	}

 	var tools []fantasy.AgentTool
@@ -251,14 +199,7 @@ func (p *Server) loadCachedWorkspaceContext(
 		tools = append(tools, chattool.NewWorkspaceMCPTool(t, getConn))
 	}

-	var skills []chattool.SkillMeta
-	if sc, ok := p.skillsCache.Load(chatID); ok {
-		if se, ok := sc.(*cachedSkills); ok && se.agentID == agent.ID {
-			skills = se.skills
-		}
-	}
-
-	return tools, skills
+	return tools
 }

 type turnWorkspaceContext struct {
@@ -2679,7 +2620,6 @@ func (p *Server) cleanupStreamIfIdle(chatID uuid.UUID, state *chatStreamState) {
 	if !state.buffering && len(state.subscribers) == 0 {
 		p.chatStreams.Delete(chatID)
 		p.workspaceMCPToolsCache.Delete(chatID)
-		p.skillsCache.Delete(chatID)
 	}
 }

@@ -3935,7 +3875,7 @@ func (p *Server) runChat(
 	if needsInstructionPersist {
 		g2.Go(func() error {
 			var persistErr error
-			instruction, persistErr = p.persistInstructionFiles(
+			instruction, skills, persistErr = p.persistInstructionFiles(
 				ctx,
 				chat,
 				modelConfig.ID,
@@ -3956,10 +3896,12 @@ func (p *Server) runChat(
 			return nil
 		})
 	} else if hasContextFiles {
-		// On subsequent turns, extract the instruction text from
-		// the persisted context-file parts so it can be re-injected
-		// via InsertSystem after compaction drops those messages.
+		// On subsequent turns, extract the instruction text and
+		// skill index from persisted parts so they can be
+		// re-injected via InsertSystem after compaction drops
+		// those messages. No workspace dial needed.
 		instruction = instructionFromContextFiles(messages)
+		skills = skillsFromParts(messages)
 	}
 	g2.Go(func() error {
 		resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID)
@@ -3983,7 +3925,7 @@ func (p *Server) runChat(
 			// query on the common subsequent-turn path.
 			agent, agentErr := workspaceCtx.getWorkspaceAgent(ctx)
 			if agentErr == nil {
-				if workspaceMCPTools, skills = p.loadCachedWorkspaceContext(
+				if workspaceMCPTools = p.loadCachedWorkspaceContext(
 					chat.ID, agent, workspaceCtx.getWorkspaceConn,
 				); workspaceMCPTools != nil {
 					return nil
@@ -4001,7 +3943,6 @@ func (p *Server) runChat(
 			if agentErr != nil {
 				if xerrors.Is(agentErr, errChatHasNoWorkspaceAgent) {
 					p.workspaceMCPToolsCache.Delete(chat.ID)
-					p.skillsCache.Delete(chat.ID)
 					return nil
 				}
 				logger.Warn(ctx, "failed to resolve workspace agent for MCP tools",
@@ -4009,29 +3950,19 @@ func (p *Server) runChat(
 				return nil
 			}

-			// Discover skills and MCP tools using the
-			// same conn to avoid a second dial attempt.
+			// List workspace MCP tools via the agent conn.
 			conn, connErr := workspaceCtx.getWorkspaceConn(workspaceMCPCtx)
 			if connErr != nil {
 				logger.Warn(ctx, "failed to get workspace conn for MCP tools",
 					slog.Error(connErr))
 				return nil
 			}
-
-			agent, agentErr = workspaceCtx.getWorkspaceAgent(workspaceMCPCtx)
-			if agentErr == nil {
-				skills = p.discoverWorkspaceSkills(
-					workspaceMCPCtx, chat.ID, agent, conn, logger,
-				)
-			}
-
 			toolsResp, listErr := conn.ListMCPTools(workspaceMCPCtx)
 			if listErr != nil {
 				logger.Warn(ctx, "failed to list workspace MCP tools",
 					slog.Error(listErr))
 				return nil
 			}
-
 			// Cache the result for subsequent turns. Skip
 			// caching when the list is empty because the
 			// agent's MCP Connect may not have finished yet;
@@ -4909,25 +4840,26 @@ func contextFileAgentID(messages []database.ChatMessage) (uuid.UUID, bool) {
 	return lastID, found
 }

-// persistInstructionFiles reads instruction files from the workspace
-// agent and persists them as context-file message parts. This is called
-// once when a workspace is first attached to a chat. Returns the
-// formatted instruction string for injection into the current turn's
-// prompt.
+// persistInstructionFiles reads instruction files and discovers
+// skills from the workspace agent, persisting both as message
+// parts. This is called once when a workspace is first attached
+// to a chat (or when the agent changes). Returns the formatted
+// instruction string and skill index for injection into the
+// current turn's prompt.
 func (p *Server) persistInstructionFiles(
 	ctx context.Context,
 	chat database.Chat,
 	modelConfigID uuid.UUID,
 	getWorkspaceAgent func(context.Context) (database.WorkspaceAgent, error),
 	getWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error),
-) (string, error) {
+) (string, []chattool.SkillMeta, error) {
 	if !chat.WorkspaceID.Valid || getWorkspaceAgent == nil {
-		return "", nil
+		return "", nil, nil
 	}

 	agent, err := getWorkspaceAgent(ctx)
 	if err != nil {
-		return "", nil
+		return "", nil, nil
 	}

 	directory := agent.ExpandedDirectory
@@ -4970,20 +4902,47 @@ func (p *Server) persistInstructionFiles(
 		}
 	}

+	// Discover skills from the workspace while we have a
+	// connection. Errors are non-fatal — a chat without skills
+	// still works, it just won't list them in the prompt.
+	var discoveredSkills []chattool.SkillMeta
+	if workspaceConnOK {
+		conn, connErr := getWorkspaceConn(ctx)
+		if connErr == nil {
+			var discoverErr error
+			discoveredSkills, discoverErr = chattool.DiscoverSkills(ctx, conn, directory)
+			if discoverErr != nil {
+				p.logger.Debug(ctx, "failed to discover skills",
+					slog.F("chat_id", chat.ID),
+					slog.Error(discoverErr),
+				)
+			}
+		}
+	}
+
 	if len(sections) == 0 {
 		if !workspaceConnOK {
-			return "", nil
+			return "", nil, nil
 		}
-		// Persist a sentinel so subsequent turns skip the
-		// workspace agent dial.
+		// Persist a sentinel (plus any discovered skill parts)
+		// so subsequent turns skip the workspace agent dial.
 		parts := []codersdk.ChatMessagePart{{
 			Type:               codersdk.ChatMessagePartTypeContextFile,
 			ContextFilePath:    "",
 			ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
 		}}
+		for _, s := range discoveredSkills {
+			parts = append(parts, codersdk.ChatMessagePart{
+				Type:               codersdk.ChatMessagePartTypeSkill,
+				SkillName:          s.Name,
+				SkillDescription:   s.Description,
+				SkillDir:           s.Dir,
+				ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
+			})
+		}
 		content, err := chatprompt.MarshalParts(parts)
 		if err != nil {
-			return "", nil
+			return "", nil, nil
 		}
 		msgParams := database.InsertChatMessagesParams{ //nolint:exhaustruct // Fields populated by appendChatMessage.
 			ChatID: chat.ID,
@@ -4996,11 +4955,12 @@ func (p *Server) persistInstructionFiles(
 			chatprompt.CurrentContentVersion,
 		))
 		_, _ = p.db.InsertChatMessages(ctx, msgParams)
-		return "", nil
+		return "", discoveredSkills, nil
 	}

-	// Build context-file parts, one per instruction file.
-	parts := make([]codersdk.ChatMessagePart, 0, len(sections))
+	// Build context-file parts (one per instruction file) and
+	// skill parts (one per discovered skill).
+	parts := make([]codersdk.ChatMessagePart, 0, len(sections)+len(discoveredSkills))
 	for _, s := range sections {
 		parts = append(parts, codersdk.ChatMessagePart{
 			Type:                 codersdk.ChatMessagePartTypeContextFile,
@@ -5012,10 +4972,19 @@ func (p *Server) persistInstructionFiles(
 			ContextFileDirectory: directory,
 		})
 	}
+	for _, s := range discoveredSkills {
+		parts = append(parts, codersdk.ChatMessagePart{
+			Type:               codersdk.ChatMessagePartTypeSkill,
+			SkillName:          s.Name,
+			SkillDescription:   s.Description,
+			SkillDir:           s.Dir,
+			ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
+		})
+	}

 	content, err := chatprompt.MarshalParts(parts)
 	if err != nil {
-		return "", xerrors.Errorf("marshal context-file parts: %w", err)
+		return "", nil, xerrors.Errorf("marshal context-file parts: %w", err)
 	}

 	msgParams := database.InsertChatMessagesParams{ //nolint:exhaustruct // Fields populated by appendChatMessage.
@@ -5029,13 +4998,13 @@ func (p *Server) persistInstructionFiles(
 		chatprompt.CurrentContentVersion,
 	))
 	if _, err := p.db.InsertChatMessages(ctx, msgParams); err != nil {
-		return "", xerrors.Errorf("persist instruction files: %w", err)
+		return "", nil, xerrors.Errorf("persist instruction files: %w", err)
 	}

-	// Return the formatted instruction text so the caller can inject
-	// it into this turn's prompt (since the prompt was built before
-	// we persisted).
-	return formatSystemInstructions(agent.OperatingSystem, directory, sections), nil
+	// Return the formatted instruction text and discovered skills
+	// so the caller can inject them into this turn's prompt (since
+	// the prompt was built before we persisted).
+	return formatSystemInstructions(agent.OperatingSystem, directory, sections), discoveredSkills, nil
 }

 // resolveUserCompactionThreshold looks up the user's per-model
@@ -24,6 +24,7 @@ import (
 	coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
 	"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
 	"github.com/coder/coder/v2/coderd/x/chatd/chattest"
+	"github.com/coder/coder/v2/coderd/x/chatd/chattool"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/workspacesdk"
 	"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
@@ -489,9 +490,8 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) {
 	conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
 		workspacesdk.LSResponse{},
 		codersdk.NewTestError(404, "POST", "/api/v0/list-directory"),
-	).Times(1)
-	conn.EXPECT().ReadFile(
-		gomock.Any(),
+	).AnyTimes()
+	conn.EXPECT().ReadFile(gomock.Any(),
 		"/home/coder/project/AGENTS.md",
 		int64(0),
 		int64(maxInstructionFileBytes+1),
@@ -520,7 +520,7 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) {
 	}
 	t.Cleanup(workspaceCtx.close)

-	instruction, err := server.persistInstructionFiles(
+	instruction, _, err := server.persistInstructionFiles(
 		ctx,
 		chat,
 		uuid.New(),
@@ -551,7 +551,7 @@ func TestPersistInstructionFilesSkipsSentinelWhenWorkspaceUnavailable(t *testing
 		logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
 	}

-	instruction, err := server.persistInstructionFiles(
+	instruction, _, err := server.persistInstructionFiles(
 		ctx,
 		chat,
 		uuid.New(),
@@ -1515,6 +1515,156 @@ func requireFieldValue(t *testing.T, entry slog.SinkEntry, name string, expected
 	t.Fatalf("field %q not found in log entry", name)
 }

+func TestSkillsFromParts(t *testing.T) {
+	t.Parallel()
+
+	t.Run("Empty", func(t *testing.T) {
+		t.Parallel()
+		got := skillsFromParts(nil)
+		require.Empty(t, got)
+	})
+
+	t.Run("NoSkillParts", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{Type: codersdk.ChatMessagePartTypeText, Text: "hello"},
+			}),
+		}
+		got := skillsFromParts(msgs)
+		require.Empty(t, got)
+	})
+
+	t.Run("SingleSkill", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{
+					Type:             codersdk.ChatMessagePartTypeSkill,
+					SkillName:        "deep-review",
+					SkillDescription: "Multi-reviewer code review",
+					SkillDir:         "/home/coder/.agents/skills/deep-review",
+				},
+			}),
+		}
+		got := skillsFromParts(msgs)
+		require.Len(t, got, 1)
+		require.Equal(t, "deep-review", got[0].Name)
+		require.Equal(t, "Multi-reviewer code review", got[0].Description)
+		require.Equal(t, "/home/coder/.agents/skills/deep-review", got[0].Dir)
+	})
+
+	t.Run("MultipleSkillsAcrossMessages", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{
+					Type:      codersdk.ChatMessagePartTypeSkill,
+					SkillName: "pull-requests",
+					SkillDir:  "/home/coder/.agents/skills/pull-requests",
+				},
+			}),
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{
+					Type:      codersdk.ChatMessagePartTypeSkill,
+					SkillName: "deep-review",
+					SkillDir:  "/home/coder/.agents/skills/deep-review",
+				},
+			}),
+		}
+		got := skillsFromParts(msgs)
+		require.Len(t, got, 2)
+		require.Equal(t, "pull-requests", got[0].Name)
+		require.Equal(t, "deep-review", got[1].Name)
+	})
+
+	t.Run("MixedPartTypes", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{
+					Type:            codersdk.ChatMessagePartTypeContextFile,
+					ContextFilePath: "/home/coder/.coder/AGENTS.md",
+				},
+				{
+					Type:      codersdk.ChatMessagePartTypeSkill,
+					SkillName: "refine-plan",
+					SkillDir:  "/home/coder/.agents/skills/refine-plan",
+				},
+			}),
+			// A text-only message should be skipped entirely.
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{Type: codersdk.ChatMessagePartTypeText, Text: "user turn"},
+			}),
+		}
+		got := skillsFromParts(msgs)
+		require.Len(t, got, 1)
+		require.Equal(t, "refine-plan", got[0].Name)
+		require.Equal(t, "/home/coder/.agents/skills/refine-plan", got[0].Dir)
+	})
+
+	t.Run("OptionalDescriptionOmitted", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			chatMessageWithParts([]codersdk.ChatMessagePart{
+				{
+					Type:      codersdk.ChatMessagePartTypeSkill,
+					SkillName: "refine-plan",
+					SkillDir:  "/home/coder/.agents/skills/refine-plan",
+				},
+			}),
+		}
+		got := skillsFromParts(msgs)
+		require.Len(t, got, 1)
+		require.Equal(t, "refine-plan", got[0].Name)
+		require.Empty(t, got[0].Description)
+	})
+
+	t.Run("InvalidJSON", func(t *testing.T) {
+		t.Parallel()
+		msgs := []database.ChatMessage{
+			{
+				Content: pqtype.NullRawMessage{
+					RawMessage: []byte(`not valid json with "skill" in it`),
+					Valid:      true,
+				},
+			},
+		}
+		got := skillsFromParts(msgs)
+		require.Empty(t, got)
+	})
+
+	t.Run("RoundTrip", func(t *testing.T) {
+		// Simulate persist -> reconstruct cycle: marshal skill
+		// parts the same way persistInstructionFiles does, then
+		// verify skillsFromParts recovers the metadata.
+		t.Parallel()
+		want := []chattool.SkillMeta{
+			{Name: "deep-review", Description: "Multi-reviewer review", Dir: "/skills/deep-review"},
+			{Name: "pull-requests", Description: "", Dir: "/skills/pull-requests"},
+		}
+		agentID := uuid.New()
+		var parts []codersdk.ChatMessagePart
+		for _, s := range want {
+			parts = append(parts, codersdk.ChatMessagePart{
+				Type:               codersdk.ChatMessagePartTypeSkill,
+				SkillName:          s.Name,
+				SkillDescription:   s.Description,
+				SkillDir:           s.Dir,
+				ContextFileAgentID: uuid.NullUUID{UUID: agentID, Valid: true},
+			})
+		}
+		msgs := []database.ChatMessage{chatMessageWithParts(parts)}
+		got := skillsFromParts(msgs)
+		require.Len(t, got, len(want))
+		for i, w := range want {
+			require.Equal(t, w.Name, got[i].Name)
+			require.Equal(t, w.Description, got[i].Description)
+			require.Equal(t, w.Dir, got[i].Dir)
+		}
+	})
+}
+
 func TestContextFileAgentID(t *testing.T) {
 	t.Parallel()

@@ -13,6 +13,7 @@ import (
 	"golang.org/x/xerrors"

 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/x/chatd/chattool"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/workspacesdk"
 )
@@ -202,6 +203,37 @@ func instructionFromContextFiles(
 	return formatSystemInstructions(os, dir, sections)
 }

+// skillsFromParts reconstructs skill metadata from persisted
+// skill parts. This is analogous to instructionFromContextFiles
+// so the skill index can be re-injected after compaction without
+// re-dialing the workspace agent.
+func skillsFromParts(
+	messages []database.ChatMessage,
+) []chattool.SkillMeta {
+	var skills []chattool.SkillMeta
+	for _, msg := range messages {
+		if !msg.Content.Valid ||
+			!bytes.Contains(msg.Content.RawMessage, []byte(`"skill"`)) {
+			continue
+		}
+		var parts []codersdk.ChatMessagePart
+		if err := json.Unmarshal(msg.Content.RawMessage, &parts); err != nil {
+			continue
+		}
+		for _, part := range parts {
+			if part.Type != codersdk.ChatMessagePartTypeSkill {
+				continue
+			}
+			skills = append(skills, chattool.SkillMeta{
+				Name:        part.SkillName,
+				Description: part.SkillDescription,
+				Dir:         part.SkillDir,
+			})
+		}
+	}
+	return skills
+}
+
 // pwdInstructionFilePath returns the absolute path to the AGENTS.md
 // file in the given working directory, or empty if directory is empty.
 func pwdInstructionFilePath(directory string) string {