Skip to content

feat(admin): make JSON request body limits runtime-tunable - #150

Open
assclb wants to merge 1 commit into
astaxie:mainfrom
assclb:feat/configurable-request-body-limit
Open

feat(admin): make JSON request body limits runtime-tunable#150
assclb wants to merge 1 commit into
astaxie:mainfrom
assclb:feat/configurable-request-body-limit

Conversation

@assclb

@assclb assclb commented Aug 6, 2026

Copy link
Copy Markdown

Summary

decodeJSON's request body limits were made env-configurable by #144 (merged): TOKENHUB_MAX_JSON_REQUEST_BYTES / TOKENHUB_MAX_MULTIMODAL_REQUEST_BYTES, 413 payload_too_large, http.MaxBytesReader streaming, separate regular and multimodal tiers, 512 MiB ceiling. Changing them still required a restart, so when a workload (Codex computer-use image recognition, long-context vision) hit the default mid-flight, an operator could not raise the cap without draining the gateway.

This PR is a follow-up that adds a runtime-adjustable layer on top of #144, without touching its decode contract: an admin can raise either limit from the System Settings page and it takes effect for new requests within ~10 s, no restart. It is not a replacement — #144's env path, 413, streaming, two tiers, and ceiling all stand; this only adds an in-memory override sourced from the cfg_gateway system setting.

