[ak684]

HUMAN:

• A human has tested these changes.

AGENT:

---

Why

Only the selected repository has git credentials in a conversation: its remote URL embeds the user's provider token. Any other private repository on the same provider cannot be cloned with plain git, from the agent's terminal or from a repository hook such as a .openhands/hooks.json SessionStart hook that clones the repositories a task needs. Azure DevOps already works differently: a global git credential helper exchanges a per-conversation JWT for the user's current token on every git call. This gives every other provider the same mechanism.

Summary

• At conversation start, mint one credential-helper exchange JWT per provider the user has a token for (GitHub, GitLab, Bitbucket Cloud, Bitbucket Data Center, Forgejo), write each to a mode-600 file under $HOME/.openhands/git-credentials/, and register them as masked conversation secrets like the Azure DevOps one.
• Install one shared git credential helper and configure it in the global git config for each provider's host (the token's host, or the default domain) with the git username that provider pairs with a token. On every git call it trades the JWT at GET /api/v1/webhooks/secrets, which already serves any provider type, for the user's current token, so other repositories also get refreshed tokens. A Bitbucket token stored as user:secret supplies its own username; a bare token or the x-token-auth:<token> form keeps the username configured for the host (for Bitbucket Data Center, the one its service resolved for that server). The selected repository keeps its authenticated remote URL, which git prefers over the helper.
• ProviderHandler.get_git_host factors the host resolution out of get_authenticated_git_url, so the helper is configured for the same host the clone URL uses. The Azure DevOps path is unchanged.

Issue Number

FDE-243

How to Test

Unit: uv run pytest tests/unit/app_server tests/unit/integrations (3419 passed). The new tests cover per-provider minting, secret registration, helper installation per host (including the Bitbucket Data Center username lookup and a refused plain-http host), and execute the produced setup command and the installed helper for real against a stand-in exchange endpoint, including the user:secret and x-token-auth:<token> forms. Pre-commit (ruff, format, mypy) passes.

Live, on R02 (self-hosted GitLab, baseline enterprise sha-40d51ea): a repository whose SessionStart hook clones five private projects the user owns. Baseline: the hook ran (exit 0) but every clone failed with exit 128 and no credential helper was configured. With this change (sha-1393ab8): the hook ran, all five clones are at their expected HEADs, the agent read a file from each, the sandbox has credential.https://<gitlab host>.helper in its global git config, and the clones' remotes are plain URLs without a token. Evidence: https://github.com/ak684/oh/tree/8794f0d0213b074d50a76c5e53c828eac1187535/runs/hook-clones-r02-gitlab-20261028. The same check on Azure DevOps (unchanged path) passes on the current release. Forgejo and Bitbucket Cloud have no live fixture; their usernames follow the existing clone-URL conventions.

Video/Screenshots

Screenshot of the candidate run (hook ok, five clones read) in the evidence link above: candidate-hook-clones-finished.png.

Type

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

Notes

No migration or configuration change. The helper is installed only when a provider token exists for the user and web_url is configured; without them the sandbox is untouched. The exchange JWTs live 12 hours like the Azure DevOps one and are scoped to the user and provider.

---

Enterprise server image for this PR:

ghcr.io/openhands/enterprise-server:sha-6204acd

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 5a0a808. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 5a0a808
REVIEW_REQUEST: 4f86ecdd274044468a15cef30d181f87
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

