feat: improve skill uploads and agent runtime stability
This commit is contained in:
1 parent
c5eefe60f0
commit
87505feac4
14 files changed
+641
-130
No files matched your search
@@ -165,10 +165,26 @@ export function SkillPickerSheet({
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const validation = await validateUpload.mutateAsync(file);
|
||||
const validation = await validateUpload.mutateAsync({
|
||||
file,
|
||||
targetCategory: "custom",
|
||||
});
|
||||
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(
|
||||
`已存在同名技能「${validation.skill_name}」,请到「技能」设置页上传以选择是否覆盖。`,
|
||||
`已存在同名技能「${validation.skill_name}」,当前不可覆盖。`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1228,7 +1228,24 @@ function InstallSkillDialog({
|
||||
const validateMutation = useValidateSkillUpload();
|
||||
const installMutation = useInstallSkillUpload();
|
||||
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(() => {
|
||||
setFile(null);
|
||||
@@ -1267,19 +1284,12 @@ function InstallSkillDialog({
|
||||
if (!lower.endsWith(".zip") && !lower.endsWith(".skill")) {
|
||||
toast.error(dialog.rejectInvalidExt);
|
||||
return;
|
||||
}
|
||||
setFile(selected);
|
||||
setValidation(null);
|
||||
setValidationError(null);
|
||||
try {
|
||||
const result = await validateMutation.mutateAsync(selected);
|
||||
setValidation(result);
|
||||
} catch (err) {
|
||||
setValidationError(err instanceof Error ? err.message : String(err));
|
||||
}
|
||||
},
|
||||
[dialog.rejectInvalidExt, validateMutation],
|
||||
);
|
||||
}
|
||||
setFile(selected);
|
||||
await validateFile(selected, targetCategory);
|
||||
},
|
||||
[dialog.rejectInvalidExt, targetCategory, validateFile],
|
||||
);
|
||||
|
||||
const onDrop = useCallback(
|
||||
(e: React.DragEvent<HTMLDivElement>) => {
|
||||
@@ -1356,12 +1366,29 @@ function InstallSkillDialog({
|
||||
}
|
||||
}, [dialog.nameZhSaved, installedName, nameZhDraft, onClose, reset, setNameZhMutation]);
|
||||
|
||||
const isPublicConflict = validation?.conflict && validation.conflict_kind === "public";
|
||||
const isCustomConflict = validation?.conflict && validation.conflict_kind === "custom";
|
||||
// Admin targeting public directory can overwrite an existing public skill.
|
||||
const adminCanOverwritePublic = isAdmin && targetCategory === "public" && !!isPublicConflict;
|
||||
const canInstall = !!validation && !validationError && !isCustomConflict && (adminCanOverwritePublic ? false : !isPublicConflict);
|
||||
const canOverwrite = !!validation && !validationError && (!!isCustomConflict || adminCanOverwritePublic);
|
||||
const isPublicConflict = validation?.conflict && validation.conflict_kind === "public";
|
||||
const isCustomConflict = validation?.conflict && validation.conflict_kind === "custom";
|
||||
// Admin targeting public directory can overwrite an existing public skill.
|
||||
const adminCanOverwritePublic = isAdmin && targetCategory === "public" && !!isPublicConflict;
|
||||
const canInstall = !!validation && !validationError && validation.can_install_as_is;
|
||||
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 (
|
||||
<Dialog open={open} onOpenChange={(o) => !o && handleClose()}>
|
||||
@@ -1480,9 +1507,12 @@ function InstallSkillDialog({
|
||||
<input
|
||||
type="radio"
|
||||
name="targetCategory"
|
||||
value={cat}
|
||||
checked={targetCategory === cat}
|
||||
onChange={() => setTargetCategory(cat)}
|
||||
value={cat}
|
||||
checked={targetCategory === cat}
|
||||
onChange={() => {
|
||||
setTargetCategory(cat);
|
||||
if (file) void validateFile(file, cat);
|
||||
}}
|
||||
className="accent-primary"
|
||||
/>
|
||||
<span>{cat === "custom" ? dialog.categoryCustom : dialog.categoryPublic}</span>
|
||||
@@ -1540,15 +1570,18 @@ function InstallSkillDialog({
|
||||
</ul>
|
||||
</Field>
|
||||
)}
|
||||
{isCustomConflict && (
|
||||
<div className="text-amber-600 mt-1 flex items-start gap-2 text-xs">
|
||||
<AlertCircle className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>
|
||||
{desensitize(
|
||||
dialog.conflictCustom.replace("{name}", validation.skill_name),
|
||||
)}
|
||||
</span>
|
||||
</div>
|
||||
{isCustomConflict && (
|
||||
<div
|
||||
className={cn(
|
||||
"mt-1 flex items-start gap-2 text-xs",
|
||||
ownCustomCanOverwrite ? "text-amber-600" : "text-destructive",
|
||||
)}
|
||||
>
|
||||
<AlertCircle className="mt-0.5 size-3.5 shrink-0" />
|
||||
<span>
|
||||
{desensitize(customConflictMessage)}
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
{isPublicConflict && (
|
||||
<div className={cn("mt-1 flex items-start gap-2 text-xs", adminCanOverwritePublic ? "text-amber-600" : "text-destructive")}>
|
||||
|
||||
@@ -591,20 +591,26 @@ export const enUS: Translations = {
|
||||
chooseFile: "Choose file",
|
||||
validating: "Validating archive…",
|
||||
installing: "Installing…",
|
||||
replace: "Replace existing",
|
||||
replace: "Upload again and replace",
|
||||
install: "Install",
|
||||
cancel: "Cancel",
|
||||
nameLabel: "Name",
|
||||
descriptionLabel: "Description",
|
||||
licenseLabel: "License",
|
||||
toolsLabel: "Allowed tools",
|
||||
warningsLabel: "Warnings",
|
||||
conflictCustom:
|
||||
"A custom skill named '{name}' is already installed. Use 'Replace existing' to overwrite (its enabled state will be kept).",
|
||||
warningsLabel: "Warnings",
|
||||
conflictOwnedByMe:
|
||||
"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:
|
||||
"A built-in public skill named '{name}' already exists and cannot be overwritten.",
|
||||
conflictPublicAdminHint:
|
||||
"A built-in public skill named '{name}' exists. Switch to 'Built-in' and click 'Replace existing' to overwrite.",
|
||||
conflictPublicAdminHint:
|
||||
"A built-in public skill named '{name}' exists. Click 'Upload again and replace' to overwrite it.",
|
||||
installSuccess: "Skill '{name}' installed.",
|
||||
installSuccessReplaced: "Skill '{name}' replaced.",
|
||||
rejectInvalidExt: "Only .zip or .skill files are accepted.",
|
||||
|
||||
@@ -525,9 +525,12 @@ export interface Translations {
|
||||
nameLabel: string;
|
||||
descriptionLabel: string;
|
||||
licenseLabel: string;
|
||||
toolsLabel: string;
|
||||
warningsLabel: string;
|
||||
conflictCustom: string;
|
||||
toolsLabel: string;
|
||||
warningsLabel: string;
|
||||
conflictOwnedByMe: string;
|
||||
conflictOwnedByMeWrongTarget: string;
|
||||
conflictOwnedByOther: string;
|
||||
conflictCustom: string;
|
||||
conflictPublic: string;
|
||||
conflictPublicAdminHint: string;
|
||||
installSuccess: string;
|
||||
|
||||
@@ -566,18 +566,24 @@ export const zhCN: Translations = {
|
||||
chooseFile: "选择文件",
|
||||
validating: "正在校验文件…",
|
||||
installing: "安装中…",
|
||||
replace: "覆盖已有",
|
||||
replace: "重复上传并覆盖",
|
||||
install: "安装",
|
||||
cancel: "取消",
|
||||
nameLabel: "名称",
|
||||
descriptionLabel: "描述",
|
||||
licenseLabel: "许可证",
|
||||
toolsLabel: "允许的工具",
|
||||
warningsLabel: "警告",
|
||||
conflictCustom:
|
||||
"已存在同名自定义技能 “{name}”。点击「覆盖已有」可替换(原启用状态保留)。",
|
||||
warningsLabel: "警告",
|
||||
conflictOwnedByMe:
|
||||
"您已上传过同名技能 “{name}”,可重复上传并覆盖原版本(原启用状态保留)。",
|
||||
conflictOwnedByMeWrongTarget:
|
||||
"同名技能 “{name}” 是您上传的自定义技能,请切换到「自定义技能」后覆盖。",
|
||||
conflictOwnedByOther:
|
||||
"技能 “{name}” 已由用户 “{owner}” 上传,不能覆盖,请修改技能名称后再上传。",
|
||||
conflictCustom:
|
||||
"已存在同名自定义技能 “{name}”,但无法确认上传用户,为避免误覆盖请修改技能名称。",
|
||||
conflictPublic: `存在同名内置技能 '{name}',无法覆盖。`,
|
||||
conflictPublicAdminHint: `存在同名内置技能 '{name}'。切换到「内置技能」并点击「覆盖已有」可替换。`,
|
||||
conflictPublicAdminHint: `存在同名内置技能 '{name}'。点击「重复上传并覆盖」可替换。`,
|
||||
installSuccess: `技能 '{name}' 已安装。`,
|
||||
installSuccessReplaced: `技能 '{name}' 已替换。`,
|
||||
rejectInvalidExt: `只接受 .zip 或 .skill 文件。`,
|
||||
|
||||
@@ -175,16 +175,24 @@ export interface PreviewSkillInstallResponse {
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
export interface ValidateSkillUploadResponse {
|
||||
ok: boolean;
|
||||
skill_name: string;
|
||||
description: string;
|
||||
license: string | null;
|
||||
allowed_tools: string[];
|
||||
conflict: boolean;
|
||||
conflict_kind: "public" | "custom" | null;
|
||||
warnings: string[];
|
||||
}
|
||||
export interface ValidateSkillUploadResponse {
|
||||
ok: boolean;
|
||||
skill_name: string;
|
||||
description: string;
|
||||
license: string | null;
|
||||
allowed_tools: string[];
|
||||
conflict: boolean;
|
||||
conflict_kind: "public" | "custom" | null;
|
||||
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(
|
||||
request: InstallSkillRequest,
|
||||
@@ -235,13 +243,20 @@ async function parseDetailedError(response: Response, fallback: string): Promise
|
||||
return new Error(message);
|
||||
}
|
||||
|
||||
export async function validateSkillUpload(file: File): Promise<ValidateSkillUploadResponse> {
|
||||
const form = new FormData();
|
||||
form.append("file", file, file.name);
|
||||
const response = await apiFetch(`${getBackendBaseURL()}/api/skills/validate`, {
|
||||
method: "POST",
|
||||
body: form,
|
||||
});
|
||||
export async function validateSkillUpload(
|
||||
file: File,
|
||||
targetCategory: "public" | "custom" = "custom",
|
||||
): Promise<ValidateSkillUploadResponse> {
|
||||
const form = new FormData();
|
||||
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) {
|
||||
throw await parseDetailedError(response, "Validation failed");
|
||||
}
|
||||
|
||||
@@ -192,11 +192,17 @@ export function useSetSkillAlwaysOn() {
|
||||
});
|
||||
}
|
||||
|
||||
export function useValidateSkillUpload() {
|
||||
return useMutation({
|
||||
mutationFn: (file: File) => validateSkillUpload(file),
|
||||
});
|
||||
}
|
||||
export function useValidateSkillUpload() {
|
||||
return useMutation({
|
||||
mutationFn: ({
|
||||
file,
|
||||
targetCategory,
|
||||
}: {
|
||||
file: File;
|
||||
targetCategory?: "public" | "custom";
|
||||
}) => validateSkillUpload(file, targetCategory),
|
||||
});
|
||||
}
|
||||
|
||||
export function useInstallSkillUpload() {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
@@ -316,7 +316,7 @@ export async function stopActiveThreadBeforeClarificationReply(
|
||||
await new Promise((resolve) => setTimeout(resolve, 120));
|
||||
}
|
||||
|
||||
function getStreamErrorMessage(error: unknown): string {
|
||||
function getStreamErrorMessage(error: unknown): string {
|
||||
if (typeof error === "string" && error.trim()) {
|
||||
return error;
|
||||
}
|
||||
@@ -336,8 +336,21 @@ function getStreamErrorMessage(error: unknown): string {
|
||||
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([
|
||||
"success",
|
||||
@@ -670,11 +683,17 @@ export function useThreadStream({
|
||||
runMetadataStorageRef.current = getRunMetadataStorage();
|
||||
}
|
||||
|
||||
const thread = useStream<AgentThreadState>({
|
||||
client: getAPIClient(isMock),
|
||||
assistantId: assistantId ?? "lead_agent",
|
||||
threadId: onStreamThreadId,
|
||||
reconnectOnMount: runMetadataStorageRef.current
|
||||
const thread = useStream<AgentThreadState>({
|
||||
client: getAPIClient(isMock),
|
||||
assistantId: assistantId ?? "lead_agent",
|
||||
threadId: onStreamThreadId,
|
||||
// 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!
|
||||
: false,
|
||||
fetchStateHistory: { limit: 1 },
|
||||
@@ -820,10 +839,21 @@ export function useThreadStream({
|
||||
toast(e.message);
|
||||
}
|
||||
},
|
||||
onError(error) {
|
||||
markStreamActivity();
|
||||
const message = getStreamErrorMessage(error);
|
||||
// Replacing a stalled local SSE reader aborts the in-flight fetch. That
|
||||
onError(error) {
|
||||
markStreamActivity();
|
||||
const message = getStreamErrorMessage(error);
|
||||
// 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.
|
||||
if (isAbortLikeError(error)) {
|
||||
return;
|
||||
@@ -1646,13 +1676,20 @@ export function useThreadStream({
|
||||
thread.isLoading,
|
||||
) ?? thread.values;
|
||||
|
||||
const mergedMessages =
|
||||
const mergedMessages =
|
||||
Array.isArray(mergedValues?.messages)
|
||||
? (mergedValues.messages as Message[])
|
||||
: thread.messages;
|
||||
|
||||
const mergedThread = {
|
||||
...thread,
|
||||
: thread.messages;
|
||||
|
||||
const visibleThreadError =
|
||||
thread.error &&
|
||||
isReactMaximumUpdateDepthError(getStreamErrorMessage(thread.error))
|
||||
? undefined
|
||||
: thread.error;
|
||||
|
||||
const mergedThread = {
|
||||
...thread,
|
||||
error: visibleThreadError,
|
||||
values: mergedValues,
|
||||
messages:
|
||||
optimisticMessages.length > 0
|
||||
|
||||
@@ -735,9 +735,13 @@ MySQL index script synchronized.
|
||||
`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
|
||||
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`.
|
||||
- **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 frontend virtual-agent chat page. The chat page goes through Gateway `POST /api/sentiment-agent/stream` (runtime-config URL/Basic Auth in the body, TLS verify off, raw AG-UI SSE passthrough). The skill package's self-contained `config/sentiment-agent.json` owns the external AG-UI address for agent-skill callers; 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.
|
||||
|
||||
|
||||
@@ -140,6 +140,7 @@ FastAPI application providing REST endpoints for frontend integration:
|
||||
| `GET/PUT /api/mcp/config` | Manage MCP server configurations |
|
||||
| `GET/PUT /api/skills` | List and manage skills |
|
||||
| `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/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) |
|
||||
@@ -511,7 +512,10 @@ Key sections:
|
||||
- `sandbox` - Execution environment provider
|
||||
- `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
|
||||
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
|
||||
- `summarization` - Context summarization settings
|
||||
- `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")
|
||||
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")
|
||||
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")
|
||||
|
||||
|
||||
@@ -972,16 +980,55 @@ async def _persist_uploaded_skill(file: UploadFile) -> Path:
|
||||
"/skills/validate",
|
||||
response_model=SkillUploadValidateResponse,
|
||||
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(
|
||||
request: Request,
|
||||
file: UploadFile = File(..., description="The .zip or .skill archive to validate"),
|
||||
target_category: SkillCategory = SkillCategory.CUSTOM,
|
||||
config: AppConfig = Depends(get_config),
|
||||
skill_store: SkillStore = Depends(get_skill_store),
|
||||
) -> SkillUploadValidateResponse:
|
||||
tmp_path = await _persist_uploaded_skill(file)
|
||||
try:
|
||||
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:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
except FileNotFoundError as e:
|
||||
@@ -999,7 +1046,7 @@ async def validate_skill_upload(
|
||||
"/skills/install-upload",
|
||||
response_model=SkillInstallResponse,
|
||||
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(
|
||||
request: Request,
|
||||
@@ -1017,22 +1064,30 @@ async def install_skill_upload(
|
||||
tmp_path = await _persist_uploaded_skill(file)
|
||||
try:
|
||||
storage = get_or_new_skill_storage(app_config=config)
|
||||
# Pre-parse the archive to learn the skill name without writing anything,
|
||||
# so we can reject the upload before touching the filesystem if another
|
||||
# user already owns this name. Admins bypass the ownership check since
|
||||
# they install to the shared public directory.
|
||||
try:
|
||||
validation = storage.validate_skill_archive(tmp_path)
|
||||
preview_name = validation.get("skill_name") if isinstance(validation, dict) else None
|
||||
except Exception:
|
||||
preview_name = None
|
||||
if preview_name and not is_admin:
|
||||
existing = await skill_store.get_any(preview_name)
|
||||
if existing and existing.get("owner_user_id") not in (None, user_id):
|
||||
raise HTTPException(
|
||||
status_code=409,
|
||||
detail=f"Skill '{preview_name}' already exists and is owned by another user",
|
||||
)
|
||||
# Re-run the same ownership-aware preflight used by /skills/validate.
|
||||
# This closes the time-of-check/time-of-use gap and prevents a crafted
|
||||
# request (including an admin installing into custom/) from replacing a
|
||||
# different user's skill.
|
||||
validation = storage.validate_skill_archive(tmp_path)
|
||||
preview_name = str(validation.get("skill_name") or "")
|
||||
(
|
||||
can_install_as_is,
|
||||
can_overwrite,
|
||||
_owned_by_me,
|
||||
_owned_by_other,
|
||||
_builtin_conflict,
|
||||
_owner_user_id,
|
||||
_owner_username,
|
||||
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)
|
||||
try:
|
||||
from deerflow.skills.usage import mark_uploaded
|
||||
@@ -1075,7 +1130,8 @@ async def _preview_install_target(
|
||||
skill_store: SkillStore,
|
||||
skill_name: str,
|
||||
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."""
|
||||
|
||||
user_id = _current_user_id(request)
|
||||
@@ -1085,27 +1141,44 @@ async def _preview_install_target(
|
||||
owner_username: str | None = None
|
||||
if owner_user_id:
|
||||
try:
|
||||
attached = await _attach_owner_usernames(
|
||||
[SkillResponse(name=skill_name, description="", license=None, category=SkillCategory.CUSTOM, owner_user_id=owner_user_id)]
|
||||
)
|
||||
attached = await _attach_owner_usernames([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
|
||||
except Exception:
|
||||
owner_username = None
|
||||
|
||||
builtin_conflict = bool(existing and owner_user_id is None)
|
||||
owned_by_me = bool(existing and owner_user_id == user_id)
|
||||
owned_by_other = bool(existing and owner_user_id not in (None, user_id))
|
||||
custom_conflict = conflict_kind == "custom" or bool(existing and owner_user_id is not None)
|
||||
builtin_conflict = conflict_kind == "public" or bool(existing and owner_user_id is None and conflict_kind != "custom")
|
||||
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:
|
||||
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:
|
||||
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:
|
||||
who = owner_username 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."
|
||||
who = owner_username or owner_user_id or "another user"
|
||||
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:
|
||||
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 True, False, False, builtin_conflict, owner_user_id, owner_username, f"Skill '{skill_name}' can be installed."
|
||||
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."
|
||||
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(
|
||||
@@ -1193,11 +1266,12 @@ async def preview_skill_install(
|
||||
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)
|
||||
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,
|
||||
skill_store=skill_store,
|
||||
skill_name=skill_name,
|
||||
target_category=body.target_category,
|
||||
conflict_kind=validation.get("conflict_kind"),
|
||||
)
|
||||
return SkillInstallPreviewResponse(
|
||||
ok=True,
|
||||
@@ -1248,11 +1322,12 @@ async def install_skill(
|
||||
storage = get_or_new_skill_storage(app_config=config)
|
||||
validation = storage.validate_skill_archive(skill_file_path, require_skill_suffix=False)
|
||||
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,
|
||||
skill_store=skill_store,
|
||||
skill_name=preview_name,
|
||||
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 requested_name and requested_name != validation.get("skill_name") and not owned_by_other and not builtin_conflict:
|
||||
|
||||
+52
-6
@@ -1,4 +1,5 @@
|
||||
import logging
|
||||
import hashlib
|
||||
import logging
|
||||
|
||||
from langchain.agents import create_agent
|
||||
from langchain.agents.middleware import AgentMiddleware
|
||||
@@ -37,7 +38,45 @@ logger = logging.getLogger(__name__)
|
||||
# 写作配置 Q&A 现在「先生成大纲并与用户多轮确认 → 再提交配置」,需要更多来回,故上限放宽。
|
||||
# 仍是兜底安全阀:到顶才用 tool_choice 强制提交,正常流程由用户确认大纲后自然提交。
|
||||
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:
|
||||
@@ -675,8 +714,8 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
|
||||
writing_mode=writing_mode,
|
||||
writing_artifact_path=writing_artifact_path,
|
||||
)
|
||||
else:
|
||||
system_prompt = apply_prompt_template(
|
||||
else:
|
||||
system_prompt = apply_prompt_template(
|
||||
subagent_enabled=subagent_enabled,
|
||||
max_concurrent_subagents=max_concurrent_subagents,
|
||||
agent_id=agent_id,
|
||||
@@ -691,8 +730,15 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
|
||||
notebook_source_ids=notebook_source_ids,
|
||||
is_scheduled_run=bool(cfg.get("is_scheduled_run", False)),
|
||||
)
|
||||
|
||||
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),
|
||||
tools=default_tools,
|
||||
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 deerflow.agents.lead_agent import agent as agent_module
|
||||
from deerflow.agents.lead_agent import prompt as prompt_module
|
||||
from deerflow.config.skills_config import EsQueryRoutingConfig, SkillsConfig
|
||||
|
||||
@@ -70,6 +72,44 @@ def test_es_query_routing_is_injected_into_lead_and_allowed_custom_agent_prompts
|
||||
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
|
||||
|
||||
|
||||
def test_formal_markdown_rules_are_scoped_to_ordinary_qa(monkeypatch) -> None:
|
||||
config = _app_config(enabled=False)
|
||||
monkeypatch.setattr(prompt_module, "_get_memory_context", lambda *args, **kwargs: "")
|
||||
|
||||
@@ -5,11 +5,16 @@ from __future__ import annotations
|
||||
import io
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
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.storage.local_skill_storage import LocalSkillStorage
|
||||
from deerflow.skills.types import SkillCategory
|
||||
|
||||
|
||||
def _make_skill_zip(
|
||||
@@ -239,3 +244,218 @@ async def test_install_refuses_to_overwrite_public_skill(
|
||||
await storage.ainstall_skill_from_archive(
|
||||
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
|
||||
Reference in new issue
Block a user