fix(orchestrator): cumulative per-conversation budget on automatic run-completion previews - #1689
Merged
Merged
Conversation
…ad of compounding a Codex rollout without bound The per-turn ceiling (v0.51.53) bounds ONE turn, but Codex persists every delivered preview as an inline input_image data URL and its compaction recopies them without a media-aware bound, so N turns still accumulated 8N images into one rollout (measured: 265 images, 18.2 GiB, ~487 MB recopied per compaction). Add the cumulative half of the budget: - a per-conversation count AND byte budget (defaults 48 images / 128 MiB, COMFYUI_MCP_SESSION_PREVIEW_ATTACHMENTS / _BYTES), enforced as an estimate at injectEvent and as fact at the drain and at the codex image loop, where the real fetched bytes are known - past the budget the completion still arrives with every output named, zero pixels attached, and an honest note with get_image coordinates - a user's explicit attachments never spend the automatic budget - the ledger persists beside the session id in the session store, so an orchestrator restart does not reset a budget the living conversation keeps accumulating against; a stored ledger is adopted only by the exact session it was recorded against, and setPreviewLedger refuses a mismatch rather than misfile a safety bound - delivered/withheld counts and bytes are exposed via PanelAgent.previewBudgetStatus()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1516.
get_imagecoordinates; nothing is swallowed and no notice claims pixels the turn does not carryRoot cause
#1521 (v0.51.53) removed the immediate replay amplifier and made the preview bound per turn (8 images). But a long session drains 8 previews per turn into the SAME conversation, and Codex persists every delivered
localImageas an inlineinput_imagedata URL in the thread rollout, which its compaction path retains and recopies without a media-aware bound (upstream: openai/codex#33493). So N turns still accumulated 8N inline images — the reporter measured 265 persistedinput_imagerecords, an 18.2 GiB rollout, ~487 MB recopied per later compaction, and a resumed app-server stalled at ~9 GB. The per-turn cap alone cannot see that shape; the conversation itself needed a ceiling.The fix
New
src/orchestrator/preview-budget.ts:MAX_SESSION_PREVIEW_ATTACHMENTS(default 48) andMAX_SESSION_PREVIEW_BYTES(default 128 MiB), env-overridable viaCOMFYUI_MCP_SESSION_PREVIEW_ATTACHMENTS/COMFYUI_MCP_SESSION_PREVIEW_BYTES. Enforced at the same estimate/fact split the per-turn budget already uses:min(what the turn has left, what the conversation has left); when the session budget binds, the notice names it (...cumulative automatic-preview budget (48 images / ~128 MB) is spent...) and lists fetchable coordinates.[panel note: ...]appended to the turn corrects the claim with coordinates. Bytes are charged per actual delivery — a replayed turn really does write the pixels into the rollout again, which is the accumulation being bounded.{images, bytes}) rides the session-store entry (Entry.pl), survives an orchestrator restart, is adopted only when the live session id is exactly the one it was recorded against, is dropped when a fresh conversation starts, andsetPreviewLedgerrefuses a session mismatch rather than misfile a safety bound.PanelAgent.previewBudgetStatus()exposes delivered images/bytes, withheld count, and the budget.Explicit user attachments and the #468 journal ack/replay semantics are unchanged.
Verification (worktree at origin/main 212132a)
npm ci --ignore-scripts+npm rebuild better-sqlite3— cleannpm run build(tsc) — cleannpm run lint(tsc --noEmit) — cleannpx vitest run src/__tests__/orchestrator/session-preview-budget.test.ts src/__tests__/orchestrator/codex-preview-byte-budget.test.ts— 9/9 (cumulative 8+2-then-none across turns with honest notices; byte budget binding with count room; user-attachment exemption; ledger persistence + exact-session adoption + mismatch refusal; codex byte backstop incl. claim correction and per-thread reset)run-completion-continuation.test.ts+session-store.test.ts— 124/124src/__tests__/orchestrator/— 162 files, 2812/2812What this deliberately does NOT do (follow-ups)
automaticPreviewBytes()is a small follow-up.previewBudgetStatus()in a panel-visible diagnostic is unwired; the data is available.