Skip to content

fix(combos): PUT /api/combos/{id} rejects empty / no-op bodies with 400 - #4904

Closed
KooshaPari wants to merge 41 commits into
diegosouzapw:release/v3.8.36from
KooshaPari:fix/api-combos-reject-empty-put-body
Closed

KooshaPari wants to merge 41 commits into
diegosouzapw:release/v3.8.36from
KooshaPari:fix/api-combos-reject-empty-put-body

Conversation

@KooshaPari

Copy link
Copy Markdown
Contributor

fix(combos): PUT /api/combos/{id} rejects empty / no-op bodies with 400

Closes #4898.

Summary

PUT /api/combos/{id} with an empty body {} (or { name: undefined } etc.) was a no-op that:

  • Still triggered the cloud-sync path (syncToCloudIfEnabled()) — wasted bandwidth on a low-bandwidth link.
  • Returned 200 with the unchanged combo — making "stuck toggle" bugs invisible: the dashboard thinks it succeeded, the user sees a button that "doesn't work", no console signal.

The new check fires after the JSON parse but before any side effect: if the body has zero updatable fields, return 400 with the standardized envelope.

Diff

src/app/api/combos/[id]/route.ts:101-122 — added 22 lines:

// Reject empty / no-op updates early. Saves a round-trip to the cloud-sync
// path and surfaces "stuck toggle" bugs to the client.
const updatableKeys = Object.keys(rawBody ?? {}).filter(
  (k) => k !== "id" && rawBody?.[k] !== undefined
);
if (updatableKeys.length === 0) {
  return NextResponse.json(
    {
      error: {
        message: "Invalid request",
        details: [
          {
            field: "body",
            message: "At least one updatable field is required",
          },
        ],
      },
    },
    { status: 400 }
  );
}

Test coverage

Three new tests in tests/unit/combo-routes-composite-tiers.test.ts:

Test Asserts
PUT /api/combos rejects an empty body {} with 400 400 with the exact standardized envelope
PUT /api/combos rejects a body of only undefined fields with 400 { name: undefined, isActive: undefined } → 400 with the same message
PUT /api/combos still accepts a single-field update (regression guard) { isActive: false } → 200; the handleToggleCombo flow stays unbroken
11/11 pass (8 prior + 3 new)

Pre-commit hooks pass: prettier, eslint, docs-sync (i18n + openapi), t11:any-budget, tracked-artifacts.

