From abe2a664366153bb07a30ca3144a69c8c4604f6b Mon Sep 17 00:00:00 2001
From: chiguyong <chiguyong@beyondsoft.com>
Date: Mon, 22 Jun 2026 15:24:31 +0800
Subject: [PATCH] fix(review): CLI field names, Pydantic validation, exception
 chaining

---
 src/agentkit/cli/admin.py                       |  6 +++---
 src/agentkit/server/admin/skill_service.py      |  4 ++--
 src/agentkit/server/routes/admin.py             | 16 ++++++++--------
 tests/integration/admin/test_skill_kb_routes.py |  4 ++--
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/agentkit/cli/admin.py b/src/agentkit/cli/admin.py
index a220b1f..427cb96 100644
--- a/src/agentkit/cli/admin.py
+++ b/src/agentkit/cli/admin.py
@@ -1141,15 +1141,15 @@ def kb_list_documents(
     table.add_column("Filename")
     table.add_column("Source ID")
     table.add_column("Department ID")
-    table.add_column("Size")
+    table.add_column("Chunks")
     table.add_column("Created")
     for d in documents:
         table.add_row(
-            str(d.get("id", "")),
+            str(d.get("document_id", "")),
             str(d.get("filename", "")),
             str(d.get("source_id", "")),
             str(d.get("department_id", "")),
-            str(d.get("size", "")),
+            str(d.get("chunks", "")),
             str(d.get("created_at", "")),
         )
     console.print(table)
diff --git a/src/agentkit/server/admin/skill_service.py b/src/agentkit/server/admin/skill_service.py
index faee160..502a4f3 100644
--- a/src/agentkit/server/admin/skill_service.py
+++ b/src/agentkit/server/admin/skill_service.py
@@ -265,14 +265,14 @@ class SkillService:
                     tool_registry=tool_registry,
                 )
                 loader.load_from_file(file_path)
-            except Exception:
+            except Exception as exc:
                 # Remove the invalid YAML file and re-raise as ValueError
                 # so the route layer maps it to 400.
                 try:
                     os.remove(file_path)
                 except OSError:
                     pass
-                raise ValueError("Skill YAML written but registration failed")
+                raise ValueError(f"Skill YAML written but registration failed: {exc}") from exc
 
         return {"name": skill_name, "path": file_path}
 
diff --git a/src/agentkit/server/routes/admin.py b/src/agentkit/server/routes/admin.py
index bc940be..284fb11 100644
--- a/src/agentkit/server/routes/admin.py
+++ b/src/agentkit/server/routes/admin.py
@@ -18,11 +18,11 @@ from __future__ import annotations
 import logging
 from datetime import datetime
 from pathlib import Path
-from typing import Any
+from typing import Any, Literal
 
 from fastapi import APIRouter, Depends, HTTPException, Request
 from fastapi.responses import PlainTextResponse
-from pydantic import BaseModel, ConfigDict
+from pydantic import BaseModel, ConfigDict, EmailStr, Field
 
 from agentkit.server.admin.department_service import get_department_service
 from agentkit.server.admin.kb_service import get_kb_service
@@ -348,9 +348,9 @@ class UserCreateRequest(BaseModel):
     model_config = ConfigDict(extra="forbid")
 
     username: str
-    email: str
-    password: str
-    role: str = "member"
+    email: EmailStr
+    password: str = Field(min_length=8)
+    role: Literal["member", "operator", "admin"] = "member"
     department_ids: list[str] | None = None
 
 
@@ -359,7 +359,7 @@ class UserUpdateRequest(BaseModel):
 
     model_config = ConfigDict(extra="forbid")
 
-    role: str | None = None
+    role: Literal["member", "operator", "admin"] | None = None
     is_active: bool | None = None
     is_terminal_authorized: bool | None = None
     is_server_terminal_authorized: bool | None = None
@@ -370,7 +370,7 @@ class ResetPasswordRequest(BaseModel):
 
     model_config = ConfigDict(extra="forbid")
 
-    new_password: str
+    new_password: str = Field(min_length=8)
 
 
 @admin_router.post("/users", status_code=201)
@@ -982,7 +982,7 @@ async def update_skill(
         raise HTTPException(status_code=400, detail=msg) from exc
 
 
-@admin_router.post("/skills/import")
+@admin_router.post("/skills/import", status_code=201)
 async def import_skill(
     payload: SkillImportRequest,
     request: Request,
diff --git a/tests/integration/admin/test_skill_kb_routes.py b/tests/integration/admin/test_skill_kb_routes.py
index 45f61bf..0f97d27 100644
--- a/tests/integration/admin/test_skill_kb_routes.py
+++ b/tests/integration/admin/test_skill_kb_routes.py
@@ -250,7 +250,7 @@ class TestSkillEnableDisable:
 
 
 class TestSkillImport:
-    def test_import_valid_yaml_returns_200(
+    def test_import_valid_yaml_returns_201(
         self,
         admin_client: TestClient,
         skills_dir: str,
@@ -260,7 +260,7 @@ class TestSkillImport:
             "/api/v1/admin/skills/import",
             json={"yaml_content": _VALID_SKILL_YAML},
         )
-        assert resp.status_code == 200, resp.text
+        assert resp.status_code == 201, resp.text
         body = resp.json()
         assert body["name"] == "admin_test_skill"
         assert os.path.isfile(body["path"])