← Back to task

Commit 254cc02f

commit 254cc02f1a0262c48ea864b97c6d1f99994cfc92
Author: Ben Sima <ben@bensima.com>
Date:   Thu Jan 1 00:53:05 2026

    Add role tag parsing to skill loader (t-308)
    
    When skills contain <role:X> tags, the skill loader now:
    1. Parses unique role names from the skill body
    2. Loads role definitions from Omni/Agent/Roles/X.md
    3. Prepends a 'Role Definitions' section to the skill
    
    This allows skills to reference roles like <role:product> and have
    the role's thinking guidelines automatically injected.
    
    Example output:
    ## Role Definitions
    
    ### Product
    When thinking as Product, focus on...
    
    ---
    
    <original skill body with role tags>
    
    Task-Id: t-308

diff --git a/Omni/Agent/Skills.hs b/Omni/Agent/Skills.hs
index 1dbf23fa..64b5ff80 100644
--- a/Omni/Agent/Skills.hs
+++ b/Omni/Agent/Skills.hs
@@ -56,7 +56,7 @@ test =
     "Omni.Agent.Skills"
     [ Test.unit "skillsDir returns correct path" <| do
         let dir = skillsDir
-        ("skills" `Text.isSuffixOf` Text.pack dir) Test.@=? True,
+        ("Skills" `Text.isSuffixOf` Text.pack dir) Test.@=? True,
       Test.unit "SkillMetadata parses from YAML frontmatter" <| do
         let yaml = "name: test-skill\ndescription: A test skill"
         case parseYamlFrontmatter yaml of
@@ -87,7 +87,19 @@ test =
         let schema = Engine.toolJsonSchema (listSkillsTool "test-user")
         case schema of
           Aeson.Object _ -> pure ()
-          _ -> Test.assertFailure "Schema should be an object"
+          _ -> Test.assertFailure "Schema should be an object",
+      Test.unit "parseRoleTags extracts role names" <| do
+        let body = "Do <role:product> stuff </role>\nThen <role:engineer> code </role>"
+        parseRoleTags body Test.@=? ["product", "engineer"],
+      Test.unit "parseRoleTags handles no roles" <| do
+        parseRoleTags "Just some text without roles" Test.@=? [],
+      Test.unit "parseRoleTags deduplicates" <| do
+        let body = "<role:product> first </role> and <role:product> again </role>"
+        parseRoleTags body Test.@=? ["product"],
+      Test.unit "capitalizeFirst works" <| do
+        capitalizeFirst "product" Test.@=? "Product"
+        capitalizeFirst "" Test.@=? ""
+        capitalizeFirst "ALREADY" Test.@=? "ALREADY"
     ]
 
 -- | Base directory for all skills
@@ -154,6 +166,85 @@ instance Aeson.ToJSON Skill where
         "path" .= skillPath s
       ]
 
+-- | Directory containing role definitions
+rolesDir :: FilePath
+rolesDir = "Omni/Agent/Roles"
+
+-- | Extract unique role names from <role:X> tags in skill body
+parseRoleTags :: Text -> [Text]
+parseRoleTags body =
+  let segments = Text.splitOn "<role:" body
+      roleNames =
+        [ Text.toLower (Text.takeWhile (/= '>') segment)
+          | segment <- drop 1 segments,  -- Skip text before first <role:
+            let roleName = Text.takeWhile (/= '>') segment,
+            not (Text.null roleName)
+        ]
+   in List.nub roleNames
+
+-- | Load a role definition file
+loadRole :: Text -> IO (Either Text Text)
+loadRole roleName = do
+  let filename = Text.unpack (capitalizeFirst roleName) <> ".md"
+      path = rolesDir FilePath.</> filename
+  exists <- Directory.doesFileExist path
+  if exists
+    then do
+      content <- TextIO.readFile path
+      -- Parse out the body (skip YAML frontmatter if present)
+      let stripped = Text.strip content
+      if Text.isPrefixOf "---" stripped
+        then do
+          let afterFirst = Text.drop 3 stripped
+              (_, rest) = Text.breakOn "---" (Text.stripStart afterFirst)
+          if Text.null rest
+            then pure <| Right stripped  -- No closing ---, use whole content
+            else pure <| Right <| Text.strip (Text.drop 3 rest)
+        else pure <| Right stripped
+    else do
+      -- List available roles for helpful error message
+      roleFiles <- Directory.listDirectory rolesDir
+      let availableRoles = [ Text.pack (FilePath.dropExtension f) | f <- roleFiles, ".md" `List.isSuffixOf` f ]
+      pure
+        <| Left
+        <| "Role not found: "
+        <> roleName
+        <> ". Available roles: "
+        <> Text.intercalate ", " (map Text.toLower availableRoles)
+
+-- | Capitalize first letter of text
+capitalizeFirst :: Text -> Text
+capitalizeFirst t = case Text.uncons t of
+  Nothing -> t
+  Just (c, rest) -> Text.cons (toUpper c) rest
+
+-- | Format role definitions as a header block to prepend to skill
+formatRoleDefinitions :: [(Text, Text)] -> Text
+formatRoleDefinitions roles
+  | null roles = ""
+  | otherwise =
+      "## Role Definitions\n\n"
+        <> Text.intercalate "\n\n" [formatRole name body | (name, body) <- roles]
+        <> "\n\n---\n\n"
+  where
+    formatRole name body = "### " <> capitalizeFirst name <> "\n" <> body
+
+-- | Inject role definitions into a skill body
+injectRoles :: Text -> IO (Either Text Text)
+injectRoles body = do
+  let roleNames = parseRoleTags body
+  if null roleNames
+    then pure <| Right body  -- No roles, return unchanged
+    else do
+      results <- forM roleNames <| \name -> do
+        result <- loadRole name
+        pure (name, result)
+      let errors = [err | (_, Left err) <- results]
+          loaded = [(name, content) | (name, Right content) <- results]
+      if not (null errors)
+        then pure <| Left (Text.intercalate "; " errors)
+        else pure <| Right <| formatRoleDefinitions loaded <> body
+
 -- | Parse SKILL.md content into metadata and body
 parseSkillMd :: Text -> Maybe (SkillMetadata, Text)
 parseSkillMd content = do
@@ -209,15 +300,20 @@ loadSkill userName skillName' = do
           content <- TextIO.readFile skillMd
           case parseSkillMd content of
             Nothing -> pure <| Left "Failed to parse SKILL.md frontmatter"
-            Just (meta, body) ->
-              pure
-                <| Right
-                <| Skill
-                  { skillName = skillMetaName meta,
-                    skillDescription = skillMetaDescription meta,
-                    skillBody = body,
-                    skillPath = dir
-                  }
+            Just (meta, body) -> do
+              -- Inject role definitions if skill has <role:X> tags
+              enrichedBodyResult <- injectRoles body
+              case enrichedBodyResult of
+                Left err -> pure <| Left ("Failed to load roles: " <> err)
+                Right enrichedBody ->
+                  pure
+                    <| Right
+                    <| Skill
+                      { skillName = skillMetaName meta,
+                        skillDescription = skillMetaDescription meta,
+                        skillBody = enrichedBody,
+                        skillPath = dir
+                      }
         else pure <| Left ("SKILL.md not found in " <> Text.pack dir)
 
 -- | List all skills in a directory