Why this is worth landing

  1. Wasted cloud-sync round-trip. A user on a low-bandwidth link saw every misfire sync to the cloud.
  2. Discoverability. A PUT /api/combos/{id} {} was a silent no-op. Users with a stale UI (the kind that triggers the prior 400-on-toggle bug from Bug: PUT /api/combos/{id} returns 400 on first GUI edit for any combo created in v3.8.31 or earlier (superRefine gate rejects pre-3.8.33 fallbackCompressionMode="lite" configs) #4773) had no signal to look at the console.
  3. Tiny diff. 22 lines added to the route, 49 lines of tests. Single-purpose.
  4. Plays nicely with [QoL] /dashboard/combos: handleToggleCombo silent failures (no toast, no error indication) #4893. The handleToggleCombo QoL fix surfaces the new 400 message in a toast: "At least one updatable field is required" is now user-visible instead of silent.

Risk

  • None for the existing happy path — all 8 prior tests still pass.
  • One regression guard test added for the legitimate { isActive: false } flow.

Related

Diego Rodrigues de Sa e Souza and others added 30 commits June 23, 2026 03:44
Scheduled VACUUM now follows Storage page settings (scheduledVacuum/vacuumHour) as single source of truth; env-flag control path removed. 11/11 vacuum-scheduler tests pass against release/v3.8.35 tip; no orphaned env refs. Integrated into release/v3.8.35.
diegosouzapw#4753)

noAuth providers now classified free (union of legacy list + NOAUTH_PROVIDERS chat-tier derivation), -free arena_elo alias, and auto/<cat>:free returns an empty pool when no free candidate matches (opt-in legacy fallback via OMNIROUTE_AUTO_FREE_FALLBACK_TO_FULL_POOL). New env var documented in .env.example + ENVIRONMENT.md; CHANGELOG bullet added (maintainer co-author). 46/46 node + 56/56 vitest tests pass on release tip; env-doc-sync, docs-sync, typecheck:core, lint, file-size all green. Integrated into release/v3.8.35.
… puros (diegosouzapw#3501) (diegosouzapw#4571)

chatCore god-file decomposition (diegosouzapw#3501): extract 6 pure leaves (cacheUsageMeta, executorClientHeaders, nonStreamingResponseBody, skillsFormat, streamErrorResult, streamFinalize) from chatCore.ts. Rebased onto release/v3.8.35 tip (resolved single chatCore.ts conflict — removed now-extracted inline buildExecutorClientHeaders). 265/265 chatcore tests, 26/26 new leaf tests, typecheck:core, cycles, file-size all green. Integrated into release/v3.8.35.
…dentials para leaves (diegosouzapw#3501) (diegosouzapw#4646)

chatCore diegosouzapw#3501: extract resolveExecutorWithProxy + getExecutionCredentials to leaves (executorProxy.ts, executionCredentials.ts). Clean cherry-pick onto release tip post-diegosouzapw#4571. 12/12 new leaf tests, typecheck:core, cycles, file-size green. Integrated into release/v3.8.35.
…egosouzapw#3501) (diegosouzapw#4708)

chatCore diegosouzapw#3501: extract Claude upstream-message transforms to leaf (claudeUpstreamMessages.ts + claudeMessageTypes.ts). Clean cherry-pick post-diegosouzapw#4646. 8/8 new leaf tests, typecheck/cycles/file-size green. Integrated into release/v3.8.35.
…#3501) (diegosouzapw#4717)

chatCore diegosouzapw#3501: extract persistAttemptLogs to leaf (attemptLogging.ts). Rebased onto release tip post-diegosouzapw#4708 (resolved imports conflict: kept tip's resolveCompressionHeader from compression Phase 3, dropped now-unused logTruncation import moved into the leaf). 288/288 chatcore tests, typecheck/cycles/file-size green. Integrated into release/v3.8.35.
…leaves (diegosouzapw#3501) (diegosouzapw#4721)

chatCore diegosouzapw#3501: extract stageTrace + compressionUsageReceipt to leaves. Clean cherry-pick post-diegosouzapw#4717. 6/6 new leaf tests, typecheck/cycles/file-size green. Integrated into release/v3.8.35.
…teProviderRequest, diegosouzapw#3501) (diegosouzapw#4730)

chatCore diegosouzapw#3501: extract prepareUpstreamBody (first sub-slice of executeProviderRequest) to leaf (upstreamBody.ts). Clean cherry-pick post-diegosouzapw#4721. 7/7 new leaf tests, full 301/301 chatcore suite, typecheck/cycles/file-size green. Completes the 6-PR chatCore decomposition stack into release/v3.8.35.
…) (diegosouzapw#4757)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…e set (diegosouzapw#4758)

The release-green pre-flight (Solution C) previously covered only a subset of the
gates that run exclusively on the release PR (PR→main), so reds still accrued
silently on release/** and surfaced in ~40-min layers at release time (v3.8.34:
3 CI rounds — CodeQL sanitization, then the fail-fast Quality Ratchet revealing
openapi then cyclomatic-complexity one push at a time, plus zizmor/integration).

Now check:release-green reproduces the COMPLETE release-PR gate set and reports
EVERY red in one pass (collected, not fail-fast):

- New DRIFT ratchets (report-only, rebaselined at release, never block):
  cyclomatic complexity, dead-code, type-coverage, compression-budget,
  openapi-coverage, workflow-lint (zizmor), codeql-ratchet.
- New HARD gates (real defects): docs-all (fabricated-docs strict + i18n mirror
  sync) and the integration test suite (gated behind !--quick).

The only release-PR gates it still cannot reproduce locally are GitHub-side CodeQL
semantic analysis and SonarQube/SonarCloud (external services).

The nightly-release-green workflow and /green-prs inherit the expanded coverage
automatically (they invoke this script), so cycle drift is now surfaced
continuously and the release PR is green on its first CI run.

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…#4698) (diegosouzapw#4755)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…iegosouzapw#4694)

Phase 4A: Output Styles registry + D0 telemetry. Integrated into release/v3.8.35.
…zapw#4694] (diegosouzapw#4707)

Phase 4B: SLM tier for ultra. Integrated into release/v3.8.35.
…acked on diegosouzapw#4707] (diegosouzapw#4716)

Phase 4C: adaptive context-budget compression. Integrated into release/v3.8.35.
…diegosouzapw#4716] (diegosouzapw#4720)

Phase 4 D1: offline evaluation harness. Integrated into release/v3.8.35.
…diegosouzapw#4712) (diegosouzapw#4756)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…ePageClient (diegosouzapw#4759, diegosouzapw#4745, diegosouzapw#4596) (diegosouzapw#4761)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
diegosouzapw#4746) (diegosouzapw#4768)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…db-rules (diegosouzapw#4775)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…#4783)

Canonical STRIDE threat model. Integrated into release/v3.8.35.
…pw#4793)

Smoke test guarding the dashboard home client render (regression diegosouzapw#4745/diegosouzapw#4759). Code fix already landed via diegosouzapw#4761; this PR's jsdom smoke test is the net-new regression guard. Integrated into release/v3.8.35.
…onfigs (pre-3.8.33 fallbackCompressionMode="lite") round-trip on the first GUI edit (diegosouzapw#4774)

Auto-promote zeroLatencyOptimizationsEnabled + strip v3.8.31-era removed keys so legacy combo configs round-trip through PUT /api/combos/{id} on first GUI edit (closes diegosouzapw#4382 followup). Pre-merge: rewrote the now-stale reject test to assert auto-promotion + added passthrough/round-trip regression guards; reconciled combos/page.tsx file-size baseline. Integrated into release/v3.8.35.
…teProviderRequest (diegosouzapw#3501) (diegosouzapw#4762)

chatCore diegosouzapw#3501: extract parseNonStreamingResponseBody + recordNonStreamingUsageStats. Integrated into release/v3.8.35.
…uzapw#3501) (diegosouzapw#4779)

chatCore diegosouzapw#3501: extract recordContextEditingTelemetryHook. Integrated into release/v3.8.35.
…3501) (diegosouzapw#4792)

chatCore diegosouzapw#3501: extract recordCompressionCacheStats. Integrated into release/v3.8.35.
…3501) (diegosouzapw#4794)

chatCore diegosouzapw#3501: extract writeCavemanOutputAnalytics. Integrated into release/v3.8.35.
…ão-streaming, diegosouzapw#3501) (diegosouzapw#4780)

chatCore diegosouzapw#3501: extract scheduleQuotaShareConsumption (non-streaming POST-hook). Integrated into release/v3.8.35.
…rtilhado DRY, diegosouzapw#3501) (diegosouzapw#4776)

chatCore diegosouzapw#3501: extract emitRequestGamificationEvent (DRY streaming/non-streaming). Integrated into release/v3.8.35.
diegosouzapw#4782)

chatCore diegosouzapw#3501: extract runPluginOnResponseHook. Integrated into release/v3.8.35.
diegosouzapw and others added 11 commits June 23, 2026 10:13
…ST-hook streaming, diegosouzapw#3501) (diegosouzapw#4784)

chatCore diegosouzapw#3501: extract scheduleStreamingQuotaShareConsumption (streaming POST-hook). Integrated into release/v3.8.35.
…age streaming, diegosouzapw#3501) (diegosouzapw#4791)

chatCore diegosouzapw#3501: extract recordStreamingUsageStats. Integrated into release/v3.8.35.
…eaming, diegosouzapw#3501) (diegosouzapw#4790)

chatCore diegosouzapw#3501: extract recordStreamingCost (per-request streaming cost). Integrated into release/v3.8.35.
…lease-green (diegosouzapw#4799)

README compression credits (ponytail/OmniCompress) + env-doc-sync ignore for eval-only OMNIROUTE_EVAL_CREDENTIALS (restores release-green after diegosouzapw#4720). Integrated into release/v3.8.35.
diegosouzapw#4774 follow-up) (diegosouzapw#4800)

Restore file-size release-green. Integrated into release/v3.8.35.
…o docs/openapi.yaml (diegosouzapw#4781)

Redoc /api/docs + OpenAPI spec consolidated to docs/openapi.yaml (canonical 201-path complete spec; old path → legacy fallback). All refs/gates/tests/CI updated. Integrated into release/v3.8.35.
…ial, per-request control (diegosouzapw#4801)

The README compression section listed the 9 input engines but not the Phase 4
layers now in production:
- Output Styles (output-axis steering: terse-prose / less-code / terse-cjk, lite/full/ultra)
- adaptive context-budget dial (reserve-output|percentage|absolute · floor|replace-autotrigger|off)
- per-request x-omniroute-compression precedence + the offline eval harness
Also bumped the highlights range to v3.8.35, expanded the compression feature bullet,
and marked the GUIDE's Phase 4 row Shipped (was 'Planned' — it's merged on v3.8.35).

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- CHANGELOG: complete 3.8.35 section (all 35 commits since v3.8.34,
  contributor attribution: @rdself @megamen32 @KooshaPari @JxnLexn)
- docs(security): align THREAT_MODEL.md refs with real code
  (routeGuard.ts, tokenLimits.ts, /api/monitoring/health) — fabricated-docs gate
- check:fabricated-docs: skip docs/superpowers/specs (dated research reports)
- i18n: sync 3.8.35 section into 41 CHANGELOG mirrors (docs-sync size gate)
- ratchet rebaseline: cyclomatic 1916->1920, eslintWarnings 3907->3912
  (inherited cycle drift; release-finalize diff is docs-only)
Cycle base-reds that only run on PR→main (not the PR→release fast-path):

- test(autoCombo): suffixComposition-4517 used node:test in a vitest-only dir
  (diegosouzapw#4753) → vitest found no suite. Switch to the vitest API. (Vitest job)
- test(agentSkills): openapiParser fixture wrote docs/reference/openapi.yaml;
  parser reads docs/openapi.yaml since diegosouzapw#4781 → point fixture at the new path.
  (Unit/Coverage/Node24/Node26 shard 4)
- test(integration): proxy-pipeline source-scan expected inline streaming-cost
  code that diegosouzapw#4790/diegosouzapw#3501 extracted to the recordStreamingCost leaf → assert the
  delegation instead. (Integration 1/2)
- fix(chatCore): derive the log trace id from crypto, not Math.random
  (CodeQL js/insecure-randomness — log-correlation id, not a secret).
- test(resilience): circuit-breaker invalid-cooldown fallback asserted t>29000,
  flaking on slow CI where ~1.6s elapsed gave t=28401 → tolerate wall-clock
  drift (t>25000). (Unit 6/8)
CodeQL js/insecure-randomness (#669): the pending-request id generated in
trackPendingRequest (usageHistory.ts) flows into attempt logging and was flagged
as insecure randomness in a security context. It's a log-correlation id, not a
secret — switch to crypto RNG to clear the alert. Pairs with the chatCore traceId
fix in 37c4978 (same sink).
@KooshaPari
KooshaPari requested a review from diegosouzapw as a code owner June 24, 2026 03:11
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@KooshaPari

Copy link
Copy Markdown
Contributor Author

Closing — verified upstream v3.8.36 (commit 8080800, 16 commits past c522454) already ships the equivalent fix. Every file in this PR exists in upstream with the same purpose. No second implementation needed.

Tracking continues on the 2 unique-value PRs (#4777 refactor with 12 new files, #4778 standalone constant extraction).

@KooshaPari KooshaPari closed this Jun 24, 2026
@KooshaPari
KooshaPari deleted the fix/api-combos-reject-empty-put-body branch July 2, 2026 22:10
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.

5 participants