diff --git a/frontend-web/src/components/workspace/agents/skill-picker-sheet.tsx b/frontend-web/src/components/workspace/agents/skill-picker-sheet.tsx index 55d4c5f9..f31fc960 100644 --- a/frontend-web/src/components/workspace/agents/skill-picker-sheet.tsx +++ b/frontend-web/src/components/workspace/agents/skill-picker-sheet.tsx @@ -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; } diff --git a/frontend-web/src/components/workspace/settings/skill-settings-page.tsx b/frontend-web/src/components/workspace/settings/skill-settings-page.tsx index 42f92a24..d63df2c6 100644 --- a/frontend-web/src/components/workspace/settings/skill-settings-page.tsx +++ b/frontend-web/src/components/workspace/settings/skill-settings-page.tsx @@ -1201,7 +1201,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); @@ -1240,19 +1257,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) => { @@ -1329,12 +1339,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 ( !o && handleClose()}> @@ -1453,9 +1480,12 @@ function InstallSkillDialog({ setTargetCategory(cat)} + value={cat} + checked={targetCategory === cat} + onChange={() => { + setTargetCategory(cat); + if (file) void validateFile(file, cat); + }} className="accent-primary" /> {cat === "custom" ? dialog.categoryCustom : dialog.categoryPublic} @@ -1513,15 +1543,18 @@ function InstallSkillDialog({ )} - {isCustomConflict && ( -
- - - {desensitize( - dialog.conflictCustom.replace("{name}", validation.skill_name), - )} - -
+ {isCustomConflict && ( +
+ + + {desensitize(customConflictMessage)} + +
)} {isPublicConflict && (
diff --git a/frontend-web/src/core/i18n/locales/en-US.ts b/frontend-web/src/core/i18n/locales/en-US.ts index e8a7ad64..24bc3ff7 100644 --- a/frontend-web/src/core/i18n/locales/en-US.ts +++ b/frontend-web/src/core/i18n/locales/en-US.ts @@ -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.", diff --git a/frontend-web/src/core/i18n/locales/types.ts b/frontend-web/src/core/i18n/locales/types.ts index 3cbdf9c0..0d33ce5d 100644 --- a/frontend-web/src/core/i18n/locales/types.ts +++ b/frontend-web/src/core/i18n/locales/types.ts @@ -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; diff --git a/frontend-web/src/core/i18n/locales/zh-CN.ts b/frontend-web/src/core/i18n/locales/zh-CN.ts index 8610ac88..d9bff801 100644 --- a/frontend-web/src/core/i18n/locales/zh-CN.ts +++ b/frontend-web/src/core/i18n/locales/zh-CN.ts @@ -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 文件。`, diff --git a/frontend-web/src/core/skills/api.ts b/frontend-web/src/core/skills/api.ts index 947a7395..0c6188ad 100644 --- a/frontend-web/src/core/skills/api.ts +++ b/frontend-web/src/core/skills/api.ts @@ -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 { - 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 { + 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"); } diff --git a/frontend-web/src/core/skills/hooks.ts b/frontend-web/src/core/skills/hooks.ts index ad68d7d5..9da8298a 100644 --- a/frontend-web/src/core/skills/hooks.ts +++ b/frontend-web/src/core/skills/hooks.ts @@ -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(); diff --git a/frontend-web/src/core/threads/hooks.ts b/frontend-web/src/core/threads/hooks.ts index 1593f32d..011bdd3b 100644 --- a/frontend-web/src/core/threads/hooks.ts +++ b/frontend-web/src/core/threads/hooks.ts @@ -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({ - client: getAPIClient(isMock), - assistantId: assistantId ?? "lead_agent", - threadId: onStreamThreadId, - reconnectOnMount: runMetadataStorageRef.current + const thread = useStream({ + 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 diff --git a/offline-backend-20260512/backend/CLAUDE.md b/offline-backend-20260512/backend/CLAUDE.md index 3371d9c1..453bb26d 100644 --- a/offline-backend-20260512/backend/CLAUDE.md +++ b/offline-backend-20260512/backend/CLAUDE.md @@ -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. diff --git a/offline-backend-20260512/backend/README.md b/offline-backend-20260512/backend/README.md index bedd9bdf..c23e9ad1 100644 --- a/offline-backend-20260512/backend/README.md +++ b/offline-backend-20260512/backend/README.md @@ -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) diff --git a/offline-backend-20260512/backend/app/gateway/routers/skills.py b/offline-backend-20260512/backend/app/gateway/routers/skills.py index 07d3f418..7b07264e 100644 --- a/offline-backend-20260512/backend/app/gateway/routers/skills.py +++ b/offline-backend-20260512/backend/app/gateway/routers/skills.py @@ -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: diff --git a/offline-backend-20260512/backend/packages/harness/deerflow/agents/lead_agent/agent.py b/offline-backend-20260512/backend/packages/harness/deerflow/agents/lead_agent/agent.py index 733110f1..f27ae6d2 100644 --- a/offline-backend-20260512/backend/packages/harness/deerflow/agents/lead_agent/agent.py +++ b/offline-backend-20260512/backend/packages/harness/deerflow/agents/lead_agent/agent.py @@ -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 = '' + + +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" + 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), diff --git a/offline-backend-20260512/backend/tests/test_es_query_routing_prompt.py b/offline-backend-20260512/backend/tests/test_es_query_routing_prompt.py index ec6080b4..e11728ae 100644 --- a/offline-backend-20260512/backend/tests/test_es_query_routing_prompt.py +++ b/offline-backend-20260512/backend/tests/test_es_query_routing_prompt.py @@ -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 '' 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 = '\n自定义 ES 路由规则\n' + + 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: "") diff --git a/offline-backend-20260512/backend/tests/test_skills_install_upload.py b/offline-backend-20260512/backend/tests/test_skills_install_upload.py index 969eea15..975cef69 100644 --- a/offline-backend-20260512/backend/tests/test_skills_install_upload.py +++ b/offline-backend-20260512/backend/tests/test_skills_install_upload.py @@ -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