feat: improve skill uploads and agent runtime stability

This commit is contained in:
吉鑫升 committed 2026-08-27 11:28:03 +08:00
1 parent 0980eea975
commit 1b43205c1b
14 files changed
+655 -142

No files matched your search

@@ -165,10 +165,26 @@ export function SkillPickerSheet({
return; return;
} }
try { try {
const validation = await validateUpload.mutateAsync(file); const validation = await validateUpload.mutateAsync({
file,
targetCategory: "custom",
});
if (validation.conflict) { if (validation.conflict) {
const owner = validation.owner_username ?? validation.owner_user_id;
if (validation.owned_by_other && owner) {
toast.error(
`技能「${validation.skill_name}」已由用户「${owner}」上传,不能覆盖。`,
);
return;
}
if (validation.owned_by_me && validation.can_overwrite) {
toast.warning(
`您已上传过同名技能「${validation.skill_name}」,可到「技能」设置页重复上传并覆盖原版本。`,
);
return;
}
toast.warning( toast.warning(
`已存在同名技能「${validation.skill_name}」,请到「技能」设置页上传以选择是否覆盖。`, `已存在同名技能「${validation.skill_name}」,当前不可覆盖。`,
); );
return; return;
} }
@@ -1201,7 +1201,24 @@ function InstallSkillDialog({
const validateMutation = useValidateSkillUpload(); const validateMutation = useValidateSkillUpload();
const installMutation = useInstallSkillUpload(); const installMutation = useInstallSkillUpload();
const setNameZhMutation = useSetSkillNameZh(); const setNameZhMutation = useSetSkillNameZh();
const generateNameZhMutation = useGenerateSkillNameZh(); const generateNameZhMutation = useGenerateSkillNameZh();
const validateFile = useCallback(
async (selected: File, category: "custom" | "public") => {
setValidation(null);
setValidationError(null);
try {
const result = await validateMutation.mutateAsync({
file: selected,
targetCategory: category,
});
setValidation(result);
} catch (err) {
setValidationError(err instanceof Error ? err.message : String(err));
}
},
[validateMutation],
);
const reset = useCallback(() => { const reset = useCallback(() => {
setFile(null); setFile(null);
@@ -1240,19 +1257,12 @@ function InstallSkillDialog({
if (!lower.endsWith(".zip") && !lower.endsWith(".skill")) { if (!lower.endsWith(".zip") && !lower.endsWith(".skill")) {
toast.error(dialog.rejectInvalidExt); toast.error(dialog.rejectInvalidExt);
return; return;
} }
setFile(selected); setFile(selected);
setValidation(null); await validateFile(selected, targetCategory);
setValidationError(null); },
try { [dialog.rejectInvalidExt, targetCategory, validateFile],
const result = await validateMutation.mutateAsync(selected); );
setValidation(result);
} catch (err) {
setValidationError(err instanceof Error ? err.message : String(err));
}
},
[dialog.rejectInvalidExt, validateMutation],
);
const onDrop = useCallback( const onDrop = useCallback(
(e: React.DragEvent<HTMLDivElement>) => { (e: React.DragEvent<HTMLDivElement>) => {
@@ -1329,12 +1339,29 @@ function InstallSkillDialog({
} }
}, [dialog.nameZhSaved, installedName, nameZhDraft, onClose, reset, setNameZhMutation]); }, [dialog.nameZhSaved, installedName, nameZhDraft, onClose, reset, setNameZhMutation]);
const isPublicConflict = validation?.conflict && validation.conflict_kind === "public"; const isPublicConflict = validation?.conflict && validation.conflict_kind === "public";
const isCustomConflict = validation?.conflict && validation.conflict_kind === "custom"; const isCustomConflict = validation?.conflict && validation.conflict_kind === "custom";
// Admin targeting public directory can overwrite an existing public skill. // Admin targeting public directory can overwrite an existing public skill.
const adminCanOverwritePublic = isAdmin && targetCategory === "public" && !!isPublicConflict; const adminCanOverwritePublic = isAdmin && targetCategory === "public" && !!isPublicConflict;
const canInstall = !!validation && !validationError && !isCustomConflict && (adminCanOverwritePublic ? false : !isPublicConflict); const canInstall = !!validation && !validationError && validation.can_install_as_is;
const canOverwrite = !!validation && !validationError && (!!isCustomConflict || adminCanOverwritePublic); const canOverwrite = !!validation && !validationError && validation.can_overwrite;
const ownCustomCanOverwrite = !!isCustomConflict && !!validation?.owned_by_me && canOverwrite;
const otherCustomConflict = !!isCustomConflict && !!validation?.owned_by_other;
const customConflictMessage = !validation
? ""
: otherCustomConflict
? dialog.conflictOwnedByOther
.replace("{name}", validation.skill_name)
.replace(
"{owner}",
validation.owner_username ?? validation.owner_user_id ?? "—",
)
: validation.owned_by_me
? (ownCustomCanOverwrite
? dialog.conflictOwnedByMe
: dialog.conflictOwnedByMeWrongTarget
).replace("{name}", validation.skill_name)
: dialog.conflictCustom.replace("{name}", validation.skill_name);
return ( return (
<Dialog open={open} onOpenChange={(o) => !o && handleClose()}> <Dialog open={open} onOpenChange={(o) => !o && handleClose()}>
@@ -1453,9 +1480,12 @@ function InstallSkillDialog({
<input <input
type="radio" type="radio"
name="targetCategory" name="targetCategory"
value={cat} value={cat}
checked={targetCategory === cat} checked={targetCategory === cat}
onChange={() => setTargetCategory(cat)} onChange={() => {
setTargetCategory(cat);
if (file) void validateFile(file, cat);
}}
className="accent-primary" className="accent-primary"
/> />
<span>{cat === "custom" ? dialog.categoryCustom : dialog.categoryPublic}</span> <span>{cat === "custom" ? dialog.categoryCustom : dialog.categoryPublic}</span>
@@ -1513,15 +1543,18 @@ function InstallSkillDialog({
</ul> </ul>
</Field> </Field>
)} )}
{isCustomConflict && ( {isCustomConflict && (
<div className="text-amber-600 mt-1 flex items-start gap-2 text-xs"> <div
<AlertCircle className="mt-0.5 size-3.5 shrink-0" /> className={cn(
<span> "mt-1 flex items-start gap-2 text-xs",
{desensitize( ownCustomCanOverwrite ? "text-amber-600" : "text-destructive",
dialog.conflictCustom.replace("{name}", validation.skill_name), )}
)} >
</span> <AlertCircle className="mt-0.5 size-3.5 shrink-0" />
</div> <span>
{desensitize(customConflictMessage)}
</span>
</div>
)} )}
{isPublicConflict && ( {isPublicConflict && (
<div className={cn("mt-1 flex items-start gap-2 text-xs", adminCanOverwritePublic ? "text-amber-600" : "text-destructive")}> <div className={cn("mt-1 flex items-start gap-2 text-xs", adminCanOverwritePublic ? "text-amber-600" : "text-destructive")}>
+12 -6
View File
@@ -591,20 +591,26 @@ export const enUS: Translations = {
chooseFile: "Choose file", chooseFile: "Choose file",
validating: "Validating archive…", validating: "Validating archive…",
installing: "Installing…", installing: "Installing…",
replace: "Replace existing", replace: "Upload again and replace",
install: "Install", install: "Install",
cancel: "Cancel", cancel: "Cancel",
nameLabel: "Name", nameLabel: "Name",
descriptionLabel: "Description", descriptionLabel: "Description",
licenseLabel: "License", licenseLabel: "License",
toolsLabel: "Allowed tools", toolsLabel: "Allowed tools",
warningsLabel: "Warnings", warningsLabel: "Warnings",
conflictCustom: conflictOwnedByMe:
"A custom skill named '{name}' is already installed. Use 'Replace existing' to overwrite (its enabled state will be kept).", "You already uploaded a skill named '{name}'. You may upload it again and replace your existing copy (its enabled state will be kept).",
conflictOwnedByMeWrongTarget:
"The custom skill '{name}' belongs to you. Switch the target to 'Custom skill' to replace it.",
conflictOwnedByOther:
"Skill '{name}' was uploaded by user '{owner}' and cannot be replaced. Rename the skill before uploading.",
conflictCustom:
"A custom skill named '{name}' already exists, but its uploader could not be identified. Rename it to avoid overwriting someone else's skill.",
conflictPublic: conflictPublic:
"A built-in public skill named '{name}' already exists and cannot be overwritten.", "A built-in public skill named '{name}' already exists and cannot be overwritten.",
conflictPublicAdminHint: conflictPublicAdminHint:
"A built-in public skill named '{name}' exists. Switch to 'Built-in' and click 'Replace existing' to overwrite.", "A built-in public skill named '{name}' exists. Click 'Upload again and replace' to overwrite it.",
installSuccess: "Skill '{name}' installed.", installSuccess: "Skill '{name}' installed.",
installSuccessReplaced: "Skill '{name}' replaced.", installSuccessReplaced: "Skill '{name}' replaced.",
rejectInvalidExt: "Only .zip or .skill files are accepted.", rejectInvalidExt: "Only .zip or .skill files are accepted.",
+6 -3
View File
@@ -525,9 +525,12 @@ export interface Translations {
nameLabel: string; nameLabel: string;
descriptionLabel: string; descriptionLabel: string;
licenseLabel: string; licenseLabel: string;
toolsLabel: string; toolsLabel: string;
warningsLabel: string; warningsLabel: string;
conflictCustom: string; conflictOwnedByMe: string;
conflictOwnedByMeWrongTarget: string;
conflictOwnedByOther: string;
conflictCustom: string;
conflictPublic: string; conflictPublic: string;
conflictPublicAdminHint: string; conflictPublicAdminHint: string;
installSuccess: string; installSuccess: string;
+11 -5
View File
@@ -566,18 +566,24 @@ export const zhCN: Translations = {
chooseFile: "选择文件", chooseFile: "选择文件",
validating: "正在校验文件…", validating: "正在校验文件…",
installing: "安装中…", installing: "安装中…",
replace: "覆盖已有", replace: "重复上传并覆盖",
install: "安装", install: "安装",
cancel: "取消", cancel: "取消",
nameLabel: "名称", nameLabel: "名称",
descriptionLabel: "描述", descriptionLabel: "描述",
licenseLabel: "许可证", licenseLabel: "许可证",
toolsLabel: "允许的工具", toolsLabel: "允许的工具",
warningsLabel: "警告", warningsLabel: "警告",
conflictCustom: conflictOwnedByMe:
"已存在同名自定义技能 “{name}”。点击「覆盖已有」可替换(原启用状态保留)。", "您已上传过同名技能 “{name}”,可重复上传并覆盖原版本(原启用状态保留)。",
conflictOwnedByMeWrongTarget:
"同名技能 “{name}” 是您上传的自定义技能,请切换到「自定义技能」后覆盖。",
conflictOwnedByOther:
"技能 “{name}” 已由用户 “{owner}” 上传,不能覆盖,请修改技能名称后再上传。",
conflictCustom:
"已存在同名自定义技能 “{name}”,但无法确认上传用户,为避免误覆盖请修改技能名称。",
conflictPublic: `存在同名内置技能 '{name}',无法覆盖。`, conflictPublic: `存在同名内置技能 '{name}',无法覆盖。`,
conflictPublicAdminHint: `存在同名内置技能 '{name}'。切换到「内置技能」并点击「覆盖已有」可替换。`, conflictPublicAdminHint: `存在同名内置技能 '{name}'。点击「重复上传并覆盖」可替换。`,
installSuccess: `技能 '{name}' 已安装。`, installSuccess: `技能 '{name}' 已安装。`,
installSuccessReplaced: `技能 '{name}' 已替换。`, installSuccessReplaced: `技能 '{name}' 已替换。`,
rejectInvalidExt: `只接受 .zip 或 .skill 文件。`, rejectInvalidExt: `只接受 .zip 或 .skill 文件。`,
+32 -17
View File
@@ -175,16 +175,24 @@ export interface PreviewSkillInstallResponse {
warnings: string[]; warnings: string[];
} }
export interface ValidateSkillUploadResponse { export interface ValidateSkillUploadResponse {
ok: boolean; ok: boolean;
skill_name: string; skill_name: string;
description: string; description: string;
license: string | null; license: string | null;
allowed_tools: string[]; allowed_tools: string[];
conflict: boolean; conflict: boolean;
conflict_kind: "public" | "custom" | null; conflict_kind: "public" | "custom" | null;
warnings: string[]; can_install_as_is: boolean;
} can_overwrite: boolean;
owned_by_me: boolean;
owned_by_other: boolean;
builtin_conflict: boolean;
owner_username?: string | null;
owner_user_id?: string | null;
message: string;
warnings: string[];
}
export async function installSkill( export async function installSkill(
request: InstallSkillRequest, request: InstallSkillRequest,
@@ -235,13 +243,20 @@ async function parseDetailedError(response: Response, fallback: string): Promise
return new Error(message); return new Error(message);
} }
export async function validateSkillUpload(file: File): Promise<ValidateSkillUploadResponse> { export async function validateSkillUpload(
const form = new FormData(); file: File,
form.append("file", file, file.name); targetCategory: "public" | "custom" = "custom",
const response = await apiFetch(`${getBackendBaseURL()}/api/skills/validate`, { ): Promise<ValidateSkillUploadResponse> {
method: "POST", const form = new FormData();
body: form, form.append("file", file, file.name);
}); const params = new URLSearchParams({ target_category: targetCategory });
const response = await apiFetch(
`${getBackendBaseURL()}/api/skills/validate?${params.toString()}`,
{
method: "POST",
body: form,
},
);
if (!response.ok) { if (!response.ok) {
throw await parseDetailedError(response, "Validation failed"); throw await parseDetailedError(response, "Validation failed");
} }
+11 -5
View File
@@ -192,11 +192,17 @@ export function useSetSkillAlwaysOn() {
}); });
} }
export function useValidateSkillUpload() { export function useValidateSkillUpload() {
return useMutation({ return useMutation({
mutationFn: (file: File) => validateSkillUpload(file), mutationFn: ({
}); file,
} targetCategory,
}: {
file: File;
targetCategory?: "public" | "custom";
}) => validateSkillUpload(file, targetCategory),
});
}
export function useInstallSkillUpload() { export function useInstallSkillUpload() {
const queryClient = useQueryClient(); const queryClient = useQueryClient();
+64 -25
View File
@@ -244,7 +244,7 @@ export async function stopActiveThreadBeforeClarificationReply(
await new Promise((resolve) => setTimeout(resolve, 120)); await new Promise((resolve) => setTimeout(resolve, 120));
} }
function getStreamErrorMessage(error: unknown): string { function getStreamErrorMessage(error: unknown): string {
if (typeof error === "string" && error.trim()) { if (typeof error === "string" && error.trim()) {
return error; return error;
} }
@@ -264,8 +264,21 @@ function getStreamErrorMessage(error: unknown): string {
return nestedError; return nestedError;
} }
} }
return "Request failed."; return "Request failed.";
} }
/**
* React production error #185 is the minified form of "Maximum update depth
* exceeded". It is a client render-loop guard, not a backend/model failure,
* so it must never be surfaced as the answer's stream error.
*/
function isReactMaximumUpdateDepthError(message: string): boolean {
return (
/maximum update depth exceeded/i.test(message) ||
/minified react error\s*#185\b/i.test(message) ||
/react\.dev\/errors\/185\b/i.test(message)
);
}
const TERMINAL_RUN_STATUSES = new Set([ const TERMINAL_RUN_STATUSES = new Set([
"success", "success",
@@ -575,11 +588,17 @@ export function useThreadStream({
runMetadataStorageRef.current = getRunMetadataStorage(); runMetadataStorageRef.current = getRunMetadataStorage();
} }
const thread = useStream<AgentThreadState>({ const thread = useStream<AgentThreadState>({
client: getAPIClient(isMock), client: getAPIClient(isMock),
assistantId: assistantId ?? "lead_agent", assistantId: assistantId ?? "lead_agent",
threadId: onStreamThreadId, threadId: onStreamThreadId,
reconnectOnMount: runMetadataStorageRef.current // The SDK's current implementation falls back to synchronous external-
// store notifications when this option is omitted. A network chunk can
// contain dozens of token events, causing nested React renders and the
// intermittent production-only React #185 error. Coalesce notifications
// to one visual update per frame; stream data itself is not dropped.
throttle: 16,
reconnectOnMount: runMetadataStorageRef.current
? () => runMetadataStorageRef.current! ? () => runMetadataStorageRef.current!
: false, : false,
fetchStateHistory: { limit: 1 }, fetchStateHistory: { limit: 1 },
@@ -720,10 +739,21 @@ export function useThreadStream({
toast(e.message); toast(e.message);
} }
}, },
onError(error) { onError(error) {
markStreamActivity(); markStreamActivity();
const message = getStreamErrorMessage(error); const message = getStreamErrorMessage(error);
// Replacing a stalled local SSE reader aborts the in-flight fetch. That // A synchronous SDK store notification used to let React's update-depth
// guard escape into the stream promise. Do not present that internal UI
// exception as a model failure. Reconcile/rejoin on the next task so the
// backend run can continue and the current answer is preserved.
if (isReactMaximumUpdateDepthError(message)) {
window.setTimeout(() => {
setStreamHoldOpen(true);
void confirmOrRejoinRef.current("error");
}, 0);
return;
}
// Replacing a stalled local SSE reader aborts the in-flight fetch. That
// is expected and must not look like a user-facing "network" failure. // is expected and must not look like a user-facing "network" failure.
if (isAbortLikeError(error)) { if (isAbortLikeError(error)) {
return; return;
@@ -1390,19 +1420,28 @@ export function useThreadStream({
} }
: thread.values; : thread.values;
const mergedMessages = const mergedMessages =
Array.isArray(mergedValues.messages) Array.isArray(mergedValues.messages)
? (mergedValues.messages as Message[]) ? (mergedValues.messages as Message[])
: thread.messages; : thread.messages;
const mergedThread = const visibleThreadError =
optimisticMessages.length > 0 || mergedValues !== thread.values thread.error &&
? ({ isReactMaximumUpdateDepthError(getStreamErrorMessage(thread.error))
...thread, ? undefined
values: mergedValues, : thread.error;
messages: [...mergedMessages, ...optimisticMessages],
} as typeof thread) const mergedThread =
: thread; optimisticMessages.length > 0 ||
mergedValues !== thread.values ||
visibleThreadError !== thread.error
? ({
...thread,
error: visibleThreadError,
values: mergedValues,
messages: [...mergedMessages, ...optimisticMessages],
} as typeof thread)
: thread;
// 第 4 个返回值 `onStreamThreadId` = 底层 useStream 当前实际绑定的线程 id(新建/未保存时为 null)。 // 第 4 个返回值 `onStreamThreadId` = 底层 useStream 当前实际绑定的线程 id(新建/未保存时为 null)。
// 调用方据此判断「流是否已切到全新线程」,避免在切换会话的那一帧把消息发进上一个会话 // 调用方据此判断「流是否已切到全新线程」,避免在切换会话的那一帧把消息发进上一个会话
+6 -2
View File
@@ -732,9 +732,13 @@ MySQL index script synchronized.
`lead_agent.prompt._build_es_query_routing_section` injects the rule into the `lead_agent.prompt._build_es_query_routing_section` injects the rule into the
default lead-agent system prompt. Custom-agent-only prompts receive it only default lead-agent system prompt. Custom-agent-only prompts receive it only
when their explicit skill allowlist contains `es_query`; disabling the switch when their explicit skill allowlist contains `es_query`; disabling the switch
or leaving the prompt blank emits no block. Tests: or leaving the prompt blank emits no block. Immediately before an ordinary Q&A
graph is created, `lead_agent.agent._log_es_query_routing_registration` checks
the exact configured block against the final system-prompt string and logs
`enabled`, allowlist eligibility, `registered`, character count, and a short
SHA-256 fingerprint (never the prompt body). Tests:
`tests/test_es_query_routing_prompt.py`. `tests/test_es_query_routing_prompt.py`.
- **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory. Direct uploads use the ownership-aware `POST /api/skills/validate?target_category=...` preflight plus `POST /api/skills/install-upload`: a same-owner custom conflict may be uploaded again with `overwrite=true`, while a different owner's conflict is non-overwritable and returns that owner's username (falling back to user id). The install endpoint repeats the ownership check, including for admins targeting `custom`, so callers cannot bypass the preflight. Public-skill overwrite remains admin-only.
- **Installable sentiment analysis package**: `skill-packages/sentiment-analysis.skill` is deliberately separate from the existing frontend virtual-agent path. Its self-contained `config/sentiment-agent.json` owns the external AG-UI address; Basic Auth is resolved from `SENTIMENT_AGENT_AUTH_USERNAME` / `SENTIMENT_AGENT_AUTH_PASSWORD` at runtime (not committed into the package). `scripts/sentiment_stream.py` uses only the Python standard library and converts upstream SSE to stable NDJSON events (`thinking_delta`, `answer_delta`, `done`), with a Markdown convenience mode for normal agent calls. Installation/calling guide: `skill-packages/sentiment-analysis/docs/调用与安装说明.md`. - **Installable sentiment analysis package**: `skill-packages/sentiment-analysis.skill` is deliberately separate from the existing frontend virtual-agent path. Its self-contained `config/sentiment-agent.json` owns the external AG-UI address; Basic Auth is resolved from `SENTIMENT_AGENT_AUTH_USERNAME` / `SENTIMENT_AGENT_AUTH_PASSWORD` at runtime (not committed into the package). `scripts/sentiment_stream.py` uses only the Python standard library and converts upstream SSE to stable NDJSON events (`thinking_delta`, `answer_delta`, `done`), with a Markdown convenience mode for normal agent calls. Installation/calling guide: `skill-packages/sentiment-analysis/docs/调用与安装说明.md`.
- **Chinese name (`name_zh`)**: a Chinese display name stored in the **`skills` DB table** (`SkillRow.name_zh`, `String(128)`, migration `20260609_01`) ? never written to the on-disk SKILL.md, so it works for built-in skills too. Threaded through `SkillStore` create/ensure_legacy/update/update_admin exactly like `detail`; surfaced on `SkillResponse.name_zh` from the ownership record in `_skill_to_response`. Authorization mirrors `detail`: custom skill ? owner or admin; built-in/public ? admin only (row materialized on demand via `ensure_legacy`). Endpoints (in `app/gateway/routers/skills.py`): `PUT /api/skills/{name}/name-zh` (set/clear), `POST /api/skills/{name}/name-zh/generate` (LLM generates one), `POST /api/skills/name-zh/generate-missing` (????: **admin-only** bulk-generate for every skill that lacks one, bounded-concurrency `asyncio.gather`). **Optional model selection**: both generate endpoints accept an optional JSON body `{model_name}` ? an explicit name wins, else the curator model setting, else the main model (`_resolve_skill_name_zh_model_name`). **Model-error surfacing**: `_generate_skill_name_zh` raises `SkillNameZhModelError` when the model itself fails (build/timeout/API error) ? distinct from the model returning an unusable name (? `None`). The single endpoint maps that to `502 ?????,?????????:?`; the bulk endpoint fails fast on a model-build error (502) and otherwise carries a representative `model_error` string in `SkillNameZhBulkResponse` so the UI can say "N ?????????". The frontend (`skill-settings-page.tsx` list + `skill-detail-dialog.tsx` editor, plus the agent skill pickers/badges in `NewAgentPage.tsx` + `agent-card.tsx`) shows `name_zh || name`, offers per-skill "AI ??" plus an **admin-only** header "???????" button, and a compact model dropdown (`components/workspace/model-name-select.tsx`, default = configured model) beside each action. - **Chinese name (`name_zh`)**: a Chinese display name stored in the **`skills` DB table** (`SkillRow.name_zh`, `String(128)`, migration `20260609_01`) ? never written to the on-disk SKILL.md, so it works for built-in skills too. Threaded through `SkillStore` create/ensure_legacy/update/update_admin exactly like `detail`; surfaced on `SkillResponse.name_zh` from the ownership record in `_skill_to_response`. Authorization mirrors `detail`: custom skill ? owner or admin; built-in/public ? admin only (row materialized on demand via `ensure_legacy`). Endpoints (in `app/gateway/routers/skills.py`): `PUT /api/skills/{name}/name-zh` (set/clear), `POST /api/skills/{name}/name-zh/generate` (LLM generates one), `POST /api/skills/name-zh/generate-missing` (????: **admin-only** bulk-generate for every skill that lacks one, bounded-concurrency `asyncio.gather`). **Optional model selection**: both generate endpoints accept an optional JSON body `{model_name}` ? an explicit name wins, else the curator model setting, else the main model (`_resolve_skill_name_zh_model_name`). **Model-error surfacing**: `_generate_skill_name_zh` raises `SkillNameZhModelError` when the model itself fails (build/timeout/API error) ? distinct from the model returning an unusable name (? `None`). The single endpoint maps that to `502 ?????,?????????:?`; the bulk endpoint fails fast on a model-build error (502) and otherwise carries a representative `model_error` string in `SkillNameZhBulkResponse` so the UI can say "N ?????????". The frontend (`skill-settings-page.tsx` list + `skill-detail-dialog.tsx` editor, plus the agent skill pickers/badges in `NewAgentPage.tsx` + `agent-card.tsx`) shows `name_zh || name`, offers per-skill "AI ??" plus an **admin-only** header "???????" button, and a compact model dropdown (`components/workspace/model-name-select.tsx`, default = configured model) beside each action.
+5 -1
View File
@@ -140,6 +140,7 @@ FastAPI application providing REST endpoints for frontend integration:
| `GET/PUT /api/mcp/config` | Manage MCP server configurations | | `GET/PUT /api/mcp/config` | Manage MCP server configurations |
| `GET/PUT /api/skills` | List and manage skills | | `GET/PUT /api/skills` | List and manage skills |
| `POST /api/skills/install` | Install skill from `.skill` archive | | `POST /api/skills/install` | Install skill from `.skill` archive |
| `POST /api/skills/validate`, `POST /api/skills/install-upload` | Validate and upload `.zip`/`.skill` packages with owner-aware conflicts: owners may explicitly overwrite their own copy; other users' copies report the uploader and cannot be overwritten |
| `GET/POST /api/agents` | List visible agents or create a user-owned agent | | `GET/POST /api/agents` | List visible agents or create a user-owned agent |
| `GET/PUT/DELETE /api/agents/{id}` | Read, update, or delete an id-addressed agent | | `GET/PUT/DELETE /api/agents/{id}` | Read, update, or delete an id-addressed agent |
| `GET/POST/PUT/DELETE /api/roundtable-chains` | Manage user-owned roundtable business chains; each seat may optionally carry `position_id` for the separate position-collaboration workspace, plus `human_validation` to mark a Step 2 client-side pause checkpoint after that seat/stage completes (the original roundtable has no extra validation card) | | `GET/POST/PUT/DELETE /api/roundtable-chains` | Manage user-owned roundtable business chains; each seat may optionally carry `position_id` for the separate position-collaboration workspace, plus `human_validation` to mark a Step 2 client-side pause checkpoint after that seat/stage completes (the original roundtable has no extra validation card) |
@@ -508,7 +509,10 @@ Key sections:
- `sandbox` - Execution environment provider - `sandbox` - Execution environment provider
- `skills` - Skills directory paths plus optional prompt routing; `skills.es_query_routing` - `skills` - Skills directory paths plus optional prompt routing; `skills.es_query_routing`
can inject an editable system-prompt rule that sends flexible Elasticsearch Query DSL can inject an editable system-prompt rule that sends flexible Elasticsearch Query DSL
requests to the `es_query` skill, and can be disabled without changing code requests to the `es_query` skill, and can be disabled without changing code. Each ordinary
Q&A agent build logs `ES query routing prompt registration` with `registered`, eligibility,
character count, and a short SHA-256 fingerprint so operators can verify the exact configured
block reached the final model system prompt without logging its contents
- `title` - Auto-title generation settings - `title` - Auto-title generation settings
- `summarization` - Context summarization settings - `summarization` - Context summarization settings
- `subagents` - Subagent system (enabled/disabled) - `subagents` - Subagent system (enabled/disabled)
@@ -272,6 +272,14 @@ class SkillUploadValidateResponse(BaseModel):
allowed_tools: list[str] = Field(default_factory=list, description="Tools the skill may invoke") allowed_tools: list[str] = Field(default_factory=list, description="Tools the skill may invoke")
conflict: bool = Field(default=False, description="True if a skill with the same name already exists") conflict: bool = Field(default=False, description="True if a skill with the same name already exists")
conflict_kind: str | None = Field(default=None, description="'public' or 'custom' when conflict=true") conflict_kind: str | None = Field(default=None, description="'public' or 'custom' when conflict=true")
can_install_as_is: bool = Field(default=True, description="Whether the archive can be installed without overwrite")
can_overwrite: bool = Field(default=False, description="Whether the caller may overwrite the conflicting skill")
owned_by_me: bool = Field(default=False, description="True when the caller owns the conflicting custom skill")
owned_by_other: bool = Field(default=False, description="True when another user owns the conflicting custom skill")
builtin_conflict: bool = Field(default=False, description="True when the name conflicts with a built-in/public skill")
owner_username: str | None = Field(default=None, description="Friendly username for a conflicting skill owned by another user")
owner_user_id: str | None = Field(default=None, description="Owner user id for a conflicting custom skill")
message: str = Field(default="", description="Human-readable upload guidance")
warnings: list[str] = Field(default_factory=list, description="Non-fatal warnings to surface to the user") warnings: list[str] = Field(default_factory=list, description="Non-fatal warnings to surface to the user")
@@ -972,16 +980,55 @@ async def _persist_uploaded_skill(file: UploadFile) -> Path:
"/skills/validate", "/skills/validate",
response_model=SkillUploadValidateResponse, response_model=SkillUploadValidateResponse,
summary="Validate Skill Archive (No Install)", summary="Validate Skill Archive (No Install)",
description="Inspect an uploaded .zip/.skill archive and report parsed metadata + conflicts. Does NOT install or run the LLM security scan.", description="Inspect an uploaded .zip/.skill archive and report parsed metadata plus ownership-aware conflicts. Does NOT install or run the LLM security scan.",
) )
async def validate_skill_upload( async def validate_skill_upload(
request: Request,
file: UploadFile = File(..., description="The .zip or .skill archive to validate"), file: UploadFile = File(..., description="The .zip or .skill archive to validate"),
target_category: SkillCategory = SkillCategory.CUSTOM,
config: AppConfig = Depends(get_config), config: AppConfig = Depends(get_config),
skill_store: SkillStore = Depends(get_skill_store),
) -> SkillUploadValidateResponse: ) -> SkillUploadValidateResponse:
tmp_path = await _persist_uploaded_skill(file) tmp_path = await _persist_uploaded_skill(file)
try: try:
result = get_or_new_skill_storage(app_config=config).validate_skill_archive(tmp_path) result = get_or_new_skill_storage(app_config=config).validate_skill_archive(tmp_path)
return SkillUploadValidateResponse(**result) (
can_install_as_is,
can_overwrite,
owned_by_me,
owned_by_other,
builtin_conflict,
owner_user_id,
owner_username,
message,
) = await _preview_install_target(
request=request,
skill_store=skill_store,
skill_name=str(result.get("skill_name") or ""),
target_category=target_category,
conflict_kind=result.get("conflict_kind"),
)
effective_conflict_kind = result.get("conflict_kind")
if effective_conflict_kind is None:
if owned_by_me or owned_by_other:
effective_conflict_kind = "custom"
elif builtin_conflict:
effective_conflict_kind = "public"
return SkillUploadValidateResponse(
**{
**result,
"conflict": bool(result.get("conflict") or effective_conflict_kind),
"conflict_kind": effective_conflict_kind,
"can_install_as_is": can_install_as_is,
"can_overwrite": can_overwrite,
"owned_by_me": owned_by_me,
"owned_by_other": owned_by_other,
"builtin_conflict": builtin_conflict,
"owner_username": owner_username,
"owner_user_id": owner_user_id,
"message": message,
}
)
except ValueError as e: except ValueError as e:
raise HTTPException(status_code=400, detail=str(e)) raise HTTPException(status_code=400, detail=str(e))
except FileNotFoundError as e: except FileNotFoundError as e:
@@ -999,7 +1046,7 @@ async def validate_skill_upload(
"/skills/install-upload", "/skills/install-upload",
response_model=SkillInstallResponse, response_model=SkillInstallResponse,
summary="Install Skill from Upload", summary="Install Skill from Upload",
description="Install a skill directly from an uploaded .zip/.skill archive. Set ?overwrite=true to replace an existing skill with the same name.", description="Install a skill directly from an uploaded .zip/.skill archive. Set ?overwrite=true to replace the caller's own custom skill, or an existing public skill when installing as an admin.",
) )
async def install_skill_upload( async def install_skill_upload(
request: Request, request: Request,
@@ -1017,22 +1064,30 @@ async def install_skill_upload(
tmp_path = await _persist_uploaded_skill(file) tmp_path = await _persist_uploaded_skill(file)
try: try:
storage = get_or_new_skill_storage(app_config=config) storage = get_or_new_skill_storage(app_config=config)
# Pre-parse the archive to learn the skill name without writing anything, # Re-run the same ownership-aware preflight used by /skills/validate.
# so we can reject the upload before touching the filesystem if another # This closes the time-of-check/time-of-use gap and prevents a crafted
# user already owns this name. Admins bypass the ownership check since # request (including an admin installing into custom/) from replacing a
# they install to the shared public directory. # different user's skill.
try: validation = storage.validate_skill_archive(tmp_path)
validation = storage.validate_skill_archive(tmp_path) preview_name = str(validation.get("skill_name") or "")
preview_name = validation.get("skill_name") if isinstance(validation, dict) else None (
except Exception: can_install_as_is,
preview_name = None can_overwrite,
if preview_name and not is_admin: _owned_by_me,
existing = await skill_store.get_any(preview_name) _owned_by_other,
if existing and existing.get("owner_user_id") not in (None, user_id): _builtin_conflict,
raise HTTPException( _owner_user_id,
status_code=409, _owner_username,
detail=f"Skill '{preview_name}' already exists and is owned by another user", conflict_message,
) ) = await _preview_install_target(
request=request,
skill_store=skill_store,
skill_name=preview_name,
target_category=target_category,
conflict_kind=validation.get("conflict_kind"),
)
if (overwrite and not can_install_as_is and not can_overwrite) or (not overwrite and not can_install_as_is):
raise HTTPException(status_code=409, detail=conflict_message)
result = await storage.ainstall_skill_from_archive(tmp_path, overwrite=overwrite, require_skill_suffix=False, target_category=target_category) result = await storage.ainstall_skill_from_archive(tmp_path, overwrite=overwrite, require_skill_suffix=False, target_category=target_category)
try: try:
from deerflow.skills.usage import mark_uploaded from deerflow.skills.usage import mark_uploaded
@@ -1075,7 +1130,8 @@ async def _preview_install_target(
skill_store: SkillStore, skill_store: SkillStore,
skill_name: str, skill_name: str,
target_category: SkillCategory, target_category: SkillCategory,
) -> tuple[bool, bool, bool, bool, str | None, str | None, str]: conflict_kind: str | None = None,
) -> tuple[bool, bool, bool, bool, bool, str | None, str | None, str]:
"""Return installability / ownership info for a parsed skill name.""" """Return installability / ownership info for a parsed skill name."""
user_id = _current_user_id(request) user_id = _current_user_id(request)
@@ -1085,27 +1141,44 @@ async def _preview_install_target(
owner_username: str | None = None owner_username: str | None = None
if owner_user_id: if owner_user_id:
try: try:
attached = await _attach_owner_usernames( attached = await _attach_owner_usernames([SkillResponse(name=skill_name, description="", license=None, category=SkillCategory.CUSTOM, owner_user_id=owner_user_id)])
[SkillResponse(name=skill_name, description="", license=None, category=SkillCategory.CUSTOM, owner_user_id=owner_user_id)]
)
owner_username = attached[0].owner_username if attached else None owner_username = attached[0].owner_username if attached else None
except Exception: except Exception:
owner_username = None owner_username = None
builtin_conflict = bool(existing and owner_user_id is None) custom_conflict = conflict_kind == "custom" or bool(existing and owner_user_id is not None)
owned_by_me = bool(existing and owner_user_id == user_id) builtin_conflict = conflict_kind == "public" or bool(existing and owner_user_id is None and conflict_kind != "custom")
owned_by_other = bool(existing and owner_user_id not in (None, user_id)) owned_by_me = bool(custom_conflict and owner_user_id == user_id)
owned_by_other = bool(custom_conflict and owner_user_id not in (None, user_id))
unattributed_custom_conflict = bool(custom_conflict and owner_user_id is None and not builtin_conflict)
if target_category == SkillCategory.PUBLIC and not is_admin: if target_category == SkillCategory.PUBLIC and not is_admin:
return False, owned_by_me, owned_by_other, builtin_conflict, owner_user_id, owner_username, "Only admin users can install skills to the public directory" return False, False, owned_by_me, owned_by_other, builtin_conflict, owner_user_id, owner_username, "Only admin users can install skills to the public directory"
if target_category == SkillCategory.PUBLIC:
if custom_conflict:
who = owner_username or owner_user_id
if owned_by_other and who:
message = f"Skill '{skill_name}' has already been uploaded by user '{who}' and cannot be overwritten."
elif owned_by_me:
message = f"You already own custom skill '{skill_name}'. Switch the target to custom to overwrite it."
else:
message = f"Skill '{skill_name}' already exists as a custom skill and its owner could not be determined."
return False, False, owned_by_me, owned_by_other, builtin_conflict, owner_user_id, owner_username, message
if builtin_conflict:
return False, True, False, False, True, owner_user_id, owner_username, f"Built-in/public skill '{skill_name}' already exists and can be overwritten by an admin."
return True, False, False, False, False, owner_user_id, owner_username, f"Skill '{skill_name}' can be installed."
if builtin_conflict: if builtin_conflict:
return False, owned_by_me, owned_by_other, True, owner_user_id, owner_username, f"Skill '{skill_name}' already exists as a built-in/public skill. Please install it under a different name." return False, False, owned_by_me, owned_by_other, True, owner_user_id, owner_username, f"Skill '{skill_name}' already exists as a built-in/public skill. Please install it under a different name."
if owned_by_other: if owned_by_other:
who = owner_username or "another user" who = owner_username or owner_user_id or "another user"
return False, owned_by_me, True, builtin_conflict, owner_user_id, owner_username, f"Skill '{skill_name}' has already been installed by {who}. Please choose a different name." return False, False, False, True, False, owner_user_id, owner_username, f"Skill '{skill_name}' has already been uploaded by user '{who}' and cannot be overwritten."
if owned_by_me: if owned_by_me:
return False, True, False, builtin_conflict, owner_user_id, owner_username, f"You have already installed skill '{skill_name}'. You can keep using it or install this copy under a different name." return False, True, True, False, False, owner_user_id, owner_username, f"You have already uploaded skill '{skill_name}'. You may upload it again and overwrite your existing copy."
return True, False, False, builtin_conflict, owner_user_id, owner_username, f"Skill '{skill_name}' can be installed." if unattributed_custom_conflict:
return False, False, False, False, False, owner_user_id, owner_username, f"Skill '{skill_name}' already exists, but its owner could not be determined. It cannot be overwritten safely."
return True, False, False, False, False, owner_user_id, owner_username, f"Skill '{skill_name}' can be installed."
async def _build_renamed_skill_archive( async def _build_renamed_skill_archive(
@@ -1193,11 +1266,12 @@ async def preview_skill_install(
skill_file_path = await aresolve_thread_virtual_path(body.thread_id, body.path) skill_file_path = await aresolve_thread_virtual_path(body.thread_id, body.path)
validation = get_or_new_skill_storage(app_config=config).validate_skill_archive(skill_file_path, require_skill_suffix=False) validation = get_or_new_skill_storage(app_config=config).validate_skill_archive(skill_file_path, require_skill_suffix=False)
skill_name = str(validation.get("skill_name") or "") skill_name = str(validation.get("skill_name") or "")
can_install_as_is, owned_by_me, owned_by_other, builtin_conflict, owner_user_id, owner_username, message = await _preview_install_target( can_install_as_is, _can_overwrite, owned_by_me, owned_by_other, builtin_conflict, owner_user_id, owner_username, message = await _preview_install_target(
request=request, request=request,
skill_store=skill_store, skill_store=skill_store,
skill_name=skill_name, skill_name=skill_name,
target_category=body.target_category, target_category=body.target_category,
conflict_kind=validation.get("conflict_kind"),
) )
return SkillInstallPreviewResponse( return SkillInstallPreviewResponse(
ok=True, ok=True,
@@ -1248,11 +1322,12 @@ async def install_skill(
storage = get_or_new_skill_storage(app_config=config) storage = get_or_new_skill_storage(app_config=config)
validation = storage.validate_skill_archive(skill_file_path, require_skill_suffix=False) validation = storage.validate_skill_archive(skill_file_path, require_skill_suffix=False)
preview_name = requested_name or str(validation.get("skill_name") or "") preview_name = requested_name or str(validation.get("skill_name") or "")
can_install_as_is, owned_by_me, owned_by_other, builtin_conflict, _owner_user_id, _owner_username, message = await _preview_install_target( can_install_as_is, _can_overwrite, owned_by_me, owned_by_other, builtin_conflict, _owner_user_id, _owner_username, message = await _preview_install_target(
request=request, request=request,
skill_store=skill_store, skill_store=skill_store,
skill_name=preview_name, skill_name=preview_name,
target_category=target_category, target_category=target_category,
conflict_kind=(validation.get("conflict_kind") if requested_name is None or requested_name == validation.get("skill_name") else None),
) )
if not can_install_as_is: if not can_install_as_is:
if requested_name and requested_name != validation.get("skill_name") and not owned_by_other and not builtin_conflict: if requested_name and requested_name != validation.get("skill_name") and not owned_by_other and not builtin_conflict:
@@ -1,4 +1,5 @@
import logging import hashlib
import logging
from langchain.agents import create_agent from langchain.agents import create_agent
from langchain.agents.middleware import AgentMiddleware from langchain.agents.middleware import AgentMiddleware
@@ -32,7 +33,45 @@ logger = logging.getLogger(__name__)
# 写作配置 Q&A 现在「先生成大纲并与用户多轮确认 → 再提交配置」,需要更多来回,故上限放宽。 # 写作配置 Q&A 现在「先生成大纲并与用户多轮确认 → 再提交配置」,需要更多来回,故上限放宽。
# 仍是兜底安全阀:到顶才用 tool_choice 强制提交,正常流程由用户确认大纲后自然提交。 # 仍是兜底安全阀:到顶才用 tool_choice 强制提交,正常流程由用户确认大纲后自然提交。
WRITING_SETUP_MAX_ROUNDS = 8 WRITING_SETUP_MAX_ROUNDS = 8
REPORT_STRUCTURE_SETUP_MAX_ROUNDS = 8 REPORT_STRUCTURE_SETUP_MAX_ROUNDS = 8
_ES_QUERY_ROUTING_OPEN_TAG = '<skill_routing skill="es_query">'
def _log_es_query_routing_registration(
*,
system_prompt: str,
app_config: AppConfig,
agent_id: str | None,
available_skills: set[str] | None,
) -> None:
"""Log whether the exact configured ES routing block reached this Q&A prompt."""
routing = app_config.skills.es_query_routing
configured_prompt = (routing.prompt or "").strip()
expected_block = (
f"{_ES_QUERY_ROUTING_OPEN_TAG}\n{configured_prompt}\n</skill_routing>"
if configured_prompt
else ""
)
eligible = available_skills is None or "es_query" in available_skills
registered = bool(expected_block) and expected_block in system_prompt
fingerprint = (
hashlib.sha256(configured_prompt.encode("utf-8")).hexdigest()[:12]
if configured_prompt
else "-"
)
log = logger.info if registered or not routing.enabled or not eligible else logger.warning
log(
"ES query routing prompt registration: agent_id=%s enabled=%s eligible=%s "
"registered=%s prompt_chars=%d prompt_sha256=%s",
agent_id or "default",
routing.enabled,
eligible,
registered,
len(configured_prompt),
fingerprint,
)
def _get_runtime_config(config: RunnableConfig) -> dict: def _get_runtime_config(config: RunnableConfig) -> dict:
@@ -661,8 +700,8 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
writing_mode=writing_mode, writing_mode=writing_mode,
writing_artifact_path=writing_artifact_path, writing_artifact_path=writing_artifact_path,
) )
else: else:
system_prompt = apply_prompt_template( system_prompt = apply_prompt_template(
subagent_enabled=subagent_enabled, subagent_enabled=subagent_enabled,
max_concurrent_subagents=max_concurrent_subagents, max_concurrent_subagents=max_concurrent_subagents,
agent_id=agent_id, agent_id=agent_id,
@@ -672,12 +711,19 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
available_skills=prompt_available_skills, available_skills=prompt_available_skills,
app_config=resolved_app_config, app_config=resolved_app_config,
memory_injection_enabled=memory_injection_enabled, memory_injection_enabled=memory_injection_enabled,
rag_mode_enabled=rag_mode_enabled, rag_mode_enabled=rag_mode_enabled,
is_notebook_mode=is_notebook_mode, is_notebook_mode=is_notebook_mode,
notebook_source_ids=notebook_source_ids, notebook_source_ids=notebook_source_ids,
) )
return create_agent( _log_es_query_routing_registration(
system_prompt=system_prompt,
app_config=resolved_app_config,
agent_id=agent_id,
available_skills=prompt_available_skills,
)
return create_agent(
model=create_chat_model(name=model_name, thinking_enabled=thinking_enabled, reasoning_effort=reasoning_effort, force_disable_thinking=thinking_force_disabled, app_config=resolved_app_config), model=create_chat_model(name=model_name, thinking_enabled=thinking_enabled, reasoning_effort=reasoning_effort, force_disable_thinking=thinking_force_disabled, app_config=resolved_app_config),
tools=default_tools, tools=default_tools,
middleware=_build_middlewares(config, model_name=model_name, agent_name=agent_id, app_config=resolved_app_config), middleware=_build_middlewares(config, model_name=model_name, agent_name=agent_id, app_config=resolved_app_config),
@@ -1,5 +1,7 @@
import logging
from types import SimpleNamespace from types import SimpleNamespace
from deerflow.agents.lead_agent import agent as agent_module
from deerflow.agents.lead_agent import prompt as prompt_module from deerflow.agents.lead_agent import prompt as prompt_module
from deerflow.config.skills_config import EsQueryRoutingConfig, SkillsConfig from deerflow.config.skills_config import EsQueryRoutingConfig, SkillsConfig
@@ -68,3 +70,41 @@ def test_es_query_routing_is_injected_into_lead_and_allowed_custom_agent_prompts
assert "自定义 ES 路由规则" in lead_prompt assert "自定义 ES 路由规则" in lead_prompt
assert '<skill_routing skill="es_query">' in allowed_custom_prompt assert '<skill_routing skill="es_query">' in allowed_custom_prompt
assert '<skill_routing skill="es_query">' not in denied_custom_prompt assert '<skill_routing skill="es_query">' not in denied_custom_prompt
def test_es_query_routing_registration_log_confirms_exact_final_prompt(caplog) -> None:
config = _app_config(enabled=True, prompt="自定义 ES 路由规则")
system_prompt = '<skill_routing skill="es_query">\n自定义 ES 路由规则\n</skill_routing>'
with caplog.at_level(logging.INFO, logger=agent_module.__name__):
agent_module._log_es_query_routing_registration(
system_prompt=system_prompt,
app_config=config,
agent_id=None,
available_skills=None,
)
assert "ES query routing prompt registration" in caplog.text
assert "agent_id=default" in caplog.text
assert "enabled=True" in caplog.text
assert "eligible=True" in caplog.text
assert "registered=True" in caplog.text
assert "prompt_chars=11" in caplog.text
assert "prompt_sha256=" in caplog.text
def test_es_query_routing_registration_log_explains_custom_agent_exclusion(caplog) -> None:
config = _app_config(enabled=True, prompt="自定义 ES 路由规则")
with caplog.at_level(logging.INFO, logger=agent_module.__name__):
agent_module._log_es_query_routing_registration(
system_prompt="custom agent prompt",
app_config=config,
agent_id="agent-without-es",
available_skills={"other_skill"},
)
assert "agent_id=agent-without-es" in caplog.text
assert "enabled=True" in caplog.text
assert "eligible=False" in caplog.text
assert "registered=False" in caplog.text
@@ -5,11 +5,16 @@ from __future__ import annotations
import io import io
import zipfile import zipfile
from pathlib import Path from pathlib import Path
from types import SimpleNamespace
import pytest import pytest
from fastapi import HTTPException
from app.gateway.routers import skills as skills_router
from deerflow.persistence.skills import MemorySkillStore
from deerflow.skills.installer import SkillAlreadyExistsError from deerflow.skills.installer import SkillAlreadyExistsError
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
from deerflow.skills.types import SkillCategory
def _make_skill_zip( def _make_skill_zip(
@@ -239,3 +244,218 @@ async def test_install_refuses_to_overwrite_public_skill(
await storage.ainstall_skill_from_archive( await storage.ainstall_skill_from_archive(
archive, overwrite=True, require_skill_suffix=False archive, overwrite=True, require_skill_suffix=False
) )
# ---------- ownership-aware direct upload conflicts ----------
def _request(user_id: str, *, is_admin: bool = False) -> SimpleNamespace:
return SimpleNamespace(
state=SimpleNamespace(
user=SimpleNamespace(
id=user_id,
system_role="admin" if is_admin else "user",
)
)
)
async def _attach_test_owner_names(responses):
for response in responses:
if response.owner_user_id == "user-b":
response.owner_username = "bob"
return responses
class _UploadStorage:
def __init__(self) -> None:
self.install_calls = 0
def validate_skill_archive(self, _path: Path) -> dict:
return {
"ok": True,
"skill_name": "demo-skill",
"description": "demo",
"license": None,
"allowed_tools": [],
"conflict": True,
"conflict_kind": "custom",
"warnings": [],
}
async def ainstall_skill_from_archive(self, *_args, **_kwargs) -> dict:
self.install_calls += 1
return {
"success": True,
"skill_name": "demo-skill",
"message": "installed",
"overwritten": True,
"category": "custom",
}
@pytest.mark.asyncio
async def test_upload_preflight_allows_owner_to_overwrite(
monkeypatch: pytest.MonkeyPatch,
) -> None:
store = MemorySkillStore()
await store.create({"name": "demo-skill", "owner_user_id": "user-a"})
monkeypatch.setattr(skills_router, "_attach_owner_usernames", _attach_test_owner_names)
result = await skills_router._preview_install_target(
request=_request("user-a"),
skill_store=store,
skill_name="demo-skill",
target_category=SkillCategory.CUSTOM,
conflict_kind="custom",
)
can_install_as_is, can_overwrite, owned_by_me, owned_by_other, *_rest = result
assert can_install_as_is is False
assert can_overwrite is True
assert owned_by_me is True
assert owned_by_other is False
assert "upload it again and overwrite" in result[-1]
@pytest.mark.asyncio
async def test_upload_preflight_names_the_other_owner(
monkeypatch: pytest.MonkeyPatch,
) -> None:
store = MemorySkillStore()
await store.create({"name": "demo-skill", "owner_user_id": "user-b"})
monkeypatch.setattr(skills_router, "_attach_owner_usernames", _attach_test_owner_names)
result = await skills_router._preview_install_target(
request=_request("user-a"),
skill_store=store,
skill_name="demo-skill",
target_category=SkillCategory.CUSTOM,
conflict_kind="custom",
)
can_install_as_is, can_overwrite, owned_by_me, owned_by_other = result[:4]
assert can_install_as_is is False
assert can_overwrite is False
assert owned_by_me is False
assert owned_by_other is True
assert result[6] == "bob"
assert "user 'bob'" in result[-1]
@pytest.mark.asyncio
async def test_validate_upload_surfaces_the_other_owner(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
store = MemorySkillStore()
await store.create({"name": "demo-skill", "owner_user_id": "user-b"})
storage = _UploadStorage()
upload_path = tmp_path / "validate.skill"
upload_path.write_bytes(b"placeholder")
async def _persist(_file) -> Path:
return upload_path
monkeypatch.setattr(skills_router, "_persist_uploaded_skill", _persist)
monkeypatch.setattr(
skills_router,
"get_or_new_skill_storage",
lambda **_kwargs: storage,
)
monkeypatch.setattr(
skills_router,
"_attach_owner_usernames",
_attach_test_owner_names,
)
response = await skills_router.validate_skill_upload(
request=_request("user-a"),
file=object(),
target_category=SkillCategory.CUSTOM,
config=object(),
skill_store=store,
)
assert response.conflict is True
assert response.conflict_kind == "custom"
assert response.can_overwrite is False
assert response.owned_by_other is True
assert response.owner_username == "bob"
@pytest.mark.asyncio
async def test_direct_upload_owner_can_overwrite_own_skill(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
store = MemorySkillStore()
await store.create({"name": "demo-skill", "owner_user_id": "user-a"})
storage = _UploadStorage()
upload_path = tmp_path / "owner-upload.skill"
upload_path.write_bytes(b"placeholder")
async def _persist(_file) -> Path:
return upload_path
async def _refresh() -> None:
return None
monkeypatch.setattr(skills_router, "_persist_uploaded_skill", _persist)
monkeypatch.setattr(
skills_router,
"get_or_new_skill_storage",
lambda **_kwargs: storage,
)
monkeypatch.setattr(
skills_router,
"refresh_skills_system_prompt_cache_async",
_refresh,
)
monkeypatch.setattr("deerflow.skills.usage.mark_uploaded", lambda _name: None)
response = await skills_router.install_skill_upload(
request=_request("user-a"),
file=object(),
overwrite=True,
target_category=SkillCategory.CUSTOM,
config=object(),
skill_store=store,
)
assert response.success is True
assert response.overwritten is True
assert storage.install_calls == 1
@pytest.mark.asyncio
async def test_direct_upload_rechecks_owner_before_overwrite(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
) -> None:
store = MemorySkillStore()
await store.create({"name": "demo-skill", "owner_user_id": "user-b"})
storage = _UploadStorage()
upload_path = tmp_path / "upload.skill"
upload_path.write_bytes(b"placeholder")
async def _persist(_file) -> Path:
return upload_path
monkeypatch.setattr(skills_router, "_persist_uploaded_skill", _persist)
monkeypatch.setattr(skills_router, "get_or_new_skill_storage", lambda **_kwargs: storage)
monkeypatch.setattr(skills_router, "_attach_owner_usernames", _attach_test_owner_names)
with pytest.raises(HTTPException) as exc_info:
await skills_router.install_skill_upload(
request=_request("user-a", is_admin=True),
file=object(),
overwrite=True,
target_category=SkillCategory.CUSTOM,
config=object(),
skill_store=store,
)
assert exc_info.value.status_code == 409
assert "user 'bob'" in str(exc_info.value.detail)
assert storage.install_calls == 0