fix(utils): percent-encode branch names in provider branch URLs

[marmar9615-cloud]

HUMAN:

I hit this with a branch named feature/ui#123. The chip showed the correct name, but the link opened to feature/ui. I ran the dev server, created that branch in a scratch repo, and watched the actual request in DevTools stop at feature/ui with a 404. After the change, the link keeps the %23, and release/1.0 still works.

---

AGENT:

Why

constructBranchUrl interpolated the branch name into the provider URL without percent-encoding it. # and % are both legal in a git ref, and both change what the browser or the server resolves:

• feature/ui#123: # opens a URL fragment, which is not sent. The href reads .../tree/feature/ui#123 while the request path is .../tree/feature/ui.
• 100%2Fdone: the raw path decodes server-side to .../tree/100/done.

Blanket-encoding would be wrong in the other direction, since release/1.0 would become release%2F1.0 on the path-based providers. The encoding has to follow where each provider puts the branch.

Summary

• github, gitlab, bitbucket and forgejo put the branch in the URL path, so a new encodeBranchPath helper encodes each /-separated segment and leaves the separators alone.
• azure_devops (?version=GB) and bitbucket_data_center (?at=refs/heads/) put it in a query value, so the whole name is encoded, / included. An unencoded & there would otherwise start a new query parameter.
• 42 new constructBranchUrl cases across all six providers, covering main, feature/ui#123, feature&x, feature+x, 100%done, 100%2Fdone and release/1.0.

Only the href changes. The displayed branch text is untouched.

Issue Number

Fixes #17106

How to Test

npm ci && npx vitest run __tests__/utils/utils.test.ts gives 52 passing: the 42 new constructBranchUrl cases plus 10 pre-existing getStatusText tests in the same file.

The tests fail without the fix, and they also fail against both plausible wrong fixes, which is the point of splitting path from query. I checked each by swapping the implementation and re-running:

implementation	result
main (unencoded)	32 failed, 20 passed
blanket encodeURIComponent on the four path providers	8 failed, release/1.0 becomes release%2F1.0 on every path provider
encodeBranchPath on the two query providers	4 failed, ?version=GBrelease/1.0 leaves the slash raw
this branch	52 passed

End to end, against the running app rather than the unit tests:

1. npm ci && npm run dev (needs uv on PATH per README option 3).

2. Create the scratch repository:

   mkdir -p /tmp/branch-escape && cd /tmp/branch-escape
   git init -q .
   git remote add origin https://github.com/OpenHands/OpenHands.git
   git checkout -q -b 'feature/ui#123'
   git commit -q --allow-empty -m init
   git rev-parse --abbrev-ref HEAD   # must print feature/ui#123, not HEAD

   The commit is required. On an unborn branch that last command prints HEAD and exits 128, use-local-git-info.ts:64 maps HEAD to null, and git-control-bar.tsx:238 then renders no branch chip at all.

3. Open a conversation on that workspace and follow the branch chip's link.

Also ran on the full tree: __tests__/utils is 69 files / 549 tests passing, the four branch-link component suites are 30 tests passing, and tsc, eslint and prettier --check are clean on both changed files.

Video/Screenshots

Before. The control bar shows feature/ui#123, but DevTools records the document request as .../tree/feature/ui with a 404:

After. The whole branch name now reaches GitHub as feature/ui%23123:

Both destination pages are errors because this scratch branch does not exist on the public remote. The difference the screenshots establish is the requested path: feature/ui before, feature/ui%23123 after. On a repository where the branch exists, the first would resolve to a different branch and the second to the right one.

Regression control. With the branch switched to release/1.0, the link keeps its unencoded slash:

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

Unrelated to this change, the JSDoc example above constructBranchUrl shows a bitbucket call returning a /projects/.../browse?at= URL, which is the bitbucket_data_center shape. I left it alone rather than widen this PR.

[github-actions]

❌ Mock-LLM E2E Tests

69/70 passed · 1 failed

Commit: 80c7bce8 · Workflow run · Test artifacts

Details

