Parse multiple SDK user agents in builder check#3149
Parse multiple SDK user agents in builder check#3149matthewlouisbrockman wants to merge 2 commits into
Conversation
matthewlouisbrockman
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 98f5ae1. Bugbot is set up for automated code reviews on this repo. Configure here. |
matthewlouisbrockman
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Failing the entire build request with an error when the User-Agent header contains an invalid semver version is fragile because User-Agent headers are client-controlled. Instead of returning an error when parsing JS or Python SDK versions fails, log a warning and fall back to a safe default version.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if err != nil { | ||
| return "", fmt.Errorf("parsing JS SDK version: %w", err) | ||
| } |
There was a problem hiding this comment.
Failing the entire build request with a 400 Bad Request error when the User-Agent header contains an invalid semver version is extremely fragile. User-Agent headers are client-controlled and can easily be malformed, customized, or modified by proxies. Instead of returning an error, log a warning and fall back to a safe default version.
if err != nil {
logger.Debug(ctx, "failed to parse JS SDK version from user agent", zap.Error(err), zap.String("user_agent", userAgent))
return templates.TemplateV2BetaVersion, nil
}There was a problem hiding this comment.
existing behavior
| if err != nil { | ||
| return "", fmt.Errorf("parsing Python SDK version: %w", err) | ||
| } |
There was a problem hiding this comment.
Failing the entire build request with a 400 Bad Request error when the User-Agent header contains an invalid semver version is extremely fragile. User-Agent headers are client-controlled and can easily be malformed, customized, or modified by proxies. Instead of returning an error, log a warning and fall back to a safe default version.
if err != nil {
logger.Debug(ctx, "failed to parse Python SDK version from user agent", zap.Error(err), zap.String("user_agent", userAgent))
return templates.TemplateV2BetaVersion, nil
}There was a problem hiding this comment.
existing behavior
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM - small, well-tested refactor to handle compound user-agent strings.
Extended reasoning...
Overview
Refactors userAgentToTemplateVersion in packages/api/internal/handlers/template_start_build_v2.go to tokenize the user agent on whitespace and check each token against the JS/Python SDK prefixes, rather than only checking the start of the full string. Adds a new test file covering current SDK, mixed SDK+CLI, old SDK, unrecognized, and empty user-agent cases.
Security risks
None. The function is a read-only lookup that maps a user-agent string to a template version constant. strings.Fields is safe; no injection or auth surface is touched.
Level of scrutiny
Low. This is a small, localized parsing fix in a single helper function with clear intent and comprehensive table-driven tests. The early-return behavior preserves the first-match-wins semantics that the description calls out.
Other factors
The new test cases include a v-prefixed version (e2b-js-sdk/v2.31.0) which IsGTEVersion handles via sanitizeVersion. The fallback debug log now correctly fires when no token matched (previously a default switch arm). No reviewer comments are outstanding.
1 participant
template builder checks the sdk version to decide whtehr to use the beta builder or ga builder. crashes on multiple user agents so this splits it up.
makes a loop to check each of the user agents for compatibility with template v2; the first agent should be the right one, but if it isn't ordered right for some reason will still check the rest