feat(bitbucket) blobless wrapper - #4872
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe new commit replaces the inline capability-prefix check with Files Reviewed (2 files)
Previous Review Summary (commit 763d8b7)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 763d8b7)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by claude-sonnet-5 · Input: 28 · Output: 4.2K · Cached: 687.3K Review guidance: REVIEW.md from base branch |
pandemicsyn
left a comment
pandemicsyn
left a comment
There was a problem hiding this comment.
one nit, and one potential issue (probably fine as follow up) inline.
| // A capability origin stays authenticated through the outbound interceptor and | ||
| // is safe to expose (scoped to one repo, useless outside this container), so it | ||
| // must stay in place for a blobless clone's later lazy blob fetches. Only a raw | ||
| // workspace token needs stripping. Either way this is a handled code-review | ||
| // remote (return true), so callers do not refresh a token over it. | ||
| if ( |
There was a problem hiding this comment.
sanitizeBitbucketCodeReviewRemote re-implements the capability-prefix check inline (request.repo.token.startsWith(BITBUCKET_CAPABILITY_PREFIX)) instead of reusing the newly added hasBitbucketReviewCapability helper, even though isBitbucketReviewSession has already narrowed request to the same shape the helper expects.
Suggested fix: Replace the inline typeof request.repo.token === 'string' && request.repo.token.startsWith(BITBUCKET_CAPABILITY_PREFIX) check with a call to hasBitbucketReviewCapability(request) to keep the single source of truth for what counts as a capability token.
| // workspace token needs stripping. Either way this is a handled code-review | ||
| // remote (return true), so callers do not refresh a token over it. | ||
| if ( | ||
| typeof request.repo.token === 'string' && |
There was a problem hiding this comment.
Related but separate from the DRY nit above: services/cloud-agent-next/src/session-service.ts has its own sanitizeBitbucketCodeReviewRemote (~lines 2610-2621) that isn't touched by this PR and has no equivalent capability check — it unconditionally calls updateGitRemoteUrl for any Bitbucket code-review session. That path runs whenever requiresPreparedDevcontainerRuntime is true (devcontainer-requested or dind- sandboxes), which is independent of createdOnPlatform === 'code-review', so a devcontainer/dind Bitbucket review session carrying a kbb1. capability token would have it stripped there instead of preserved. Suggest mirroring this capability check (or extracting a shared hasBitbucketReviewCapability-style helper) into session-service.ts as a follow-up so the optimization doesn't silently regress for that overlap.
…apability(request) to keep the single source of truth for what counts as a capability token.
… sanitize (#4876) The worker-side sanitizeBitbucketCodeReviewRemote unconditionally stripped the bitbucket origin credential for any code-review session. Mirror the wrapper: skip the strip when the session is credential-contained, so a kbb1. capability origin stays authenticated for a blobless clone's lazy fetches instead of being wiped. Adds cold- and warm-path tests (per #4872 review) asserting a contained bitbucket review does not strip the origin and does not run the warm-resume token refresh. Removing the guard makes both tests fail, confirming they cover the regression.
2 participants
Summary
Completes blobless partial clone support for Bitbucket code review in the wrapper. GitHub and GitLab already use a blobless clone to stay under the clone timeout on large repositories. This extends the same treatment to Bitbucket, but only when the session origin carries an outbound capability (the
kbb1.prefix), because a capability origin stays authenticated through the outbound interceptor and can serve the lazy blob fetches a partial clone defers. A raw-token Bitbucket origin is credential-stripped after bootstrap, so it still gets a full clone.This pairs with the Bitbucket outbound credential injection in #4868 (already merged), which mints those capabilities.
Changes
isBloblessReviewCloneEligiblenow returns true for a Bitbucket review session when the origin token is akbb1.capability, in addition to the existing GitHub and GitLab cases.hasBitbucketReviewCapabilityand theBITBUCKET_CAPABILITY_PREFIX(kbb1.) constant.sanitizeBitbucketCodeReviewRemoteskips stripping the origin when it holds a capability, so the credential stays available for the partial clone's later lazy blob fetches. A raw workspace token is still stripped as before.useBloblesscomment, which previously said only GitHub/GitLab qualify and described history retention incorrectly.Verification
Manually confirmed that Bitbucket Cloud (
bitbucket.org) honorsgit clone --filter=blob:none: a partial clone of a public bitbucket.org repo showed the server withholding blobs (promised objects, fetched lazily), with no "filtering not recognized by server" warning. This was the load-bearing assumption for the change.No end-to-end contained-session run yet. The path is dormant until the
BITBUCKET_TOKEN_CONTAINMENT_ORG_IDSallow-list enables an org, so a live Bitbucket review will be smoke-tested after enabling post-deploy. Automated coverage: 49 wrapper session-bootstrap tests pass, including the new capability blobless case.Visual Changes
N/A
Reviewer Notes
kbb1.capability. Without a capability, the origin is credential-stripped and a partial clone's lazy fetches would fail, so those sessions keep a full clone.sanitizeBitbucketCodeReviewRemote(it is scoped to one repo and useless outside the container), so blob fetches during the review authenticate through the outbound interceptor.--filteroutright, the clone retries once as a full clone, so a review is never lost to the optimization.