{{ message }}
fix: use shared git helpers for code-mappings repo inference#1087
Merged
Conversation
Replace buggy custom git helpers in code-mappings upload with the well-tested parseRemoteUrl from src/lib/git.ts. Fixes: - SSH_REMOTE_RE incorrectly matched HTTPS URLs (e.g., the colon in https://github.com/owner/repo matched the SCP-style regex, producing garbage repo names like '//github.com/owner/repo') - execSync with string interpolation replaced with execFileSync via the shared git() helper (no shell injection risk) - inferRepo/inferDefaultBranch consolidated into git.ts as inferRepositoryName/inferDefaultBranch with upstream→origin fallback Also removes ASCII art section dividers from code-mappings and dart-symbol-map files per AGENTS.md prohibited comment styles.
Contributor
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Comment on lines
+272
to
+274
| return "main"; | ||
| } | ||
| } |
There was a problem hiding this comment.
inferDefaultBranch truncates branch names containing '/'
Splitting the ref by / and taking .at(-1) discards all but the last path segment, so a default branch like release/2.0 (output: refs/remotes/origin/release/2.0) would be returned as "2.0" instead of "release/2.0". Use output.slice(\refs/remotes/${remote}/`.length) || "main"` instead.
Evidence
git symbolic-ref refs/remotes/origin/HEADemitsrefs/remotes/origin/release/2.0for a default branch namedrelease/2.0.- In
inferDefaultBranch,output.split("/")produces["refs", "remotes", "origin", "release", "2.0"]and.at(-1)returns only"2.0", dropping therelease/prefix. - The
?? "main"guard is ineffective:split("/")always returns a non-empty array, so.at(-1)is neverundefined; an empty output yields""(falsy, not nullish) and still bypasses the fallback. - The truncated branch flows through
inferDefaultBranch(repoInfo?.remote ?? "origin", this.cwd)intouploadCodeMappings({ ... })insrc/commands/code-mappings/upload.ts.
Identified by Warden find-bugs · VXV-VGS
Contributor
Codecov Results 📊❌ Patch coverage is 0.00%. Project has 5019 uncovered lines. Files with missing lines (2)Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.13% 81.17% +0.04%
==========================================
Files 383 383 —
Lines 26671 26659 -12
Branches 17340 17336 -4
==========================================
+ Hits 21640 21640 —
- Misses 5031 5019 -12
- Partials 1797 1796 -1Generated by Codecov Action |
BYK
added a commit
that referenced
this pull request
Jun 9, 2026
## Summary Follow-up to #1087 — fixes a warden bot finding where `inferDefaultBranch()` truncated branch names containing slashes. ## Bug `inferDefaultBranch()` used `output.split('/').at(-1)` to extract the branch name from a git symbolic-ref output like `refs/remotes/origin/main`. This broke for branches with slashes: | Ref output | Expected | Got | |-----------|----------|-----| | `refs/remotes/origin/main` | `main` | `main` | | `refs/remotes/origin/release/2.0` | `release/2.0` | `2.0` | ## Fix Use prefix-based slicing: strip the known `refs/remotes/{remote}/` prefix, preserving the full branch name regardless of slashes.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up fix for #1086 — replaces buggy custom git helpers in
code-mappings uploadwith the well-tested shared helpers fromsrc/lib/git.ts.What was broken
The
SSH_REMOTE_REregex (:(.+?)(?:\.git)?$) inupload.tsincorrectly matched HTTPS URLs because they contain:(inhttps:). This produced garbage repo names:https://github.com/owner/repo.gitowner/repo//github.com/owner/repossh://git@github.com:22/owner/repo.gitowner/repo//git@github.com:22/owner/repoThe corrupted name was sent to the Sentry API as the
repositoryfield.What this fixes
Buggy regex replaced — Uses
parseRemoteUrl()fromsrc/lib/git.tswhich correctly triesnew URL()first (handles https/ssh/git protocols) and only falls back to SCP-style regex when URL parsing fails.Shell injection risk removed — Replaced
execSync(shell) withexecFileSync(no shell) via the sharedgit()helper.Code consolidated — New
inferRepositoryName()andinferDefaultBranch()functions ingit.tsreplace the duplicated logic inupload.ts. Both usethis.cwdfor correct working directory.ASCII art dividers removed — Per AGENTS.md prohibited comment styles, replaced
// ── Section ───dividers with plain// Sectionin code-mappings and dart-symbol-map files.Files changed
src/lib/git.tsinferRepositoryName()andinferDefaultBranch()src/commands/code-mappings/upload.tssrc/lib/api/code-mappings.tssrc/commands/dart-symbol-map/upload.tssrc/lib/api/dart-symbols.tsNet: 66 insertions, 96 deletions (less code, more correct)