diff --git a/internal/http/skills_upload.go b/internal/http/skills_upload.go index fbbbb6de..ea1cbe43 100644 --- a/internal/http/skills_upload.go +++ b/internal/http/skills_upload.go @@ -54,17 +54,27 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "failed to create temp file")}) return } - defer os.Remove(tmp.Name()) - defer tmp.Close() + tmpName := tmp.Name() + defer os.Remove(tmpName) size, err := io.Copy(tmp, file) if err != nil { + tmp.Close() writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "failed to save upload")}) return } + if err := tmp.Sync(); err != nil { + tmp.Close() + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "failed to finalize upload")}) + return + } + if err := tmp.Close(); err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "failed to finalize upload")}) + return + } // Open as zip - zr, err := zip.OpenReader(tmp.Name()) + zr, err := zip.OpenReader(tmpName) if err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidRequest, "invalid ZIP file")}) return diff --git a/ui/web/src/pages/skills/lib/resolve-upload-skills.test.ts b/ui/web/src/pages/skills/lib/resolve-upload-skills.test.ts new file mode 100644 index 00000000..9692b6bb --- /dev/null +++ b/ui/web/src/pages/skills/lib/resolve-upload-skills.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it, vi } from "vitest"; + +import { resolveUploadSkills } from "./resolve-upload-skills"; +import type { MultiSkillZipValidation } from "./validate-skill-zip"; + +function makeZipFile(name = "skill.zip"): File { + return new File([new Uint8Array([0x50, 0x4b, 0x03, 0x04])], name, { + type: "application/zip", + }); +} + +describe("resolveUploadSkills", () => { + it("falls back to direct upload when browser ZIP parsing reports invalidZip", async () => { + const validateArchive = vi.fn<(_: File) => Promise>() + .mockResolvedValue({ skills: [], error: "upload.invalidZip" }); + + const result = await resolveUploadSkills(makeZipFile(), validateArchive); + + expect(result).toEqual([{ dir: "", status: "valid" }]); + }); + + it("falls back to direct upload when browser ZIP parsing throws", async () => { + const validateArchive = vi.fn<(_: File) => Promise>() + .mockRejectedValue(new Error("unsupported zip variant")); + + const result = await resolveUploadSkills(makeZipFile(), validateArchive); + + expect(result).toEqual([{ dir: "", status: "valid" }]); + }); + + it("keeps real validation errors blocking", async () => { + const validateArchive = vi.fn<(_: File) => Promise>() + .mockResolvedValue({ skills: [], error: "upload.onlyZip" }); + + const result = await resolveUploadSkills(makeZipFile("skill.txt"), validateArchive); + + expect(result).toEqual([{ dir: "", status: "invalid", error: "upload.onlyZip" }]); + }); +}); diff --git a/ui/web/src/pages/skills/lib/resolve-upload-skills.ts b/ui/web/src/pages/skills/lib/resolve-upload-skills.ts new file mode 100644 index 00000000..3a94c0ee --- /dev/null +++ b/ui/web/src/pages/skills/lib/resolve-upload-skills.ts @@ -0,0 +1,45 @@ +import { validateMultiSkillZip, type MultiSkillZipValidation } from "./validate-skill-zip"; +import type { SkillEntry, SkillStatus } from "./skill-upload-types"; + +type SkillEntrySeed = Omit; + +type ValidateSkillArchive = (file: File) => Promise; + +// Browser-side ZIP parsing is best-effort only. Some valid ZIP variants are +// accepted by the backend but rejected by JSZip, so fall back to a direct +// single-file upload instead of blocking the user locally. +export async function resolveUploadSkills( + file: File, + validateArchive: ValidateSkillArchive = validateMultiSkillZip, +): Promise { + try { + const validation = await validateArchive(file); + if (validation.error === "upload.invalidZip") { + return [fallbackUploadSkill()]; + } + if (validation.error) { + return [invalidSkill(validation.error)]; + } + if (validation.skills.length === 0) { + return [invalidSkill("upload.noSkillMd")]; + } + return validation.skills.map((skill) => ({ + dir: skill.dir, + status: skill.valid ? ("valid" as SkillStatus) : ("invalid" as SkillStatus), + name: skill.name, + slug: skill.slug, + contentHash: skill.contentHash, + error: skill.error, + })); + } catch { + return [fallbackUploadSkill()]; + } +} + +function fallbackUploadSkill(): SkillEntrySeed { + return { dir: "", status: "valid" }; +} + +function invalidSkill(error: string): SkillEntrySeed { + return { dir: "", status: "invalid", error }; +} diff --git a/ui/web/src/pages/skills/skill-upload-dialog.tsx b/ui/web/src/pages/skills/skill-upload-dialog.tsx index a3ef8d29..c48bf853 100644 --- a/ui/web/src/pages/skills/skill-upload-dialog.tsx +++ b/ui/web/src/pages/skills/skill-upload-dialog.tsx @@ -10,8 +10,8 @@ import { DialogTitle, } from "@/components/ui/dialog"; import { Button } from "@/components/ui/button"; -import { validateMultiSkillZip } from "./lib/validate-skill-zip"; import { createSkillSubZip } from "./lib/create-skill-sub-zip"; +import { resolveUploadSkills } from "./lib/resolve-upload-skills"; import { uniqueId } from "@/lib/utils"; import type { SkillUploadResponse } from "./hooks/use-skills"; import type { FileEntry, SkillStatus } from "./lib/skill-upload-types"; @@ -54,39 +54,14 @@ export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadD // Validate all files concurrently const results = await Promise.all( pending.map(async (entry) => { - try { - const validation = await validateMultiSkillZip(entry.file); - const placeholderId = entry.skills[0]?.id ?? uniqueId(); - if (validation.error) { - return { - id: entry.id, - skills: [{ id: placeholderId, dir: "", status: "invalid" as SkillStatus, error: validation.error }], - }; - } - if (validation.skills.length === 0) { - return { - id: entry.id, - skills: [{ id: placeholderId, dir: "", status: "invalid" as SkillStatus, error: "upload.noSkillMd" }], - }; - } - return { - id: entry.id, - skills: validation.skills.map((s) => ({ - id: uniqueId(), - dir: s.dir, - status: s.valid ? ("valid" as SkillStatus) : ("invalid" as SkillStatus), - name: s.name, - slug: s.slug, - contentHash: s.contentHash, - error: s.error, - })), - }; - } catch { - return { - id: entry.id, - skills: [{ id: entry.skills[0]?.id ?? uniqueId(), dir: "", status: "invalid" as SkillStatus, error: "upload.invalidZip" }], - }; - } + const resolved = await resolveUploadSkills(entry.file); + return { + id: entry.id, + skills: resolved.map((skill) => ({ + id: uniqueId(), + ...skill, + })), + }; }), );