Status	Test	Duration
✅	automations/mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 1: setup LLM profile and register automation trajectory	7.7s
✅	automations/mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 2: create automation and dispatch run via the UI	30.6s
✅	automations/mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page	5.9s
✅	automations/mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › automation card sends the correct slash command to a conversation	15.6s
✅	automations/mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › direct slash command from home page triggers skill activation	13.6s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: fresh install with runtime-injected key › reaches the onboarding modal without pre-seeded localStorage	1.4s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: non-public key rotation › recovers when localStorage has a stale session API key	5.4s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows first-run onboarding before the auth screen when no key is configured	1.3s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error	1.5s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › allows access after pasting the correct key	1.5s
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › skips auth screen for returning user with valid stored key	914ms
✅	backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › re-prompts when the server rotates its key (stale localStorage)	1.3s
✅	backends/mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → backend-only › frontend-only connects to a separate backend-only instance	17.2s
✅	backends/mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → multiple backends › connects to two separate backends and switches between them	21.7s
✅	backends/mock-llm-cross-connect.spec.ts › cross-connect: sidebar links pin their backend › cmd-clicking a sidebar conversation opens it on the owning backend	31.6s
✅	backends/mock-llm-partial-stack.spec.ts › partial stack: --frontend-only › serves the frontend but returns 503 for backend routes	7.3s
✅	backends/mock-llm-partial-stack.spec.ts › partial stack: --backend-only › serves backend APIs but returns 503 for the frontend root	15.1s
✅	backends/mock-llm-partial-stack.spec.ts › partial stack: port conflict › fails with a clear error when the ingress port is occupied	103ms
✅	backends/mock-llm-partial-stack.spec.ts › partial stack: port conflict › starts successfully on a free port after a conflict	6.0s
✅	canvas-extensions/mock-llm-canvas-extensions.spec.ts › Canvas Extensions lifecycle › step 1: install from a source path lands disabled with no sidebar item	5.7s
✅	canvas-extensions/mock-llm-canvas-extensions.spec.ts › Canvas Extensions lifecycle › step 2: enabling behind the trust confirmation registers the sidebar item and renders the page	6.4s
✅	canvas-extensions/mock-llm-canvas-extensions.spec.ts › Canvas Extensions lifecycle › step 3: disabling tears down the sidebar item and the page	6.1s
✅	canvas-extensions/mock-llm-canvas-extensions.spec.ts › Canvas Extensions lifecycle › step 4: uninstalling empties the inventory	5.7s
✅	conversations/mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM server	6.1s
✅	conversations/mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 2: activate the mock-llm profile and verify settings API	6.2s
✅	conversations/mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 3: run a conversation with the mock LLM	7.4s
✅	conversations/mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 4: resume conversation from sidebar after navigating away	5.9s
✅	conversations/mock-llm-image-upload.spec.ts › mock-LLM image upload › attaching an image embeds it as base64 in the LLM completion call	13.8s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 1: ensure mock LLM profile is configured	6.3s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 2: start conversation and attach workspace metadata	12.4s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 3: conversation overview shows workspace and git identity	26.1s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 4: commits tab opens for attached workspace	5.9s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 5: browser tab shows empty state	6.3s
✅	files/mock-llm-files-and-git.spec.ts › files tab, conversation overview git, and browser tab › step 6: files tab defaults to file-tree view without attached workspace	7.9s
✅	home/mock-llm-folder-workspace.spec.ts › mock-LLM folder browser → workspace → conversation › step 1: browse to a folder, add it as a workspace, and launch a conversation with the correct working_dir	8.5s
✅	mcp/mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 1: GitHub card is visible on the MCP marketplace page	5.5s
✅	mcp/mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 2: clicking GitHub add control opens the install modal with correct fields	5.7s
✅	mcp/mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 3: full install flow — fill PAT, submit, verify installed	11.7s
✅	mcp/mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 4: installed GitHub server can be deleted	5.8s
✅	mcp/mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › regression: sibling create, edit, and delete preserve GitHub credentials with one MCP request each	12.5s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › install: invalid Slack credentials are blocked with a credential-check error	5.8s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › install: a valid token missing only a scope still installs (missing_scope is not a credential failure)	5.8s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › install: an older agent server that omits tool_result still installs (compat)	5.8s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › edit: Test Connection verifies the stored credentials and surfaces a credential failure	5.7s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › edit: Test Connection reports success for valid stored credentials	5.7s
✅	mcp/mock-llm-mcp-slack-credentials.spec.ts › MCP Test Connection credential verification (Slack) › custom (non-catalog) server: Test Connection attaches no verification probe	5.8s
✅	onboarding/mock-llm-onboarding-happy-path.spec.ts › onboarding happy path › completes the full onboarding flow and launches a conversation	4.9s
✅	onboarding/mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › keeps the modal open on backdrop click and Escape	1.5s
✅	onboarding/mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › defaults the LLM setup step to OpenAI GPT-5.6 Sol	1.6s
✅	regressions/mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell	1.2s
✅	regressions/mock-llm-ui-regressions.spec.ts › UI regressions › renders critic results on agent messages and finish actions	1.4s
✅	regressions/mock-llm-ui-regressions.spec.ts › UI regressions › loads older events when scrolling up	1.9s
✅	regressions/mock-llm-ui-regressions.spec.ts › UI regressions › selected workspace persists after navigating away and returning	2.3s
✅	regressions/mock-llm-ui-regressions.spec.ts › UI regressions › cleared sessionStorage yields empty workspace selection	1.2s
✅	settings/mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 1: configure ACP agent via Settings → Agent UI	13.0s
✅	settings/mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 2: reload and verify ACP settings are persisted in UI	5.7s
✅	settings/mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 3: start ACP conversation and verify agent reply	6.3s
✅	settings/mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 4: resume ACP conversation from sidebar after navigating away	5.9s
✅	settings/mock-llm-acp-auth-banner.spec.ts › mock-LLM ACP credentials-configured banner (#1244) › Claude credential in the store surfaces a 'configured' banner, never 'signed in'	6.7s
✅	settings/mock-llm-cloud-providers-pagination.spec.ts › cloud LLM provider-picker pagination › surfaces providers past page 1 (e.g. xai) in the picker on a cloud backend	6.1s
✅	settings/mock-llm-cloud-providers-pagination.spec.ts › cloud LLM provider-picker pagination › does not regress the local path: 150+ providers render in the picker	5.7s
✅	settings/mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 1: configure LLM, create switch-target profile, register trajectory	6.9s
✅	settings/mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 2: start conversation, switch profile via /model, verify switch	7.7s
✅	settings/mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile	9.5s
✅	settings/mock-llm-profile-management.spec.ts › same-model profile identity › chat header shows the correct profile when two profiles share the same model	15.3s
❌	settings/mock-llm-profile-management.spec.ts › OpenHands provider hidden base_url preservation › re-saving an OpenHands profile from Basic view preserves hidden base_url	38.8s
✅	settings/mock-llm-provider-connection-selector.spec.ts › provider connection selector › selects a supported provider in Add provider	8.2s
✅	skills/mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keyword	13.8s
✅	skills/mock-llm-skills.spec.ts › skill loading: project, user, and deletion › user skill in ~/.openhands/skills/ triggers on matching keyword	8.5s
✅	skills/mock-llm-skills.spec.ts › skill loading: project, user, and deletion › deleting a user skill removes it from subsequent conversations	8.5s

🔍 Failure details (1)

❌ settings/mock-llm-profile-management.spec.ts › OpenHands provider hidden base_url preservation › re-saving an OpenHands profile from Basic view preserves hidden base_url

Error: expect(locator).toBeVisible() failed

Locator: getByTestId('add-llm-profile')
Expected: visible
Timeout: 30000ms
Error: element(s) not found

Call log:
  - Expect "toBeVisible" with timeout 30000ms
  - waiting for getByTestId('add-llm-profile')

_{Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)}

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Verdict: no material findings. The change is small, well-scoped, and correct.

