fix(github): tell a branch holding slashes from a path below it by vinokurig · Pull Request #1089 · eclipse-che/che-server

vinokurig · 2026-10-21T11:57:00Z

What does this PR do?

A tree or blob URL does not say where the ref ends: tree/branch/with/slash and tree/main/sub/dir have the same shape. The parser kept the whole tail as the branch, so a URL pointing into a directory produced a branch that does not exist, which a clone then fails on.

Git settles it, because a ref cannot be a path prefix of another ref: refs/heads/branch is a file where refs/heads/branch/with/slash needs a directory. At most one prefix of the tail is a real ref, so asking the API about the prefixes in turn has exactly one answer. Probe shortest first, so the common URL, a short branch followed by a path, answers on the first question, and reuse the commit it returns as the latest commit the parse ends with: tree/main/sub costs the one API call it cost before.

Only a prefix the API reports absent is skipped. An unreachable server, a rejected token or a rate limit means the refs cannot be enumerated at all rather than that this one is missing, so the tail is kept whole, as before. A URL that resolves today keeps resolving when the API cannot be reached.

The subfolder needs its own field rather than staying folded into the branch: rawFileLocation substitutes the latest commit for the branch once it knows it, and a subfolder carried in the branch would be dropped along with it.

Two bugs found on the way: the devfile name was stripped at its first occurrence rather than its last, mangling a directory named after the devfile, and getLatestCommit indexed into the commit array unguarded, so a branch with no commit raised an ArrayIndexOutOfBoundsException past every declared handler.

The parsing rows no longer reach github.com from a unit test, which also drops the suite from 5.4s to 0.9s.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://redhat.atlassian.net/browse/CRW-13626

How to test this PR?

Apply the pull request image: quay.io/eclipse/che-server:pr-1089.
Start a workspace from the test url: https://github.com/vinokurig/test/tree/new/branch/path.
See: Workspace starts from the path subdirectory and the cloned project is checked out to the /tree/new branch.

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Summary by CodeRabbit

New Features
- Improved GitHub URL handling for branches containing slashes and repositories with nested subfolders.
- Generated raw file links now correctly include detected or configured subfolders for branches and commits.
- Devfile filenames are excluded from detected repository paths.
Bug Fixes
- GitHub URLs remain usable when branch detection cannot contact the API or no matching reference is found.
- Empty commit responses no longer cause an error when resolving the latest commit.

coderabbitai · 2026-10-21T11:57:23Z

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: cf709a58-dd4f-4572-a709-64bb48147100

📥 Commits

Reviewing files that changed from the base of the PR and between ade2db7 and 8fa356c.

📒 Files selected for processing: 5

wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java
wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/GithubApiClient.java
wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java
wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java
wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubUrlTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

The parser now identifies branch and subfolder paths in GitHub tree and blob URLs. It limits branch-prefix probes, handles configured devfile filenames, and reuses successful probe results. GithubUrl stores the subfolder and includes it in raw file locations.

Changes: GitHub URL parsing

Layer / File(s)	Summary
Subfolder storage and raw file locations `wsmaster/che-core-api-factory-github-common/.../GithubUrl.java`, `wsmaster/che-core-api-factory-github/src/test/.../GithubUrlTest.java`	`GithubUrl` stores an optional subfolder and includes it in raw file locations for branch and commit references. Tests cover both reference types.
Branch and subfolder resolution `wsmaster/che-core-api-factory-github-common/.../AbstractGithubURLParser.java`, `wsmaster/che-core-api-factory-github-common/.../GithubApiClient.java`, `wsmaster/che-core-api-factory-github/src/test/.../GithubURLParserTest.java`	The parser probes up to eight branch prefixes for tree and blob URLs, removes a trailing configured devfile filename, and preserves the full tail when resolution cannot identify a ref. It reuses a successful probe’s commit unless an explicit revision is supplied. `GithubApiClient` returns `null` for an empty commit response. Tests cover parsing, fallback, and probe limits.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AbstractGithubURLParser
  participant GithubApiClient
  participant GitHubAPI
  AbstractGithubURLParser->>GithubApiClient: Request commit for a branch prefix
  GithubApiClient->>GitHubAPI: Look up the ref
  GitHubAPI-->>GithubApiClient: Return commit or missing-ref response
  GithubApiClient-->>AbstractGithubURLParser: Return commit result
  AbstractGithubURLParser->>AbstractGithubURLParser: Split branch from subfolder

Merge Risk: Minimal · up to 8fa35

The identified parsing concerns do not block the GitHub URL behavior; the change is ready to merge after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: distinguishing slash-containing branch names from paths below them in GitHub URLs.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

A `tree` or `blob` URL does not say where the ref ends: `tree/branch/with/slash` and `tree/main/sub/dir` have the same shape. The parser kept the whole tail as the branch, so a URL pointing into a directory produced a branch that does not exist, which a clone then fails on. Git settles it, because a ref cannot be a path prefix of another ref: `refs/heads/branch` is a file where `refs/heads/branch/with/slash` needs a directory. At most one prefix of the tail is a real ref, so asking the API about the prefixes in turn has exactly one answer. Probe shortest first, so the common URL, a short branch followed by a path, answers on the first question, and reuse the commit it returns as the latest commit the parse ends with: `tree/main/sub` costs the one API call it cost before. Only a prefix the API reports absent is skipped. An unreachable server, a rejected token or a rate limit means the refs cannot be enumerated at all rather than that this one is missing, so the tail is kept whole, as before. A URL that resolves today keeps resolving when the API cannot be reached. The subfolder needs its own field rather than staying folded into the branch: rawFileLocation substitutes the latest commit for the branch once it knows it, and a subfolder carried in the branch would be dropped along with it. Two bugs found on the way: the devfile name was stripped at its first occurrence rather than its last, mangling a directory named after the devfile, and getLatestCommit indexed into the commit array unguarded, so a branch with no commit raised an ArrayIndexOutOfBoundsException past every declared handler. The parsing rows no longer reach github.com from a unit test, which also drops the suite from 5.4s to 0.9s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-10-21T12:16:24Z

Docker image build succeeded: quay.io/eclipse/che-server:pr-1089

kubectl patch command

kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1089", name: che}]}}]"

vinokurig · 2026-10-21T13:25:25Z

/retest

vinokurig · 2026-10-21T14:22:17Z

/retest

openshift-ci · 2026-10-27T14:02:20Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tolusha, vinokurig

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vinokurig requested review from SDawley, ibuziuk and tolusha as code owners October 21, 2026 11:57

vinokurig force-pushed the CRW-13626 branch from 62c7965 to 8fa356c Compare October 21, 2026 11:58

tolusha approved these changes Oct 27, 2026

View reviewed changes

openshift-ci Bot assigned tolusha Oct 27, 2026

openshift-ci Bot added the lgtm label Oct 27, 2026

Navigation Menu

Repository navigation

fix(github): tell a branch holding slashes from a path below it - #1089

fix(github): tell a branch holding slashes from a path below it#1089
vinokurig wants to merge 1 commit into
mainfrom
CRW-13626

vinokurig commented Oct 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

vinokurig commented Oct 21, 2026

Uh oh!

vinokurig commented Oct 21, 2026

Uh oh!

openshift-ci Bot commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vinokurig commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Release Notes

Reviewers

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

vinokurig commented Oct 21, 2026

Uh oh!

vinokurig commented Oct 21, 2026

Uh oh!

openshift-ci Bot commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vinokurig commented Oct 21, 2026 •

edited

Loading

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading