Conversation
GitAutoCommitHelperImpl#autoCommitApplication used
String.replaceFirst("origin/", "") to normalise a branch name before
calling findByBranchNameAndBaseApplicationId. That regex substitutes the
first occurrence of "origin/", so a legitimate ref name like
"feature/origin/main" silently becomes "feature/main" and the lookup
targets the wrong (or no) branched application.
CentralGitServiceCEImpl#stripOriginPrefix and the matching unit tests
already define the canonical contract for this normalisation: strip only a
leading "origin/" prefix and leave any mid-string occurrence untouched.
Align this call site with that contract so the autocommit publish flow
behaves consistently across the three git services and stops mis-routing
on legal ref names that contain "origin/" mid-string.
The branch-name string is already null-checked above, so the
.startsWith/ternary form needs no extra guard. No public API change; the
existing test suite (which passes plain branch names like "develop")
remains green.
github-actions
Bot
added
awaiting-maintainer
|
Thanks for contributing to Appsmith! Credential-free formatting, lint, type, and unit checks will run after GitHub's workflow approval. An Appsmith maintainer will start privileged integration tests or a deploy preview when needed. No action is required from you while this PR has the |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: appsmithorg/appsmith/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe auto-commit helper now removes ChangesBranch Name Normalization
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
A branch begins with origin’s sign, Comment |
Contributor
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
This branch has not been deployed
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.
{
"title": "fix(git): strip only leading origin/ prefix in autocommit branch name",
"body": "## Summary\n\n
GitAutoCommitHelperImpl#autoCommitApplicationnormalised a branch name with\n\njava\nfinal String finalBranchName = branchName.replaceFirst(\"origin/\", \"\");\n\n\nbefore callingfindByBranchNameAndBaseApplicationId.String.replaceFirst\nsubstitutes the first occurrence of\"origin/\", so a legitimate ref like\nfeature/origin/mainsilently becomesfeature/mainand the lookup targets\nthe wrong (or no) branched application during the autocommit publish flow.\n\nCentralGitServiceCEImpl#stripOriginPrefixand the matching unit tests\n(CommonGitServiceCEImplTest/CentralGitServiceCEImplTest) already define\nthe canonical contract for this normalisation: strip only a leading\n\"origin/\"prefix and leave any mid-string occurrence untouched.\n\nThis PR aligns the autocommit call site with that contract so:\n\n-origin/main→main(unchanged behaviour)\n-feature/origin/main→feature/origin/main(previously corrupted tofeature/main)\n-origin/feature/origin/main→feature/origin/main(previously corrupted tofeature/origin/main)\n-develop→develop(no-op, matching the existing test fixtures)\n\nThe branch-name string is already null-checked above the call, so the\nstartsWith/ternary form needs no extra guard.\n\n## Scope\n\nSingle-line behaviour change inGitAutoCommitHelperImplplus a comment\nreferencing the existing canonical helper. No public API change. The existing\ntest suite (which passes plain branch names likedevelop) remains green;\nadding a regression test for the mid-stringorigin/case is left for a\nfollow-up that wires a Mockito fixture aroundfindByBranchNameAndBaseApplicationId,\nwhich is currently spied only at the high-level flow boundary.\n\n## Diff\n\ndiff\n- final String finalBranchName = branchName.replaceFirst(\"origin/\", \"\");\n+ // Strip only a leading \"origin/\" prefix; replaceFirst would corrupt ref names\n+ // that legitimately contain \"origin/\" mid-string (see CentralGitServiceCEImpl#stripOriginPrefix).\n+ final String ORIGIN_PREFIX = \"origin/\";\n+ final String finalBranchName = branchName.startsWith(ORIGIN_PREFIX)\n+ ? branchName.substring(ORIGIN_PREFIX.length())\n+ : branchName;\n\n\n## Validation\n\n-git diff --checkclean\n- Compile-bound (Java/Spring); not built locally because the workspace has no\n Maven toolchain in this checkout. CI will validate."}
Summary by CodeRabbit
origin/prefix.origin/text appearing elsewhere in branch names.