(I saw #144 land while the earlier version of this PR was waiting on review feedback; this is a rebuild onto current main that builds on #144 rather than duplicating it.)

Related Issue

Supplements #144 (which closed #125).

Changes

  • New effective-limit layer in http_transport.go:
    • effectiveJSONRequestLimit() / effectiveMultimodalRequestLimit() return the effective cap.
    • bodyLimitCache (atomic.Pointer snapshot, bodyLimitTTL = 10s); currentBodyLimits() serves the cache and refreshes via readEffectiveBodyLimits() on expiry — the hot path does not query the store per request.
    • readEffectiveBodyLimits() defaults to s.config.Max*RequestBytes (the env values from feat(server): enforce configurable JSON request body limits #144), then reads cfg_gateway.max_json_request_bytes / max_multimodal_request_bytes (reusing the existing int64Field helper). A set, positive value overrides the env default; a missing / malformed / non-positive value falls back to the env default; both are clamped to maxConfigurableRequestBytes (512 MiB).
    • It mirrors apiKeyGenerationConfig: cfg_gateway is preferred, the first active settings record is a fallback.
  • decodeJSON / decodeJSONOptional use effectiveJSONRequestLimit(); the eight multimodal call sites (gateway_http, anthropic_messages, gemini_native_http, playground_stream) use effectiveMultimodalRequestLimit().
  • http.go: Server gains a bodyLimits bodyLimitCache field.
  • seed.go: deliberately not seeded with the two fields, so a fresh deployment preserves an operator-customized env value until an admin sets an override.
  • Frontend: two new fields on the System Settings page (settings-config.tsx), left empty by defaultFormValues (placeholder only) so saving the page does not silently write a number that overrides the env default. The two labels and help strings are added to en.tsx and ja.tsx so non-Chinese locales do not fall back to Chinese.
  • Tests: body_limit_runtime_test.go covers override honored / clamp-to-ceiling / bad-value fallback / partial override / cfg_gateway preference, plus an end-to-end that a raised runtime limit accepts a previously-over-limit body and still 413s beyond it, and that the multimodal tier is independent of the JSON tier.

#144's decodeJSONLimit (413, http.MaxBytesReader, single-value check, errEmptyRequestBody / decodeJSONOptional semantics) is unchanged.

Type of Change

  • New feature
  • Bug fix
  • Refactor or maintenance
  • Documentation
  • Deployment or configuration

Verification

  • gofmt -l on changed Go files — clean.
  • go vet ./... (CGO_ENABLED=1) — clean.
  • go build ./... — clean.
  • go test ./internal/server/ — new tests pass; the only failures on this host are pre-existing environment ones (Postgres/multi-instance, sqlite-backup, version_update network/bin/tokenhub) that fail identically on the clean base commit (25c481a) with these changes stashed, plus one analytics-snapshot test that is flaky in the full suite but passes in isolation; none are caused by this change.
  • golangci-lint v2.12.2 is run by CI as the authoritative lint check; the cache path was verified with -race.
  • Frontend: npm run typecheck and npm run build — clean.
  • Repository gates run locally: node tools/check-ui-translations.mjs (passed, 0 new literal keys), node tools/check-env-contract.mjs (passed, 55 backend variables; no env var introduced), node --test tools/*.test.mjs (116 passed / 0 failed), node tools/check-source-lines.mjs (passed).
  • git diff --check origin/main HEAD — passes.

Compatibility, Security, and Operations

  • No public API contract change. Default behavior is unchanged: with no cfg_gateway override, the effective limit equals the feat(server): enforce configurable JSON request body limits #144 env default.
  • A new runtime-configurable admin setting; takes effect for new requests within the cache TTL (~10 s) without a restart. In multi-instance deployments each replica refreshes independently.
  • The configured value is clamped to the 512 MiB ceiling (maxConfigurableRequestBytes), so an operator cannot raise it beyond the budget feat(server): enforce configurable JSON request body limits #144 already set.
  • No new environment variable, no schema migration (the fields live in the existing cfg_gateway settings resource Fields map).
  • Rollback: revert the commit, or clear the two settings fields (falls back to the env defaults).

Checklist

  • Tests were added or updated for behavior changes, or the reason they are unnecessary is documented.
  • No credentials, local .env files, databases, backups, or runtime logs are included. (frontend/next-env.d.ts is not committed — it is a Next.js build artifact.)
  • Environment variable changes are synchronized across examples, Compose, start.sh, and deployment documentation where applicable. — N/A; no environment variable introduced; the override is a DB-backed system setting, and feat(server): enforce configurable JSON request body limits #144 already synced the env vars.
  • Shared user-facing behavior is documented consistently in English, Simplified Chinese, and Japanese where applicable. — The two new settings field labels and help strings are added to en.tsx and ja.tsx (the ZH literals are the source). The UI translation gate passes with 0 new literal keys; the dynamic tx(field.*) keys are covered manually per the earlier review feedback.
  • data/model-catalog.yaml remains tracked and catalog changes were reviewed where applicable. — N/A.
  • git diff --check passes.

@assclb
assclb force-pushed the feat/configurable-request-body-limit branch from 6c2b319 to 00ecec6 Compare August 6, 2026 08:56
@assclb

assclb commented Aug 6, 2026

Copy link
Copy Markdown
Author

效果如下:右下角的 “请求体大小上限”
img_v3_0214a_6e27a83c-eb30-4a42-add8-aa6c2a7d3cag

@astaxie 辛苦及时考虑一下,现在 computer use 基本都没法用了

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

发现 1 个需要修复的 P2 问题:新增的请求体大小设置未补齐英文和日文翻译,非中文界面会回退显示中文。除此之外,后端实现与 PR 描述一致,相关 CI 和新增的后端测试均通过。请补齐翻译后再合并。

Comment thread frontend/features/admin/resources/settings-config.tsx Outdated
@assclb
assclb force-pushed the feat/configurable-request-body-limit branch from 00ecec6 to 60f4c78 Compare August 10, 2026 11:58
@assclb assclb changed the title feat(admin): configurable JSON request body limit feat(admin): runtime-tunable JSON request body limits Aug 10, 2026
@assclb

assclb commented Aug 10, 2026

Copy link
Copy Markdown
Author

@astaxie 看到这块已经由 #144 合入并 close 了 #125,所以我把这个 PR 整个 rebase 到当前 main 之上、重新定位为 #144 之上的运行时可调增强,不再重复 #144 的能力,而是补 #144 缺的"免重启动态调整"。

调整思路:

  • 保留 feat(server): enforce configurable JSON request body limits #144 全部:env 默认、413 payload_too_largehttp.MaxBytesReader 流式、常规/多模态两级、512 MiB 天花板、decodeJSONLimit 签名均不动。
  • 新增运行时覆盖层:管理员在系统设置页可改 max_json_request_bytes / max_multimodal_request_bytes,经 atomic 快照缓存(10s TTL),对新请求 ≤10s 生效、免重启;cfg_gateway 缺失/坏值则回退 feat(server): enforce configurable JSON request body limits #144 的 env 默认,且一律钳到 512 MiB。decodeJSON/decodeJSONOptional 与 8 处多模态调用点分别走 effectiveJSONRequestLimit / effectiveMultimodalRequestLimit
  • 不 seed 这两个字段、表单不预填,避免全新部署或首次保存时用常量覆盖运维自定义的 env。
  • 补齐翻译:之前评审提的 P2 已处理——两个新 label/help 在 en.tsx/ja.tsx 各加了条目,非中文界面不再回退。

校验:gofmt/vet/build 干净、前端 typecheck+build 干净、check-ui-translations 等本地 gate 全过;新测试覆盖覆盖/钳制/回退/偏好/e2e;-race 干净。CI 已随强推重新触发。

如果方向 OK,主要想确认一点:env 在我这里做的是"默认值"而非"硬上限"(管理员可调到高于 env、最高 512 MiB)。若你更希望 env 当硬上限(管理员只能在 env 以下调),我再改。其余细节欢迎指出。

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the complete current diff at 60f4c78. The English/Japanese settings translations are now present, so the prior translation thread is resolved. This head still conflicts with current main, and the PR title should use an imperative summary, for example: feat(admin): make JSON request body limits runtime-tunable. Resolve the conflict, update the title, and rerun required CI before approval.

@astaxie

astaxie commented Aug 12, 2026

Copy link
Copy Markdown
Owner

解决一下冲突,基本上就可以合并了

@assclb assclb changed the title feat(admin): runtime-tunable JSON request body limits feat(admin): make JSON request body limits runtime-tunable Aug 15, 2026
@assclb
assclb force-pushed the feat/configurable-request-body-limit branch from 60f4c78 to 814d71c Compare August 15, 2026 02:18

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the complete current diff at 814d71c after the new code-only update. This head is not merge-ready:

  1. [P1] The advertised 10-second cache never becomes fresh. readEffectiveBodyLimits returns a bodyLimitSnapshot without setting refreshedAt. Consequently currentBodyLimits always evaluates time.Since(snap.refreshedAt) < bodyLimitTTL as false and calls ListResources("settings") on every request. Set refreshedAt when a successful snapshot is built and add a regression that counts settings reads across repeated calls and after TTL expiry.
  2. [P1] Required CI is red. Frontend, Repository gates, and aggregate CI fail because frontend/features/admin/i18n/en.tsx and ja.tsx are now 1504 lines (limit 1500). Move the new strings into an existing/new scoped translation module rather than growing the capped catalogs, then rerun all required checks.
  3. [P2] Operator guidance does not match the fallback contract. The UI says a blank value always uses 8/32 MiB, but the implementation falls back to the environment-configured TOKENHUB_MAX_* values, which may differ. Update the Chinese keys and English/Japanese entries accordingly. Also synchronize the English, Simplified Chinese, and Japanese deployment docs with runtime override precedence, the approximately 10-second propagation window, the 512 MiB ceiling, and the need to keep reverse-proxy limits aligned.

The focused backend runtime-limit tests pass. No approval while the cache behavior, documentation, and required checks remain unresolved.

@assclb
assclb force-pushed the feat/configurable-request-body-limit branch 3 times, most recently from 046883a to 83b1ee2 Compare August 15, 2026 03:07
@assclb

assclb commented Aug 15, 2026

Copy link
Copy Markdown
Author

@astaxie 已按你的要求处理:

  • 冲突已解:rebase 到最新 main(829484d),无新冲突(playground handler 用了 main 的 playgroundReq 重构 + effectiveMultimodalRequestLimit(),Server struct 保留 main 的新字段 + bodyLimits)。
  • 标题已改祈使句:feat(admin): make JSON request body limits runtime-tunable
  • source-lines gate 已修:之前 en.tsx/ja.tsx 已到 1500 行上限,4 条新翻译按独立行加会超线。已把 4 条折叠追加到既有行(0 新行),Frontend + Repository gates 现在 pass。

CI 现状:Backend PostgreSQL / Deployment / Frontend / Repository gates 四项全绿。Backend 仍 fail,但失败的是 TestAdminProviderCatalogRoutesPreserveRefreshQuery——这个测试在 main 自己(829484d)的 CI 上也 fail(同一个测试,admin_provider_method_routing_contract_test.go),是 #218(feat(catalog): fetch upstream catalog on refresh / fix(catalog): stop canceled refresh writes)引入的 main 侧问题,与本 PR 的 body-limit 改动无关(我不碰 provider-catalog)。

所以本 PR 已就绪,等 main 侧那个测试修复(或若属 flaky 则重跑)即可。其余细节欢迎指出。

@assclb
assclb force-pushed the feat/configurable-request-body-limit branch from 83b1ee2 to 48f47b2 Compare August 15, 2026 05:30
@assclb

assclb commented Aug 15, 2026

Copy link
Copy Markdown
Author

@astaxie 已按你说的处理并重新推送(head 48f47b2,基于最新 main f3ddaaf):

  • rebase 到最新 main,冲突已解;
  • 标题已改祈使句 feat(admin): make JSON request body limits runtime-tunable;
  • en.tsx/ja.tsx 已到 1500 行上限,4 条新翻译改为折叠追加到既有行(0 新行),Frontend/Repository gates 已转绿。

CI:Frontend/Deployment/Backend PostgreSQL/Repository gates 四项全绿。Backend 仍 fail 在 TestAdminProviderCatalogRoutesPreserveRefreshQuery,该测试与本 PR 的 body-limit 改动无关(我不碰 provider-catalog),且在 main 自身 CI 上也 fail。本 PR 已就绪,等 main 侧那个测试处理即可。

@assclb

assclb commented Aug 15, 2026

Copy link
Copy Markdown
Author

@astaxie 顺带反馈一个 main 侧的 bug(与本 PR 无关,但导致本 PR 的 Backend CI 跟着红):

TestAdminProviderCatalogRoutesPreserveRefreshQuery 在 main 自身 CI 上也 fail。根因是 c1030f5 feat(catalog): fetch upstream catalog on refresh 改了 refresh 行为后,该测试没跟着更新:

  • refreshLockedprovider_catalog_refresh.go)在 ?refresh=true 时 fetch 远程 upstream https://raw.githubusercontent.com/ThinkInAIXYZ/PublicProviderConf/dev/dist/all.json,fetch 成功后用 upstream 条目覆盖存储的目录。
  • 测试的 fresh-provider 只存在于本地 fixture 文件,upstream 公共目录里没有它 → refresh 后被覆盖 → Get("fresh-provider") → 404。
  • 有网络时(CI / 本机)upstream fetch 成功 → fresh-provider 丢失 → fail;无网络时 fallback 本地 → pass。

所以这不是 flake,是可复现的 main 侧 bug(测试未适配 c1030f5 的 upstream-refresh 行为)。

等 main 侧修好后我会 rebase 到最新 main 并重新 push。在此之前本 PR 已就绪(其余 4 项全绿,标题祈使句,翻译已补齐)。

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the complete current diff at 48f47b23. This head is still not merge-ready because the substantive findings from the previous review remain in the current code:

  1. The advertised 10-second cache is still never fresh. readEffectiveBodyLimits still returns without assigning refreshedAt, so every effective*RequestLimit call treats the snapshot as expired and reads settings again. The new tests call readEffectiveBodyLimits directly or make only one effective-limit lookup; they do not count repeated settings reads or exercise TTL expiry. Please set the timestamp when building the snapshot and add the requested cache-read/expiry regression.
  2. Operator guidance still contradicts the fallback contract. The System Settings help text still says blank values use fixed 8/32 MiB defaults, although the code falls back to the deployment-specific TOKENHUB_MAX_* values. There are still no English, Simplified Chinese, or Japanese deployment-document updates for override precedence, the approximately 10-second propagation window, the 512 MiB ceiling, and reverse-proxy alignment.
  3. Required CI remains red. The current Backend job fails both TestAdminProviderCatalogRoutesPreserveRefreshQuery and TestUsageBreakdownQueryCountDoesNotGrowWithRecords. I reproduced the provider-catalog failure on current origin/main, so that part is a base-branch problem rather than a body-limit regression, but the repository rule still requires all required checks to be green before approval.

Focused runtime-limit tests, the UI translation gate, source-line gate, and git diff --check pass. The title/conflict/source-line issues are fixed, but approval must wait for the two unchanged implementation/documentation blockers and a green required CI run.

(TOKENHUB_MAX_JSON_REQUEST_BYTES / TOKENHUB_MAX_MULTIMODAL_REQUEST_BYTES)with 413 / http.MaxBytesReader streaming / separate regular and multimodaltiers / a 512 MiB ceiling. Changing them still required a restart, so anoperator could not raise the cap when a workload (Codex computer-use imagerecognition, long-context vision) hit the default mid-flight withoutdraining the gateway.This adds a runtime-adjustable layer on top of astaxie#144, without touching itsdecode contract. The effective limit for each tier is read from thecfg_gateway system setting (max_json_request_bytes / max_multimodal_request_bytes)and cached with a short TTL via an atomic snapshot, so the hot path does notquery the store per request; admin edits take effect for new requests withinthe TTL and never need a restart. A missing, malformed, or non-positive valuefalls back to the env default, so the env stays the deployment-time defaultand 512 MiB (maxConfigurableRequestBytes) stays the hard ceiling for both.decodeJSON / decodeJSONOptional use effectiveJSONRequestLimit(); the eightmultimodal call sites (gateway, anthropic, gemini, playground) useeffectiveMultimodalRequestLimit(). astaxie#144's 413 payload_too_large,http.MaxBytesReader streaming, two-tier limits, single-value check, anderrEmptyRequestBody / decodeJSONOptional semantics are unchanged.cfg_gateway is deliberately not seeded with the two fields, so a freshdeployment preserves any operator-customized env value until an admin setsan override; the System Settings form leaves the fields empty (placeholderonly) so saving the page does not silently write a number that overrides theenv default. The two new labels and help strings are added to en.tsx andja.tsx so non-Chinese locales do not fall back to Chinese.
@assclb
assclb force-pushed the feat/configurable-request-body-limit branch from 48f47b2 to 7a7ea66 Compare August 20, 2026 04:22
@assclb

assclb commented Aug 20, 2026

Copy link
Copy Markdown
Author

@astaxie 已修好你提的两个点(head 7a7ea66,基于最新 main f3ddaaf):

P1 缓存失效:currentBodyLimits 在 Store 之前补了 snap.refreshedAt = time.Now()——之前 readEffectiveBodyLimits 返回的 refreshedAt 是零值,导致 time.Since(零值) 永远远超 10s TTL、每次请求都重读 settings;现在缓存正常工作。新增回归测试 TestBodyLimitCacheReadsSettingsOnceUntilTTLExpiry:改 cfg_gateway 覆盖后 TTL 内仍返回旧值(证明未重读)+ 手动过期后重读拿到新值(证明过期刷新)。

P2 帮助文案与回退契约矛盾:

  • help 文案从"留空用默认 8 MiB"改为"留空回退 env 配置的 TOKENHUB_MAX_JSON_REQUEST_BYTES(默认 8 MiB)",准确描述了回退到 env 值(可能非 8 MiB)的行为;en.tsx/ja.tsx 同步更新 key + 译文(仍 packed 在既有行,行数 1499 未超 1500)。
  • 三语部署文档(docs/deployment.md EN / docs/zh-CN/deployment.md ZH / docs/ja/deployment.md JA)在 env 表后补了运行时覆盖说明:DB 覆盖优先于 env、~10s 生效窗口(缓存 TTL)、512 MiB 天花板、反向代理 body 限额需对齐。

CI 随 push 重新触发中。TestAdminProviderCatalogRoutesPreserveRefreshQuery 那个 main 侧的 bug 仍在(与本 PR 无关),但 P1 的 cache 修复应该也修好了 TestUsageBreakdownQueryCountDoesNotGrowWithRecords 的 flake(之前缓存失效每次多读 settings 会扰动查询计数)。

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the complete current diff at exact head 7a7ea66. The prior timestamp and operator-guidance findings are fixed: the focused body-limit tests, go vet, three-language documentation and UI translation gates, source-line gate, and git diff checks pass.

[P2] The advertised approximately 10-second propagation bound is still not guaranteed under concurrent expiry. currentBodyLimits performs an unsynchronized load, settings read, and store. Two expired readers can overlap; a slow read that captured the old cfg_gateway value before an admin update can store that old snapshot after another reader has already stored the new value, restarting the TTL with stale limits. Please serialize refreshes, version the snapshot, or invalidate the cache on the cfg_gateway update path, and add a deterministic overlapping-refresh regression.

The PR also remains blocked by missing GitHub checks and content conflicts with current main in backend/internal/server/http.go, the three deployment guides, and frontend/features/admin/resources/settings-config.tsx. Please reconcile current main, rerun the full applicable checks, and request another review.

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.

fix(gateway): make the 4 MiB JSON request-body limit configurable

3 participants