fix(agent): make managed-fallback provisional buffer limits configurable via env - #4602
fix(agent): make managed-fallback provisional buffer limits configurable via env#4602Veritas-7 wants to merge 1 commit into
Conversation
a050ca4 to
fc56ea2
Compare
|
MERGE_READY — maintainer fix-forward executed (terminal disposition pending independent exact-head approval) Exact head / base / CI
Root defects found in the submitted head (all fixed forward)
Reproduction / validation (all at
|
|
Bounded hold evidence — awaiting independent exact-head approval (signed) State at
Blocker (single, human): @snowykr — an APPROVED review on this exact head. Per the enforced contract, the check turns green on that review ( — |
…ble via env Managed fallback stages streamed events in a provisional buffer whose caps were hardcoded (10,000 events / 16 MiB). Reasoning-heavy streaming can trip the cap during transient provider failures, and the resulting ManagedAttemptBufferOverflowError terminates the whole agent turn — in gjc team runs this leaves workers dead at the prompt (Yeachan-Heo#4618). Expose both caps as operator knobs, read once per transaction: - GJC_FALLBACK_MAX_STAGED_EVENTS (default 10000) - GJC_FALLBACK_MAX_STAGED_BYTES (default 16 MiB) Rebuilt onto current dev by the maintainer from the original submission (a050ca4), resolving the overlap with Yeachan-Heo#4610 (superseded-delta reclamation) semantically: - keep the exported MANAGED_ATTEMPT_MAX_STAGED_* constants as the canonical defaults and feed them to $envpos from @gajae-code/utils, so exponents, hex, and unsafe integers can no longer silently unbind the memory guard (positive integer, digits only; anything else silently falls back to the default) - read the caps through field initializers; the reclamation logic from Yeachan-Heo#4610 is untouched and still runs before the configurable caps reject a batch - #wouldOverflow is shared by managed and non-managed lossless staging, so the knobs bound the provisional transaction in both modes; in non-managed sessions the cap only decides how much reasoning buffers before the batch flushes and streams through (Yeachan-Heo#4601) - port the env tests without as-any casts: events-cap trip, bytes-cap trip, "0" -> default, and 3e0/0x3 digits-only rejection; scrub both env keys in afterEach so baseline tests stay host-env independent - document both knobs in the agent/runtime toggles section with the dual-mode scope and silent-fallback contract; add the agent CHANGELOG [Unreleased] entry alongside the Yeachan-Heo#4610 sibling entry Lore-id: 4602-fixforward Constraint: managed overflow stays fatal/non-retryable (1b4fc60); knobs move only the threshold Constraint: lossless flush-on-overflow (Yeachan-Heo#4601) and superseded-delta reclamation (Yeachan-Heo#4610) unchanged Tested: bun test packages/agent/test/managed-attempt-transaction.test.ts (59 pass) Tested: bun test packages/coding-agent/test/agent-session-fallback-attempt-transaction.test.ts (20 pass) Tested: bun --cwd=packages/agent run check; bun --cwd=packages/agent run test (793 pass) Tested: verify-gjc-state-writers --fail; changelog-history-guard; git diff --check Confidence: high Scope-risk: narrow Reversibility: easy Co-authored-by: Veritas-7 <koreacmc.kr@gmail.com>
fc56ea2 to
18d53e0
Compare
|
Rebuilt onto live dev Why the previous verdict was replacedThe prior PR-body verdict declared Exact head / base / digest
Semantic conflict resolution (vs #4610, now on dev)dev gained superseded-delta reclamation in the same staging path. Resolution: Validation at the exact head (local)
Owner / blocker / nextSingle blocker: an authenticated non-author APPROVED GitHub review at — |
18d53e0 to
151466c
Compare
|
Rebased onto live dev Exact head / base / digest (current)
Semantic resolution (unchanged in substance)
Validation at
|
151466c to
70a1584
Compare
|
Reconstructed onto current dev Exact head / base / digest (current, authoritative)
Overlap inspection since
|
|
Signed status @ head
Blocker (only): one authenticated non-author APPROVED review at — |
70a1584 to
04eca0e
Compare
|
Signed status @ head Reconciliation against fresh dev (this push)
Product CI at the prior head
|
…ow (#4618) ManagedAttemptBufferOverflowError surfaced as one static sentence, so a subagent killed by the provisional staging cap was indistinguishable from a provider or context-window failure — the exact misread reported in recordored." and the error text carried no stage, counts, or limits. The overflow now reports its shape everywhere it can reach: - agent: the typed error carries stage, staged event/byte counts at rejection, and both caps; the message keeps its stable prefix (session retry policy prefix-classifies on it) and appends a shape-only parenthetical stating this is a local staging-buffer limit that reproduces on re-issue, not a provider/context-window failure. Baked into the error itself because the non-retryable local exit path surfaces the thrown error, not the managedFailureMessage wrapper. - coding-agent: the executor retains a bounded, redaction-safe localErrorSummary (closed kind set, sanitized summary) from the subagent's terminal assistant error; receipt preview, errorSummary, subagent tool output, and the await renderer name the local kind and carry the diagnostic instead of the generic error preview. Does not touch cap configurability (#4602) or superseded-delta reclamation (#4610). Lore-id: 4618-buffer-diagnostics Constraint: message prefix must stay byte-identical for session prefix classification Constraint: diagnostics must be shape-only (no provider/prompt text can reach a parent receipt) Constraint: do not duplicate #4602 configurability or #4610 delta reclamation Rejected: enriching managedFailureMessage only | the non-retryable local exit surfaces the thrown error, not the wrapper Rejected: enlarging the caps | workaround path ships with #4602 Confidence: high Scope-risk: moderate Reversibility: trivial Tested: byte-cap + event-cap surfaced diagnostics (agent), propagation, redaction, foreign-kind degradation, generic-error fallback isolation (coding-agent) Not-tested: live provider delta-storm reproduction Closes: #4618
|
Signed status @ head
Blocker (single, human): one authenticated non-author APPROVED review at — |
|
Signed status @ head Check-rollup triage (no product failure)The failing rollup entries at this head come from exactly two runs:
Current standing
Blocker (single, human)One authenticated non-author APPROVED review at — |
probepark
left a comment
There was a problem hiding this comment.
Independent maintainer review — merge blocked.
major — the memory guard becomes disableable
packages/agent/src/agent-loop.ts:98-124: $envpos accepts any positive safe integer, so an operator can set the event/byte limits near Number.MAX_SAFE_INTEGER and effectively remove the provisional-staging memory guard entirely. The failure mode then moves from a bounded, typed local_buffer_overflow to process OOM — strictly worse, and much harder to diagnose.
Required:
- documented hard ceilings
- above-ceiling values clamp to the ceiling with a warning, rather than being honoured (this is the existing session-context budget convention in this repo — follow it)
- boundary-value tests: at ceiling, above ceiling, zero, negative, non-numeric
otherwise
Environment variables are a defensible operator surface for a low-level staging knob, so the mechanism choice is fine. The problem is purely that it is unbounded.
What
Exposes the managed-fallback provisional staging caps as operator knobs, read once per attempt transaction:
GJC_FALLBACK_MAX_STAGED_EVENTS(default10000)GJC_FALLBACK_MAX_STAGED_BYTES(default16777216= 16 MiB)#wouldOverflowis shared by the managed-fallback and the non-managed (lossless) staging transactions, so the knobs bound the provisional staging transaction in both managed fallback and ordinary (non-managed lossless) sessions; in non-managed sessions the cap only decides how much reasoning buffers before the batch flushes and streams through (#4601). Values must be positive integers (digits only); invalid or non-positive values silently fall back to the defaults.Why
Reasoning-heavy streaming (e.g. high/xhigh thinking tiers) can trip the caps during transient provider failures, and the resulting
ManagedAttemptBufferOverflowErrorterminates the whole agent turn — ingjc teamruns this leaves workers dead at the prompt (#4618). Managed-fallback overflow is intentionally non-retryable (local_buffer_overflow, 1b4fc60), so operators need a bounded way to raise the caps without a rebuild.Maintainer rebuild note
The original submission (
a050ca4ac061255758b6e20591c35e3205c78ae1, 0.13.3-era base) was rebuilt onto currentdev(416201eb5e9e50586c60747647dcc48c61d13600) by the maintainer with authorship preserved (Author: Veritas-7, original author date kept; committer Yeachan-Heo). Semantic resolution of the overlap with #4610 (superseded-delta reclamation, on dev): reclamation stays untouched and still runs before the configurable caps reject a batch; the knobs only replace the two threshold operands in#wouldOverflow. Other deltas vs the original: kept the exportedMANAGED_ATTEMPT_MAX_STAGED_*constants as canonical defaults (dev's tests import them), switched env parsing to the repo-standard$envpos(digits-only, safe-integer; rejects1e4/0x10/unsafe values that a bareNumber()parse accepts), read caps via field initializers, dual-mode docs, agent CHANGELOG entry beside the #4610 sibling entry, noas any, plus a non-digit rejection test and env scrubbing inafterEach.Testing
bun test packages/agent/test/managed-attempt-transaction.test.ts→ 59 pass / 0 fail (55 dev baseline incl. the fix(agent): bound managed staging by reclaiming superseded deltas #4610 reclamation test + 4 env-knob tests)bun test packages/coding-agent/test/agent-session-fallback-attempt-transaction.test.ts→ 20 pass / 0 failbun --cwd=packages/agent run check→ clean (biome + tsc)bun --cwd=packages/agent run test→ 793 pass / 0 failbun scripts/verify-gjc-state-writers.ts --fail→ 0 write sites outside sanctioned writersbun scripts/changelog-history-guard.ts→ no released sections removed;git diff --check→ cleanGJC verdict
devbun checkpasses