Skip to content

fix(site): drop the per-file asset size limit from the static manifest - #609

Merged
netanelgilad merged 3 commits into
mainfrom
claude/disc-upload-observability-x1nzfd
Aug 31, 2026
Merged

fix(site): drop the per-file asset size limit from the static manifest#609
netanelgilad merged 3 commits into
mainfrom
claude/disc-upload-observability-x1nzfd

Conversation

@netanelgilad

@netanelgilad netanelgilad commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Note

Description

Removes the 25 MiB per-file size limit that buildAssetManifest() enforced on static assets. The limit existed because assets were read fully into memory via readFile() before hashing; hashing now streams the file in chunks through createReadStream(), so an asset is never held whole in memory at manifest time and there is no reason to reject large files. Deployments containing large assets (videos, WASM bundles, model files) no longer fail at manifest build.

Related Issue

None

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Other (please describe):

Changes Made

  • Dropped MAX_ASSET_SIZE_BYTES and the InvalidInputError thrown for oversized assets in packages/cli/src/core/site/manifest.ts.
  • Added an internal hashAssetFile() helper that computes the same app-id-salted sha256 digest as hashAsset() but consumes the file as a read stream, so memory use is bounded by the chunk size rather than the file size.
  • buildAssetManifest() still stat()s each file for the manifest size field, but no longer reads its contents into a Buffer.
  • Updated docs/deployments.md: replaced the "files over 25 MiB fail with a per-file error" note with a statement that there is no per-file limit and that assets are hashed by streaming; the 100,000-file cap is unchanged.
  • Tests: replaced the oversized-file rejection test with one asserting a >25 MiB file lands in the manifest with the correct size, and added a test that a multi-chunk (200 KiB) file hashes to the same digest as hashAsset() over the whole buffer, guarding against chunk-boundary regressions.
  • Kept hashAssetFile() unexported (Knip flags unused exports) and formatted the new assertions per Biome.

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed
  • All tests pass (npm test)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • I have updated docs/ (AGENTS.md) if I made architectural changes

Additional Notes

The hash format is unchanged — sha256(utf8(app_id) || raw file bytes) truncated to 32 hex chars — so previously deployed manifests and server-side dedup by hash remain valid. Upload of large assets is still subject to whatever limits the presigned-URL storage backend imposes; this change only removes the client-side manifest-time rejection.


🤖 Generated by Claude | 2026-08-31 09:43 UTC | cbdaa40

buildAssetManifest is used only by deployStaticSite — the S3 arm, where
assets go straight to storage by presigned PUT and never pass through our
servers. There is no per-file ceiling to mirror: the python upload driver
this arm replaces has none, so the 25 MiB check was a limit the other arm
never had, and it turned any app with a large file in dist/ into a
permanently failing publish once the flag caught it. Seen in prod at 10%:
one app retried a 109 MB build artifact in dist/ every 1-3 minutes, 16
failures in an hour.

The check was also the thing keeping whole-file reads safe, so hashing now
streams: hashAssetFile updates the digest chunk by chunk, making peak memory
one chunk instead of the file. The buffer form stays for callers that already
hold the bytes, and a test pins the two to the same digest across a
chunk boundary.

Note the upload side still buffers a whole file per worker
(uploadPresignedAsset -> readFile, MAX_UPLOAD_CONCURRENCY documents it), so
peak memory there is concurrency x largest asset. Streaming that too needs a
per-attempt stream, since a consumed stream cannot be replayed across the
three upload retries — left as a follow-up rather than folded in here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G1E31AZvYBa86zpcYJFD1g
@netanelgilad netanelgilad self-assigned this Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.1.12-pr.609.cbdaa40

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.1.12-pr.609.cbdaa40"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.1.12-pr.609.cbdaa40"
  }
}

Preview published to npm registry — try new features instantly!

@netanelgilad

Copy link
Copy Markdown
Contributor Author

@claude fix the failing PR checks here

@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown

Claude finished @netanelgilad's task in 3m 36s —— View job


Failing checks fixed

  • Gather context (PR diff, CI status)
  • Reproduce failures locally
  • Fix Lint failure
  • Fix Knip failure
  • Verify typecheck + tests
  • Commit & push (ad78df8)

What was failing

Two checks were red; Typecheck, Test, and Package Preview were already green.

1. Lint — Biome formatting. The new assertion in packages/cli/tests/core/site-manifest.spec.ts:186 exceeded the line width and Biome wanted it wrapped:

expect(manifest["/chunky.bin"].hash).toBe(
  hashAsset("test-app-id", content),
);

2. Knip — unused export:

Unused exports (1)
hashAssetFile  function  packages/cli/src/core/site/manifest.ts:41:23

hashAssetFile is only called from buildAssetManifest in the same file and isn't referenced by tests, so the export was dead. Dropped the keyword rather than adding a Knip ignore — it stays a module-private helper, which matches how the rest of core/site/ is structured. hashAsset remains exported (used by tests and as the documented hashing contract).

Also cleaned up

docs/deployments.md:25 still claimed "Files over 25 MiB fail with a per-file error" — the stale sentence your PR description flagged. Replaced it with a note that there is no per-file limit and that hashing streams in chunks, per the repo's keep-docs-updated rule.

Verification

Check Result
bun run lint pass (1 pre-existing unrelated info on MiniflareFunctionManager, not an error)
bun run knip pass
bun run typecheck pass
bun run test 719 passed, 17 skipped, 1 failed

The one test failure is tests/cli/dev.spec.ts > serves both conventions under the Deno fallback runtime, which needs a deno binary this runner doesn't have. It's unrelated to the manifest change and the PR's Test check is green on CI. All site-manifest.spec.ts cases pass, including the new >25 MiB manifest case and the chunk-boundary digest case.
· Branch

- Format the chunked-hash assertion in site-manifest.spec.ts (Biome).
- Stop exporting hashAssetFile; it is only used inside manifest.ts, and
  the unused export failed Knip.
- Drop the now-stale "files over 25 MiB fail" sentence from
  docs/deployments.md.

Co-authored-by: Netanel Gilad <3474905+netanelgilad@users.noreply.github.com>
@netanelgilad
netanelgilad enabled auto-merge (squash) August 31, 2026 10:26
@netanelgilad
netanelgilad merged commit 36f1c65 into main Aug 31, 2026
14 checks passed
@netanelgilad
netanelgilad deleted the claude/disc-upload-observability-x1nzfd branch August 31, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants