Skip to content

fix(sse): keep Responses continuity when input would ship empty - #14673

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
Laksopan23:fix/copilot-empty-input-400
Sep 24, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
Laksopan23:fix/copilot-empty-input-400

Conversation

@Laksopan23

Copy link
Copy Markdown
Contributor

Summary

  • Inject a placeholder user item into Responses input when chat→Responses translation would otherwise ship input: [] with no conversation/continuity field (GitHub Copilot default chat 400: One of input or previous_response_id or '''prompt''' or '''conversation''' must be provided.).
  • Under auto Responses state policy, keep previous_response_id when input is empty (explicit strip mode still strips).

Root cause: chat→Responses translation can emit input: [] while responsesStatePolicy (mode auto + store-off) strips previous_response_id, leaving a body with neither field. Reverse direction already had a placeholder guard.

Test plan

  • TDD: failing tests first (responses-state-policy.test.ts keep-on-empty, new translator-openai-responses-empty-input-github-400.test.ts)
  • npm run test:vitest — 482/482 PASS
  • chat-pipeline.test.ts — 29/29 PASS
  • Focused suites: responses-handler, responses-state-policy, empty-input-github-400, empty-input-419, strip-store-responses, orphaned-tool-filter, chat-previous-response-id-preserve-mode
  • Focused eslint on changed files — 0 errors

⚠️ base-red inherited: #14547

Files

  • open-sse/translator/request/openai-responses/toResponses.ts — placeholder inject
  • open-sse/utils/responsesStatePolicy.ts — keep previous_response_id when input empty under auto
  • tests updated/added

@diegosouzapw

Copy link
Copy Markdown
Owner

Clean fix for both sides of the empty-input GitHub Copilot 400 — nice touch mirroring the
already-existing reverse-direction placeholder guard (9router#419) instead of inventing a new
pattern. Traced the call order between the translator and responsesStatePolicy and the two
guards don't conflict. Wasn't able to run the suite this round due to devbox contention.
Only pre-merge item: please add a changelog.d fragment.

GitHub Copilot /responses rejects a body with neither non-empty input nor previous_response_id/prompt/conversation. Chat→Responses translation can emit input:[] (system-hoist, empty messages, orphan filter) while responsesStatePolicy auto mode strips previous_response_id, shipping neither field.

- Inject a placeholder user item in toResponses when input is empty and no continuity field is present (mirrors the reverse-direction guard in openai-responses.ts / 9router#419).
- Under auto mode, keep previous_response_id when input is empty so the only continuity field is never stripped; explicit strip mode still wins.
- Pin both paths with unit tests, including the id-preserving continuation case.
@Laksopan23
Laksopan23 force-pushed the fix/copilot-empty-input-400 branch from f8d5954 to 8e168a0 Compare September 24, 2026 09:56
@Laksopan23

Copy link
Copy Markdown
Contributor Author

Rebased onto 7d23bcf8ec and force-pushed to re-trigger CI (old runs were stuck at action_required).

Local verification (green)

  • translator-openai-responses-empty-input-github-400.test.ts + responses-state-policy.test.ts: 12/12 pass
  • responses-handler.test.ts filtered to our change: pass
  • Diff vs base is only the 5 intended files (toResponses.ts, responsesStatePolicy.ts, + 3 tests)

CI failures — all base-red inherited (⚠️ base-red inherited: #14547)

None of these touch files in this PR:

Job Failure Why not ours
API Route Typecheck auggie.ts TS2769/TS18047, projectCombo.ts TS2459/TS2724 Not in this diff
Docs Gates check:env-doc-sync: DEEP_HEALTH_CHECK_ENABLED missing from .env.example Not in this diff
Unit Tests 1–4 e.g. qwen38-max-bare-id-alias.test.ts, credential-path map deepEqual Not in this diff
Fast Quality Gates gates deps file-size mutation-test-coverage open-sse-typecheck typecheck:core Base tip red (see #14547)
responses-handler.test.ts keepalive assertion expects bare {"type":"response.in_progress"}; base emits #14330 frame with sequence_number+response Reproduced on pure base checkout @ 7d23bcf8ec

Green here: Vitest, No new ESLint warnings, Merge integrity, Change Classification, semgrep, semgrep-cloud.

Please merge only after base-red clears (#14547) or with explicit operator override — do not chase these failures in this PR.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 2cf2521 into diegosouzapw:release/v3.8.51 Sep 24, 2026
10 of 16 checks passed
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.

2 participants