Skip to content

fix(translate): find the first user message past leading system/developer turns - #1078

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787836016-openai-first-user-message
Open

fix(translate): find the first user message past leading system/developer turns#1078
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787836016-openai-first-user-message

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Rewrite of #1073 by @Adityakk9031 to match repo conventions; original credited via Co-authored-by.

FirstUserMessageText only looked at messages.0, so any OpenAI-format body — where system/developer lives in messages[] — returned "":

-first := gjson.GetBytes(e.body, "messages.0")
-if first.Get("role").String() != "user" { return "" }
+// scan messages[] for the first role=="user" entry (ForEach, as openAISystemText does)

Consequence in DeriveSessionKey: with no first-user text the discriminator fell back to SystemText(), so two unrelated Codex/OpenAI conversations sharing one system prompt under one API key hashed to the same session key and thrashed a single pin slot. geminiFirstUserMessageText already scanned for the first user turn, so this also removes a per-format inconsistency.

Also updates the now-stale DeriveSessionKey comments, which documented the old "first user message is empty on OpenAI bodies" fallback rationale — #1073 left them describing behavior that no longer exists.

Note: OpenAI-format session keys change value, so existing pins for those sessions are invalidated once on deploy.

Dropped from the original: an unrelated hunk in internal/proxy/conformance_golden_test.go that CRLF-normalized both sides of the golden comparison. No golden fixture in the repo contains CRLF; that is a Windows-checkout (core.autocrlf) workaround which permanently weakens the assertion.

Validation

  • make precommit green; gofmt -l ./internal ./cmd clean
  • go test ./internal/translate/... ./internal/proxy/... ./internal/router/turntype/...
  • Confirmed the new TestDeriveSessionKey_OpenAISharedSystemDifferentUserPromptDoesNotCollide fails on main (identical digests) and passes here

Link to Devin session: https://app.devin.ai/sessions/ed6a7cbbb8c4489d8c056590b3cc174b
Open in Devin Desktop: https://app.devin.ai/desktop/session/ed6a7cbbb8c4489d8c056590b3cc174b?variant=devin
Requested by: @steventohme

Co-authored-by: Adityakk9031 <Adityakk9031@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

PR author is not in the allowed authors list.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown

Claude finished @devin-ai-integration[bot]'s task —— View job


No comment-length issues found — no added comment block is 3 or more consecutive lines.

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.

1 participant