fix(parse): guard against branch names containing "..." in parse_git_status#1372
fix(parse): guard against branch names containing "..." in parse_git_status#1372devteamaegis wants to merge 1 commit into
Conversation
…taining "..."
When a git branch name contains the literal substring "...", the call
normalized_branch.split("...") produces more than 2 parts and the
two-variable unpacking raises ValueError. Passing maxsplit=1 ensures
only the first "..." is used to split branch from upstream, regardless
of how many "..." appear in the branch name.
devteamaegis
requested review from
ValentaTomas,
jakubno and
mishushakov
as code owners
|
We require contributors to sign our Contributor License Agreement, and we don't have @devteamaegis on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9ff450b67
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| detached = True | ||
| elif "..." in normalized_branch: | ||
| branch, upstream_branch = normalized_branch.split("...") | ||
| branch, upstream_branch = normalized_branch.split("...", 1) |
There was a problem hiding this comment.
This changes the shared Python status parser, but the repo-level AGENTS.md says SDK package changes must have equivalent JS and sync/async Python updates. The Python sync/async clients both use this parser, while packages/js-sdk/src/sandbox/git/utils.ts still does normalizedBranch.split('...'), so for the new multi-ellipsis header case the JS SDK keeps returning a different upstream (v2) than Python (v2...origin/feat...v2). Please update the JS parser/test in the same change so the SDKs remain behaviorally consistent.
Useful? React with 👍 / 👎.
1 participant
What's broken
parse_git_statusinpackages/python-sdk/e2b/sandbox/_git/parse.pycrashes withValueError: too many values to unpack (expected 2)when git reports a branch whose name contains the substring.... For example, a branch namedfeat...v2trackingorigin/feat...v2produces porcelain output## feat...v2...origin/feat...v2. The callnormalized_branch.split("...")splits on every occurrence and yields more than two parts, causing the two-variable unpacking to fail immediately.Why it happens
str.split(sep)with nomaxsplitargument splits on every occurrence of the separator, so branch names that contain...produce more than two segments.Fix
Changed
normalized_branch.split("...")tonormalized_branch.split("...", 1)so only the first...is used to separate the local branch name from the upstream tracking ref, regardless of how many...appear in the branch name.Test
Added
tests/test_git_parse.pywith one test that feeds porcelain output for a...-containing branch intoparse_git_statusand asserts it returns without raising and produces the correctcurrent_branchandupstreamvalues.Fixes #1371