What I verified against the workspace code:

• Ordering guarantee holds. BaseConversation.compose_callbacks iterates callbacks in a plain for loop with no try/except, so composed_list = [_default_callback] + callback_list means a ValueError raised by EventLog.append (duplicate ID / missing parent) short-circuits the loop and the caller-supplied publish callback never runs. That directly delivers the "never announce an event that isn't durable" invariant.
• Return-type change is safe. I searched the codebase: no production caller consumes append_event's previous Event return value, and every events.append(...) call site ignores the return. EventLog.append now captures self._length into idx once and returns it, which is the correct assigned sequence number (O(1), no extra work).
• RemoteEventsList.append's placeholder return is appropriately documented. It returns len(self._cached_events) - 1 only to satisfy the shared EventsListBase interface; the PR's follow-up (step 3, session socket) targets the local EventLog path, so the non-meaningful remote value isn't consumed anywhere.
• Tests are meaningful and exercise real code paths (no mocks): test_user_callback_runs_after_event_is_persisted and test_persist_failure_blocks_downstream_callback both assert observable behavior that regresses if the order is reverted. I ran the two touched test files locally: 34 passed.

Scope check: this is event persistence/ordering plumbing, not agent decision logic, tool behavior, prompt templates, or condenser/planning code, so it is not in the eval-risk category.

