mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(source-control): align dirty+unavailable intent expectation
Local preparation (stage/commit changes) is safe without review-lookup authority; remote actions stay blocked. Prevents dirty trees from dead-ending at sync-first when lookup is unavailable.
This commit is contained in:
@@ -1579,25 +1579,12 @@ function SourceControlInner(): React.JSX.Element {
|
||||
remoteInferredHostedReviewProvider
|
||||
]
|
||||
)
|
||||
const unavailableHostedReviewProvider =
|
||||
hostedReview?.provider ??
|
||||
hostedReviewCreation?.provider ??
|
||||
(linkedGitLabMR != null
|
||||
? 'gitlab'
|
||||
: linkedAzureDevOpsPR != null
|
||||
? 'azure-devops'
|
||||
: linkedGiteaPR != null
|
||||
? 'gitea'
|
||||
: linkedGitHubPR != null || fallbackGitHubPRNumber != null
|
||||
? 'github'
|
||||
: linkedBitbucketPR != null
|
||||
? 'bitbucket'
|
||||
: remoteInferredHostedReviewProvider)
|
||||
const resolveCurrentHostedReviewCreationProvider = useEffectEvent(() =>
|
||||
resolveHostedReviewCreationProviderForTarget(
|
||||
hostedReviewCreationProviderHintRef.current,
|
||||
{ repoId: activeRepoId, worktreeId: activeWorktreeId ?? null, branch: branchName },
|
||||
unavailableHostedReviewProvider ?? 'unsupported'
|
||||
// Why: provisional already infers the remote host and defaults to github; never fall back to unsupported mid-load.
|
||||
provisionalHostedReviewProvider
|
||||
)
|
||||
)
|
||||
useEffect(() => {
|
||||
@@ -1639,7 +1626,7 @@ function SourceControlInner(): React.JSX.Element {
|
||||
const provider = resolveHostedReviewCreationProviderForTarget(
|
||||
hostedReviewCreationProviderHintRef.current,
|
||||
{ repoId: activeRepoId, worktreeId: activeWorktreeId ?? null, branch: branchName },
|
||||
unavailableHostedReviewProvider ?? provisionalHostedReviewProvider
|
||||
provisionalHostedReviewProvider
|
||||
)
|
||||
return buildLoadingHostedReviewCreationEligibility(provider)
|
||||
}
|
||||
@@ -1650,8 +1637,7 @@ function SourceControlInner(): React.JSX.Element {
|
||||
branchName,
|
||||
hostedReviewCreation,
|
||||
isHostedReviewCreationLoading,
|
||||
provisionalHostedReviewProvider,
|
||||
unavailableHostedReviewProvider
|
||||
provisionalHostedReviewProvider
|
||||
])
|
||||
const hasHostedReviewLink = hasPositiveHostedReviewNumberLink({
|
||||
linkedGitHubPR,
|
||||
@@ -3691,6 +3677,9 @@ function SourceControlInner(): React.JSX.Element {
|
||||
})
|
||||
} catch (error) {
|
||||
console.warn('[SourceControl] Create PR intent eligibility failed', error)
|
||||
// Why: when local status still yields a prep step (dirty/push/sync), keep the intent
|
||||
// moving. If nothing actionable can be synthesized, rethrow so the outer intent
|
||||
// catch surfaces a retry notice instead of leaving "Preparing…" stuck forever.
|
||||
const fallback = buildCreatePrIntentUnavailableEligibility(token.provider, {
|
||||
branch: token.branch,
|
||||
baseRef: token.baseRef,
|
||||
@@ -3700,7 +3689,7 @@ function SourceControlInner(): React.JSX.Element {
|
||||
behind: upstreamStatus?.behind
|
||||
})
|
||||
if (!fallback) {
|
||||
return null
|
||||
throw error
|
||||
}
|
||||
result = fallback
|
||||
}
|
||||
@@ -3775,7 +3764,9 @@ function SourceControlInner(): React.JSX.Element {
|
||||
worktreeId: activeWorktreeId,
|
||||
worktreePath,
|
||||
branch: branchName,
|
||||
provider: unavailableHostedReviewProvider ?? 'unsupported',
|
||||
// Why: token carries the same provisional provider used for UI copy so a failed
|
||||
// eligibility IPC can synthesize local prep steps for the correct host.
|
||||
provider: provisionalHostedReviewProvider,
|
||||
// Why: intent crosses async commit/push steps, so the base stays tied to what was selected when the run started.
|
||||
baseRef: effectiveBaseRef ?? null
|
||||
})
|
||||
@@ -3981,7 +3972,17 @@ function SourceControlInner(): React.JSX.Element {
|
||||
hasUncommittedChanges: latestStatusEntries.length > 0,
|
||||
upstreamStatus: latestUpstreamStatus
|
||||
})
|
||||
if (abortIfStale() || !eligibility) {
|
||||
if (abortIfStale()) {
|
||||
return
|
||||
}
|
||||
if (!eligibility) {
|
||||
setCreatePrIntentNoticeForWorktree(token.worktreeId, {
|
||||
tone: 'destructive',
|
||||
message: translate(
|
||||
'auto.components.right.sidebar.SourceControl.d7492cafce',
|
||||
'Could not refresh Source Control. Retry Create PR.'
|
||||
)
|
||||
})
|
||||
return
|
||||
}
|
||||
if (shouldAttemptCreateHostedReviewForIntent(eligibility)) {
|
||||
@@ -4094,12 +4095,16 @@ function SourceControlInner(): React.JSX.Element {
|
||||
}
|
||||
return
|
||||
}
|
||||
// Why: prefer the blocked-reason notice (incl. unavailable lookup) over a generic stop.
|
||||
const blockedNotice = resolveBlockedCreateReviewNoticeMessage(eligibility)
|
||||
setCreatePrIntentNoticeForWorktree(token.worktreeId, {
|
||||
tone: 'muted',
|
||||
message: translate(
|
||||
'auto.components.right.sidebar.SourceControl.995c5e67ec',
|
||||
'Review setup needs attention.'
|
||||
)
|
||||
tone: blockedNotice ? 'destructive' : 'muted',
|
||||
message:
|
||||
blockedNotice ??
|
||||
translate(
|
||||
'auto.components.right.sidebar.SourceControl.995c5e67ec',
|
||||
'Review setup needs attention.'
|
||||
)
|
||||
})
|
||||
} catch (error) {
|
||||
console.warn('[SourceControl] Create PR intent failed', error)
|
||||
@@ -4146,10 +4151,10 @@ function SourceControlInner(): React.JSX.Element {
|
||||
readHostedReviewCreationEligibilityForIntent,
|
||||
refreshGitStatusForCreatePrIntent,
|
||||
refreshBranchCompareForCreatePrIntent,
|
||||
provisionalHostedReviewProvider,
|
||||
remoteStatus,
|
||||
runRemoteAction,
|
||||
setCreatePrIntentNoticeForWorktree,
|
||||
unavailableHostedReviewProvider,
|
||||
updateCommitDrafts,
|
||||
worktreePath
|
||||
])
|
||||
|
||||
+8
-10
@@ -68,16 +68,14 @@ export function resolveHostedReviewCreationProviderForTarget(
|
||||
* times out, so the UI can show branch guidance without treating the failed
|
||||
* lookup as authority to create a review.
|
||||
*
|
||||
* Mirrors the main process's own lookup-failure fallback exactly — both its
|
||||
* `canReturnLocalBlocker` guard and its blocker ordering
|
||||
* (`src/main/source-control/hosted-review-creation.ts`). The guard is
|
||||
* load-bearing: without it a failed probe on the default branch or a detached
|
||||
* HEAD would synthesize `dirty`/`commit` and surface an *enabled* Create PR
|
||||
* that commits onto the base branch, where the real probe returns
|
||||
* `default_branch`/`detached_head` (disabled). Returns null when no local
|
||||
* blocker is determinable (unknown upstream, ahead-only, fully synced) so the
|
||||
* caller can surface the retryable state — matching main, which won't offer a
|
||||
* push it can't first auth-check.
|
||||
* Blocker ordering matches main (`dirty` → `no_upstream` → `needs_sync`). The
|
||||
* branch/base guard is load-bearing and intentionally stricter than main when
|
||||
* base is unknown: without a known base, a failed probe must not synthesize
|
||||
* `dirty`/`commit` on what might be the default branch (main would return
|
||||
* `default_branch`/`detached_head` and keep Create disabled). Returns null
|
||||
* when no local blocker is determinable (unknown base, unknown upstream,
|
||||
* ahead-only, fully synced) so the caller can surface the retryable state —
|
||||
* matching main, which won't offer a push it can't first auth-check.
|
||||
*/
|
||||
export function buildLocalBlockerHostedReviewCreationEligibility(
|
||||
provider: HostedReviewProvider,
|
||||
|
||||
@@ -35,6 +35,38 @@ describe('resolveCreateReviewIntentEligibility', () => {
|
||||
).toEqual({ eligible: false, kind: null })
|
||||
})
|
||||
|
||||
it('rejects unavailable eligibility when the default branch is blank', () => {
|
||||
expect(
|
||||
resolveCreateReviewIntentEligibility({
|
||||
stagedCount: 1,
|
||||
hasStageableChanges: true,
|
||||
hasMessage: true,
|
||||
hasUnresolvedConflicts: false,
|
||||
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
|
||||
hostedReviewCreation: {
|
||||
...unavailableEligibility('dirty'),
|
||||
defaultBaseRef: ' '
|
||||
}
|
||||
})
|
||||
).toEqual({ eligible: false, kind: null })
|
||||
})
|
||||
|
||||
it('stays ineligible when no local blocker remains under unavailable lookup', () => {
|
||||
expect(
|
||||
resolveCreateReviewIntentEligibility({
|
||||
stagedCount: 0,
|
||||
hasStageableChanges: false,
|
||||
hasMessage: true,
|
||||
hasUnresolvedConflicts: false,
|
||||
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
|
||||
hostedReviewCreation: {
|
||||
...unavailableEligibility('dirty'),
|
||||
blockedReason: null
|
||||
}
|
||||
})
|
||||
).toEqual({ eligible: false, kind: null })
|
||||
})
|
||||
|
||||
it('keeps dirty local preparation eligible when review lookup is unavailable', () => {
|
||||
expect(
|
||||
resolveCreateReviewIntentEligibility({
|
||||
|
||||
Reference in New Issue
Block a user