Skip to content

Latest commit

 

History

History
129 lines (106 loc) · 6.19 KB

File metadata and controls

129 lines (106 loc) · 6.19 KB

Bug Sweep: request transform / response handler / adapters

Scope reviewed:

  • lib/request/request-transformer.ts
  • lib/request/response-handler.ts
  • lib/request/fetch-helpers.ts
  • lib/request/helpers/{tool-utils,input-utils,model-map}.ts
  • lib/oc-chatgpt-import-adapter.ts
  • lib/oc-chatgpt-orchestrator.ts
  • lib/oc-chatgpt-target-detection.ts
  • lib/prompts/codex.ts

Finding 1 — trimInputForFastSession discards the leading developer/system context it deliberately preserved

File: lib/request/request-transformer.ts:646-650

Buggy code:

const trimmed = input.filter((_item, index) => keepIndexes.has(index));
if (trimmed.length === 0) return input;
if (input.length <= maxItems && excludedHeadIndexes.size === 0) return input;
if (trimmed.length <= safeMax) return trimmed;
return trimmed.slice(trimmed.length - safeMax);

Why it's wrong: Earlier in the function the head loop (lines 623-639) deliberately adds the first one or two short developer/system items to keepIndexes, and the tail loop (lines 641-644) adds the last safeMax items (safeMax = Math.max(8, Math.floor(maxItems))). Because trimmed is built via input.filter(...has(index)), the preserved head items appear FIRST in trimmed.

When the conversation is longer than safeMax, the head indices (0/1) are distinct from the tail range (input.length - safeMax .. end), so trimmed.length becomes safeMax + 1 or safeMax + 2. The final trimmed.slice(trimmed.length - safeMax) then keeps only the last safeMax entries — which are exactly the tail items — and slices the leading head items off the front.

The function's own docstring states: "Keeps a small leading developer/system context plus the most recent items." The slice undoes that for any history longer than safeMax.

Triggering input -> actual vs expected:

  • Input: 50 items, maxItems = 30 (safeMax = 30), item 0 = short developer instruction, preferLatestUserOnly = false (non-trivial turn — the common multi-turn fast-session case).
  • keepIndexes = {0, 1, 20..49} -> trimmed.length = 32 -> trimmed.slice(2) = items[20..49].
  • Actual: the leading developer/system instruction (items 0/1) is dropped.
  • Expected: leading developer/system context retained alongside the most recent items.

Reachable in production: resolveFastSessionInputTrimPlan only sets preferLatestUserOnly=true for trivial single-line turns (which return early before this slice). Non-trivial fast-session turns hit this path with preferLatestUserOnly=false.

Severity: MEDIUM (degraded prompt context in fast-session mode; non-host project/developer instructions are stripped from the request). Confidence: HIGH (deterministic; the code adds the head indices then unconditionally removes them).

Suggested fix: Reserve room for the head when slicing, e.g. partition trimmed into head vs tail and cap only the tail, or compute the slice as [...headItems, ...tailItems.slice(tailItems.length - (safeMax - headItems.length))], so the preserved leading context is never sliced away.


Finding 2 — response.output_text.done (and reasoning-summary .done) read text via getStringField, which can wipe accumulated deltas on an empty/whitespace final payload

File: lib/request/response-handler.ts:630-639 (and 670-677 for reasoning summary)

Buggy code:

if (data.type === "response.output_text.done") {
	setOutputTextValue(
		state,
		outputIndex,
		getNumberField(eventRecord, "content_index"),
		getStringField(eventRecord, "text"),   // <-- trimmed/non-empty gate
		eventRecord.phase,
	);
	return;
}

getStringField returns null when the value is empty or whitespace-only (value.trim().length > 0 ? value : null). The file's own doc comment (lines 54-59) explicitly warns: "For textual payloads where whitespace is meaningful, use a field-specific accessor such as getDeltaField instead of reusing this helper." The .delta handlers correctly use getDeltaField, but the .done handlers use getStringField.

Why it's wrong / wrong behavior: setOutputTextValue(..., null, ...) deletes the accumulated key:

if (!text) {
	state.outputText.delete(key);   // line 262-265
	setPhaseTextSegment(state, phase, key, null);
	return;
}

So if deltas accumulated content (e.g. "Hello") and a terminal response.output_text.done arrives with an empty or whitespace-only text, the accumulated text for that output:content key is deleted instead of finalized, dropping it from the synthesized final response.

Triggering input -> actual vs expected:

  • Stream: output_text.delta "Hello world", then output_text.done with text: "" (or " ").
  • Actual: accumulated "Hello world" is deleted; final JSON loses that content part's text.
  • Expected: the accumulated delta text is preserved as the final value.

Severity: LOW (the OpenAI Responses API normally carries the full text on .done; an empty/ whitespace .done is the edge that triggers loss). Confidence: MEDIUM (depends on upstream emitting an empty terminal text event).

Suggested fix: Use getDeltaField (length>0 only, no trim) for the .done text fields, and/or guard setOutputTextValue so an empty .done does not delete already-accumulated delta text.


Notes / examined and considered correct

  • getModelConfig variant parsing, coerceReasoningEffort fallback tables, and resolveInclude (always re-adds reasoning.encrypted_content) behave as documented.
  • Model-family mapping in model-map.ts (codex aliases -> gpt-5.3-codex; gpt-5.4/5.5 -> gpt-5.2 prompt family) is internally consistent with MODEL_PROFILES.
  • mergeRecord / applyAccumulatedOutputText / appendPhaseTextSegment ordering and the delta-append fast path are correct.
  • filterInput stripIds gating (stripped only when not background mode) is correct.
  • Import-adapter dedup precedence, remapActiveIndex, and matchDestination index handling are correct; previewOcChatgptImportMerge index alignment between merged.accounts and destinationAccounts is sound.
  • Orchestrator atomic write (temp+rename, 0o600/0o700, retry) and target-detection scope/ambiguity logic are correct.
  • convertSseToJson pre-append size cap, malformed-chunk handling, and readWithTimeout cleanup are correct.