Risk assessment: 🟢 LOW — backward-compatible return-type widening on a method whose prior return was unused; the only behavior change is the callback invocation order, which is the explicit point of the fix.

Review decision: COMMENT (no blocking issues). This is a clean, self-contained fix ready for a human maintainer to merge.

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Verdict: no material findings. The change is small, well-scoped, and correct.

This PR fixes #17106 by percent-encoding branch names when building provider branch URLs, and it correctly splits the encoding strategy by where each provider places the branch:

• Path providers (github, forgejo, gitlab, bitbucket) use the new encodeBranchPath helper, which encodes each /-separated segment while preserving the slashes. So feature/ui#123 → feature/ui%23123, but release/1.0 keeps its literal slash (nested refs are expressed as nested path segments). This is the right behavior.
• Query providers (bitbucket_data_center ?at=refs/heads/…, azure_devops ?version=GB…) use encodeURIComponent on the whole value, encoding / too, so an unencoded &, +, or # can't be misinterpreted as a query delimiter. Correct for a single query value.

I traced the callers (git-control-bar-branch-button.tsx, conversation-overview-git-section.tsx): constructBranchUrl is used only to build the href, so the displayed branch text is untouched and there's no UI regression.

Tests are real-logic and proportionate: 52 cases across all six providers plus a release/1.0 regression control, and the PR documents that the suite also fails against both plausible wrong fixes (blanket encodeURIComponent on path providers, and reusing encodeBranchPath on query providers). No mocks.

Scope check: this is pure URL-string construction, not agent decision logic, tool selection, prompts, or evaluation paths, so it's outside the eval-risk category.

Risk assessment: 🟢 LOW — a small, pure, well-tested change to link generation. No dependency, security, or contract changes. The only theoretical edge is encodeURIComponent throwing on a lone-surrogate branch name, which is not realistically reachable from a git ref and predates this change in spirit (unencoded interpolation already produced invalid URLs).

Automated review used the wrong decision (APPROVED instead of COMMENT) and is dismissed. Findings are reposted as a comment.

[all-hands-bot]

The correct review is posted. Note: an initial APPROVED review with unrelated stale content (from a shared temp file that leaked in) was also posted in error, but I cannot delete already-submitted reviews — only PENDING reviews can be deleted. The authoritative review for this PR is the second one (COMMENTED), which contains the correct branch-URL-encoding analysis.

The correct review content is now live on the PR.

GITHUB_REVIEW_POSTED

This comment was posted by an AI agent (OpenHands).

[VascoSch92]

LGTM

thanks

[all-hands-bot]

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.

[openhands-release-bot]

🚀 Released in v1.17.0.