fix(js-sdk): don't treat "detached" in a branch name as a detached HEAD#1518
fix(js-sdk): don't treat "detached" in a branch name as a detached HEAD#1518anxkhn wants to merge 1 commit into
Conversation
parseGitStatus flagged detached: true whenever the porcelain branch line
contained the substring "detached", via `rawBranch.includes('detached')`.
A normal status line like `## main...origin/detached-work` (or a local
branch such as `feature/detached-fix`) was therefore misclassified as a
detached HEAD, and currentBranch/upstream were dropped.
Real detached HEADs print `## HEAD (no branch)` or
`## HEAD (detached at <sha>)`, which are already handled by the
`startsWith('HEAD (detached at ')` clause and the
`normalizedBranch.startsWith('HEAD')` branch, so the substring check only
ever produced false positives. Drop it.
This is the JS twin of the Python SDK fix for the same defect (e2b-dev#1373).
|
We require contributors to sign our Contributor License Agreement, and we don't have @anxkhn 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' |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1b109c8c2
ℹ️ 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".
| const isDetached = | ||
| rawBranch.startsWith('HEAD (detached at ') || | ||
| rawBranch.includes('detached') | ||
| const isDetached = rawBranch.startsWith('HEAD (detached at ') |
There was a problem hiding this comment.
Apply the detached-status fix to the Python SDK too
Because this commit changes an SDK package, the root AGENTS.md requires equivalent JS and Python SDK changes. This line fixes only the JS parser, but packages/python-sdk/e2b/sandbox/_git/parse.py still has "detached" in raw_branch, and both sync and async Python git.status() paths call that parser, so Python users on a normal branch/upstream like main...origin/detached-work will still get detached=True with branch/upstream dropped while the changeset claims the defect is fixed.
Useful? React with 👍 / 👎.
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
parseGitStatusin the JS SDK misreports a normal repository as being in detached HEAD state whenever the current branch or its upstream merely contains the substringdetached.Given a branch
maintrackingorigin/detached-work,git status --porcelain=1 -bprints:The branch part is
main...origin/detached-work, so the old substring checkrawBranch.includes('detached')matched and the parser took the detached-HEAD path, returningdetached: trueand droppingcurrentBranch/upstreamentirely.Root cause
packages/js-sdk/src/sandbox/git/utils.ts:A real detached HEAD is reported by git as
## HEAD (no branch)or## HEAD (detached at <sha>), never as the bare worddetachedinside an ordinary branch name. Those two real forms are already handled:## HEAD (detached at <sha>)->rawBranch.startsWith('HEAD (detached at ').## HEAD (no branch)->normalizeBranchNamemaps it toHEAD, caught by the existingnormalizedBranch.startsWith('HEAD')branch.So the
includes('detached')clause can only ever produce false positives. This removes it:Behavior change (before / after)
git statusbranch line## main...origin/detached-workdetached: truedetached: false,currentBranch: 'main',upstream: 'origin/detached-work'## feature/detached-session-fixdetached: truedetached: false,currentBranch: 'feature/detached-session-fix'## HEAD (no branch)detached: truedetached: true(unchanged)## HEAD (detached at 1a2b3c4)detached: truedetached: true(unchanged)Usage example
Parity with the Python SDK
The repo guidelines ask that SDK changes be mirrored across JS and Python. This is the JS counterpart of the same defect tracked in #1373; the Python side is addressed in #1374 using the identical approach (drop the
"detached" in raw_branchsubstring check). This PR brings the JS SDK to parity for the detached-HEAD case.Tests
Adds
packages/js-sdk/tests/git.parseStatus.test.ts(offlineunitvitest project, no sandbox / API key) covering all four cases above: the two false positives are nowdetached: falsewithcurrentBranch/upstreampreserved, and both real detached-HEAD forms still reportdetached: true.A changeset (
e2b, patch) is included.Checklist
pnpm run format(changed files passprettier --check)pnpm run lint(oxlint, clean)pnpm run typecheck(clean)pnpm run test; new unit test 4/4)pnpm changeset;e2bpatch)