Address Greptile company logo feedback

docs/api/companies.md

@@ -48,7 +48,7 @@ PATCH /api/companies/{companyId}
 Upload an image for a company icon and store it as that company’s logo.
 
 ```
-POST /api/companies/{companyId}/assets/images
+POST /api/companies/{companyId}/logo
 Content-Type: multipart/form-data
 ```
 

server/src/__tests__/assets.test.ts

@@ -92,7 +92,62 @@ describe("POST /api/companies/:companyId/assets/images", () => {
 
     const res = await request(app)
       .post("/api/companies/company-1/assets/images")
-      .field("namespace", "companies")
+      .field("namespace", "goals")
+      .attach("file", Buffer.from("png"), "logo.png");
+
+    expect(res.status).toBe(201);
+    expect(res.body.contentPath).toBe("/api/assets/asset-1/content");
+    expect(createAssetMock).toHaveBeenCalledTimes(1);
+    expect(png.putFile).toHaveBeenCalledWith({
+      companyId: "company-1",
+      namespace: "assets/goals",
+      originalFilename: "logo.png",
+      contentType: "image/png",
+      body: expect.any(Buffer),
+    });
+  });
+
+  it("allows supported non-image attachments outside the company logo flow", async () => {
+    const text = createStorageService("text/plain");
+    const app = createApp(text);
+
+    createAssetMock.mockResolvedValue({
+      ...createAsset(),
+      contentType: "text/plain",
+      originalFilename: "note.txt",
+    });
+
+    const res = await request(app)
+      .post("/api/companies/company-1/assets/images")
+      .field("namespace", "issues/drafts")
+      .attach("file", Buffer.from("hello"), { filename: "note.txt", contentType: "text/plain" });
+
+    expect(res.status).toBe(201);
+    expect(text.putFile).toHaveBeenCalledWith({
+      companyId: "company-1",
+      namespace: "assets/issues/drafts",
+      originalFilename: "note.txt",
+      contentType: "text/plain",
+      body: expect.any(Buffer),
+    });
+  });
+});
+
+describe("POST /api/companies/:companyId/logo", () => {
+  afterEach(() => {
+    createAssetMock.mockReset();
+    getAssetByIdMock.mockReset();
+    logActivityMock.mockReset();
+  });
+
+  it("accepts PNG logo uploads and returns an asset path", async () => {
+    const png = createStorageService("image/png");
+    const app = createApp(png);
+
+    createAssetMock.mockResolvedValue(createAsset());
+
+    const res = await request(app)
+      .post("/api/companies/company-1/logo")
       .attach("file", Buffer.from("png"), "logo.png");
 
     expect(res.status).toBe(201);
@@ -107,7 +162,7 @@ describe("POST /api/companies/:companyId/assets/images", () => {
     });
   });
 
-  it("sanitizes SVG image uploads before storing them", async () => {
+  it("sanitizes SVG logo uploads before storing them", async () => {
     const svg = createStorageService("image/svg+xml");
     const app = createApp(svg);
 
@@ -118,8 +173,7 @@ describe("POST /api/companies/:companyId/assets/images", () => {
     });
 
     const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
+      .post("/api/companies/company-1/logo")
-      .field("namespace", "companies")
       .attach(
         "file",
         Buffer.from(
@@ -141,47 +195,38 @@ describe("POST /api/companies/:companyId/assets/images", () => {
     expect(body).not.toContain("https://evil.example/");
   });
 
-  it("rejects files larger than 100 KB", async () => {
+  it("allows a logo exactly 100 KB in size", async () => {
+    const png = createStorageService("image/png");
+    const app = createApp(png);
+    createAssetMock.mockResolvedValue(createAsset());
+
+    const file = Buffer.alloc(100 * 1024, "a");
+    const res = await request(app)
+      .post("/api/companies/company-1/logo")
+      .attach("file", file, "exact-limit.png");
+
+    expect(res.status).toBe(201);
+  });
+
+  it("rejects logo files larger than 100 KB", async () => {
     const app = createApp(createStorageService());
     createAssetMock.mockResolvedValue(createAsset());
 
     const file = Buffer.alloc(100 * 1024 + 1, "a");
     const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
+      .post("/api/companies/company-1/logo")
-      .field("namespace", "companies")
       .attach("file", file, "too-large.png");
 
     expect(res.status).toBe(422);
     expect(res.body.error).toBe("Image exceeds 102400 bytes");
   });
 
-  it("allows larger non-logo images within the general asset limit", async () => {
-    const png = createStorageService("image/png");
-    const app = createApp(png);
-
-    createAssetMock.mockResolvedValue({
-      ...createAsset(),
-      contentType: "image/png",
-      originalFilename: "goal.png",
-    });
-
-    const file = Buffer.alloc(150 * 1024, "a");
-    const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
-      .field("namespace", "goals")
-      .attach("file", file, "goal.png");
-
-    expect(res.status).toBe(201);
-    expect(createAssetMock).toHaveBeenCalledTimes(1);
-  });
-
   it("rejects unsupported image types", async () => {
     const app = createApp(createStorageService("text/plain"));
     createAssetMock.mockResolvedValue(createAsset());
 
     const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
+      .post("/api/companies/company-1/logo")
-      .field("namespace", "companies")
       .attach("file", Buffer.from("not an image"), "note.txt");
 
     expect(res.status).toBe(422);
@@ -189,38 +234,12 @@ describe("POST /api/companies/:companyId/assets/images", () => {
     expect(createAssetMock).not.toHaveBeenCalled();
   });
 
-  it("allows supported non-image attachments outside the company logo namespace", async () => {
-    const text = createStorageService("text/plain");
-    const app = createApp(text);
-
-    createAssetMock.mockResolvedValue({
-      ...createAsset(),
-      contentType: "text/plain",
-      originalFilename: "note.txt",
-    });
-
-    const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
-      .field("namespace", "issues/drafts")
-      .attach("file", Buffer.from("hello"), "note.txt");
-
-    expect(res.status).toBe(201);
-    expect(text.putFile).toHaveBeenCalledWith({
-      companyId: "company-1",
-      namespace: "assets/issues/drafts",
-      originalFilename: "note.txt",
-      contentType: "text/plain",
-      body: expect.any(Buffer),
-    });
-  });
-
   it("rejects SVG image uploads that cannot be sanitized", async () => {
     const app = createApp(createStorageService("image/svg+xml"));
     createAssetMock.mockResolvedValue(createAsset());
 
     const res = await request(app)
-      .post("/api/companies/company-1/assets/images")
+      .post("/api/companies/company-1/logo")
-      .field("namespace", "companies")
       .attach("file", Buffer.from("not actually svg"), "logo.svg");
 
     expect(res.status).toBe(422);

server/src/routes/assets.ts

@@ -10,6 +10,14 @@ import { isAllowedContentType, MAX_ATTACHMENT_BYTES } from "../attachment-types.
 import { assertCompanyAccess, getActorInfo } from "./authz.js";
 const MAX_COMPANY_LOGO_BYTES = 100 * 1024;
 const SVG_CONTENT_TYPE = "image/svg+xml";
+const ALLOWED_COMPANY_LOGO_CONTENT_TYPES = new Set([
+  "image/png",
+  "image/jpeg",
+  "image/jpg",
+  "image/webp",
+  "image/gif",
+  SVG_CONTENT_TYPE,
+]);
 
 function sanitizeSvgBuffer(input: Buffer): Buffer | null {
   const raw = input.toString("utf8").trim();
@@ -78,12 +86,20 @@ function sanitizeSvgBuffer(input: Buffer): Buffer | null {
 export function assetRoutes(db: Db, storage: StorageService) {
   const router = Router();
   const svc = assetService(db);
-  const upload = multer({
+  const assetUpload = multer({
     storage: multer.memoryStorage(),
     limits: { fileSize: MAX_ATTACHMENT_BYTES, files: 1 },
   });
+  const companyLogoUpload = multer({
+    storage: multer.memoryStorage(),
+    limits: { fileSize: MAX_COMPANY_LOGO_BYTES + 1, files: 1 },
+  });
 
-  async function runSingleFileUpload(req: Request, res: Response) {
+  async function runSingleFileUpload(
+    upload: ReturnType<typeof multer>,
+    req: Request,
+    res: Response,
+  ) {
     await new Promise<void>((resolve, reject) => {
       upload.single("file")(req, res, (err: unknown) => {
         if (err) reject(err);
@@ -97,7 +113,7 @@ export function assetRoutes(db: Db, storage: StorageService) {
     assertCompanyAccess(req, companyId);
 
     try {
-      await runSingleFileUpload(req, res);
+      await runSingleFileUpload(assetUpload, req, res);
     } catch (err) {
       if (err instanceof multer.MulterError) {
         if (err.code === "LIMIT_FILE_SIZE") {
@@ -123,12 +139,7 @@ export function assetRoutes(db: Db, storage: StorageService) {
     }
 
     const namespaceSuffix = parsedMeta.data.namespace ?? "general";
-    const isCompanyLogoNamespace = namespaceSuffix === "companies" || namespaceSuffix.startsWith("companies/");
     const contentType = (file.mimetype || "").toLowerCase();
-    if (isCompanyLogoNamespace && !contentType.startsWith("image/")) {
-      res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` });
-      return;
-    }
     if (contentType !== SVG_CONTENT_TYPE && !isAllowedContentType(contentType)) {
       res.status(422).json({ error: `Unsupported file type: ${contentType || "unknown"}` });
       return;
@@ -146,7 +157,7 @@ export function assetRoutes(db: Db, storage: StorageService) {
       res.status(422).json({ error: "Image is empty" });
       return;
     }
-    if (isCompanyLogoNamespace && fileBody.length > MAX_COMPANY_LOGO_BYTES) {
+    if (fileBody.length > MAX_COMPANY_LOGO_BYTES) {
       res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` });
       return;
     }
@@ -204,6 +215,105 @@ export function assetRoutes(db: Db, storage: StorageService) {
     });
   });
 
+  router.post("/companies/:companyId/logo", async (req, res) => {
+    const companyId = req.params.companyId as string;
+    assertCompanyAccess(req, companyId);
+
+    try {
+      await runSingleFileUpload(companyLogoUpload, req, res);
+    } catch (err) {
+      if (err instanceof multer.MulterError) {
+        if (err.code === "LIMIT_FILE_SIZE") {
+          res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` });
+          return;
+        }
+        res.status(400).json({ error: err.message });
+        return;
+      }
+      throw err;
+    }
+
+    const file = (req as Request & { file?: { mimetype: string; buffer: Buffer; originalname: string } }).file;
+    if (!file) {
+      res.status(400).json({ error: "Missing file field 'file'" });
+      return;
+    }
+
+    const contentType = (file.mimetype || "").toLowerCase();
+    if (!ALLOWED_COMPANY_LOGO_CONTENT_TYPES.has(contentType)) {
+      res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` });
+      return;
+    }
+
+    let fileBody = file.buffer;
+    if (contentType === SVG_CONTENT_TYPE) {
+      const sanitized = sanitizeSvgBuffer(file.buffer);
+      if (!sanitized || sanitized.length <= 0) {
+        res.status(422).json({ error: "SVG could not be sanitized" });
+        return;
+      }
+      fileBody = sanitized;
+    }
+
+    if (fileBody.length <= 0) {
+      res.status(422).json({ error: "Image is empty" });
+      return;
+    }
+
+    const actor = getActorInfo(req);
+    const stored = await storage.putFile({
+      companyId,
+      namespace: "assets/companies",
+      originalFilename: file.originalname || null,
+      contentType,
+      body: fileBody,
+    });
+
+    const asset = await svc.create(companyId, {
+      provider: stored.provider,
+      objectKey: stored.objectKey,
+      contentType: stored.contentType,
+      byteSize: stored.byteSize,
+      sha256: stored.sha256,
+      originalFilename: stored.originalFilename,
+      createdByAgentId: actor.agentId,
+      createdByUserId: actor.actorType === "user" ? actor.actorId : null,
+    });
+
+    await logActivity(db, {
+      companyId,
+      actorType: actor.actorType,
+      actorId: actor.actorId,
+      agentId: actor.agentId,
+      runId: actor.runId,
+      action: "asset.created",
+      entityType: "asset",
+      entityId: asset.id,
+      details: {
+        originalFilename: asset.originalFilename,
+        contentType: asset.contentType,
+        byteSize: asset.byteSize,
+        namespace: "assets/companies",
+      },
+    });
+
+    res.status(201).json({
+      assetId: asset.id,
+      companyId: asset.companyId,
+      provider: asset.provider,
+      objectKey: asset.objectKey,
+      contentType: asset.contentType,
+      byteSize: asset.byteSize,
+      sha256: asset.sha256,
+      originalFilename: asset.originalFilename,
+      createdByAgentId: asset.createdByAgentId,
+      createdByUserId: asset.createdByUserId,
+      createdAt: asset.createdAt,
+      updatedAt: asset.updatedAt,
+      contentPath: `/api/assets/${asset.id}/content`,
+    });
+  });
+
   router.get("/assets/:assetId/content", async (req, res, next) => {
     const assetId = req.params.assetId as string;
     const asset = await svc.getById(assetId);

ui/src/api/assets.ts

@@ -11,11 +11,19 @@ export const assetsApi = {
     const safeFile = new File([buffer], file.name, { type: file.type });
 
     const form = new FormData();
-    form.append("file", safeFile);
     if (namespace && namespace.trim().length > 0) {
       form.append("namespace", namespace.trim());
     }
+    form.append("file", safeFile);
     return api.postForm<AssetImage>(`/companies/${companyId}/assets/images`, form);
   },
-};
 
+  uploadCompanyLogo: async (companyId: string, file: File) => {
+    const buffer = await file.arrayBuffer();
+    const safeFile = new File([buffer], file.name, { type: file.type });
+
+    const form = new FormData();
+    form.append("file", safeFile);
+    return api.postForm<AssetImage>(`/companies/${companyId}/logo`, form);
+  },
+};

ui/src/pages/CompanySettings.tsx

@@ -140,7 +140,7 @@ export function CompanySettings() {
   const logoUploadMutation = useMutation({
     mutationFn: (file: File) =>
       assetsApi
-        .uploadImage(selectedCompanyId!, file, "companies")
+        .uploadCompanyLogo(selectedCompanyId!, file)
         .then((asset) => companiesApi.update(selectedCompanyId!, { logoAssetId: asset.assetId })),
     onSuccess: (company) => {
       syncLogoState(company.logoUrl);
@@ -160,8 +160,8 @@ export function CompanySettings() {
     const file = event.target.files?.[0] ?? null;
     event.currentTarget.value = "";
     if (!file) return;
-    if (file.size >= 100 * 1024) {
+    if (file.size > 100 * 1024) {
-      setLogoUploadError("Logo image must be smaller than 100 KB.");
+      setLogoUploadError("Logo image must be 100 KB or smaller.");
       return;
     }
     setLogoUploadError(null);