Skip to content

fix(tools): stop forcing empty instructions on Responses requests - #1646

Open
Agnik47 wants to merge 1 commit into
supermemoryai:mainfrom
Agnik47:fix/openai-responses-instructions
Open

fix(tools): stop forcing empty instructions on Responses requests#1646
Agnik47 wants to merge 1 commit into
supermemoryai:mainfrom
Agnik47:fix/openai-responses-instructions

Conversation

@Agnik47

@Agnik47 Agnik47 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Problem

prepareResponsesWithMemory sets instructions on every forwarded request:

return {
  request: originalResponsesCreate.call(openaiClient.responses, {
    ...params,
    input: cleanedInput,
    instructions: enhancedInstructions,   // always present
  }, requestOptions),
}

When the caller passed no instructions and there are no memories to inject, replaceMemoryContext("", "") returns "" — so the request goes out with instructions: "" where the caller sent nothing.

Per the Responses API contract, supplying instructions at all discards the instructions carried over from a previous_response_id turn. An empty string therefore doesn't just add noise: it silently wipes the caller's system prompt partway through a multi-turn chain.

The no-input early return in the same function already handles this correctly:

...(typeof params.instructions === "string"
  ? { instructions: stripMemoryContext(params.instructions) }
  : {}),

so the two paths in one function disagree about the same field.

Fix

Apply the early return's rule to the main path — spread instructions only when there is something to send or the caller supplied a string:

const instructionsOverride =
  enhancedInstructions || typeof params.instructions === "string"
    ? { instructions: enhancedInstructions }
    : {}

Behaviour by case:

caller instructions memories before after
absent none "" (wipes carried-over prompt) field absent
absent present memory block memory block (unchanged)
string none stale context stripped stale context stripped (unchanged)
string present context replaced context replaced (unchanged)
null none "" null preserved via ...params

Only the first row changes. The catch branch is covered by the same guard, so a failed memory search no longer blanks the field either.

Tests

Three cases added to test/openai-middleware.unit.test.ts, the suite CI already runs when packages/tools changes:

  • caller sent no instructions and nothing to inject → field absent, previous_response_id untouched
  • caller sent no instructions but memories exist → memories still injected
  • caller sent instructions → preserved, stale <supermemory> block stripped

Verified the first test fails on main and passes with the fix; the other two pass either way and are there to catch over-correction. Full file: 4/4 passing.

Scope

One file changed plus tests. No API surface change, no new dependencies, no behaviour change for callers that already pass instructions.

The Responses middleware always set `instructions` on the forwarded
request, even when the caller never passed one and there were no
memories to inject. `replaceMemoryContext("", "")` returns `""`, so
those requests went out with `instructions: ""`.

Sending the field at all tells the Responses API to drop the
instructions carried over from a `previous_response_id` turn, so an
empty string silently wipes the caller's system prompt mid-conversation
instead of leaving it in place.

The no-input early return in the same function already gets this right —
it spreads `instructions` only when the caller supplied a string. Apply
the same rule to the main path, while still injecting when there are
memories to add.

Adds three regression tests to the existing OpenAI middleware unit
suite, which CI already runs on changes to packages/tools.
@yesprasad

Copy link
Copy Markdown

🔎 TracePull from @yesprasad reviewed this PR

@MaheshtheDev @Dhravya

Review outcome

Decision Severity Confidence Scope
Change requested High Medium packages/tools

Executive summary

This PR introduces an instructionsOverride path intended to add enhanced instructions only when available. Its current boolean/ternary expression can send an empty instructions value when the caller has already supplied instructions.

Verified change flow

flowchart LR
  A[Caller supplies params.instructions] --> B[instructionsOverride]
  C[enhancedInstructions is empty] --> B
  B --> D[Empty instructions override]
  D --> E[Outbound Responses API request]
  E --> F[Instruction behavior changes]
Loading

Finding — empty instructions can override caller instructions

Changed code: packages/tools/src/openai/middleware.ts:977

const instructionsOverride =
  enhancedInstructions || typeof params.instructions === 'string'
    ? { instructions: enhancedInstructions }
    : {}

Impacted code path

params.instructions
  → instructionsOverride
  → outbound Responses API request
  → response / continuation instruction behavior

Failure condition

enhancedInstructions === ''
typeof params.instructions === 'string'

Because || is evaluated before ?:, the condition becomes true and the code constructs { instructions: '' } rather than omitting the override.

This conflicts with the intended behavior of leaving caller-provided instructions untouched when no enhanced replacement is available. It can replace a meaningful caller instruction with an empty value.

Suggested repair

const hasEnhancedInstructions =
  typeof enhancedInstructions === 'string' &&
  enhancedInstructions.trim().length > 0

const instructionsOverride = hasEnhancedInstructions
  ? { instructions: enhancedInstructions }
  : {}

Required regression coverage

Add a test where enhancedInstructions === '' and params.instructions === 'existing instructions'. Assert that the outbound request does not serialize { instructions: '' }.

Review evidence

Signal Result
Changed files reviewed 2
Changed source citation middleware.ts:977
Compiler-resolved direct consumers 3
Static analysis findings 0
Finding source Changed source + semantic review
Downstream API behavior Requires regression-test confirmation

TracePull is the merge Intelligence Engine for your Pull Requests to help you merge and deploy with confidence.

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