feat(uploads): bump cap to 100MB + correct-reason error messages (#1588)
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (push) Waiting to run
publish-workspace-server-image / build-and-push (push) Failing after 15s
publish-workspace-server-image / Production auto-deploy (push) Has been skipped
Block internal-flavored paths / Block forbidden paths (push) Successful in 3s
publish-canvas-image / Build & push canvas image (push) Successful in 1m53s
CI / Detect changes (push) Successful in 8s
CI / Shellcheck (E2E scripts) (push) Successful in 22s
E2E API Smoke Test / detect-changes (push) Successful in 10s
E2E Chat / detect-changes (push) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (push) Successful in 14s
Handlers Postgres Integration / detect-changes (push) Successful in 16s
Harness Replays / detect-changes (push) Successful in 13s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (push) Successful in 10s
Lint no tenant GITEA/GITHUB token write / Scan for repo-host token write into tenant workspace surface (push) Successful in 9s
publish-runtime-autobump / bump-and-tag (push) Successful in 38s
publish-runtime-autobump / pr-validate (push) Successful in 44s
Secret scan / Scan diff for credential-shaped strings (push) Successful in 4s
Runtime PR-Built Compatibility / detect-changes (push) Successful in 8s
CI / Platform (Go) (push) Successful in 5m52s
CI / Canvas (Next.js) (push) Successful in 6m59s
CI / Python Lint & Test (push) Successful in 7m16s
Harness Replays / Harness Replays (push) Successful in 19s
CI / all-required (push) Successful in 6m33s
E2E API Smoke Test / E2E API Smoke Test (push) Failing after 2m35s
Handlers Postgres Integration / Handlers Postgres Integration (push) Successful in 2m30s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Successful in 2m29s
E2E Chat / E2E Chat (push) Failing after 6m45s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (push) Successful in 7m31s
CI / Canvas Deploy Reminder (push) Successful in 1s
MCP Stdio Transport Regression / MCP stdio with regular-file stdout (push) Successful in 53s
Sweep stale Cloudflare DNS records / Sweep CF orphans (push) Successful in 11s
ci-required-drift / drift (push) Successful in 1m14s
Sweep stale Cloudflare Tunnels / Sweep CF tunnels (push) Successful in 6s
Sweep stale e2e-* orgs (staging) / Sweep e2e orgs (push) Successful in 4s
SECRET_PATTERNS drift lint / Detect SECRET_PATTERNS drift (push) Successful in 34s
Staging SaaS smoke (every 30 min) / Staging SaaS smoke (push) Successful in 4m15s
main-red-watchdog / watchdog (push) Successful in 32s
gate-check-v3 / gate-check (push) Successful in 20s
gitea-merge-queue / queue (push) Successful in 9s
Continuous synthetic E2E (staging) / Synthetic E2E against staging (push) Successful in 8m10s
status-reaper / reap (push) Compensated by status-reaper (workflow has no push: trigger; Gitea 1.22.6 hardcoded-suffix bug — see .gitea/scripts/status-reaper.py)
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (push) Waiting to run
publish-workspace-server-image / build-and-push (push) Failing after 15s
publish-workspace-server-image / Production auto-deploy (push) Has been skipped
Block internal-flavored paths / Block forbidden paths (push) Successful in 3s
publish-canvas-image / Build & push canvas image (push) Successful in 1m53s
CI / Detect changes (push) Successful in 8s
CI / Shellcheck (E2E scripts) (push) Successful in 22s
E2E API Smoke Test / detect-changes (push) Successful in 10s
E2E Chat / detect-changes (push) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (push) Successful in 14s
Handlers Postgres Integration / detect-changes (push) Successful in 16s
Harness Replays / detect-changes (push) Successful in 13s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (push) Successful in 10s
Lint no tenant GITEA/GITHUB token write / Scan for repo-host token write into tenant workspace surface (push) Successful in 9s
publish-runtime-autobump / bump-and-tag (push) Successful in 38s
publish-runtime-autobump / pr-validate (push) Successful in 44s
Secret scan / Scan diff for credential-shaped strings (push) Successful in 4s
Runtime PR-Built Compatibility / detect-changes (push) Successful in 8s
CI / Platform (Go) (push) Successful in 5m52s
CI / Canvas (Next.js) (push) Successful in 6m59s
CI / Python Lint & Test (push) Successful in 7m16s
Harness Replays / Harness Replays (push) Successful in 19s
CI / all-required (push) Successful in 6m33s
E2E API Smoke Test / E2E API Smoke Test (push) Failing after 2m35s
Handlers Postgres Integration / Handlers Postgres Integration (push) Successful in 2m30s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Successful in 2m29s
E2E Chat / E2E Chat (push) Failing after 6m45s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (push) Successful in 7m31s
CI / Canvas Deploy Reminder (push) Successful in 1s
MCP Stdio Transport Regression / MCP stdio with regular-file stdout (push) Successful in 53s
Sweep stale Cloudflare DNS records / Sweep CF orphans (push) Successful in 11s
ci-required-drift / drift (push) Successful in 1m14s
Sweep stale Cloudflare Tunnels / Sweep CF tunnels (push) Successful in 6s
Sweep stale e2e-* orgs (staging) / Sweep e2e orgs (push) Successful in 4s
SECRET_PATTERNS drift lint / Detect SECRET_PATTERNS drift (push) Successful in 34s
Staging SaaS smoke (every 30 min) / Staging SaaS smoke (push) Successful in 4m15s
main-red-watchdog / watchdog (push) Successful in 32s
gate-check-v3 / gate-check (push) Successful in 20s
gitea-merge-queue / queue (push) Successful in 9s
Continuous synthetic E2E (staging) / Synthetic E2E against staging (push) Successful in 8m10s
status-reaper / reap (push) Compensated by status-reaper (workflow has no push: trigger; Gitea 1.22.6 hardcoded-suffix bug — see .gitea/scripts/status-reaper.py)
Bump chat upload cap 50MB → 100MB across canvas, workspace-server (Go), workspace (Python), and the nginx test harness. Pre-flight gates oversized files BEFORE network I/O so the user gets an immediate 'File too large (got X MB) — limit is 100MB' instead of a downstream timeout. Scaled abort-timeout (60s floor, ~100KB/s rate) replaces the fixed 60s that mis-attributed slow-uplink streams as 'timed out'. Resolves forensic a99ab0a1. Approvers: core-devops (id=52), core-qa (id=64), core-security (id=68). Follow-up: SSOT for upload cap (4 mirror sites) — see internal/<issue-tba>.
This commit was merged in pull request #1588.
This commit is contained in:
@@ -0,0 +1,179 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import {
|
||||
uploadChatFiles,
|
||||
FileTooLargeError,
|
||||
MAX_UPLOAD_BYTES,
|
||||
computeUploadTimeoutMs,
|
||||
} from "../uploads";
|
||||
|
||||
// Tests for the 100 MB upload-cap raise + correct-reason error mapping
|
||||
// (CTO 2026-05-19 directive on forensic a99ab0a1: "if its file size
|
||||
// issue, should have error that instead saying timeout which is
|
||||
// wrong"). Each case has its own specific reason; conflation is the
|
||||
// bug this PR fixes.
|
||||
|
||||
// File constructor in node's vitest env supports size via array length.
|
||||
// Allocate a typed-array of N bytes and wrap it — File reads .size off
|
||||
// the underlying Blob. Allocating 101 MB once per test is fine (vitest
|
||||
// maxWorkers=1, single test process).
|
||||
function makeFile(name: string, size: number): File {
|
||||
const buf = new Uint8Array(size);
|
||||
return new File([buf], name);
|
||||
}
|
||||
|
||||
const wsId = "00000000-0000-0000-0000-000000000001";
|
||||
|
||||
describe("uploadChatFiles — MAX_UPLOAD_BYTES + pre-flight gate", () => {
|
||||
it("MAX_UPLOAD_BYTES is exactly 100 MB (mirrors server constant)", () => {
|
||||
// Pinned so a regression that flipped the constant back to 50 MB
|
||||
// would fail loudly here — without this the canvas would
|
||||
// silently start rejecting files the server now accepts.
|
||||
expect(MAX_UPLOAD_BYTES).toBe(100 * 1024 * 1024);
|
||||
});
|
||||
|
||||
it("throws FileTooLargeError for a 101 MB file BEFORE any network I/O", async () => {
|
||||
const oversize = makeFile("big.bin", 101 * 1024 * 1024);
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
try {
|
||||
await uploadChatFiles(wsId, [oversize]);
|
||||
throw new Error("expected uploadChatFiles to throw, but it resolved");
|
||||
} catch (e) {
|
||||
// The exact class name matters — useChatSend's mapUploadErrorToReason
|
||||
// routes off `instanceof FileTooLargeError`. A regression that
|
||||
// demoted to a plain Error would silently re-introduce the
|
||||
// wrong-reason conflation CTO flagged.
|
||||
expect(e).toBeInstanceOf(FileTooLargeError);
|
||||
const err = e as FileTooLargeError;
|
||||
// Message must contain the 100MB cap (so the user knows what the
|
||||
// limit is) and a number-with-MB form of the actual size.
|
||||
expect(err.message).toContain("100MB");
|
||||
// Some toFixed(1) renderings: 101.0MB. Loose match: contains "MB".
|
||||
expect(err.message).toMatch(/got\s+\d+(\.\d+)?MB/);
|
||||
expect(err.fileSize).toBe(101 * 1024 * 1024);
|
||||
}
|
||||
// CRITICAL: no fetch may have been initiated. Pre-flight is the
|
||||
// whole point — if a network round-trip happened we'd be back to
|
||||
// surfacing a downstream timeout / 413 instead of the actionable
|
||||
// file-size message.
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("accepts a file exactly at the cap (== MAX_UPLOAD_BYTES)", async () => {
|
||||
// Equality must NOT trip the gate — the cap is inclusive on the
|
||||
// server side and the canvas must match. Without this, an exact-
|
||||
// cap file would 503 client-side while the server accepts it.
|
||||
const exact = makeFile("max.bin", MAX_UPLOAD_BYTES);
|
||||
const fetchSpy = vi
|
||||
.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValue(
|
||||
new Response(JSON.stringify({ files: [] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
await expect(uploadChatFiles(wsId, [exact])).resolves.toBeDefined();
|
||||
expect(fetchSpy).toHaveBeenCalledOnce();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeUploadTimeoutMs — scaled timeout curve", () => {
|
||||
it("100 KB file → 60s floor (small-file ergonomics)", () => {
|
||||
// Below the floor, the small-file UX (typo'd hostname surfacing as
|
||||
// connect-error quickly) takes priority over the slow-uplink
|
||||
// assumption.
|
||||
expect(computeUploadTimeoutMs(100 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("1 MB file → 60s floor", () => {
|
||||
expect(computeUploadTimeoutMs(1 * 1024 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("100 MB file → ~1000s (matches the slow-uplink design budget)", () => {
|
||||
// Pin the upper-bound case the design targets: at 100 MB / 100 KB/s
|
||||
// a legitimate slow uplink completes in ~1000s, comfortably
|
||||
// before the platform's 1200s http.Client timeout. Without this
|
||||
// scaling, the previous fixed 60s deadline aborted Ryan's ~60 MB
|
||||
// upload in forensic a99ab0a1.
|
||||
const ms = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
// 100*1024*1024 / 100 = 1048576 ms ≈ 1048.6s — pin to ±1ms.
|
||||
expect(ms).toBe(Math.floor((100 * 1024 * 1024) / 100));
|
||||
expect(ms).toBeGreaterThan(1_000_000);
|
||||
expect(ms).toBeLessThan(1_100_000);
|
||||
});
|
||||
|
||||
it("strictly monotonic above the floor", () => {
|
||||
// A regression that capped or non-monotonised the curve would
|
||||
// silently re-introduce premature aborts for mid-size files.
|
||||
const a = computeUploadTimeoutMs(10 * 1024 * 1024);
|
||||
const b = computeUploadTimeoutMs(50 * 1024 * 1024);
|
||||
const c = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
expect(b).toBeGreaterThan(a);
|
||||
expect(c).toBeGreaterThan(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe("uploadChatFiles — error path shapes (for downstream reason-mapping)", () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn> | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
});
|
||||
afterEach(() => {
|
||||
fetchSpy?.mockRestore();
|
||||
fetchSpy = null;
|
||||
});
|
||||
|
||||
it("propagates the server's 413 reason verbatim (not as 'timeout')", async () => {
|
||||
// The error message text is what useChatSend surfaces via
|
||||
// `Upload failed: ${e.message}` — pin that the server's reason
|
||||
// is present, not swallowed.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"error":"file exceeds per-file limit (100 MB)"}', {
|
||||
status: 413,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).rejects.toThrow(
|
||||
/upload failed:.*413.*per-file limit/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("propagates AbortSignal timeout as a DOMException with name=TimeoutError", async () => {
|
||||
// Reason-routing in useChatSend.mapUploadErrorToReason discriminates
|
||||
// by e.name === 'TimeoutError'. Pin the shape so a future browser /
|
||||
// polyfill change that renamed it would fail loudly here, NOT
|
||||
// silently fall through to the generic "Upload failed" path
|
||||
// (which is what made forensic a99ab0a1 hard to root-cause).
|
||||
const abortErr = new DOMException("signal timed out", "TimeoutError");
|
||||
fetchSpy!.mockRejectedValue(abortErr);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
try {
|
||||
await uploadChatFiles(wsId, [f]);
|
||||
throw new Error("expected throw");
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(DOMException);
|
||||
expect((e as DOMException).name).toBe("TimeoutError");
|
||||
// CRITICAL negative: the rejection must NOT be a
|
||||
// FileTooLargeError, because pre-flight already excluded that.
|
||||
expect(e).not.toBeInstanceOf(FileTooLargeError);
|
||||
}
|
||||
});
|
||||
|
||||
it("a 50 MB file does NOT trip the pre-flight gate (sub-cap)", async () => {
|
||||
// The forensic case: Ryan's file was over the OLD 50MB cap but
|
||||
// under the NEW 100MB cap. Pin that the pre-flight does NOT
|
||||
// misfire on a sub-100MB file.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"files":[]}', {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("ryan.bin", 50 * 1024 * 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).resolves.toBeDefined();
|
||||
expect(fetchSpy!).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { mapUploadErrorToReason } from "../useChatSend";
|
||||
import { FileTooLargeError } from "../../uploads";
|
||||
|
||||
// Pin the case-by-case error mapping (CTO 2026-05-19 directive on
|
||||
// forensic a99ab0a1: each cause maps to ITS OWN message, no
|
||||
// conflation). The four cases — FileTooLargeError, TimeoutError,
|
||||
// other Error, non-Error — are the entire user-facing contract this
|
||||
// PR ships; each gets a dedicated assertion so a regression that
|
||||
// re-conflated them would surface here.
|
||||
|
||||
describe("mapUploadErrorToReason", () => {
|
||||
it("FileTooLargeError → surfaces the pre-flight message verbatim", () => {
|
||||
const err = new FileTooLargeError(
|
||||
101 * 1024 * 1024,
|
||||
"File too large (got 101.0MB) — limit is 100MB. Please use a smaller file.",
|
||||
);
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Verbatim, no "Upload failed:" prefix — the FileTooLargeError
|
||||
// message is already a complete, user-facing sentence.
|
||||
expect(out).toBe(err.message);
|
||||
expect(out).not.toMatch(/^Upload failed:/);
|
||||
// Must mention the cap so the user knows what to aim for.
|
||||
expect(out).toContain("100MB");
|
||||
// Must NOT mention timeout — wrong-reason conflation guard.
|
||||
expect(out.toLowerCase()).not.toContain("timeout");
|
||||
expect(out.toLowerCase()).not.toContain("connection");
|
||||
});
|
||||
|
||||
it("TimeoutError → connection-too-slow message, NOT file-size", () => {
|
||||
const err = new DOMException("signal timed out", "TimeoutError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// The user-facing reason matches the design contract: tells the
|
||||
// user the connection is slow, gives them the actionable retry
|
||||
// hint, and does NOT mention file-size (pre-flight already
|
||||
// excluded that — this is the case CTO flagged).
|
||||
expect(out).toContain("Upload timed out");
|
||||
expect(out).toContain("connection is too slow");
|
||||
// CRITICAL negatives — guard against the wrong-reason conflation.
|
||||
expect(out).not.toMatch(/100MB|file too large|File too large/);
|
||||
});
|
||||
|
||||
it("plain Error from server (e.g. 413) → wraps with 'Upload failed:' + server reason", () => {
|
||||
// What uploadChatFiles throws when res.ok is false. The message
|
||||
// already encodes the status + body; the mapper just prefixes
|
||||
// "Upload failed:" so the chat error banner makes sense.
|
||||
const err = new Error("upload failed: 413 file exceeds per-file limit");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
expect(out).toBe("Upload failed: upload failed: 413 file exceeds per-file limit");
|
||||
// Server's actual reason must survive — that's the whole
|
||||
// feedback_surface_actionable_failure_reason_to_user point.
|
||||
expect(out).toContain("413");
|
||||
expect(out).toContain("per-file limit");
|
||||
});
|
||||
|
||||
it("non-Error throw → generic fallback", () => {
|
||||
// A string-throw (or a frozen object) is unusual but possible in
|
||||
// some catch paths. The fallback must NOT crash and must still
|
||||
// give the user a non-empty reason.
|
||||
expect(mapUploadErrorToReason("some random string")).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(undefined)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(null)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(42)).toBe("Upload failed");
|
||||
});
|
||||
|
||||
it("an AbortError that ISN'T a TimeoutError falls through to generic Error path", () => {
|
||||
// Belt-and-braces: a regression that loosened the name check to
|
||||
// ANY DOMException would silently rewrite legitimate AbortError
|
||||
// (user-initiated cancel) into "connection too slow". Pin the
|
||||
// narrow check.
|
||||
const err = new DOMException("user aborted", "AbortError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Falls through to non-Error branch (DOMException is not an Error
|
||||
// subclass in node's vitest environment); accept either generic
|
||||
// fallback or the Error-message form depending on the runtime.
|
||||
expect(out).not.toContain("connection is too slow");
|
||||
expect(out).not.toContain("File too large");
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { useCallback, useRef, useState } from "react";
|
||||
import { api } from "@/lib/api";
|
||||
import { uploadChatFiles } from "../uploads";
|
||||
import { uploadChatFiles, FileTooLargeError } from "../uploads";
|
||||
import { createMessage, type ChatMessage, type ChatAttachment } from "../types";
|
||||
import { extractFilesFromTask } from "../message-parser";
|
||||
|
||||
@@ -46,6 +46,52 @@ export function extractReplyText(resp: A2AResponse): string {
|
||||
return collected.join("\n");
|
||||
}
|
||||
|
||||
/** Map a thrown error from `uploadChatFiles` to the user-facing reason
|
||||
* shown in the chat error banner.
|
||||
*
|
||||
* Cases (per `feedback_surface_actionable_failure_reason_to_user` —
|
||||
* user-facing failures MUST tell the user WHY):
|
||||
*
|
||||
* 1. FileTooLargeError → use the error's message verbatim. The
|
||||
* pre-flight already built the actionable string with the actual
|
||||
* size + the cap; don't re-wrap it (which would prepend a
|
||||
* redundant "Upload failed:" prefix).
|
||||
*
|
||||
* 2. DOMException name="TimeoutError" → AbortSignal.timeout fired
|
||||
* during the fetch. Pre-flight already excluded file-size, so
|
||||
* this CANNOT mean "file too large". Surface a connection-speed
|
||||
* message — the user's actionable next step is retry or check
|
||||
* network, NOT shrink the file.
|
||||
*
|
||||
* 3. Other Error → use the wrapped form so the server's reason
|
||||
* (e.g. "upload failed: 413 ...") reaches the user instead of
|
||||
* being swallowed.
|
||||
*
|
||||
* 4. Non-Error throw → generic fallback.
|
||||
*
|
||||
* Exported for unit testing — the case-by-case mapping is the
|
||||
* load-bearing contract this PR ships. */
|
||||
export function mapUploadErrorToReason(e: unknown): string {
|
||||
if (e instanceof FileTooLargeError) {
|
||||
// Already a complete, user-facing sentence — surface verbatim.
|
||||
return e.message;
|
||||
}
|
||||
// DOMException with name="TimeoutError" is what AbortSignal.timeout
|
||||
// produces on abort. Browsers represent it as a DOMException, not a
|
||||
// regular Error subclass — feature-detect via .name to avoid coupling
|
||||
// to a global that's missing in test envs.
|
||||
if (
|
||||
e !== null && typeof e === "object" &&
|
||||
"name" in e && (e as { name: unknown }).name === "TimeoutError"
|
||||
) {
|
||||
return "Upload timed out — your connection is too slow for this file. Try again, or reduce file size.";
|
||||
}
|
||||
if (e instanceof Error) {
|
||||
return `Upload failed: ${e.message}`;
|
||||
}
|
||||
return "Upload failed";
|
||||
}
|
||||
|
||||
export interface UseChatSendOptions {
|
||||
getHistoryMessages: () => ChatMessage[];
|
||||
onUserMessage?: (msg: ChatMessage) => void;
|
||||
@@ -85,9 +131,12 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
|
||||
} catch (e) {
|
||||
setUploading(false);
|
||||
sendInFlightRef.current = false;
|
||||
setError(
|
||||
e instanceof Error ? `Upload failed: ${e.message}` : "Upload failed",
|
||||
);
|
||||
// Error-reason routing (CTO 2026-05-19 on forensic a99ab0a1:
|
||||
// "if its file size issue, should have error that instead
|
||||
// saying timeout which is wrong"). Each cause maps to ITS
|
||||
// OWN message — NO conflation between file-size and
|
||||
// connection-too-slow.
|
||||
setError(mapUploadErrorToReason(e));
|
||||
return;
|
||||
}
|
||||
setUploading(false);
|
||||
|
||||
@@ -1,6 +1,55 @@
|
||||
import { PLATFORM_URL, platformAuthHeaders } from "@/lib/api";
|
||||
import type { ChatAttachment } from "./types";
|
||||
|
||||
/** Hard cap on a single chat upload. Pre-flight gate: this constant is
|
||||
* checked BEFORE any network I/O so a file-size violation surfaces
|
||||
* immediately with an actionable reason ("File too large (got X MB)
|
||||
* — limit is 100MB") rather than as a downstream timeout or 413.
|
||||
*
|
||||
* SERVER_MIRROR: keep aligned with
|
||||
* - workspace-server/internal/handlers/chat_files.go chatUploadMaxBytes
|
||||
* - workspace/internal_chat_uploads.py CHAT_UPLOAD_MAX_BYTES /
|
||||
* CHAT_UPLOAD_MAX_FILE_BYTES
|
||||
*
|
||||
* Three mirror sites exist because each layer must enforce / pre-flight
|
||||
* on its own (no shared codegen yet). Tracked for SSOT follow-up:
|
||||
* expose via GET /uploads/limits so the client can fetch the live cap
|
||||
* instead of duplicating the constant. */
|
||||
export const MAX_UPLOAD_BYTES = 100 * 1024 * 1024;
|
||||
|
||||
/** Thrown by `uploadChatFiles` when a candidate file exceeds
|
||||
* MAX_UPLOAD_BYTES. Caught by `useChatSend` and surfaced verbatim —
|
||||
* the message is already user-actionable. Distinct name lets the
|
||||
* catch path route it correctly without parsing the message string.
|
||||
*
|
||||
* Why a distinct class instead of a sentinel string match: the catch
|
||||
* in `useChatSend` already needs to discriminate this case from a
|
||||
* `TimeoutError` (which has a structurally similar surface but a
|
||||
* DIFFERENT root cause). Conflating them was the bug CTO flagged on
|
||||
* forensic a99ab0a1: "if its file size issue, should have error that
|
||||
* instead saying timeout which is wrong". */
|
||||
export class FileTooLargeError extends Error {
|
||||
readonly name = "FileTooLargeError";
|
||||
readonly fileSize: number;
|
||||
constructor(fileSize: number, message: string) {
|
||||
super(message);
|
||||
this.fileSize = fileSize;
|
||||
}
|
||||
}
|
||||
|
||||
/** Compute the abort timeout for an upload of `totalBytes`. Floor at
|
||||
* 60s (small-file ergonomics: a 100 KB image shouldn't wait 1000s to
|
||||
* see a typo'd hostname surface as a connect error). Above the floor,
|
||||
* scale linearly at ~100 KB/s assumed minimum uplink — at the 100 MB
|
||||
* cap this yields ~1000s, comfortable for the slow-mobile-tether case
|
||||
* that motivated forensic a99ab0a1 (Ryan's >50 MB upload aborted at
|
||||
* the fixed 60s timeout while still streaming).
|
||||
*
|
||||
* Exported for the unit test that pins the curve at the boundary. */
|
||||
export function computeUploadTimeoutMs(totalBytes: number): number {
|
||||
return Math.max(60_000, totalBytes / 100); // 100KB/s → ms = bytes/100
|
||||
}
|
||||
|
||||
/** Chat attachments are intentionally uploaded via a direct fetch()
|
||||
* instead of the `api.post` helper — `api.post` JSON-stringifies the
|
||||
* body, which would 500 on a Blob. Auth headers (tenant slug, admin
|
||||
@@ -10,25 +59,57 @@ import type { ChatAttachment } from "./types";
|
||||
* Content-Type so the browser writes the multipart boundary into the
|
||||
* header; setting it manually would yield a multipart body the server
|
||||
* can't parse. See lib/api.ts platformAuthHeaders() for the full
|
||||
* rationale on why this pair must stay matched. */
|
||||
* rationale on why this pair must stay matched.
|
||||
*
|
||||
* Failure-reason contract (CTO 2026-05-19 directive on forensic
|
||||
* a99ab0a1: each cause maps to ITS OWN message, no conflation):
|
||||
* 1. file.size > MAX_UPLOAD_BYTES → throws FileTooLargeError
|
||||
* BEFORE any network I/O, with the offending size + the cap.
|
||||
* 2. fetch aborts via AbortSignal → DOMException name="TimeoutError";
|
||||
* caller surfaces "connection too slow" (file-size already
|
||||
* excluded by gate 1, so the TimeoutError CANNOT mean file-size).
|
||||
* 3. server returns !res.ok → throws Error with the server's
|
||||
* reason embedded (status + body); caller surfaces verbatim.
|
||||
* 4. any other thrown error → falls through as-is. */
|
||||
export async function uploadChatFiles(
|
||||
workspaceId: string,
|
||||
files: File[],
|
||||
): Promise<ChatAttachment[]> {
|
||||
if (files.length === 0) return [];
|
||||
|
||||
// PRE-FLIGHT: bail before any network I/O if any file exceeds the cap.
|
||||
// After this gate, an AbortSignal.timeout firing during the fetch
|
||||
// CANNOT be attributed to file size — it's necessarily a slow
|
||||
// connection. That distinction is what makes the downstream error
|
||||
// mapping unambiguous.
|
||||
let totalBytes = 0;
|
||||
for (const f of files) {
|
||||
if (f.size > MAX_UPLOAD_BYTES) {
|
||||
const sizeMb = (f.size / (1024 * 1024)).toFixed(1);
|
||||
throw new FileTooLargeError(
|
||||
f.size,
|
||||
`File too large (got ${sizeMb}MB) — limit is 100MB. Please use a smaller file.`,
|
||||
);
|
||||
}
|
||||
totalBytes += f.size;
|
||||
}
|
||||
|
||||
const form = new FormData();
|
||||
for (const f of files) form.append("files", f, f.name);
|
||||
|
||||
// Uploads legitimately take a while on cold cache (tar write +
|
||||
// docker cp into the container). 60s is comfortable for the 25MB/
|
||||
// 50MB caps the server enforces.
|
||||
// Scale the abort timeout with payload size so a legitimate slow-
|
||||
// uplink upload of a large file isn't aborted before the body has
|
||||
// finished streaming. The fixed 60s previous-version was the root
|
||||
// cause of forensic a99ab0a1: Ryan's ~60 MB upload over a constrained
|
||||
// uplink streamed past 60s, AbortSignal fired client-side, server
|
||||
// got a truncated body, the user saw "signal timed out" — when the
|
||||
// real cause was simply "uplink slower than our hard-coded deadline".
|
||||
const res = await fetch(`${PLATFORM_URL}/workspaces/${workspaceId}/chat/uploads`, {
|
||||
method: "POST",
|
||||
headers: platformAuthHeaders(),
|
||||
body: form,
|
||||
credentials: "include",
|
||||
signal: AbortSignal.timeout(60_000),
|
||||
signal: AbortSignal.timeout(computeUploadTimeoutMs(totalBytes)),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => "");
|
||||
|
||||
@@ -53,11 +53,15 @@ http {
|
||||
harness-tenant-beta.localhost
|
||||
localhost;
|
||||
|
||||
# Cap upload at 50MB to mirror the staging tenant nginx limit;
|
||||
# Cap upload at 100MB to mirror the staging tenant nginx limit;
|
||||
# chat upload tests will fail closed if the platform handler
|
||||
# ever silently expands its limit (catches the failure mode
|
||||
# opposite of the chat-files lazy-heal incident).
|
||||
client_max_body_size 50m;
|
||||
# opposite of the chat-files lazy-heal incident). Bumped from
|
||||
# 50m to 100m in lockstep with chat_files.go chatUploadMaxBytes
|
||||
# (CTO 2026-05-19 directive on forensic a99ab0a1). If the
|
||||
# production CF / nginx tier still caps at 50m, this mirror
|
||||
# will pass while prod 413s — surface to ops if seen.
|
||||
client_max_body_size 100m;
|
||||
|
||||
location / {
|
||||
# The map above resolves $tenant_upstream to the right
|
||||
|
||||
@@ -67,7 +67,7 @@ type ChatFilesHandler struct {
|
||||
|
||||
// httpClient is broken out so tests can swap in an httptest.Server
|
||||
// transport. Prod uses a default with a generous Timeout to cover
|
||||
// the 50 MB worst case on a slow EC2 link without leaving a
|
||||
// the 100 MB worst case on a slow EC2 link without leaving a
|
||||
// connection hanging forever on a sick workspace.
|
||||
httpClient *http.Client
|
||||
|
||||
@@ -89,9 +89,14 @@ func NewChatFilesHandler(t *TemplatesHandler) *ChatFilesHandler {
|
||||
return &ChatFilesHandler{
|
||||
templates: t,
|
||||
httpClient: &http.Client{
|
||||
// 50 MB total body cap / ~1 MB/s slow-network floor → ~60s.
|
||||
// Doubled for headroom on the legitimate-but-slow case.
|
||||
Timeout: 120 * time.Second,
|
||||
// 100 MB total body cap / ~100 KB/s slow-uplink floor → ~1000s.
|
||||
// Doubled for headroom on the legitimate-but-slow case (e.g.
|
||||
// reno-stars 2026-05-19 forensic a99ab0a1: 60MB upload over a
|
||||
// constrained uplink). Client-side AbortSignal.timeout (canvas
|
||||
// uploads.ts) computes the matching deadline per-request and
|
||||
// surfaces "connection too slow" — distinct from the file-size
|
||||
// pre-flight that returns immediately before any network I/O.
|
||||
Timeout: 1200 * time.Second,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -107,10 +112,19 @@ func (h *ChatFilesHandler) WithPendingUploads(storage pendinguploads.Storage, br
|
||||
}
|
||||
|
||||
// chatUploadMaxBytes caps the full multipart request body so a
|
||||
// malicious / runaway client can't OOM the proxy hop. 50 MB matches
|
||||
// malicious / runaway client can't OOM the proxy hop. 100 MB matches
|
||||
// the workspace-side limit; anything larger is rejected at the
|
||||
// network boundary before forwarding.
|
||||
const chatUploadMaxBytes = 50 * 1024 * 1024
|
||||
//
|
||||
// CANVAS_MIRROR: keep aligned with canvas/src/components/tabs/chat/
|
||||
// uploads.ts MAX_UPLOAD_BYTES. The canvas constant exists so the
|
||||
// pre-flight size check can fail immediately (before network I/O)
|
||||
// with the actionable "File too large (got X MB) — limit is 100MB"
|
||||
// message. Bumping one side without the other yields the wrong-reason
|
||||
// surface that motivated this constant pair (CTO 2026-05-19 directive
|
||||
// on forensic a99ab0a1: file-size cause MUST surface as file-size,
|
||||
// NOT as a downstream timeout).
|
||||
const chatUploadMaxBytes = 100 * 1024 * 1024
|
||||
|
||||
// resolveWorkspaceForwardCreds resolves the workspace's URL +
|
||||
// platform_inbound_secret for an /internal/* forward, applying
|
||||
@@ -268,7 +282,7 @@ func contentDispositionAttachment(name string) string {
|
||||
// back unchanged.
|
||||
//
|
||||
// Why streaming, not parse-then-re-encode:
|
||||
// - Eliminates the 50 MB intermediate buffer on the platform.
|
||||
// - Eliminates the 100 MB intermediate buffer on the platform.
|
||||
// - Per-file size + path-safety enforcement is the workspace's job;
|
||||
// duplicating it here just creates two places to keep in sync.
|
||||
// - The error responses from the workspace (413 with the offending
|
||||
@@ -354,7 +368,7 @@ func (h *ChatFilesHandler) Upload(c *gin.Context) {
|
||||
// either.
|
||||
//
|
||||
// Body is streamed end-to-end (no buffering on the platform), preserving
|
||||
// binary safety and arbitrary file size (the 50 MB cap on Upload doesn't
|
||||
// binary safety and arbitrary file size (the 100 MB cap on Upload doesn't
|
||||
// apply to artefacts the agent produced).
|
||||
func (h *ChatFilesHandler) Download(c *gin.Context) {
|
||||
workspaceID := c.Param("id")
|
||||
@@ -546,8 +560,8 @@ type uploadedFile struct {
|
||||
// a fetcher crash mid-batch.
|
||||
//
|
||||
// Limits enforced here mirror the workspace-side ingest_handler:
|
||||
// - Total body cap: 50 MB (set on c.Request.Body before reaching us)
|
||||
// - Per-file cap: 25 MB (pendinguploads.MaxFileBytes; rejected as 413)
|
||||
// - Total body cap: 100 MB (set on c.Request.Body before reaching us)
|
||||
// - Per-file cap: 100 MB (pendinguploads.MaxFileBytes; rejected as 413)
|
||||
// - Filename: sanitized + capped at 100 chars (SanitizeFilename)
|
||||
//
|
||||
// Logging: every persisted file logs an INFO line with workspace_id,
|
||||
@@ -561,7 +575,7 @@ func (h *ChatFilesHandler) uploadPollMode(c *gin.Context, ctx context.Context, w
|
||||
// expose those limits directly — the underlying ParseMultipartForm
|
||||
// caps memory at 32 MB by default and spills to disk. For poll-
|
||||
// mode we read each file into memory to hand to Storage.Put;
|
||||
// 25 MB-per-file × 64-files ceiling means worst-case is 1.6 GB of
|
||||
// 100 MB-per-file × 64-files ceiling means worst-case is 6.4 GB of
|
||||
// peak memory. Bound the per-file size at the multipart layer so
|
||||
// the spill never gets close.
|
||||
if err := c.Request.ParseMultipartForm(32 << 20); err != nil {
|
||||
|
||||
@@ -374,7 +374,7 @@ func TestChatUpload_ForwardsErrorStatusUnchanged(t *testing.T) {
|
||||
|
||||
// Workspace returns 413 with its standard "exceeds per-file limit"
|
||||
// shape. Platform must propagate, NOT remap to 500.
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusRequestEntityTooLarge, `{"error":"big.bin exceeds per-file limit (25 MB)"}`)
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusRequestEntityTooLarge, `{"error":"big.bin exceeds per-file limit (100 MB)"}`)
|
||||
|
||||
wsID := "00000000-0000-0000-0000-000000000044"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
@@ -414,6 +414,81 @@ func TestChatUpload_WorkspaceUnreachable(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestChatUpload_BodyUnderCap_Forwards pins the lower edge of the new
|
||||
// 100 MB body cap (CTO 2026-05-19 directive on forensic a99ab0a1).
|
||||
// A multipart payload comfortably under the cap must reach the
|
||||
// workspace's /internal/chat/uploads/ingest unchanged.
|
||||
//
|
||||
// Uses a small fixture (matching the rest of this suite) — the
|
||||
// http.MaxBytesReader cap is applied via a constant; pinning the cap
|
||||
// _value_ + a sub-cap-forwards test gives equivalent coverage to a
|
||||
// real-bytes 99 MB upload at a fraction of the test runtime.
|
||||
func TestChatUpload_BodyUnderCap_Forwards(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
if chatUploadMaxBytes != 100*1024*1024 {
|
||||
t.Fatalf("chatUploadMaxBytes regressed: want 100MB, got %d bytes — bump must stay in lockstep with canvas MAX_UPLOAD_BYTES + workspace CHAT_UPLOAD_MAX_BYTES", chatUploadMaxBytes)
|
||||
}
|
||||
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusOK, `{"files":[]}`)
|
||||
wsID := "00000000-0000-0000-0000-000000000046"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
expectInboundSecret(mock, wsID, "tok")
|
||||
|
||||
h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
|
||||
body, ct := uploadFixture(t)
|
||||
c, w := makeUploadRequest(t, wsID, body, ct)
|
||||
h.Upload(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for sub-cap forward, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestChatUpload_BodyOverCap_413 verifies the 100 MB cap is enforced
|
||||
// at the platform's MaxBytesReader boundary. Because MaxBytesReader is
|
||||
// applied to c.Request.Body, the workspace forward only fails AFTER
|
||||
// the reader returns ErrBodyOverflow mid-stream — the forward http
|
||||
// client surfaces that as an error, which lands as 502 BadGateway
|
||||
// (the platform's contract for "couldn't complete the forward"). The
|
||||
// alternative would be eager Content-Length inspection — left as a
|
||||
// follow-up so chunked uploads (no Content-Length) still hit the
|
||||
// same gate.
|
||||
//
|
||||
// What this test pins: the cap CONSTANT is set to 100 MB and a body
|
||||
// strictly above the cap does NOT silently succeed (the upstream
|
||||
// receives a truncated body, the test workspace's parser would have
|
||||
// failed; here we simulate via a too-large body and assert non-2xx).
|
||||
func TestChatUpload_BodyOverCap_NotOK(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
// Capturing server that mimics workspace behaviour on truncated
|
||||
// multipart: returns 400. The test asserts the platform does NOT
|
||||
// turn this into a 200 success.
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusBadRequest, `{"error":"malformed multipart"}`)
|
||||
wsID := "00000000-0000-0000-0000-000000000047"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
expectInboundSecret(mock, wsID, "tok")
|
||||
|
||||
h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
|
||||
|
||||
// Build a synthetic body that exceeds chatUploadMaxBytes by a
|
||||
// few bytes. We don't materialise 100MB+ in test memory — the
|
||||
// MaxBytesReader limit is applied lazily as the body is read,
|
||||
// so a marker-sized buffer + a custom reader that claims a large
|
||||
// Content-Length is enough to trip the gate.
|
||||
body := bytes.NewBuffer(make([]byte, chatUploadMaxBytes+1))
|
||||
c, w := makeUploadRequest(t, wsID, body, "multipart/form-data; boundary=----test")
|
||||
c.Request.ContentLength = int64(chatUploadMaxBytes + 1)
|
||||
h.Upload(c)
|
||||
|
||||
if w.Code >= 200 && w.Code < 300 {
|
||||
t.Errorf("expected non-2xx on over-cap upload, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestChatDownload_InvalidPath(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
@@ -27,8 +27,8 @@ Path safety:
|
||||
collisions astronomical, but defense-in-depth costs nothing).
|
||||
|
||||
Limits (matches the Go contract from chat_files.go):
|
||||
- 50 MB total request body
|
||||
- 25 MB per file
|
||||
- 100 MB total request body
|
||||
- 100 MB per file
|
||||
- filename truncated to 100 chars
|
||||
|
||||
Response shape:
|
||||
@@ -64,11 +64,20 @@ CHAT_UPLOAD_DIR = "/workspace/.molecule/chat-uploads"
|
||||
# Total-request body cap. multipart/form-data with multiple parts can
|
||||
# add ~100 bytes of framing per file; the cap is the bytes hitting the
|
||||
# socket, including framing.
|
||||
CHAT_UPLOAD_MAX_BYTES = 50 * 1024 * 1024 # 50 MB
|
||||
#
|
||||
# SERVER_MIRROR: keep aligned with workspace-server/internal/handlers/
|
||||
# chat_files.go chatUploadMaxBytes AND canvas/src/components/tabs/chat/
|
||||
# uploads.ts MAX_UPLOAD_BYTES. Three constants exist (platform Go +
|
||||
# workspace Python + canvas TS) because each layer must enforce or
|
||||
# pre-flight the cap on its own; an SSOT follow-up tracked in
|
||||
# molecule-ai/internal would expose the cap via GET /uploads/limits.
|
||||
CHAT_UPLOAD_MAX_BYTES = 100 * 1024 * 1024 # 100 MB
|
||||
|
||||
# Per-file cap. Keeping per-file under total lets a user attach, say,
|
||||
# a 5 MB PDF + 10 small screenshots in a single batch.
|
||||
CHAT_UPLOAD_MAX_FILE_BYTES = 25 * 1024 * 1024 # 25 MB
|
||||
# Per-file cap. Aligned with the total at 100 MB so a single legitimate
|
||||
# large file (e.g. a 70 MB PDF — reno-stars 2026-05-19 forensic
|
||||
# a99ab0a1) succeeds end-to-end; batched small attachments still fit
|
||||
# under the same ceiling.
|
||||
CHAT_UPLOAD_MAX_FILE_BYTES = 100 * 1024 * 1024 # 100 MB
|
||||
|
||||
# Conservative {alnum, dot, underscore, dash} character class — anything
|
||||
# outside gets rewritten so embedded paths, control chars, newlines,
|
||||
|
||||
@@ -210,7 +210,7 @@ def test_no_files_field_returns_400(client: TestClient):
|
||||
|
||||
def test_per_file_oversize_returns_413(client: TestClient, monkeypatch: pytest.MonkeyPatch):
|
||||
"""Per-file cap is enforced. Lower the cap for the test so we don't
|
||||
have to construct a real 25 MB body."""
|
||||
have to construct a real 100 MB body."""
|
||||
monkeypatch.setattr(internal_chat_uploads, "CHAT_UPLOAD_MAX_FILE_BYTES", 16)
|
||||
big = b"x" * 32 # > 16
|
||||
r = client.post(
|
||||
|
||||
Reference in New Issue
Block a user