Uh oh! There was an unexpected error starting the job :(

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
openhands/app_server/app_conversation
app_conversation_models.py
app_conversation_service_base.py					1158-1159, 1191, 1208-1209, 1218, 1236
live_status_app_conversation_service.py					1483
openhands/app_server/integrations
provider.py
openhands/app_server/sandbox
sandbox_service.py
Project Total

This report was generated by python-coverage-comment-action

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 1393ab8. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 1393ab8
REVIEW_REQUEST: ae47a46022e14214a7ca33d8d4551673
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

Uh oh! There was an unexpected error starting the job :(

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 1393ab8. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 1393ab8
REVIEW_REQUEST: 12d79854fa49454aaa66defeac0eacd8
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

Uh oh! There was an unexpected error starting the job :(

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 1393ab8. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 1393ab8
REVIEW_REQUEST: b1eccec2fc7a4c9d95ed42ec0bcacf38
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

Uh oh! There was an unexpected error starting the job :(

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 1393ab8. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 1393ab8
REVIEW_REQUEST: 59122580d71b43efbeae58effe75e0c1
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

I'm on it! ak684 can track my progress at all-hands.dev

[ak684]

Code review — 1393ab8

Taste: good. The change reuses the existing Azure DevOps mechanism instead of inventing a second one, adds no new config surface, and keeps the new behaviour strictly additive.

Material findings

None. I could not produce a failure that a user, caller, or acceptance criterion would hit on this head.

What I verified on this head

• Target: head 1393ab8bed571356b29e970b9afa8c92fd4e8f7b (PR is a draft, base 3b2db47). Reviewed only this commit.
• Tests: uv run pytest tests/unit/app_server tests/unit/integrations → 3419 passed (matches the PR claim). Ran with a real PostgreSQL container.
• Lint/type: pre-commit ... --files <changed files> → ruff, ruff-format, mypy all pass (matches the claim). CI on 1393ab8 is green, incl. all four Python shards and Lint python.
• get_git_host refactor: the extracted block is logically identical to the inline code it replaced (same domain resolution, same ADO exception, same protocol detection before normalisation, same /api/… strip). Only callers are get_authenticated_git_url and the new helper, and ProviderHandler has no subclasses overriding PROVIDER_DOMAINS. ADO's clone-URL path is untouched apart from a docstring mention.
• Helper semantics, run for real: git invokes !"<helper>" <provider> → <helper> <provider> get; $helper is expanded at config-write time, so git stores the absolute path. A bare token becomes password= and git supplies credential.<host>.username; a user:secret token supplies its own username; colons inside the password survive; store/erase and non-get short-circuit. Confirmed with git credential fill against a stand-in helper.
• Base URL: the helper derives base_url from OH_WEBHOOKS_0_BASE_URL (which the sandboxes set to …/api/v1/webhooks) or from web_url, matching the ADO helper.
• Secrets handling: the JWT is a reference (user_id + provider_type, 12 h), not the provider token. Files are 600, dir/helper 700, the token is never logged (only provider@host), every interpolation is shlex-quoted, and the JWT is registered as a masked secret exactly like OH_AZURE_DEVOPS_SECRETS_ACCESS_TOKEN.
• Live evidence matches the description. baseline-sandbox-probe.json (sha-40d51ea): credential_helpers empty, 5/5 clones exit 128. candidate-* (sha-1393ab8, i.e. this head): 5/5 clones exit 0 at the expected HEADs, reads succeed, remotes are plain URLs, and credential.https://gitlab.platform-team.all-hands.dev.helper is present. The screenshot shows Hook: SessionStart ok. The evidence directory's hashes are listed in manifest.json.

Non-blocking notes

1. PR template: the ## Type section is missing (this repo's triage gate). The PR is already a draft, which the guide treats as the preferred state, so nothing to fix mechanically — but add it before marking ready.
2. Sandbox reuse: under GROUP_BY_NEWEST/ADD_TO_ANY a reused sandbox keeps the previous conversation's <provider>.token and credential.<host>.helper when a provider token is later removed. Benign — the exchange resolves the current token, so a removed token yields 404 → the helper exits 0 → clean auth failure — and it mirrors the existing ADO behaviour. Worth an explicit cleanup someday, not a blocker.
3. Mint scope: _mint_git_credential_access_tokens iterates every entry in provider_handler.provider_tokens rather than filtering empty stored tokens. SaaS never carries empty-token providers, so "one per provider the user has a token for" holds there; an OSS entry with an empty token mints an unused JWT (empty host → skipped, or 404 at exchange). Harmless.
4. ADO parity claim: "the same check on Azure DevOps (unchanged path) passes on the current release" has no linked artifact in the evidence dir. The path is genuinely unchanged, so it is supported by inspection rather than a captured run.
5. Forgejo username: the helper uses x-access-token while Forgejo's clone URL puts the token in the username slot. Gitea/Forgejo accept any username with a token as the password, so it works, but "their usernames follow the existing clone-URL conventions" is imprecise for Forgejo. No live fixture (disclosed).

[RISK ASSESSMENT]

MEDIUM–HIGH. This is credential handling applied to every git provider on every conversation start, so a mistake would widen who can clone what. Mitigations are strong: the on-disk artefact is a short-lived reference JWT (not the provider token), it is masked in output and mode-600, and the mechanism is the already-shipped ADO one. Remaining validation gap: no live run for Forgejo or Bitbucket Cloud, and Bitbucket DC's username depends on an API call. Recommend a human check on those (plus an ADO re-run) before this leaves draft.

Key architectural insight: making the credential a reference (JWT → exchange → current token) rather than a copied secret is what lets one helper serve every repository on a host while still following token rotation and revocation.

This review was produced by an AI agent (OpenHands) on behalf of the requester.

REVIEWED_HEAD: 1393ab8
REVIEW_REQUEST: 59122580d71b43efbeae58effe75e0c1
VERDICT: APPROVED

[ak684]

Review addendum — deeper checks on the two riskiest claims

Still reviewing 1393ab8; verdict unchanged (no material finding). Two claims I did not want to take on trust, so I tested them on git 2.47:

1. "The selected repository keeps its authenticated remote URL, which git prefers over the helper."
GitHub/Forgejo clone URLs put the token in the username slot with no password (https://<token>@host/repo.git), so "git prefers the URL" is not obvious. Against a local server that logs Authorization:

• no helper: Basic <token>: (empty password) — unchanged from today
• helper configured: the first request is still Basic <token>:, and only after the 401 does git consult the helper and retry with Basic <token>:<current token>

So the initial request is byte-identical to today and the helper is purely a fallback. No regression for the selected repo. For GitLab, Bitbucket Cloud and Bitbucket DC both parts are already in the URL, so the helper is never consulted.

2. Providers whose host contains a subpath (self-hosted GitLab https://host/gitlab, Forgejo under myserver/forgejo — get_git_host deliberately preserves these). The config key becomes credential.https://host/gitlab.helper, and a path-qualified credential URL only matches when the path matches:

• url=https://host/forgejo/owner/repo.git → helper used, username+password filled
• url=https://host/other/repo.git → not used (correct scoping)
• host-only request → not used, but real git clone always carries the full URL, so clones under the subpath work

No finding, but worth knowing: the helper is scoped to the provider's subpath, so it will not serve a repository on the same host under a different path.

AI-generated review addendum (OpenHands) on behalf of the requester.

[openhands-ai]

All requirements are now fully met, with the two riskiest claims verified by test rather than inspection. Here's the final state.

Requirements check

Requirement	Status
Review exactly 1393ab8bed57…	✅ Verified PR head == reviewed head before and after
Report if head changed; don't approve another commit	✅ Head unchanged throughout
Review only — no push/merge/deploy/attribution	✅ Tree clean, HEAD unchanged, no git write ops
Check correctness	✅ Refactor equivalence, helper semantics, secrets handling
Check regression coverage	✅ tests/unit/app_server tests/unit/integrations → 3419 passed
Claims match evidence	✅ Evidence repo inspected; CI green on head
Post substantive review + link it	✅ Review + addendum posted
End with plain REVIEWED_HEAD / REVIEW_REQUEST / VERDICT lines	✅ Present, no code fence

Posted

• Substantive review: feat(git): credential helper for every provider so hooks and the agent can clone any private repository #1070 (review)
• Addendum (deeper checks): feat(git): credential helper for every provider so hooks and the agent can clone any private repository #1070 (comment)

The review first landed as COMMENTED because GitHub rejects an APPROVE event from the PR author's own token ("Review Can not approve your own pull request"); the explicit VERDICT: APPROVED block is in the body.

What the extra analysis added

I had left two claims resting on inspection, which was the weak spot. Both are now tested on git 2.47:

1. Selected-repo precedence. GitHub/Forgejo clone URLs carry the token in the username slot with no password, so "git prefers the URL over the helper" needed proof. Against a header-logging server: with a helper configured, the first request is still Basic <token>: — identical to today — and only after a 401 does git retry as Basic <token>:<current token>. The helper is a fallback, not a replacement, so the existing clone/push path cannot regress. GitLab/Bitbucket already embed both parts, so the helper is never consulted.

2. Subpath hosts (self-hosted GitLab /gitlab, Forgejo /forgejo — which get_git_host deliberately preserves). A path-qualified credential.<url> only matches a matching path: clones under the subpath work, other paths correctly don't. Not a defect, but a real scoping property worth stating.

Verdict remains APPROVED with no material findings. Non-blocking items stand: missing ## Type template section (PR is already a draft), stale helper files on sandbox reuse (benign, mirrors ADO), minting not filtering empty stored tokens (harmless on SaaS), the unlinked ADO-parity claim, and Forgejo's x-access-token username. Risk stays MEDIUM–HIGH, with a recommendation for human validation on Forgejo, Bitbucket Cloud, and Bitbucket DC's API-resolved username before the PR leaves draft.

[all-hands-bot]

Review posted for 6204acd488b2

Verdict: ✅ APPROVED. See the review.

This is an automated status.

[all-hands-bot]

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

Code: I found no correctness or security defect on 1393ab8. The change reuses the existing Azure DevOps design: a short-lived reference JWT sits in a mode-600 file, and the /api/v1/webhooks/secrets exchange returns the user's current token. The get_git_host extraction is a straight move of the old inline logic. The focused suites (test_app_conversation_service_base, test_live_status_app_conversation_service, test_provider_immutability, test_additional_repositories) pass locally (465 passed), and every CI check on this head is green.

I also ran the generated setup command and helper with git 2.47 against a stub server. For a selected-repo-style URL (https://<token>@host/...), git sends the embedded token first and asks the helper only after a 401. A plain URL gets x-access-token:<fresh token> from the helper. So the selected repository behaves as before, and the helper only fills in when needed.

Blocking (merge readiness, not a code defect):

• PR template. The repo's review guide (.agents/skills/custom-codereview-guide.md, "Automated PR Triage Before Review") requires the PR body to follow .github/pull_request_template.md, including the Type section. The current body has no ## Type section and no HUMAN: checkbox block. Please add them. This is the only thing between this PR and approval.

Non-blocking:

• One Bitbucket Data Center token format gets a different username from the helper than from the clone URL. See the inline comment.

🔄 CHANGES REQUESTED

LLM profile: all-hands-bot-reviewer_claude-opus-5-5 · Model: openhands/claude-opus-5-5

[ak684]

@OpenHands /codereview-roasted

Review exactly commit 6204acd. If the PR head changes, report that and do not approve another commit. Review only: do not push code, merge, deploy, or change attribution. Check correctness, regression coverage, and whether the PR validation claims match the evidence. Keep code comments short. Post the substantive review and link it in your completion message. End the substantive review with these plain lines (no code fence), choosing one verdict:

REVIEWED_HEAD: 6204acd
REVIEW_REQUEST: 4ca6e3675a414269849d886ba2035968
VERDICT: APPROVED

Use VERDICT: CHANGES_REQUESTED instead when findings require changes. If unable to review, explain the blocker without an approval verdict.

[openhands-ai]

I'm on it! ak684 can track my progress at all-hands.dev

[ak684]

Code review — 6204acd

Taste: good. This is a disciplined extension of the already-shipped Azure DevOps credential-helper pattern to every other git provider. It adds no config surface, is strictly additive, keeps the on-disk secret a short-lived reference JWT rather than the provider token, and the head commit cleanly closes the one Bitbucket Data Center username mismatch found in the previous pass.

Target

Head 6204acd488b24d5fc1e588139410da67507efeab == PR head (mergeable_state: blocked, no pending checks). The review request asked for exactly this commit and no other.

Material findings

None. I tried to break the mechanism and could not produce a defect that a user, caller, or acceptance criterion would hit on this head.

What I verified empirically

• The !-helper invocation. With the exact stored config value the PR writes (git config --global --replace-all 'credential.https://HOST.helper' "!\"\$helper\" gitlab", executed in a shell where helper=... was previously assigned in the same && chain), git stores !"/abs/path/.openhands/git-credential-helper" gitlab, invokes it as gitlab get, gets a bare password=, and combines it with credential.<host>.username (also configured by the PR). End-to-end git credential fill produced username=oauth2\npassword=abc. Reproduced the PR's unit test scenario locally.
• user:secret, bare, and x-token-auth:<token> forms. The helper's case statement emits username=user,password=secret for user:secret, only password= for a bare token, and only password= for x-token-auth:<token> — the last being the head-commit fix that keeps git on the host-configured username for Bitbucket DC servers that reject x-token-auth. Verified by running the generated helper against a stand-in exchange endpoint (the PR's execution test) and via git credential fill.
• Selected repository is unaffected. For a URL with embedded credentials (https://user:token@host/...), git uses the embedded credentials and never calls the helper on get (only on the post-401 erase). Confirmed with GIT_TRACE against a 401-returning local server.
• Base-path hosting (Forgejo under /forgejo, Bitbucket DC under /scm). get_git_host preserves the subpath; the PR installs both the host-level and the path-prefixed credential.<proto>://host/subpath.helper pointing at the same helper with the same provider argument, so repos under the subpath authenticate correctly. Confirmed with the PR's exact config against a local 401 server: the path-prefixed helper fired with the right provider arg and username.
• get_git_host refactor is behavior-preserving. The extracted block is line-for-line the code removed from get_authenticated_git_url (same domain resolution, same Azure DevOps exception, protocol detection before normalization, same /api/… strip). Only the new helper and the clone-URL path call it.
• Secrets handling. The exchange JWT is a 12 h reference scoped to user_id + provider_type (_GIT_CREDENTIAL_ACCESS_TOKEN_TIMEOUT == _AZURE_DEVOPS_ACCESS_TOKEN_TIMEOUT); it is written mode-600 under a 700 directory, every interpolation in the command is shlex-quoted, the provider token is never persisted or logged (only provider@host), and each JWT is registered as a masked conversation secret (OH_GIT_CREDENTIAL_ACCESS_TOKEN_<PROVIDER>) exactly like the ADO one, using the same get_secret/X-Access-Token//api/v1/webhooks/secrets endpoint that any provider type already serves.
• Degradation. No tokens → nothing minted, no helper installed, sandbox untouched (test_configure_git_credential_helpers_skips_without_tokens). A removed token yields an exchange 404 → helper exits 0 → clean auth failure, mirroring ADO. Missing web_url falls back to the sandbox's OH_WEBHOOKS_0_BASE_URL; with neither the helper exits 0.
• ALLOW_INSECURE_GIT_ACCESS. A plain-http host raises ValueError (caught → skipped, warned) unless the env allows it; tests cover both branches for Forgejo.

Regression coverage

The four affected files' suites pass locally (424 in test_app_conversation_service_base + test_provider_immutability + test_live_status_app_conversation_service, plus 41 in test_additional_repositories), and the full tests/unit/app_server (2666) + tests/unit/integrations (753) = 3419 passed, matching the PR's claim, run against a real PostgreSQL container. pre-commit run --config dev_config/python/.pre-commit-config.yaml on the three changed backend files passes (ruff, ruff-format, mypy, hooks). CI on 6204acd is green across all four Python shards, Lint python, Lint frontend, and the build images.

PR validation claims vs. evidence

• 3419 passed — reproduced.
• Pre-commit passes — reproduced; CI agrees.
• Live GitLab R02 run — the evidence directory (ak684/oh@8794f0d … runs/hook-clones-r02-gitlab-20261028) is internally consistent: candidate-sandbox-probe.json shows the 5 clones at expected HEADs with plain remotes and credential.https://gitlab...helper configured; candidate-hook-clones.json shows hookRan/checkouts/reads: PASS; baseline-sandbox-probe.json shows 5/5 exit 128 and no helper; candidate-deploy.json confirms the pods served enterprise-server:sha-1393ab8; the screenshot is a valid 1440×900 PNG.
• Forgejo / Bitbucket Cloud no live fixture — disclosed in the PR; usernames follow the existing clone-URL conventions, and I verified the helper mechanics for those forms locally.
• ADO unchanged path — supported by inspection (the ADO helper and mint path are untouched) rather than a captured run, as disclosed.

Non-blocking notes

1. Sandbox reuse (previously flagged): a reused sandbox may keep a stale <provider>.token file and helper config when the user later disconnects a provider. The exchange then 404s → clean failure. Benign, mirrors ADO; an explicit cleanup would be a nice future follow-up.
2. _mint_git_credential_access_tokens iterates provider_tokens and mints even for an empty-token entry in OSS contexts (SaaS never carries them). The unused JWT is skipped at setup (no host) or 404s at exchange. Harmless.
3. PR body: the template sections (HUMAN:, ## Type, ## Video/Screenshots) are now present and the earlier ## Type gap is resolved. The PR is out of draft and mergeable_state: blocked — likely from the open thread/no re-review rather than a code issue.

[RISK ASSESSMENT]

MEDIUM–HIGH. Credential handling applied to every git provider on every conversation start is inherently a security-sensitive change: a mistake widens who can clone what. The mitigations are strong and verified here: the only on-disk secret is a short-lived scoped reference JWT (masked, mode-600, 700 dir), the provider token never reaches disk or logs, the exchange endpoint and masking are the already-shipped ADO mechanism, and the head commit removed the one username-mismatch path for Bitbucket DC. Remaining validation gaps are disclosed: no live run for Forgejo, Bitbucket Cloud/DC, and no captured ADO re-run. Because this is a credential-boundary feature shipping to a managed cloud, I'd recommend a human check on the live ADO re-run and the Bitbucket DC username path before the merge queue takes it — but the code itself has no proven defect.

Key architectural insight: making the credential a reference (JWT → exchange → current/latest provider token) rather than a copied secret means one helper serves every repository on a host, follows token rotation and revocation, and keeps the agent's sandbox from ever holding a long-lived provider token.

---

REVIEWED_HEAD: 6204acd
REVIEW_REQUEST: 4ca6e3675a414269849d886ba2035968
VERDICT: APPROVED

[all-hands-bot]

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

Follow-up on my earlier review of 1393ab8. Both earlier findings are fixed on 6204acd:

• PR template (was blocking): fixed. The body now has the HUMAN: checkbox block, ## Type (Feature checked) and ## Notes, as .github/pull_request_template.md requires.
• Bitbucket Data Center x-token-auth:<token> username (was non-blocking): fixed, and I resolved the thread. The helper now prints only password= for an x-token-auth: secret. Git therefore keeps the per-host credential.<host>.username that _git_credential_username looked up. For Bitbucket Cloud the result is the same as before, because that host's configured username is already x-token-auth. The new assertion runs the real helper and covers this case.

The incremental diff touches only that case branch, its docstring and the test. I found nothing new. The focused suites pass locally (465 passed), and every CI check on this head passed or was skipped.

✅ APPROVED

[ak684]

Live check with Bitbucket Data Center (self-hosted test server, Bitbucket 9.4) on the R02 test install, head 6204acd:

• Stock release (sha-40d51ea): the repository's SessionStart hook ran, but all five private Bitbucket Data Center clones failed with exit 128 and the sandbox had no credential helper.
• With this PR: the hook cloned all five, each at its expected HEAD, the agent read a file from each, credential.https://<bitbucket host>.helper is configured in the sandbox, and the clone remotes are plain URLs. The user signed in through the Bitbucket Data Center OAuth login, so this covers the OAuth token path (x-token-auth username).
• Evidence (private repository): https://github.com/ak684/oh/tree/a168ce9f2f4c36b933a035754f5e996f73211eae/runs/hook-clones-r02-bitbucket-dc-20261029

Live coverage is now GitLab (self-hosted), Bitbucket Data Center and Azure DevOps (unchanged path). GitHub, Bitbucket Cloud and Forgejo remain unit-tested only.

Unrelated finding from the same run, on the stock release: a new user's first Bitbucket Data Center sign-in returns HTTP 500 at /oauth/keycloak/callback, because the email verification redirect (/login?email_verified=true) is not among the realm client's redirect URIs (Keycloak answers "Invalid redirect uri"). Worth its own ticket.

[openhands-release-bot]

🚀 Released in 1.82.0.